[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v3,1/3] lib/kms: Commit primary plane props with COMMIT_LEGACY

[Ville Syrjälä]

On Thu, Apr 22, 2021 at 12:01:42PM +0200, Daniel Vetter wrote:
> On Wed, Apr 21, 2021 at 08:11:10PM +0300, Ville Syrjälä wrote:
> > On Wed, Apr 21, 2021 at 03:42:56AM -0000, Patchwork wrote:
> > > == Series Details ==
> > > 
> > > Series: series starting with [i-g-t,v3,1/3] lib/kms: Commit primary plane props with COMMIT_LEGACY
> > > URL   : https://patchwork.freedesktop.org/series/89278/
> > > State : success
> > 
> > So apparently avoiding those two redundant color encoding/range
> > setprop ioctls when chaging the fb does avoid the test failure
> > on glk. How on earth two setprop ioctl can do this is a mystery
> > The setprops should essentially be nops, and should just result
> > in reprogramming the plane registers a few times using an
> > identical state.
> 
> Might we need a full modeset for the hw to pick them up perhaps? Or at
> least plane enable/disable cycle. Doesn't make sense, but hey hw, maybe it
> only picks these up on initial enable.

I was pondering about a resurgence of the old "some DSPCNTR bits
can't be changed while in CxSR" issue. But that sort of thing
should likely make the pixel format tests fail since they do
switch these around dynamically. Another issue is that these bits
aren't supposed to affect RGB scanout at all, and those extra
setprop ioctls aren't actually changing the state at all.

> Only other thing I can think of is to maybe double check the state
> readout to make sure we really program this correctly in both cases.

Yeah, some kind of mishap in programming is what I was thinking
initially. So far didn't see any obvious issues in the code. I guess
I cound stick a bit of readout somewhere to confirm some of the
bits at least make sense.

But it might actually be some crc enable/disable race. At least I was
seeing some oddball crcs yesterday when I started to run the inner
bits of these tests in a loop. The code even still has that super
sketchy "let's throw out an extra crc on gen8 because it sometimes
looks bad" hack. I guess no one ever root caused that. I'll dig
into it a bit more.

-- 
Ville Syrjälä
Intel