So the issue is that SVGA_3D_CMD_DX_PRED_COPY_REGION between 2
surfaces that are the size of the mode fails. Technically for this to
work the filter will have to be 1/2 of graphics mem. I was just lucky
that the next mode in the list was already less than 1/2. 3/4 is not
actually going to work. Also this only happens on X/Gnome and seems
more like an issue with the compositor. Wayland/Gnome displays the
desktop but it's unusable and glitches even with the 1/2 limit. I
don't think wayland even abides by the mode limits as I see it trying
to create surfaces larger than the mode. It might be using texture
limits instead.
On Fri, Feb 2, 2024 at 1:29 PM Zack Rusin <zack.rusin@xxxxxxxxxxxx> wrote:
>
> On Fri, Feb 2, 2024 at 11:58 AM Ian Forbes <ian.forbes@xxxxxxxxxxxx> wrote:
> >
> > 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.
>
> Sorry, I didn't notice this originally but that's not quite true. svga
> doesn't require all mob memory to stay within max_mob_pages (which is
> SVGA_REG_GBOBJECT_MEM_SIZE_KB). max_mob_pages is really max resident
> memory or suggested-guest-memory-for-best-performance. we can grow
> that memory (and we do). I think what's causing problems on systems
> with low memory is that cursor mobs and the fb's need to be both
> resident but can't. Now SVGA_REG_MAX_PRIMARY_MEM is the max memory in
> which our topology needs to fit in (which is max_primary_mem on
> vmwgfx) but afaict that's not the issue here and it's checked later in
> vmw_kms_validate_mode_vram
>
> > 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.
>
> Yes, we should probably rename max_mob_pages to max_resident_memory
> instead to make this obvious.
>
> > 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.
>
> Does this work if you disable gbobjects? Without gbobject's we won't
> have screen targets and thus won't be offsetting by 1/4 so I wonder if
> 4mb vram with legacy display would work with 1280x800 resolution.
>
> Also, you want to add a "V2" section to your change to describe what
> changed in v2 vs v1 (and same for any subsequent change).
>
> >
> > Signed-off-by: Ian Forbes <ian.forbes@xxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 ++++++++++++++--------
> > 1 file changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index cd4925346ed4..84e1b765cda3 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -2858,12 +2858,17 @@ 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;
> > -
> > - if (dev_priv->assume_16bpp)
> > - assumed_cpp = 2;
> > + 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;
> > + u64 max_mem_for_st = dev_priv->max_mob_pages * PAGE_SIZE * 3 / 4;
> > + /* ^^^ Max memory for the mode fb when using Screen Target / MOBs.
> > + * We need a carveout (1/4) to account for other gfx resources that are
> > + * required in gfx mem for an fb update to complete with low gfx mem (<64MiB).
> > + */
>
> Same wording issue as mentioned above and lets use normal comment
> style (i.e. comments attach to the code below). max_mem_for_st should
> probably be max_mem_for_mode or max_mem_for_mode_st.
>
> > - 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);
> > }
> > @@ -2874,9 +2879,10 @@ 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 > max_mem_for_st || total > dev_priv->max_mob_size))
> > + return MODE_MEM;
> > +
> > + if (!vmw_kms_validate_mode_vram(dev_priv, pitch, mode->vdisplay))
> > return MODE_MEM;
>
> It might make sense to just reuse vmw_kms_validate_mode_vram , it does
> what we're claiming to do here and even though it's called
> vmw_kms_validate_mode_vram it does actually validate st primary
> memory.
>
> z
