[Mesa-dev] [PATCH] i965: Fix GLX_MESA_query_renderer video memory on 32-bit.

[Kenneth Graunke]

On Thursday, March 30, 2017 4:38:14 PM PDT Chris Wilson wrote:
> On Thu, Mar 30, 2017 at 04:28:19PM -0700, Kenneth Graunke wrote:
> > On modern systems with 4GB apertures, the size in bytes is 4294967296,
> > or (1ull << 32).  The kernel gives us the aperture size as a __u64,
> > which works out great.
> > 
> > Unfortunately, libdrm "helpfully" returns the data as a size_t, which
> > on 32-bit systems means it truncates the aperture size to 0 bytes.
> > We've happily reported this value as 0 MB of video memory via
> > GLX_MESA_query_renderer since it was originally exposed.
> > 
> > This patch bypasses libdrm and calls the ioctl ourselves so we can
> > use a proper uint64_t, avoiding the 32-bit integer overflow.  We now
> > report a proper video memory size on 32-bit systems.
> > ---
> >  src/mesa/drivers/dri/i965/intel_screen.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> > index 811a9c5a867..f94e8a77c10 100644
> > --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > @@ -950,6 +950,17 @@ static const __DRIimageExtension intelImageExtension = {
> >      .createImageWithModifiers           = intel_create_image_with_modifiers,
> >  };
> >  
> > +static uint64_t
> > +get_aperture_size(int fd)
> > +{
> > +   struct drm_i915_gem_get_aperture aperture;
> > +
> > +   if (drmIoctl(fd, DRM_IOCTL_I915_GEM_GET_APERTURE, &aperture) != 0)
> > +      return 0;
> 
> The aperture is nothing to do with the video memory limits... You want
> to query the context for the size of the GTT, e.g.
> https://patchwork.freedesktop.org/patch/62189/
> 
> i.e.
> static uint64_t get_gtt_size(int fd)
> {
>    struct drm_i915_gem_context_param p;
>    size_t mappable_size, aper_size;
> 
>    memset(&p, 0, sizeof(p));
>    p.param = I915_CONTEXT_PARAM_GTT_SIZE;
>    if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &p) == 0)
>       return p.value;
> 
>    /* do sometheing useful for old kernels */
> 
>    drm_intel_get_aperture_sizes(fd, &mappable_size, &aper_size);
> 
>    return aper_size;
> }

It's somewhat debatable what a unified memory GPU should return for a
"Number of megabytes of video memory available to the renderer" query,
as there really isn't a concept of video RAM.

When Ian implemented this, he chose to pick the amount of memory that
a single batch can reference, which is 3/4 of the aperture.  This may
be too small - applications can certainly use more memory than that.
However, their working set for a draw had better fit within this limit,
or else there will be a performance penalty.  I think it's pretty
reasonable for what applications want.  They're trying to gauge how
high-res their textures can be without incurring penalties.

I know Ian also spoke with a number of game vendors when drafting
and implementing this extension, so I'm inclined to trust his
interpretation.

I don't think exposing GTT_SIZE is useful.  With 48-bit addressing
and PPGTT, the result of that query will be (1ull << 48) aka 256
terabytes.  There is no way in hell that an application can use
that much RAM.  We could restrict it to the total system RAM, but
even then, it would not be performant to try and use all of RAM.

Regardless, this patch fixes an clear bug where we expose a value
of 0 MB for 32-bit applications but 3072 MB for 64-bit applications.
I'd like to fix that before altering the limit that we advertise.

Looking again...libdrm_intel sets bufmgr_gem->gtt_size to
drm_i915_gem_get_aperture::aper_available_size - and uses that field
to -ENOSPC your execbuffers.  drm_intel_get_aperture_sizes, and this
query, use drm_i915_gem_get_aperture::aper_size - which is not quite
the same.  Reading the kernel sources, it looks like aper_available_size
subtracts any pinned memory.  At least in a PPGTT world, that's probably
not materially different given how early we're calling it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170330/76cdc097/attachment-0001.sig>