Re: [Intel-gfx] [PATCH 06/12] drm/i915: Simplify watermark state checker calling convention

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Oct 04, 2023 at 07:57:08PM +0300, Jani Nikula wrote:
> On Wed, 04 Oct 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > There is never any reason to pass in both the crtc and its state
> > as one can always dig out the crtc from its state.
> 
> I'm wondering whether we shouldn't just always pass 
> 
> struct intel_atomic_state *state, struct intel_crtc *crtc
> 
> to the checker, everywhere, even if that's excessive in some cases. I
> think some of the confusion you're fixing in this series comes from
> people cargo culting the conventions, and then adding some new
> parameters as needed, deviating from the state, crtc pair.

Yeah, I guess that makes sense. I can respin with that.

> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_modeset_verify.c | 2 +-
> >  drivers/gpu/drm/i915/display/skl_watermark.c        | 4 ++--
> >  drivers/gpu/drm/i915/display/skl_watermark.h        | 3 +--
> >  3 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_verify.c b/drivers/gpu/drm/i915/display/intel_modeset_verify.c
> > index 47d45ba1e707..bbee79aad0cd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_modeset_verify.c
> > +++ b/drivers/gpu/drm/i915/display/intel_modeset_verify.c
> > @@ -233,7 +233,7 @@ void intel_modeset_verify_crtc(struct intel_crtc *crtc,
> >  	    !intel_crtc_needs_fastset(new_crtc_state))
> >  		return;
> >  
> > -	intel_wm_state_verify(crtc, new_crtc_state);
> > +	intel_wm_state_verify(new_crtc_state);
> >  	verify_connector_state(state, crtc);
> >  	verify_crtc_state(state, crtc);
> >  	intel_shared_dpll_state_verify(state, crtc);
> > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> > index d51cf92c96ae..e7a9fb4b1f6b 100644
> > --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> > @@ -3134,9 +3134,9 @@ static void skl_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915)
> >  	skl_wm_sanitize(i915);
> >  }
> >  
> > -void intel_wm_state_verify(struct intel_crtc *crtc,
> > -			   const struct intel_crtc_state *new_crtc_state)
> > +void intel_wm_state_verify(const struct intel_crtc_state *new_crtc_state)
> >  {
> > +	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> >  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> >  	struct skl_hw_state {
> >  		struct skl_ddb_entry ddb[I915_MAX_PLANES];
> > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
> > index 18e4b0661cbb..ca4312bf7012 100644
> > --- a/drivers/gpu/drm/i915/display/skl_watermark.h
> > +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
> > @@ -38,8 +38,7 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
> >  				 const struct skl_ddb_entry *entries,
> >  				 int num_entries, int ignore_idx);
> >  
> > -void intel_wm_state_verify(struct intel_crtc *crtc,
> > -			   const struct intel_crtc_state *new_crtc_state);
> > +void intel_wm_state_verify(const struct intel_crtc_state *new_crtc_state);
> >  
> >  void skl_watermark_ipc_init(struct drm_i915_private *i915);
> >  void skl_watermark_ipc_update(struct drm_i915_private *i915);
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel