[igt-dev] [PATCH i-g-t v2] tests/kms_cursor_crc: refactoring cursor-alpha subtests

[Ville Syrjälä]

On Mon, Oct 26, 2020 at 12:13:56AM +0200, Juha-Pekka Heikkila wrote:
> On 23.10.2020 14.47, Melissa Wen wrote:
> > On 10/22, Juha-Pekka Heikkila wrote:
> >> On 22.10.2020 13.37, Ville Syrjälä wrote:
> >>> On Wed, Oct 21, 2020 at 05:59:56AM -0300, Melissa Wen wrote:
> >>>> Considering just a fully opaque or fully transparent cursor is not enough
> >>>> to check the composition of an ARGB cursor plane. For example, the cairo
> >>>> ARGB32 format uses pre-multiplied alpha, and this representation may only
> >>>> be evident when testing a translucent cursor.
> >>>>
> >>>> Therefore, this patch refactors the cursor-alpha-opaque and
> >>>> cursor-alpha-transparent subtests into just one subtest (cursor-alpha)
> >>>> that checks the alpha blending of a white cursor, with different alpha
> >>>> values, in the primary plane. This refactoring also generates some setup
> >>>> stuffs savings.
> >>>
> >>> Black background means you can't actually tell the
> >>> difference between premultiplied blending and no
> >>> blending.
> >>>
> >>> Oh, I guess restore_image() does end up painting
> >>> the test pattern into the primary plane's fb? Are
> >>> we guaranteed to position the cursor over some non-black
> >>> parts?
> >>>
> >>> The other obvious concern is whether hardware and software
> >>> blending will produce the exact same results or not. I do see
> >>> you use 1/4 steps for the alpha, which I guess can make that
> >>> more likely.
> >>
> >> I don't think verifying those blending levels will be possible when
> >> comparing hw vs sw image, to me it seemed different gens behave different
> >> with cursor alpha. On this test to make it fail reliably even with alpha
> >> value 1.0 one just need to change from cursor image white sub block into
> >> grey level 0.5.
> >>
> > Hi, Ville and Juha-Pekka
> > 
> > Thanks for feedback.
> > 
> > To be honest, I'm a little confused by the issues you raised.
> > Maybe I'm not getting it right.
> > 
> > (1) We currently have two test cases to check the composition of the
> > cursor plane with the primary: alpha = 0 and alpha = 1, both with solid
> > black background and white cursor. For these cases, the premultiplied or
> > straight alpha blending behave equally, since the cursor is just all 00
> > or all FF.
> > 
> > but for any other non-extreme alpha value, if the alpha blending is
> > straight, it will end up "applying alpha twice". For example, if I
> > remember correctly, cairo draws a white cursor alpha 0.5 = 80808080; and
> > doing a *straight* alpha blending on black background results in
> > FF404040. And this is why this patch checks the composition for extreme
> > and non-extreme alpha values.
> > Another way could be just to check a third alpha value, without all
> > these alpha levels... I mean, if you think checking non-extreme values
> > makes sense.
> > 
> > (2) On the other hand, I guess that the "CRC mismatch for alpha = 0.5"
> > for IGTPW_5080 is not really related to the proper composition, but it
> > is a rounding issue (225 * 0.5 = 127.5). Because it didn't fail for any
> > other non-extreme alpha levels and to get 128 without rounding issue,
> > alpha should be 0.502.
> > 
> > Therefore, if the problem here is rounding, would be better to take this
> > problematic case off, right?
> > 
> 
> ..just some random thoughts..
> 
> All cursor tests are running with alpha enabled, most of them just set 
> it to maximum value. This is because cursor plane did support only pixel 
> formats with alpha. As for those alphas, this test for example 
> kms_cursor_crc at pipe-*-cursor-*x*-onscreen is quite fragile across 
> platforms with alphas even if it explicitly doesn't try to test alpha, 
> from this I draw those alphas generally are not behaving same in all 
> platforms.
> 
> For those differences on gens with alpha behavior on kernel side you'll 
> find in intel_display.c icl_set_pipe_chicken(..) where is set display wa 
> #1153 which was put in place just to make cursor tests happy for icl 
> back in time. I was checking same bit for tgl but didn't find one hence 
> I was correcting colors some time ago on this test to 'safe' colors (see 
> 43fa6db in igt git log). Earlier there was in use colors which had 0.5 
> value failing the test, this sound really familiar to your 0.5 alpha 
> problem. TBH it is interesting when you run test on tgl with 0.5 alpha 
> and it _doesn't_ fail because using those 0.5 colors on tgl did fail 
> this test.
> 
> As for your conclusion I think you are spot on this issue is about 
> rounding but I don't know was there a promise on hw side 0.5 means same 
> as what Cairo does. It was on test results now failing only on glk for 
> you? With those terms knowing alpha subtest is as fragile as everything 
> else on kms_cursor_crc that could be pushed upstream knowing there is 
> history baggage to drag on. IGT anyway is already committed to idea of 
> what some alpha/color/.. value mean for cairo will mean same thing for 
> display hw, there is kms_color struggling with similar problems.
> 
> I wonder what does Ville think about that Cairo vs hw issue?

One thing we could try is the "throw away some lsbs using the LUT"
trick from kms_plane pixel format tests. But depending on the blending
formula implemented in the hw I guess even that might not always work.

-- 
Ville Syrjälä
Intel