[PATCH] drm/vmwgfx: Filter modes which exceed 3/4 of graphics memory.

[Thomas Zimmermann]

Hi

Am 12.01.24 um 21:38 schrieb Ian Forbes:
> SVGA requires 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.
> 
> Additionally, device commands which use multiple graphics resources must
> have all their resources fit within graphics memory for the duration of the
> command. Thus we need a small carve out of 1/4 of graphics memory to ensure
> commands likes surface copies to the primary framebuffer for cursor
> composition or damage clips can fit within graphics memory.
> 
> 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.

That is a long-standing problem, which we have observed with other 
drivers as well. On low-memory devices, TTM doesn't play well. The real 
fix would be to export all modes that possibly fit and sort out the 
invalid configurations in atomic_check. It's just a lot more work.

Did you consider simply ignoring vmwgfx devices with less than 64 MiB of 
VRAM?

> 
> Signed-off-by: Ian Forbes <ian.forbes at broadcom.com>
> ---
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 20 ++++++++++++--------
>   1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 28ff30e32fab..39d6d17fc488 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2854,12 +2854,12 @@ enum drm_mode_status vmw_connector_mode_valid(struct drm_connector *connector,
>   	struct vmw_private *dev_priv = vmw_priv(dev);
>   	u32 max_width = dev_priv->texture_max_width;
>   	u32 max_height = dev_priv->texture_max_height;
> -	u32 assumed_cpp = 4;
> +	u32 assumed_cpp = dev_priv->assume_16bpp ? 2 : 4;
> +	u32 pitch = mode->hdisplay * assumed_cpp;
> +	u64 total = mode->vdisplay * pitch;
> +	bool using_stdu = dev_priv->active_display_unit == vmw_du_screen_target;
>   
> -	if (dev_priv->assume_16bpp)
> -		assumed_cpp = 2;
> -
> -	if (dev_priv->active_display_unit == vmw_du_screen_target) {
> +	if (using_stdu) {
>   		max_width  = min(dev_priv->stdu_max_width,  max_width);
>   		max_height = min(dev_priv->stdu_max_height, max_height);
>   	}
> @@ -2870,9 +2870,13 @@ enum drm_mode_status vmw_connector_mode_valid(struct drm_connector *connector,
>   	if (max_height < mode->vdisplay)
>   		return MODE_BAD_VVALUE;
>   
> -	if (!vmw_kms_validate_mode_vram(dev_priv,
> -			mode->hdisplay * assumed_cpp,
> -			mode->vdisplay))
> +	if (using_stdu &&
> +		(total > (dev_priv->max_mob_pages * PAGE_SIZE * 3 / 4) ||

IDK the details of vmwgfx's memory management, but instead of this '3/4 
test', wouldn't it be better to partition the VRAM via TTM and test 
against the partition sizes?

Best regards
Thomas

> +		total > dev_priv->max_mob_size)) {
> +		return MODE_MEM;
> +	}
> +
> +	if (!vmw_kms_validate_mode_vram(dev_priv, pitch, mode->vdisplay))
>   		return MODE_MEM;
>   
>   	return MODE_OK;

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240115/d2e748f7/attachment.sig>