On Wed, Oct 30, 2019 at 10:17:52AM +0100, Maarten Lankhorst wrote: > Op 29-10-2019 om 19:35 schreef Ville Syrjälä: > > On Tue, Oct 29, 2019 at 08:22:18AM +0100, Maarten Lankhorst wrote: > >> Use this in all the places where we try to acquire planes after the planes > >> atomic_check(). > >> > >> In case of intel_modeset_all_pipes() this is not yet done after > >> atomic_check, > >> but seems like it will be in the future. To add some paranoia, add all > >> planes > >> rather than active planes, because of bigjoiner and planar YUV support > >> having > >> extra planes outside of the core's view that wouldn't be added otherwise. > >> > >> Changes since v1: > >> - Always add all planes, to handle force plane updates to work correctly > >> with a disabled cursor plane. > >> > >> Signed-off-by: Maarten Lankhorst <[email protected]> > >> --- > >> drivers/gpu/drm/i915/display/intel_atomic.c | 41 +++++++++---------- > >> .../gpu/drm/i915/display/intel_atomic_plane.c | 15 +++++++ > >> drivers/gpu/drm/i915/display/intel_cdclk.c | 15 ++++--- > >> drivers/gpu/drm/i915/display/intel_color.c | 7 ++-- > >> .../drm/i915/display/intel_display_types.h | 6 +++ > >> drivers/gpu/drm/i915/intel_pm.c | 14 ++++--- > >> 6 files changed, 62 insertions(+), 36 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > >> b/drivers/gpu/drm/i915/display/intel_atomic.c > >> index 9cd6d2348a1e..80df6c233581 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_atomic.c > >> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > >> @@ -313,13 +313,10 @@ int intel_atomic_setup_scalers(struct > >> drm_i915_private *dev_priv, > >> struct intel_crtc *intel_crtc, > >> struct intel_crtc_state *crtc_state) > >> { > >> - struct drm_plane *plane = NULL; > >> - struct intel_plane *intel_plane; > >> - struct intel_plane_state *plane_state = NULL; > >> struct intel_crtc_scaler_state *scaler_state = > >> &crtc_state->scaler_state; > >> struct drm_atomic_state *drm_state = crtc_state->base.state; > >> - struct intel_atomic_state *intel_state = > >> to_intel_atomic_state(drm_state); > >> + struct intel_atomic_state *state = to_intel_atomic_state(drm_state); > >> int num_scalers_need; > >> int i; > >> > >> @@ -346,6 +343,7 @@ int intel_atomic_setup_scalers(struct drm_i915_private > >> *dev_priv, > >> > >> /* walkthrough scaler_users bits and start assigning scalers */ > >> for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) { > >> + struct intel_plane_state *plane_state = NULL; > >> int *scaler_id; > >> const char *name; > >> int idx; > >> @@ -361,19 +359,16 @@ int intel_atomic_setup_scalers(struct > >> drm_i915_private *dev_priv, > >> /* panel fitter case: assign as a crtc scaler */ > >> scaler_id = &scaler_state->scaler_id; > >> } else { > >> - name = "PLANE"; > >> + struct intel_plane *plane; > >> > >> /* plane scaler case: assign as a plane scaler */ > >> /* find the plane that set the bit as scaler_user */ > >> - plane = drm_state->planes[i].ptr; > >> > >> /* > >> * to enable/disable hq mode, add planes that are using > >> scaler > >> * into this transaction > >> */ > >> - if (!plane) { > >> - struct drm_plane_state *state; > >> - > >> + if (!drm_state->planes[i].ptr) { > >> /* > >> * GLK+ scalers don't have a HQ mode so it > >> * isn't necessary to change between HQ and dyn > >> mode > >> @@ -382,24 +377,28 @@ int intel_atomic_setup_scalers(struct > >> drm_i915_private *dev_priv, > >> if (INTEL_GEN(dev_priv) >= 10 || > >> IS_GEMINILAKE(dev_priv)) > >> continue; > >> > >> - plane = drm_plane_from_index(&dev_priv->drm, i); > >> - state = drm_atomic_get_plane_state(drm_state, > >> plane); > >> - if (IS_ERR(state)) { > >> - DRM_DEBUG_KMS("Failed to add [PLANE:%d] > >> to drm_state\n", > >> - plane->base.id); > >> - return PTR_ERR(state); > >> + plane = > >> to_intel_plane(drm_plane_from_index(&dev_priv->drm, i)); > >> + plane_state = > >> + > >> intel_atomic_get_plane_state_after_check(state, > >> + > >> crtc_state, > >> + > >> plane); > >> + if (IS_ERR(plane_state)) { > >> + DRM_DEBUG_KMS("Failed to add [PLANE:%d] > >> to drm_state: %li\n", > >> + plane->base.base.id, > >> PTR_ERR(plane_state)); > >> + return PTR_ERR(plane_state); > >> } > >> + } else { > >> + plane = > >> to_intel_plane(drm_state->planes[i].ptr); > >> + plane_state = > >> intel_atomic_get_new_plane_state(state, > >> + > >> plane); > >> } > >> > >> - intel_plane = to_intel_plane(plane); > >> - idx = plane->base.id; > >> - > >> /* plane on different crtc cannot be a scaler user of > >> this crtc */ > >> - if (WARN_ON(intel_plane->pipe != intel_crtc->pipe)) > >> + if (WARN_ON(plane->pipe != intel_crtc->pipe)) > >> continue; > >> > >> - plane_state = > >> intel_atomic_get_new_plane_state(intel_state, > >> - > >> intel_plane); > >> + name = "PLANE"; > >> + idx = plane->base.base.id; > >> scaler_id = &plane_state->scaler_id; > >> } > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c > >> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > >> index 98f557a9f8ee..a9e2684c6b6d 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c > >> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > >> @@ -402,6 +402,21 @@ void i9xx_update_planes_on_crtc(struct > >> intel_atomic_state *state, > >> } > >> } > >> > >> +struct intel_plane_state * > >> +intel_atomic_get_plane_state_after_check(struct intel_atomic_state *state, > >> + struct intel_crtc_state > >> *new_crtc_state, > >> + struct intel_plane *plane) > >> +{ > >> + struct intel_plane_state *plane_state = > >> + intel_atomic_get_plane_state(state, plane); > >> + > >> + if (IS_ERR(plane_state)) > >> + return plane_state; > >> + > >> + new_crtc_state->update_planes |= BIT(plane->id); > >> + return plane_state; > >> +} > > With the hw state being persistent now I guess we don't need this > > anymore? > > Yes, it would stil be useful for annotation though. :)
To me it seems to just hide things (the update_planes bitmask adjustment), so it's not at all ovbvious for the reader that calling this will result in the plane registers really getting programmed. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
