[1/2] drm/i915: Use CPU mapping for userspace dma-buf mmap()
diff mbox

Message ID 55C26DA9.9020907@intel.com
State New
Headers show

Commit Message

Tiago Vignatti Aug. 5, 2015, 8:10 p.m. UTC
On 08/05/2015 04:08 AM, Daniel Vetter wrote:
> On Tue, Aug 04, 2015 at 06:30:25PM -0300, Tiago Vignatti wrote:
> Nah they don't have to be equal since the problem isn't that nothing goes
> out to memory where the display can see it, but usually only parts of it.
> I.e. you need to change your test to
> - draw black screen (it starts that way so nothing to do really), grab crtc
> - draw white screen and make sure you flush correctly, don't bother with
>    crc (we can't test for inequality
>    because collisions are too easy)
> - draw black screen again without flushing, grab crc
>
> Then assert that your two crc will be inequal (which they shouldn't be
> because some cachelines will still be stuck). Maybe also add a delay
> somewhere so you can see the cacheline dirt pattern, it's very
> characteristic.

Cool, I've got it now. The test below makes the cachelines dirt, 
requiring them to get flushed correctly -- I'll work on it now. Should 
we add that kind of test somewhere in igt BTW?

PS: I had an issue with the original kms_pwrite_crc which returns 
frequent fails. Paulo helped though and showed me that pwrite is 
currently broken: https://bugs.freedesktop.org/show_bug.cgi?id=86422

Tiago

  	igt_display_t *display = &data->display;
@@ -57,6 +71,7 @@ static void test(data_t *data)
  	struct igt_fb *fb = &data->fb[1];
  	drmModeModeInfo *mode;
  	cairo_t *cr;
+	char *ptr;
  	uint32_t caching;
  	void *buf;
  	igt_crc_t crc;
@@ -67,6 +82,8 @@ static void test(data_t *data)
  	igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
  		      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, fb);

+	ptr = dmabuf_mmap_framebuffer(data->drm_fd, fb);
+
  	cr = igt_get_cairo_ctx(data->drm_fd, fb);
  	igt_paint_test_pattern(cr, fb->width, fb->height);
  	cairo_destroy(cr);
@@ -83,11 +100,11 @@ static void test(data_t *data)
  	caching = gem_get_caching(data->drm_fd, fb->gem_handle);
  	igt_assert(caching == I915_CACHING_NONE || caching == 
I915_CACHING_DISPLAY);

-	/* use pwrite to make the other fb all white too */
+	/* use dmabuf pointer to make the other fb all white too */
  	buf = malloc(fb->size);
  	igt_assert(buf != NULL);
  	memset(buf, 0xff, fb->size);
-	gem_write(data->drm_fd, fb->gem_handle, 0, buf, fb->size);
+	memcpy(ptr, buf, fb->size);
  	free(buf);

  	/* and flip to it */

Comments

Daniel Vetter Aug. 6, 2015, 1:17 p.m. UTC | #1
On Wed, Aug 05, 2015 at 05:10:17PM -0300, Tiago Vignatti wrote:
> On 08/05/2015 04:08 AM, Daniel Vetter wrote:
> >On Tue, Aug 04, 2015 at 06:30:25PM -0300, Tiago Vignatti wrote:
> >Nah they don't have to be equal since the problem isn't that nothing goes
> >out to memory where the display can see it, but usually only parts of it.
> >I.e. you need to change your test to
> >- draw black screen (it starts that way so nothing to do really), grab crtc
> >- draw white screen and make sure you flush correctly, don't bother with
> >   crc (we can't test for inequality
> >   because collisions are too easy)
> >- draw black screen again without flushing, grab crc
> >
> >Then assert that your two crc will be inequal (which they shouldn't be
> >because some cachelines will still be stuck). Maybe also add a delay
> >somewhere so you can see the cacheline dirt pattern, it's very
> >characteristic.
> 
> Cool, I've got it now. The test below makes the cachelines dirt, requiring
> them to get flushed correctly -- I'll work on it now. Should we add that
> kind of test somewhere in igt BTW?

