| Message ID | 1345061882-9743-3-git-send-email-rob.clark@linaro.org |
|---|---|
| State | Accepted |
| Commit | 8451b5adae5ca9b5f022cfdd029b7a5b5283f2ce |
| Headers | show |
On Wed, Aug 15, 2012 at 03:18:02PM -0500, Rob Clark wrote: > From: Andre Renaud <andre@bluewatersys.com> > > Added support for zorder changes through DRM plane properties > > Signed-off-by: Andre Renaud <andre@bluewatersys.com> > Signed-off-by: Rob Clark <rob@ti.com> > --- > drivers/staging/omapdrm/omap_drv.h | 1 + > drivers/staging/omapdrm/omap_plane.c | 19 +++++++++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/staging/omapdrm/omap_drv.h b/drivers/staging/omapdrm/omap_drv.h > index b103d28..9dc72d1 100644 > --- a/drivers/staging/omapdrm/omap_drv.h > +++ b/drivers/staging/omapdrm/omap_drv.h > @@ -62,6 +62,7 @@ struct omap_drm_private { > > /* properties: */ > struct drm_property *rotation_prop; > + struct drm_property *zorder_prop; > }; > > /* this should probably be in drm-core to standardize amongst drivers */ > diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c > index 6931d06..4bde639 100644 > --- a/drivers/staging/omapdrm/omap_plane.c > +++ b/drivers/staging/omapdrm/omap_plane.c > @@ -433,6 +433,15 @@ void omap_plane_install_properties(struct drm_plane *plane, > priv->rotation_prop = prop; > } > drm_object_attach_property(obj, prop, 0); > + > + prop = priv->zorder_prop; > + if (!prop) { > + prop = drm_property_create_range(dev, 0, "zorder", 0, 3); > + if (prop == NULL) > + return; > + priv->zorder_prop = prop; > + } > + drm_object_attach_property(obj, prop, 0); > } > > int omap_plane_set_property(struct drm_plane *plane, > @@ -452,6 +461,16 @@ int omap_plane_set_property(struct drm_plane *plane, > ret = omap_plane_dpms(plane, DRM_MODE_DPMS_ON); > else > ret = 0; > + } else if (property == priv->zorder_prop) { > + struct omap_overlay *ovl = omap_plane->ovl; > + > + DBG("%s: zorder: %d", ovl->name, (uint32_t)val); > + omap_plane->info.zorder = val; What would happen when there's a conflicting assignment between two planes? I tried to think of a decent way to do this stuff, but some hardware can have rather complicated stacking order limitations. One idea I came up with was to have an enum prop on the crtc, where the individual enum value names would somehow describe the whole stacking order within the crtc. That way user space couldn't even try to use an unsupported configuration. The downside is that user space would need to parse those strings if it wants to do some automagic stacking order changes, which means the string format would need some though.
On Thu, Aug 16, 2012 at 8:00 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Aug 15, 2012 at 03:18:02PM -0500, Rob Clark wrote: >> From: Andre Renaud <andre@bluewatersys.com> >> >> Added support for zorder changes through DRM plane properties >> >> Signed-off-by: Andre Renaud <andre@bluewatersys.com> >> Signed-off-by: Rob Clark <rob@ti.com> >> --- >> drivers/staging/omapdrm/omap_drv.h | 1 + >> drivers/staging/omapdrm/omap_plane.c | 19 +++++++++++++++++++ >> 2 files changed, 20 insertions(+) >> >> diff --git a/drivers/staging/omapdrm/omap_drv.h b/drivers/staging/omapdrm/omap_drv.h >> index b103d28..9dc72d1 100644 >> --- a/drivers/staging/omapdrm/omap_drv.h >> +++ b/drivers/staging/omapdrm/omap_drv.h >> @@ -62,6 +62,7 @@ struct omap_drm_private { >> >> /* properties: */ >> struct drm_property *rotation_prop; >> + struct drm_property *zorder_prop; >> }; >> >> /* this should probably be in drm-core to standardize amongst drivers */ >> diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c >> index 6931d06..4bde639 100644 >> --- a/drivers/staging/omapdrm/omap_plane.c >> +++ b/drivers/staging/omapdrm/omap_plane.c >> @@ -433,6 +433,15 @@ void omap_plane_install_properties(struct drm_plane *plane, >> priv->rotation_prop = prop; >> } >> drm_object_attach_property(obj, prop, 0); >> + >> + prop = priv->zorder_prop; >> + if (!prop) { >> + prop = drm_property_create_range(dev, 0, "zorder", 0, 3); >> + if (prop == NULL) >> + return; >> + priv->zorder_prop = prop; >> + } >> + drm_object_attach_property(obj, prop, 0); >> } >> >> int omap_plane_set_property(struct drm_plane *plane, >> @@ -452,6 +461,16 @@ int omap_plane_set_property(struct drm_plane *plane, >> ret = omap_plane_dpms(plane, DRM_MODE_DPMS_ON); >> else >> ret = 0; >> + } else if (property == priv->zorder_prop) { >> + struct omap_overlay *ovl = omap_plane->ovl; >> + >> + DBG("%s: zorder: %d", ovl->name, (uint32_t)val); >> + omap_plane->info.zorder = val; > > What would happen when there's a conflicting assignment between two > planes? non-good things.. basically as part of re-working the omapdss<->omapdrm stuff, I'll have a good point in omapdrm before setting GO bit(s) where I can calculate the actual z-order from that is set from userspace. If two planes had same z-order from userspace, omapdrm would simply put one in front of the other. If the planes aren't overlapping, this is fine. > I tried to think of a decent way to do this stuff, but some hardware > can have rather complicated stacking order limitations. One idea I > came up with was to have an enum prop on the crtc, where the individual > enum value names would somehow describe the whole stacking order within > the crtc. That way user space couldn't even try to use an unsupported > configuration. The downside is that user space would need to parse > those strings if it wants to do some automagic stacking order changes, > which means the string format would need some though. I was thinking about this, but then you run into the same issue if you move a plane from one CRTC to another without userspace setting the property again. In the end if userspace is ambiguous the driver has to just arbitrarily pick some z-order to keep the hw happy. BR, -R > -- > Ville Syrjälä > Intel OTC > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/staging/omapdrm/omap_drv.h b/drivers/staging/omapdrm/omap_drv.h index b103d28..9dc72d1 100644 --- a/drivers/staging/omapdrm/omap_drv.h +++ b/drivers/staging/omapdrm/omap_drv.h @@ -62,6 +62,7 @@ struct omap_drm_private { /* properties: */ struct drm_property *rotation_prop; + struct drm_property *zorder_prop; }; /* this should probably be in drm-core to standardize amongst drivers */ diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c index 6931d06..4bde639 100644 --- a/drivers/staging/omapdrm/omap_plane.c +++ b/drivers/staging/omapdrm/omap_plane.c @@ -433,6 +433,15 @@ void omap_plane_install_properties(struct drm_plane *plane, priv->rotation_prop = prop; } drm_object_attach_property(obj, prop, 0); + + prop = priv->zorder_prop; + if (!prop) { + prop = drm_property_create_range(dev, 0, "zorder", 0, 3); + if (prop == NULL) + return; + priv->zorder_prop = prop; + } + drm_object_attach_property(obj, prop, 0); } int omap_plane_set_property(struct drm_plane *plane, @@ -452,6 +461,16 @@ int omap_plane_set_property(struct drm_plane *plane, ret = omap_plane_dpms(plane, DRM_MODE_DPMS_ON); else ret = 0; + } else if (property == priv->zorder_prop) { + struct omap_overlay *ovl = omap_plane->ovl; + + DBG("%s: zorder: %d", ovl->name, (uint32_t)val); + omap_plane->info.zorder = val; + + if (ovl->is_enabled(ovl)) + ret = omap_plane_dpms(plane, DRM_MODE_DPMS_ON); + else + ret = 0; } return ret;