| Message ID | 20241105071600.235338-2-vinod.govindapillai@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | use hw support for min/interim ddb allocation for async flip | expand |
On Tue, 05 Nov 2024, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote: > Update intel_enabled_dbuf_slices_mask to use intel_display instead > of drm_i915_private object. This is a prepratory patch for the next > patch in the series, where all intel_de_read calls in skl_watermarks.c > are updated to use intel_display instead of drm_i915_private. > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display_power.c | 2 +- > drivers/gpu/drm/i915/display/intel_display_power_well.c | 2 +- > drivers/gpu/drm/i915/display/skl_watermark.c | 9 +++++---- > drivers/gpu/drm/i915/display/skl_watermark.h | 3 ++- > 4 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 2766fd9208b0..62e0faffca40 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -1090,7 +1090,7 @@ static void gen9_dbuf_enable(struct drm_i915_private *dev_priv) > u8 slices_mask; > > dev_priv->display.dbuf.enabled_slices = > - intel_enabled_dbuf_slices_mask(dev_priv); > + intel_enabled_dbuf_slices_mask(&dev_priv->display); Please add a local struct intel_display *display variable and pass that to intel_enabled_dbuf_slices_mask(). The point is, all of the i915/dev_priv references need to go, and if you add &dev_priv->display, this line needs to be updated again. > > slices_mask = BIT(DBUF_S1) | dev_priv->display.dbuf.enabled_slices; > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c > index f0131dd853de..f792db191fcf 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c > @@ -973,7 +973,7 @@ static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv, > > static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv) > { > - u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(dev_priv); > + u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(&dev_priv->display); Ditto. > u8 enabled_dbuf_slices = dev_priv->display.dbuf.enabled_slices; > > drm_WARN(&dev_priv->drm, > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c > index 3b0e87edbacf..d9d7238f0fb4 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -52,13 +52,13 @@ struct skl_wm_params { > u32 dbuf_block_size; > }; > > -u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915) > +u8 intel_enabled_dbuf_slices_mask(struct intel_display *display) > { > u8 enabled_slices = 0; > enum dbuf_slice slice; > > - for_each_dbuf_slice(i915, slice) { > - if (intel_de_read(i915, DBUF_CTL_S(slice)) & DBUF_POWER_STATE) > + for_each_dbuf_slice(display, slice) { > + if (intel_de_read(display, DBUF_CTL_S(slice)) & DBUF_POWER_STATE) > enabled_slices |= BIT(slice); > } > > @@ -3126,6 +3126,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state, > struct intel_crtc *crtc) > { > struct drm_i915_private *i915 = to_i915(state->base.dev); > + struct intel_display *display = to_intel_display(state); Please prefer putting the display variable first if at all possible. > const struct intel_crtc_state *new_crtc_state = > intel_atomic_get_new_crtc_state(state, crtc); > struct skl_hw_state { > @@ -3149,7 +3150,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state, > > skl_pipe_ddb_get_hw_state(crtc, hw->ddb, hw->ddb_y); > > - hw_enabled_slices = intel_enabled_dbuf_slices_mask(i915); > + hw_enabled_slices = intel_enabled_dbuf_slices_mask(display); > > if (DISPLAY_VER(i915) >= 11 && > hw_enabled_slices != i915->display.dbuf.enabled_slices) > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h > index e73baec94873..990793e36272 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.h > +++ b/drivers/gpu/drm/i915/display/skl_watermark.h > @@ -17,11 +17,12 @@ struct intel_atomic_state; > struct intel_bw_state; > struct intel_crtc; > struct intel_crtc_state; > +struct intel_display; > struct intel_plane; > struct skl_pipe_wm; > struct skl_wm_level; > > -u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915); > +u8 intel_enabled_dbuf_slices_mask(struct intel_display *display); > > void intel_sagv_pre_plane_update(struct intel_atomic_state *state); > void intel_sagv_post_plane_update(struct intel_atomic_state *state);
Thanks Jani! Yes.. I was thinking the same and but got a bit mixed-up and also was in a bit of confusion how deep these intel_display changes should go! I see that you have added comment in the next patch that all of those i915 could be changed in one function! I can update the series based on that. Thanks Vinod On Tue, 2024-11-05 at 10:52 +0200, Jani Nikula wrote: > On Tue, 05 Nov 2024, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote: > > Update intel_enabled_dbuf_slices_mask to use intel_display instead > > of drm_i915_private object. This is a prepratory patch for the next > > patch in the series, where all intel_de_read calls in skl_watermarks.c > > are updated to use intel_display instead of drm_i915_private. > > > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display_power.c | 2 +- > > drivers/gpu/drm/i915/display/intel_display_power_well.c | 2 +- > > drivers/gpu/drm/i915/display/skl_watermark.c | 9 +++++---- > > drivers/gpu/drm/i915/display/skl_watermark.h | 3 ++- > > 4 files changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > index 2766fd9208b0..62e0faffca40 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -1090,7 +1090,7 @@ static void gen9_dbuf_enable(struct drm_i915_private *dev_priv) > > u8 slices_mask; > > > > dev_priv->display.dbuf.enabled_slices = > > - intel_enabled_dbuf_slices_mask(dev_priv); > > + intel_enabled_dbuf_slices_mask(&dev_priv->display); > > Please add a local struct intel_display *display variable and pass that > to intel_enabled_dbuf_slices_mask(). > > The point is, all of the i915/dev_priv references need to go, and if you > add &dev_priv->display, this line needs to be updated again. > > > > > slices_mask = BIT(DBUF_S1) | dev_priv->display.dbuf.enabled_slices; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c > > b/drivers/gpu/drm/i915/display/intel_display_power_well.c > > index f0131dd853de..f792db191fcf 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c > > @@ -973,7 +973,7 @@ static bool gen9_dc_off_power_well_enabled(struct drm_i915_private > > *dev_priv, > > > > static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv) > > { > > - u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(dev_priv); > > + u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(&dev_priv->display); > > Ditto. > > > u8 enabled_dbuf_slices = dev_priv->display.dbuf.enabled_slices; > > > > drm_WARN(&dev_priv->drm, > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > > b/drivers/gpu/drm/i915/display/skl_watermark.c > > index 3b0e87edbacf..d9d7238f0fb4 100644 > > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > > @@ -52,13 +52,13 @@ struct skl_wm_params { > > u32 dbuf_block_size; > > }; > > > > -u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915) > > +u8 intel_enabled_dbuf_slices_mask(struct intel_display *display) > > { > > u8 enabled_slices = 0; > > enum dbuf_slice slice; > > > > - for_each_dbuf_slice(i915, slice) { > > - if (intel_de_read(i915, DBUF_CTL_S(slice)) & DBUF_POWER_STATE) > > + for_each_dbuf_slice(display, slice) { > > + if (intel_de_read(display, DBUF_CTL_S(slice)) & DBUF_POWER_STATE) > > enabled_slices |= BIT(slice); > > } > > > > @@ -3126,6 +3126,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state, > > struct intel_crtc *crtc) > > { > > struct drm_i915_private *i915 = to_i915(state->base.dev); > > + struct intel_display *display = to_intel_display(state); > > Please prefer putting the display variable first if at all possible. > > > > const struct intel_crtc_state *new_crtc_state = > > intel_atomic_get_new_crtc_state(state, crtc); > > struct skl_hw_state { > > @@ -3149,7 +3150,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state, > > > > skl_pipe_ddb_get_hw_state(crtc, hw->ddb, hw->ddb_y); > > > > - hw_enabled_slices = intel_enabled_dbuf_slices_mask(i915); > > + hw_enabled_slices = intel_enabled_dbuf_slices_mask(display); > > > > if (DISPLAY_VER(i915) >= 11 && > > hw_enabled_slices != i915->display.dbuf.enabled_slices) > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h > > b/drivers/gpu/drm/i915/display/skl_watermark.h > > index e73baec94873..990793e36272 100644 > > --- a/drivers/gpu/drm/i915/display/skl_watermark.h > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.h > > @@ -17,11 +17,12 @@ struct intel_atomic_state; > > struct intel_bw_state; > > struct intel_crtc; > > struct intel_crtc_state; > > +struct intel_display; > > struct intel_plane; > > struct skl_pipe_wm; > > struct skl_wm_level; > > > > -u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915); > > +u8 intel_enabled_dbuf_slices_mask(struct intel_display *display); > > > > void intel_sagv_pre_plane_update(struct intel_atomic_state *state); > > void intel_sagv_post_plane_update(struct intel_atomic_state *state); >
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 2766fd9208b0..62e0faffca40 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -1090,7 +1090,7 @@ static void gen9_dbuf_enable(struct drm_i915_private *dev_priv) u8 slices_mask; dev_priv->display.dbuf.enabled_slices = - intel_enabled_dbuf_slices_mask(dev_priv); + intel_enabled_dbuf_slices_mask(&dev_priv->display); slices_mask = BIT(DBUF_S1) | dev_priv->display.dbuf.enabled_slices; diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c index f0131dd853de..f792db191fcf 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c @@ -973,7 +973,7 @@ static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv, static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv) { - u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(dev_priv); + u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(&dev_priv->display); u8 enabled_dbuf_slices = dev_priv->display.dbuf.enabled_slices; drm_WARN(&dev_priv->drm, diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 3b0e87edbacf..d9d7238f0fb4 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -52,13 +52,13 @@ struct skl_wm_params { u32 dbuf_block_size; }; -u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915) +u8 intel_enabled_dbuf_slices_mask(struct intel_display *display) { u8 enabled_slices = 0; enum dbuf_slice slice; - for_each_dbuf_slice(i915, slice) { - if (intel_de_read(i915, DBUF_CTL_S(slice)) & DBUF_POWER_STATE) + for_each_dbuf_slice(display, slice) { + if (intel_de_read(display, DBUF_CTL_S(slice)) & DBUF_POWER_STATE) enabled_slices |= BIT(slice); } @@ -3126,6 +3126,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state, struct intel_crtc *crtc) { struct drm_i915_private *i915 = to_i915(state->base.dev); + struct intel_display *display = to_intel_display(state); const struct intel_crtc_state *new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); struct skl_hw_state { @@ -3149,7 +3150,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state, skl_pipe_ddb_get_hw_state(crtc, hw->ddb, hw->ddb_y); - hw_enabled_slices = intel_enabled_dbuf_slices_mask(i915); + hw_enabled_slices = intel_enabled_dbuf_slices_mask(display); if (DISPLAY_VER(i915) >= 11 && hw_enabled_slices != i915->display.dbuf.enabled_slices) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h index e73baec94873..990793e36272 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.h +++ b/drivers/gpu/drm/i915/display/skl_watermark.h @@ -17,11 +17,12 @@ struct intel_atomic_state; struct intel_bw_state; struct intel_crtc; struct intel_crtc_state; +struct intel_display; struct intel_plane; struct skl_pipe_wm; struct skl_wm_level; -u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915); +u8 intel_enabled_dbuf_slices_mask(struct intel_display *display); void intel_sagv_pre_plane_update(struct intel_atomic_state *state); void intel_sagv_post_plane_update(struct intel_atomic_state *state);
Update intel_enabled_dbuf_slices_mask to use intel_display instead of drm_i915_private object. This is a prepratory patch for the next patch in the series, where all intel_de_read calls in skl_watermarks.c are updated to use intel_display instead of drm_i915_private. Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> --- drivers/gpu/drm/i915/display/intel_display_power.c | 2 +- drivers/gpu/drm/i915/display/intel_display_power_well.c | 2 +- drivers/gpu/drm/i915/display/skl_watermark.c | 9 +++++---- drivers/gpu/drm/i915/display/skl_watermark.h | 3 ++- 4 files changed, 9 insertions(+), 7 deletions(-)