Yeah if you expect me to merge dma-buf mmap with the begin/end stuff I'll
ask for an igt for it ;-)

> PS: I had an issue with the original kms_pwrite_crc which returns frequent
> fails. Paulo helped though and showed me that pwrite is currently broken:
> https://bugs.freedesktop.org/show_bug.cgi?id=86422

If you do dma-buf mmap with begin/end it should work, since in there we'll
just to manual range-based clflushing.
-Daniel


> 
> Tiago
> 
> diff --git a/tests/kms_pwrite_crc.c b/tests/kms_pwrite_crc.c
> index 05b9e38..419b46d 100644
> --- a/tests/kms_pwrite_crc.c
> +++ b/tests/kms_pwrite_crc.c
> @@ -50,6 +50,20 @@ typedef struct {
>  	uint32_t devid;
>  } data_t;
> 
> +static char *dmabuf_mmap_framebuffer(int drm_fd, struct igt_fb *fb)
> +{
> +	int dma_buf_fd;
> +	char *ptr = NULL;
> +
> +	dma_buf_fd = prime_handle_to_fd(drm_fd, fb->gem_handle);
> +	igt_assert(errno == 0);
> +
> +	ptr = mmap(NULL, fb->size, PROT_READ | PROT_WRITE, MAP_SHARED, dma_buf_fd,
> 0);
> +	igt_assert(ptr != MAP_FAILED);
> +
> +	return ptr;
> +}
> +
>  static void test(data_t *data)
>  {
>  	igt_display_t *display = &data->display;
> @@ -57,6 +71,7 @@ static void test(data_t *data)
>  	struct igt_fb *fb = &data->fb[1];
>  	drmModeModeInfo *mode;
>  	cairo_t *cr;
> +	char *ptr;
>  	uint32_t caching;
>  	void *buf;
>  	igt_crc_t crc;
> @@ -67,6 +82,8 @@ static void test(data_t *data)
>  	igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
>  		      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, fb);
> 
> +	ptr = dmabuf_mmap_framebuffer(data->drm_fd, fb);
> +
>  	cr = igt_get_cairo_ctx(data->drm_fd, fb);
>  	igt_paint_test_pattern(cr, fb->width, fb->height);
>  	cairo_destroy(cr);
> @@ -83,11 +100,11 @@ static void test(data_t *data)
>  	caching = gem_get_caching(data->drm_fd, fb->gem_handle);
>  	igt_assert(caching == I915_CACHING_NONE || caching ==
> I915_CACHING_DISPLAY);
> 
> -	/* use pwrite to make the other fb all white too */
> +	/* use dmabuf pointer to make the other fb all white too */
>  	buf = malloc(fb->size);
>  	igt_assert(buf != NULL);
>  	memset(buf, 0xff, fb->size);
> -	gem_write(data->drm_fd, fb->gem_handle, 0, buf, fb->size);
> +	memcpy(ptr, buf, fb->size);
>  	free(buf);
> 
>  	/* and flip to it */
>

Patch
diff mbox

diff --git a/tests/kms_pwrite_crc.c b/tests/kms_pwrite_crc.c
index 05b9e38..419b46d 100644
--- a/tests/kms_pwrite_crc.c
+++ b/tests/kms_pwrite_crc.c
@@ -50,6 +50,20 @@  typedef struct {
  	uint32_t devid;
  } data_t;

+static char *dmabuf_mmap_framebuffer(int drm_fd, struct igt_fb *fb)
+{
+	int dma_buf_fd;
+	char *ptr = NULL;
+
+	dma_buf_fd = prime_handle_to_fd(drm_fd, fb->gem_handle);
+	igt_assert(errno == 0);
+
+	ptr = mmap(NULL, fb->size, PROT_READ | PROT_WRITE, MAP_SHARED, 
dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+
+	return ptr;
+}
+
  static void test(data_t *data)
  {