From 2f196b7c4b82eeff3574eb2999e78add33ef4361 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 2 Jun 2016 16:21:44 +0200 Subject: [PATCH] drm/atomic: Add drm_atomic_crtc_state_for_each_plane_state ... 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 Cc: Rob Clark Cc: Maarten Lankhorst Cc: Eric Engestrom Reviewed-by: Maarten Lankhorst Signed-off-by: Daniel Vetter 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 | 10 ++----- drivers/gpu/drm/vc4/vc4_crtc.c | 13 ++------- drivers/gpu/drm/vc4/vc4_drv.h | 2 +- drivers/gpu/drm/vc4/vc4_plane.c | 5 ++-- include/drm/drm_atomic.h | 36 ++++++++++++++++++++++++ include/drm/drm_atomic_helper.h | 24 ++++++++++++++-- 6 files changed, 66 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c index 88fe256c1931..4e8ed739f558 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c @@ -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); diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 904d0754ad78..ba2e373ec901 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -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. */ diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 37cac59401d7..c799baabc008 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -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); diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 4037b52fde31..5d2c3d9fd17a 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -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; } diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 92c84e9ab09a..4e97186293be 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -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); diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index d473dcc91f54..b03bd83703b4 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -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 -- 2.20.1