Re: [PATCH] drm/i915: Wait for vblank after register read

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Apr 19, 2018 at 10:03:05AM +0300, Mika Kahola wrote:
> On Thu, 2018-04-19 at 09:11 +0300, Lofstedt, Marta wrote:
> > For the PW results: 
> > https://patchwork.freedesktop.org/series/41877/
> > 
> > it didn't fix the CRC mismatch on:
> >  https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8731/shard-
> > snb6/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
> This was expected that removing one vblank doesn't fix have an effect
> on crc mismatches. Theres something more in this issue.
> 
> > 
> > but that test has always failed on SNB: 
> > http://gfx-ci.fi.intel.com/cibuglog/?action_failures_history=-1&failu
> > res_test=igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-
> > multidraw&failures_machine=shard-snb
> > 
> > so I don't think you brake anything with this patch.
> > 
> > Mika, do you know if waiting for the extra vblank was done with some
> > purpose?
> That I don't know if it was intentional. I'll try to find out why it
> was needed.

sink CRC calculation starts on the next active frame and takes a full
frame to finish the calculation. So 2 vblanks before the result is ready.

I don't believe we should move it for after reading it.

But it is a fact that this sink crc was always only a headache.
If we manage to move PSR tests to use the interrupts and status bits only
than we will be able to kill this sink crc code entirely....

> 
> > 
> > 
> > > 
> > > -----Original Message-----
> > > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> > > Behalf
> > > Of Mika Kahola
> > > Sent: Wednesday, April 18, 2018 10:57 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Subject: [Intel-gfx] [PATCH] drm/i915: Wait for vblank after
> > > register read
> > > 
> > > When reading out CRC's we  wait for a vblank on
> > > intel_dp_sink_crc_start()
> > > function. When we start reading out CRC's in intel_dp_sink_crc()
> > > loop we
> > > first wait for a vblank yielding that all in all we end up waiting
> > > two vblanks on
> > > the first iteration round. Therefore, let's move the
> > > intel_wait_for_vblank() as the last routine that we do in an
> > > iteration loop in
> > > intel_dp_sink_crc().
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103166
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c index 62f82c4..6eb97fa 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3972,13 +3972,14 @@ int intel_dp_sink_crc(struct intel_dp
> > > *intel_dp,
> > > struct intel_crtc_state *crtc_s
> > >  		return ret;
> > > 
> > >  	do {
> > > -		intel_wait_for_vblank(dev_priv, intel_crtc-
> > > > 
> > > > pipe);
> > > -
> > >  		if (drm_dp_dpcd_readb(&intel_dp->aux,
> > > 
> > > DP_TEST_SINK_MISC, &buf) < 0) {
> > >  			ret = -EIO;
> > >  			goto stop;
> > >  		}
> > > +
> > > +		intel_wait_for_vblank(dev_priv, intel_crtc-
> > > > 
> > > > pipe);
> > > +
> > >  		count = buf & DP_TEST_COUNT_MASK;
> > > 
> > >  	} while (--attempts && count == 0);
> > > --
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -- 
> Mika Kahola - Intel OTC
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx