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

["Kazlauskas, Nicholas" <Nicholas.Kazlauskas@amd.com>]

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.

Nicholas Kazlauskas

> ---
>   tests/kms_plane.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 13d36254dfe3..e4f3b8d68748 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -30,6 +30,13 @@
>   #include <stdio.h>
>   #include <string.h>
>   
> +/*
> + * Throw away enough lsbs in pixel formats tests
> + * to get a match despite some differences between
> + * the software and hardware YCbCr<->RGB conversion
> + * routines.
> + */
> +#define LUT_MASK 0xf800
>   
>   typedef struct {
>   	float red;
> @@ -690,7 +697,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
>   			continue;
>   
>   		if (format == DRM_FORMAT_C8) {
> -			if (!set_c8_legacy_lut(data, pipe, 0xfc00))
> +			if (!set_c8_legacy_lut(data, pipe, LUT_MASK))
>   				continue;
>   		} else {
>   			if (!igt_fb_supported_format(format))
> @@ -709,7 +716,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
>   							ref_crc, &fb);
>   
>   		if (format == DRM_FORMAT_C8)
> -			set_legacy_lut(data, pipe, 0xfc00);
> +			set_legacy_lut(data, pipe, LUT_MASK);
>   	}
>   
>   	igt_pipe_crc_stop(data->pipe_crc);
> @@ -744,7 +751,7 @@ test_pixel_formats(data_t *data, enum pipe pipe)
>   
>   	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
>   
> -	set_legacy_lut(data, pipe, 0xfc00);
> +	set_legacy_lut(data, pipe, LUT_MASK);
>   
>   	test_init(data, pipe);
>   
> 

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