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 <maarten.lankhorst@xxxxxxxxxxxxxxx> >> --- >> 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. :) _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx