[PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Douglas Anderson]

If we're unable to read the EDID for a display because it's corrupt /
bogus / invalid then we'll add a set of standard modes for the
display. When userspace looks at these modes it doesn't really have a
good concept for which mode to pick and it'll likely pick the highest
resolution one by default. That's probably not ideal because the modes
were purely guesses on the part of the Linux kernel.

Let's instead set 640x480 as the "preferred" mode when we have no EDID.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Note that this is the second of two related and similar-sounding but
different patches. See also ("drm/probe-helper: For DP, add 640x480 if
all other modes are bad") [1]. I'm hoping to land _both_ of the
patches since they address different issues. This patch addresses the
case of a corrupt EDID and having 640x480 be the default in the
"guessed" modes. The other patch handles the case where the EDID
_isn't_ corrupt but all the modes listed can't be made with the
existing situations. The two patches can land in either order.

Also note that I didn't carry any Tested-by / Reviewed-by tags since
the patch is now quite different.

[1] https://lore.kernel.org/r/20220510131309.v2.2.I4ac7f55aa446699f8c200a23c10463256f6f439f@changeid

Changes in v2:
- Don't modify drm_add_modes_noedid() 'cause that'll break others
- Set 640x480 as preferred in drm_helper_probe_single_connector_modes()

 drivers/gpu/drm/drm_probe_helper.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 682359512996..1fbb9a8c315c 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -516,8 +516,17 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		count = drm_add_override_edid_modes(connector);
 
 	if (count == 0 && (connector->status == connector_status_connected ||
-			   connector->status == connector_status_unknown))
+			   connector->status == connector_status_unknown)) {
 		count = drm_add_modes_noedid(connector, 1024, 768);
+
+		/*
+		 * Give userspace a hint that we don't have a lot of confidence
+		 * in these modes (we totally guessed) by marking 640x480 as
+		 * preferred. This is low clock rate and incredibly common as
+		 * a failsafe mode.
+		 */
+		drm_set_preferred_mode(connector, 640, 480);
+	}
 	count += drm_helper_probe_add_cmdline_mode(connector);
 	if (count == 0)
 		goto prune;
-- 
2.36.0.550.gb090851708-goog

[PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Douglas Anderson]

If we're unable to read the EDID for a display because it's corrupt /
bogus / invalid then we'll add a set of standard modes for the
display. When userspace looks at these modes it doesn't really have a
good concept for which mode to pick and it'll likely pick the highest
resolution one by default. That's probably not ideal because the modes
were purely guesses on the part of the Linux kernel.

Let's instead set 640x480 as the "preferred" mode when we have no EDID.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Note that this is the second of two related and similar-sounding but
different patches. See also ("drm/probe-helper: For DP, add 640x480 if
all other modes are bad") [1]. I'm hoping to land _both_ of the
patches since they address different issues. This patch addresses the
case of a corrupt EDID and having 640x480 be the default in the
"guessed" modes. The other patch handles the case where the EDID
_isn't_ corrupt but all the modes listed can't be made with the
existing situations. The two patches can land in either order.

Also note that I didn't carry any Tested-by / Reviewed-by tags since
the patch is now quite different.

[1] https://lore.kernel.org/r/20220510131309.v2.2.I4ac7f55aa446699f8c200a23c10463256f6f439f@changeid

Changes in v2:
- Don't modify drm_add_modes_noedid() 'cause that'll break others
- Set 640x480 as preferred in drm_helper_probe_single_connector_modes()

 drivers/gpu/drm/drm_probe_helper.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 682359512996..1fbb9a8c315c 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -516,8 +516,17 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		count = drm_add_override_edid_modes(connector);
 
 	if (count == 0 && (connector->status == connector_status_connected ||
-			   connector->status == connector_status_unknown))
+			   connector->status == connector_status_unknown)) {
 		count = drm_add_modes_noedid(connector, 1024, 768);
+
+		/*
+		 * Give userspace a hint that we don't have a lot of confidence
+		 * in these modes (we totally guessed) by marking 640x480 as
+		 * preferred. This is low clock rate and incredibly common as
+		 * a failsafe mode.
+		 */
+		drm_set_preferred_mode(connector, 640, 480);
+	}
 	count += drm_helper_probe_add_cmdline_mode(connector);
 	if (count == 0)
 		goto prune;
