drm/atomic: Add drm_atomic_crtc_state_for_each_plane_state
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 2 Jun 2016 14:21:44 +0000 (16:21 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 2 Jun 2016 14:59:05 +0000 (16:59 +0200)
... and use it in msm&vc4. Again just want to encapsulate
drm_atomic_state internals a bit.

The const threading is a bit awkward in vc4 since C sucks, but I still
think it's worth to enforce this. Eventually I want to make all the
obj->state pointers const too, but that's a lot more work ...

v2: Provide safe macro to wrap up the unsafe helper better, suggested
by Maarten.

v3: Fixup subject (Maarten) and spelling fixes (Eric Engestrom).

Cc: Eric Anholt <eric@anholt.net>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1464877304-4213-1-git-send-email-daniel.vetter@ffwll.ch
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
drivers/gpu/drm/vc4/vc4_crtc.c
drivers/gpu/drm/vc4/vc4_drv.h
drivers/gpu/drm/vc4/vc4_plane.c
include/drm/drm_atomic.h
include/drm/drm_atomic_helper.h

index 88fe256..4e8ed73 100644 (file)
@@ -374,6 +374,7 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
        struct drm_device *dev = crtc->dev;
        struct plane_state pstates[STAGE_MAX + 1];
        const struct mdp5_cfg_hw *hw_cfg;
+       const struct drm_plane_state *pstate;
        int cnt = 0, i;
 
        DBG("%s: check", mdp5_crtc->name);
@@ -382,20 +383,13 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
         * and that we don't have conflicting mixer stages:
         */
        hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
-       drm_atomic_crtc_state_for_each_plane(plane, state) {
-               struct drm_plane_state *pstate;
+       drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
                if (cnt >= (hw_cfg->lm.nb_stages)) {
                        dev_err(dev->dev, "too many planes!\n");
                        return -EINVAL;
                }
 
-               pstate = state->state->plane_states[drm_plane_index(plane)];
 
-               /* plane might not have changed, in which case take
-                * current state:
-                */
-               if (!pstate)
-                       pstate = plane->state;
                pstates[cnt].plane = plane;
                pstates[cnt].state = to_mdp5_plane_state(pstate);
 
index 904d075..ba2e373 100644 (file)
@@ -395,6 +395,7 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
        struct vc4_dev *vc4 = to_vc4_dev(dev);
        struct drm_plane *plane;
        unsigned long flags;
+       const struct drm_plane_state *plane_state;
        u32 dlist_count = 0;
        int ret;
 
@@ -404,18 +405,8 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
        if (hweight32(state->connector_mask) > 1)
                return -EINVAL;
 
-       drm_atomic_crtc_state_for_each_plane(plane, state) {
-               struct drm_plane_state *plane_state =
-                       state->state->plane_states[drm_plane_index(plane)];
-
-               /* plane might not have changed, in which case take
-                * current state:
-                */
-               if (!plane_state)
-                       plane_state = plane->state;
-
+       drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, state)
                dlist_count += vc4_plane_dlist_size(plane_state);
-       }
 
        dlist_count++; /* Account for SCALER_CTL0_END. */
 
index 37cac59..c799baa 100644 (file)
@@ -469,7 +469,7 @@ int vc4_kms_load(struct drm_device *dev);
 struct drm_plane *vc4_plane_init(struct drm_device *dev,
                                 enum drm_plane_type type);
 u32 vc4_plane_write_dlist(struct drm_plane *plane, u32 __iomem *dlist);
-u32 vc4_plane_dlist_size(struct drm_plane_state *state);
+u32 vc4_plane_dlist_size(const struct drm_plane_state *state);
 void vc4_plane_async_set_fb(struct drm_plane *plane,
                            struct drm_framebuffer *fb);
 
index 4037b52..5d2c3d9 100644 (file)
@@ -690,9 +690,10 @@ u32 vc4_plane_write_dlist(struct drm_plane *plane, u32 __iomem *dlist)
        return vc4_state->dlist_count;
 }
 
