On Wed, Aug 2, 2017 at 5:43 PM, Ben Widawsky ben@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@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