* [PATCH v2 0/3] drm: detect panel type from DisplayID 2.x
@ 2026-05-20 2:13 Chenyu Chen
2026-05-20 2:13 ` [PATCH v2 1/3] drm/edid: extract section header processing into helper Chenyu Chen
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Chenyu Chen @ 2026-05-20 2:13 UTC (permalink / raw)
To: amd-gfx, dri-devel
Cc: Harry Wentland, Leo Li, Ray Wu, Limonciello Mario, Jani Nikula,
Chenyu Chen
This series adds parsing of the Display Device Technology field from
the DisplayID v2.x Display Parameters Data Block (tag 0x21), enabling
panel type detection (LCD/OLED) through a standards-based source.
Previously, amdgpu_dm determined panel type only from AMD VSDB, DPCD
sink extended caps, and a Samsung luminance heuristic. A TODO comment
in dm_set_panel_type() acknowledged the need to also use DisplayID as
a source. This series resolves that by parsing the Display Parameters
block in DRM core and wiring it into amdgpu_dm's detection priority
chain as: VSDB > DPCD > DisplayID > Samsung heuristic.
Patch 1 extracts the section header processing into a helper and
removes the break so the iterator can walk through all data blocks.
The helper is invoked only once via a header_processed flag because
displayid_version() and displayid_primary_use() always return values
captured from the base section — they are fixed regardless of which
extension section the iterator is currently in, so processing the
header more than once would be redundant.
Chenyu Chen (3):
drm/edid: extract section header processing into helper
drm/edid: parse panel type from DisplayID 2.x Display Parameters
drm/amd/display: use DisplayID panel type in dm_set_panel_type
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +--
drivers/gpu/drm/drm_displayid_internal.h | 25 ++++++
drivers/gpu/drm/drm_edid.c | 79 +++++++++++++++----
include/drm/drm_connector.h | 6 ++
include/uapi/drm/drm_mode.h | 1 +
5 files changed, 103 insertions(+), 20 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/3] drm/edid: extract section header processing into helper
2026-05-20 2:13 [PATCH v2 0/3] drm: detect panel type from DisplayID 2.x Chenyu Chen
@ 2026-05-20 2:13 ` Chenyu Chen
2026-05-20 13:10 ` Jani Nikula
2026-05-20 2:13 ` [PATCH v2 2/3] drm/edid: parse panel type from DisplayID 2.x Display Parameters Chenyu Chen
2026-05-20 2:13 ` [PATCH v2 3/3] drm/amd/display: use DisplayID panel type in dm_set_panel_type Chenyu Chen
2 siblings, 1 reply; 8+ messages in thread
From: Chenyu Chen @ 2026-05-20 2:13 UTC (permalink / raw)
To: amd-gfx, dri-devel
Cc: Harry Wentland, Leo Li, Ray Wu, Limonciello Mario, Jani Nikula,
Chenyu Chen, Mario Limonciello
Extract the DisplayID section header logging and non_desktop
detection from update_displayid_info() into a dedicated helper,
drm_displayid_process_section_header(). Remove the break so the
iterator walks through all data blocks, preparing for future
patches that will parse additional block types within the loop.
The helper is called only once for the base section via a
header_processed flag. Since version and primary_use are only
captured from the base section, and extension sections carry a
primary use of zero per spec, the non_desktop logic is unaffected.
No functional change.
Assisted-by: Copilot:Claude-Opus-4.6
Signed-off-by: Chenyu Chen <chen-yu.chen@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/gpu/drm/drm_edid.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 8031f021d4d0..04878478ab78 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6715,30 +6715,35 @@ static void drm_reset_display_info(struct drm_connector *connector)
memset(&info->amd_vsdb, 0, sizeof(info->amd_vsdb));
}
+static void drm_displayid_process_section_header(struct drm_connector *connector,
+ const struct displayid_iter *iter)
+{
+ struct drm_display_info *info = &connector->display_info;
+
+ drm_dbg_kms(connector->dev,
+ "[CONNECTOR:%d:%s] DisplayID extension version 0x%02x, primary use 0x%02x\n",
+ connector->base.id, connector->name,
+ displayid_version(iter),
+ displayid_primary_use(iter));
+ if (displayid_version(iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
+ (displayid_primary_use(iter) == PRIMARY_USE_HEAD_MOUNTED_VR ||
+ displayid_primary_use(iter) == PRIMARY_USE_HEAD_MOUNTED_AR))
+ info->non_desktop = true;
+}
+
static void update_displayid_info(struct drm_connector *connector,
const struct drm_edid *drm_edid)
{
- struct drm_display_info *info = &connector->display_info;
const struct displayid_block *block;
struct displayid_iter iter;
+ bool header_processed = false;
displayid_iter_edid_begin(drm_edid, &iter);
displayid_iter_for_each(block, &iter) {
- drm_dbg_kms(connector->dev,
- "[CONNECTOR:%d:%s] DisplayID extension version 0x%02x, primary use 0x%02x\n",
- connector->base.id, connector->name,
- displayid_version(&iter),
- displayid_primary_use(&iter));
- if (displayid_version(&iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
- (displayid_primary_use(&iter) == PRIMARY_USE_HEAD_MOUNTED_VR ||
- displayid_primary_use(&iter) == PRIMARY_USE_HEAD_MOUNTED_AR))
- info->non_desktop = true;
-
- /*
- * We're only interested in the base section here, no need to
- * iterate further.
- */
- break;
+ if (!header_processed) {
+ drm_displayid_process_section_header(connector, &iter);
+ header_processed = true;
+ }
}
displayid_iter_end(&iter);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] drm/edid: parse panel type from DisplayID 2.x Display Parameters
2026-05-20 2:13 [PATCH v2 0/3] drm: detect panel type from DisplayID 2.x Chenyu Chen
2026-05-20 2:13 ` [PATCH v2 1/3] drm/edid: extract section header processing into helper Chenyu Chen
@ 2026-05-20 2:13 ` Chenyu Chen
2026-05-20 13:30 ` Jani Nikula
2026-05-20 2:13 ` [PATCH v2 3/3] drm/amd/display: use DisplayID panel type in dm_set_panel_type Chenyu Chen
2 siblings, 1 reply; 8+ messages in thread
From: Chenyu Chen @ 2026-05-20 2:13 UTC (permalink / raw)
To: amd-gfx, dri-devel
Cc: Harry Wentland, Leo Li, Ray Wu, Limonciello Mario, Jani Nikula,
Chenyu Chen, Mario Limonciello
Parse the Display Parameters Data Block (tag 0x21) defined in
DisplayID v2.1a Section 4.2.6. Extract the Display Device Technology
field from payload byte 27 bits [6:4], which indicates whether the
panel uses LCD (001b) or OLED (010b) technology.
Add a did_panel_type field to struct drm_display_info and populate it
during DisplayID iteration so downstream drivers can use it for
panel-type-dependent behavior. Add DRM_MODE_PANEL_TYPE_LCD to the
UAPI constants for use as an internal communication value between
DRM core and drivers.
Assisted-by: Copilot:Claude-Opus-4.6
Signed-off-by: Chenyu Chen <chen-yu.chen@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/gpu/drm/drm_displayid_internal.h | 25 ++++++++++++++
drivers/gpu/drm/drm_edid.c | 44 ++++++++++++++++++++++++
include/drm/drm_connector.h | 6 ++++
include/uapi/drm/drm_mode.h | 1 +
4 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_displayid_internal.h b/drivers/gpu/drm/drm_displayid_internal.h
index 5b1b32f73516..e0f7c54d2987 100644
--- a/drivers/gpu/drm/drm_displayid_internal.h
+++ b/drivers/gpu/drm/drm_displayid_internal.h
@@ -142,6 +142,31 @@ struct displayid_formula_timing_block {
struct displayid_formula_timings_9 timings[];
} __packed;
+/*
+ * DisplayID v2.x Display Parameters Data Block (tag 0x21).
+ *
+ * Per VESA DisplayID v2.1a, Section 4.2.6, Table 4-14:
+ * Offset 0x1E (payload byte 27) contains Native Color Depth and
+ * Display Device Technology fields.
+ * bits [2:0] = Native Color Depth
+ * bit [3] = RESERVED
+ * bits [6:4] = Display Device Technology
+ * 000b = not specified, 001b = LCD, 010b = OLED, others reserved
+ * bit [7] = Display Device Theme Preference
+ */
+#define DISPLAYID_DISPLAY_PARAMS_DEVICE_TECH GENMASK(6, 4)
+
+struct displayid_display_params_block {
+ struct displayid_block base;
+ u8 payload[27];
+ u8 device_tech_byte; /* bits [6:4] = Display Device Technology */
+ u8 reserved;
+} __packed;
+
+#define DISPLAYID_DISPLAY_PARAMS_MIN_LEN \
+ (sizeof(struct displayid_display_params_block) - \
+ sizeof(struct displayid_block))
+
#define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0)
#define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 04878478ab78..d1de1a398677 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6713,6 +6713,8 @@ static void drm_reset_display_info(struct drm_connector *connector)
info->source_physical_address = CEC_PHYS_ADDR_INVALID;
memset(&info->amd_vsdb, 0, sizeof(info->amd_vsdb));
+
+ info->did_panel_type = DRM_MODE_PANEL_TYPE_UNKNOWN;
}
static void drm_displayid_process_section_header(struct drm_connector *connector,
@@ -6731,6 +6733,44 @@ static void drm_displayid_process_section_header(struct drm_connector *connector
info->non_desktop = true;
}
+static void
+drm_displayid_parse_display_params(struct drm_connector *connector,
+ const struct displayid_block *block)
+{
+ struct drm_display_info *info = &connector->display_info;
+ const struct displayid_display_params_block *params =
+ (const struct displayid_display_params_block *)block;
+
+ u8 tech;
+
+ if (block->num_bytes < DISPLAYID_DISPLAY_PARAMS_MIN_LEN) {
+ drm_dbg_kms(connector->dev,
+ "[CONNECTOR:%d:%s] DisplayID Display Parameters block too short (%u < %zu)\n",
+ connector->base.id, connector->name,
+ block->num_bytes,
+ DISPLAYID_DISPLAY_PARAMS_MIN_LEN);
+ return;
+ }
+
+ tech = FIELD_GET(DISPLAYID_DISPLAY_PARAMS_DEVICE_TECH,
+ params->device_tech_byte);
+
+ drm_dbg_kms(connector->dev,
+ "[CONNECTOR:%d:%s] DisplayID Display Parameters: device technology %u\n",
+ connector->base.id, connector->name, tech);
+
+ switch (tech) {
+ case 1: /* LCD */
+ info->did_panel_type = DRM_MODE_PANEL_TYPE_LCD;
+ break;
+ case 2: /* OLED */
+ info->did_panel_type = DRM_MODE_PANEL_TYPE_OLED;
+ break;
+ default:
+ break;
+ }
+}
+
static void update_displayid_info(struct drm_connector *connector,
const struct drm_edid *drm_edid)
{
@@ -6744,6 +6784,10 @@ static void update_displayid_info(struct drm_connector *connector,
drm_displayid_process_section_header(connector, &iter);
header_processed = true;
}
+
+ if (displayid_version(&iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
+ block->tag == DATA_BLOCK_2_DISPLAY_PARAMETERS)
+ drm_displayid_parse_display_params(connector, block);
}
displayid_iter_end(&iter);
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index c398dbc68bbc..b95aec34ddb7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -899,6 +899,12 @@ struct drm_display_info {
* @amd_vsdb: AMD-specific VSDB information.
*/
struct drm_amd_vsdb_info amd_vsdb;
+
+ /**
+ * @did_panel_type: Panel type from DisplayID Display Parameters
+ * Data Block (tag 0x21). Uses DRM_MODE_PANEL_TYPE_* constants.
+ */
+ u8 did_panel_type;
};
int drm_display_info_set_bus_formats(struct drm_display_info *info,
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 3693d82b5279..d7ca1040b92e 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -169,6 +169,7 @@ extern "C" {
/* Panel type property */
#define DRM_MODE_PANEL_TYPE_UNKNOWN 0
#define DRM_MODE_PANEL_TYPE_OLED 1
+#define DRM_MODE_PANEL_TYPE_LCD 2
/*
* DRM_MODE_ROTATE_<degrees>
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] drm/amd/display: use DisplayID panel type in dm_set_panel_type
2026-05-20 2:13 [PATCH v2 0/3] drm: detect panel type from DisplayID 2.x Chenyu Chen
2026-05-20 2:13 ` [PATCH v2 1/3] drm/edid: extract section header processing into helper Chenyu Chen
2026-05-20 2:13 ` [PATCH v2 2/3] drm/edid: parse panel type from DisplayID 2.x Display Parameters Chenyu Chen
@ 2026-05-20 2:13 ` Chenyu Chen
2 siblings, 0 replies; 8+ messages in thread
From: Chenyu Chen @ 2026-05-20 2:13 UTC (permalink / raw)
To: amd-gfx, dri-devel
Cc: Harry Wentland, Leo Li, Ray Wu, Limonciello Mario, Jani Nikula,
Chenyu Chen, Mario Limonciello
Wire up the newly parsed did_panel_type from drm_display_info into
amdgpu_dm's panel type detection path. When neither the AMD VSDB
nor DPCD determines the panel type, fall back to the DisplayID
Display Device Technology field to set PANEL_TYPE_LCD or
PANEL_TYPE_OLED accordingly.
Assisted-by: Copilot:Claude-Opus-4.6
Signed-off-by: Chenyu Chen <chen-yu.chen@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index e42a5eecdf46..353a4982333d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3944,11 +3944,13 @@ static void dm_set_panel_type(struct amdgpu_dm_connector *aconnector)
link->panel_type = PANEL_TYPE_OLED;
}
- /*
- * TODO: get panel type from DID2 that has device technology field
- * to specify if it's OLED or not. But we need to wait for DID2
- * support in DC and EDID parser to be able to use it here.
- */
+ /* If VSDB and DPCD didn't determine panel type, check DID */
+ if (link->panel_type == PANEL_TYPE_NONE) {
+ if (display_info->did_panel_type == DRM_MODE_PANEL_TYPE_LCD)
+ link->panel_type = PANEL_TYPE_LCD;
+ else if (display_info->did_panel_type == DRM_MODE_PANEL_TYPE_OLED)
+ link->panel_type = PANEL_TYPE_OLED;
+ }
if (link->panel_type == PANEL_TYPE_NONE) {
struct drm_amd_vsdb_info *vsdb = &display_info->amd_vsdb;
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] drm/edid: extract section header processing into helper
2026-05-20 2:13 ` [PATCH v2 1/3] drm/edid: extract section header processing into helper Chenyu Chen
@ 2026-05-20 13:10 ` Jani Nikula
2026-05-25 14:06 ` Chen, Chen-Yu
0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2026-05-20 13:10 UTC (permalink / raw)
To: Chenyu Chen, amd-gfx, dri-devel
Cc: Harry Wentland, Leo Li, Ray Wu, Limonciello Mario, Chenyu Chen,
Mario Limonciello
On Wed, 20 May 2026, Chenyu Chen <chen-yu.chen@amd.com> wrote:
> Extract the DisplayID section header logging and non_desktop
> detection from update_displayid_info() into a dedicated helper,
> drm_displayid_process_section_header(). Remove the break so the
> iterator walks through all data blocks, preparing for future
> patches that will parse additional block types within the loop.
>
> The helper is called only once for the base section via a
> header_processed flag. Since version and primary_use are only
> captured from the base section, and extension sections carry a
> primary use of zero per spec, the non_desktop logic is unaffected.
>
> No functional change.
>
> Assisted-by: Copilot:Claude-Opus-4.6
> Signed-off-by: Chenyu Chen <chen-yu.chen@amd.com>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> drivers/gpu/drm/drm_edid.c | 37 +++++++++++++++++++++----------------
> 1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 8031f021d4d0..04878478ab78 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6715,30 +6715,35 @@ static void drm_reset_display_info(struct drm_connector *connector)
> memset(&info->amd_vsdb, 0, sizeof(info->amd_vsdb));
> }
>
> +static void drm_displayid_process_section_header(struct drm_connector *connector,
> + const struct displayid_iter *iter)
The name's a bit grandiose, yet loses the bit about "base section" which
was the crucial part in the comment that gets removed below.
> +{
> + struct drm_display_info *info = &connector->display_info;
> +
> + drm_dbg_kms(connector->dev,
> + "[CONNECTOR:%d:%s] DisplayID extension version 0x%02x, primary use 0x%02x\n",
> + connector->base.id, connector->name,
> + displayid_version(iter),
> + displayid_primary_use(iter));
> + if (displayid_version(iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
> + (displayid_primary_use(iter) == PRIMARY_USE_HEAD_MOUNTED_VR ||
> + displayid_primary_use(iter) == PRIMARY_USE_HEAD_MOUNTED_AR))
> + info->non_desktop = true;
> +}
The indent is all wrong in this function.
> +
> static void update_displayid_info(struct drm_connector *connector,
> const struct drm_edid *drm_edid)
> {
> - struct drm_display_info *info = &connector->display_info;
> const struct displayid_block *block;
> struct displayid_iter iter;
> + bool header_processed = false;
>
> displayid_iter_edid_begin(drm_edid, &iter);
> displayid_iter_for_each(block, &iter) {
> - drm_dbg_kms(connector->dev,
> - "[CONNECTOR:%d:%s] DisplayID extension version 0x%02x, primary use 0x%02x\n",
> - connector->base.id, connector->name,
> - displayid_version(&iter),
> - displayid_primary_use(&iter));
> - if (displayid_version(&iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
> - (displayid_primary_use(&iter) == PRIMARY_USE_HEAD_MOUNTED_VR ||
> - displayid_primary_use(&iter) == PRIMARY_USE_HEAD_MOUNTED_AR))
> - info->non_desktop = true;
> -
> - /*
> - * We're only interested in the base section here, no need to
> - * iterate further.
> - */
> - break;
> + if (!header_processed) {
> + drm_displayid_process_section_header(connector, &iter);
> + header_processed = true;
Every DisplayID Section has a header. Every DisplayID Data Block within
a DisplayID Section has a header. This is about handling the information
in the Base Section header only. IMO header_processed is misleading.
BR,
Jani.
> + }
> }
> displayid_iter_end(&iter);
> }
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] drm/edid: parse panel type from DisplayID 2.x Display Parameters
2026-05-20 2:13 ` [PATCH v2 2/3] drm/edid: parse panel type from DisplayID 2.x Display Parameters Chenyu Chen
@ 2026-05-20 13:30 ` Jani Nikula
2026-05-25 14:06 ` Chen, Chen-Yu
0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2026-05-20 13:30 UTC (permalink / raw)
To: Chenyu Chen, amd-gfx, dri-devel
Cc: Harry Wentland, Leo Li, Ray Wu, Limonciello Mario, Chenyu Chen,
Mario Limonciello
On Wed, 20 May 2026, Chenyu Chen <chen-yu.chen@amd.com> wrote:
> Parse the Display Parameters Data Block (tag 0x21) defined in
> DisplayID v2.1a Section 4.2.6. Extract the Display Device Technology
> field from payload byte 27 bits [6:4], which indicates whether the
> panel uses LCD (001b) or OLED (010b) technology.
>
> Add a did_panel_type field to struct drm_display_info and populate it
> during DisplayID iteration so downstream drivers can use it for
> panel-type-dependent behavior. Add DRM_MODE_PANEL_TYPE_LCD to the
> UAPI constants for use as an internal communication value between
> DRM core and drivers.
>
> Assisted-by: Copilot:Claude-Opus-4.6
> Signed-off-by: Chenyu Chen <chen-yu.chen@amd.com>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> drivers/gpu/drm/drm_displayid_internal.h | 25 ++++++++++++++
> drivers/gpu/drm/drm_edid.c | 44 ++++++++++++++++++++++++
> include/drm/drm_connector.h | 6 ++++
> include/uapi/drm/drm_mode.h | 1 +
> 4 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_displayid_internal.h b/drivers/gpu/drm/drm_displayid_internal.h
> index 5b1b32f73516..e0f7c54d2987 100644
> --- a/drivers/gpu/drm/drm_displayid_internal.h
> +++ b/drivers/gpu/drm/drm_displayid_internal.h
> @@ -142,6 +142,31 @@ struct displayid_formula_timing_block {
> struct displayid_formula_timings_9 timings[];
> } __packed;
>
> +/*
> + * DisplayID v2.x Display Parameters Data Block (tag 0x21).
> + *
> + * Per VESA DisplayID v2.1a, Section 4.2.6, Table 4-14:
> + * Offset 0x1E (payload byte 27) contains Native Color Depth and
> + * Display Device Technology fields.
> + * bits [2:0] = Native Color Depth
> + * bit [3] = RESERVED
> + * bits [6:4] = Display Device Technology
> + * 000b = not specified, 001b = LCD, 010b = OLED, others reserved
> + * bit [7] = Display Device Theme Preference
> + */
Please drop the comment...
> +#define DISPLAYID_DISPLAY_PARAMS_DEVICE_TECH GENMASK(6, 4)
> +
> +struct displayid_display_params_block {
> + struct displayid_block base;
> + u8 payload[27];
> + u8 device_tech_byte; /* bits [6:4] = Display Device Technology */
> + u8 reserved;
...and actually define the information here.
Sure, you're only interested in one thing, but don't leave the rest for
someone who comes after you, since you clearly have access to the spec,
and whoever reviews this also needs to have access to the spec.
> +} __packed;
> +
> +#define DISPLAYID_DISPLAY_PARAMS_MIN_LEN \
> + (sizeof(struct displayid_display_params_block) - \
> + sizeof(struct displayid_block))
Is this helpful? Do you suggest we should add the define for all of the
blocks? What about when there's a minimum, and you can optionally have
more?
> +
> #define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0)
> #define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 04878478ab78..d1de1a398677 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6713,6 +6713,8 @@ static void drm_reset_display_info(struct drm_connector *connector)
>
> info->source_physical_address = CEC_PHYS_ADDR_INVALID;
> memset(&info->amd_vsdb, 0, sizeof(info->amd_vsdb));
> +
> + info->did_panel_type = DRM_MODE_PANEL_TYPE_UNKNOWN;
> }
>
> static void drm_displayid_process_section_header(struct drm_connector *connector,
> @@ -6731,6 +6733,44 @@ static void drm_displayid_process_section_header(struct drm_connector *connector
> info->non_desktop = true;
> }
>
> +static void
> +drm_displayid_parse_display_params(struct drm_connector *connector,
> + const struct displayid_block *block)
> +{
> + struct drm_display_info *info = &connector->display_info;
> + const struct displayid_display_params_block *params =
> + (const struct displayid_display_params_block *)block;
> +
Superfluous blank line.
> + u8 tech;
> +
> + if (block->num_bytes < DISPLAYID_DISPLAY_PARAMS_MIN_LEN) {
> + drm_dbg_kms(connector->dev,
> + "[CONNECTOR:%d:%s] DisplayID Display Parameters block too short (%u < %zu)\n",
> + connector->base.id, connector->name,
> + block->num_bytes,
> + DISPLAYID_DISPLAY_PARAMS_MIN_LEN);
> + return;
> + }
> +
> + tech = FIELD_GET(DISPLAYID_DISPLAY_PARAMS_DEVICE_TECH,
> + params->device_tech_byte);
> +
> + drm_dbg_kms(connector->dev,
> + "[CONNECTOR:%d:%s] DisplayID Display Parameters: device technology %u\n",
> + connector->base.id, connector->name, tech);
It would be more useful to print LCD or OLED, not the number.
> +
> + switch (tech) {
> + case 1: /* LCD */
> + info->did_panel_type = DRM_MODE_PANEL_TYPE_LCD;
> + break;
> + case 2: /* OLED */
The comments above are useless.
> + info->did_panel_type = DRM_MODE_PANEL_TYPE_OLED;
> + break;
> + default:
> + break;
> + }
> +}
> +
> static void update_displayid_info(struct drm_connector *connector,
> const struct drm_edid *drm_edid)
> {
> @@ -6744,6 +6784,10 @@ static void update_displayid_info(struct drm_connector *connector,
> drm_displayid_process_section_header(connector, &iter);
> header_processed = true;
> }
> +
> + if (displayid_version(&iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
> + block->tag == DATA_BLOCK_2_DISPLAY_PARAMETERS)
> + drm_displayid_parse_display_params(connector, block);
> }
> displayid_iter_end(&iter);
> }
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index c398dbc68bbc..b95aec34ddb7 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -899,6 +899,12 @@ struct drm_display_info {
> * @amd_vsdb: AMD-specific VSDB information.
> */
> struct drm_amd_vsdb_info amd_vsdb;
> +
> + /**
> + * @did_panel_type: Panel type from DisplayID Display Parameters
> + * Data Block (tag 0x21). Uses DRM_MODE_PANEL_TYPE_* constants.
> + */
> + u8 did_panel_type;
What does the did_ prefix give us?
> };
>
> int drm_display_info_set_bus_formats(struct drm_display_info *info,
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 3693d82b5279..d7ca1040b92e 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -169,6 +169,7 @@ extern "C" {
> /* Panel type property */
> #define DRM_MODE_PANEL_TYPE_UNKNOWN 0
> #define DRM_MODE_PANEL_TYPE_OLED 1
> +#define DRM_MODE_PANEL_TYPE_LCD 2
This is a UABI header.
Should we add this to the panel type property too?
BR,
Jani.
>
> /*
> * DRM_MODE_ROTATE_<degrees>
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] drm/edid: extract section header processing into helper
2026-05-20 13:10 ` Jani Nikula
@ 2026-05-25 14:06 ` Chen, Chen-Yu
0 siblings, 0 replies; 8+ messages in thread
From: Chen, Chen-Yu @ 2026-05-25 14:06 UTC (permalink / raw)
To: Jani Nikula, amd-gfx, dri-devel
Cc: Harry Wentland, Leo Li, Ray Wu, Limonciello Mario,
Mario Limonciello
On 5/20/2026 9:10 PM, Jani Nikula wrote:
> On Wed, 20 May 2026, Chenyu Chen <chen-yu.chen@amd.com> wrote:
>> Extract the DisplayID section header logging and non_desktop
>> detection from update_displayid_info() into a dedicated helper,
>> drm_displayid_process_section_header(). Remove the break so the
>> iterator walks through all data blocks, preparing for future
>> patches that will parse additional block types within the loop.
>>
>> The helper is called only once for the base section via a
>> header_processed flag. Since version and primary_use are only
>> captured from the base section, and extension sections carry a
>> primary use of zero per spec, the non_desktop logic is unaffected.
>>
>> No functional change.
>>
>> Assisted-by: Copilot:Claude-Opus-4.6
>> Signed-off-by: Chenyu Chen <chen-yu.chen@amd.com>
>> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> drivers/gpu/drm/drm_edid.c | 37 +++++++++++++++++++++----------------
>> 1 file changed, 21 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 8031f021d4d0..04878478ab78 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -6715,30 +6715,35 @@ static void drm_reset_display_info(struct drm_connector *connector)
>> memset(&info->amd_vsdb, 0, sizeof(info->amd_vsdb));
>> }
>>
>> +static void drm_displayid_process_section_header(struct drm_connector *connector,
>> + const struct displayid_iter *iter)
>
> The name's a bit grandiose, yet loses the bit about "base section" which
> was the crucial part in the comment that gets removed below.
>
I'll rename it to drm_displayid_process_base_section_header().
>> +{
>> + struct drm_display_info *info = &connector->display_info;
>> +
>> + drm_dbg_kms(connector->dev,
>> + "[CONNECTOR:%d:%s] DisplayID extension version 0x%02x, primary use 0x%02x\n",
>> + connector->base.id, connector->name,
>> + displayid_version(iter),
>> + displayid_primary_use(iter));
>> + if (displayid_version(iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
>> + (displayid_primary_use(iter) == PRIMARY_USE_HEAD_MOUNTED_VR ||
>> + displayid_primary_use(iter) == PRIMARY_USE_HEAD_MOUNTED_AR))
>> + info->non_desktop = true;
>> +}
>
> The indent is all wrong in this function.
>
I will fix it.
>> +
>> static void update_displayid_info(struct drm_connector *connector,
>> const struct drm_edid *drm_edid)
>> {
>> - struct drm_display_info *info = &connector->display_info;
>> const struct displayid_block *block;
>> struct displayid_iter iter;
>> + bool header_processed = false;
>>
>> displayid_iter_edid_begin(drm_edid, &iter);
>> displayid_iter_for_each(block, &iter) {
>> - drm_dbg_kms(connector->dev,
>> - "[CONNECTOR:%d:%s] DisplayID extension version 0x%02x, primary use 0x%02x\n",
>> - connector->base.id, connector->name,
>> - displayid_version(&iter),
>> - displayid_primary_use(&iter));
>> - if (displayid_version(&iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
>> - (displayid_primary_use(&iter) == PRIMARY_USE_HEAD_MOUNTED_VR ||
>> - displayid_primary_use(&iter) == PRIMARY_USE_HEAD_MOUNTED_AR))
>> - info->non_desktop = true;
>> -
>> - /*
>> - * We're only interested in the base section here, no need to
>> - * iterate further.
>> - */
>> - break;
>> + if (!header_processed) {
>> + drm_displayid_process_section_header(connector, &iter);
>> + header_processed = true;
>
> Every DisplayID Section has a header. Every DisplayID Data Block within
> a DisplayID Section has a header. This is about handling the information
> in the Base Section header only. IMO header_processed is misleading.
>
> BR,
> Jani.
>
I will rename it to `bool base_section_header_processed`.
Regards,
Chenyu
>> + }
>> }
>> displayid_iter_end(&iter);
>> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] drm/edid: parse panel type from DisplayID 2.x Display Parameters
2026-05-20 13:30 ` Jani Nikula
@ 2026-05-25 14:06 ` Chen, Chen-Yu
0 siblings, 0 replies; 8+ messages in thread
From: Chen, Chen-Yu @ 2026-05-25 14:06 UTC (permalink / raw)
To: Jani Nikula, amd-gfx, dri-devel
Cc: Harry Wentland, Leo Li, Ray Wu, Limonciello Mario,
Mario Limonciello
On 5/20/2026 9:30 PM, Jani Nikula wrote:
> On Wed, 20 May 2026, Chenyu Chen <chen-yu.chen@amd.com> wrote:
>> Parse the Display Parameters Data Block (tag 0x21) defined in
>> DisplayID v2.1a Section 4.2.6. Extract the Display Device Technology
>> field from payload byte 27 bits [6:4], which indicates whether the
>> panel uses LCD (001b) or OLED (010b) technology.
>>
>> Add a did_panel_type field to struct drm_display_info and populate it
>> during DisplayID iteration so downstream drivers can use it for
>> panel-type-dependent behavior. Add DRM_MODE_PANEL_TYPE_LCD to the
>> UAPI constants for use as an internal communication value between
>> DRM core and drivers.
>>
>> Assisted-by: Copilot:Claude-Opus-4.6
>> Signed-off-by: Chenyu Chen <chen-yu.chen@amd.com>
>> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> drivers/gpu/drm/drm_displayid_internal.h | 25 ++++++++++++++
>> drivers/gpu/drm/drm_edid.c | 44 ++++++++++++++++++++++++
>> include/drm/drm_connector.h | 6 ++++
>> include/uapi/drm/drm_mode.h | 1 +
>> 4 files changed, 76 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_displayid_internal.h b/drivers/gpu/drm/drm_displayid_internal.h
>> index 5b1b32f73516..e0f7c54d2987 100644
>> --- a/drivers/gpu/drm/drm_displayid_internal.h
>> +++ b/drivers/gpu/drm/drm_displayid_internal.h
>> @@ -142,6 +142,31 @@ struct displayid_formula_timing_block {
>> struct displayid_formula_timings_9 timings[];
>> } __packed;
>>
>> +/*
>> + * DisplayID v2.x Display Parameters Data Block (tag 0x21).
>> + *
>> + * Per VESA DisplayID v2.1a, Section 4.2.6, Table 4-14:
>> + * Offset 0x1E (payload byte 27) contains Native Color Depth and
>> + * Display Device Technology fields.
>> + * bits [2:0] = Native Color Depth
>> + * bit [3] = RESERVED
>> + * bits [6:4] = Display Device Technology
>> + * 000b = not specified, 001b = LCD, 010b = OLED, others reserved
>> + * bit [7] = Display Device Theme Preference
>> + */
>
> Please drop the comment...
>
>> +#define DISPLAYID_DISPLAY_PARAMS_DEVICE_TECH GENMASK(6, 4)
>> +
>> +struct displayid_display_params_block {
>> + struct displayid_block base;
>> + u8 payload[27];
>> + u8 device_tech_byte; /* bits [6:4] = Display Device Technology */
>> + u8 reserved;
>
> ...and actually define the information here.
>
> Sure, you're only interested in one thing, but don't leave the rest for
> someone who comes after you, since you clearly have access to the spec,
> and whoever reviews this also needs to have access to the spec.
>
I will replace the block comment and the u8 payload[27] with fully named
fields per spec Table 4-7 (image size, pixel count, features, primary
colors, white point, luminance, color_depth_and_tech, gamma_eotf).
I also add the named defines DISPLAYID_DEVICE_TECH_{UNSPECIFIED,LCD,OLED}
for the technology values.
>> +} __packed;
>> +
>> +#define DISPLAYID_DISPLAY_PARAMS_MIN_LEN \
>> + (sizeof(struct displayid_display_params_block) - \
>> + sizeof(struct displayid_block))
>
> Is this helpful? Do you suggest we should add the define for all of the
> blocks? What about when there's a minimum, and you can optionally have
> more?
>
I think defining a macro for every block doesn’t make sense.
So I will remove the macro and use sizeof(*params) - sizeof(params->base)
inline instead.
>> +
>> #define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0)
>> #define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 04878478ab78..d1de1a398677 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -6713,6 +6713,8 @@ static void drm_reset_display_info(struct drm_connector *connector)
>>
>> info->source_physical_address = CEC_PHYS_ADDR_INVALID;
>> memset(&info->amd_vsdb, 0, sizeof(info->amd_vsdb));
>> +
>> + info->did_panel_type = DRM_MODE_PANEL_TYPE_UNKNOWN;
>> }
>>
>> static void drm_displayid_process_section_header(struct drm_connector *connector,
>> @@ -6731,6 +6733,44 @@ static void drm_displayid_process_section_header(struct drm_connector *connector
>> info->non_desktop = true;
>> }
>>
>> +static void
>> +drm_displayid_parse_display_params(struct drm_connector *connector,
>> + const struct displayid_block *block)
>> +{
>> + struct drm_display_info *info = &connector->display_info;
>> + const struct displayid_display_params_block *params =
>> + (const struct displayid_display_params_block *)block;
>> +
>
> Superfluous blank line.
>
Will fix it.
>> + u8 tech;
>> +
>> + if (block->num_bytes < DISPLAYID_DISPLAY_PARAMS_MIN_LEN) {
>> + drm_dbg_kms(connector->dev,
>> + "[CONNECTOR:%d:%s] DisplayID Display Parameters block too short (%u < %zu)\n",
>> + connector->base.id, connector->name,
>> + block->num_bytes,
>> + DISPLAYID_DISPLAY_PARAMS_MIN_LEN);
>> + return;
>> + }
>> +
>> + tech = FIELD_GET(DISPLAYID_DISPLAY_PARAMS_DEVICE_TECH,
>> + params->device_tech_byte);
>> +
>> + drm_dbg_kms(connector->dev,
>> + "[CONNECTOR:%d:%s] DisplayID Display Parameters: device technology %u\n",
>> + connector->base.id, connector->name, tech);
>
> It would be more useful to print LCD or OLED, not the number.
>
I will change it to print "LCD", "OLED", or "unspecified".
>> +
>> + switch (tech) {
>> + case 1: /* LCD */
>> + info->did_panel_type = DRM_MODE_PANEL_TYPE_LCD;
>> + break;
>> + case 2: /* OLED */
>
> The comments above are useless.
>
I will remove the inline comments and use the named defines instead of
magic numbers in the switch cases.
>> + info->did_panel_type = DRM_MODE_PANEL_TYPE_OLED;
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> static void update_displayid_info(struct drm_connector *connector,
>> const struct drm_edid *drm_edid)
>> {
>> @@ -6744,6 +6784,10 @@ static void update_displayid_info(struct drm_connector *connector,
>> drm_displayid_process_section_header(connector, &iter);
>> header_processed = true;
>> }
>> +
>> + if (displayid_version(&iter) == DISPLAY_ID_STRUCTURE_VER_20 &&
>> + block->tag == DATA_BLOCK_2_DISPLAY_PARAMETERS)
>> + drm_displayid_parse_display_params(connector, block);
>> }
>> displayid_iter_end(&iter);
>> }
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index c398dbc68bbc..b95aec34ddb7 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -899,6 +899,12 @@ struct drm_display_info {
>> * @amd_vsdb: AMD-specific VSDB information.
>> */
>> struct drm_amd_vsdb_info amd_vsdb;
>> +
>> + /**
>> + * @did_panel_type: Panel type from DisplayID Display Parameters
>> + * Data Block (tag 0x21). Uses DRM_MODE_PANEL_TYPE_* constants.
>> + */
>> + u8 did_panel_type;
>
> What does the did_ prefix give us?
>
Good point, it doesn't add much value.
The intent was to indicate that the value is derived from DisplayID,
but I agree that encoding the data source in the field name is not
particularly helpful.
I'll rename it to "panel_type" and treat it as a generic field,
currently populated from DisplayID if available.
>> };
>>
>> int drm_display_info_set_bus_formats(struct drm_display_info *info,
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index 3693d82b5279..d7ca1040b92e 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -169,6 +169,7 @@ extern "C" {
>> /* Panel type property */
>> #define DRM_MODE_PANEL_TYPE_UNKNOWN 0
>> #define DRM_MODE_PANEL_TYPE_OLED 1
>> +#define DRM_MODE_PANEL_TYPE_LCD 2
>
> This is a UABI header.
>
> Should we add this to the panel type property too?
>
>
> BR,
> Jani.
>
>
Yes. I think we can expose the panel type too.
Regards,
Chenyu
>>
>> /*
>> * DRM_MODE_ROTATE_<degrees>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-25 14:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-20 2:13 [PATCH v2 0/3] drm: detect panel type from DisplayID 2.x Chenyu Chen
2026-05-20 2:13 ` [PATCH v2 1/3] drm/edid: extract section header processing into helper Chenyu Chen
2026-05-20 13:10 ` Jani Nikula
2026-05-25 14:06 ` Chen, Chen-Yu
2026-05-20 2:13 ` [PATCH v2 2/3] drm/edid: parse panel type from DisplayID 2.x Display Parameters Chenyu Chen
2026-05-20 13:30 ` Jani Nikula
2026-05-25 14:06 ` Chen, Chen-Yu
2026-05-20 2:13 ` [PATCH v2 3/3] drm/amd/display: use DisplayID panel type in dm_set_panel_type Chenyu Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox