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

2018-04-05 Thread 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 
> ---
>  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 
> +#include 
>  
>  #include 
>  #include 
> @@ -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) {
> + 

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

2018-03-26 Thread Benoit Parrot
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.

Signed-off-by: Benoit Parrot 
---
 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 
+#include 
 
 #include 
 #include 
@@ -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