Re: [PATCH 2/2] tests/pm_rpm: add planes subtests

[Matt Roper <matthew.d.roper@intel.com>]

On Mon, Jul 28, 2014 at 03:37:15PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Just like the cursor subtests, these also trigger WARNs on the current
> Kernel.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I feel like a lot of the setup you have to do here is duplicating logic
we have in the igt_kms library.  Was there functionality lacking from
that library that prevented you from using it to write this test?  If
so, I can look into adding it.

Anyway, the test still looks good, just one or two minor comments inline
below.

> ---
>  tests/pm_rpm.c | 212 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 211 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index f720f86..048d9ad 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -51,6 +51,9 @@
>  #include "igt_kms.h"
>  #include "igt_debugfs.h"
>  
> +/* One day, this will be on your libdrm. */
> +#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2

I think there was just an official release of libdrm today that finally
includes this and the plane type enum below...maybe we can just bump the
libdrm requirement for igt and stop including these by hand?

> +
>  #define MSR_PC8_RES	0x630
>  #define MSR_PC9_RES	0x631
>  #define MSR_PC10_RES	0x632
> @@ -72,6 +75,12 @@ enum screen_type {
>  	SCREEN_TYPE_ANY,
>  };
>  
> +enum plane_type {
> +	PLANE_OVERLAY,
> +	PLANE_PRIMARY,
> +	PLANE_CURSOR,
> +};
> +
>  /* Wait flags */
>  #define DONT_WAIT	0
>  #define WAIT_STATUS	1
> @@ -1504,6 +1513,199 @@ static void cursor_subtest(bool dpms)
>  	igt_assert(wait_for_suspended());
>  }
>  
> +static enum plane_type get_plane_type(uint32_t plane_id)
> +{
> +	drmModeObjectPropertiesPtr props;
> +	int i, j;
> +	enum plane_type type;
> +	bool found = false;
> +
> +	props = drmModeObjectGetProperties(drm_fd, plane_id,
> +					   DRM_MODE_OBJECT_PLANE);
> +	igt_assert(props);
> +
> +	for (i = 0; i < props->count_props && !found; i++) {
> +		drmModePropertyPtr prop;
> +		const char *enum_name = NULL;
> +
> +		prop = drmModeGetProperty(drm_fd, props->props[i]);
> +		igt_assert(prop);
> +
> +		if (strcmp(prop->name, "type") == 0) {
> +			igt_assert(prop->flags & DRM_MODE_PROP_ENUM);
> +			igt_assert(props->prop_values[i] < prop->count_enums);
> +
> +			for (j = 0; j < prop->count_enums; j++) {
> +				if (prop->enums[j].value ==
> +				    props->prop_values[i]) {
> +					enum_name = prop->enums[j].name;
> +					break;
> +				}
> +			}
> +			igt_assert(enum_name);
> +
> +			if (strcmp(enum_name, "Overlay") == 0)
> +				type = PLANE_OVERLAY;
> +			else if (strcmp(enum_name, "Primary") == 0)
> +				type = PLANE_PRIMARY;
> +			else if (strcmp(enum_name, "Cursor") == 0)
> +				type = PLANE_CURSOR;
> +			else
> +				igt_assert(0);
> +
> +			found = true;
> +		}
> +
> +		drmModeFreeProperty(prop);
> +	}
> +	igt_assert(found);
> +
> +	drmModeFreeObjectProperties(props);
> +	return type;
> +}
> +
> +static void test_one_plane(bool dpms, uint32_t plane_id,
> +			   enum plane_type plane_type)
> +{
> +	int rc;
> +	uint32_t plane_format, plane_w, plane_h;
> +	uint32_t crtc_id, connector_id;
> +	struct igt_fb scanout_fb, plane_fb1, plane_fb2;
> +	drmModeModeInfoPtr mode;
> +	int32_t crtc_x = 0, crtc_y = 0;
> +
> +	disable_all_screens(&ms_data);
> +	igt_assert(wait_for_suspended());
> +
> +	igt_require(find_connector_for_modeset(&ms_data, SCREEN_TYPE_ANY,
> +					       &connector_id, &mode));
> +
> +	crtc_id = ms_data.res->crtcs[0];
> +	igt_assert(crtc_id);
> +
> +	igt_create_fb(drm_fd, mode->hdisplay, mode->vdisplay,
> +		      DRM_FORMAT_XRGB8888, false, &scanout_fb);
> +
> +	fill_igt_fb(&scanout_fb, 0xFF);
> +
> +	switch (plane_type) {
> +	case PLANE_OVERLAY:
> +		plane_format = DRM_FORMAT_XRGB8888;
> +		plane_w = 64;
> +		plane_h = 64;
> +		break;
> +	case PLANE_PRIMARY:
> +		plane_format = DRM_FORMAT_XRGB8888;
> +		plane_w = mode->hdisplay;
> +		plane_h = mode->vdisplay;
> +		break;
> +	case PLANE_CURSOR:
> +		plane_format = DRM_FORMAT_ARGB8888;
> +		plane_w = 64;
> +		plane_h = 64;
> +		break;
> +	default:
> +		igt_assert(0);
> +		break;
> +	}
> +
> +	igt_create_fb(drm_fd, plane_w, plane_h, plane_format, false,
> +		      &plane_fb1);
> +	igt_create_fb(drm_fd, plane_w, plane_h, plane_format, false,
> +		      &plane_fb2);
> +	fill_igt_fb(&plane_fb1, 0xFF00FFFF);
> +	fill_igt_fb(&plane_fb2, 0xFF00FF00);
> +
> +	rc = drmModeSetCrtc(drm_fd, crtc_id, scanout_fb.fb_id, 0, 0,
> +			    &connector_id, 1, mode);
> +	igt_assert(rc == 0);
> +	igt_assert(wait_for_active());
> +
> +	rc = drmModeSetPlane(drm_fd, plane_id, crtc_id, plane_fb1.fb_id, 0,
> +			     0, 0, plane_fb1.width, plane_fb1.height,
> +			     0 << 16, 0 << 16, plane_fb1.width << 16,
> +			     plane_fb1.height << 16);
> +	igt_assert(rc == 0);
> +
> +	if (dpms)
> +		disable_all_screens_dpms(&ms_data);
> +	else
> +		disable_all_screens(&ms_data);
> +	igt_assert(wait_for_suspended());
> +
> +	/* Just move the plane around. */
> +	if (plane_type != PLANE_PRIMARY) {
> +		crtc_x++;
> +		crtc_y++;
> +	}
> +	rc = drmModeSetPlane(drm_fd, plane_id, crtc_id, plane_fb1.fb_id, 0,
> +			     crtc_x, crtc_y, plane_fb1.width, plane_fb1.height,
> +			     0 << 16, 0 << 16, plane_fb1.width << 16,
> +			     plane_fb1.height << 16);
> +	igt_assert(rc == 0);
> +
> +	/* Unset, then change the plane. */
> +	rc = drmModeSetPlane(drm_fd, plane_id, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
> +	igt_assert(rc == 0);
> +
> +	rc = drmModeSetPlane(drm_fd, plane_id, crtc_id, plane_fb2.fb_id, 0,
> +			     crtc_x, crtc_y, plane_fb2.width, plane_fb2.height,
> +			     0 << 16, 0 << 16, plane_fb2.width << 16,
> +			     plane_fb2.height << 16);
> +	igt_assert(rc == 0);
> +
> +	/* Now change the plane without unsetting first. */
> +	rc = drmModeSetPlane(drm_fd, plane_id, crtc_id, plane_fb1.fb_id, 0,
> +			     crtc_x, crtc_y, plane_fb1.width, plane_fb1.height,
> +			     0 << 16, 0 << 16, plane_fb1.width << 16,
> +			     plane_fb1.height << 16);
> +	igt_assert(rc == 0);
> +
> +	/* Make sure nothing remains for the other tests. */
> +	rc = drmModeSetPlane(drm_fd, plane_id, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
> +	igt_assert(rc == 0);
> +}
> +
> +/* This one also triggered WARNs on our driver at some point in time. */
> +static void planes_subtest(bool universal, bool dpms)
> +{
> +	int i, rc, planes_tested = 0;
> +	drmModePlaneResPtr planes;
> +
> +	rc = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES,
> +			     universal);
> +	igt_require(rc == 0);

I think you want to make the setcap call conditional on universal.  If
we're running on an older kernel (pre-universal planes), the setcap
ioctl will return -EINVAL since it doesn't recognize
DRM_CLIENT_CAP_UNIVERSAL_PLANES and you'll never even get to try your
non-universal tests.

(I assume you still want to run on older kernels since otherwise I don't
think there'd be a need to test legacy and universal
separately...universal just adds extra stuff to the plane list, but you
still get all the same legacy planes in the list too.)


Matt

> +
> +	planes = drmModeGetPlaneResources(drm_fd);
> +	for (i = 0; i < planes->count_planes; i++) {
> +		drmModePlanePtr plane;
> +
> +		plane = drmModeGetPlane(drm_fd, planes->planes[i]);
> +		igt_assert(plane);
> +
> +		/* We just pick the first CRTC on the list, so we can test for
> +		 * 0x1 as the index. */
> +		if (plane->possible_crtcs & 0x1) {
> +			enum plane_type type;
> +
> +			type = universal ? get_plane_type(plane->plane_id) :
> +					   PLANE_OVERLAY;
> +			test_one_plane(dpms, plane->plane_id, type);
> +			planes_tested++;
> +		}
> +		drmModeFreePlane(plane);
> +	}
> +	drmModeFreePlaneResources(planes);
> +
> +	rc = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
> +	igt_assert(rc == 0);
> +
> +	if (universal)
> +		igt_assert(planes_tested >= 3);
> +	else
> +		igt_assert(planes_tested >= 1);
> +}
> +
>  int rounds = 50;
>  bool stay = false;
>  
> @@ -1577,11 +1779,19 @@ int main(int argc, char *argv[])
>  	igt_subtest("gem-idle")
>  		gem_idle_subtest();
>  
> -	/* Cursors */
> +	/* Planes and cursors */
>  	igt_subtest("cursor")
>  		cursor_subtest(false);
>  	igt_subtest("cursor-dpms")
>  		cursor_subtest(true);
> +	igt_subtest("legacy-planes")
> +		planes_subtest(false, false);
> +	igt_subtest("legacy-planes-dpms")
> +		planes_subtest(false, true);
> +	igt_subtest("universal-planes")
> +		planes_subtest(true, false);
> +	igt_subtest("universal-planes-dpms")
> +		planes_subtest(true, true);
>  
>  	/* Misc */
>  	igt_subtest("reg-read-ioctl")
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795