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

From: Mika Kahola <mika.kahola@intel.com>
To: dhinakaran.pandiyan@intel.com, Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Wait for vblank after register read
Date: Tue, 24 Apr 2018 11:14:34 +0300	[thread overview]
Message-ID: <1524557674.11552.2.camel@intel.com> (raw)
In-Reply-To: <1524257781.24522.13.camel@dk-H97M-D3H>

On Fri, 2018-04-20 at 13:56 -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-04-20 at 11:15 -0700, Rodrigo Vivi wrote:
> > 
> > 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.ht
> > > > ml
> > > 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....
> Yup.
> 
> Patch has nothing to do with the bug that it is trying to fix. The
> test
> doesn't read sink-crc's, so I don't think even a References tag is
> appropriate here.
Ok. I don't mind dropping this patch if you guys feel that this has no
relevance. As I said earlier, this is something that caught my eye when
looking into this CRC bug that we have.

> 
> 
> 
-- 
Mika Kahola - Intel OTC

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