diff mbox series

drm/vmwgfx: Add common resolutions to the connector mode list

Message ID 20241028201412.576163-1-ian.forbes@broadcom.com (mailing list archive)
State New, archived
Headers show
Series drm/vmwgfx: Add common resolutions to the connector mode list | expand

Commit Message

Ian Forbes Oct. 28, 2024, 8:14 p.m. UTC
We replaced our custom list of resolutions with the noedid list, which is
based on the VESA DMT standard, in the referenced fixes commit. The reason
for this was that the user can technically set any resolution they want by
using Autofit or the vmwgfxctl utility.

Unfortunately the Autofit feature is a global setting that is applied to
all VMs running in Workstation and some users preferred to set the mode
manually on certain VMs. Additionally the DMT standard does not include a
number of modern resolutions as it was last updated in 2013 and has since
been superseded.

This commit adds back some of the removed modes and adds some additional
common ones.

Fixes: 935f795045a6 ("drm/vmwgfx: Refactor drm connector probing for display modes")
Closes: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2081803
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 31 ++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

Comments

Zack Rusin Oct. 30, 2024, 7:33 p.m. UTC | #1
On Mon, Oct 28, 2024 at 4:14 PM Ian Forbes <ian.forbes@broadcom.com> wrote:
>
> We replaced our custom list of resolutions with the noedid list, which is
> based on the VESA DMT standard, in the referenced fixes commit. The reason
> for this was that the user can technically set any resolution they want by
> using Autofit or the vmwgfxctl utility.
>
> Unfortunately the Autofit feature is a global setting that is applied to
> all VMs running in Workstation and some users preferred to set the mode
> manually on certain VMs. Additionally the DMT standard does not include a
> number of modern resolutions as it was last updated in 2013 and has since
> been superseded.
>
> This commit adds back some of the removed modes and adds some additional
> common ones.
>
> Fixes: 935f795045a6 ("drm/vmwgfx: Refactor drm connector probing for display modes")
> Closes: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2081803
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 31 ++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index f39bf992364d..879b78543dee 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2681,6 +2681,16 @@ enum drm_mode_status vmw_connector_mode_valid(struct drm_connector *connector,
>         return MODE_OK;
>  }
>
> +/*
> + * Common modes not present in the VESA DMT standard or assigned a VIC.
> + */
> +static struct {
> +       int width;
> +       int height;
> +} common_modes[] = {   {2560, 1440}, // QHD
> +                       {3440, 1440}, // UWQHD
> +                       {3840, 2400}}; // WQUXGA

"static const" and maybe a better name would be vmw_extra_modes.

> +
>  /**
>   * vmw_connector_get_modes - implements drm_connector_helper_funcs.get_modes callback
>   *
> @@ -2725,7 +2735,26 @@ int vmw_connector_get_modes(struct drm_connector *connector)
>                 max_height = min(dev_priv->stdu_max_height, max_height);
>         }
>
> -       num_modes = 1 + drm_add_modes_noedid(connector, max_width, max_height);
> +       num_modes = 1;

If you could move it either under the drm_mode_probed_add or just
assign it at declaration that'd be a little better than what looks
almost like a random assignment now. BTW, preferred mode it might make
sense to just remove vmw_guess_mode_timing and use drm_cvt_mode
instead.

> +
> +       mode = drm_display_mode_from_cea_vic(dev, 97); // 4K UHD 16:9
> +       if (mode) {
> +               drm_mode_probed_add(connector, mode);
> +               num_modes++;
> +       }
> +
> +       for (int i = 0; i < ARRAY_SIZE(common_modes); i++) {
> +               mode = drm_cvt_mode(dev, common_modes[i].width,
> +                                   common_modes[i].height,
> +                                   60, true, false, false);
> +               if (mode) {
> +                       mode->type |= DRM_MODE_TYPE_DRIVER;
> +                       drm_mode_probed_add(connector, mode);
> +                       num_modes++;
> +               }
> +       }

Apart from those minor things, it looks great. We should probably
update the drm_add_modes_noedid to include the newer modes, but that
list is already pretty long so it might make sense to just include the
modes we really care about like you have here.

z
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index f39bf992364d..879b78543dee 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2681,6 +2681,16 @@  enum drm_mode_status vmw_connector_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
+/*
+ * Common modes not present in the VESA DMT standard or assigned a VIC.
+ */
+static struct {
+	int width;
+	int height;
+} common_modes[] = {	{2560, 1440}, // QHD
+			{3440, 1440}, // UWQHD
+			{3840, 2400}}; // WQUXGA
+
 /**
  * vmw_connector_get_modes - implements drm_connector_helper_funcs.get_modes callback
  *
@@ -2725,7 +2735,26 @@  int vmw_connector_get_modes(struct drm_connector *connector)
 		max_height = min(dev_priv->stdu_max_height, max_height);
 	}
 
-	num_modes = 1 + drm_add_modes_noedid(connector, max_width, max_height);
+	num_modes = 1;
+
+	mode = drm_display_mode_from_cea_vic(dev, 97); // 4K UHD 16:9
+	if (mode) {
+		drm_mode_probed_add(connector, mode);
+		num_modes++;
+	}
+
+	for (int i = 0; i < ARRAY_SIZE(common_modes); i++) {
+		mode = drm_cvt_mode(dev, common_modes[i].width,
+				    common_modes[i].height,
+				    60, true, false, false);
+		if (mode) {
+			mode->type |= DRM_MODE_TYPE_DRIVER;
+			drm_mode_probed_add(connector, mode);
+			num_modes++;
+		}
+	}
+
+	num_modes += drm_add_modes_noedid(connector, max_width, max_height);
 
 	return num_modes;
 }