| Message ID | 1383618208-21310-4-git-send-email-keithp@keithp.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Mon, Nov 04, 2013 at 06:23:23PM -0800, Keith Packard wrote: > Instead of assuming that the size will be height * pitch, have the caller pass > in the size explicitly. > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > src/mesa/drivers/dri/i915/intel_regions.c | 4 ++-- > src/mesa/drivers/dri/i915/intel_regions.h | 2 +- > src/mesa/drivers/dri/i915/intel_screen.c | 2 +- > src/mesa/drivers/dri/i965/intel_regions.c | 4 ++-- > src/mesa/drivers/dri/i965/intel_regions.h | 1 + > src/mesa/drivers/dri/i965/intel_screen.c | 2 +- > 6 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/src/mesa/drivers/dri/i915/intel_regions.c b/src/mesa/drivers/dri/i915/intel_regions.c > index 44f7030..9f5b89e 100644 > --- a/src/mesa/drivers/dri/i915/intel_regions.c > +++ b/src/mesa/drivers/dri/i915/intel_regions.c > @@ -209,6 +209,7 @@ struct intel_region * > intel_region_alloc_for_fd(struct intel_screen *screen, > GLuint cpp, > GLuint width, GLuint height, GLuint pitch, > + GLuint size, > int fd, const char *name) > { > struct intel_region *region; > @@ -216,8 +217,7 @@ intel_region_alloc_for_fd(struct intel_screen *screen, > int ret; > uint32_t bit_6_swizzle, tiling; > > - buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, > - fd, height * pitch); > + buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size); The 3.12 kernel let's you get the bo size from lseek on the dma_buf fd. I added libdrm support for getting the size that, and if that works, it overrides the user provided size: http://cgit.freedesktop.org/mesa/drm/commit/?id=9c52c3dc4763336884277d8005eac7e6efb77600 3.12 is the first kernel where dma_buf fd passing works reliably anyway and the first kernel with render-nodes, so it's not worth the trouble to try to make this work for older kernels. Regardless, this patchs looks good. Reviewed-by: Kristian Høgsberg <krh@bitplanet.net> > if (buffer == NULL) > return NULL; > ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle); > diff --git a/src/mesa/drivers/dri/i915/intel_regions.h b/src/mesa/drivers/dri/i915/intel_regions.h > index 5c612a9..6bc4a42 100644 > --- a/src/mesa/drivers/dri/i915/intel_regions.h > +++ b/src/mesa/drivers/dri/i915/intel_regions.h > @@ -91,7 +91,7 @@ struct intel_region * > intel_region_alloc_for_fd(struct intel_screen *screen, > GLuint cpp, > GLuint width, GLuint height, GLuint pitch, > - int fd, const char *name); > + GLuint size, int fd, const char *name); > > bool > intel_region_flink(struct intel_region *region, uint32_t *name); > diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c > index 3f54752..085e894 100644 > --- a/src/mesa/drivers/dri/i915/intel_screen.c > +++ b/src/mesa/drivers/dri/i915/intel_screen.c > @@ -653,7 +653,7 @@ intel_create_image_from_fds(__DRIscreen *screen, > return NULL; > > image->region = intel_region_alloc_for_fd(intelScreen, > - 1, width, height, > + 1, width, height, height * strides[0], > strides[0], fds[0], "image"); > if (image->region == NULL) { > free(image); > diff --git a/src/mesa/drivers/dri/i965/intel_regions.c b/src/mesa/drivers/dri/i965/intel_regions.c > index a6b80fd..3920f4f 100644 > --- a/src/mesa/drivers/dri/i965/intel_regions.c > +++ b/src/mesa/drivers/dri/i965/intel_regions.c > @@ -209,6 +209,7 @@ struct intel_region * > intel_region_alloc_for_fd(struct intel_screen *screen, > GLuint cpp, > GLuint width, GLuint height, GLuint pitch, > + GLuint size, > int fd, const char *name) > { > struct intel_region *region; > @@ -216,8 +217,7 @@ intel_region_alloc_for_fd(struct intel_screen *screen, > int ret; > uint32_t bit_6_swizzle, tiling; > > - buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, > - fd, height * pitch); > + buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size); > if (buffer == NULL) > return NULL; > ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle); > diff --git a/src/mesa/drivers/dri/i965/intel_regions.h b/src/mesa/drivers/dri/i965/intel_regions.h > index f08a113..05dfef3 100644 > --- a/src/mesa/drivers/dri/i965/intel_regions.h > +++ b/src/mesa/drivers/dri/i965/intel_regions.h > @@ -92,6 +92,7 @@ struct intel_region * > intel_region_alloc_for_fd(struct intel_screen *screen, > GLuint cpp, > GLuint width, GLuint height, GLuint pitch, > + GLuint size, > int fd, const char *name); > > bool > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c > index ce8124b..b89b1a5 100644 > --- a/src/mesa/drivers/dri/i965/intel_screen.c > +++ b/src/mesa/drivers/dri/i965/intel_screen.c > @@ -718,7 +718,7 @@ intel_create_image_from_fds(__DRIscreen *screen, > return NULL; > > image->region = intel_region_alloc_for_fd(intelScreen, > - 1, width, height, > + 1, width, height, height * strides[0], > strides[0], fds[0], "image"); > if (image->region == NULL) { > free(image); > -- > 1.8.4.2 >
Kristian Høgsberg <hoegsberg@gmail.com> writes: > The 3.12 kernel let's you get the bo size from lseek on the dma_buf > fd. I added libdrm support for getting the size that, and if that > works, it overrides the user provided size: Yeah, I'm happy just sending the size and not trusting that the kernel will have the new API; back-porting happens... > Regardless, this patchs looks good. > > Reviewed-by: Kristian Høgsberg <krh@bitplanet.net> Thanks!
On Tue, 2013-11-05 at 14:23 -0800, Kristian Høgsberg wrote: > On Mon, Nov 04, 2013 at 06:23:23PM -0800, Keith Packard wrote: > > Instead of assuming that the size will be height * pitch, have the caller pass > > in the size explicitly. > > > > Signed-off-by: Keith Packard <keithp@keithp.com> > > --- > > src/mesa/drivers/dri/i915/intel_regions.c | 4 ++-- > > src/mesa/drivers/dri/i915/intel_regions.h | 2 +- > > src/mesa/drivers/dri/i915/intel_screen.c | 2 +- > > src/mesa/drivers/dri/i965/intel_regions.c | 4 ++-- > > src/mesa/drivers/dri/i965/intel_regions.h | 1 + > > src/mesa/drivers/dri/i965/intel_screen.c | 2 +- > > 6 files changed, 8 insertions(+), 7 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i915/intel_regions.c b/src/mesa/drivers/dri/i915/intel_regions.c > > index 44f7030..9f5b89e 100644 > > --- a/src/mesa/drivers/dri/i915/intel_regions.c > > +++ b/src/mesa/drivers/dri/i915/intel_regions.c > > @@ -209,6 +209,7 @@ struct intel_region * > > intel_region_alloc_for_fd(struct intel_screen *screen, > > GLuint cpp, > > GLuint width, GLuint height, GLuint pitch, > > + GLuint size, > > int fd, const char *name) > > { > > struct intel_region *region; > > @@ -216,8 +217,7 @@ intel_region_alloc_for_fd(struct intel_screen *screen, > > int ret; > > uint32_t bit_6_swizzle, tiling; > > > > - buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, > > - fd, height * pitch); > > + buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size); > > The 3.12 kernel let's you get the bo size from lseek on the dma_buf > fd. I added libdrm support for getting the size that, and if that > works, it overrides the user provided size: > > http://cgit.freedesktop.org/mesa/drm/commit/?id=9c52c3dc4763336884277d8005eac7e6efb77600 > > 3.12 is the first kernel where dma_buf fd passing works reliably > anyway and the first kernel with render-nodes, so it's not worth the > trouble to try to make this work for older kernels. > > Regardless, this patchs looks good. > > Reviewed-by: Kristian Høgsberg <krh@bitplanet.net> > > > if (buffer == NULL) > > return NULL; > > ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle); > > diff --git a/src/mesa/drivers/dri/i915/intel_regions.h b/src/mesa/drivers/dri/i915/intel_regions.h > > index 5c612a9..6bc4a42 100644 > > --- a/src/mesa/drivers/dri/i915/intel_regions.h > > +++ b/src/mesa/drivers/dri/i915/intel_regions.h > > @@ -91,7 +91,7 @@ struct intel_region * > > intel_region_alloc_for_fd(struct intel_screen *screen, > > GLuint cpp, > > GLuint width, GLuint height, GLuint pitch, > > - int fd, const char *name); > > + GLuint size, int fd, const char *name); > > > > bool > > intel_region_flink(struct intel_region *region, uint32_t *name); > > diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c > > index 3f54752..085e894 100644 > > --- a/src/mesa/drivers/dri/i915/intel_screen.c > > +++ b/src/mesa/drivers/dri/i915/intel_screen.c > > @@ -653,7 +653,7 @@ intel_create_image_from_fds(__DRIscreen *screen, > > return NULL; > > > > image->region = intel_region_alloc_for_fd(intelScreen, > > - 1, width, height, > > + 1, width, height, height * strides[0], > > strides[0], fds[0], "image"); You've presumably noticed this already, but this is the wrong way round - you're passing height * stride as pitch, and stride as size. This makes for awesome rendering. > > if (image->region == NULL) { > > free(image); > > diff --git a/src/mesa/drivers/dri/i965/intel_regions.c b/src/mesa/drivers/dri/i965/intel_regions.c > > index a6b80fd..3920f4f 100644 > > --- a/src/mesa/drivers/dri/i965/intel_regions.c > > +++ b/src/mesa/drivers/dri/i965/intel_regions.c > > @@ -209,6 +209,7 @@ struct intel_region * > > intel_region_alloc_for_fd(struct intel_screen *screen, > > GLuint cpp, > > GLuint width, GLuint height, GLuint pitch, > > + GLuint size, > > int fd, const char *name) > > { > > struct intel_region *region; > > @@ -216,8 +217,7 @@ intel_region_alloc_for_fd(struct intel_screen *screen, > > int ret; > > uint32_t bit_6_swizzle, tiling; > > > > - buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, > > - fd, height * pitch); > > + buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size); > > if (buffer == NULL) > > return NULL; > > ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle); > > diff --git a/src/mesa/drivers/dri/i965/intel_regions.h b/src/mesa/drivers/dri/i965/intel_regions.h > > index f08a113..05dfef3 100644 > > --- a/src/mesa/drivers/dri/i965/intel_regions.h > > +++ b/src/mesa/drivers/dri/i965/intel_regions.h > > @@ -92,6 +92,7 @@ struct intel_region * > > intel_region_alloc_for_fd(struct intel_screen *screen, > > GLuint cpp, > > GLuint width, GLuint height, GLuint pitch, > > + GLuint size, > > int fd, const char *name); > > > > bool > > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c > > index ce8124b..b89b1a5 100644 > > --- a/src/mesa/drivers/dri/i965/intel_screen.c > > +++ b/src/mesa/drivers/dri/i965/intel_screen.c > > @@ -718,7 +718,7 @@ intel_create_image_from_fds(__DRIscreen *screen, > > return NULL; > > > > image->region = intel_region_alloc_for_fd(intelScreen, > > - 1, width, height, > > + 1, width, height, height * strides[0], > > strides[0], fds[0], "image"); > > if (image->region == NULL) { > > free(image); > > -- > > 1.8.4.2 > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/src/mesa/drivers/dri/i915/intel_regions.c b/src/mesa/drivers/dri/i915/intel_regions.c index 44f7030..9f5b89e 100644 --- a/src/mesa/drivers/dri/i915/intel_regions.c +++ b/src/mesa/drivers/dri/i915/intel_regions.c @@ -209,6 +209,7 @@ struct intel_region * intel_region_alloc_for_fd(struct intel_screen *screen, GLuint cpp, GLuint width, GLuint height, GLuint pitch, + GLuint size, int fd, const char *name) { struct intel_region *region; @@ -216,8 +217,7 @@ intel_region_alloc_for_fd(struct intel_screen *screen, int ret; uint32_t bit_6_swizzle, tiling; - buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, - fd, height * pitch); + buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size); if (buffer == NULL) return NULL; ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle); diff --git a/src/mesa/drivers/dri/i915/intel_regions.h b/src/mesa/drivers/dri/i915/intel_regions.h index 5c612a9..6bc4a42 100644 --- a/src/mesa/drivers/dri/i915/intel_regions.h +++ b/src/mesa/drivers/dri/i915/intel_regions.h @@ -91,7 +91,7 @@ struct intel_region * intel_region_alloc_for_fd(struct intel_screen *screen, GLuint cpp, GLuint width, GLuint height, GLuint pitch, - int fd, const char *name); + GLuint size, int fd, const char *name); bool intel_region_flink(struct intel_region *region, uint32_t *name); diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c index 3f54752..085e894 100644 --- a/src/mesa/drivers/dri/i915/intel_screen.c +++ b/src/mesa/drivers/dri/i915/intel_screen.c @@ -653,7 +653,7 @@ intel_create_image_from_fds(__DRIscreen *screen, return NULL; image->region = intel_region_alloc_for_fd(intelScreen, - 1, width, height, + 1, width, height, height * strides[0], strides[0], fds[0], "image"); if (image->region == NULL) { free(image); diff --git a/src/mesa/drivers/dri/i965/intel_regions.c b/src/mesa/drivers/dri/i965/intel_regions.c index a6b80fd..3920f4f 100644 --- a/src/mesa/drivers/dri/i965/intel_regions.c +++ b/src/mesa/drivers/dri/i965/intel_regions.c @@ -209,6 +209,7 @@ struct intel_region * intel_region_alloc_for_fd(struct intel_screen *screen, GLuint cpp, GLuint width, GLuint height, GLuint pitch, + GLuint size, int fd, const char *name) { struct intel_region *region; @@ -216,8 +217,7 @@ intel_region_alloc_for_fd(struct intel_screen *screen, int ret; uint32_t bit_6_swizzle, tiling; - buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, - fd, height * pitch); + buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size); if (buffer == NULL) return NULL; ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle); diff --git a/src/mesa/drivers/dri/i965/intel_regions.h b/src/mesa/drivers/dri/i965/intel_regions.h index f08a113..05dfef3 100644 --- a/src/mesa/drivers/dri/i965/intel_regions.h +++ b/src/mesa/drivers/dri/i965/intel_regions.h @@ -92,6 +92,7 @@ struct intel_region * intel_region_alloc_for_fd(struct intel_screen *screen, GLuint cpp, GLuint width, GLuint height, GLuint pitch, + GLuint size, int fd, const char *name); bool diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index ce8124b..b89b1a5 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -718,7 +718,7 @@ intel_create_image_from_fds(__DRIscreen *screen, return NULL; image->region = intel_region_alloc_for_fd(intelScreen, - 1, width, height, + 1, width, height, height * strides[0], strides[0], fds[0], "image"); if (image->region == NULL) { free(image);
Instead of assuming that the size will be height * pitch, have the caller pass in the size explicitly. Signed-off-by: Keith Packard <keithp@keithp.com> --- src/mesa/drivers/dri/i915/intel_regions.c | 4 ++-- src/mesa/drivers/dri/i915/intel_regions.h | 2 +- src/mesa/drivers/dri/i915/intel_screen.c | 2 +- src/mesa/drivers/dri/i965/intel_regions.c | 4 ++-- src/mesa/drivers/dri/i965/intel_regions.h | 1 + src/mesa/drivers/dri/i965/intel_screen.c | 2 +- 6 files changed, 8 insertions(+), 7 deletions(-)