Op 02-08-17 om 10:02 schreef Daniel Vetter:
> On Tue, Aug 1, 2017 at 12:20 PM, Laurent Pinchart
> <laurent.pinch...@ideasonboard.com> wrote:
>> On Tuesday 01 Aug 2017 07:59:13 Maarten Lankhorst wrote:
>>> +       pri_state = drm_atomic_get_new_plane_state(crtc->primary,
>>> state->state);
>>> +       if (pri_state) {
>>> +               struct omap_crtc_state *omap_crtc_state =
>>> +                       to_omap_crtc_state(state);
>>> +
>>> +               omap_crtc_state->zpos = pri_state->zpos;
>>> +               omap_crtc_state->rotation = pri_state->rotation;
>>> +       }
>>>
>>> That way even when updating the property through the primary plane, it gets
>>> reflected correctly. For example when vt switching with fbdev.
>> Let's not make it over-complicated. This hack is only needed fo the legacy X
>> OMAP modesetting driver. The CRTC zpos and rotation properties should not be
>> used through the atomic API.
> Marten is right, the atomic properties are all such that you can
> unconditionally restore them (e.g. fences report a no-op value), to
> make compositor switching easier. Not sure anyone implements that, but
> I think it's a useful idea to keep. I'll respin (or maybe Maarten
> simply submits his patch with a proper sob ...).
> -Daniel

Here you go, compile tested version. :)
----8<----
I want/need to rework the core property handling, and this hack is
getting in the way. But since it's a non-standard propety only used by
legacy userspace we know that this will only every be called from
ioctl code. And never on some other free-standing state struct, where
this old hack wouldn't work either.

v2: don't forget zorder and get_property!

v3: Shadow the legacy state to avoid locking issues in get_property
(Maarten).

v4: Review from Laurent
- Move struct omap_crtc_state into omap_crtc.c
- Clean up comments.
- Don't forget to copy the shadowed state over on duplicate.

v5: Don't forget to update the reset handler (Maarten).
v6: Update omap_crtc_state shadow values in omap_crtc_atomic_check (Maarten).

Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> (v4)
Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Cc: Tomi Valkeinen <tomi.valkei...@ti.com
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 118 +++++++++++++++++++++++++-----------
 1 file changed, 81 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c 
b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 14e8a7738b06..b842fcd9914a 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -26,6 +26,16 @@
 
 #include "omap_drv.h"
 
+#define to_omap_crtc_state(x) container_of(x, struct omap_crtc_state, base)
+
+struct omap_crtc_state {
+       /* Must be first. */
+       struct drm_crtc_state base;
+       /* Shadow values for legacy userspace support. */
+       unsigned int rotation;
+       unsigned int zpos;
+};
+
 #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
 
 struct omap_crtc {
@@ -445,6 +455,8 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 static int omap_crtc_atomic_check(struct drm_crtc *crtc,
                                struct drm_crtc_state *state)
 {
+       struct drm_plane_state *pri_state;
+
        if (state->color_mgmt_changed && state->gamma_lut) {
                uint length = state->gamma_lut->length /
                        sizeof(struct drm_color_lut);
@@ -453,6 +465,16 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc,
                        return -EINVAL;
        }
 
+       pri_state = drm_atomic_get_new_plane_state(state->state, crtc->primary);
+       if (pri_state) {
+               struct omap_crtc_state *omap_crtc_state =
+                       to_omap_crtc_state(state);
+
+               /* Mirror new values for zpos and rotation in omap_crtc_state */
+               omap_crtc_state->zpos = pri_state->zpos;
+               omap_crtc_state->rotation = pri_state->rotation;
+       }
+
        return 0;
 }
 
@@ -498,39 +520,32 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
        spin_unlock_irq(&crtc->dev->event_lock);
 }
 
