On Wed, 20 May 2026, Chenyu Chen <[email protected]> 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 <[email protected]>
> Reviewed-by: Mario Limonciello (AMD) <[email protected]>
> ---
> 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
