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

Nicolas Frattaroli
2026-04-28T11:53:48Z

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.

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.

>
> @@ -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 = {
>