[Intel-gfx] [PATCH] drm/i915: Wait for vblank after register read

[Mika Kahola]

On Fri, 2018-04-20 at 11:22 +0300, Jani Nikula wrote:
> On Fri, 20 Apr 2018, Mika Kahola <mika.kahola at intel.com> wrote:
> > 
> > On Thu, 2018-04-19 at 17:09 +0300, Jani Nikula wrote:
> > > 
> > > On Wed, 18 Apr 2018, Mika Kahola <mika.kahola at intel.com> wrote:
> > > > 
> > > > 
> > > > 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
> > > Umm, do the CI failures in the bug really use sink crc, or are
> > > they
> > > rather about pipe crc?
> > > 
> > The bug is more on pipe crc. This just caught my attention while I
> > was
> > looking into these bugs. 
> I think the practice we've adopted is,
> 
> Bugzilla: <bug that this patch should fix>
> 
> References: <bug or something else that this patch is related to>
Got it :) I try to remember this notation.

> 
> > 
> > Was there a reason why we need to wait two vblanks here before
> > running
> > the loop?
> I can't remember by heart. I'm not sure if it would make more sense
> to
> remove the vblank wait from intel_dp_sink_crc_start() instead. Even
> with
> your patch, there'll still be an extra vblank wait, you just move it
> to
> a different place.
We could remove vblank wait form intel_dp_sink_crc_start(). Maybe that
would be more logical place for the removal. As CI runs pointed out
this patch didn't fix the actual bug so should I drop this change or
should we still try optimize the code a bit?

Cheers,
Mika

> 
> BR,
> Jani.
> 
> 
> > 
> > 
> > > 
> > > BR,
> > > Jani.
> > > 
> > > 
> > > > 
> > > > 
> > > > Signed-off-by: Mika Kahola <mika.kahola at 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);