On 5/20/2026 9:10 PM, Jani Nikula wrote:
> 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.
>
I'll rename it to drm_displayid_process_base_section_header().
>> +{
>> + 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.
>
I will fix it.
>> +
>> 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.
>
I will rename it to `bool base_section_header_processed`.
Regards,
Chenyu
>> + }
>> }
>> displayid_iter_end(&iter);
>> }
>
