linux-kernel - Re: [PATCH] drm/display: hdmi: Do not read EDID on disconnected connectors

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: <aed7387e-196a-4819-b24e-788c925e1dee@collabora.com>
Date: Mon, 13 Jan 2025 14:00:17 +0200
From: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
To: Maxime Ripard <mripard@...nel.org>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, kernel@...labora.com,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/display: hdmi: Do not read EDID on disconnected
 connectors

Hi Maxime,

On 1/13/25 11:35 AM, Maxime Ripard wrote:
> On Sat, Jan 11, 2025 at 12:04:09AM +0200, Cristian Ciocaltea wrote:
>> The recently introduced hotplug event handler in the HDMI Connector
>> framework attempts to unconditionally read the EDID data, leading to a
>> bunch of non-harmful, yet quite annoying DDC/I2C related errors being
>> reported.
>>
>> Ensure the operation is performed only for connectors having the status
>> connected or unknown.
>>
>> Fixes: ab716b74dc9d ("drm/display/hdmi: implement hotplug functions")
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
>> ---
>>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> index 2691e8b3e480131ac6e4e4b74b24947be55694bd..8e4b30e09b53b84cfd36199d56db3221a00085b0 100644
>> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> @@ -786,8 +786,10 @@ drm_atomic_helper_connector_hdmi_update(struct drm_connector *connector,
>>  	const struct drm_edid *drm_edid;
>>  
>>  	if (status == connector_status_disconnected) {
>> +		drm_edid_connector_update(connector, NULL);
> 
> Why is this needed? It's not mentionned in your commit log.

The original implementation has it after reading the EDID, but I'm not
sure if we need the explicit reset in this case.

I was going to submit a new revision switching the order, as Dmitry
suggested, or should we simply drop it?

Thanks,
Cristian

> 
>>  		// TODO: also handle CEC and scramber, HDMI sink disconnected.
>>  		drm_connector_hdmi_audio_plugged_notify(connector, false);
>> +		return;
>>  	}
>>  
>>  	if (connector->hdmi.funcs->read_edid)
> 
> Maxime