[Intel-gfx] [PATCH 2/2] drm/i915/skl+: Enable pipe CSC on cursor planes.

[Bob Paauwe]

On Fri, 28 Aug 2015 17:12:12 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
> 
> On 28 August 2015 at 16:55, Bob Paauwe <bob.j.paauwe at intel.com> wrote:
> > On Fri, 28 Aug 2015 15:19:04 +0100
> > Daniel Stone <daniel at fooishbar.org> wrote:
> >> For both this and the previous patch, cf. the corresponding patch for
> >> HSW/BDW[0], have you ensured these values are sanitised at startup,
> >> even if UEFI hasn't set something clever? Enabling fastboot on my
> >> (UEFI-based) BDW caused a black screen because were enabling CSC but
> >> with an empty table.
> >>
> >> Cheers,
> >> Daniel
> >>
> >> [0]: https://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg67294.html
> >
> > Hmm, no I didn't.  I assumed that it was all set up correctly since for
> > SKL+ the primary plane always has PIPE_CSC enabled.
> 
> So does HSW/BDW. ;) The problem is that a CSC is only applied in the
> modeset path; when using 'fastboot' to skip the original modeset (i.e.
> to achieve flicker-free and quicker boot), the plane configuration
> applies CSC/gamma, but the tables are only applied on modeset. So you
> may end up with an invalid configuration (and blank screen), without a
> similar patch.
> 
> > My two patches are
> > just to ensure that both the cursor and sprite planes also have it
> > enabled.  If all the planes are configured the same, it causes a lot of
> > CRC failures in the igt tests.
> >
> > Unless I'm missing something (very possible), the pipe CSC setup/lack of
> > setup is a separate issue.
> 
> Yeah, it is. But it'd be good to make sure Maarten's patch gets dragged in too.

That's what I thought you meant, but wanted to make sure I wasn't
confused. 

> 
> > Looking at Maarten's patch, it looks like mine above should have been
> > written as
> >
> >   if (HAS_DDI(dev))
> >
> > instead of all the separate conditions.
> 
> Yeah, indeed. I'd misread the HAS_DDI bit myself, so it seems fine.
> Are you able to test Maarten's patch to always program the CSC tables
> and make sure it doesn't break SKL/BXT?

I did apply his series and it doesn't change the behavior on my SKL,
but I also don't have any issues if I enable fastboot without his
series.  Which I guess means that my UEFI is setting up the table
correctly.
> 
> Thanks, and sorry for the confusion.

You made me dig into how CSC works a bit more and that a good thing!
Thanks for bring it up.

> 
> Cheers,
> Daniel

Bob

-- 
--
Bob Paauwe                  
Bob.J.Paauwe at intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193