Re: [Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage

2013-11-13 Thread Eric Anholt
Kristian Høgsberg k...@bitplanet.net writes:

 On Tue, Nov 12, 2013 at 12:15 PM, Eric Anholt e...@anholt.net wrote:
 Kristian Høgsberg k...@bitplanet.net writes:

 Drivers that only call getBuffers to request color buffers can use these
 generic, __DRIimage based helpers to implement the allocBuffer and
 releaseBuffer functions of __DRIdri2Extension.

 For the intel dri driver, this consolidates window system color buffer
 allocation in intel_create_image().

 Signed-off-by: Kristian Høgsberg k...@bitplanet.net
 ---
  src/mesa/drivers/dri/common/dri_util.c   | 75 
 
  src/mesa/drivers/dri/common/dri_util.h   | 10 +
  src/mesa/drivers/dri/i915/intel_screen.c | 56 +---
  src/mesa/drivers/dri/i965/intel_screen.c | 55 +--
  4 files changed, 89 insertions(+), 107 deletions(-)

 diff --git a/src/mesa/drivers/dri/common/dri_util.c 
 b/src/mesa/drivers/dri/common/dri_util.c
 index 86cf24c..a7328e4 100644
 --- a/src/mesa/drivers/dri/common/dri_util.c
 +++ b/src/mesa/drivers/dri/common/dri_util.c
 @@ -115,6 +115,7 @@ driCreateNewScreen2(int scrn, int fd,
  {
  static const __DRIextension *emptyExtensionList[] = { NULL };
  __DRIscreen *psp;
 +int i;

  psp = calloc(1, sizeof(*psp));
  if (!psp)
 @@ -161,6 +162,11 @@ driCreateNewScreen2(int scrn, int fd,
   return NULL;
  }

 +for (i = 0; psp-extensions[i]; i++) {
 +   if (strcmp(psp-extensions[i]-name, __DRI_IMAGE) == 0)
 +  psp-image_extension = (__DRIimageExtension *) 
 psp-extensions[i];
 +}
 +
  int gl_version_override = _mesa_get_gl_version_override();
  if (gl_version_override = 31) {
 psp-max_gl_core_version = MAX2(psp-max_gl_core_version,
 @@ -862,6 +868,75 @@ driImageFormatToGLFormat(uint32_t image_format)
 }
  }

 +struct dri_image_buffer {
 +   __DRIbuffer base;
 +   __DRIimage *image;
 +};
 +
 +__DRIbuffer *
 +driAllocateBuffer(__DRIscreen *screen,
 +  unsigned attachment, unsigned format,
 +  int width, int height)
 +{
 +   struct dri_image_buffer *buffer;
 +   __DRIimageExtension *image = screen-image_extension;
 +   int dri_format, name, stride;
 +
 +   assert(attachment == __DRI_BUFFER_FRONT_LEFT ||
 +  attachment == __DRI_BUFFER_BACK_LEFT);
 +
 +   /* We just need a __DRI_IMAGE_FORMAT code that has a cpp that matches
 +* format / 8.  The image format code is stored in the __DRIimage, but 
 the
 +* __DRIbuffer we create from the image, only stores the cpp. */

 s/, only/ only/

 +
 +   switch (format) {
 +   case 32:
 +  dri_format = __DRI_IMAGE_FORMAT_XRGB;
 +  break;
 +   case 16:
 +  dri_format = __DRI_IMAGE_FORMAT_RGB565;
 +  break;
 +   default:
 +  return NULL;
 +   }
 +
 +   buffer = calloc(1, sizeof *buffer);
 +   if (buffer == NULL)
 +  return NULL;
 +
 +   buffer-image = image-createImage(screen,
 +  width, height, dri_format,
 +  __DRI_IMAGE_USE_SHARE |
 +  __DRI_IMAGE_USE_SCANOUT,
 +  buffer);

 Chad's right about scanout here.

 These helpers are only for drivers that no longer use DRI2 back
 buffers.  This __DRIimage helper can only allocate color buffers and
 they have to be usable for scanout.

 There is no impact on cacheability or tiling of aux buffers, since
 this function will never allocate those.  A given driver knows whether
 or not it will call out to DRI2 getBuffersWithFormat to ask for aux
 buffers or not.  If it allocates its own aux buffers and supports the
 __DRIimage extension, it can plug in these helpers and avoid
 implementing allocateBuffer/releaseBuffer.  If the driver still relies
 on DRI2 getBufferWithFormat for aux buffers, it must implement its own
 allocBuffer/releaseBuffer pair.

You've convinced me.


pgpeMmBS3rEe4.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage

2013-11-12 Thread Eric Anholt
Kristian Høgsberg k...@bitplanet.net writes:

 Drivers that only call getBuffers to request color buffers can use these
 generic, __DRIimage based helpers to implement the allocBuffer and
 releaseBuffer functions of __DRIdri2Extension.

 For the intel dri driver, this consolidates window system color buffer
 allocation in intel_create_image().

 Signed-off-by: Kristian Høgsberg k...@bitplanet.net
 ---
  src/mesa/drivers/dri/common/dri_util.c   | 75 
 
  src/mesa/drivers/dri/common/dri_util.h   | 10 +
  src/mesa/drivers/dri/i915/intel_screen.c | 56 +---
  src/mesa/drivers/dri/i965/intel_screen.c | 55 +--
  4 files changed, 89 insertions(+), 107 deletions(-)

 diff --git a/src/mesa/drivers/dri/common/dri_util.c 
 b/src/mesa/drivers/dri/common/dri_util.c
 index 86cf24c..a7328e4 100644
 --- a/src/mesa/drivers/dri/common/dri_util.c
 +++ b/src/mesa/drivers/dri/common/dri_util.c
 @@ -115,6 +115,7 @@ driCreateNewScreen2(int scrn, int fd,
  {
  static const __DRIextension *emptyExtensionList[] = { NULL };
  __DRIscreen *psp;
 +int i;
  
  psp = calloc(1, sizeof(*psp));
  if (!psp)
 @@ -161,6 +162,11 @@ driCreateNewScreen2(int scrn, int fd,
   return NULL;
  }
  
 +for (i = 0; psp-extensions[i]; i++) {
 +   if (strcmp(psp-extensions[i]-name, __DRI_IMAGE) == 0)
 +  psp-image_extension = (__DRIimageExtension *) psp-extensions[i];
 +}
 +
  int gl_version_override = _mesa_get_gl_version_override();
  if (gl_version_override = 31) {
 psp-max_gl_core_version = MAX2(psp-max_gl_core_version,
 @@ -862,6 +868,75 @@ driImageFormatToGLFormat(uint32_t image_format)
 }
  }
  
 +struct dri_image_buffer {
 +   __DRIbuffer base;
 +   __DRIimage *image;
 +};
 +
 +__DRIbuffer *
 +driAllocateBuffer(__DRIscreen *screen,
 +  unsigned attachment, unsigned format,
 +  int width, int height)
 +{
 +   struct dri_image_buffer *buffer;
 +   __DRIimageExtension *image = screen-image_extension;
 +   int dri_format, name, stride;
 +
 +   assert(attachment == __DRI_BUFFER_FRONT_LEFT ||
 +  attachment == __DRI_BUFFER_BACK_LEFT);
 +
 +   /* We just need a __DRI_IMAGE_FORMAT code that has a cpp that matches
 +* format / 8.  The image format code is stored in the __DRIimage, but the
 +* __DRIbuffer we create from the image, only stores the cpp. */

s/, only/ only/

 +
 +   switch (format) {
 +   case 32:
 +  dri_format = __DRI_IMAGE_FORMAT_XRGB;
 +  break;
 +   case 16:
 +  dri_format = __DRI_IMAGE_FORMAT_RGB565;
 +  break;
 +   default:
 +  return NULL;
 +   }
 +
 +   buffer = calloc(1, sizeof *buffer);
 +   if (buffer == NULL)
 +  return NULL;
 +
 +   buffer-image = image-createImage(screen,
 +  width, height, dri_format,
 +  __DRI_IMAGE_USE_SHARE |
 +  __DRI_IMAGE_USE_SCANOUT,
 +  buffer);

Chad's right about scanout here.

Other than that, this series is:

Reviewed-by: Eric Anholt e...@anholt.net

I don't have a strong opinion on the stable nomination for the later two
patches.  It feels wrong to me to cherry-pick feature-ish work after the
branchpoint, but I'm not the maintainer of the code in question so it's
not my business (and I understand how desirable not generating names for
all your buffers is, from a security perspective).


pgpvemsTpnkGn.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage

2013-11-12 Thread Kristian Høgsberg
On Tue, Nov 12, 2013 at 12:15 PM, Eric Anholt e...@anholt.net wrote:
 Kristian Høgsberg k...@bitplanet.net writes:

 Drivers that only call getBuffers to request color buffers can use these
 generic, __DRIimage based helpers to implement the allocBuffer and
 releaseBuffer functions of __DRIdri2Extension.

 For the intel dri driver, this consolidates window system color buffer
 allocation in intel_create_image().

 Signed-off-by: Kristian Høgsberg k...@bitplanet.net
 ---
  src/mesa/drivers/dri/common/dri_util.c   | 75 
 
  src/mesa/drivers/dri/common/dri_util.h   | 10 +
  src/mesa/drivers/dri/i915/intel_screen.c | 56 +---
  src/mesa/drivers/dri/i965/intel_screen.c | 55 +--
  4 files changed, 89 insertions(+), 107 deletions(-)

 diff --git a/src/mesa/drivers/dri/common/dri_util.c 
 b/src/mesa/drivers/dri/common/dri_util.c
 index 86cf24c..a7328e4 100644
 --- a/src/mesa/drivers/dri/common/dri_util.c
 +++ b/src/mesa/drivers/dri/common/dri_util.c
 @@ -115,6 +115,7 @@ driCreateNewScreen2(int scrn, int fd,
  {
  static const __DRIextension *emptyExtensionList[] = { NULL };
  __DRIscreen *psp;
 +int i;

  psp = calloc(1, sizeof(*psp));
  if (!psp)
 @@ -161,6 +162,11 @@ driCreateNewScreen2(int scrn, int fd,
   return NULL;
  }

 +for (i = 0; psp-extensions[i]; i++) {
 +   if (strcmp(psp-extensions[i]-name, __DRI_IMAGE) == 0)
 +  psp-image_extension = (__DRIimageExtension *) psp-extensions[i];
 +}
 +
  int gl_version_override = _mesa_get_gl_version_override();
  if (gl_version_override = 31) {
 psp-max_gl_core_version = MAX2(psp-max_gl_core_version,
 @@ -862,6 +868,75 @@ driImageFormatToGLFormat(uint32_t image_format)
 }
  }

 +struct dri_image_buffer {
 +   __DRIbuffer base;
 +   __DRIimage *image;
 +};
 +
 +__DRIbuffer *
 +driAllocateBuffer(__DRIscreen *screen,
 +  unsigned attachment, unsigned format,
 +  int width, int height)
 +{
 +   struct dri_image_buffer *buffer;
 +   __DRIimageExtension *image = screen-image_extension;
 +   int dri_format, name, stride;
 +
 +   assert(attachment == __DRI_BUFFER_FRONT_LEFT ||
 +  attachment == __DRI_BUFFER_BACK_LEFT);
 +
 +   /* We just need a __DRI_IMAGE_FORMAT code that has a cpp that matches
 +* format / 8.  The image format code is stored in the __DRIimage, but 
 the
 +* __DRIbuffer we create from the image, only stores the cpp. */

 s/, only/ only/

 +
 +   switch (format) {
 +   case 32:
 +  dri_format = __DRI_IMAGE_FORMAT_XRGB;
 +  break;
 +   case 16:
 +  dri_format = __DRI_IMAGE_FORMAT_RGB565;
 +  break;
 +   default:
 +  return NULL;
 +   }
 +
 +   buffer = calloc(1, sizeof *buffer);
 +   if (buffer == NULL)
 +  return NULL;
 +
 +   buffer-image = image-createImage(screen,
 +  width, height, dri_format,
 +  __DRI_IMAGE_USE_SHARE |
 +  __DRI_IMAGE_USE_SCANOUT,
 +  buffer);

 Chad's right about scanout here.

These helpers are only for drivers that no longer use DRI2 back
buffers.  This __DRIimage helper can only allocate color buffers and
they have to be usable for scanout.

There is no impact on cacheability or tiling of aux buffers, since
this function will never allocate those.  A given driver knows whether
or not it will call out to DRI2 getBuffersWithFormat to ask for aux
buffers or not.  If it allocates its own aux buffers and supports the
__DRIimage extension, it can plug in these helpers and avoid
implementing allocateBuffer/releaseBuffer.  If the driver still relies
on DRI2 getBufferWithFormat for aux buffers, it must implement its own
allocBuffer/releaseBuffer pair.

 Other than that, this series is:

 Reviewed-by: Eric Anholt e...@anholt.net

 I don't have a strong opinion on the stable nomination for the later two
 patches.  It feels wrong to me to cherry-pick feature-ish work after the
 branchpoint, but I'm not the maintainer of the code in question so it's
 not my business (and I understand how desirable not generating names for
 all your buffers is, from a security perspective).

I would also prefer not to pick these feature-ish patches back to the
10 branch, but there are a few caveats: the patches depend on the
__DRIimageLoaderExtension, but as we branched for 10 just as those
patches landed there wasn't any time to get these in before branching.
 We're also still very close the the branch point, so we'll still have
some time between pushing the patches and the release. Finally, with
DRI3 seemingly impossible to build, the GBM and Wayland EGL platforms
are the best ways to test the new DRI driver changes we're shipping in
10.

Kristian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage

2013-11-11 Thread Chad Versace

On 11/11/2013 01:22 PM, Kristian Høgsberg wrote:

Drivers that only call getBuffers to request color buffers can use these
generic, __DRIimage based helpers to implement the allocBuffer and
releaseBuffer functions of __DRIdri2Extension.

For the intel dri driver, this consolidates window system color buffer
allocation in intel_create_image().

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
  src/mesa/drivers/dri/common/dri_util.c   | 75 
  src/mesa/drivers/dri/common/dri_util.h   | 10 +
  src/mesa/drivers/dri/i915/intel_screen.c | 56 +---
  src/mesa/drivers/dri/i965/intel_screen.c | 55 +--
  4 files changed, 89 insertions(+), 107 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index 86cf24c..a7328e4 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c




+__DRIbuffer *
+driAllocateBuffer(__DRIscreen *screen,
+  unsigned attachment, unsigned format,
+  int width, int height)
+{
+   struct dri_image_buffer *buffer;
+   __DRIimageExtension *image = screen-image_extension;
+   int dri_format, name, stride;
+
+   assert(attachment == __DRI_BUFFER_FRONT_LEFT ||
+  attachment == __DRI_BUFFER_BACK_LEFT);
+
+   /* We just need a __DRI_IMAGE_FORMAT code that has a cpp that matches
+* format / 8.  The image format code is stored in the __DRIimage, but the
+* __DRIbuffer we create from the image, only stores the cpp. */
+
+   switch (format) {
+   case 32:
+  dri_format = __DRI_IMAGE_FORMAT_XRGB;
+  break;
+   case 16:
+  dri_format = __DRI_IMAGE_FORMAT_RGB565;
+  break;
+   default:
+  return NULL;
+   }
+
+   buffer = calloc(1, sizeof *buffer);
+   if (buffer == NULL)
+  return NULL;
+
+   buffer-image = image-createImage(screen,
+  width, height, dri_format,
+  __DRI_IMAGE_USE_SHARE |
+  __DRI_IMAGE_USE_SCANOUT,
+  buffer);


It's incorrect to specify __DRI_IMAGE_USE_SCANOUT regardless of
attachment type. GBM, Wayland, and Android use driAllocateBuffer
to allocate more than just the scanout buffer. They use it to
allocate auxillary buffers too. If i965 were to respect the
caching restrictions implied by __DRI_IMAGE_USE_SCANOUT, its
use for aux buffers would hurt performance. (As far as I can
tell, though, intel_create_image() wrongly ignores __DRI_IMAGE_USE_SCANOUT).

Instead, I think you should set the USE_SCANOUT bit if and only if
attachment is one of __DRI_BUFFER_(BACK|FRONT)_(LEFT|RIGHT).


+
+   if (buffer-image == NULL) {
+  free(buffer);
+  return NULL;
+   }
+
+   image-queryImage(buffer-image, __DRI_IMAGE_ATTRIB_NAME, name);
+   image-queryImage(buffer-image, __DRI_IMAGE_ATTRIB_STRIDE, stride);
+
+   buffer-base.attachment = attachment;
+   buffer-base.name = name;
+   buffer-base.pitch = stride;
+   buffer-base.cpp = format / 8;
+
+   return buffer-base;
+}


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage

2013-11-11 Thread Kristian Høgsberg
On Mon, Nov 11, 2013 at 1:57 PM, Chad Versace
chad.vers...@linux.intel.com wrote:
 On 11/11/2013 01:22 PM, Kristian Høgsberg wrote:

 Drivers that only call getBuffers to request color buffers can use these
 generic, __DRIimage based helpers to implement the allocBuffer and
 releaseBuffer functions of __DRIdri2Extension.

 For the intel dri driver, this consolidates window system color buffer
 allocation in intel_create_image().

 Signed-off-by: Kristian Høgsberg k...@bitplanet.net
 ---
   src/mesa/drivers/dri/common/dri_util.c   | 75
 
   src/mesa/drivers/dri/common/dri_util.h   | 10 +
   src/mesa/drivers/dri/i915/intel_screen.c | 56 +---
   src/mesa/drivers/dri/i965/intel_screen.c | 55 +--
   4 files changed, 89 insertions(+), 107 deletions(-)

 diff --git a/src/mesa/drivers/dri/common/dri_util.c
 b/src/mesa/drivers/dri/common/dri_util.c
 index 86cf24c..a7328e4 100644
 --- a/src/mesa/drivers/dri/common/dri_util.c
 +++ b/src/mesa/drivers/dri/common/dri_util.c



 +__DRIbuffer *
 +driAllocateBuffer(__DRIscreen *screen,
 +  unsigned attachment, unsigned format,
 +  int width, int height)
 +{
 +   struct dri_image_buffer *buffer;
 +   __DRIimageExtension *image = screen-image_extension;
 +   int dri_format, name, stride;
 +
 +   assert(attachment == __DRI_BUFFER_FRONT_LEFT ||
 +  attachment == __DRI_BUFFER_BACK_LEFT);
 +
 +   /* We just need a __DRI_IMAGE_FORMAT code that has a cpp that matches
 +* format / 8.  The image format code is stored in the __DRIimage, but
 the
 +* __DRIbuffer we create from the image, only stores the cpp. */
 +
 +   switch (format) {
 +   case 32:
 +  dri_format = __DRI_IMAGE_FORMAT_XRGB;
 +  break;
 +   case 16:
 +  dri_format = __DRI_IMAGE_FORMAT_RGB565;
 +  break;
 +   default:
 +  return NULL;
 +   }
 +
 +   buffer = calloc(1, sizeof *buffer);
 +   if (buffer == NULL)
 +  return NULL;
 +
 +   buffer-image = image-createImage(screen,
 +  width, height, dri_format,
 +  __DRI_IMAGE_USE_SHARE |
 +  __DRI_IMAGE_USE_SCANOUT,
 +  buffer);


 It's incorrect to specify __DRI_IMAGE_USE_SCANOUT regardless of
 attachment type. GBM, Wayland, and Android use driAllocateBuffer
 to allocate more than just the scanout buffer. They use it to
 allocate auxillary buffers too. If i965 were to respect the
 caching restrictions implied by __DRI_IMAGE_USE_SCANOUT, its
 use for aux buffers would hurt performance. (As far as I can
 tell, though, intel_create_image() wrongly ignores __DRI_IMAGE_USE_SCANOUT).

 Instead, I think you should set the USE_SCANOUT bit if and only if
 attachment is one of __DRI_BUFFER_(BACK|FRONT)_(LEFT|RIGHT).

The commit message says:

Drivers that only call getBuffers to request color buffers can use these
generic, __DRIimage based helpers to implement the allocBuffer and
releaseBuffer functions of __DRIdri2Extension.

which is true for the Intel DRI driver.  The DRI2 interface allows the
driver to ask for auxillary buffers, but since we stopped supporting
multiple processes rendering to the same X window, our driver no
longer does that.  This is under driver control and thus if a driver
knows it will never ask for aux buffers, it can use these helpers.

Kristian

 +
 +   if (buffer-image == NULL) {
 +  free(buffer);
 +  return NULL;
 +   }
 +
 +   image-queryImage(buffer-image, __DRI_IMAGE_ATTRIB_NAME, name);
 +   image-queryImage(buffer-image, __DRI_IMAGE_ATTRIB_STRIDE, stride);
 +
 +   buffer-base.attachment = attachment;
 +   buffer-base.name = name;
 +   buffer-base.pitch = stride;
 +   buffer-base.cpp = format / 8;
 +
 +   return buffer-base;
 +}


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage

2013-11-11 Thread Chad Versace

On 11/11/2013 02:20 PM, Kristian Høgsberg wrote:

On Mon, Nov 11, 2013 at 1:57 PM, Chad Versace
chad.vers...@linux.intel.com wrote:

On 11/11/2013 01:22 PM, Kristian Høgsberg wrote:


Drivers that only call getBuffers to request color buffers can use these
generic, __DRIimage based helpers to implement the allocBuffer and
releaseBuffer functions of __DRIdri2Extension.

For the intel dri driver, this consolidates window system color buffer
allocation in intel_create_image().

Signed-off-by: Kristian Høgsberg k...@bitplanet.net
---
   src/mesa/drivers/dri/common/dri_util.c   | 75

   src/mesa/drivers/dri/common/dri_util.h   | 10 +
   src/mesa/drivers/dri/i915/intel_screen.c | 56 +---
   src/mesa/drivers/dri/i965/intel_screen.c | 55 +--
   4 files changed, 89 insertions(+), 107 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c
b/src/mesa/drivers/dri/common/dri_util.c
index 86cf24c..a7328e4 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c





+__DRIbuffer *
+driAllocateBuffer(__DRIscreen *screen,
+  unsigned attachment, unsigned format,
+  int width, int height)
+{
+   struct dri_image_buffer *buffer;
+   __DRIimageExtension *image = screen-image_extension;
+   int dri_format, name, stride;
+
+   assert(attachment == __DRI_BUFFER_FRONT_LEFT ||
+  attachment == __DRI_BUFFER_BACK_LEFT);
+
+   /* We just need a __DRI_IMAGE_FORMAT code that has a cpp that matches
+* format / 8.  The image format code is stored in the __DRIimage, but
the
+* __DRIbuffer we create from the image, only stores the cpp. */
+
+   switch (format) {
+   case 32:
+  dri_format = __DRI_IMAGE_FORMAT_XRGB;
+  break;
+   case 16:
+  dri_format = __DRI_IMAGE_FORMAT_RGB565;
+  break;
+   default:
+  return NULL;
+   }
+
+   buffer = calloc(1, sizeof *buffer);
+   if (buffer == NULL)
+  return NULL;
+
+   buffer-image = image-createImage(screen,
+  width, height, dri_format,
+  __DRI_IMAGE_USE_SHARE |
+  __DRI_IMAGE_USE_SCANOUT,
+  buffer);



It's incorrect to specify __DRI_IMAGE_USE_SCANOUT regardless of
attachment type. GBM, Wayland, and Android use driAllocateBuffer
to allocate more than just the scanout buffer. They use it to
allocate auxillary buffers too. If i965 were to respect the
caching restrictions implied by __DRI_IMAGE_USE_SCANOUT, its
use for aux buffers would hurt performance. (As far as I can
tell, though, intel_create_image() wrongly ignores __DRI_IMAGE_USE_SCANOUT).

Instead, I think you should set the USE_SCANOUT bit if and only if
attachment is one of __DRI_BUFFER_(BACK|FRONT)_(LEFT|RIGHT).


The commit message says:

Drivers that only call getBuffers to request color buffers can use these
generic, __DRIimage based helpers to implement the allocBuffer and
releaseBuffer functions of __DRIdri2Extension.

which is true for the Intel DRI driver.  The DRI2 interface allows the
driver to ask for auxillary buffers, but since we stopped supporting
multiple processes rendering to the same X window, our driver no
longer does that.  This is under driver control and thus if a driver
knows it will never ask for aux buffers, it can use these helpers.

Kristian


Ah, thanks for correcting me. I also solved the questions I had regarding
patch 3, so this series is
Reviewed-by: Chad Versace chad.vers...@linux.intel.com

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev