Re: [PATCH v3 12/13] drm: bridge: dw_hdmi: Use dw_hdmi_connector_status_update()

Jonas Karlman
2026-04-28T21:47:47Z

Hi Nicolas,

On 4/28/2026 1:53 PM, Nicolas Frattaroli wrote:
> On Friday, 3 April 2026 20:52:58 Central European Summer Time Jonas Karlman wrote:
>> Update connector EDID and CEC phys addr from detect and force funcs to
>> ensure that userspace always have access to latest read EDID after a
>> sink use a HPD low voltage pulse to indicate that EDID has changed.
>>
>> With EDID being updated in detect and force funcs, there should no
>> longer be a need to re-read EDID in get_modes funcs, so drop it.
>>
>> This change make the dw-hdmi connector work more closely like the bridge
>> connector does with a hdmi bridge.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v3: Reworked 'Update EDID during hotplug processing' patch
>> ---
>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 24 +++++++++++++----------
>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index 0d42fdf9a386..5b5654ef6015 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2474,33 +2474,36 @@ dw_hdmi_connector_status_update(struct drm_connector *connector,
>> struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector);
>> const struct drm_edid *drm_edid;
>>
>> + if (status == connector_status_disconnected) {
>> + drm_edid_connector_update(connector, NULL);
>> + cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>> + return;
>> + }
>> +
>> drm_edid = dw_hdmi_edid_read(hdmi, connector);
>> drm_edid_connector_update(connector, drm_edid);
>> drm_edid_free(drm_edid);
>>
>> - cec_notifier_set_phys_addr(hdmi->cec_notifier,
>> - connector->display_info.source_physical_address);
>> + if (status == connector_status_connected)
>> + cec_notifier_set_phys_addr(hdmi->cec_notifier,
>> + connector->display_info.source_physical_address);
>> }
>>
>> static enum drm_connector_status
>> dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
>> {
>> - struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
>> - connector);
>> + struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector);
>> enum drm_connector_status status;
>>
>> status = dw_hdmi_detect(hdmi);
>>
>> - if (status == connector_status_disconnected)
>> - cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>> + dw_hdmi_connector_status_update(connector, status);
>>
>> return status;
>> }
>>
>> static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
>> {
>> - dw_hdmi_connector_status_update(connector, connector->status);
>> -
>> return drm_edid_connector_add_modes(connector);
>> }
>
> Are we absolutely sure status_update is called before this, so that the
> EDID doesn't need to be re-read? E.g. drm_connector_helper_get_modes
> does do an EDID read beforehand.

With the change to re-read EDID at .detect() we should not need to
re-read EDID at .get_modes(). This also closely match how the bridge
connector handle EDID update at .detect() and not .get_modes() for hdmi
bridges.

At least following call paths should ensure EDID have been re-read:

drm_helper_probe_single_connector_modes():
-> connector->funcs->force()
-> dw_hdmi_connector_force()
-> dw_hdmi_connector_status_update()
-> drm_edid_connector_update() <<-- here
-> drm_helper_probe_detect()
-> detect_connector_status()
-> connector->funcs->detect();
-> dw_hdmi_connector_detect()
-> dw_hdmi_connector_status_update()
-> drm_edid_connector_update() <<-- or here
-> drm_helper_probe_get_modes()
-> connector_funcs->get_modes()
-> dw_hdmi_connector_get_modes() <<-- now updated

drm_helper_hpd_irq_event():
-> detect_connector_status()
-> connector->funcs->detect();
-> dw_hdmi_connector_detect()
-> dw_hdmi_connector_status_update()
-> drm_edid_connector_update() <<-- also after HPD

> Speaking of which: if we had connector->ddc set, we could just use
> drm_connector_helper_get_modes for this callback. But maybe it's not
> worth spending the time refactoring the resource handling around
> hdmi->ddc when the eventual goal is to refactor this all into using
> a bridge connector anyway.

Correct, hopefully all connector related functions can be dropped from
dw-hdmi in a near future when dw-hdmi is converted to a hdmi bridge.

I currently have a work-in-progress snapshot at [1] where dw-hdmi have
been converted to a hdmi bridge and a bridge connector is used instead.

The changes in this series only aim to align dw-hdmi to the hdmi bridge
where possible to reduce any functional change in such future series.

I will likely keep this as-is in a v4 unless there is some objections.

[1] https://github.com/Kwiboo/linux-rockchip/commits/next-20260427-rk-hdmi-v2/

Regards,
Jonas

>>
>> @@ -2530,14 +2533,15 @@ static int dw_hdmi_connector_atomic_check(struct drm_connector *connector,
>>
>> static void dw_hdmi_connector_force(struct drm_connector *connector)
>> {
>> - struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
>> - connector);
>> + struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector);
>>
>> mutex_lock(&hdmi->mutex);
>> hdmi->force = connector->force;
>> hdmi->last_connector_result = connector->status;
>> dw_hdmi_update_phy_mask(hdmi);
>> mutex_unlock(&hdmi->mutex);
>> +
>> + dw_hdmi_connector_status_update(connector, connector->status);
>> }
>>
>> static const struct drm_connector_funcs dw_hdmi_connector_funcs = {
>>
>
>
>
>