linux-kernel - Re: [PATCH v2 09/10] drm: bridge: dw_hdmi: Update EDID during hotplug processing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0dcb03be-dae1-4dcb-84d8-6ec204eab6ba@kwiboo.se>
Date: Thu, 19 Sep 2024 22:34:38 +0200
From: Jonas Karlman <jonas@...boo.se>
To: neil.armstrong@...aro.org
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, Robert Foss <rfoss@...nel.org>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
 Jernej Skrabec <jernej.skrabec@...il.com>,
 Christian Hewitt <christianshewitt@...il.com>,
 Diederik de Haas <didi.debian@...ow.org>, dri-devel@...ts.freedesktop.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 09/10] drm: bridge: dw_hdmi: Update EDID during hotplug
 processing

Hi Neil,

On 2024-09-13 10:02, Neil Armstrong wrote:
> On 08/09/2024 15:28, Jonas Karlman wrote:
>> Update successfully read EDID during hotplug processing to ensure the
>> connector diplay_info is always up-to-date.
>>
>> Signed-off-by: Jonas Karlman <jonas@...boo.se>
>> ---
>> v2: No change
>> ---
>>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index c19307120909..7bd9f895f03f 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2457,6 +2457,18 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
>>   
>>   	status = dw_hdmi_detect(hdmi);
>>   
>> +	/* Update EDID during hotplug processing (force=false) */
>> +	if (status == connector_status_connected && !force) {
>> +		const struct drm_edid *drm_edid;
>> +
>> +		drm_edid = dw_hdmi_edid_read(hdmi, connector);
>> +		if (drm_edid)
>> +			drm_edid_connector_update(connector, drm_edid);
>> +		cec_notifier_set_phys_addr(hdmi->cec_notifier,
>> +			connector->display_info.source_physical_address);
>> +		drm_edid_free(drm_edid);
>> +	}
>> +
>>   	if (status == connector_status_disconnected)
>>   		cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>>   
> 
> I wonder why we should read edid at each dw_hdmi_connector_detect() call,
> AFAIK it should only be when we have HPD pulses

That is what this change intends to help do.

As stated in the short comment EDID is only updated at HPD processing,
i.e. when force=false. To be on the safe side EDID is also only updated
here when connected and EDID could be read.

drm_helper_probe_detect() is called with force=true in the
fill_modes/get_modes call path that is triggered by userspace
or the kernel kms client.

After a HPD interrupt the call to drm_helper_hpd_irq_event() will call
check_connector_changed() that in turn calls drm_helper_probe_detect()
with force=false to check/detect if connector status has changed. It is
in this call chain the EDID may be read and updated in this detect ops.

Reading EDID here at HPD processing may not be fully needed, however it
help kernel keep the internal EDID state in better sync with sink when
userspace does not act on the HOTPLUG=1 uevent.

Regards,
Jonas

> 
> Neil