[igt-dev] [PATCH i-g-t 3/3] tests/kms_plane: Throw away yet another bi

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>Sent: Tuesday, July 2, 2019 10:35 PM
>To: Shankar, Uma <uma.shankar at intel.com>
>Cc: Kazlauskas, Nicholas <Nicholas.Kazlauskas at amd.com>; igt-
>dev at lists.freedesktop.org
>Subject: Re: [igt-dev] [PATCH i-g-t 3/3] tests/kms_plane: Throw away yet another bi
>
>On Tue, Jul 02, 2019 at 04:44:28PM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>> >Sent: Tuesday, July 2, 2019 7:00 PM
>> >To: Kazlauskas, Nicholas <Nicholas.Kazlauskas at amd.com>
>> >Cc: igt-dev at lists.freedesktop.org; Shankar, Uma
>> ><uma.shankar at intel.com>
>> >Subject: Re: [igt-dev] [PATCH i-g-t 3/3] tests/kms_plane: Throw away
>> >yet another bi
>> >
>> >On Tue, Jul 02, 2019 at 01:20:47PM +0000, Kazlauskas, Nicholas wrote:
>> >> On 6/28/19 3:44 PM, Ville Syrjala wrote:
>> >> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> >> >
>> >> > CHV pipe A/C sprites are causing a crc mismatch with BT.601 when
>> >> > we keep the six msbs. If we throw away one more bit we get
>> >> > matches for BT.601. BT.709 matches even with 6 bits, as do the
>> >> > pipe B planes with their programmable CSC. Also IVB and KBL were
>> >> > both happy with 6 bits, so doesn't seem that that these CHV
>> >> > mismatches are due to bugs in our code, just the hw is a bit imprecise.
>> >> >
>> >> > Cc: Uma Shankar <uma.shankar at intel.com>
>> >> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> >>
>> >> With the title of this patch corrected ("Throw away yet another
>> >> bit"), this series is:
>> >>
>> >> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>> >>
>> >> These patches are useful to have.
>> >>
>> >> However, I'm not sure how I feel about the naming on all of these
>> >> new *_full functions or if they're really all needed. I think just
>> >> having one function that explicitly sets all of this is probably
>> >> enough rather than having these 4 new functions.
>> >
>> >I suppose I could just use igt_create_fb_with_bo_size() and hand roll
>> >the solid fill in the test.
>>
>> Yeah this sounds good. Overall this patch looks ok.
>> Reviewed-by: Uma Shankar <uma.shanka at intel.com>
>>
>> I tested this as well on ICL and looks like below test passes without any failure:
>> Subtest pixel-format-pipe-A-planes: SUCCESS (423.398s)
>
>Yeah, it already passes with the current upstream code with the wonky icsc
>coefficients. Might be nice to test how many correct bits we can get with the current
>vs. fixed coefficents.

Tried with below series:
https://patchwork.freedesktop.org/series/60480/

which has fixes for the coefficients.  I can see the tests passing still with the current IGT changes
introduced in this series.

There are some known limitations if we go for testing with 4K where I see some flips getting rejected
due to hardware limitations. For testing limited it to 1024x768 (1920x1080 as well should be fine I guess).

>> So,
>> Tested-by: Uma Shankar <uma.shankar at intel.com>
>>
>> >--
>> >Ville Syrjälä
>> >Intel
>
>--
>Ville Syrjälä
>Intel