[Patch v2 5/6] drm/omap: Add virtual plane support to omap_plane

[Tomi Valkeinen]

On 26/03/18 19:21, Benoit Parrot wrote:
> Add virtual wide plane support by adding an secondary
> plane_id so that an "omap_plane" can be composed of up to
> two physical planes.
> 
> When at least one 'plane' child node is present in DT then
> omap_plane_init will only use the plane described in DT.
> Some of these nodes may be a virtual wide plane if they are defined
> as two physical planes.
> Planes can also be associated with various crtcs independently.
> Therefore we can restrict the use of virtual plane to specific
> CRTC/video port if need be, if crtc_mask is not specified then
> the plane will be available to all available crtcs.
> Physical planes which are not described will essentially be hidden
> from the driver.
> 
> If no 'plane' child nodes exist then the normal plane
> allocation will take place.

The descs in many of the patches are a bit messy. How do you format
them? At least vim's autoformat gives a more consistent wrapping.

> Signed-off-by: Benoit Parrot <bparrot at ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c   | 127 +++++++++++++++++++++++++++--
>  drivers/gpu/drm/omapdrm/omap_fb.c    |  66 ++++++++++-----
>  drivers/gpu/drm/omapdrm/omap_fb.h    |   4 +-
>  drivers/gpu/drm/omapdrm/omap_plane.c | 151 ++++++++++++++++++++++++++---------
>  drivers/gpu/drm/omapdrm/omap_plane.h |   4 +-
>  5 files changed, 283 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 37ee20c773c7..4c43ef239136 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -16,6 +16,7 @@
>   */
>  
>  #include <linux/sys_soc.h>
> +#include <linux/sort.h>
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -116,6 +117,112 @@ static void omap_atomic_commit_tail(struct drm_atomic_state *old_state)
>  	priv->dispc_ops->runtime_put();
>  }
>  
> +static int omap_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;
> +}
> +
> +static int omap_atomic_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, inc, n = 0;
> +	int ret = 0;
> +
> +	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> +			 crtc->base.id, crtc->name);
> +
> +	states = kmalloc_array(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), omap_atomic_state_zpos_cmp, NULL);
> +
> +	for (inc = 0, i = 0; i < n; i++) {
> +		plane = states[i]->plane;
> +
> +		states[i]->normalized_zpos = i + inc;
> +		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
> +				 plane->base.id, plane->name, i + inc);
> +		/*
> +		 * If the current plane is virtual it uses 2 hw planes
> +		 * therefore the very next zpos is used by the secondary/aux
> +		 * plane so we need to skip one zpos from this point on.
> +		 */
> +		if (is_omap_plane_virtual(plane))
> +			inc++;
> +	}
> +	crtc_state->zpos_changed = true;
> +
> +done:
> +	kfree(states);
> +	return ret;
> +}
> +
> +static int omap_atomic_normalize_zpos(struct drm_device *dev,
> +				      struct drm_atomic_state *state)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> +	int i, ret = 0;
> +
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> +		if (old_crtc_state->plane_mask != new_crtc_state->plane_mask ||
> +		    new_crtc_state->zpos_changed) {
> +			ret = omap_atomic_crtc_normalize_zpos(crtc,
> +							      new_crtc_state);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int omap_atomic_check(struct drm_device *dev,
> +			     struct drm_atomic_state *state)
> +{
> +	int ret;
> +
> +	ret = drm_atomic_helper_check(dev, state);
> +	if (ret)
> +		return ret;
> +
> +	if (dev->mode_config.normalize_zpos) {
> +		ret = omap_atomic_normalize_zpos(dev, state);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs = {
>  	.atomic_commit_tail = omap_atomic_commit_tail,
>  };
> @@ -123,7 +230,7 @@ static const struct drm_mode_config_helper_funcs omap_mode_config_helper_funcs =
>  static const struct drm_mode_config_funcs omap_mode_config_funcs = {
>  	.fb_create = omap_framebuffer_create,
>  	.output_poll_changed = drm_fb_helper_output_poll_changed,
> -	.atomic_check = drm_atomic_helper_check,
> +	.atomic_check = omap_atomic_check,
>  	.atomic_commit = drm_atomic_helper_commit,
>  };

I don't think it's a good idea to copypaste
drm_atomic_helper_crtc_normalize_zpos from the drm core. With Peter's
patches, the drm core does the normalizing, doesn't it? So they are
already sorted properly, and you can just go through them and adjust the
zpos if there are virtual planes.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki