Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_plane: Print count of mismatched colors

[Petri Latvala <petri.latvala@intel.com>]

On Fri, Mar 15, 2019 at 12:41:35PM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2019-03-15 at 13:56 +0200, Martin Peres wrote:
> > On 15/03/2019 00:35, Dhinakaran Pandiyan wrote:
> > > On Thu, 2019-03-14 at 13:03 +0200, Arkadiusz Hiler wrote:
> > > > Currently we are printing one igt_warn for each CRC mismatch,
> > > > which
> > > > gets
> > > > quite overwhelming with having to see the same error 8 times for
> > > > each
> > > > color tested:
> > > > 
> > > > WARNING: CRC mismatch with format NV12 (0x3231564e) on A.3
> > > > WARNING: CRC mismatch with format NV12 (0x3231564e) on A.3
> > > > WARNING: CRC mismatch with format NV12 (0x3231564e) on A.3
> > > > WARNING: CRC mismatch with format NV12 (0x3231564e) on A.3
> > > > WARNING: CRC mismatch with format NV12 (0x3231564e) on A.3
> > > > WARNING: CRC mismatch with format NV12 (0x3231564e) on A.3
> > > > WARNING: CRC mismatch with format NV12 (0x3231564e) on A.3
> > > > WARNING: CRC mismatch with format NV12 (0x3231564e) on A.3
> > > > 
> > > > Since the most interesting information here is which format on
> > > > which
> > > > pipe/plane is broken we can skip igt_warn just once.
> > > > 
> > > > For those weirder and rarer case where just certain colors would
> > > > fail
> > > > we
> > > > still provide the count:
> > > 
> > > IMO it's better to have all the information in the logs,
> > > particularly
> > > for those 'weirder and rare cases, which are hard to reproduce. And
> > > don't see how count is useful. I think, we should be even logging
> > > the
> > > RGB values for the mismatching cases.
> > 
> > I definitely stand behind your reasoning here, however I have a
> > strong
> > requirement that I need to be able to file filters for this, and
> > multiple lines are just too hard to file filters when you also need
> > to
> > check that other formats are also failing and no new one comes in
> > without me to get new values.
> It would have been nice to include this justification in the commit
> message.
> 
> > 
> > Would you have an idea on how to get the colors failing in one line?
> > I
> > was initially proposing to just have a bitfield representing the
> > colors
> > failing in one value, but Arek found it hard to understand.
> > 
> > What do you think?
> A bit mask, like what Petri has done, is still useful to quickly
> identify failing patterns; we can always go back and check the color
> during debug.


Can I interpret this as an acked-by on the mask printing version?


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev