[Intel-gfx] [PATCH v19 1/8] drm/i915: Start passing latency as parameter

[Lisovskiy, Stanislav]

On Tue, 2020-03-10 at 16:32 +0200, Ville Syrjälä wrote:
> On Mon, Mar 09, 2020 at 06:11:57PM +0200, Stanislav Lisovskiy wrote:
> > We need to start passing memory latency as a
> > parameter when calculating plane wm levels,
> > as latency can get changed in different
> > circumstances(for example with or without SAGV).
> > So we need to be more flexible on that matter.
> > 
> > Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 8375054ba27d..c7928c870b0a 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4016,6 +4016,7 @@ static int skl_compute_wm_params(const struct
> > intel_crtc_state *crtc_state,
> >  				 int color_plane);
> >  static void skl_compute_plane_wm(const struct intel_crtc_state
> > *crtc_state,
> >  				 int level,
> > +				 u32 latency,
> 
> So you didn't change the types?

Yes, I saw your comment there - and looked into this, however I just
wondered, does it make any sense do to that. The reason is because
skl_latency is anyway defined as u16 in i915_drv.h, just as pri/spr/cur
latencies, so wonder how this "unsigned int" going to fit into this. 
Should I maybe then change it to u16 - at least that would somehow
comply with the current declarations.

> 
> >  				 const struct skl_wm_params *wp,
> >  				 const struct skl_wm_level
> > *result_prev,
> >  				 struct skl_wm_level *result /* out
> > */);
> > @@ -4038,7 +4039,9 @@ skl_cursor_allocation(const struct
> > intel_crtc_state *crtc_state,
> >  	drm_WARN_ON(&dev_priv->drm, ret);
> >  
> >  	for (level = 0; level <= max_level; level++) {
> > -		skl_compute_plane_wm(crtc_state, level, &wp, &wm, &wm);
> > +		u32 latency = dev_priv->wm.skl_latency[level];
> > +
> > +		skl_compute_plane_wm(crtc_state, level, latency, &wp,
> > &wm, &wm);
> >  		if (wm.min_ddb_alloc == U16_MAX)
> >  			break;
> >  
> > @@ -4972,12 +4975,12 @@ static bool skl_wm_has_lines(struct
> > drm_i915_private *dev_priv, int level)
> >  
> >  static void skl_compute_plane_wm(const struct intel_crtc_state
> > *crtc_state,
> >  				 int level,
> > +				 u32 latency,
> >  				 const struct skl_wm_params *wp,
> >  				 const struct skl_wm_level
> > *result_prev,
> >  				 struct skl_wm_level *result /* out */)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(crtc_state-
> > >uapi.crtc->dev);
> > -	u32 latency = dev_priv->wm.skl_latency[level];
> >  	uint_fixed_16_16_t method1, method2;
> >  	uint_fixed_16_16_t selected_result;
> >  	u32 res_blocks, res_lines, min_ddb_alloc = 0;
> > @@ -5106,9 +5109,10 @@ skl_compute_wm_levels(const struct
> > intel_crtc_state *crtc_state,
> >  
> >  	for (level = 0; level <= max_level; level++) {
> >  		struct skl_wm_level *result = &levels[level];
> > +		u32 latency = dev_priv->wm.skl_latency[level];
> >  
> > -		skl_compute_plane_wm(crtc_state, level, wm_params,
> > -				     result_prev, result);
> > +		skl_compute_plane_wm(crtc_state, level, latency,
> > +				     wm_params, result_prev, result);
> >  
> >  		result_prev = result;
> >  	}
> > -- 
> > 2.24.1.485.gad05a3d8e5
> 
>