public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Sharma, Swati2" <swati2.sharma@intel.com>
To: Jani Nikula <jani.nikula@intel.com>,
	Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: <igt-dev@lists.freedesktop.org>,
	Suraj Kandpal <suraj.kandpal@intel.com>,
	 Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>,
	Juha-Pekka Heikkila <juha-pekka.heikkila@intel.com>,
	Karthik B S <karthik.b.s@intel.com>
Subject: Re: [PATCH i-g-t, v3 1/5] lib/igt_hdr: Move HDR helpers from kms_hdr into shared library
Date: Tue, 28 Apr 2026 17:03:25 +0530	[thread overview]
Message-ID: <c1ac896c-bf97-4133-b941-de9dba3d71ce@intel.com> (raw)
In-Reply-To: <033657672987bf60eab2d60756c105a8b8010f2c@intel.com>

Hi Jani

On 21-04-2026 06:11 pm, Jani Nikula wrote:
> On Mon, 20 Apr 2026, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
>> Hi Swati,
>> On 2026-04-18 at 03:08:14 +0530, Swati Sharma wrote:
>>> Introduce lib/igt_hdr.{c,h} containing metadata fill helpers, EOTF enums,
>>> ST2084 construction, and blob programming utilities. This allows
>>> kms_hdr and upcoming tests (e.g., HDR support in kms_frontbuffer_tracking)
>>> to share common HDR code.
>>>
>>> v2: -place igt headers in alphabetical order (Kamil)
>>>
>>> Co-developed-by: Claude Opus 4.6
>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>>> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
>>> ---
>>>   lib/igt_hdr.c   | 217 +++++++++++++++++++++++++++++++++++
>>>   lib/igt_hdr.h   |  33 ++++++
>>>   lib/meson.build |   1 +
>>>   tests/kms_hdr.c | 297 ++++++++----------------------------------------
>>>   4 files changed, 300 insertions(+), 248 deletions(-)
>>>   create mode 100644 lib/igt_hdr.c
>>>   create mode 100644 lib/igt_hdr.h
>>>
>>> diff --git a/lib/igt_hdr.c b/lib/igt_hdr.c
>>> new file mode 100644
>>> index 000000000..5feb1d917
>>> --- /dev/null
>>> +++ b/lib/igt_hdr.c
>>> @@ -0,0 +1,217 @@
>>> +// SPDX-License-Identifier: MIT
>>> +/*
>>> + * Copyright © 2025 Intel Corporation
>> Year 2026.
>>
>>> + */
>>> +
>>> +#include "igt.h"
>>> +#include "igt_edid.h"
>>> +#include "igt_hdr.h"
>>> +
>>> +#include <fcntl.h>
>>> +#include <termios.h>
>>> +#include <unistd.h>
>>> +
>>> +/* HDR EDID parsing. */
>> Should this be in lib/igt_edid.c?
> Well, should this just use libdislay-info, like I suggested in another
> series adding more ad hoc EDID parsing [1].
>
> There is no point in reimplementing full blown EDID parsing in IGT. It's
> more than likely to be wrong. Like, just at a glance, the current HDR
> parsing doesn't look at CTA data blocks inside DisplayID extensions. It
> doesn't understand HF-EEODB. Etc. Arguably it's more work to fix it than
> to switch to libdislay-info.
>
> BR,
> Jani.
>
>
> [1] https://lore.kernel.org/r/57222f9789fda352ed26606c1f80e35fcdeb3d37@intel.com

Ack

Kamil has created 
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/work_items/204

Will work on this.

