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 |
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 --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 };
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(-)