[Intel-gfx] [PATCH 6/6] drm/i915: Add support for CCS modifiers

[Daniel Vetter]

On Wed, Aug 2, 2017 at 5:43 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On 17-08-02 12:14:15, Daniel Vetter wrote:
>>
>> On Tue, Aug 01, 2017 at 09:14:50AM -0700, Ben Widawsky wrote:
>>>
>>> On 17-07-31 10:29:55, Daniel Vetter wrote:
>>> > On Sat, Jul 29, 2017 at 09:25:50AM -0700, Ben Widawsky wrote:
>>> > > On 17-07-29 13:53:10, Daniel Stone wrote:
>>> > > > Hi Ben,
>>> > > >
>>> > > > On 26 July 2017 at 19:08, Ben Widawsky <ben at bwidawsk.net> wrote:
>>> > > > > +       } else if (INTEL_GEN(dev_priv) >= 9) {
>>> > > > >                 intel_primary_formats = skl_primary_formats;
>>> > > > >                 num_formats = ARRAY_SIZE(skl_primary_formats);
>>> > > > > -               modifiers = skl_format_modifiers;
>>> > > > > +               if (pipe >= PIPE_C)
>>> > > > > +                       modifiers = skl_format_modifiers_ccs;
>>> > > > > +               else
>>> > > > > +                       modifiers = skl_format_modifiers_noccs;
>>> > > >
>>> > > > Shouldn't that be (pipe < PIPE_C)?
>>> > > >
>>> > > > Cheers,
>>> > > > Daniel
>>> > >
>>> > > Yep. It was wrong in v7 too :/. I have it fixed locally.
>>> >
>>> > Shouldn't the igt catch this, or does it not try to exercise all the
>>> > plane/crtc combos there are?
>>> > -Daniel
>>>
>>> I don't know whether or not IGT should have caught this, but it wouldn't
>>> have
>>> because I had been sending these without Ville's patches, so my patches
>>> alone
>>> don't even build (since his patches defined the modifiers).
>>
>>
>> You can run igt testcases locally. I expect you to do that, at least for
>> the stuff you're touching.
>>
>> Does this mean there's no igts for this at all?
>> -Daniel
>
>
> I haven't been running IGT locally, so I don't know if they touch this path.
> We've done all testing so far with kmscube, X, and weston; then measure the
> overall bandwidth. In this case, I'd stop testing these since Jason/Daniel
> took
> over my other patches. I don't even know how to locally make sure I get a
> display on pipe C.
>
> You could easily make an IGT make sure that you get back the right list of
> modifiers for a given pipe on a given GEN. Probably should do one.

That's not a terribly interesting testcase (we have none of this kind
I think). But I thought there was a CCS igt that created some simple
compressed buffers and tried to display them, and then checked it all
looked good using crcs, i.e. an actual functional testcase, and that
should have caught the mislisten on pipe C (if it bothered to test all
planes that expose the CCS modifier). Iirc Ville had that. We do need
that for merging the kernel side.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch