Re: [PATCH v5 1/5] drm: add generic zpos property

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	linux-samsung-soc@vger.kernel.org,
	Benjamin Gaignard <benjamin.gaignard@linaro.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	dri-devel@lists.freedesktop.org,
	Andrzej Hajda <a.hajda@samsung.com>,
	Tobias Jakobi <tjakobi@math.uni-bielefeld.de>,
	fabien.dessenne@st.com, vincent.abriou@st.com
Subject: Re: [PATCH v5 1/5] drm: add generic zpos property
Date: Mon, 29 Feb 2016 17:12:19 +0200	[thread overview]
Message-ID: <20160229151219.GW15993@intel.com> (raw)
In-Reply-To: <1453905883-6807-2-git-send-email-m.szyprowski@samsung.com>

On Wed, Jan 27, 2016 at 03:44:39PM +0100, Marek Szyprowski wrote:
> This patch adds support for generic plane's zpos property property with
> well-defined semantics:
> - added zpos properties to drm core 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@samsung.com>
> Tested-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  Documentation/DocBook/gpu.tmpl      |  14 ++-
>  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         | 227 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_crtc_internal.h |   3 +
>  include/drm/drm_crtc.h              |  17 +++
>  7 files changed, 270 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_blend.c
> 
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index a8669330b456..ae7d913adf60 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -1816,7 +1816,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >Description/Restrictions</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="37" valign="top" >DRM</td>
> +	<td rowspan="38" valign="top" >DRM</td>
>  	<td valign="top" >Generic</td>
>  	<td valign="top" >“rotation”</td>
>  	<td valign="top" >BITMASK</td>
> @@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >property to suggest an Y offset for a connector</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="3" valign="top" >Optional</td>
> +	<td rowspan="4" valign="top" >Optional</td>
>  	<td valign="top" >“scaling mode”</td>
>  	<td valign="top" >ENUM</td>
>  	<td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
> @@ -2092,6 +2092,16 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> +	<td valign="top" > "zpos" </td>
> +	<td valign="top" >RANGE</td>
> +	<td valign="top" >Min=0, 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 61766dec6a8d..eeefaba1c092 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 3f74193885f1..a19201efb7d1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -630,6 +630,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 == config->zpos_property) {
> +		state->zpos = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -686,6 +688,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 == config->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 53db2d9120be..9fbfb672b317 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
>   *
> @@ -530,6 +532,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 000000000000..cdcb647c8ed9
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -0,0 +1,227 @@
> +/*
> + * Copyright (C) 2016 Samsung Electronics Co.Ltd
> + * Authors:
> + *	Marek Szyprowski <m.szyprowski@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 <linux/slab.h>
> +#include <linux/sort.h>
> +#include <linux/export.h>
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_atomic.h>
> +
> +#include "drm_internal.h"
> +
> +/**
> + * drm_mode_create_zpos_property - create muttable zpos property
> + * @dev: DRM device
> + * @min: minimal possible value of zpos property
> + * @max: maximal possible value of zpos property
> + *
> + * This function initializes generic muttable 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 muttable 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 backgroud 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_mode_create_zpos_property(struct drm_device *dev, unsigned int min,
> +				  unsigned int max)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create_range(dev, 0, "zpos", min, max);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	dev->mode_config.zpos_property = prop;
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_create_zpos_property);
> +
> +/**
> + * drm_mode_create_zpos_immutable_property - create immuttable zpos property
> + * @dev: DRM device
> + * @min: minimal possible value of zpos property
> + * @max: maximal possible value of zpos property
> + *
> + * This function initializes generic immuttable 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_mode_create_zpos_immutable_property(struct drm_device *dev,
> +					    unsigned int min, unsigned int max)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE, "zpos",
> +					 min, max);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	dev->mode_config.zpos_immutable_property = prop;

I want per-plane min/max ranges, so putting this here is no good.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel