drivers/gpu/drm/msm/dp/dp_panel.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
From: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
drm_edid_read_ddc() can return a structurally valid EDID from which
drm_edid_connector_add_modes() still returns zero modes. This triggers
the error:
[drm:msm_dp_bridge_get_modes [msm]] *ERROR* failed to get DP sink
modes, rc=0
even though the link is ready. Since the EDID is only read once, this
error persists and the display comes up with 640x480 resolution.
Add a retry of drm_edid_read_ddc() inside get_modes() when the initial
read produces no usable modes. The bad EDID is freed before retrying
and container_of ensures access to the DDC channel. This directly
addresses the observed "valid but empty/garbage" EDID case on
flaky DP plugs and adapters.
I tested this on a few of my "flaky" type-c to HDMI adapters and hubs,
getting no "retry failed" messages and the desired resolution. Without
the patch most plugs would result in 640x480 external display.
Fixes: [5bea90ad9743d2] "drm/msm/dp: switch to struct drm_edid"
Signed-off-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
---
drivers/gpu/drm/msm/dp/dp_panel.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 6bb021820d7c5..4f27c8129b0ef 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -327,11 +327,39 @@ u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel,
int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
struct drm_connector *connector)
{
+ int modes;
+ int retry;
+ struct msm_dp_panel_private *panel;
+
if (!msm_dp_panel) {
DRM_ERROR("invalid input\n");
return -EINVAL;
}
+ panel = container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
+
+ if (msm_dp_panel->drm_edid) {
+ modes = drm_edid_connector_add_modes(connector);
+ if (modes > 0)
+ return modes;
+
+ drm_edid_free(msm_dp_panel->drm_edid);
+ msm_dp_panel->drm_edid = NULL;
+ }
+
+ for (retry = 0; retry < 5; retry++) {
+ usleep_range(20000, 25000);
+ msm_dp_panel->drm_edid =
+ drm_edid_read_ddc(connector, &panel->aux->ddc);
+ if (msm_dp_panel->drm_edid)
+ break;
+
+ drm_dbg_dp(connector->dev,
+ "get_modes re-read attempt %d/5 failed\n",
+ retry + 1);
+ }
+
+ drm_edid_connector_update(connector, msm_dp_panel->drm_edid);
if (msm_dp_panel->drm_edid)
return drm_edid_connector_add_modes(connector);
---
base-commit: 7da7f07112610a520567421dd2ffcb51beaefbcc
change-id: 20260531-drm_plug_flaky_edid-cc7743f6f909
Best regards,
--
Jens Glathe <jens.glathe@oldschoolsolutions.biz>On Sun, May 31, 2026 at 11:16:51PM +0200, Jens Glathe via B4 Relay wrote: > From: Jens Glathe <jens.glathe@oldschoolsolutions.biz> > > drm_edid_read_ddc() can return a structurally valid EDID from which When? > drm_edid_connector_add_modes() still returns zero modes. This triggers > the error: Please at least add EDID contents. > > [drm:msm_dp_bridge_get_modes [msm]] *ERROR* failed to get DP sink > modes, rc=0 > > even though the link is ready. Since the EDID is only read once, this > error persists and the display comes up with 640x480 resolution. Please check with the current next tree, we have reworked HPD handling a bit. Also, it might be the case that the dongle sends a placeholder first and then sends a HPD or IRQ_HPD event. Please check if that's the case. > > Add a retry of drm_edid_read_ddc() inside get_modes() when the initial > read produces no usable modes. The bad EDID is freed before retrying > and container_of ensures access to the DDC channel. This directly > addresses the observed "valid but empty/garbage" EDID case on > flaky DP plugs and adapters. > > I tested this on a few of my "flaky" type-c to HDMI adapters and hubs, > getting no "retry failed" messages and the desired resolution. Without > the patch most plugs would result in 640x480 external display. > > Fixes: [5bea90ad9743d2] "drm/msm/dp: switch to struct drm_edid" Hard to believe. > > Signed-off-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz> > --- > drivers/gpu/drm/msm/dp/dp_panel.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > -- With best wishes Dmitry
On Sun, 31 May 2026, Jens Glathe via B4 Relay <devnull+jens.glathe.oldschoolsolutions.biz@kernel.org> wrote:
> From: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
>
> drm_edid_read_ddc() can return a structurally valid EDID from which
> drm_edid_connector_add_modes() still returns zero modes. This triggers
> the error:
>
> [drm:msm_dp_bridge_get_modes [msm]] *ERROR* failed to get DP sink
> modes, rc=0
>
> even though the link is ready. Since the EDID is only read once, this
> error persists and the display comes up with 640x480 resolution.
>
> Add a retry of drm_edid_read_ddc() inside get_modes() when the initial
> read produces no usable modes. The bad EDID is freed before retrying
> and container_of ensures access to the DDC channel. This directly
> addresses the observed "valid but empty/garbage" EDID case on
> flaky DP plugs and adapters.
>
> I tested this on a few of my "flaky" type-c to HDMI adapters and hubs,
> getting no "retry failed" messages and the desired resolution. Without
> the patch most plugs would result in 640x480 external display.
>
> Fixes: [5bea90ad9743d2] "drm/msm/dp: switch to struct drm_edid"
Did you actually bisect to this commit? It's a bit of a surprise.
Side note, the Fixes: trailer has a strict format. Please look at git
logs for what it should be like.
>
> Signed-off-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> ---
> drivers/gpu/drm/msm/dp/dp_panel.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 6bb021820d7c5..4f27c8129b0ef 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -327,11 +327,39 @@ u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel,
> int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
> struct drm_connector *connector)
> {
> + int modes;
> + int retry;
> + struct msm_dp_panel_private *panel;
> +
> if (!msm_dp_panel) {
> DRM_ERROR("invalid input\n");
> return -EINVAL;
> }
>
> + panel = container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
> +
> + if (msm_dp_panel->drm_edid) {
> + modes = drm_edid_connector_add_modes(connector);
> + if (modes > 0)
> + return modes;
> +
> + drm_edid_free(msm_dp_panel->drm_edid);
> + msm_dp_panel->drm_edid = NULL;
> + }
> +
> + for (retry = 0; retry < 5; retry++) {
> + usleep_range(20000, 25000);
> + msm_dp_panel->drm_edid =
> + drm_edid_read_ddc(connector, &panel->aux->ddc);
> + if (msm_dp_panel->drm_edid)
> + break;
> +
> + drm_dbg_dp(connector->dev,
> + "get_modes re-read attempt %d/5 failed\n",
> + retry + 1);
> + }
> +
> + drm_edid_connector_update(connector, msm_dp_panel->drm_edid);
> if (msm_dp_panel->drm_edid)
> return drm_edid_connector_add_modes(connector);
>
>
> ---
> base-commit: 7da7f07112610a520567421dd2ffcb51beaefbcc
> change-id: 20260531-drm_plug_flaky_edid-cc7743f6f909
>
> Best regards,
--
Jani Nikula, Intel
On 01.06.26 12:57, Jani Nikula wrote: > Did you actually bisect to this commit? It's a bit of a surprise. Hi Jani, haha yes, sort of. I did a few weeks of bisecting for a different issue [1] and this error message came up increasingly, so after resolving (or so) the first one I took a shot at it. The commit was the result of git blame for the line that the retry was programmed around. And as v2 shows, it was the right intuition. [1]: https://lore.kernel.org/all/20260530-typc-mux-modeset-v1-1-64b0281e2cd6@oldschoolsolutions.biz/ with best regards Jens
On Sun, 31 May 2026, Jens Glathe via B4 Relay <devnull+jens.glathe.oldschoolsolutions.biz@kernel.org> wrote:
> From: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
>
> drm_edid_read_ddc() can return a structurally valid EDID from which
> drm_edid_connector_add_modes() still returns zero modes. This triggers
> the error:
What are the EDID contents in that case?
Where does it originate from? A valid but "not what you wanted" EDID
doesn't show up from thin air.
Perhaps it's the hub/adapter replying with some placeholder EDID instead
of the actual sink EDID. Maybe it's the timing and/or ordering.
Feels like root causing would be in order before adding hacks like this
that are notoriously difficult to remove in the future, and will easily
block future progress.
BR,
Jani.
>
> [drm:msm_dp_bridge_get_modes [msm]] *ERROR* failed to get DP sink
> modes, rc=0
>
> even though the link is ready. Since the EDID is only read once, this
> error persists and the display comes up with 640x480 resolution.
>
> Add a retry of drm_edid_read_ddc() inside get_modes() when the initial
> read produces no usable modes. The bad EDID is freed before retrying
> and container_of ensures access to the DDC channel. This directly
> addresses the observed "valid but empty/garbage" EDID case on
> flaky DP plugs and adapters.
>
> I tested this on a few of my "flaky" type-c to HDMI adapters and hubs,
> getting no "retry failed" messages and the desired resolution. Without
> the patch most plugs would result in 640x480 external display.
>
> Fixes: [5bea90ad9743d2] "drm/msm/dp: switch to struct drm_edid"
>
> Signed-off-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> ---
> drivers/gpu/drm/msm/dp/dp_panel.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 6bb021820d7c5..4f27c8129b0ef 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -327,11 +327,39 @@ u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel,
> int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
> struct drm_connector *connector)
> {
> + int modes;
> + int retry;
> + struct msm_dp_panel_private *panel;
> +
> if (!msm_dp_panel) {
> DRM_ERROR("invalid input\n");
> return -EINVAL;
> }
>
> + panel = container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
> +
> + if (msm_dp_panel->drm_edid) {
> + modes = drm_edid_connector_add_modes(connector);
> + if (modes > 0)
> + return modes;
> +
> + drm_edid_free(msm_dp_panel->drm_edid);
> + msm_dp_panel->drm_edid = NULL;
> + }
> +
> + for (retry = 0; retry < 5; retry++) {
> + usleep_range(20000, 25000);
> + msm_dp_panel->drm_edid =
> + drm_edid_read_ddc(connector, &panel->aux->ddc);
> + if (msm_dp_panel->drm_edid)
> + break;
> +
> + drm_dbg_dp(connector->dev,
> + "get_modes re-read attempt %d/5 failed\n",
> + retry + 1);
> + }
> +
> + drm_edid_connector_update(connector, msm_dp_panel->drm_edid);
> if (msm_dp_panel->drm_edid)
> return drm_edid_connector_add_modes(connector);
>
>
> ---
> base-commit: 7da7f07112610a520567421dd2ffcb51beaefbcc
> change-id: 20260531-drm_plug_flaky_edid-cc7743f6f909
>
> Best regards,
--
Jani Nikula, Intel
© 2016 - 2026 Red Hat, Inc.