piglit_drm_dma_buf: fix GPU offsets and strides

Submitted by Marek Olšák on June 19, 2017, 10:13 p.m.

Details

Message ID 1497910394-2968-1-git-send-email-maraeo@gmail.com
State New
Headers show
Series "piglit_drm_dma_buf: fix GPU offsets and strides" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Marek Olšák June 19, 2017, 10:13 p.m.
From: Marek Olšák <marek.olsak@amd.com>

Most of the original code is simply wrong.

This patch makes sure that at least the returned GPU offsets and strides
are correct. This doesn't fix the incorrect CPU upload path for non-zero
planes. GBM doesn't seem to have the capability to map a specific plane
for CPU access.
---
 .../util/piglit-framework-gl/piglit_drm_dma_buf.c  | 23 +++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/util/piglit-framework-gl/piglit_drm_dma_buf.c b/tests/util/piglit-framework-gl/piglit_drm_dma_buf.c
index c3225c3..869d9db 100644
--- a/tests/util/piglit-framework-gl/piglit_drm_dma_buf.c
+++ b/tests/util/piglit-framework-gl/piglit_drm_dma_buf.c
@@ -322,54 +322,55 @@  piglit_gbm_buf_create(unsigned w, unsigned h, unsigned fourcc,
 	dst_data = gbm_bo_map(bo, 0, 0, buf_w, buf_h, GBM_BO_TRANSFER_WRITE,
 			      &dst_stride, &map_data);
 	if (!dst_data) {
 		fprintf(stderr, "Failed to map GBM bo\n");
 		gbm_bo_destroy(bo);
 		return NULL;
 	}
 
 	buf->w = w;
 	buf->h = h;
-	buf->offset[0] = 0;
-	buf->stride[0] = dst_stride;
+	buf->offset[0] = gbm_bo_get_offset(bo, 0);
+	buf->stride[0] = gbm_bo_get_stride_for_plane(bo, 0);
 	buf->fd = -1;
 	buf->priv = bo;
 
 	for (i = 0; i < h; ++i) {
 		memcpy((char *)dst_data + i * dst_stride,
 		       src_data + i * src_stride,
 		       w * cpp);
 	}
 
 	switch (fourcc) {
 	case DRM_FORMAT_NV12:
-		buf->offset[1] = dst_stride * h;
-		buf->stride[1] = dst_stride;
 		for (i = 0; i < h/2; ++i) {
-			memcpy(((char *)dst_data + buf->offset[1]) + i * buf->stride[1],
+			memcpy(((char *)dst_data + dst_stride * h) + i * dst_stride,
 				(src_data + (w*h)) + i * src_stride, w);
 		}
+		buf->offset[1] = gbm_bo_get_offset(bo, 1);
+		buf->stride[1] = gbm_bo_get_stride_for_plane(bo, 1);
 		break;
 	case DRM_FORMAT_YUV420:
 	case DRM_FORMAT_YVU420:
-		buf->offset[1] = dst_stride * h;
-		buf->stride[1] = dst_stride / 2;
 		for (i = 0; i < h/2; ++i) {
-			memcpy(((char *)dst_data + buf->offset[1]) + i * buf->stride[1],
+			memcpy(((char *)dst_data + dst_stride * h) + i * dst_stride / 2,
 				(src_data + (w*h)) + i * src_stride / 2, w / 2);
 		}
-		buf->offset[2] = buf->offset[1] + (dst_stride * h / 2 / 2);
-		buf->stride[2] = dst_stride / 2;
+		unsigned cpu_offset2 = dst_stride * h + (dst_stride * h / 2 / 2);
 		for (i = 0; i < h/2; ++i) {
-			memcpy(((char *)dst_data + buf->offset[2]) + i * buf->stride[2],
+			memcpy(((char *)dst_data + cpu_offset2) + i * dst_stride / 2,
 				(src_data + (w*h) + (w*h/4)) + i * src_stride / 2, w / 2);
 		}
+		buf->offset[1] = gbm_bo_get_offset(bo, 1);
+		buf->stride[1] = gbm_bo_get_stride_for_plane(bo, 1);
+		buf->offset[2] = gbm_bo_get_offset(bo, 2);
+		buf->stride[2] = gbm_bo_get_stride_for_plane(bo, 2);
 		break;
 	default:
 		break;
 	}
 
 	gbm_bo_unmap(bo, map_data);
 
 
 	return true;
 }

Comments

Marek Olšák <maraeo@gmail.com> writes:

> From: Marek Olšák <marek.olsak@amd.com>
>
> Most of the original code is simply wrong.
>
> This patch makes sure that at least the returned GPU offsets and strides
> are correct. This doesn't fix the incorrect CPU upload path for non-zero
> planes. GBM doesn't seem to have the capability to map a specific plane
> for CPU access.

Reviewed-by: Eric Anholt <eric@anholt.net>
On Mon, 2017-06-19 at 16:22 -0700, Eric Anholt wrote:
> Marek Olšák <maraeo@gmail.com> writes:
> 
> > From: Marek Olšák <marek.olsak@amd.com>
> > 
> > Most of the original code is simply wrong.
> > 
> > This patch makes sure that at least the returned GPU offsets and strides
> > are correct. This doesn't fix the incorrect CPU upload path for non-zero
> > planes. GBM doesn't seem to have the capability to map a specific plane
> > for CPU access.
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>

This patch breaks build using older mesa/gbm:
$ rpm -q mesa-libgbm-devel
mesa-libgbm-devel-17.0.5-3.fc25.x86_64

/usr/bin/cc -I/home/vesely/mesa/include -Wall -std=gnu99 -Werror=vla
-Werror=pointer-arith -Werror=variadic-macros -g  -rdynamic
CMakeFiles/point-sprite.dir/point-sprite.c.o  -o ../../../../bin/point-
sprite -Wl,-rpath,/home/vesely/piglit/lib::
../../../../lib/libpiglitutil_gl.so.0 -lGL
../../../../lib/libpiglitutil.so.0 -lrt -ldl -lxkbcommon -lpng -lz -lm
-lEGL -lgbm -lwaffle-1 -ldrm -lxcb -lxcb-dri2 -ldrm_intel -ldrm -lxcb
-lxcb-dri2 -ldrm_intel -lX11 -lwayland-egl -lwayland-client -lGL 
../../../../lib/libpiglitutil_gl.so.0: undefined reference to
`gbm_bo_get_offset'
../../../../lib/libpiglitutil_gl.so.0: undefined reference to
`gbm_bo_get_stride_for_plane'
collect2: error: ld returned 1 exit status

looks like the used functions were added in 17.1, so this needs to bump
gbm requirement (or check for those functions explicitly).

Jan

> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit