Re: [igt-dev] [i-g-t, v3, 4/5] lib/igt_kms: Hook up connector dithering prop

Jeremy Cline
2020-08-18T21:18:34Z

Hi,

On Fri, Apr 17, 2020 at 05:10:24PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
>
> Nvidia display hardware provides a set of flexible dithering options for
> CRTCs. This dithering is actually noticeable in the CRC output for all
> available tap points, and can be seen as CRC values for identical frames
> cycling between either 2 or 4 values repeatedly (each one of these
> values is a different dithering phase applied to the source output). Of
> course, this is very likely to break tests using CRC readback since we
> don't expect the CRC to change if the source content hasn't changed.
>
> So, hook up support for configuring the dithering property and reset it
> to off from igt_display_reset() when applicable.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---

I'm not in a position to speak to the correctness of the change, but
from a pure code point of view it looks good to me, so for whatever it's
worth:

Reviewed-by: Jeremy Cline <jcline@redhat.com>

> lib/igt_kms.c | 6 ++++++
> lib/igt_kms.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e9621e7e..d45adfaf 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -421,6 +421,7 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> [IGT_CONNECTOR_LINK_STATUS] = "link-status",
> [IGT_CONNECTOR_MAX_BPC] = "max bpc",
> [IGT_CONNECTOR_HDR_OUTPUT_METADATA] = "HDR_OUTPUT_METADATA",
> + [IGT_CONNECTOR_DITHERING_MODE] = "dithering mode",
> };
>
> /*
> @@ -1802,6 +1803,10 @@ static void igt_output_reset(igt_output_t *output)
> if (igt_output_has_prop(output, IGT_CONNECTOR_HDR_OUTPUT_METADATA))
> igt_output_set_prop_value(output,
> IGT_CONNECTOR_HDR_OUTPUT_METADATA, 0);
> +
> + if (igt_output_has_prop(output, IGT_CONNECTOR_DITHERING_MODE))
> + igt_output_set_prop_enum(output, IGT_CONNECTOR_DITHERING_MODE,
> + "off");
> }
>
> /**
> @@ -1816,6 +1821,7 @@ static void igt_output_reset(igt_output_t *output)
> * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> * %IGT_CONNECTOR_CONTENT_PROTECTION (if applicable)
> * %IGT_CONNECTOR_HDR_OUTPUT_METADATA (if applicable)
> + * - %IGT_CONNECTOR_DITHERING_MODE (if applicable)
> * - igt_output_override_mode() to default.
> *
> * For pipes:
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index adca59ac..4899e765 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -127,6 +127,7 @@ enum igt_atomic_connector_properties {
> IGT_CONNECTOR_LINK_STATUS,
> IGT_CONNECTOR_MAX_BPC,
> IGT_CONNECTOR_HDR_OUTPUT_METADATA,
> + IGT_CONNECTOR_DITHERING_MODE,
> IGT_NUM_CONNECTOR_PROPS
> };
>