igt-dev.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_force_connector_basic: Verify planes are restored after load detection, v2.
Date: Wed, 21 Mar 2018 16:22:46 +0200	[thread overview]
Message-ID: <20180321142246.GZ5453@intel.com> (raw)
In-Reply-To: <20180319170245.60095-1-maarten.lankhorst@linux.intel.com>

On Mon, Mar 19, 2018 at 06:02:45PM +0100, Maarten Lankhorst wrote:
> There's a bug in our load detection in which we don't correctly
> restore planes to their previous states. Strictly verify this
> is the case by setting a fb on all planes.
> 
> Changes since v1:
> - Remove igt_assert(found) in verification, would always fail.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  tests/kms_force_connector_basic.c | 61 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/tests/kms_force_connector_basic.c b/tests/kms_force_connector_basic.c
> index d11a775a089d..26e5d01fc2cf 100644
> --- a/tests/kms_force_connector_basic.c
> +++ b/tests/kms_force_connector_basic.c
> @@ -122,6 +122,14 @@ int main(int argc, char **argv)
>  	}
>  
>  	igt_subtest("force-load-detect") {
> +		int i, j, w = 64, h = 64;
> +		drmModePlaneRes *plane_resources;
> +		struct igt_fb xrgb_fb, argb_fb;
> +
> +		igt_create_fb(drm_fd, w, h, DRM_FORMAT_XRGB8888, 0, &xrgb_fb);
> +		igt_create_fb(drm_fd, w, h, DRM_FORMAT_ARGB8888, 0, &argb_fb);
> +		igt_assert(drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1) == 0);
> +
>  		/*
>  		 * disable all outputs to make sure we have a
>  		 * free crtc available for load detect
> @@ -131,6 +139,42 @@ int main(int argc, char **argv)
>  
>  		igt_set_module_param_int("load_detect_test", 1);
>  
> +		plane_resources = drmModeGetPlaneResources(drm_fd);
> +		igt_assert(plane_resources);
> +
> +		for (i = 0; i < plane_resources->count_planes; i++) {
> +			drmModePlane *drm_plane;
> +			bool found = false;
> +			uint32_t plane_id = plane_resources->planes[i];
> +
> +			drm_plane = drmModeGetPlane(drm_fd, plane_id);
> +			igt_assert(drm_plane);
> +
> +			for (j = 0; j < drm_plane->count_formats; j++) {
> +				uint32_t format = drm_plane->formats[j];
> +				uint32_t crtc = ffs(drm_plane->possible_crtcs) - 1;
> +				uint32_t crtc_id = res->crtcs[crtc];
> +
> +				if (format == DRM_FORMAT_XRGB8888)
> +					do_or_die(drmModeSetPlane(drm_fd, plane_id, crtc_id,
> +							xrgb_fb.fb_id,
> +							0, 0, 0, w, h,
> +							0, 0, IGT_FIXED(w, 0), IGT_FIXED(h, 0)));
> +				else if (format == DRM_FORMAT_ARGB8888)
> +					do_or_die(drmModeSetPlane(drm_fd, plane_id, crtc_id,
> +							argb_fb.fb_id,
> +							0, 0, 0, w, h,
> +							0, 0, IGT_FIXED(w, 0), IGT_FIXED(h, 0)));
> +				else
> +					continue;
> +
> +				found = true;
> +				break;
> +			}
> +			drmModeFreePlane(drm_plane);
> +			igt_assert(found);
> +		}
> +
>  		/* This can't use drmModeGetConnectorCurrent
>  		 * because connector probing is the point of this test.
>  		 */
> @@ -141,6 +185,23 @@ int main(int argc, char **argv)
>  		igt_assert(temp->connection != DRM_MODE_UNKNOWNCONNECTION);
>  
>  		drmModeFreeConnector(temp);
> +
> +		/* Look if planes are unmodified. */
> +		for (i = 0; i < plane_resources->count_planes; i++) {
> +			drmModePlane *drm_plane;
> +
> +			drm_plane = drmModeGetPlane(drm_fd,
> +						    plane_resources->planes[i]);
> +			igt_assert(drm_plane);
> +
> +			igt_assert(drm_plane->crtc_id);
> +			igt_assert(drm_plane->fb_id);
> +
> +			if (drm_plane->fb_id != xrgb_fb.fb_id)
> +				igt_assert_eq(drm_plane->fb_id, argb_fb.fb_id);

Maybe just 'igt_assert(fb_id == a || fb_id == b)'?

Either way this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now we just need to get the leak fixes to the kernel.

> +
> +			drmModeFreePlane(drm_plane);
> +		}
>  	}
>  
>  	igt_subtest("force-connector-state") {
> -- 
> 2.16.2

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2018-03-21 14:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-19 16:55 [igt-dev] [PATCH i-g-t] tests/kms_force_connector_basic: Verify planes are restored after load detection Maarten Lankhorst
2018-03-19 17:02 ` [igt-dev] [PATCH i-g-t] tests/kms_force_connector_basic: Verify planes are restored after load detection, v2 Maarten Lankhorst
2018-03-21 14:22   ` Ville Syrjälä [this message]
2018-08-10 10:16     ` Maarten Lankhorst
2018-03-19 21:28 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_force_connector_basic: Verify planes are restored after load detection (rev2) Patchwork
2018-07-25 12:22 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
2018-07-25 13:50 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180321142246.GZ5453@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).