OSDN Git Service

minigbm: work with strides, not widths
authorGurchetan Singh <gurchetansingh@chromium.org>
Wed, 29 Mar 2017 15:23:40 +0000 (08:23 -0700)
committerchrome-bot <chrome-bot@chromium.org>
Fri, 31 Mar 2017 04:21:54 +0000 (21:21 -0700)
When doing alignment, many times we get the bytes per pixel, try
to align to a certain boundary, and then pass in the "aligned width"
to drv_bo_from_format. This back and forth is confusing and error
prone. Let's change our drivers and helpers to eliminate this, and
try to work in strides as soon as possible.

Additionally, let's make drv_bpp_from_format a static function. This
is because bits per pixel is ill-defined for YUV formats, and we should
work in strides whenever possible.

BUG=none
TEST=graphics_Gbm runs successfully on Cyan, ui boots

Change-Id: I207ce6fd5eaac472b7b82f0d952b46697e325498
Reviewed-on: https://chromium-review.googlesource.com/462479
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>
amdgpu.c
cros_gralloc/cros_alloc_device.cc
drv.c
drv.h
helpers.c
helpers.h
i915.c
mediatek.c
rockchip.c
vc4.c

index 8e3c778..59242e9 100644 (file)
--- a/amdgpu.c
+++ b/amdgpu.c
@@ -148,6 +148,7 @@ static int amdgpu_addrlib_compute(void *addrlib, uint32_t width,
        ADDR_COMPUTE_SURFACE_INFO_INPUT addr_surf_info_in = {0};
        ADDR_TILEINFO addr_tile_info = {0};
        ADDR_TILEINFO addr_tile_info_out = {0};
+       uint32_t bits_per_pixel;
 
        addr_surf_info_in.size = sizeof(ADDR_COMPUTE_SURFACE_INFO_INPUT);
 
@@ -159,8 +160,9 @@ static int amdgpu_addrlib_compute(void *addrlib, uint32_t width,
        else if (width <= 16 || height <= 16)
                addr_surf_info_in.tileMode = ADDR_TM_1D_TILED_THIN1;
 
+       bits_per_pixel = drv_stride_from_format(format, 1, 0) * 8;
        /* Bits per pixel should be calculated from format*/
-       addr_surf_info_in.bpp = drv_bpp_from_format(format, 0);
+       addr_surf_info_in.bpp = bits_per_pixel;
        addr_surf_info_in.numSamples = 1;
        addr_surf_info_in.width = width;
        addr_surf_info_in.height = height;
index 2281fa0..86d5e34 100644 (file)
@@ -91,12 +91,10 @@ static struct cros_gralloc_handle *cros_gralloc_handle_from_bo(struct bo *bo)
        hnd->width = drv_bo_get_width(bo);
        hnd->height = drv_bo_get_height(bo);
        hnd->format = drv_bo_get_format(bo);
+       hnd->pixel_stride = drv_bo_get_stride_in_pixels(bo);
 
        hnd->magic = cros_gralloc_magic();
 
-       hnd->pixel_stride = hnd->strides[0];
-       hnd->pixel_stride /= drv_bytes_per_pixel(hnd->format, 0);
-
        return hnd;
 }
 
diff --git a/drv.c b/drv.c
index 41bd1f1..cfeb4b5 100644 (file)
--- a/drv.c
+++ b/drv.c
@@ -563,12 +563,6 @@ size_t drv_num_planes_from_format(uint32_t format)
        return 0;
 }
 
