On 06/08/2016 08:15 AM, Brian Starkey wrote:
> Hi Laura,
>
> On Mon, Jun 06, 2016 at 11:23:27AM -0700, Laura Abbott wrote:
>> The ABI for Ion's ioctl interface are a pain to work with. The heap IDs
>> are a 32-bit non-discoverable namespace that form part of the ABI. There's
>> no way to determine what ABI version is in use which leads to problems
>> if the ABI changes or needs to be updated.
>>
>> This series is a first approach to give a better ABI for Ion. This includes:
>>
>> - Following the advice in botching-up-ioctls.txt
>> - Ioctl for ABI version
>> - Dynamic assignment of heap ids
>> - queryable heap ids
>> - Runtime mapping of heap ids, including fallbacks. This avoids the need to
>> encode the fallbacks as an ABI.
>>
>> I'm most interested in feedback if this ABI is actually an improvement and
>> usable. The heap id map/query interface seems error prone but I didn't have
>> a cleaner solution. There aren't any kernel APIs for the new features as the
>> focus was on a userspace API but I anticipate that following easily once
>> the userspace API is established.
>>
>
> If I understand it right, the big improvement here is that userspace can
> find out what heap IDs are available, and what type of heap they
> correspond to? This seems good.
>
> I'm not sure about how userspace is meant to use the usage mappings
> though. Who calls ION_IOC_ID_MAP?
>
The thought was daemons (surfaceflinger, mediaserver etc.) would set this
up.
> (also, map/mapping is pretty overloaded. How about groups/groupings?)
>
That's a good suggestion.
> If I assume that the thing calling ION_IOC_ID_MAP is the same thing
> doing the allocating, then there doesn't seem to be much need for
> creating mappings. The combined mapper/allocator would necessarily need
> some knowledge about which types can satisfy which usage, and so could
> follow something like this:
> 1. The heaps can be queried, finding their IDs and types
> 2. A mask of heap IDs can be created which satisfies a "usage", based
> on the queried types
> 3. Allocation operations can then simply use this constructed mask
>
> On the other hand, if I assume that the thing doing the allocating is
> different to the thing doing the usage mapping (i.e. the allocator
> doesn't need to know about heap _types_, only usages); then I can't see
> a way for the allocator to figure out which usage_id it's meant to
> allocate with - which puts it right back to the old problem of opaque
> heap IDs (-> opaque usage_ids), except it's worse because they can
> change dynamically.
>
I see your point about the mapping. My thought was that whoever was doing
the mapping would have a way to pass information to the allocator or the
allocator could do a query. Relying on the passing of information may
not be plausible and I realize there isn't enough information from a query
to determine what usage id to use.
I like the suggestion of just using the query. One of
the goals I initially had with this series was to get rid of the heap mask.
The 32-bit heap mask became a name space that needed to be controlled
across all targets and the fallbacks were difficult to change. The problem
that actually wants to be solved is giving userspace enough information
to determine what heap masks to allocate from.
Just a query ioctl should be able to give that information as long as the
requirement is that userspace clients match on the heap name + type only
and not rely on the heap ids being constant (I don't think I made that
clear with this version of the query ioctl so I'll make sure to clarify).
The platform registration can adjust the priority order of the heap ids
as necessary. Different names should be able to take care of any changes
to the platform configuration.
I'll think about this more when I work on v2.
Thanks,
Laura
> Thanks,
> Brian
>
>>
>> Thanks,
>> Laura
>>
>> P.S. Not to turn this into a bike shedding session but if you have suggestions
>> for a name for this framework other than Ion I would be interested to hear
>> them. Too many other things are already named Ion.
>>
>> Laura Abbott (6):
>> staging: android: ion: return error value for ion_device_add_heap
>> staging: android: ion: Switch to using an idr to manage heaps
>> staging: android: ion: Drop heap type masks
>> staging: android: ion: Pull out ion ioctls to a separate file
>> staging: android: ion: Add an ioctl for ABI checking
>> staging: android: ion: Introduce new ioctls for dynamic heaps
>>
>> drivers/staging/android/ion/Makefile | 3 +-
>> drivers/staging/android/ion/ion-ioctl.c | 243 ++++++++++++++++++
>> drivers/staging/android/ion/ion.c | 438 ++++++++++++++++----------------
>> drivers/staging/android/ion/ion_priv.h | 109 +++++++-
>> drivers/staging/android/uapi/ion.h | 164 +++++++++++-
>> 5 files changed, 728 insertions(+), 229 deletions(-)
>> create mode 100644 drivers/staging/android/ion/ion-ioctl.c
>>
>> -- 2.5.5
The ABI for Ion's ioctl interface are a pain to work with. The heap IDs
are a 32-bit non-discoverable namespace that form part of the ABI. There's
no way to determine what ABI version is in use which leads to problems
if the ABI changes or needs to be updated.
This series is a first approach to give a better ABI for Ion. This includes:
- Following the advice in botching-up-ioctls.txt
- Ioctl for ABI version
- Dynamic assignment of heap ids
- queryable heap ids
- Runtime mapping of heap ids, including fallbacks. This avoids the need to
encode the fallbacks as an ABI.
I'm most interested in feedback if this ABI is actually an improvement and
usable. The heap id map/query interface seems error prone but I didn't have
a cleaner solution. There aren't any kernel APIs for the new features as the
focus was on a userspace API but I anticipate that following easily once
the userspace API is established.
Thanks,
Laura
P.S. Not to turn this into a bike shedding session but if you have suggestions
for a name for this framework other than Ion I would be interested to hear
them. Too many other things are already named Ion.
Laura Abbott (6):
staging: android: ion: return error value for ion_device_add_heap
staging: android: ion: Switch to using an idr to manage heaps
staging: android: ion: Drop heap type masks
staging: android: ion: Pull out ion ioctls to a separate file
staging: android: ion: Add an ioctl for ABI checking
staging: android: ion: Introduce new ioctls for dynamic heaps
drivers/staging/android/ion/Makefile | 3 +-
drivers/staging/android/ion/ion-ioctl.c | 243 ++++++++++++++++++
drivers/staging/android/ion/ion.c | 438 ++++++++++++++++----------------
drivers/staging/android/ion/ion_priv.h | 109 +++++++-
drivers/staging/android/uapi/ion.h | 164 +++++++++++-
5 files changed, 728 insertions(+), 229 deletions(-)
create mode 100644 drivers/staging/android/ion/ion-ioctl.c
--
2.5.5
On 06/06/2016 11:59 PM, Chen Feng wrote:
> The idea is good, define the heap ids in header file is inconvenient.
>
> But if we query the heaps information from user-space.
>
> It need to maintain this ids and name userspace one by one. The code may
> be complicated in different module user-space.
>
> In android, the gralloc and other lib will all use ion to alloc memory.
>
> This will make it more difficult to maintain user-space code.
>
This was a concern I had had as well. Anything that involves dynamic
probing is going to involve more tracking. Do you think some library
code in libion would help?
Thanks,
Laura
>
> But beyond this, The new alloc2 with not-handle flag is good.
> And the pull out of ioctl interface is also a good cleanup.
>
> On 2016/6/7 2:23, Laura Abbott wrote:
>>
>> The ABI for Ion's ioctl interface are a pain to work with. The heap IDs
>> are a 32-bit non-discoverable namespace that form part of the ABI. There's
>> no way to determine what ABI version is in use which leads to problems
>> if the ABI changes or needs to be updated.
>>
>> This series is a first approach to give a better ABI for Ion. This includes:
>>
>> - Following the advice in botching-up-ioctls.txt
>> - Ioctl for ABI version
>> - Dynamic assignment of heap ids
>> - queryable heap ids
>> - Runtime mapping of heap ids, including fallbacks. This avoids the need to
>> encode the fallbacks as an ABI.
>>
>> I'm most interested in feedback if this ABI is actually an improvement and
>> usable. The heap id map/query interface seems error prone but I didn't have
>> a cleaner solution. There aren't any kernel APIs for the new features as the
>> focus was on a userspace API but I anticipate that following easily once
>> the userspace API is established.
>>
>>
>> Thanks,
>> Laura
>>
>> P.S. Not to turn this into a bike shedding session but if you have suggestions
>> for a name for this framework other than Ion I would be interested to hear
>> them. Too many other things are already named Ion.
>>
>> Laura Abbott (6):
>> staging: android: ion: return error value for ion_device_add_heap
>> staging: android: ion: Switch to using an idr to manage heaps
>> staging: android: ion: Drop heap type masks
>> staging: android: ion: Pull out ion ioctls to a separate file
>> staging: android: ion: Add an ioctl for ABI checking
>> staging: android: ion: Introduce new ioctls for dynamic heaps
>>
>> drivers/staging/android/ion/Makefile | 3 +-
>> drivers/staging/android/ion/ion-ioctl.c | 243 ++++++++++++++++++
>> drivers/staging/android/ion/ion.c | 438 ++++++++++++++++----------------
>> drivers/staging/android/ion/ion_priv.h | 109 +++++++-
>> drivers/staging/android/uapi/ion.h | 164 +++++++++++-
>> 5 files changed, 728 insertions(+), 229 deletions(-)
>> create mode 100644 drivers/staging/android/ion/ion-ioctl.c
>>
>
Hi,
This series cleans up Ion a bit to be more in line with existing standards for
caching and dma mapping.
The most controversial part of this is probably going to be the first patch.
Ion takes quite a few liberties with how the DMA APIs are used for cache
syncing. dma_sync_sg is used without calling dma_map first. There isn't a
good way to get the cache synchronization with the DMA APIs. The behavior of
Ion is closer to the DRM framework which uses its own private cache APIs for
synchronization so I took that approach.
Assuming the approach of the first patch is appropriate, the next two patches
are fairly simple. dma_buf added support for a sync ioctl. Ion has a similar
ioctl already so this fixes the Ion APIs to be compatible with the dma_buf
ioctl. My plan would be to put a timeline on deprecation for the old Ion
sync ioctl. The map_dma_buf calls were also missing calls to the underlying
DMA APIs so the final patch in the series adds the appropriate calls.
Feedback is appreciated.
Thanks,
Laura
Laura Abbott (3):
staging: ion: Move away from the DMA APIs for cache flushing
staging: ion: Add support for syncing with DMA_BUF_IOCTL_SYNC
staging: ion: Add dma_map/dma_unmap calls to dma_buf calls
drivers/staging/android/ion/Kconfig | 14 ++++-
drivers/staging/android/ion/Makefile | 3 +
drivers/staging/android/ion/ion-arm.c | 83 ++++++++++++++++++++++++
drivers/staging/android/ion/ion-arm64.c | 46 ++++++++++++++
drivers/staging/android/ion/ion-x86.c | 34 ++++++++++
drivers/staging/android/ion/ion.c | 84 +++++++++++++------------
drivers/staging/android/ion/ion_carveout_heap.c | 5 +-
drivers/staging/android/ion/ion_chunk_heap.c | 7 +--
drivers/staging/android/ion/ion_page_pool.c | 3 +-
drivers/staging/android/ion/ion_priv.h | 14 ++---
drivers/staging/android/ion/ion_system_heap.c | 5 +-
11 files changed, 235 insertions(+), 63 deletions(-)
create mode 100644 drivers/staging/android/ion/ion-arm.c
create mode 100644 drivers/staging/android/ion/ion-arm64.c
create mode 100644 drivers/staging/android/ion/ion-x86.c
--
2.5.5
Hi Linus,
A tiny pull request with mostly cosmetic changes in dma-buf for 4.7;
may I request you to please pull?
The following changes since commit 852f42a69b93dc71507adedeed876d57b8c2c2fa:
Merge branch 'uuid' (lib/uuid fixes from Andy) (2016-05-30 15:27:07 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/sumits/dma-buf.git
tags/dma-buf-for-4.7
for you to fetch changes up to b02da6f8236148009c22167cd7013d5ce04a2d37:
dma-buf: use vma_pages() (2016-05-31 22:17:05 +0530)
----------------------------------------------------------------
Minor dma-buf updates for 4.7:
- use of vma_pages instead of explicit computation.
- DocBook and headerdoc updates for dma-buf.
----------------------------------------------------------------
Luis de Bethencourt (1):
fence: add missing descriptions for fence
Muhammad Falak R Wani (1):
dma-buf: use vma_pages()
Rob Clark (3):
dma-buf: headerdoc fixes
reservation: add headerdoc comments
doc: update/fixup dma-buf related DocBook
Documentation/DocBook/device-drivers.tmpl | 36 ++++++++++++++--
drivers/dma-buf/dma-buf.c | 7 +--
drivers/dma-buf/reservation.c | 72 +++++++++++++++++++++++++++++--
include/linux/dma-buf.h | 13 ++++--
include/linux/fence.h | 2 +
include/linux/reservation.h | 53 +++++++++++++++++++++++
6 files changed, 169 insertions(+), 14 deletions(-)
Thanks and best regards,
Sumit.
From: Marek Szyprowski <m.szyprowski(a)samsung.com>
This patch adds support for generic plane's zpos property property with
well-defined semantics:
- added zpos properties to plane and plane state structures
- added helpers for normalizing zpos properties of given set of planes
- well defined semantics: planes are sorted by zpos values and then plane
id value if zpos equals
Normalized zpos values are calculated automatically when generic
muttable zpos property has been initialized. Drivers can simply use
plane_state->normalized_zpos in their atomic_check and/or plane_update
callbacks without any additional calls to DRM core.
Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com>
Compare to Marek's original patch zpos property is now specific to each
plane and no more to the core.
Normalize function take care of the range of per plane defined range
before set normalized_zpos.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard(a)linaro.org>
Cc: Inki Dae <inki.dae(a)samsung.com>
Cc: Daniel Vetter <daniel(a)ffwll.ch>
Cc: Ville Syrjala <ville.syrjala(a)linux.intel.com>
Cc: Joonyoung Shim <jy0922.shim(a)samsung.com>
Cc: Seung-Woo Kim <sw0312.kim(a)samsung.com>
Cc: Andrzej Hajda <a.hajda(a)samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski(a)samsung.com>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie(a)samsung.com>
Cc: Tobias Jakobi <tjakobi(a)math.uni-bielefeld.de>
Cc: Gustavo Padovan <gustavo(a)padovan.org>
Cc: vincent.abriou(a)st.com
Cc: fabien.dessenne(a)st.com
Cc: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com>
---
Documentation/DocBook/gpu.tmpl | 10 ++
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_atomic.c | 4 +
drivers/gpu/drm/drm_atomic_helper.c | 6 +
drivers/gpu/drm/drm_blend.c | 229 ++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_crtc_internal.h | 3 +
include/drm/drm_crtc.h | 28 +++++
7 files changed, 281 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_blend.c
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 9dd48f7..0df6abb 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2151,6 +2151,16 @@ void intel_crt_init(struct drm_device *dev)
the underlying hardware).</td>
</tr>
<tr>
+ <td valign="top" > "zpos" </td>
+ <td valign="top" >RANGE</td>
+ <td valign="top" >Min= driver dependent, Max= driver dependent</td>
+ <td valign="top" >Plane</td>
+ <td valign="top" >Plane's 'z' position during blending operation (0 for background, highest for frontmost).
+ If two planes assigned to same CRTC have equal zpos values, the plane with higher plane
+ id is treated as closer to front. Can be IMMUTABLE if driver doesn't support changing
+ planes' order. Exact value range is driver dependent.</td>
+ </tr>
+ <tr>
<td rowspan="20" valign="top" >i915</td>
<td rowspan="2" valign="top" >Generic</td>
<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 2bd3e5a..df88253 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -2,7 +2,7 @@
# Makefile for the drm device driver. This driver provides support for the
# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
-drm-y := drm_auth.o drm_bufs.o drm_cache.o \
+drm-y := drm_auth.o drm_bufs.o drm_blend.o drm_cache.o \
drm_context.o drm_dma.o \
drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \
drm_lock.o drm_memory.o drm_drv.o drm_vm.o \
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 86e89db..bd457ca 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -700,6 +700,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
state->src_h = val;
} else if (property == config->rotation_property) {
state->rotation = val;
+ } else if (property == plane->zpos_property) {
+ state->zpos = val;
} else if (plane->funcs->atomic_set_property) {
return plane->funcs->atomic_set_property(plane, state,
property, val);
@@ -756,6 +758,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_h;
} else if (property == config->rotation_property) {
*val = state->rotation;
+ } else if (property == plane->zpos_property) {
+ *val = state->zpos;
} else if (plane->funcs->atomic_get_property) {
return plane->funcs->atomic_get_property(plane, state, property, val);
} else {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 997fd21..f10652f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -32,6 +32,8 @@
#include <drm/drm_atomic_helper.h>
#include <linux/fence.h>
+#include "drm_crtc_internal.h"
+
/**
* DOC: overview
*
@@ -587,6 +589,10 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
struct drm_plane_state *plane_state;
int i, ret = 0;
+ ret = drm_atomic_helper_normalize_zpos(dev, state);
+ if (ret)
+ return ret;
+
for_each_plane_in_state(state, plane, plane_state, i) {
const struct drm_plane_helper_funcs *funcs;
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
new file mode 100644
index 0000000..eef59bb
--- /dev/null
+++ b/drivers/gpu/drm/drm_blend.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) 2016 Samsung Electronics Co.Ltd
+ * Authors:
+ * Marek Szyprowski <m.szyprowski(a)samsung.com>
+ *
+ * DRM core plane blending related functions
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission. The copyright holders make no representations
+ * about the suitability of this software for any purpose. It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_crtc.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <linux/sort.h>
+
+#include "drm_internal.h"
+
+/**
+ * drm_plane_create_zpos_property - create mutable zpos property
+ * @plane: drm plane
+ * @min: minimal possible value of zpos property
+ * @max: maximal possible value of zpos property
+ *
+ * This function initializes generic mutable zpos property and enables support
+ * for it in drm core. Drivers can then attach this property to planes to enable
+ * support for configurable planes arrangement during blending operation.
+ * Once mutable zpos property has been enabled, the DRM core will automatically
+ * calculate drm_plane_state->normalized_zpos values. Usually min should be set
+ * to 0 and max to maximal number of planes for given crtc - 1.
+ *
+ * If zpos of some planes cannot be changed (like fixed background or
+ * cursor/topmost planes), driver should adjust min/max values and assign those
+ * planes immutable zpos property with lower or higher values (for more
+ * information, see drm_mode_create_zpos_immutable_property() function). In such
+ * case driver should also assign proper initial zpos values for all planes in
+ * its plane_reset() callback, so the planes will be always sorted properly.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_plane_create_zpos_property(struct drm_plane *plane,
+ unsigned int min, unsigned int max)
+{
+ struct drm_property *prop;
+
+ prop = drm_property_create_range(plane->dev, 0, "zpos", min, max);
+ if (!prop)
+ return -ENOMEM;
+
+ drm_object_attach_property(&plane->base, prop, min);
+
+ plane->zpos_property = prop;
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_zpos_property);
+
+/**
+ * drm_plane_create_zpos_immutable_property - create immuttable zpos property
+ * @plane: drm plane
+ * @min: minimal possible value of zpos property
+ * @max: maximal possible value of zpos property
+ *
+ * This function initializes generic immutable zpos property and enables
+ * support for it in drm core. Using this property driver lets userspace
+ * to get the arrangement of the planes for blending operation and notifies
+ * it that the hardware (or driver) doesn't support changing of the planes'
+ * order.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_plane_create_zpos_immutable_property(struct drm_plane *plane,
+ unsigned int min, unsigned int max)
+{
+ struct drm_property *prop;
+
+ prop = drm_property_create_range(plane->dev, DRM_MODE_PROP_IMMUTABLE,
+ "zpos", min, max);
+ if (!prop)
+ return -ENOMEM;
+
+ drm_object_attach_property(&plane->base, prop, min);
+
+ plane->zpos_property = prop;
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_zpos_immutable_property);
+
+static int drm_atomic_state_zpos_cmp(const void *a, const void *b)
+{
+ const struct drm_plane_state *sa = *(struct drm_plane_state **)a;
+ const struct drm_plane_state *sb = *(struct drm_plane_state **)b;
+
+ if (sa->zpos != sb->zpos)
+ return sa->zpos - sb->zpos;
+ else
+ return sa->plane->base.id - sb->plane->base.id;
+}
+
+/**
+ * drm_atomic_helper_crtc_normalize_zpos - calculate normalized zpos values
+ * @crtc: crtc with planes, which have to be considered for normalization
+ * @crtc_state: new atomic state to apply
+ *
+ * This function checks new states of all planes assigned to given crtc and
+ * calculates normalized zpos value for them. Planes are compared first by their
+ * zpos values, then by plane id (if zpos equals). Plane with lowest zpos value
+ * is at the bottom. The plane_state->normalized_zpos is then filled with unique
+ * values from 0 to number of active planes in crtc minus one.
+ *
+ * RETURNS
+ * Zero for success or -errno
+ */
+int drm_atomic_helper_crtc_normalize_zpos(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state)
+{
+ struct drm_atomic_state *state = crtc_state->state;
+ struct drm_device *dev = crtc->dev;
+ int total_planes = dev->mode_config.num_total_plane;
+ struct drm_plane_state **states;
+ struct drm_plane *plane;
+ int i, zpos, n = 0;
+ int ret = 0;
+
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
+ crtc->base.id, crtc->name);
+
+ states = kmalloc(total_planes * sizeof(*states), GFP_KERNEL);
+ if (!states)
+ return -ENOMEM;
+
+ /*
+ * Normalization process might create new states for planes which
+ * normalized_zpos has to be recalculated.
+ */
+ drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
+ struct drm_plane_state *plane_state =
+ drm_atomic_get_plane_state(state, plane);
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ goto done;
+ }
+ states[n++] = plane_state;
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] processing zpos value %d\n",
+ plane->base.id, plane->name,
+ plane_state->zpos);
+ }
+
+ sort(states, n, sizeof(*states), drm_atomic_state_zpos_cmp, NULL);
+
+ for (i = 0, zpos = 0; i < n; i++, zpos++) {
+ plane = states[i]->plane;
+
+ zpos = max_t(int, zpos, states[i]->zpos);
+
+ states[i]->normalized_zpos = zpos;
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
+ plane->base.id, plane->name, zpos);
+ }
+ crtc_state->zpos_changed = true;
+
+done:
+ kfree(states);
+ return ret;
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_normalize_zpos);
+
+/**
+ * drm_atomic_helper_normalize_zpos - calculate normalized zpos values for all
+ * crtcs
+ * @dev: DRM device
+ * @state: atomic state of DRM device
+ *
+ * This function calculates normalized zpos value for all modified planes in
+ * the provided atomic state of DRM device. For more information, see
+ * drm_atomic_helper_crtc_normalize_zpos() function.
+ *
+ * RETURNS
+ * Zero for success or -errno
+ */
+int drm_atomic_helper_normalize_zpos(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ struct drm_plane *plane;
+ struct drm_plane_state *plane_state;
+ int i, ret = 0;
+
+ for_each_plane_in_state(state, plane, plane_state, i) {
+ crtc = plane_state->crtc;
+ if (!crtc)
+ continue;
+ if (plane->state->zpos != plane_state->zpos) {
+ crtc_state =
+ drm_atomic_get_existing_crtc_state(state, crtc);
+ crtc_state->zpos_changed = true;
+ }
+ }
+
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ if (crtc_state->plane_mask != crtc->state->plane_mask ||
+ crtc_state->zpos_changed) {
+ ret = drm_atomic_helper_crtc_normalize_zpos(crtc,
+ crtc_state);
+ if (ret)
+ return ret;
+ }
+ }
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_normalize_zpos);
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index a78c138..37d9068 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -42,3 +42,6 @@ int drm_atomic_get_property(struct drm_mode_object *obj,
int drm_mode_atomic_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv);
+/* drm_blend.c */
+int drm_atomic_helper_normalize_zpos(struct drm_device *dev,
+ struct drm_atomic_state *state);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d1559cd..921c93f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -305,6 +305,7 @@ struct drm_plane_helper_funcs;
* @mode_changed: crtc_state->mode or crtc_state->enable has been changed
* @active_changed: crtc_state->active has been toggled.
* @connectors_changed: connectors to this crtc have been updated
+ * @zpos_changed: zpos values of planes on this crtc have been updated
* @color_mgmt_changed: color management properties have changed (degamma or
* gamma LUT or CSC matrix)
* @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
@@ -340,6 +341,7 @@ struct drm_crtc_state {
bool mode_changed : 1;
bool active_changed : 1;
bool connectors_changed : 1;
+ bool zpos_changed : 1;
bool color_mgmt_changed : 1;
/* attached planes bitmask:
@@ -1263,6 +1265,9 @@ struct drm_connector {
* plane (in 16.16)
* @src_w: width of visible portion of plane (in 16.16)
* @src_h: height of visible portion of plane (in 16.16)
+ * @zpos: priority of the given plane on crtc (optional)
+ * @normalized_zpos: normalized value of zpos: unique, range from 0 to N-1
+ * for given crtc
* @state: backpointer to global drm_atomic_state
*/
struct drm_plane_state {
@@ -1283,6 +1288,10 @@ struct drm_plane_state {
/* Plane rotation */
unsigned int rotation;
+ /* Plane zpos */
+ unsigned int zpos;
+ unsigned int normalized_zpos;
+
struct drm_atomic_state *state;
};
@@ -1516,6 +1525,7 @@ enum drm_plane_type {
* @properties: property tracking for this plane
* @type: type of plane (overlay, primary, cursor)
* @state: current atomic state for this plane
+ * @zpos_property: zpos property for this plane
*/
struct drm_plane {
struct drm_device *dev;
@@ -1546,6 +1556,8 @@ struct drm_plane {
const struct drm_plane_helper_funcs *helper_private;
struct drm_plane_state *state;
+
+ struct drm_property *zpos_property;
};
/**
@@ -2554,6 +2566,22 @@ extern struct drm_property *drm_mode_create_rotation_property(struct drm_device
extern unsigned int drm_rotation_simplify(unsigned int rotation,
unsigned int supported_rotations);
+int drm_plane_atomic_set_zpos_property(struct drm_plane *plane,
+ struct drm_plane_state *state,
+ struct drm_property *property,
+ uint64_t val);
+
+int drm_plane_atomic_get_zpos_property(struct drm_plane *plane,
+ const struct drm_plane_state *state,
+ struct drm_property *property,
+ uint64_t *val);
+
+int drm_plane_create_zpos_property(struct drm_plane *plane,
+ unsigned int min, unsigned int max);
+int drm_plane_create_zpos_immutable_property(struct drm_plane *plane,
+ unsigned int min,
+ unsigned int max);
+
/* Helpers */
static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
--
1.9.1
On Mon, May 23, 2016 at 05:08:42PM +0530, Muhammad Falak R Wani wrote:
> Replace explicit computation of vma page count by a call to
> vma_pages().
> Also, include <linux/mm.h>
All good!
Reviewed-by: Eric Engestrom <eric.engestrom(a)imgtec.com>
On Sat, May 21, 2016 at 06:53:32PM +0530, Muhammad Falak R Wani wrote:
> Replace explicit computation of vma page count by a call to
> vma_pages()
>
> Signed-off-by: Muhammad Falak R Wani <falakreyaz(a)gmail.com>
> ---
Good change, but this function is defined in mm.h
Please add:
#include <linux/mm.h>
With that fixed:
Reviewed-by: Eric Engestrom <eric.engestrom(a)imgtec.com>
version 8 chanegs:
- rework of the structures used within ioctl
by adding a version field and padding to be futur proof
- rename fake secure moduel to test secure module
- fix the various remarks done on the previous patcheset
version 7 changes:
- rebased on kernel 4.6-rc7
- simplify secure module API
- add vma ops to be able to detect mmap/munmap calls
- add ioctl to get number and allocator names
- update libsmaf with adding tests
https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
- add debug log in fake secure module
version 6 changes:
- rebased on kernel 4.5-rc4
- fix mmapping bug while requested allocation size isn't a a multiple of
PAGE_SIZE (add a test for this in libsmaf)
version 5 changes:
- rebased on kernel 4.3-rc6
- rework locking schema and make handle status use an atomic_t
- add a fake secure module to allow performing tests without trusted
environment
version 4 changes:
- rebased on kernel 4.3-rc3
- fix missing EXPORT_SYMBOL for smaf_create_handle()
version 3 changes:
- Remove ioctl for allocator selection instead provide the name of
the targeted allocator with allocation request.
Selecting allocator from userland isn't the prefered way of working
but is needed when the first user of the buffer is a software component.
- Fix issues in case of error while creating smaf handle.
- Fix module license.
- Update libsmaf and tests to care of the SMAF API evolution
https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
version 2 changes:
- Add one ioctl to allow allocator selection from userspace.
This is required for the uses case where the first user of
the buffer is a software IP which can't perform dma_buf attachement.
- Add name and ranking to allocator structure to be able to sort them.
- Create a tiny library to test SMAF:
https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
- Fix one issue when try to secure buffer without secure module registered
The outcome of the previous RFC about how do secure data path was the need
of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551)
SMAF goal is to provide a framework that allow allocating and securing
memory by using dma_buf. Each platform have it own way to perform those two
features so SMAF design allow to register helper modules to perform them.
To be sure to select the best allocation method for devices SMAF implement
deferred allocation mechanism: memory allocation is only done when the first
device effectively required it.
Allocator modules have to implement a match() to let SMAF know if they are
compatibles with devices needs.
This patch set provide an example of allocator module which use
dma_{alloc/free/mmap}_attrs() and check if at least one device have
coherent_dma_mask set to DMA_BIT_MASK(32) in match function.
I have named smaf-cma.c like it is done for drm_gem_cma_helper.c even if
a better name could be found for this file.
Secure modules are responsibles of granting and revoking devices access rights
on the memory. Secure module is also called to check if CPU map memory into
kernel and user address spaces.
An example of secure module implementation can be found here:
http://git.linaro.org/people/benjamin.gaignard/optee-sdp.git
This code isn't yet part of the patch set because it depends on generic TEE
which is still under discussion (https://lwn.net/Articles/644646/)
For allocation part of SMAF code I get inspirated by Sumit Semwal work about
constraint aware allocator.
Benjamin Gaignard (3):
create SMAF module
SMAF: add CMA allocator
SMAF: add test secure module
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/smaf/Kconfig | 17 +
drivers/smaf/Makefile | 3 +
drivers/smaf/smaf-cma.c | 188 ++++++++++
drivers/smaf/smaf-core.c | 818 +++++++++++++++++++++++++++++++++++++++++
drivers/smaf/smaf-testsecure.c | 90 +++++
include/linux/smaf-allocator.h | 45 +++
include/linux/smaf-secure.h | 65 ++++
include/uapi/linux/smaf.h | 85 +++++
10 files changed, 1314 insertions(+)
create mode 100644 drivers/smaf/Kconfig
create mode 100644 drivers/smaf/Makefile
create mode 100644 drivers/smaf/smaf-cma.c
create mode 100644 drivers/smaf/smaf-core.c
create mode 100644 drivers/smaf/smaf-testsecure.c
create mode 100644 include/linux/smaf-allocator.h
create mode 100644 include/linux/smaf-secure.h
create mode 100644 include/uapi/linux/smaf.h
--
1.9.1
