Re: [Mesa3d-dev] [PATCH 3/6] dri/intel: Add explicit size parameter to intel_region_alloc_for_fd

[Ian R. <id...@fr...>]

On 10/31/2013 04:13 PM, 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 <ke...@ke...>

One nit below.  With that changed,

Reviewed-by: Ian Romanick <ian...@in...>

> ---
>  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);
>     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,

Since this isn't exposed to the GL API, use either unsigned or size_t.

>                            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 eafafa2..0bd0789 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -712,7 +712,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);
>