-- 
2.36.0.550.gb090851708-goog

Re: [PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Abhinav Kumar]

On 5/10/2022 1:51 PM, Douglas Anderson wrote:
> If we're unable to read the EDID for a display because it's corrupt /
> bogus / invalid then we'll add a set of standard modes for the
> display. When userspace looks at these modes it doesn't really have a
> good concept for which mode to pick and it'll likely pick the highest
> resolution one by default. That's probably not ideal because the modes
> were purely guesses on the part of the Linux kernel.
> 
> Let's instead set 640x480 as the "preferred" mode when we have no EDID.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
> Note that this is the second of two related and similar-sounding but
> different patches. See also ("drm/probe-helper: For DP, add 640x480 if
> all other modes are bad") [1]. I'm hoping to land _both_ of the
> patches since they address different issues. This patch addresses the
> case of a corrupt EDID and having 640x480 be the default in the
> "guessed" modes. The other patch handles the case where the EDID
> _isn't_ corrupt but all the modes listed can't be made with the
> existing situations. The two patches can land in either order.
> 
> Also note that I didn't carry any Tested-by / Reviewed-by tags since
> the patch is now quite different.
> 
> [1] https://lore.kernel.org/r/20220510131309.v2.2.I4ac7f55aa446699f8c200a23c10463256f6f439f@changeid
> 
> Changes in v2:
> - Don't modify drm_add_modes_noedid() 'cause that'll break others
> - Set 640x480 as preferred in drm_helper_probe_single_connector_modes()
> 
>   drivers/gpu/drm/drm_probe_helper.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 682359512996..1fbb9a8c315c 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -516,8 +516,17 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>   		count = drm_add_override_edid_modes(connector);
>   
>   	if (count == 0 && (connector->status == connector_status_connected ||
> -			   connector->status == connector_status_unknown))
> +			   connector->status == connector_status_unknown)) {
>   		count = drm_add_modes_noedid(connector, 1024, 768);
> +
> +		/*
> +		 * Give userspace a hint that we don't have a lot of confidence
> +		 * in these modes (we totally guessed) by marking 640x480 as
> +		 * preferred. This is low clock rate and incredibly common as
> +		 * a failsafe mode.
> +		 */
> +		drm_set_preferred_mode(connector, 640, 480);
> +	}
>   	count += drm_helper_probe_add_cmdline_mode(connector);
>   	if (count == 0)
>   		goto prune;

Re: [PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Abhinav Kumar]

On 5/10/2022 1:51 PM, Douglas Anderson wrote:
> If we're unable to read the EDID for a display because it's corrupt /
> bogus / invalid then we'll add a set of standard modes for the
> display. When userspace looks at these modes it doesn't really have a
> good concept for which mode to pick and it'll likely pick the highest
> resolution one by default. That's probably not ideal because the modes
> were purely guesses on the part of the Linux kernel.
> 
> Let's instead set 640x480 as the "preferred" mode when we have no EDID.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
> Note that this is the second of two related and similar-sounding but
> different patches. See also ("drm/probe-helper: For DP, add 640x480 if
> all other modes are bad") [1]. I'm hoping to land _both_ of the
> patches since they address different issues. This patch addresses the
> case of a corrupt EDID and having 640x480 be the default in the
> "guessed" modes. The other patch handles the case where the EDID
> _isn't_ corrupt but all the modes listed can't be made with the
> existing situations. The two patches can land in either order.
> 
> Also note that I didn't carry any Tested-by / Reviewed-by tags since
> the patch is now quite different.
> 
> [1] https://lore.kernel.org/r/20220510131309.v2.2.I4ac7f55aa446699f8c200a23c10463256f6f439f@changeid
> 
> Changes in v2:
> - Don't modify drm_add_modes_noedid() 'cause that'll break others
> - Set 640x480 as preferred in drm_helper_probe_single_connector_modes()
> 
>   drivers/gpu/drm/drm_probe_helper.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 682359512996..1fbb9a8c315c 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -516,8 +516,17 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>   		count = drm_add_override_edid_modes(connector);
>   
>   	if (count == 0 && (connector->status == connector_status_connected ||
> -			   connector->status == connector_status_unknown))
> +			   connector->status == connector_status_unknown)) {
>   		count = drm_add_modes_noedid(connector, 1024, 768);
> +
> +		/*
> +		 * Give userspace a hint that we don't have a lot of confidence
> +		 * in these modes (we totally guessed) by marking 640x480 as
> +		 * preferred. This is low clock rate and incredibly common as
> +		 * a failsafe mode.
> +		 */
> +		drm_set_preferred_mode(connector, 640, 480);
> +	}
>   	count += drm_helper_probe_add_cmdline_mode(connector);
>   	if (count == 0)
>   		goto prune;