-static bool omap_crtc_is_plane_prop(struct drm_crtc *crtc,
-       struct drm_property *property)
-{
-       struct drm_device *dev = crtc->dev;
-       struct omap_drm_private *priv = dev->dev_private;
-
-       return property == priv->zorder_prop ||
-               property == crtc->primary->rotation_property;
-}
-
 static int omap_crtc_atomic_set_property(struct drm_crtc *crtc,
                                         struct drm_crtc_state *state,
                                         struct drm_property *property,
                                         uint64_t val)
 {
-       if (omap_crtc_is_plane_prop(crtc, property)) {
-               struct drm_plane_state *plane_state;
-               struct drm_plane *plane = crtc->primary;
-
-               /*
-                * Delegate property set to the primary plane. Get the plane
-                * state and set the property directly.
-                */
-
-               plane_state = drm_atomic_get_plane_state(state->state, plane);
-               if (IS_ERR(plane_state))
-                       return PTR_ERR(plane_state);
+       struct omap_drm_private *priv = crtc->dev->dev_private;
+       struct drm_plane_state *plane_state;
 
-               return drm_atomic_plane_set_property(plane, plane_state,
-                               property, val);
+       /*
+        * Delegate property set to the primary plane. Get the plane state and
+        * set the property directly, but keep a shadow copy for the
+        * atomic_get_property callback.
+        */
+       plane_state = drm_atomic_get_plane_state(state->state, crtc->primary);
+       if (IS_ERR(plane_state))
+               return PTR_ERR(plane_state);
+
+       if (property == crtc->primary->rotation_property) {
+               plane_state->rotation = val;
+       } else if (property == priv->zorder_prop) {
+               plane_state->zpos = val;
+       } else {
+               return -EINVAL;
        }
 
-       return -EINVAL;
+       return 0;
 }
 
 static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
@@ -538,28 +553,57 @@ static int omap_crtc_atomic_get_property(struct drm_crtc 
*crtc,
                                         struct drm_property *property,
                                         uint64_t *val)
 {
-       if (omap_crtc_is_plane_prop(crtc, property)) {
-               /*
-                * Delegate property get to the primary plane. The
-                * drm_atomic_plane_get_property() function isn't exported, but
-                * can be called through drm_object_property_get_value() as that
-                * will call drm_atomic_get_property() for atomic drivers.
-                */
-               return drm_object_property_get_value(&crtc->primary->base,
-                               property, val);
-       }
+       struct omap_drm_private *priv = crtc->dev->dev_private;
+       struct omap_crtc_state *omap_state = to_omap_crtc_state(state);
+
+       if (property == crtc->primary->rotation_property)
+               return omap_state->rotation;
+       else if (property == priv->zorder_prop)
+               return omap_state->zpos;
 
        return -EINVAL;
 }
 
+static void omap_crtc_reset(struct drm_crtc *crtc)
+{
+       if (crtc->state)
+               __drm_atomic_helper_crtc_destroy_state(crtc->state);
+
+       kfree(crtc->state);
+       crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
+
+       if (crtc->state)
+               crtc->state->crtc = crtc;
+}
+
+static struct drm_crtc_state *
+omap_crtc_duplicate_state(struct drm_crtc *crtc)
+{
+       struct omap_crtc_state *state, *current_state;
+
+       if (WARN_ON(!crtc->state))
+               return NULL;
+
+       current_state = to_omap_crtc_state(crtc->state);
+
+       state = kmalloc(sizeof(*state), GFP_KERNEL);
+       if (state)
+               __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base);
+
+       state->zpos = current_state->zpos;
+       state->rotation = current_state->rotation;
+
+       return &state->base;
+}
+
 static const struct drm_crtc_funcs omap_crtc_funcs = {
-       .reset = drm_atomic_helper_crtc_reset,
+       .reset = omap_crtc_reset,
        .set_config = drm_atomic_helper_set_config,
        .destroy = omap_crtc_destroy,
        .page_flip = drm_atomic_helper_page_flip,
        .gamma_set = drm_atomic_helper_legacy_gamma_set,
        .set_property = drm_atomic_helper_crtc_set_property,
-       .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+       .atomic_duplicate_state = omap_crtc_duplicate_state,
        .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
        .atomic_set_property = omap_crtc_atomic_set_property,
        .atomic_get_property = omap_crtc_atomic_get_property,
-- 
2.11.0


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to