On 5/16/2026 7:47 AM, Leo Li wrote:
>
>
> On 2026-05-15 04:23, Jani Nikula wrote:
>> On Thu, 14 May 2026, Chenyu Chen <[email protected]> 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 is LCD (001b) or OLED (010b).
>>>
>>> Store the result in drm_display_info.did_panel_type so that downstream
>>> drivers can use it for panel-type-dependent behavior.
>>>
>>> Assisted-by: Copilot:Claude-Opus-4.6
>>> Signed-off-by: Chenyu Chen <[email protected]>
>>> Reviewed-by: Mario Limonciello <[email protected]>
>>> ---
>>> drivers/gpu/drm/drm_displayid_internal.h | 25 ++++++++++
>>> drivers/gpu/drm/drm_edid.c | 61 +++++++++++++++++++-----
>>> include/drm/drm_connector.h | 6 +++
>>> include/uapi/drm/drm_mode.h | 1 +
>>> 4 files changed, 82 insertions(+), 11 deletions(-)
>>>
>>> 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 8031f021d4d0..9b160a878df4 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 update_displayid_info(struct drm_connector *connector,
>>> @@ -6721,24 +6723,61 @@ static void update_displayid_info(struct
>>> drm_connector *connector,
>>> struct drm_display_info *info = &connector->display_info;
>>> const struct displayid_block *block;
>>> struct displayid_iter iter;
>>> + const u8 *section = NULL;
>>>
>>> displayid_iter_edid_begin(drm_edid, &iter);
>>> displayid_iter_for_each(block, &iter) {
>>> + if (section != iter.section) {
>>> + 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;
>>> + section = iter.section;
>>> + }
>>
>> What is this even supposed to do?
>>
>
> I think the intention is to log and process the DisplayID base section header
> only once instead on every
> iteration. Maybe a `bool header_processed` works better?
>
> - Leo
>
The intention was to keep section-level drm_dbg_kms logging for better
debugging visibility.
That said, I agree that this does not belong in the iterator directly and
should be refactored into a helper.
>>> +
>>> drm_dbg_kms(connector->dev,
>>> - "[CONNECTOR:%d:%s] DisplayID extension version
>>> 0x%02x, primary use 0x%02x\n",
>>> + "[CONNECTOR:%d:%s] DisplayID block tag 0x%02x, rev
>>> 0x%02x, size %u\n",
>>> connector->base.id, connector->name,
>>> - displayid_version(&iter),
>>> - displayid_primary_use(&iter));
>>> + block->tag, block->rev, block->num_bytes);
>>> +
>>> 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;
>>> + block->tag == DATA_BLOCK_2_DISPLAY_PARAMETERS) {
>>> + 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);
>>> + continue;
>>> + }
>>>
>>> - /*
>>> - * We're only interested in the base section here, no need to
>>> - * iterate further.
>>> - */
>>> - break;
>>> + 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;
>>> + }
>>> + }
>>
>> Please tell copilot to not add so much crap in the iterator block. Add
>> functions. Add the first function as a refactor for non_desktop, add
>> more stuff on top, i.e. split this into multiple patches.
>>
>> BR,
>> Jani.
>>
>>
You're right, the current patches are hard to follow due to mixing refactoring
and new logic.
I will refactor the code into helper functions and split the changes into
smaller patches, then resend the updated series.
Thanks for the suggestions.
Regards,
Chenyu
>>> }
>>> 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>
>>
>