Re: [PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3159 bytes --]

Hi

Am 10.05.22 um 22:51 schrieb Douglas Anderson:
> If we're unable to read the EDID for a display because it's corrupt /
> bogus / invalid then we'll add a set of standard modes for the
> display. When userspace looks at these modes it doesn't really have a
> good concept for which mode to pick and it'll likely pick the highest
> resolution one by default. That's probably not ideal because the modes
> were purely guesses on the part of the Linux kernel.

I'm skeptical. Why does the kernel do a better job than userspace here? 
Only the graphics driver could possibly make such a decision.

Not setting any preferred mode at least gives a clear message to userspace.

Best regards
Thomas

> 
> Let's instead set 640x480 as the "preferred" mode when we have no EDID.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Note that this is the second of two related and similar-sounding but
> different patches. See also ("drm/probe-helper: For DP, add 640x480 if
> all other modes are bad") [1]. I'm hoping to land _both_ of the
> patches since they address different issues. This patch addresses the
> case of a corrupt EDID and having 640x480 be the default in the
> "guessed" modes. The other patch handles the case where the EDID
> _isn't_ corrupt but all the modes listed can't be made with the
> existing situations. The two patches can land in either order.
> 
> Also note that I didn't carry any Tested-by / Reviewed-by tags since
> the patch is now quite different.
> 
> [1] https://lore.kernel.org/r/20220510131309.v2.2.I4ac7f55aa446699f8c200a23c10463256f6f439f@changeid
> 
> Changes in v2:
> - Don't modify drm_add_modes_noedid() 'cause that'll break others
> - Set 640x480 as preferred in drm_helper_probe_single_connector_modes()
> 
>   drivers/gpu/drm/drm_probe_helper.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 682359512996..1fbb9a8c315c 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -516,8 +516,17 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>   		count = drm_add_override_edid_modes(connector);
>   
>   	if (count == 0 && (connector->status == connector_status_connected ||
> -			   connector->status == connector_status_unknown))
> +			   connector->status == connector_status_unknown)) {
>   		count = drm_add_modes_noedid(connector, 1024, 768);
> +
> +		/*
> +		 * Give userspace a hint that we don't have a lot of confidence
> +		 * in these modes (we totally guessed) by marking 640x480 as
> +		 * preferred. This is low clock rate and incredibly common as
> +		 * a failsafe mode.
> +		 */
> +		drm_set_preferred_mode(connector, 640, 480);
> +	}
>   	count += drm_helper_probe_add_cmdline_mode(connector);
>   	if (count == 0)
>   		goto prune;

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3159 bytes --]

Hi

Am 10.05.22 um 22:51 schrieb Douglas Anderson:
> If we're unable to read the EDID for a display because it's corrupt /
> bogus / invalid then we'll add a set of standard modes for the
> display. When userspace looks at these modes it doesn't really have a
> good concept for which mode to pick and it'll likely pick the highest
> resolution one by default. That's probably not ideal because the modes
> were purely guesses on the part of the Linux kernel.

I'm skeptical. Why does the kernel do a better job than userspace here? 
Only the graphics driver could possibly make such a decision.

Not setting any preferred mode at least gives a clear message to userspace.

Best regards
Thomas