>
>>> +#define CTA_EXTENSION_VERSION		0x03
>>> +#define HDR_STATIC_METADATA_BLOCK       0x06
>>> +#define USE_EXTENDED_TAG		0x07
>>> +
>>> +static bool cta_block(const char *edid_ext)
>>> +{
>>> +	/*
>>> +	 * Byte 1: 0x07 indicates Extended Tag
>>> +	 * Byte 2: 0x06 indicates HDMI Static Metadata Block
>>> +	 * Byte 3: bits 0 to 5 identify EOTF functions supported by sink
>>> +	 *	       where ET_0: Traditional Gamma - SDR Luminance Range
>>> +	 *	             ET_1: Traditional Gamma - HDR Luminance Range
>>> +	 *	             ET_2: SMPTE ST 2084
>>> +	 *	             ET_3: Hybrid Log-Gamma (HLG)
>>> +	 *	             ET_4 to ET_5: Reserved for future use
>>> +	 */
>>> +
>>> +	if ((((edid_ext[0] & 0xe0) >> 5 == USE_EXTENDED_TAG) &&
>>> +	      (edid_ext[1] == HDR_STATIC_METADATA_BLOCK)) &&
>>> +	     ((edid_ext[2] & HDMI_EOTF_TRADITIONAL_GAMMA_HDR) ||
>>> +	      (edid_ext[2] & HDMI_EOTF_SMPTE_ST2084)))
>>> +			return true;
>>> +
>>> +	return false;
>>> +}
>>> +
>>> +/* Returns true if panel supports HDR. */
>> All public lib functions, this one and following ones, needs
>> a description, see for example lib/igt_configfs.c
>>
>>> +bool igt_is_panel_hdr(int fd, igt_output_t *output)
>> Or maybe it should be in lib/igt_hdr_panel.h|c?
>>
>> +cc Karthik and J-P
>>
>>> +{
>>> +	bool ok;
>>> +	int i, j, offset;
>>> +	uint64_t edid_blob_id;
>>> +	drmModePropertyBlobRes *edid_blob;
>>> +	const struct edid_ext *edid_ext;
>>> +	const struct edid *edid;
>>> +	const struct edid_cea *edid_cea;
>>> +	const char *cea_data;
>>> +	bool ret = false;
>>> +
>>> +	ok = kmstest_get_property(fd, output->id,
>>> +			DRM_MODE_OBJECT_CONNECTOR, "EDID",
>>> +			NULL, &edid_blob_id, NULL);
>>> +
>>> +	if (!ok || !edid_blob_id)
>>> +		return ret;
>>> +
>>> +	edid_blob = drmModeGetPropertyBlob(fd, edid_blob_id);
>>> +	igt_assert(edid_blob);
>>> +
>>> +	edid = (const struct edid *) edid_blob->data;
>>> +	igt_assert(edid);
>>> +
>>> +	for (i = 0; i < edid->extensions_len; i++) {
>>> +		edid_ext = &edid->extensions[i];
>>> +		edid_cea = &edid_ext->data.cea;
>>> +
>>> +		/* HDR not defined in CTA Extension Version < 3. */
>>> +		if ((edid_ext->tag != EDID_EXT_CEA) ||
>>> +		    (edid_cea->revision != CTA_EXTENSION_VERSION))
>>> +				continue;
>>> +		else {
>>> +			offset = edid_cea->dtd_start;
>>> +			cea_data = edid_cea->data;
>>> +
>>> +			for (j = 0; j < offset; j += (cea_data[j] & 0x1f) + 1) {
>>> +				ret = cta_block(cea_data + j);
>>> +
>>> +				if (ret)
>>> +					break;
>>> +			}
>>> +		}
>>> +	}
>>> +
>>> +	drmModeFreePropertyBlob(edid_blob);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +/* Converts a double to 861-G spec FP format. */
>>> +uint16_t igt_hdr_calc_float(double val)
>>> +{
>>> +	return (uint16_t)(val * 50000.0);
>>> +}
>>> +
>>> +/* Fills some test values for ST2084 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.
>>> + */
>>> +void igt_hdr_fill_st2084(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 =
>>> +		igt_hdr_calc_float(0.708); /* Red */
>>> +	meta->hdmi_metadata_type1.display_primaries[0].y =
>>> +		igt_hdr_calc_float(0.292);
>>> +	meta->hdmi_metadata_type1.display_primaries[1].x =
>>> +		igt_hdr_calc_float(0.170); /* Green */
>>> +	meta->hdmi_metadata_type1.display_primaries[1].y =
>>> +		igt_hdr_calc_float(0.797);
>>> +	meta->hdmi_metadata_type1.display_primaries[2].x =
>>> +		igt_hdr_calc_float(0.131); /* Blue */
>>> +	meta->hdmi_metadata_type1.display_primaries[2].y =
>>> +		igt_hdr_calc_float(0.046);
>>> +	meta->hdmi_metadata_type1.white_point.x = igt_hdr_calc_float(0.3127);
>>> +	meta->hdmi_metadata_type1.white_point.y = igt_hdr_calc_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 */
>>> +}
>>> +
>>> +/* Fills some test values for HDR metadata targeting SDR. */
>>> +void igt_hdr_fill_sdr(struct hdr_output_metadata *meta)
>>> +{
>>> +	memset(meta, 0, sizeof(*meta));
>>> +
>>> +	meta->metadata_type = HDMI_STATIC_METADATA_TYPE1;
>>> +	meta->hdmi_metadata_type1.eotf = HDMI_EOTF_TRADITIONAL_GAMMA_SDR;
>>> +
>>> +	/* Rec. 709 */
>>> +	meta->hdmi_metadata_type1.display_primaries[0].x =
>>> +		igt_hdr_calc_float(0.640); /* Red */
>>> +	meta->hdmi_metadata_type1.display_primaries[0].y =
>>> +		igt_hdr_calc_float(0.330);
>>> +	meta->hdmi_metadata_type1.display_primaries[1].x =
>>> +		igt_hdr_calc_float(0.300); /* Green */
>>> +	meta->hdmi_metadata_type1.display_primaries[1].y =
>>> +		igt_hdr_calc_float(0.600);
>>> +	meta->hdmi_metadata_type1.display_primaries[2].x =
>>> +		igt_hdr_calc_float(0.150); /* Blue */
>>> +	meta->hdmi_metadata_type1.display_primaries[2].y =
>>> +		igt_hdr_calc_float(0.006);
>>> +	meta->hdmi_metadata_type1.white_point.x = igt_hdr_calc_float(0.3127);
>>> +	meta->hdmi_metadata_type1.white_point.y = igt_hdr_calc_float(0.3290);
>>> +
>>> +	meta->hdmi_metadata_type1.max_display_mastering_luminance = 0;
>>> +	meta->hdmi_metadata_type1.min_display_mastering_luminance = 0;
>>> +	meta->hdmi_metadata_type1.max_fall = 0;
>>> +	meta->hdmi_metadata_type1.max_cll = 0;
>>> +}
>>> +
>>> +/* Sets the HDR output metadata prop. */
>>> +void igt_hdr_set_metadata(igt_output_t *output,
>>> +                          const struct hdr_output_metadata *meta)
>>> +{
>>> +	igt_output_replace_prop_blob(output,
>>> +				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
>>> +				     meta ? sizeof(*meta) : 0);
>>> +}
>>> +
>>> +/* Sets the HDR output metadata prop with invalid size. */
>>> +int igt_hdr_set_invalid_metadata(igt_output_t *output,
>> Do we need this function in lib?
>>
>>> +                                 const struct hdr_output_metadata *meta,
>>> +				 size_t len)
>>> +{
>>> +	igt_output_replace_prop_blob(output,
>>> +				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
>>> +				     meta ? len : 0);
>>> +
>>> +	return igt_display_try_commit_atomic(output->display,
>>> +	                                     DRM_MODE_ATOMIC_ALLOW_MODESET,
>>> +					     NULL);
>>> +}
>>> +
>>> +/* Returns true if an output supports max bpc property. */
>>> +bool igt_output_supports_max_bpc(igt_output_t *output)
>>> +{
>>> +	return igt_output_has_prop(output, IGT_CONNECTOR_MAX_BPC) &&
>>> +	       igt_output_get_prop(output, IGT_CONNECTOR_MAX_BPC);
>>> +}
>>> +
>>> +/* Returns true if an output supports HDR metadata property. */
>>> +bool igt_output_supports_hdr(igt_output_t *output)
>>> +{
>>> +	return igt_output_has_prop(output, IGT_CONNECTOR_HDR_OUTPUT_METADATA);
>>> +}
>>> +
>>> +void igt_hdr_disable(igt_output_t *output)
>>> +{
>>> +	igt_hdr_set_metadata(output, NULL);
>>> +	igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 8);
>>> +}
>>> +
>>> +void igt_hdr_enable(igt_output_t *output)
>>> +{
>>> +	struct hdr_output_metadata meta;
>>> +
>>> +	/* Fill HDR metadata and enable it on the output */
>>> +	igt_hdr_fill_st2084(&meta);
>>> +	igt_hdr_set_metadata(output, &meta);
>>> +	igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 10);
>>> +}
>>> diff --git a/lib/igt_hdr.h b/lib/igt_hdr.h
>>> new file mode 100644
>>> index 000000000..0df1ac5ed
>>> --- /dev/null
>>> +++ b/lib/igt_hdr.h
>>> @@ -0,0 +1,33 @@
>>> +#ifndef IGT_HDR_H
>>> +#define IGT_HDR_H
>>> +
>>> +#include "igt_edid.h"
>>> +#include "igt_kms.h"
>>> +
>>> +enum hdmi_eotf {
>>> +	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
>>> +	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
>>> +	HDMI_EOTF_SMPTE_ST2084,
>>> +};
>>> +
>>> +/* DRM HDR definitions. Not in the UAPI header, unfortunately. */
>>> +enum hdmi_metadata_type {
>>> +	HDMI_STATIC_METADATA_TYPE1 = 0,
>>> +};
>>> +
>>> +bool igt_is_panel_hdr(int fd, igt_output_t *output);
>>> +
>>> +uint16_t igt_hdr_calc_float(double val);
>>> +void igt_hdr_fill_st2084(struct hdr_output_metadata *meta);
>>> +void igt_hdr_fill_sdr(struct hdr_output_metadata *meta);
>>> +
>>> +void igt_hdr_set_metadata(igt_output_t *output,
>>> +                          const struct hdr_output_metadata *meta);
>>> +int igt_hdr_set_invalid_metadata(igt_output_t *output,
>>> +                                 const struct hdr_output_metadata *meta,
>>> +				 size_t len);
>>> +
>>> +bool igt_output_supports_max_bpc(igt_output_t *output);
>>> +bool igt_output_supports_hdr(igt_output_t *output);
>>> +
>>> +#endif /* IGT_HDR_H */
>>> diff --git a/lib/meson.build b/lib/meson.build
>>> index 0e7efadf3..d76a0d332 100644
>>> --- a/lib/meson.build
>>> +++ b/lib/meson.build
>>> @@ -111,6 +111,7 @@ lib_sources = [
>>>   	'igt_vc4.c',
>>>   	'igt_vmwgfx.c',
>>>   	'igt_psr.c',
>>> +	'igt_hdr.c',
>> Can you move it down to more proper place along with igt_psr.c?
>>
>> Regards,
>> Kamil
>>
>>>   	'igt_amd.c',
>>>   	'igt_edid.c',
>>>   	'igt_eld.c',
>>> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
>>> index b215b0e6c..eb336f14d 100644
>>> --- a/tests/kms_hdr.c
>>> +++ b/tests/kms_hdr.c
>>> @@ -33,6 +33,7 @@
>>>   #include <termios.h>
>>>   #include <unistd.h>
>>>   #include "igt_edid.h"
>>> +#include "igt_hdr.h"
>>>   
>>>   /**
>>>    * SUBTEST: bpc-switch
>>> @@ -70,24 +71,8 @@
>>>   
>>>   IGT_TEST_DESCRIPTION("Test HDR metadata interfaces and bpc switch");
>>>   
>>> -/* HDR EDID parsing. */
>>> -#define CTA_EXTENSION_VERSION		0x03
>>> -#define HDR_STATIC_METADATA_BLOCK       0x06
>>> -#define USE_EXTENDED_TAG		0x07
>>> -
>>>   #define BACKLIGHT_PATH "/sys/class/backlight"
>>>   
>>> -/* DRM HDR definitions. Not in the UAPI header, unfortunately. */
>>> -enum hdmi_metadata_type {
>>> -	HDMI_STATIC_METADATA_TYPE1 = 0,
>>> -};
>>> -
>>> -enum hdmi_eotf {
>>> -	HDMI_EOTF_TRADITIONAL_GAMMA_SDR,
>>> -	HDMI_EOTF_TRADITIONAL_GAMMA_HDR,
>>> -	HDMI_EOTF_SMPTE_ST2084,
>>> -};
>>> -
>>>   /* HDR test formats: 10bpc + FP16 */
>>>   static const uint32_t hdr_test_formats[] = {
>>>   	DRM_FORMAT_XRGB2101010,
>>> @@ -154,59 +139,6 @@ static void draw_hdr_pattern(igt_fb_t *fb)
>>>   	igt_paint_test_pattern_color_fb(fb->fd, fb, 1.0, 1.0, 1.0);
>>>   }
>>>   
>>> -/* 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 ST2084 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_st2084(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, igt_crtc_t *crtc)
>>>   {
>>> @@ -308,13 +240,6 @@ static void test_bpc_switch_on_output(data_t *data, igt_crtc_t *crtc,
>>>   	igt_remove_fb(data->fd, &afb);
>>>   }
>>>   
>>> -/* Returns true if an output supports max bpc property. */
>>> -static bool has_max_bpc(igt_output_t *output)
>>> -{
>>> -	return igt_output_has_prop(output, IGT_CONNECTOR_MAX_BPC) &&
>>> -	       igt_output_get_prop(output, IGT_CONNECTOR_MAX_BPC);
>>> -}
>>> -
>>>   static void test_bpc_switch(data_t *data, uint32_t flags)
>>>   {
>>>   	igt_display_t *display = &data->display;
>>> @@ -325,7 +250,7 @@ static void test_bpc_switch(data_t *data, uint32_t flags)
>>>   	for_each_connected_output(display, output) {
>>>   		igt_crtc_t *crtc;
>>>   
>>> -		if (!has_max_bpc(output)) {
>>> +		if (!igt_output_supports_max_bpc(output)) {
>>>   			igt_info("%s: Doesn't support IGT_CONNECTOR_MAX_BPC.\n",
>>>   				 igt_output_name(output));
>>>   			continue;
>>> @@ -375,92 +300,6 @@ static void test_bpc_switch(data_t *data, uint32_t flags)
>>>   	}
>>>   }
>>>   
>>> -static bool cta_block(const char *edid_ext)
>>> -{
>>> -	/*
>>> -	 * Byte 1: 0x07 indicates Extended Tag
>>> -	 * Byte 2: 0x06 indicates HDMI Static Metadata Block
>>> -	 * Byte 3: bits 0 to 5 identify EOTF functions supported by sink
>>> -	 *	       where ET_0: Traditional Gamma - SDR Luminance Range
>>> -	 *	             ET_1: Traditional Gamma - HDR Luminance Range
>>> -	 *	             ET_2: SMPTE ST 2084
>>> -	 *	             ET_3: Hybrid Log-Gamma (HLG)
>>> -	 *	             ET_4 to ET_5: Reserved for future use
>>> -	 */
>>> -
>>> -	if ((((edid_ext[0] & 0xe0) >> 5 == USE_EXTENDED_TAG) &&
>>> -	      (edid_ext[1] == HDR_STATIC_METADATA_BLOCK)) &&
>>> -	     ((edid_ext[2] & HDMI_EOTF_TRADITIONAL_GAMMA_HDR) ||
>>> -	      (edid_ext[2] & HDMI_EOTF_SMPTE_ST2084)))
>>> -			return true;
>>> -
>>> -	return false;
>>> -}
>>> -
>>> -/* Returns true if panel supports HDR. */
>>> -static bool is_panel_hdr(data_t *data, igt_output_t *output)
>>> -{
>>> -	bool ok;
>>> -	int i, j, offset;
>>> -	uint64_t edid_blob_id;
>>> -	drmModePropertyBlobRes *edid_blob;
>>> -	const struct edid_ext *edid_ext;
>>> -	const struct edid *edid;
>>> -	const struct edid_cea *edid_cea;
>>> -	const char *cea_data;
>>> -	bool ret = false;
>>> -
>>> -	ok = kmstest_get_property(data->fd, output->id,
>>> -			DRM_MODE_OBJECT_CONNECTOR, "EDID",
>>> -			NULL, &edid_blob_id, NULL);
>>> -
>>> -	if (!ok || !edid_blob_id)
>>> -		return ret;
>>> -
>>> -	edid_blob = drmModeGetPropertyBlob(data->fd, edid_blob_id);
>>> -	igt_assert(edid_blob);
>>> -
>>> -	edid = (const struct edid *) edid_blob->data;
>>> -	igt_assert(edid);
>>> -
>>> -	drmModeFreePropertyBlob(edid_blob);
>>> -
>>> -	for (i = 0; i < edid->extensions_len; i++) {
>>> -		edid_ext = &edid->extensions[i];
>>> -		edid_cea = &edid_ext->data.cea;
>>> -
>>> -		/* HDR not defined in CTA Extension Version < 3. */
>>> -		if ((edid_ext->tag != EDID_EXT_CEA) ||
>>> -		    (edid_cea->revision != CTA_EXTENSION_VERSION))
>>> -				continue;
>>> -		else {
>>> -			offset = edid_cea->dtd_start;
>>> -			cea_data = edid_cea->data;
>>> -
>>> -			for (j = 0; j < offset; j += (cea_data[j] & 0x1f) + 1) {
>>> -				ret = cta_block(cea_data + j);
>>> -
>>> -				if (ret)
>>> -					break;
>>> -			}
>>> -		}
>>> -	}
>>> -
>>> -	return ret;
>>> -}
>>> -
>>> -/* 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,
>>> -					   size_t length)
>>> -{
>>> -	igt_output_replace_prop_blob(data->output,
>>> -				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
>>> -				     meta ? length : 0);
>>> -
>>> -	return igt_display_try_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>> -}
>>> -
>>>   static void adjust_brightness(data_t *data, uint32_t flags)
>>>   {
>>>   	igt_backlight_context_t context;
>>> @@ -484,7 +323,6 @@ static void adjust_brightness(data_t *data, uint32_t flags)
>>>   }
>>>   
>>>   static void test_static_toggle(data_t *data, igt_crtc_t *crtc,
>>> -			       igt_output_t *output,
>>>   			       uint32_t format, uint32_t flags)
>>>   {
>>>   	igt_display_t *display = &data->display;
>>> @@ -500,30 +338,30 @@ static void test_static_toggle(data_t *data, igt_crtc_t *crtc,
>>>   
>>>   	draw_hdr_pattern(&afb);
>>>   
>>> -	fill_hdr_output_metadata_st2084(&hdr);
>>> +	igt_hdr_fill_st2084(&hdr);
>>>   
>>>   	/* Start with no metadata. */
>>>   	igt_plane_set_fb(data->primary, &afb);
>>>   	igt_plane_set_size(data->primary, data->w, data->h);
>>> -	set_hdr_output_metadata(data, NULL);
>>> +	igt_hdr_set_metadata(data->output, NULL);
>>>   	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 8);
>>>   
>>>   	if (flags & TEST_NEEDS_DSC) {
>>> -		igt_force_dsc_enable(data->fd, output->name);
>>> -		igt_assert(igt_is_force_dsc_enabled(data->fd, output->name));
>>> +		igt_force_dsc_enable(data->fd, data->output->name);
>>> +		igt_assert(igt_is_force_dsc_enabled(data->fd, data->output->name));
>>>   	}
>>>   
>>>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>>   	igt_assert_output_bpc_equal(crtc,
>>> -				    output, 8);
>>> +				    data->output, 8);
>>>   
>>>   	if (flags & TEST_NEEDS_DSC) {
>>> -		igt_force_dsc_disable(data->fd, output->name);
>>> -		igt_assert(igt_is_force_dsc_disabled(data->fd, output->name));
>>> +		igt_force_dsc_disable(data->fd, data->output->name);
>>> +		igt_assert(igt_is_force_dsc_disabled(data->fd, data->output->name));
>>>   	}
>>>   
>>>   	/* Apply HDR metadata and 10bpc. We expect a modeset for entering. */
>>> -	set_hdr_output_metadata(data, &hdr);
>>> +	igt_hdr_set_metadata(data->output, &hdr);
>>>   	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 10);
>>>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>>   	if (flags & TEST_INVALID_HDR) {
>>> @@ -537,7 +375,7 @@ static void test_static_toggle(data_t *data, igt_crtc_t *crtc,
>>>   	}
>>>   
>>>   	igt_assert_output_bpc_equal(crtc,
>>> -				    output, 10);
>>> +				    data->output, 10);
>>>   
>>>   	/* Verify that the CRC are equal after DPMS or suspend. */
>>>   	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
>>> @@ -545,23 +383,23 @@ static void test_static_toggle(data_t *data, igt_crtc_t *crtc,
>>>   	igt_pipe_crc_collect_crc(data->pipe_crc, &new_crc);
>>>   
>>>   	/* Disable HDR metadata and drop back to 8bpc. We expect a modeset for exiting. */
>>> -	set_hdr_output_metadata(data, NULL);
>>> +	igt_hdr_set_metadata(data->output, NULL);
>>>   	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 8);
>>>   
>>>   	if (flags & TEST_NEEDS_DSC) {
>>> -		igt_force_dsc_enable(data->fd, output->name);
>>> -		igt_assert(igt_is_force_dsc_enabled(data->fd, output->name));
>>> +		igt_force_dsc_enable(data->fd, data->output->name);
>>> +		igt_assert(igt_is_force_dsc_enabled(data->fd, data->output->name));
>>>   	}
>>>   
>>>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>>   	igt_assert_output_bpc_equal(crtc,
>>> -				    output, 8);
>>> +				    data->output, 8);
>>>   
>>>   	igt_assert_crc_equal(&ref_crc, &new_crc);
>>>   
>>>   	if (flags & TEST_NEEDS_DSC) {
>>> -		igt_force_dsc_disable(data->fd, output->name);
>>> -		igt_assert(igt_is_force_dsc_disabled(data->fd, output->name));
>>> +		igt_force_dsc_disable(data->fd, data->output->name);
>>> +		igt_assert(igt_is_force_dsc_disabled(data->fd, data->output->name));
>>>   	}
>>>   
>>>   cleanup:
>>> @@ -569,38 +407,7 @@ cleanup:
>>>   	igt_remove_fb(data->fd, &afb);
>>>   }
>>>   
>>> -/* Fills some test values for HDR metadata targeting SDR. */
>>> -static void fill_hdr_output_metadata_sdr(struct hdr_output_metadata *meta)
>>> -{
>>> -	memset(meta, 0, sizeof(*meta));
>>> -
>>> -	meta->metadata_type = HDMI_STATIC_METADATA_TYPE1;
>>> -	meta->hdmi_metadata_type1.eotf = HDMI_EOTF_TRADITIONAL_GAMMA_SDR;
>>> -
>>> -	/* Rec. 709 */
>>> -	meta->hdmi_metadata_type1.display_primaries[0].x =
>>> -		calc_hdr_float(0.640); /* Red */
>>> -	meta->hdmi_metadata_type1.display_primaries[0].y =
>>> -		calc_hdr_float(0.330);
>>> -	meta->hdmi_metadata_type1.display_primaries[1].x =
>>> -		calc_hdr_float(0.300); /* Green */
>>> -	meta->hdmi_metadata_type1.display_primaries[1].y =
>>> -		calc_hdr_float(0.600);
>>> -	meta->hdmi_metadata_type1.display_primaries[2].x =
>>> -		calc_hdr_float(0.150); /* Blue */
>>> -	meta->hdmi_metadata_type1.display_primaries[2].y =
>>> -		calc_hdr_float(0.006);
>>> -	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 = 0;
>>> -	meta->hdmi_metadata_type1.min_display_mastering_luminance = 0;
>>> -	meta->hdmi_metadata_type1.max_fall = 0;
>>> -	meta->hdmi_metadata_type1.max_cll = 0;
>>> -}
>>> -
>>>   static void test_static_swap(data_t *data, igt_crtc_t *crtc,
>>> -			     igt_output_t *output,
>>>   			     uint32_t format, uint32_t flags)
>>>   {
>>>   	igt_display_t *display = &data->display;
>>> @@ -622,26 +429,26 @@ static void test_static_swap(data_t *data, igt_crtc_t *crtc,
>>>   	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 8);
>>>   
>>>   	if (flags & TEST_NEEDS_DSC) {
>>> -		igt_force_dsc_enable(data->fd, output->name);
>>> -		igt_assert(igt_is_force_dsc_enabled(data->fd, output->name));
>>> +		igt_force_dsc_enable(data->fd, data->output->name);
>>> +		igt_assert(igt_is_force_dsc_enabled(data->fd, data->output->name));
>>>   	}
>>>   
>>>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>>   	igt_assert_output_bpc_equal(crtc,
>>> -				    output, 8);
>>> +				    data->output, 8);
>>>   
>>>   	if (flags & TEST_NEEDS_DSC) {
>>> -		igt_force_dsc_disable(data->fd, output->name);
>>> -		igt_assert(igt_is_force_dsc_disabled(data->fd, output->name));
>>> +		igt_force_dsc_disable(data->fd, data->output->name);
>>> +		igt_assert(igt_is_force_dsc_disabled(data->fd, data->output->name));
>>>   	}
>>>   
>>>   	/* Enter HDR, a modeset is allowed here. */
>>> -	fill_hdr_output_metadata_st2084(&hdr);
>>> -	set_hdr_output_metadata(data, &hdr);
>>> +	igt_hdr_fill_st2084(&hdr);
>>> +	igt_hdr_set_metadata(data->output, &hdr);
>>>   	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 10);
>>>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>>   	igt_assert_output_bpc_equal(crtc,
>>> -				    output, 10);
>>> +				    data->output, 10);
>>>   
>>>   	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
>>>   
>>> @@ -652,21 +459,21 @@ static void test_static_swap(data_t *data, igt_crtc_t *crtc,
>>>   	hdr.hdmi_metadata_type1.max_fall = 200;
>>>   	hdr.hdmi_metadata_type1.max_cll = 100;
>>>   
>>> -	set_hdr_output_metadata(data, &hdr);
>>> +	igt_hdr_set_metadata(data->output, &hdr);
>>>   	if (is_amdgpu_device(data->fd))
>>>   		igt_display_commit_atomic(display, 0, NULL);
>>>   	else
>>>   		igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>>   
>>>   	if (flags & TEST_NEEDS_DSC) {
>>> -		igt_force_dsc_enable(data->fd, output->name);
>>> -		igt_assert(igt_is_force_dsc_enabled(data->fd, output->name));
>>> +		igt_force_dsc_enable(data->fd, data->output->name);
>>> +		igt_assert(igt_is_force_dsc_enabled(data->fd, data->output->name));
>>>   	}
>>>   	/* Enter SDR via metadata, no modeset allowed for
>>>   	 * amd driver, whereas a modeset is required for
>>>   	 * intel driver. */
>>> -	fill_hdr_output_metadata_sdr(&hdr);
>>> -	set_hdr_output_metadata(data, &hdr);
>>> +	igt_hdr_fill_sdr(&hdr);
>>> +	igt_hdr_set_metadata(data->output, &hdr);
>>>   	if (is_amdgpu_device(data->fd))
>>>   		igt_display_commit_atomic(display, 0, NULL);
>>>   	else
>>> @@ -675,45 +482,39 @@ static void test_static_swap(data_t *data, igt_crtc_t *crtc,
>>>   	igt_pipe_crc_collect_crc(data->pipe_crc, &new_crc);
>>>   
>>>   	/* Exit SDR and enter 8bpc, cleanup. */
>>> -	set_hdr_output_metadata(data, NULL);
>>> +	igt_hdr_set_metadata(data->output, NULL);
>>>   	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 8);
>>>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>>   	igt_assert_output_bpc_equal(crtc,
>>> -				    output, 8);
>>> +				    data->output, 8);
>>>   
>>>   	/* Verify that the CRC didn't change while cycling metadata. */
>>>   	igt_assert_crc_equal(&ref_crc, &new_crc);
>>>   
>>>   	if (flags & TEST_NEEDS_DSC) {
>>> -		igt_force_dsc_disable(data->fd, output->name);
>>> -		igt_assert(igt_is_force_dsc_disabled(data->fd, output->name));
>>> +		igt_force_dsc_disable(data->fd, data->output->name);
>>> +		igt_assert(igt_is_force_dsc_disabled(data->fd, data->output->name));
>>>   	}
>>>   
>>>   	test_fini(data);
>>>   	igt_remove_fb(data->fd, &afb);
>>>   }
>>>   
>>> -static void test_invalid_metadata_sizes(data_t *data, igt_output_t *output)
>>> +static void test_invalid_metadata_sizes(data_t *data)
>>>   {
>>>   	struct hdr_output_metadata hdr;
>>>   	size_t metadata_size = sizeof(hdr);
>>>   
>>> -	fill_hdr_output_metadata_st2084(&hdr);
>>> +	igt_hdr_fill_st2084(&hdr);
>>>   
>>> -	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, 1), -EINVAL);
>>> -	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size + 1), -EINVAL);
>>> -	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size - 1), -EINVAL);
>>> -	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size * 2), -EINVAL);
>>> +	igt_assert_eq(igt_hdr_set_invalid_metadata(data->output, &hdr, 1), -EINVAL);
>>> +	igt_assert_eq(igt_hdr_set_invalid_metadata(data->output, &hdr, metadata_size + 1), -EINVAL);
>>> +	igt_assert_eq(igt_hdr_set_invalid_metadata(data->output, &hdr, metadata_size - 1), -EINVAL);
>>> +	igt_assert_eq(igt_hdr_set_invalid_metadata(data->output, &hdr, metadata_size * 2), -EINVAL);
>>>   
>>>   	test_fini(data);
>>>   }
>>>   
>>> -/* Returns true if an output supports HDR metadata property. */
>>> -static bool has_hdr(igt_output_t *output)
>>> -{
>>> -	return igt_output_has_prop(output, IGT_CONNECTOR_HDR_OUTPUT_METADATA);
>>> -}
>>> -
>>>   static void test_hdr(data_t *data, uint32_t flags)
>>>   {
>>>   	igt_display_t *display = &data->display;
>>> @@ -729,20 +530,20 @@ static void test_hdr(data_t *data, uint32_t flags)
>>>   		 * set MAX_BPC property to 10bpc prior to setting
>>>   		 * HDR metadata property. Therefore, checking.
>>>   		 */
>>> -		if (!has_max_bpc(output) || !has_hdr(output)) {
>>> +		if (!igt_output_supports_max_bpc(output) || !igt_output_supports_hdr(output)) {
>>>   			igt_info("%s: Doesn't support IGT_CONNECTOR_MAX_BPC or IGT_CONNECTOR_HDR_OUTPUT_METADATA.\n",
>>>   				 igt_output_name(output));
>>>   			continue;
>>>   		}
>>>   
>>>   		/* For negative test, panel should be non-hdr. */
>>> -		if ((flags & TEST_INVALID_HDR) && is_panel_hdr(data, output)) {
>>> +		if ((flags & TEST_INVALID_HDR) && igt_is_panel_hdr(data->fd, output)) {
>>>   			igt_info("%s: Can't run negative test on HDR panel.\n",
>>>   				 igt_output_name(output));
>>>   			continue;
>>>   		}
>>>   
>>> -		if ((flags & ~TEST_INVALID_HDR) && !is_panel_hdr(data, output)) {
>>> +		if ((flags & ~TEST_INVALID_HDR) && !igt_is_panel_hdr(data->fd, output)) {
>>>   			igt_info("%s: Can't run HDR tests on non-HDR panel.\n",
>>>   				 igt_output_name(output));
>>>   			continue;
>>> @@ -772,8 +573,8 @@ static void test_hdr(data_t *data, uint32_t flags)
>>>   					     crtc);
>>>   
>>>   				/* Signal HDR requirement via metadata */
>>> -				fill_hdr_output_metadata_st2084(&hdr);
>>> -				set_hdr_output_metadata(data, &hdr);
>>> +				igt_hdr_fill_st2084(&hdr);
>>> +				igt_hdr_set_metadata(data->output, &hdr);
>>>   				if (igt_display_try_commit2(display, display->is_atomic ?
>>>   							    COMMIT_ATOMIC : COMMIT_LEGACY)) {
>>>   					igt_info("%s: Couldn't set HDR metadata\n",
>>> @@ -796,7 +597,7 @@ static void test_hdr(data_t *data, uint32_t flags)
>>>   				else
>>>   					flags &= ~TEST_NEEDS_DSC;
>>>   
>>> -				set_hdr_output_metadata(data, NULL);
>>> +				igt_hdr_set_metadata(data->output, NULL);
>>>   				igt_display_commit2(display, display->is_atomic ?
>>>   						    COMMIT_ATOMIC : COMMIT_LEGACY);
>>>   
>>> @@ -811,13 +612,13 @@ static void test_hdr(data_t *data, uint32_t flags)
>>>   						     TEST_INVALID_HDR | TEST_BRIGHTNESS))
>>>   						test_static_toggle(data,
>>>   								   crtc,
>>> -								   output, hdr_test_formats[i], flags);
>>> +								   hdr_test_formats[i], flags);
>>>   					if (flags & TEST_SWAP)
>>>   						test_static_swap(data,
>>>   								 crtc,
>>> -								 output, hdr_test_formats[i], flags);
>>> +								 hdr_test_formats[i], flags);
>>>   					if (flags & TEST_INVALID_METADATA_SIZES)
>>> -						test_invalid_metadata_sizes(data, output);
>>> +						test_invalid_metadata_sizes(data);
>>>   				}
>>>   			}
>>>   
>>> -- 
>>> 2.25.1
>>>

  parent reply	other threads:[~2026-04-28 11:33 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-17 21:38 [PATCH i-g-t,v3 0/5] Enable HDR in IGT Frontbuffer Feature Tests Swati Sharma
2026-04-17 21:38 ` [PATCH i-g-t, v3 1/5] lib/igt_hdr: Move HDR helpers from kms_hdr into shared library Swati Sharma
2026-04-17 23:36   ` Alex Hung
2026-04-28 11:15     ` Sharma, Swati2
2026-04-20 18:46   ` Kamil Konieczny
2026-04-21 12:41     ` Jani Nikula
2026-04-22 17:01       ` Kamil Konieczny
2026-04-28 11:33       ` Sharma, Swati2 [this message]
2026-04-28 11:28     ` Sharma, Swati2
2026-04-17 21:38 ` [PATCH i-g-t,v3 2/5] lib/igt_hdr: Fix EOTF bit flag checking Swati Sharma
2026-04-17 21:38 ` [PATCH i-g-t, v3 3/5] tests/intel/kms_frontbuffer_tracking: Add HDR feature support Swati Sharma
2026-04-17 21:38 ` [PATCH i-g-t, v3 4/5] lib/igt_hdr: Add helpers to enable and disable HDR on an output Swati Sharma
2026-04-17 21:38 ` [PATCH i-g-t, v3 5/5] tests/intel/kms_frontbuffer_tracking: Enable HDR in feature tests Swati Sharma
2026-04-21 12:31 ` ✓ i915.CI.BAT: success for Enable HDR in IGT Frontbuffer Feature Tests (rev3) Patchwork
2026-04-21 12:50 ` ✓ Xe.CI.BAT: " Patchwork
2026-04-21 13:59 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-04-21 17:01 ` ✗ i915.CI.Full: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c1ac896c-bf97-4133-b941-de9dba3d71ce@intel.com \
    --to=swati2.sharma@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=juha-pekka.heikkila@intel.com \
    --cc=juhapekka.heikkila@gmail.com \
    --cc=kamil.konieczny@linux.intel.com \
    --cc=karthik.b.s@intel.com \
    --cc=suraj.kandpal@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox