diff mbox series

drm/vmwgfx: Filter modes which exceed graphics memory

Message ID 20240401195645.31081-1-ian.forbes@broadcom.com (mailing list archive)
State New, archived
Headers show
Series drm/vmwgfx: Filter modes which exceed graphics memory | expand

Commit Message

Ian Forbes April 1, 2024, 7:56 p.m. UTC
SVGA requires individual surfaces to fit within graphics memory
(max_mob_pages) which means that modes with a final buffer size that would
exceed graphics memory must be pruned otherwise creation will fail.

This fixes an issue where VMs with low graphics memory (< 64MiB) configured
with high resolution mode boot to a black screen because surface creation
fails.

Fixes: d947d1b71deb ("drm/vmwgfx: Add and connect connector helper function")
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 32 +++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

Comments

Zack Rusin April 2, 2024, 11:42 a.m. UTC | #1
On Mon, Apr 1, 2024 at 4:35 PM Ian Forbes <ian.forbes@broadcom.com> wrote:
>
> SVGA requires individual surfaces to fit within graphics memory
> (max_mob_pages) which means that modes with a final buffer size that would
> exceed graphics memory must be pruned otherwise creation will fail.
>
> This fixes an issue where VMs with low graphics memory (< 64MiB) configured
> with high resolution mode boot to a black screen because surface creation
> fails.
>
> Fixes: d947d1b71deb ("drm/vmwgfx: Add and connect connector helper function")
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 32 +++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 3c8414a13dba..49583b186a7d 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -830,7 +830,37 @@ static void vmw_stdu_connector_destroy(struct drm_connector *connector)
>         vmw_stdu_destroy(vmw_connector_to_stdu(connector));
>  }
>
> +static enum drm_mode_status
> +vmw_stdu_connector_mode_valid(struct drm_connector *connector,
> +                             struct drm_display_mode *mode)
> +{
> +       enum drm_mode_status ret;
> +       struct drm_device *dev = connector->dev;
> +       struct vmw_private *dev_priv = vmw_priv(dev);
> +       u64 assumed_cpp = dev_priv->assume_16bpp ? 2 : 4;
> +       u64 required_mem = mode->hdisplay * assumed_cpp * mode->vdisplay;
> +
> +       ret = drm_mode_validate_size(mode, dev_priv->stdu_max_width,
> +                                    dev_priv->stdu_max_height);
> +       if (ret != MODE_OK)
> +               return ret;
> +
> +       ret = drm_mode_validate_size(mode, dev_priv->texture_max_width,
> +                                    dev_priv->texture_max_height);
> +       if (ret != MODE_OK)
> +               return ret;
>
> +       if (required_mem > dev_priv->max_primary_mem)
> +               return MODE_MEM;
> +
> +       if (required_mem > dev_priv->max_mob_pages * PAGE_SIZE)
> +               return MODE_MEM;
> +
> +       if (required_mem > dev_priv->max_mob_size)
> +               return MODE_MEM;
> +
> +       return MODE_OK;
> +}
>
>  static const struct drm_connector_funcs vmw_stdu_connector_funcs = {
>         .dpms = vmw_du_connector_dpms,
> @@ -846,7 +876,7 @@ static const struct drm_connector_funcs vmw_stdu_connector_funcs = {
>  static const struct
>  drm_connector_helper_funcs vmw_stdu_connector_helper_funcs = {
>         .get_modes = vmw_connector_get_modes,
> -       .mode_valid = vmw_connector_mode_valid
> +       .mode_valid = vmw_stdu_connector_mode_valid
>  };
>
>
> --
> 2.34.1
>

This looks like a great start. Some improvements that I'd suggest is
to take a look at
bora/vmcore/frobos/test/common/svga/1523068-svga-screen-limits/main.c
where those computations are spelled out a bit more verbose. I'd
suggest following them because those are being tested all the time.
It'd be great if we also covered the multimon case here, but it's not
our main concern.

The second thing that we'd want to adjust is that if we're not using
vmw_connector_mode_valid then we need to remove the stdu paths from
it.

Finally I'd suggest making this a series, i.e. include all the changes
we've talked about like fixing all of the display technologies,
disabling 3d etc iirc we talked about priority list among those at
some time.

z
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 3c8414a13dba..49583b186a7d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -830,7 +830,37 @@  static void vmw_stdu_connector_destroy(struct drm_connector *connector)
 	vmw_stdu_destroy(vmw_connector_to_stdu(connector));
 }
 
+static enum drm_mode_status
+vmw_stdu_connector_mode_valid(struct drm_connector *connector,
+			      struct drm_display_mode *mode)
+{
+	enum drm_mode_status ret;
+	struct drm_device *dev = connector->dev;
+	struct vmw_private *dev_priv = vmw_priv(dev);
+	u64 assumed_cpp = dev_priv->assume_16bpp ? 2 : 4;
+	u64 required_mem = mode->hdisplay * assumed_cpp * mode->vdisplay;
+
+	ret = drm_mode_validate_size(mode, dev_priv->stdu_max_width,
+				     dev_priv->stdu_max_height);
+	if (ret != MODE_OK)
+		return ret;
+
+	ret = drm_mode_validate_size(mode, dev_priv->texture_max_width,
+				     dev_priv->texture_max_height);
+	if (ret != MODE_OK)
+		return ret;
 
+	if (required_mem > dev_priv->max_primary_mem)
+		return MODE_MEM;
+
+	if (required_mem > dev_priv->max_mob_pages * PAGE_SIZE)
+		return MODE_MEM;
+
+	if (required_mem > dev_priv->max_mob_size)
+		return MODE_MEM;
+
+	return MODE_OK;
+}
 
 static const struct drm_connector_funcs vmw_stdu_connector_funcs = {
 	.dpms = vmw_du_connector_dpms,
@@ -846,7 +876,7 @@  static const struct drm_connector_funcs vmw_stdu_connector_funcs = {
 static const struct
 drm_connector_helper_funcs vmw_stdu_connector_helper_funcs = {
 	.get_modes = vmw_connector_get_modes,
-	.mode_valid = vmw_connector_mode_valid
+	.mode_valid = vmw_stdu_connector_mode_valid
 };