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?

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


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

-- 
Jani Nikula, Intel

Reply via email to