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> >> >