> 
> Let's instead set 640x480 as the "preferred" mode when we have no EDID.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Note that this is the second of two related and similar-sounding but
> different patches. See also ("drm/probe-helper: For DP, add 640x480 if
> all other modes are bad") [1]. I'm hoping to land _both_ of the
> patches since they address different issues. This patch addresses the
> case of a corrupt EDID and having 640x480 be the default in the
> "guessed" modes. The other patch handles the case where the EDID
> _isn't_ corrupt but all the modes listed can't be made with the
> existing situations. The two patches can land in either order.
> 
> Also note that I didn't carry any Tested-by / Reviewed-by tags since
> the patch is now quite different.
> 
> [1] https://lore.kernel.org/r/20220510131309.v2.2.I4ac7f55aa446699f8c200a23c10463256f6f439f@changeid
> 
> Changes in v2:
> - Don't modify drm_add_modes_noedid() 'cause that'll break others
> - Set 640x480 as preferred in drm_helper_probe_single_connector_modes()
> 
>   drivers/gpu/drm/drm_probe_helper.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 682359512996..1fbb9a8c315c 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -516,8 +516,17 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>   		count = drm_add_override_edid_modes(connector);
>   
>   	if (count == 0 && (connector->status == connector_status_connected ||
> -			   connector->status == connector_status_unknown))
> +			   connector->status == connector_status_unknown)) {
>   		count = drm_add_modes_noedid(connector, 1024, 768);
> +
> +		/*
> +		 * Give userspace a hint that we don't have a lot of confidence
> +		 * in these modes (we totally guessed) by marking 640x480 as
> +		 * preferred. This is low clock rate and incredibly common as
> +		 * a failsafe mode.
> +		 */
> +		drm_set_preferred_mode(connector, 640, 480);
> +	}
>   	count += drm_helper_probe_add_cmdline_mode(connector);
>   	if (count == 0)
>   		goto prune;

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Doug Anderson]

Hi,

On Wed, May 11, 2022 at 12:14 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 10.05.22 um 22:51 schrieb Douglas Anderson:
> > If we're unable to read the EDID for a display because it's corrupt /
> > bogus / invalid then we'll add a set of standard modes for the
> > display. When userspace looks at these modes it doesn't really have a
> > good concept for which mode to pick and it'll likely pick the highest
> > resolution one by default. That's probably not ideal because the modes
> > were purely guesses on the part of the Linux kernel.
>
> I'm skeptical. Why does the kernel do a better job than userspace here?
> Only the graphics driver could possibly make such a decision.
>
> Not setting any preferred mode at least gives a clear message to userspace.

OK, that's a fair point. So I tried to find out what our userspace is
doing. I believe it's:

https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/drm/common/drm_util.cc;l=529

Specifically this bit of code:

  // If we still have no preferred mode, then use the first one since it should
  // be the best mode.
  if (!*out_native_mode && !modes.empty())
    *out_native_mode = modes.front().get();

Do you agree with what our userspace is doing here, or is it wrong?

If our userspace is doing the right thing, then I guess the problem is
the call to "drm_mode_sort(&connector->modes);" at the end of
drm_helper_probe_single_connector_modes(). Would you be OK with me
_not_ sorting the modes in the "bad EDID" case? That also seems to fix
my problem...

-Doug

Re: [PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Doug Anderson]

Hi,

On Wed, May 11, 2022 at 12:14 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 10.05.22 um 22:51 schrieb Douglas Anderson:
> > If we're unable to read the EDID for a display because it's corrupt /
> > bogus / invalid then we'll add a set of standard modes for the
> > display. When userspace looks at these modes it doesn't really have a
> > good concept for which mode to pick and it'll likely pick the highest
> > resolution one by default. That's probably not ideal because the modes
> > were purely guesses on the part of the Linux kernel.
>
> I'm skeptical. Why does the kernel do a better job than userspace here?
> Only the graphics driver could possibly make such a decision.
>
> Not setting any preferred mode at least gives a clear message to userspace.

OK, that's a fair point. So I tried to find out what our userspace is
doing. I believe it's:

https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/drm/common/drm_util.cc;l=529

