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

Shankar, Uma
2019-07-02T16:44:33Z

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, July 2, 2019 7:00 PM
>To: Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>
>Cc: igt-dev@lists.freedesktop.org; Shankar, Uma <uma.shankar@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@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@intel.com>
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> With the title of this patch corrected ("Throw away yet another bit"),
>> this series is:
>>
>> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@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@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)
So,
Tested-by: Uma Shankar <uma.shankar@intel.com>

>--
>Ville Syrjälä
>Intel