Re: [RFC PATCH] drm/edid: drm_add_modes_noedid() should set lowest resolution as preferred

[Abhinav Kumar]

On 4/26/2022 1:21 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@xxxxxxxxxxxx>

drm_dmt_modes array is sorted but you are also relying on this check to 
eliminate the non-60fps modes

5611 		if (drm_mode_vrefresh(ptr) > 61)
5612 			continue;

I am not sure why we filter out the modes > 61 vrefresh.

If that check will remain this is okay.

If its not, its not reliable that the first mode will be 640x480@60

> ---
>
>   drivers/gpu/drm/drm_edid.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7a8482b75071..64ccfff4167e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5839,6 +5839,15 @@ int drm_add_modes_noedid(struct drm_connector *connector,
>   			continue;
>   		mode = drm_mode_duplicate(dev, ptr);
>   		if (mode) {
> +			/*
> +			 * The drm_dmt_modes array is sorted so that lower
> +			 * resolutions come first. We'll set the lowest
> +			 * resolution mode as preferred. We have no EDID so
> +			 * we should prefer the lowest resolution mode as
> +			 * the safest one.
> +			 */
> +			if (num_modes == 0)
> +				mode->type |= DRM_MODE_TYPE_PREFERRED;
>   			drm_mode_probed_add(connector, mode);
>   			num_modes++;
>   		}