Re: [PATCH v2 1/3] drm/edid: extract section header processing into helper

[Jani Nikula <jani.nikula@intel.com>]

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