-uint32_t
-drv_bytes_per_pixel(uint32_t format, size_t plane)
-{
-       return DIV_ROUND_UP(drv_bpp_from_format(format, plane), 8);
-}
-
 uint32_t drv_size_from_format(uint32_t format, uint32_t stride,
                              uint32_t height, size_t plane)
 {
diff --git a/drv.h b/drv.h
index 42bc2ce..02c45fa 100644 (file)
--- a/drv.h
+++ b/drv.h
@@ -160,15 +160,15 @@ uint32_t
 drv_bo_get_format(struct bo *bo);
 
 uint32_t
+drv_bo_get_stride_in_pixels(struct bo *bo);
+
+uint32_t
 drv_resolve_format(struct driver *drv, uint32_t format);
 
 size_t
 drv_num_planes_from_format(uint32_t format);
 
 uint32_t
-drv_bytes_per_pixel(uint32_t format, size_t plane);
-
-uint32_t
 drv_size_from_format(uint32_t format, uint32_t stride, uint32_t height,
                     size_t plane);
 
index 951093c..800d314 100644 (file)
--- a/helpers.c
+++ b/helpers.c
 #include "helpers.h"
 #include "util.h"
 
-int drv_bpp_from_format(uint32_t format, size_t plane)
+static uint32_t subsample_stride(uint32_t stride, uint32_t format,
+                                size_t plane) {
+
+       if (plane != 0) {
+               switch (format) {
+               case DRM_FORMAT_YVU420:
+               case DRM_FORMAT_YVU420_ANDROID:
+                       stride = DIV_ROUND_UP(stride, 2);
+                       break;
+               }
+       }
+
+       return stride;
+}
+
+static uint32_t bpp_from_format(uint32_t format, size_t plane)
 {
        assert(plane < drv_num_planes_from_format(format));
 
@@ -31,25 +46,8 @@ int drv_bpp_from_format(uint32_t format, size_t plane)
        case DRM_FORMAT_YVU420_ANDROID:
                return 8;
 
-       /*
-        * NV12 is laid out as follows. Each letter represents a byte.
-        * Y plane:
-        * Y0_0, Y0_1, Y0_2, Y0_3, ..., Y0_N
-        * Y1_0, Y1_1, Y1_2, Y1_3, ..., Y1_N
-        * ...
-        * YM_0, YM_1, YM_2, YM_3, ..., YM_N
-        * CbCr plane:
-        * Cb01_01, Cr01_01, Cb01_23, Cr01_23, ..., Cb01_(N-1)N, Cr01_(N-1)N
-        * Cb23_01, Cr23_01, Cb23_23, Cr23_23, ..., Cb23_(N-1)N, Cr23_(N-1)N
-        * ...
-        * Cb(M-1)M_01, Cr(M-1)M_01, ..., Cb(M-1)M_(N-1)N, Cr(M-1)M_(N-1)N
-        *
-        * Pixel (0, 0) requires Y0_0, Cb01_01 and Cr01_01. Pixel (0, 1) requires
-        * Y0_1, Cb01_01 and Cr01_01.  So for a single pixel, 2 bytes of luma data
-        * are required.
-        */
        case DRM_FORMAT_NV12:
-               return (plane == 0) ? 8 : 16;
+               return (plane == 0) ? 8 : 4;
 
        case DRM_FORMAT_ABGR1555:
        case DRM_FORMAT_ABGR4444:
@@ -105,45 +103,37 @@ int drv_bpp_from_format(uint32_t format, size_t plane)
        return 0;
 }
 
+uint32_t drv_bo_get_stride_in_pixels(struct bo *bo)
+{
+       uint32_t bytes_per_pixel = DIV_ROUND_UP(bpp_from_format(bo->format, 0),
+                                               8);
+       return DIV_ROUND_UP(bo->strides[0], bytes_per_pixel);
+}
+
 /*
  * This function returns the stride for a given format, width and plane.
  */
 uint32_t drv_stride_from_format(uint32_t format, uint32_t width, size_t plane)
 {
-       uint32_t stride = DIV_ROUND_UP(width * drv_bpp_from_format(format, plane),
+       uint32_t stride = DIV_ROUND_UP(width * bpp_from_format(format, plane),
                                       8);
 
        /*
-        * Only downsample for certain multiplanar formats which have horizontal
-        * subsampling for chroma planes.  Only formats supported by our drivers
-        * are listed here -- add more as needed.
-        */
-       if (plane != 0) {
-               switch (format) {
-               case DRM_FORMAT_NV12:
-               case DRM_FORMAT_YVU420:
-               case DRM_FORMAT_YVU420_ANDROID:
-                       stride = DIV_ROUND_UP(stride, 2);
-                       break;
-               }
-       }
-
-       /*
         * The stride of Android YV12 buffers is required to be aligned to 16 bytes
         * (see <system/graphics.h>).
         */
        if (format == DRM_FORMAT_YVU420_ANDROID)
-               stride = ALIGN(stride, 16);
+               stride = (plane == 0) ? ALIGN(stride, 32): ALIGN(stride, 16);
 
        return stride;
 }
 
 /*
- * This function fills in the buffer object given driver aligned dimensions
- * (in pixels) and a format. This function assumes there is just one kernel
- * buffer per buffer object.
+ * This function fills in the buffer object given the driver aligned stride of
+ * the first plane, height and a format. This function assumes there is just
+ * one kernel buffer per buffer object.
  */
-int drv_bo_from_format(struct bo *bo, uint32_t aligned_width,
+int drv_bo_from_format(struct bo *bo, uint32_t stride,
                       uint32_t aligned_height, uint32_t format)
 {
 
@@ -155,8 +145,7 @@ int drv_bo_from_format(struct bo *bo, uint32_t aligned_width,
        bo->total_size = 0;
 
        for (p = 0; p < num_planes; p++) {
-               bo->strides[p] = drv_stride_from_format(format, aligned_width,
-                                                       p);
+               bo->strides[p] = subsample_stride(stride, format, p);
                bo->sizes[p] = drv_size_from_format(format, bo->strides[p],
                                                    bo->height, p);
                bo->offsets[p] = offset;
@@ -173,15 +162,14 @@ int drv_dumb_bo_create(struct bo *bo, uint32_t width, uint32_t height,
 {
        int ret;
        size_t plane;
-       uint32_t aligned_width, aligned_height, bytes_per_pixel;
+       uint32_t aligned_width, aligned_height;
        struct drm_mode_create_dumb create_dumb;
 
        aligned_width = width;
        aligned_height = height;
-       bytes_per_pixel = drv_bytes_per_pixel(format, 0);
        if (format == DRM_FORMAT_YVU420_ANDROID) {
                /*
-                * Align width to 16 pixels, so chroma strides are 16 bytes as
+                * Align width to 32 pixels, so chroma strides are 16 bytes as
                 * Android requires.
                 */
                aligned_width = ALIGN(width, 32);
@@ -191,7 +179,7 @@ int drv_dumb_bo_create(struct bo *bo, uint32_t width, uint32_t height,
        memset(&create_dumb, 0, sizeof(create_dumb));
        create_dumb.height = aligned_height;
        create_dumb.width = aligned_width;
-       create_dumb.bpp = drv_bpp_from_format(format, 0);
+       create_dumb.bpp = bpp_from_format(format, 0);
        create_dumb.flags = 0;
 
        ret = drmIoctl(bo->drv->fd, DRM_IOCTL_MODE_CREATE_DUMB, &create_dumb);
@@ -200,8 +188,7 @@ int drv_dumb_bo_create(struct bo *bo, uint32_t width, uint32_t height,
                return ret;
        }
 
-       drv_bo_from_format(bo, DIV_ROUND_UP(create_dumb.pitch, bytes_per_pixel),
-                          height, format);
+       drv_bo_from_format(bo, create_dumb.pitch, height, format);
 
        for (plane = 0; plane < bo->num_planes; plane++)
                bo->handles[plane].u32 = create_dumb.handle;
index a8e685c..5acd01f 100644 (file)
--- a/helpers.h
+++ b/helpers.h
@@ -9,9 +9,8 @@
 
 #include "drv.h"
 
-int drv_bpp_from_format(uint32_t format, size_t plane);
 uint32_t drv_stride_from_format(uint32_t format, uint32_t width, size_t plane);
-int drv_bo_from_format(struct bo *bo, uint32_t aligned_width,
+int drv_bo_from_format(struct bo *bo, uint32_t stride,
                       uint32_t aligned_height, uint32_t format);
 int drv_dumb_bo_create(struct bo *bo, uint32_t width, uint32_t height,
                       uint32_t format, uint32_t flags);
diff --git a/i915.c b/i915.c
index 915a4ce..b744d38 100644 (file)
--- a/i915.c
+++ b/i915.c
@@ -146,54 +146,49 @@ static int i915_add_combinations(struct driver *drv)
        return 0;
 }
 
-static void i915_align_dimensions(struct driver *drv, uint32_t tiling_mode,
-                                 uint32_t *width, uint32_t *height, int bpp)
+static int i915_align_dimensions(struct bo *bo, uint32_t tiling,
+                                uint32_t *stride, uint32_t *aligned_height)
 {
-       struct i915_device *i915_dev = (struct i915_device *)drv->priv;
-       uint32_t width_alignment = 4, height_alignment = 4;
+       struct i915_device *i915 = bo->drv->priv;
+       uint32_t horizontal_alignment = 4;
+       uint32_t vertical_alignment = 4;
 
-       switch (tiling_mode) {
+       switch (tiling) {
        default:
        case I915_TILING_NONE:
-               width_alignment = 64 / bpp;
+               horizontal_alignment = 64;
                break;
 
        case I915_TILING_X:
-               width_alignment = 512 / bpp;
-               height_alignment = 8;
+               horizontal_alignment = 512;
+               vertical_alignment = 8;
                break;
 
        case I915_TILING_Y:
-               if (i915_dev->gen == 3) {
-                       width_alignment = 512 / bpp;
-                       height_alignment = 8;
+               if (i915->gen == 3) {
+                       horizontal_alignment = 512;
+                       vertical_alignment = 8;
                } else  {
-                       width_alignment = 128 / bpp;
-                       height_alignment = 32;
+                       horizontal_alignment = 128;
+                       vertical_alignment = 32;
                }
                break;
        }
 
-       if (i915_dev->gen > 3) {
-               *width = ALIGN(*width, width_alignment);
-               *height = ALIGN(*height, height_alignment);
+       *aligned_height = ALIGN(bo->height, vertical_alignment);
+       if (i915->gen > 3) {
+               *stride = ALIGN(*stride, horizontal_alignment);
        } else {
-               uint32_t w;
-               for (w = width_alignment; w < *width;  w <<= 1)
-                       ;
-               *width = w;
-               *height = ALIGN(*height, height_alignment);
+               while (*stride > horizontal_alignment)
+                       horizontal_alignment <<= 1;
+
+               *stride = horizontal_alignment;
        }
-}
 
-static int i915_verify_dimensions(struct driver *drv, uint32_t stride,
-                                 uint32_t height)
-{
-       struct i915_device *i915_dev = (struct i915_device *)drv->priv;
-       if (i915_dev->gen <= 3 && stride > 8192)
-               return 0;
+       if (i915->gen <= 3 && *stride > 8192)
+               return -EINVAL;
 
-       return 1;
+       return 0;
 }
 
 static int i915_init(struct driver *drv)
@@ -246,10 +241,11 @@ static int i915_bo_create(struct bo *bo, uint32_t width, uint32_t height,
        int ret;
        size_t plane;
        char name[20];
+       uint32_t stride;
        uint32_t tiling_mode;
        struct i915_bo *i915_bo;
 
-       int bpp = drv_stride_from_format(format, 1, 0);
+       stride = drv_stride_from_format(format, width, 0);
        struct i915_device *i915_dev = (struct i915_device *)bo->drv->priv;
 
        if (flags & (BO_USE_CURSOR | BO_USE_LINEAR |
@@ -266,15 +262,15 @@ static int i915_bo_create(struct bo *bo, uint32_t width, uint32_t height,
         */
        if (format == DRM_FORMAT_YVU420 ||
            format == DRM_FORMAT_YVU420_ANDROID) {
-               width = ALIGN(width, 128);
+               stride = ALIGN(stride, 128);
                tiling_mode = I915_TILING_NONE;
        }
 
-       i915_align_dimensions(bo->drv, tiling_mode, &width, &height, bpp);
-       drv_bo_from_format(bo, width, height, format);
+       ret = i915_align_dimensions(bo, tiling_mode, &stride, &height);
+       if (ret)
+               return ret;
 
-       if (!i915_verify_dimensions(bo->drv, bo->strides[0], height))
-               return -EINVAL;
+       drv_bo_from_format(bo, stride, height, format);
 
        snprintf(name, sizeof(name), "i915-buffer-%u", i915_dev->count);
        i915_dev->count++;
index 22d89d3..ab59e9a 100644 (file)
@@ -33,15 +33,16 @@ static int mediatek_bo_create(struct bo *bo, uint32_t width, uint32_t height,
 {
        int ret;
        size_t plane;
+       uint32_t stride;
        struct drm_mtk_gem_create gem_create;
-       uint32_t bytes_per_pixel = drv_stride_from_format(format, 1, 0);
 
        /*
         * Since the ARM L1 cache line size is 64 bytes, align to that as a
         * performance optimization.
         */
-       width = ALIGN(width, DIV_ROUND_UP(64, bytes_per_pixel));
-       drv_bo_from_format(bo, width, height, format);
+       stride = drv_stride_from_format(format, width, 0);
+       stride = ALIGN(stride, 64);
+       drv_bo_from_format(bo, stride, height, format);
 
        memset(&gem_create, 0, sizeof(gem_create));
        gem_create.size = bo->total_size;
index f901ae7..468d054 100644 (file)
@@ -185,13 +185,14 @@ static int rockchip_bo_create_with_modifiers(struct bo *bo,
                        return -1;
                }
 
+               uint32_t stride;
                /*
                 * Since the ARM L1 cache line size is 64 bytes, align to that
                 * as a performance optimization.
                 */
-               uint32_t bytes_per_pixel = drv_stride_from_format(format, 1, 0);
-               width = ALIGN(width, DIV_ROUND_UP(64, bytes_per_pixel));
-               drv_bo_from_format(bo, width, height, format);
+               stride = drv_stride_from_format(format, width, 0);
+               stride = ALIGN(stride, 64);
+               drv_bo_from_format(bo, stride, height, format);
        }
 
        memset(&gem_create, 0, sizeof(gem_create));
diff --git a/vc4.c b/vc4.c
index 5dcd0e8..7036de7 100644 (file)
--- a/vc4.c
+++ b/vc4.c
@@ -31,9 +31,16 @@ static int vc4_bo_create(struct bo *bo, uint32_t width, uint32_t height,
 {
        int ret;
        size_t plane;
+       uint32_t stride;
        struct drm_vc4_create_bo bo_create;
 
-       drv_bo_from_format(bo, width, height, format);
+       /*
+        * Since the ARM L1 cache line size is 64 bytes, align to that as a
+        * performance optimization.
+        */
+       stride = drv_stride_from_format(format, width, 0);
+       stride = ALIGN(stride, 64);
+       drv_bo_from_format(bo, stride, height, format);
 
        memset(&bo_create, 0, sizeof(bo_create));
        bo_create.size = bo->total_size;