[Mesa-dev] [PATCH 07/18] i915: Wire up initial support for DRI_RENDERER_QUERY extension

[Daniel Vetter]

On Mon, Nov 11, 2013 at 11:03:49AM -0800, Ian Romanick wrote:
> On 11/09/2013 02:44 AM, Daniel Vetter wrote:
> > On Fri, Oct 11, 2013 at 03:10:14PM -0700, Ian Romanick wrote:
> >> From: Ian Romanick <ian.d.romanick at intel.com>
> >>
> >> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> >> ---
> >>  src/mesa/drivers/dri/i915/intel_screen.c | 79 ++++++++++++++++++++++++++++++++
> >>  1 file changed, 79 insertions(+)
> >>
> >> diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
> >> index 4f8c342..fa4fdc0 100644
> >> --- a/src/mesa/drivers/dri/i915/intel_screen.c
> >> +++ b/src/mesa/drivers/dri/i915/intel_screen.c
> >> @@ -27,6 +27,7 @@
> >>  
> >>  #include <errno.h>
> >>  #include <time.h>
> >> +#include <sys/sysinfo.h>
> >>  #include "main/glheader.h"
> >>  #include "main/context.h"
> >>  #include "main/framebuffer.h"
> >> @@ -741,6 +742,84 @@ static struct __DRIimageExtensionRec intelImageExtension = {
> >>      .createImageFromFds                 = intel_create_image_from_fds
> >>  };
> >>  
> >> +static int
> >> +i915_query_renderer_integer(__DRIscreen *psp, int param, int *value)
> >> +{
> >> +   const struct intel_screen *const intelScreen =
> >> +      (struct intel_screen *) psp->driverPrivate;
> >> +
> >> +   switch (param) {
> >> +   case __DRI2_RENDERER_VENDOR_ID:
> >> +      value[0] = 0x8086;
> >> +      return 0;
> >> +   case __DRI2_RENDERER_DEVICE_ID:
> >> +      value[0] = intelScreen->deviceID;
> >> +      return 0;
> >> +   case __DRI2_RENDERER_ACCELERATED:
> >> +      value[0] = 1;
> >> +      return 0;
> >> +   case __DRI2_RENDERER_VIDEO_MEMORY: {
> >> +      struct sysinfo info;
> >> +      uint64_t system_memory_bytes;
> >> +      unsigned system_memory_megabytes;
> >> +
> >> +      /* Once a batch uses more than 75% of the maximum mappable size, we
> >> +       * assume that there's some fragmentation, and we start doing extra
> >> +       * flushing, etc.  That's the big cliff apps will care about.
> >> +       */
> >> +      const unsigned long agp_bytes = drmAgpSize(psp->fd);
> > 
> > So despite me shooting at this in the next patch saying that this is
> > - the wrong interface, it doesn't actually really tell you what you want
> >   to know (since it fails to take pinnned crap into account),
> > - doesn't work on half the platforms i915_dri supports already,
> > - and is massively deprecated on all others and a major pain for us to
> >   keep on live support in the kernel
> 
> In fairness, you missed this specific issue on your first review and
> shot at it after I committed it. :(  There was no malice... just
> timezone fail.  In the future, I'll CC you any Mesa changes that
> interact with the kernel so that you'll notice them sooner.

Yeah, I try to read most of mesa-devel, but can't really go into details
of the patches everywhere. I only replied to the i965 patch since Ken's
review made me curious to check the details. Then your reply later on made
me check the previous patch.

Anyway, I've cooled off now, just happy that we've caught this zoombie in
the nick of time ;-)

Another thing I've noticed is that you adjust the advertised vram size
with the available memory. I don't think that's an issue already since the
aperture space checker in libdrm (which mesa uses to avoid overfilling
batches) doesn't do that. But it'll be one on memory constrained
phones/tablets and also with the much bigger gtt on bdw.

I think the right fix for that is to adjust the
aperture.aper_available_size in the kernel, so that all users of this
interface have correct data. The kernel already adjusts this for pinned
objects and similar stuff, so would fit neatly. I'll wip up a kernel patch
for that. I guess we could leave the current stuff in for 10.0 and remove
it in master once the kernel fix has landed.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch