On Thu, Apr 26, 2018 at 2:40 PM, Gurchetan Singh <
gurchetansi...@chromium.org> wrote:

> On Thu, Apr 26, 2018 at 10:23 AM, Marek Olšák <mar...@gmail.com> wrote:
> > Gurchetan, can you confirm that the stride returned at alloc() is only
> used
> > for CPU access and not for display programming?
>
> It really depends on how you couple your HWComposer and gralloc HALs
> together.  For example, drm_hwcomposer calls drmModeAddFB2 from with
> the allocation time stride:
>
> https://anongit.freedesktop.org/git/drm_hwcomposer.git
>
> For the minigbm + wayland_hwcomposer use case, we have a
> GRALLOC_DRM_GET_STRIDE call that returns the allocation time pixel
> stride to the Wayland client.
>
> We could:
>       - Return the stride that works for display with
> GRALLOC_DRM_GET_STRIDE
>       - Return the stride that works for CPU access with the alloc() call.
>
> Just curious -- which Android app and/or test case is running into this
> issue?
>

There is some cros_gralloc test case that is failing, but nobody told me
which one. Can you have a look at the attached minigbm patch?

Marek
From 64865522a2fb41d338a34ee5aa79ffee059b071b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= <marek.ol...@amd.com>
Date: Thu, 26 Apr 2018 17:58:15 -0400
Subject: [PATCH] amdgpu: always set the CPU stride as the BO stride

---
 amdgpu.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/amdgpu.c b/amdgpu.c
index 3bf5eb2..600572b 100644
--- a/amdgpu.c
+++ b/amdgpu.c
@@ -139,15 +139,33 @@ static int amdgpu_create_bo(struct bo *bo, uint32_t width, uint32_t height, uint
 	if (!combo)
 		return -EINVAL;
 
-	if (combo->metadata.tiling == TILE_TYPE_DRI)
-		return dri_bo_create(bo, width, height, format, use_flags);
-
+	/* Compute the linear stride that's also use for CPU mappings. */
 	stride = drv_stride_from_format(format, width, 0);
 	if (format == DRM_FORMAT_YVU420_ANDROID)
 		stride = ALIGN(stride, 128);
 	else
 		stride = ALIGN(stride, 64);
 
+	if (combo->metadata.tiling == TILE_TYPE_DRI) {
+		int r = dri_bo_create(bo, width, height, format, use_flags);
+		if (r)
+			return r;
+
+		/* __DRI_IMAGE_ATTRIB_STRIDE is the display stride (tiled stride) and is used
+		 * for programming the display.
+		 *
+		 * For CPU mappings, we have to use the CPU stride (linear stride).
+		 *
+		 * In practice, we only have to make sure that the display and CPU strides are
+		 * the same for display resolutions that we support. In all other cases, we
+		 * don't need the display stride.
+		 *
+		 * Set the CPU stride so that CPU mappings work.
+		 */
+		bo->strides[0] = stride;
+		return 0;
+	}
+
 	drv_bo_from_format(bo, stride, height, format);
 
 	memset(&gem_create, 0, sizeof(gem_create));
-- 
2.17.0

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to