-u32 vc4_plane_dlist_size(struct drm_plane_state *state)
+u32 vc4_plane_dlist_size(const struct drm_plane_state *state)
 {
-       struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
+       const struct vc4_plane_state *vc4_state =
+               container_of(state, typeof(*vc4_state), base);
 
        return vc4_state->dlist_count;
 }
index 92c84e9..4e97186 100644 (file)
@@ -109,6 +109,42 @@ drm_atomic_get_existing_connector_state(struct drm_atomic_state *state,
        return state->connector_states[index];
 }
 
+/**
+ * __drm_atomic_get_current_plane_state - get current plane state
+ * @state: global atomic state object
+ * @plane: plane to grab
+ *
+ * This function returns the plane state for the given plane, either from
+ * @state, or if the plane isn't part of the atomic state update, from @plane.
+ * This is useful in atomic check callbacks, when drivers need to peek at, but
+ * not change, state of other planes, since it avoids threading an error code
+ * back up the call chain.
+ *
+ * WARNING:
+ *
+ * Note that this function is in general unsafe since it doesn't check for the
+ * required locking for access state structures. Drivers must ensure that it is
+ * save to access the returned state structure through other means. One commone
+ * example is when planes are fixed to a single CRTC, and the driver knows that
+ * the CRTC locks is held already. In that case holding the CRTC locks gives a
+ * read-lock on all planes connected to that CRTC. But if planes can be
+ * reassigned things get more tricky. In that case it's better to use
+ * drm_atomic_get_plane_state and wire up full error handling.
+ *
+ * Returns:
+ *
+ * Read-only pointer to the current plane state.
+ */
+static inline const struct drm_plane_state *
+__drm_atomic_get_current_plane_state(struct drm_atomic_state *state,
+                                    struct drm_plane *plane)
+{
+       if (state->plane_states[drm_plane_index(plane)])
+               return state->plane_states[drm_plane_index(plane)];
+
+       return plane->state;
+}
+
 int __must_check
 drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
                             struct drm_display_mode *mode);
index d473dcc..b03bd83 100644 (file)
@@ -159,7 +159,7 @@ void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
  * This iterates over the current state, useful (for example) when applying
  * atomic state after it has been checked and swapped.  To iterate over the
  * planes which *will* be attached (for ->atomic_check()) see
- * drm_crtc_for_each_pending_plane()
+ * drm_crtc_for_each_pending_plane().
  */
 #define drm_atomic_crtc_for_each_plane(plane, crtc) \
        drm_for_each_plane_mask(plane, (crtc)->dev, (crtc)->state->plane_mask)
@@ -171,11 +171,31 @@ void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
  *
  * Similar to drm_crtc_for_each_plane(), but iterates the planes that will be
  * attached if the specified state is applied.  Useful during (for example)
- * ->atomic_check() operations, to validate the incoming state
+ * ->atomic_check() operations, to validate the incoming state.
  */
 #define drm_atomic_crtc_state_for_each_plane(plane, crtc_state) \
        drm_for_each_plane_mask(plane, (crtc_state)->state->dev, (crtc_state)->plane_mask)
 
+/**
+ * drm_crtc_atomic_state_for_each_plane_state - iterate over attached planes in new state
+ * @plane: the loop cursor
+ * @plane_state: loop cursor for the plane's state, must be const
+ * @crtc_state: the incoming crtc-state
+ *
+ * Similar to drm_crtc_for_each_plane(), but iterates the planes that will be
+ * attached if the specified state is applied.  Useful during (for example)
+ * ->atomic_check() operations, to validate the incoming state.
+ *
+ * Compared to just drm_atomic_crtc_state_for_each_plane() this also fills in a
+ * const plane_state. This is useful when a driver just wants to peek at other
+ * active planes on this crtc, but does not need to change it.
+ */
+#define drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, crtc_state) \
+       drm_for_each_plane_mask(plane, (crtc_state)->state->dev, (crtc_state)->plane_mask) \
+               for_each_if ((plane_state = \
+                             __drm_atomic_get_current_plane_state((crtc_state)->state, \
+                                                                  plane)))
+
 /*
  * drm_atomic_plane_disabling - check whether a plane is being disabled
  * @plane: plane object