On 19/09/2024 22:34, Jonas Karlman wrote:
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@xxxxxxxxx>
---
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.
I understand but if somehow a dw-hdmi integration fails to have HDP working
properly, EDID will be read continuously which is really not what we want.
HDMI 1.4b specifies in Section 8.5 and Appendix A:
============><==========================================
An HDMI Sink shall not assert high voltage level on its Hot Plug Detect pin when the E-EDID
is not available for reading.
An HDMI Sink shall indicate any change to the contents of the E-EDID by driving a low
voltage level pulse on the Hot Plug Detect pin. This pulse shall be at least 100 msec.
============><==========================================
So this is OK with the first sentence, and should also work with the second one because
right after the pulse we will read the EDID again, but I think we should have a much
more robust way to detect those 100ms pulses, no ?
Maxime, do you have an opinion on this ?
Neil
Regards,
Jonas
Neil
