Re: [Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage
Kristian Høgsberg writes: > On Tue, Nov 12, 2013 at 12:15 PM, Eric Anholt wrote: >> Kristian Høgsberg 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 >>> --- >>> 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
On Tue, Nov 12, 2013 at 12:15 PM, Eric Anholt wrote: > Kristian Høgsberg 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 >> --- >> 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 > > 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
Re: [Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage
Kristian Høgsberg 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 > --- > 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 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
On 11/11/2013 02:20 PM, Kristian Høgsberg wrote: On Mon, Nov 11, 2013 at 1:57 PM, Chad Versace 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 --- 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 ___ 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
On Mon, Nov 11, 2013 at 1:57 PM, Chad Versace 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 >> --- >> 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
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 --- 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
[Mesa-dev] [PATCH 1/3] dri: Add helpers for implementing allocBuffer/releaseBuffer with __DRIimage
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 --- 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. */ + + 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); + + 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; +} + +void +driReleaseBuffer(__DRIscreen *screen, __DRIbuffer *_buffer) +{ + struct dri_image_buffer *buffer = (struct dri_image_buffer *) _buffer; + __DRIimageExtension *image = screen->image_extension; + + image->destroyImage(buffer->image); + free(buffer); +} + + /** Image driver interface */ const __DRIimageDriverExtension driImageDriverExtension = { .base = { __DRI_IMAGE_DRIVER, __DRI_IMAGE_DRIVER_VERSION }, diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h index 79a8564..240213d 100644 --- a/src/mesa/drivers/dri/common/dri_util.h +++ b/src/mesa/drivers/dri/common/dri_util.h @@ -165,6 +165,7 @@ struct __DRIscreenRec { int max_gl_es2_version; const __DRIextension **extensions; + __DRIimageExtension *image_extension; const __DRIswrastLoaderExtension *swrast_loader; @@ -291,4 +292,13 @@ driUpdateFramebufferSize(struct gl_context *ctx, const __DRIdrawable *dPriv); extern const __DRIimageDriverExtension driImageDriverExtension; +extern __DRIbuffer * +driAllocateBuffer(__DRIscreen *screen, + unsigned attachment, unsigned format, + int width, int height); + +extern void +driReleaseBuffer(__DRIscreen *screen, __DRIbuffer *_buffer); + + #endif /* _DRI_UTIL_H_ */ diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c index 2c309ed..a143652 100644 --- a/src/mesa/drivers/dri/i915/intel_screen.c +++ b/src/mesa/drivers/dri/i915/intel_screen.c @@ -1185,58 +1185,6 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp) return (const __DRIconfig**) intel_screen_make_configs(psp); } -struct intel_buffer {