On Mon, Aug 25, 2025 at 03:43:11PM +0200, Maxime Ripard wrote: > The __drm_atomic_get_current_plane_state() function tries to get and > return the existing plane state, and if it doesn't exist returns the one > stored in the drm_plane->state field. > > Using the current nomenclature, it tries to get the existing plane state > with an ad-hoc implementation of drm_atomic_get_existing_plane_state(), > and falls back to either the old or new plane state, depending on > whether it is called before or after drm_atomic_helper_swap_state(). > > The existing plane state itself is deprecated, because it also changes > when swapping states from the new state to the old state. > > Fortunately for us, we can simplify things. Indeed, > __drm_atomic_get_current_plane_state() is only used in two macros: > intel_atomic_crtc_state_for_each_plane_state and > drm_atomic_crtc_state_for_each_plane_state(). > > The intel variant is only used through the intel_wm_compute() function > that is only ever called in intel_crtc_atomic_check().
Ugh. I've been meaning to clean up that mess for years. I suppose I should revisit it again... > > The generic variant is more widely used, and can be found in the malidp, > msm, tegra and vc4 drivers. All of these call sites though are during > atomic_check(), so we end up in the same situation than Intel's. > > Thus, we only ever use the existing state as the new state, and > plane->state is always going to be the old state. Any plane isn't > guaranteed to be part of the state though, so we can't rely on > drm_atomic_get_old_plane_state() and we still need to use plane->state. > > Signed-off-by: Maxime Ripard <[email protected]> > --- > include/drm/drm_atomic.h | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index > 798d33b50ef7497ce938ce3dbabee32487dda2d6..82e74d9444c4fa7f02ee0e472c8c68f7bc44cc6a > 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -789,15 +789,15 @@ drm_atomic_get_new_connector_state(const struct > drm_atomic_state *state, > /** > * __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. > + * This function returns the plane state for the given plane, either the > + * new plane state 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 > @@ -814,13 +814,19 @@ drm_atomic_get_new_connector_state(const struct > drm_atomic_state *state, > */ > static inline const struct drm_plane_state * > __drm_atomic_get_current_plane_state(const struct drm_atomic_state *state, > struct drm_plane *plane) > { > - if (state->planes[drm_plane_index(plane)].state) > - return state->planes[drm_plane_index(plane)].state; > + struct drm_plane_state *plane_state; > > + plane_state = drm_atomic_get_new_plane_state(state, plane); > + if (plane_state) > + return plane_state; > + > + /* > + * If the plane isn't part of the state, fallback to the currently > active one. > + */ > return plane->state; > } > > int __must_check > drm_atomic_add_encoder_bridges(struct drm_atomic_state *state, > > -- > 2.50.1 -- Ville Syrjälä Intel
