Re: [PATCH 3/8] dri/intel: Add explicit size parameter to intel_region_alloc_for_fd

["Kristian Høgsberg" <hoegsberg@gmail.com>]

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
>