Specifically this bit of code:

  // If we still have no preferred mode, then use the first one since it should
  // be the best mode.
  if (!*out_native_mode && !modes.empty())
    *out_native_mode = modes.front().get();

Do you agree with what our userspace is doing here, or is it wrong?

If our userspace is doing the right thing, then I guess the problem is
the call to "drm_mode_sort(&connector->modes);" at the end of
drm_helper_probe_single_connector_modes(). Would you be OK with me
_not_ sorting the modes in the "bad EDID" case? That also seems to fix
my problem...

-Doug

Re: [PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Doug Anderson]

Hi,

On Wed, May 11, 2022 at 2:32 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, May 11, 2022 at 12:14 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > Hi
> >
> > Am 10.05.22 um 22:51 schrieb Douglas Anderson:
> > > If we're unable to read the EDID for a display because it's corrupt /
> > > bogus / invalid then we'll add a set of standard modes for the
> > > display. When userspace looks at these modes it doesn't really have a
> > > good concept for which mode to pick and it'll likely pick the highest
> > > resolution one by default. That's probably not ideal because the modes
> > > were purely guesses on the part of the Linux kernel.
> >
> > I'm skeptical. Why does the kernel do a better job than userspace here?
> > Only the graphics driver could possibly make such a decision.
> >
> > Not setting any preferred mode at least gives a clear message to userspace.
>
> OK, that's a fair point. So I tried to find out what our userspace is
> doing. I believe it's:
>
> https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/drm/common/drm_util.cc;l=529
>
> Specifically this bit of code:
>
>   // If we still have no preferred mode, then use the first one since it should
>   // be the best mode.
>   if (!*out_native_mode && !modes.empty())
>     *out_native_mode = modes.front().get();
>
> Do you agree with what our userspace is doing here, or is it wrong?
>
> If our userspace is doing the right thing, then I guess the problem is
> the call to "drm_mode_sort(&connector->modes);" at the end of
> drm_helper_probe_single_connector_modes(). Would you be OK with me
> _not_ sorting the modes in the "bad EDID" case? That also seems to fix
> my problem...

I've implemented the "don't mark preferred, but don't sort" as a v3.
Hopefully it looks good.

https://lore.kernel.org/r/20220513130533.v3.1.I31ec454f8d4ffce51a7708a8092f8a6f9c929092@changeid

-Doug

Re: [PATCH v2] drm/probe-helper: Default to 640x480 if no EDID

[Doug Anderson]

Hi,

On Wed, May 11, 2022 at 2:32 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, May 11, 2022 at 12:14 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > Hi
> >
> > Am 10.05.22 um 22:51 schrieb Douglas Anderson:
> > > If we're unable to read the EDID for a display because it's corrupt /
> > > bogus / invalid then we'll add a set of standard modes for the
> > > display. When userspace looks at these modes it doesn't really have a
> > > good concept for which mode to pick and it'll likely pick the highest
> > > resolution one by default. That's probably not ideal because the modes
> > > were purely guesses on the part of the Linux kernel.
> >
> > I'm skeptical. Why does the kernel do a better job than userspace here?
> > Only the graphics driver could possibly make such a decision.
> >
> > Not setting any preferred mode at least gives a clear message to userspace.
>
> OK, that's a fair point. So I tried to find out what our userspace is
> doing. I believe it's:
>
> https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/drm/common/drm_util.cc;l=529
>
> Specifically this bit of code:
>
>   // If we still have no preferred mode, then use the first one since it should
>   // be the best mode.
>   if (!*out_native_mode && !modes.empty())
>     *out_native_mode = modes.front().get();
>
> Do you agree with what our userspace is doing here, or is it wrong?
>
> If our userspace is doing the right thing, then I guess the problem is
> the call to "drm_mode_sort(&connector->modes);" at the end of
> drm_helper_probe_single_connector_modes(). Would you be OK with me
> _not_ sorting the modes in the "bad EDID" case? That also seems to fix
> my problem...

I've implemented the "don't mark preferred, but don't sort" as a v3.
Hopefully it looks good.

https://lore.kernel.org/r/20220513130533.v3.1.I31ec454f8d4ffce51a7708a8092f8a6f9c929092@changeid

-Doug