| Message ID | 20191029072229.27092-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [CI,01/12] drm/i915: Introduce intel_atomic_get_plane_state_after_check(), v2. | expand |
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@linux.intel.com> > --- > 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? > + > const struct drm_plane_helper_funcs intel_plane_helper_funcs = { > .prepare_fb = intel_prepare_plane_fb, > .cleanup_fb = intel_cleanup_plane_fb, > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 0caef2592a7e..a9e9510d2d07 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -2275,6 +2275,7 @@ static int intel_modeset_all_pipes(struct intel_atomic_state *state) > */ > for_each_intel_crtc(&dev_priv->drm, crtc) { > struct intel_crtc_state *crtc_state; > + struct intel_plane *plane; > int ret; > > crtc_state = intel_atomic_get_crtc_state(&state->base, crtc); > @@ -2292,12 +2293,14 @@ static int intel_modeset_all_pipes(struct intel_atomic_state *state) > if (ret) > return ret; > > - ret = drm_atomic_add_affected_planes(&state->base, > - &crtc->base); > - if (ret) > - return ret; > - > - crtc_state->update_planes |= crtc_state->active_planes; > + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) { > + struct intel_plane_state *plane_state = > + intel_atomic_get_plane_state_after_check(state, > + crtc_state, > + plane); > + if (IS_ERR(plane_state)) > + return PTR_ERR(plane_state); > + } > } > > return 0; > diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c > index fa44eb73d088..95586a588234 100644 > --- a/drivers/gpu/drm/i915/display/intel_color.c > +++ b/drivers/gpu/drm/i915/display/intel_color.c > @@ -1077,11 +1077,12 @@ intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state) > if (!need_plane_update(plane, new_crtc_state)) > continue; > > - plane_state = intel_atomic_get_plane_state(state, plane); > + plane_state = > + intel_atomic_get_plane_state_after_check(state, > + new_crtc_state, > + plane); > if (IS_ERR(plane_state)) > return PTR_ERR(plane_state); > - > - new_crtc_state->update_planes |= BIT(plane->id); > } > > return 0; > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index 40184e823c84..61e9db041613 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1561,4 +1561,10 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state) > return i915_ggtt_offset(state->vma); > } > > +/* intel_atomic_plane.c */ > +struct intel_plane_state * > +intel_atomic_get_plane_state_after_check(struct intel_atomic_state *state, > + struct intel_crtc_state *crtc_state, > + struct intel_plane *plane); > + > #endif /* __INTEL_DISPLAY_TYPES_H__ */ > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 5d2b460d3ee5..03a236ad6a6b 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5158,11 +5158,12 @@ skl_ddb_add_affected_planes(const struct intel_crtc_state *old_crtc_state, > &new_crtc_state->wm.skl.plane_ddb_uv[plane_id])) > continue; > > - plane_state = intel_atomic_get_plane_state(state, plane); > + plane_state = > + intel_atomic_get_plane_state_after_check(state, > + new_crtc_state, > + plane); > if (IS_ERR(plane_state)) > return PTR_ERR(plane_state); > - > - new_crtc_state->update_planes |= BIT(plane_id); > } > > return 0; > @@ -5434,11 +5435,12 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state, > &new_crtc_state->wm.skl.optimal.planes[plane_id])) > continue; > > - plane_state = intel_atomic_get_plane_state(state, plane); > + plane_state = > + intel_atomic_get_plane_state_after_check(state, > + new_crtc_state, > + plane); > if (IS_ERR(plane_state)) > return PTR_ERR(plane_state); > - > - new_crtc_state->update_planes |= BIT(plane_id); > } > > return 0; > -- > 2.23.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@linux.intel.com> >> --- >> 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. :)
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 <maarten.lankhorst@linux.intel.com> > >> --- > >> 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.
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; +} + const struct drm_plane_helper_funcs intel_plane_helper_funcs = { .prepare_fb = intel_prepare_plane_fb, .cleanup_fb = intel_cleanup_plane_fb, diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0caef2592a7e..a9e9510d2d07 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2275,6 +2275,7 @@ static int intel_modeset_all_pipes(struct intel_atomic_state *state) */ for_each_intel_crtc(&dev_priv->drm, crtc) { struct intel_crtc_state *crtc_state; + struct intel_plane *plane; int ret; crtc_state = intel_atomic_get_crtc_state(&state->base, crtc); @@ -2292,12 +2293,14 @@ static int intel_modeset_all_pipes(struct intel_atomic_state *state) if (ret) return ret; - ret = drm_atomic_add_affected_planes(&state->base, - &crtc->base); - if (ret) - return ret; - - crtc_state->update_planes |= crtc_state->active_planes; + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) { + struct intel_plane_state *plane_state = + intel_atomic_get_plane_state_after_check(state, + crtc_state, + plane); + if (IS_ERR(plane_state)) + return PTR_ERR(plane_state); + } } return 0; diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index fa44eb73d088..95586a588234 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1077,11 +1077,12 @@ intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state) if (!need_plane_update(plane, new_crtc_state)) continue; - plane_state = intel_atomic_get_plane_state(state, plane); + plane_state = + intel_atomic_get_plane_state_after_check(state, + new_crtc_state, + plane); if (IS_ERR(plane_state)) return PTR_ERR(plane_state); - - new_crtc_state->update_planes |= BIT(plane->id); } return 0; diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 40184e823c84..61e9db041613 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1561,4 +1561,10 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state) return i915_ggtt_offset(state->vma); } +/* intel_atomic_plane.c */ +struct intel_plane_state * +intel_atomic_get_plane_state_after_check(struct intel_atomic_state *state, + struct intel_crtc_state *crtc_state, + struct intel_plane *plane); + #endif /* __INTEL_DISPLAY_TYPES_H__ */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5d2b460d3ee5..03a236ad6a6b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5158,11 +5158,12 @@ skl_ddb_add_affected_planes(const struct intel_crtc_state *old_crtc_state, &new_crtc_state->wm.skl.plane_ddb_uv[plane_id])) continue; - plane_state = intel_atomic_get_plane_state(state, plane); + plane_state = + intel_atomic_get_plane_state_after_check(state, + new_crtc_state, + plane); if (IS_ERR(plane_state)) return PTR_ERR(plane_state); - - new_crtc_state->update_planes |= BIT(plane_id); } return 0; @@ -5434,11 +5435,12 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state, &new_crtc_state->wm.skl.optimal.planes[plane_id])) continue; - plane_state = intel_atomic_get_plane_state(state, plane); + plane_state = + intel_atomic_get_plane_state_after_check(state, + new_crtc_state, + plane); if (IS_ERR(plane_state)) return PTR_ERR(plane_state); - - new_crtc_state->update_planes |= BIT(plane_id); } return 0;
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@linux.intel.com> --- 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(-)