Re: [PATCH v3 4/6] drm/i915/crc: Make IPS workaround generic

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

On Thu, Feb 28, 2019 at 06:56:48PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 27, 2019 at 05:32:57PM -0800, José Roberto de Souza wrote:
> > Other features like PSR2 also needs to be disabled while getting CRC
> > so lets rename ips_force_disable to crc_enabled, drop all this checks
> > for pipe A and HSW and BDW and make it generic and
> > hsw_compute_ips_config() will take care of all the checks removed
> > from here.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c  | 10 +++++--
> >  drivers/gpu/drm/i915/intel_drv.h      |  3 +-
> >  drivers/gpu/drm/i915/intel_pipe_crc.c | 42 +++++++++------------------
> >  3 files changed, 24 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 816e8f124b3b..328967c642b3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6751,7 +6751,13 @@ static bool hsw_compute_ips_config(struct intel_crtc_state *crtc_state)
> >  	if (!hsw_crtc_state_ips_capable(crtc_state))
> >  		return false;
> >  
> > -	if (crtc_state->ips_force_disable)
> > +	/*
> > +	 * When IPS gets enabled, the pipe CRC changes. Since IPS gets
> > +	 * enabled and disabled dynamically based on package C states,
> > +	 * user space can't make reliable use of the CRCs, so let's just
> > +	 * completely disable it.
> > +	 */
> > +	if (crtc_state->crc_enabled)
> >  		return false;
> 
> Hmm. I was wondering how we even manage to pass the state checker with
> the current code. But apparently we don't have state checking for IPS.
> I would suggest moving this into hsw_compute_ips_config() and then
> adding the state checker (for HSW only though since BDW can't do the
> readout).

And of course this _is_ hsw_compute_ips_config(). Not sure what code
I was reading.

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