[0/3] drm/client: Fix display-mode selection

Message

Thomas Zimmermann May 11, 2022, 6:31 p.m. UTC

Pick user-defined display mode in DRM clients if the mode has been
validated by the driver. Otherwise pick a preferred display mode.

Booting the kernel with video=<mode> and giving an unsupported display
mode can easily turn the display unusable. This is best tested by
booting simpledrm with a display mode that does not use the firmware
framebuffer's resolution. While simpledrm filter's out the mode as
invalid, the DRM client still picks it and the console won't show up.

Several factors contribute to this problem.

 * The connector invalidates the user-defined display mode, but never
   tells the user about it.
 * The DRM client doesn't look for user-defined display modes, but for
   modes that are similar.
 * If no similar mode can be found, the client adds the invalid display
   mode back to the connector's mode list for use.

Each of the patches in this patchset addresses one of these problems.
Overall the DRM client has no business in display-mode detection and
should only pick one of the modes that has been detected and validated 
by the connector.

Thomas Zimmermann (3):
  drm: Always warn if user-defined modes are not supported
  drm/client: Look for command-line modes first
  drm/client: Don't add new command-line mode

 drivers/gpu/drm/drm_client_modeset.c | 28 ++++++++++++++++------------
 drivers/gpu/drm/drm_modes.c          |  4 ++++
 2 files changed, 20 insertions(+), 12 deletions(-)

Comments

Maxime Ripard May 12, 2022, 8:55 a.m. UTC | #1

On Wed, May 11, 2022 at 08:31:22PM +0200, Thomas Zimmermann wrote:
> Pick user-defined display mode in DRM clients if the mode has been
> validated by the driver. Otherwise pick a preferred display mode.
> 
> Booting the kernel with video=<mode> and giving an unsupported display
> mode can easily turn the display unusable. This is best tested by
> booting simpledrm with a display mode that does not use the firmware
> framebuffer's resolution. While simpledrm filter's out the mode as
> invalid, the DRM client still picks it and the console won't show up.
> 
> Several factors contribute to this problem.
> 
>  * The connector invalidates the user-defined display mode, but never
>    tells the user about it.
>  * The DRM client doesn't look for user-defined display modes, but for
>    modes that are similar.
>  * If no similar mode can be found, the client adds the invalid display
>    mode back to the connector's mode list for use.
> 
> Each of the patches in this patchset addresses one of these problems.
> Overall the DRM client has no business in display-mode detection and
> should only pick one of the modes that has been detected and validated 
> by the connector.

That's awesome, thanks!

For the series,
Reviewed-by: Maxime Ripard <[e-mail]>

Maxime