[PATCH i-g-t 2/3] tests/kms_hdr: Set HDR meta-data while testing for panel capability

[Sharma, Swati2]

Hi Chaitanya,

On 11-08-2025 01:28 pm, Chaitanya Kumar Borah wrote:
> Checking if the panel supports a max_bpc of 10 is not enough to tell
> if the panel supports HDR. Set HDR metadata on the connector before
> going through the modes. Reset after done.
Can you please add closes tag gitlab# for the bug which is getting fixed 
(if exists) ?
Also, please elaborate "why" existing checks not enough?
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>
> ---
>   tests/kms_hdr.c | 114 ++++++++++++++++++++++++++----------------------
>   1 file changed, 61 insertions(+), 53 deletions(-)
>
> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
> index 33b6c42a6..76e103ac4 100644
> --- a/tests/kms_hdr.c
> +++ b/tests/kms_hdr.c
> @@ -153,6 +153,59 @@ static void draw_hdr_pattern(igt_fb_t *fb)
>   	igt_put_cairo_ctx(cr);
>   }
>   
> +/* Converts a double to 861-G spec FP format. */
> +static uint16_t calc_hdr_float(double val)
> +{
> +	return (uint16_t)(val * 50000.0);
> +}
> +
> +/* Fills some test values for ST2048 HDR output metadata.
> + *
> + * Note: there isn't really a standard for what the metadata is supposed
> + * to do on the display side of things. The display is free to ignore it
> + * and clip the output, use it to help tonemap to the content range,
> + * or do anything they want, really.
> + */
> +static void fill_hdr_output_metadata_st2048(struct hdr_output_metadata *meta)
> +{
> +	memset(meta, 0, sizeof(*meta));
> +
> +	meta->metadata_type = HDMI_STATIC_METADATA_TYPE1;
> +	meta->hdmi_metadata_type1.eotf = HDMI_EOTF_SMPTE_ST2084;
> +
> +	/* Rec. 2020 */
> +	meta->hdmi_metadata_type1.display_primaries[0].x =
> +		calc_hdr_float(0.708); /* Red */
> +	meta->hdmi_metadata_type1.display_primaries[0].y =
> +		calc_hdr_float(0.292);
> +	meta->hdmi_metadata_type1.display_primaries[1].x =
> +		calc_hdr_float(0.170); /* Green */
> +	meta->hdmi_metadata_type1.display_primaries[1].y =
> +		calc_hdr_float(0.797);
> +	meta->hdmi_metadata_type1.display_primaries[2].x =
> +		calc_hdr_float(0.131); /* Blue */
> +	meta->hdmi_metadata_type1.display_primaries[2].y =
> +		calc_hdr_float(0.046);
> +	meta->hdmi_metadata_type1.white_point.x = calc_hdr_float(0.3127);
> +	meta->hdmi_metadata_type1.white_point.y = calc_hdr_float(0.3290);
> +
> +	meta->hdmi_metadata_type1.max_display_mastering_luminance =
> +		1000; /* 1000 nits */
> +	meta->hdmi_metadata_type1.min_display_mastering_luminance =
> +		500;				   /* 0.05 nits */
> +	meta->hdmi_metadata_type1.max_fall = 1000; /* 1000 nits */
> +	meta->hdmi_metadata_type1.max_cll = 500;   /* 500 nits */
> +}
> +
> +/* Sets the HDR output metadata prop. */
> +static void set_hdr_output_metadata(data_t *data,
> +				    struct hdr_output_metadata const *meta)
> +{
> +	igt_output_replace_prop_blob(data->output,
> +				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
> +				     meta ? sizeof(*meta) : 0);
> +}
> +
>   /* Prepare test data. */
>   static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>   {
> @@ -380,15 +433,6 @@ static bool is_panel_hdr(data_t *data, igt_output_t *output)
>   	return ret;
>   }
>   
> -/* Sets the HDR output metadata prop. */
> -static void set_hdr_output_metadata(data_t *data,
> -				    struct hdr_output_metadata const *meta)
> -{
> -	igt_output_replace_prop_blob(data->output,
> -				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
> -				     meta ? sizeof(*meta) : 0);
> -}
> -
>   /* Sets the HDR output metadata prop with invalid size. */
>   static int set_invalid_hdr_output_metadata(data_t *data,
>   					   struct hdr_output_metadata const *meta,
> @@ -401,50 +445,6 @@ static int set_invalid_hdr_output_metadata(data_t *data,
>   	return igt_display_try_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>   }
>   
> -/* Converts a double to 861-G spec FP format. */
> -static uint16_t calc_hdr_float(double val)
> -{
> -	return (uint16_t)(val * 50000.0);
> -}
> -
> -/* Fills some test values for ST2048 HDR output metadata.
> - *
> - * Note: there isn't really a standard for what the metadata is supposed
> - * to do on the display side of things. The display is free to ignore it
> - * and clip the output, use it to help tonemap to the content range,
> - * or do anything they want, really.
> - */
> -static void fill_hdr_output_metadata_st2048(struct hdr_output_metadata *meta)
> -{
> -	memset(meta, 0, sizeof(*meta));
> -
> -	meta->metadata_type = HDMI_STATIC_METADATA_TYPE1;
> -	meta->hdmi_metadata_type1.eotf = HDMI_EOTF_SMPTE_ST2084;
> -
> -	/* Rec. 2020 */
> -	meta->hdmi_metadata_type1.display_primaries[0].x =
> -		calc_hdr_float(0.708); /* Red */
> -	meta->hdmi_metadata_type1.display_primaries[0].y =
> -		calc_hdr_float(0.292);
> -	meta->hdmi_metadata_type1.display_primaries[1].x =
> -		calc_hdr_float(0.170); /* Green */
> -	meta->hdmi_metadata_type1.display_primaries[1].y =
> -		calc_hdr_float(0.797);
> -	meta->hdmi_metadata_type1.display_primaries[2].x =
> -		calc_hdr_float(0.131); /* Blue */
> -	meta->hdmi_metadata_type1.display_primaries[2].y =
> -		calc_hdr_float(0.046);
> -	meta->hdmi_metadata_type1.white_point.x = calc_hdr_float(0.3127);
> -	meta->hdmi_metadata_type1.white_point.y = calc_hdr_float(0.3290);
> -
> -	meta->hdmi_metadata_type1.max_display_mastering_luminance =
> -		1000; /* 1000 nits */
> -	meta->hdmi_metadata_type1.min_display_mastering_luminance =
> -		500;				   /* 0.05 nits */
> -	meta->hdmi_metadata_type1.max_fall = 1000; /* 1000 nits */
> -	meta->hdmi_metadata_type1.max_cll = 500;   /* 500 nits */
> -}
> -
>   static void adjust_brightness(data_t *data, uint32_t flags)
>   {
>   	igt_backlight_context_t context;
> @@ -652,6 +652,7 @@ static void test_hdr(data_t *data, uint32_t flags)
>   {
>   	igt_display_t *display = &data->display;
>   	igt_output_t *output;
> +	struct hdr_output_metadata hdr;
>   
>   	igt_display_reset(display);
>   
> @@ -701,6 +702,9 @@ static void test_hdr(data_t *data, uint32_t flags)
>   
>   			prepare_test(data, output, pipe);
Please add comment here, "why" this is required.
>   
> +			fill_hdr_output_metadata_st2048(&hdr);
> +			set_hdr_output_metadata(data, &hdr);
Don't we need a commit here?
> +
>   			if (is_intel_device(data->fd) &&
>   			    !igt_max_bpc_constraint(display, pipe, output, 10)) {
>   				igt_info("%s: No suitable mode found to use 10 bpc.\n",
> @@ -710,6 +714,10 @@ static void test_hdr(data_t *data, uint32_t flags)
>   				break;
>   			}
>   
> +			set_hdr_output_metadata(data, NULL);
> +			igt_display_commit2(display, display->is_atomic ?
> +					    COMMIT_ATOMIC : COMMIT_LEGACY);
> +
>   			data->mode = igt_output_get_mode(output);
>   			data->w = data->mode->hdisplay;
>   			data->h = data->mode->vdisplay;