diff mbox series

[3/3] drm/client: Don't add new command-line mode

Message ID 20220511183125.14294-4-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/client: Fix display-mode selection | expand

Commit Message

Thomas Zimmermann May 11, 2022, 6:31 p.m. UTC
Don't add a mode for the kernel's command-line parameters from
within the DRM client code. Doing so can result in an unusable
display. If there's no compatible command-line mode, the client
will use one of the connector's preferred modes.

All mode creation and validation has to be performed by the
connector. When clients run, the connector's fill_modes callback
has already processes the kernel parameters and validates each
mode before adding it. The connector's mode list does not contain
invalid modes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_client_modeset.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

Comments

Javier Martinez Canillas May 12, 2022, 7:20 a.m. UTC | #1
On 5/11/22 20:31, Thomas Zimmermann wrote:
> Don't add a mode for the kernel's command-line parameters from
> within the DRM client code. Doing so can result in an unusable
> display. If there's no compatible command-line mode, the client
> will use one of the connector's preferred modes.
> 
> All mode creation and validation has to be performed by the
> connector. When clients run, the connector's fill_modes callback
> has already processes the kernel parameters and validates each

s/process/processed and s/validates/validated ?

or 

remove the "has" in the sentence, either work I think.

> mode before adding it. The connector's mode list does not contain
> invalid modes.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_client_modeset.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index b777faa87f04..48e6ce16439f 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -158,8 +158,7 @@ drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int
>  	return NULL;
>  }
>  
> -static struct drm_display_mode *
> -drm_connector_pick_cmdline_mode(struct drm_connector *connector)
> +static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_connector *connector)

This seems like an unrelated cleanup.

The rest looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index b777faa87f04..48e6ce16439f 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -158,8 +158,7 @@  drm_connector_has_preferred_mode(struct drm_connector *connector, int width, int
 	return NULL;
 }
 
-static struct drm_display_mode *
-drm_connector_pick_cmdline_mode(struct drm_connector *connector)
+static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_connector *connector)
 {
 	struct drm_cmdline_mode *cmdline_mode;
 	struct drm_display_mode *mode;
@@ -180,11 +179,10 @@  drm_connector_pick_cmdline_mode(struct drm_connector *connector)
 	if (cmdline_mode->specified == false)
 		return NULL;
 
-	/* attempt to find a matching mode in the list of modes
-	 *  we have gotten so far, if not add a CVT mode that conforms
+	/*
+	 * Attempt to find a matching mode in the list of modes we
+	 * have gotten so far.
 	 */
-	if (cmdline_mode->rb || cmdline_mode->margins)
-		goto create_mode;
 
 	prefer_non_interlace = !cmdline_mode->interlace;
 again:
@@ -218,12 +216,7 @@  drm_connector_pick_cmdline_mode(struct drm_connector *connector)
 		goto again;
 	}
 
-create_mode:
-	mode = drm_mode_create_from_cmdline_mode(connector->dev, cmdline_mode);
-	if (mode)
-		list_add(&mode->head, &connector->modes);
-
-	return mode;
+	return NULL;
 }
 
 static bool drm_connector_enabled(struct drm_connector *connector, bool strict)