Re: [PATCH 2/9] drm/i915: Reinstate an early latency==0 check for skl+

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

On Tue, Jan 29, 2019 at 03:54:30PM -0800, Matt Roper wrote:
> On Fri, Dec 21, 2018 at 07:14:29PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I thought we could remove all the early latency==0 checks
> > and rely on skl_wm_method{1,2}() checking for it. But
> > skl_compute_plane_wm() applies a bunch of workarounds to bump
> > up the latency before calling those guys so clearly it won't
> > end up doing the right thing. Also not sure if the calculations
> > based on the method1/2 results are safe agaisnt overflows so
> > it might not work all that well in any case. Let's put the
> > early check back.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Should we remove the tests from skl_wm_method{1,2}() now?  I suppose
> someone could still use the debugfs interface to set a latency value of
> exactly (UINT32_MAX - workaround amount) to make latency wrap around and
> hit 0, but I'm not sure if that's really any worse than if they shoot
> themselves in the foot by setting a too-low non-zero latency.  I don't
> think we divide by latency anywhere.

I'd probably prefer to fix the overflows somehow so that the code
could work the same way as the pre-skl code (ie. just check for 0
latency in the method1/2 functions). But that would require some
actual thinking.

> 
> Either way,
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Thanks for the reviews. Series pushed to dinq.

> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index d132ef10fa60..0aac7e7b660f 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4701,6 +4701,9 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
> >  		to_intel_atomic_state(cstate->base.state);
> >  	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
> >  
> > +	if (latency == 0)
> > +		return;
> > +
> >  	/* Display WA #1141: kbl,cfl */
> >  	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> >  	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx