[PATCH v2 5/5] tests: make drm_read platform agnostic

[PATCH v2 5/5] tests: make drm_read platform agnostic

[Micah Fedke]

Update the drm_read test to operate on any platform to demonstrate the use of
drm_open_driver(OPEN_ANY_GPU).

To work on exynos, the event generation code is converted to use the new CRTC
selection API for vblank.  The first valid crtc is selected at fixture-time.

pipe0_enabled() is updated to use the drmMode* wrapper functions instead of
direct ioctls, and the unnecessary, intel-specific pipe<->crtc mapping ioctl is
dropped.

With these updates in place, drm_read can run successfully on intel and exynos.
Tested on ivb and peach-pi, respectively.

Signed-off-by: Micah Fedke <micah.fedke@collabora.co.uk>
---
 tests/drm_read.c | 87 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 50 insertions(+), 37 deletions(-)

diff --git a/tests/drm_read.c b/tests/drm_read.c
index fdaf126..38fde26 100644
--- a/tests/drm_read.c
+++ b/tests/drm_read.c
@@ -45,10 +45,15 @@
 #include "drm.h"
 #include "ioctl_wrappers.h"
 #include "drmtest.h"
+#include "igt_core.h"
 #include "igt_aux.h"
+#include "igt_kms.h"
 
 IGT_TEST_DESCRIPTION("Call read(drm) and see if it behaves.");
 
+static drmModeRes *resources;
+static int crtc_idx;
+
 static void sighandler(int sig)
 {
 }
@@ -61,16 +66,19 @@ static void assert_empty(int fd)
 
 static void generate_event(int fd)
 {
-	union drm_wait_vblank vbl;
+	drmVBlank wait_vbl;
+	unsigned crtc_idx_mask;
+	memset(&wait_vbl, 0, sizeof(wait_vbl));
 
-	/* We require that pipe 0 is running */
+	crtc_idx_mask = crtc_idx << DRM_VBLANK_HIGH_CRTC_SHIFT;
+	igt_assert(!(crtc_idx_mask & ~DRM_VBLANK_HIGH_CRTC_MASK));
 
-	vbl.request.type =
-		DRM_VBLANK_RELATIVE |
-		DRM_VBLANK_EVENT;
-	vbl.request.sequence = 0;
+	wait_vbl.request.type = crtc_idx_mask;
+	wait_vbl.request.type |= DRM_VBLANK_RELATIVE;
+	wait_vbl.request.type |= DRM_VBLANK_EVENT;
+	wait_vbl.request.sequence = 1;
 
-	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+	igt_assert(!drmWaitVBlank(fd, &wait_vbl));
 }
 
 static void wait_for_event(int fd)
@@ -154,44 +162,27 @@ static void test_short_buffer(int in, int nonblock)
 
 static int pipe0_enabled(int fd)
 {
-	struct drm_mode_card_res res;
-	uint32_t crtcs[32];
-	int i;
+	drmModeRes *res;
+	drmModeCrtc *crtc;
+	int ret;
 
 	/* We assume we can generate events on pipe 0. So we have better
 	 * make sure that is running!
 	 */
 
-	memset(&res, 0, sizeof(res));
-	res.count_crtcs = 32;
-	res.crtc_id_ptr = (uintptr_t)crtcs;
-
-	if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res))
-		return 0;
-
-	if (res.count_crtcs > 32)
+	res = drmModeGetResources(fd);
+	igt_assert(res);
+	crtc = drmModeGetCrtc(fd, res->crtcs[crtc_idx]);
+	if (!crtc){
 		return 0;
+	}
 
-	for (i = 0; i < res.count_crtcs; i++) {
-		struct drm_i915_get_pipe_from_crtc_id get_pipe;
-		struct drm_mode_crtc mode;
-
-		memset(&get_pipe, 0, sizeof(get_pipe));
-		memset(&mode, 0, sizeof(mode));
-
-		mode.crtc_id = crtcs[i];
-
-		get_pipe.pipe = -1;
-		get_pipe.crtc_id = mode.crtc_id;
-		drmIoctl(fd, DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &get_pipe);
-		if (get_pipe.pipe)
-			continue;
+	ret = crtc->mode_valid && crtc->mode.clock;
 
-		drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &mode);
-		return mode.mode_valid && mode.mode.clock;
-	}
+	drmModeFreeCrtc(crtc);
+	drmModeFreeResources(res);
 
-	return 0;
+	return ret;
 }
 
 igt_main
@@ -202,8 +193,30 @@ igt_main
 	siginterrupt(SIGALRM, 1);
 
 	igt_fixture {
-		fd = drm_open_driver_master(DRIVER_INTEL);
+		struct kmstest_connector_config config;
+		int i, n;
+
+		fd = drm_open_driver_master(OPEN_ANY_GPU);
+		igt_enable_connectors(fd);
+		kmstest_set_vt_graphics_mode();
+
 		igt_require(pipe0_enabled(fd));
+
+		resources = drmModeGetResources(fd);
+		igt_assert(resources);
+
+		for (i = 0; i < resources->count_connectors; i++) {
+			for (n = 0; n < resources->count_crtcs; n++) {
+				//use the first connector config we find
+				if(kmstest_get_connector_config(fd, resources->connectors[i],
+						1 << n, &config)){
+					crtc_idx = config.crtc_idx;
+					break;
+				}
+			}
+		}
+		drmModeFreeCrtc(config.crtc);
+
 	}
 
 	igt_subtest("invalid-buffer")
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] tests: make drm_read platform agnostic

[Chris Wilson]

On Tue, Aug 11, 2015 at 11:59:16AM -0400, Micah Fedke wrote:
> 
> Update the drm_read test to operate on any platform to demonstrate the use of
> drm_open_driver(OPEN_ANY_GPU).
> 
> To work on exynos, the event generation code is converted to use the new CRTC
> selection API for vblank.  The first valid crtc is selected at fixture-time.

Pardon? If exoynos doesn't implement the generic drmWaitVBlank(), fix
it.
 
> pipe0_enabled() is updated to use the drmMode* wrapper functions instead of
> direct ioctls, and the unnecessary, intel-specific pipe<->crtc mapping ioctl is
> dropped.

Just drop the pipe lookup, we can just indeed just use index 0.
 
> With these updates in place, drm_read can run successfully on intel and exynos.
> Tested on ivb and peach-pi, respectively.

You don't need to change that much, especially changing the path through
the kernel.

Nak.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] tests: make drm_read platform agnostic

[Daniel Vetter]

On Tue, Aug 11, 2015 at 11:59:16AM -0400, Micah Fedke wrote:
> 
> Update the drm_read test to operate on any platform to demonstrate the use of
> drm_open_driver(OPEN_ANY_GPU).
> 
> To work on exynos, the event generation code is converted to use the new CRTC
> selection API for vblank.  The first valid crtc is selected at fixture-time.
> 
> pipe0_enabled() is updated to use the drmMode* wrapper functions instead of
> direct ioctls, and the unnecessary, intel-specific pipe<->crtc mapping ioctl is
> dropped.
> 
> With these updates in place, drm_read can run successfully on intel and exynos.
> Tested on ivb and peach-pi, respectively.
> 
> Signed-off-by: Micah Fedke <micah.fedke@collabora.co.uk>

drm_read isn't the greatest test since it implicitly relies upon fbcon
enabling the screens for us. I think that should be fixed first.

> ---
>  tests/drm_read.c | 87 ++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 50 insertions(+), 37 deletions(-)
> 
> diff --git a/tests/drm_read.c b/tests/drm_read.c
> index fdaf126..38fde26 100644
> --- a/tests/drm_read.c
> +++ b/tests/drm_read.c
> @@ -45,10 +45,15 @@
>  #include "drm.h"
>  #include "ioctl_wrappers.h"
>  #include "drmtest.h"
> +#include "igt_core.h"
>  #include "igt_aux.h"
> +#include "igt_kms.h"
>  
>  IGT_TEST_DESCRIPTION("Call read(drm) and see if it behaves.");
>  
> +static drmModeRes *resources;
> +static int crtc_idx;
> +
>  static void sighandler(int sig)
>  {
>  }
> @@ -61,16 +66,19 @@ static void assert_empty(int fd)
>  
>  static void generate_event(int fd)
>  {
> -	union drm_wait_vblank vbl;
> +	drmVBlank wait_vbl;
> +	unsigned crtc_idx_mask;
> +	memset(&wait_vbl, 0, sizeof(wait_vbl));
>  
> -	/* We require that pipe 0 is running */
> +	crtc_idx_mask = crtc_idx << DRM_VBLANK_HIGH_CRTC_SHIFT;
> +	igt_assert(!(crtc_idx_mask & ~DRM_VBLANK_HIGH_CRTC_MASK));
>  
> -	vbl.request.type =
> -		DRM_VBLANK_RELATIVE |
> -		DRM_VBLANK_EVENT;
> -	vbl.request.sequence = 0;
> +	wait_vbl.request.type = crtc_idx_mask;
> +	wait_vbl.request.type |= DRM_VBLANK_RELATIVE;
> +	wait_vbl.request.type |= DRM_VBLANK_EVENT;
> +	wait_vbl.request.sequence = 1;
>  
> -	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> +	igt_assert(!drmWaitVBlank(fd, &wait_vbl));
>  }
>  
>  static void wait_for_event(int fd)
> @@ -154,44 +162,27 @@ static void test_short_buffer(int in, int nonblock)
>  
>  static int pipe0_enabled(int fd)
>  {
> -	struct drm_mode_card_res res;
> -	uint32_t crtcs[32];
> -	int i;
> +	drmModeRes *res;
> +	drmModeCrtc *crtc;
> +	int ret;
>  
>  	/* We assume we can generate events on pipe 0. So we have better
>  	 * make sure that is running!
>  	 */
>  
> -	memset(&res, 0, sizeof(res));
> -	res.count_crtcs = 32;
> -	res.crtc_id_ptr = (uintptr_t)crtcs;
> -
> -	if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res))
> -		return 0;
> -
> -	if (res.count_crtcs > 32)
> +	res = drmModeGetResources(fd);
> +	igt_assert(res);
> +	crtc = drmModeGetCrtc(fd, res->crtcs[crtc_idx]);
> +	if (!crtc){
>  		return 0;
> +	}
>  
> -	for (i = 0; i < res.count_crtcs; i++) {
> -		struct drm_i915_get_pipe_from_crtc_id get_pipe;
> -		struct drm_mode_crtc mode;
> -
> -		memset(&get_pipe, 0, sizeof(get_pipe));
> -		memset(&mode, 0, sizeof(mode));
> -
> -		mode.crtc_id = crtcs[i];
> -
> -		get_pipe.pipe = -1;
> -		get_pipe.crtc_id = mode.crtc_id;
> -		drmIoctl(fd, DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &get_pipe);
> -		if (get_pipe.pipe)
> -			continue;
> +	ret = crtc->mode_valid && crtc->mode.clock;
>  
> -		drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &mode);
> -		return mode.mode_valid && mode.mode.clock;
> -	}
> +	drmModeFreeCrtc(crtc);
> +	drmModeFreeResources(res);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  igt_main
> @@ -202,8 +193,30 @@ igt_main
>  	siginterrupt(SIGALRM, 1);
>  
>  	igt_fixture {
> -		fd = drm_open_driver_master(DRIVER_INTEL);
> +		struct kmstest_connector_config config;
> +		int i, n;
> +
> +		fd = drm_open_driver_master(OPEN_ANY_GPU);
> +		igt_enable_connectors(fd);
> +		kmstest_set_vt_graphics_mode();

Because in general we don't want fbcon to do things behind our back, like
suspending the display.
-Daniel

> +
>  		igt_require(pipe0_enabled(fd));
> +
> +		resources = drmModeGetResources(fd);
> +		igt_assert(resources);
> +
> +		for (i = 0; i < resources->count_connectors; i++) {
> +			for (n = 0; n < resources->count_crtcs; n++) {
> +				//use the first connector config we find
> +				if(kmstest_get_connector_config(fd, resources->connectors[i],
> +						1 << n, &config)){
> +					crtc_idx = config.crtc_idx;
> +					break;
> +				}
> +			}
> +		}
> +		drmModeFreeCrtc(config.crtc);
> +
>  	}
>  
>  	igt_subtest("invalid-buffer")
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] tests: make drm_read platform agnostic

[Micah Fedke]

Yup, in fact I see that if the console blanking kicks on, that the 
drm_read test fails the first time, and then succeed on the second run. 
  It must wake up the console when it runs?

(Chris, I tried drm_read without the vblank api switch and it does work 
just fine - I must have carried that over from some older work.)

Anyway, both core_get_client_auth and core_getversion work with nothing 
more than:
- int fd = drm_open_driver(DRIVER_INTEL);
+ int fd = drm_open_driver(OPEN_ANY_GPU);

I'll just spin up a patchset with one or both of those, if that makes 
sense to everyone.


-mf

On 08/12/2015 09:26 AM, Daniel Vetter wrote:
> On Tue, Aug 11, 2015 at 11:59:16AM -0400, Micah Fedke wrote:
>>
>> Update the drm_read test to operate on any platform to demonstrate the use of
>> drm_open_driver(OPEN_ANY_GPU).
>>
>> To work on exynos, the event generation code is converted to use the new CRTC
>> selection API for vblank.  The first valid crtc is selected at fixture-time.
>>
>> pipe0_enabled() is updated to use the drmMode* wrapper functions instead of
>> direct ioctls, and the unnecessary, intel-specific pipe<->crtc mapping ioctl is
>> dropped.
>>
>> With these updates in place, drm_read can run successfully on intel and exynos.
>> Tested on ivb and peach-pi, respectively.
>>
>> Signed-off-by: Micah Fedke <micah.fedke@collabora.co.uk>
>
> drm_read isn't the greatest test since it implicitly relies upon fbcon
> enabling the screens for us. I think that should be fixed first.
>
>> ---
>>   tests/drm_read.c | 87 ++++++++++++++++++++++++++++++++------------------------
>>   1 file changed, 50 insertions(+), 37 deletions(-)
>>
>> diff --git a/tests/drm_read.c b/tests/drm_read.c
>> index fdaf126..38fde26 100644
>> --- a/tests/drm_read.c
>> +++ b/tests/drm_read.c
>> @@ -45,10 +45,15 @@
>>   #include "drm.h"
>>   #include "ioctl_wrappers.h"
>>   #include "drmtest.h"
>> +#include "igt_core.h"
>>   #include "igt_aux.h"
>> +#include "igt_kms.h"
>>
>>   IGT_TEST_DESCRIPTION("Call read(drm) and see if it behaves.");
>>
>> +static drmModeRes *resources;
>> +static int crtc_idx;
>> +
>>   static void sighandler(int sig)
>>   {
>>   }
>> @@ -61,16 +66,19 @@ static void assert_empty(int fd)
>>
>>   static void generate_event(int fd)
>>   {
>> -	union drm_wait_vblank vbl;
>> +	drmVBlank wait_vbl;
>> +	unsigned crtc_idx_mask;
>> +	memset(&wait_vbl, 0, sizeof(wait_vbl));
>>
>> -	/* We require that pipe 0 is running */
>> +	crtc_idx_mask = crtc_idx << DRM_VBLANK_HIGH_CRTC_SHIFT;
>> +	igt_assert(!(crtc_idx_mask & ~DRM_VBLANK_HIGH_CRTC_MASK));
>>
>> -	vbl.request.type =
>> -		DRM_VBLANK_RELATIVE |
>> -		DRM_VBLANK_EVENT;
>> -	vbl.request.sequence = 0;
>> +	wait_vbl.request.type = crtc_idx_mask;
>> +	wait_vbl.request.type |= DRM_VBLANK_RELATIVE;
>> +	wait_vbl.request.type |= DRM_VBLANK_EVENT;
>> +	wait_vbl.request.sequence = 1;
>>
>> -	do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> +	igt_assert(!drmWaitVBlank(fd, &wait_vbl));
>>   }
>>
>>   static void wait_for_event(int fd)
>> @@ -154,44 +162,27 @@ static void test_short_buffer(int in, int nonblock)
>>
>>   static int pipe0_enabled(int fd)
>>   {
>> -	struct drm_mode_card_res res;
>> -	uint32_t crtcs[32];
>> -	int i;
>> +	drmModeRes *res;
>> +	drmModeCrtc *crtc;
>> +	int ret;
>>
>>   	/* We assume we can generate events on pipe 0. So we have better
>>   	 * make sure that is running!
>>   	 */
>>
>> -	memset(&res, 0, sizeof(res));
>> -	res.count_crtcs = 32;
>> -	res.crtc_id_ptr = (uintptr_t)crtcs;
>> -
>> -	if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res))
>> -		return 0;
>> -
>> -	if (res.count_crtcs > 32)
>> +	res = drmModeGetResources(fd);
>> +	igt_assert(res);
>> +	crtc = drmModeGetCrtc(fd, res->crtcs[crtc_idx]);
>> +	if (!crtc){
>>   		return 0;
>> +	}
>>
>> -	for (i = 0; i < res.count_crtcs; i++) {
>> -		struct drm_i915_get_pipe_from_crtc_id get_pipe;
>> -		struct drm_mode_crtc mode;
>> -
>> -		memset(&get_pipe, 0, sizeof(get_pipe));
>> -		memset(&mode, 0, sizeof(mode));
>> -
>> -		mode.crtc_id = crtcs[i];
>> -
>> -		get_pipe.pipe = -1;
>> -		get_pipe.crtc_id = mode.crtc_id;
>> -		drmIoctl(fd, DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &get_pipe);
>> -		if (get_pipe.pipe)
>> -			continue;
>> +	ret = crtc->mode_valid && crtc->mode.clock;
>>
>> -		drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &mode);
>> -		return mode.mode_valid && mode.mode.clock;
>> -	}
>> +	drmModeFreeCrtc(crtc);
>> +	drmModeFreeResources(res);
>>
>> -	return 0;
>> +	return ret;
>>   }
>>
>>   igt_main
>> @@ -202,8 +193,30 @@ igt_main
>>   	siginterrupt(SIGALRM, 1);
>>
>>   	igt_fixture {
>> -		fd = drm_open_driver_master(DRIVER_INTEL);
>> +		struct kmstest_connector_config config;
>> +		int i, n;
>> +
>> +		fd = drm_open_driver_master(OPEN_ANY_GPU);
>> +		igt_enable_connectors(fd);
>> +		kmstest_set_vt_graphics_mode();
>
> Because in general we don't want fbcon to do things behind our back, like
> suspending the display.
> -Daniel
>
>> +
>>   		igt_require(pipe0_enabled(fd));
>> +
>> +		resources = drmModeGetResources(fd);
>> +		igt_assert(resources);
>> +
>> +		for (i = 0; i < resources->count_connectors; i++) {
>> +			for (n = 0; n < resources->count_crtcs; n++) {
>> +				//use the first connector config we find
>> +				if(kmstest_get_connector_config(fd, resources->connectors[i],
>> +						1 << n, &config)){
>> +					crtc_idx = config.crtc_idx;
>> +					break;
>> +				}
>> +			}
>> +		}
>> +		drmModeFreeCrtc(config.crtc);
>> +
>>   	}
>>
>>   	igt_subtest("invalid-buffer")
>> --
>> 2.5.0
>>
>

-- 

Micah Fedke
Collabora Ltd.
+44 1223 362967
https://www.collabora.com/
https://twitter.com/collaboraltd
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx