Re: [PATCH i-g-t] tests/kms_plane: Ensure planes recover from DPMS

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

On Thu, Mar 05, 2015 at 01:32:19PM +0100, Daniel Vetter wrote:
> On Wed, Mar 04, 2015 at 10:50:53AM -0800, Matt Roper wrote:
> > i915 was using the main atomic 'disable plane' to turn off sprite planes
> > during a CRTC disable.  This was problematic because it modified the
> > plane state, preventing us from recovering the original state later.
> > One such case was that during a DPMS OFF followed by a DPMS ON, any
> > sprite planes would not be restored properly.
> > 
> > Let's add a test that toggles DPMS off and on and ensures that the CRC
> > remains the same (i.e., planes are successfully restored unchanged).
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  tests/kms_plane.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> > index 8a08f20..b66ab1d 100644
> > --- a/tests/kms_plane.c
> > +++ b/tests/kms_plane.c
> > @@ -145,6 +145,7 @@ create_fb_for_mode__position(data_t *data, drmModeModeInfo *mode,
> >  
> >  enum {
> >  	TEST_POSITION_FULLY_COVERED = 1 << 0,
> > +	TEST_DPMS = 1 << 1,
> >  };
> >  
> >  static void
> > @@ -158,7 +159,7 @@ test_plane_position_with_output(data_t *data,
> >  	igt_plane_t *primary, *sprite;
> >  	struct igt_fb primary_fb, sprite_fb;
> >  	drmModeModeInfo *mode;
> > -	igt_crc_t crc;
> > +	igt_crc_t crc, crc2;
> >  
> >  	igt_info("Testing connector %s using pipe %s plane %d\n",
> >  		 igt_output_name(output), kmstest_pipe_name(pipe), plane);
> > @@ -194,11 +195,24 @@ test_plane_position_with_output(data_t *data,
> >  
> >  	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> >  
> > +	if (flags & TEST_DPMS) {
> > +		kmstest_set_connector_dpms(data->drm_fd,
> > +					   output->config.connector,
> > +					   DRM_MODE_DPMS_OFF);
> > +		kmstest_set_connector_dpms(data->drm_fd,
> > +					   output->config.connector,
> > +					   DRM_MODE_DPMS_ON);
> > +	}
> 
> I think yet one more subtest to test against system suspend/resume in this
> spot would be good.

We actually already do have a subtest for suspend/resume
("plane-panning-bottom-right-suspend-pipe-%s-plane-%d") which was
probably failing before my kernel patch (although when I tried running
it my system had bigger problems and simply wasn't ever coming back from
suspend).  I just tried it again and it does seem to be running properly
now.

> 
> > +
> > +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc2);
> > +
> >  	if (flags & TEST_POSITION_FULLY_COVERED)
> >  		igt_assert(igt_crc_equal(&test.reference_crc, &crc));
> >  	else
> >  		igt_assert(!igt_crc_equal(&test.reference_crc, &crc));
> >  
> > +	igt_assert(igt_crc_equal(&crc, &crc2));
> > +
> >  	igt_plane_set_fb(primary, NULL);
> >  	igt_plane_set_fb(sprite, NULL);
> >  
> > @@ -358,6 +372,11 @@ run_tests_for_pipe_plane(data_t *data, enum pipe pipe, enum igt_plane plane)
> >  		      kmstest_pipe_name(pipe), plane)
> >  		test_plane_position(data, pipe, plane, 0);
> >  
> > +	igt_subtest_f("plane-position-hole-dpms-pipe-%s-plane-%d",
> 
> Usually we call these -vs-dpms (and -vs-suspend for the suspend/resume
> one).

Yeah, that's what I usually see, but the existing suspend test for
kms_plane lacked the "vs" so I left it off dpms as well for consistency.
Should I update the name of the existing test as well or leave it as is?
I'm not sure if renaming an existing test would cause problems for PRTS
or QA tracking.


Matt

> -Daniel
> 
> > +		      kmstest_pipe_name(pipe), plane)
> > +		test_plane_position(data, pipe, plane,
> > +				    TEST_DPMS);
> > +
> >  	igt_subtest_f("plane-panning-top-left-pipe-%s-plane-%d",
> >  		      kmstest_pipe_name(pipe), plane)
> >  		test_plane_panning(data, pipe, plane, TEST_PANNING_TOP_LEFT);
> > -- 
> > 1.8.5.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx