Re: [PATCH v2 05/13] drm/i915: Fix latency==0 handling for level 0 watermark on skl+

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 05/13] drm/i915: Fix latency==0 handling for level 0 watermark on skl+
Date: Wed, 21 Nov 2018 21:09:41 +0200	[thread overview]
Message-ID: <20181121190941.GM9144@intel.com> (raw)
In-Reply-To: <20181119231434.GM32505@mdroper-desk.amr.corp.intel.com>

On Mon, Nov 19, 2018 at 03:14:34PM -0800, Matt Roper wrote:
> On Wed, Nov 14, 2018 at 11:07:21PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If the level 0 latency is 0 we can't do anything. Return an error
> > rather than success.
> > 
> > While this can't happen due to WaWmMemoryReadLatency, it can
> > happen if the user clears out the level 0 latency via debugfs.
> > 
> > v2: Clarify how how we can end here with zero level 0 latency (Matt)
> > 
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I wonder whether we should really be letting users shoot themselves in
> the foot with debugfs here. Maybe we should pull the sanitization and
> hardware workarounds out of intel_read_wm_latency() and also apply it to
> whatever debugfs gives us.

I've considered that. But OTOH it's rather nice to be able to override
the workarounds and just test exactly what you want. And no normal
user should be poking around in debugfs anyway. It's only for debugging
and as such is generally understood to contain some amount of dragons.

> 
> This is good enough for now though.
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 27498ded4949..25f589c4f68c 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4704,8 +4704,10 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> >  	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
> >  	uint32_t min_disp_buf_needed;
> >  
> > -	if (latency == 0 ||
> > -	    !intel_wm_plane_visible(cstate, intel_pstate)) {
> > +	if (latency == 0)
> > +		return level == 0 ? -EINVAL : 0;
> > +
> > +	if (!intel_wm_plane_visible(cstate, intel_pstate)) {
> >  		result->plane_en = false;
> >  		return 0;
> >  	}
> > -- 
> > 2.18.1
> > 
> 
> -- 
> 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