Hi, Thanks for review comments! > -----Original Message----- > From: Daniel Stone [mailto:daniel@xxxxxxxxxxxxx] > Sent: Wednesday, December 7, 2016 5:02 PM > To: Kahola, Mika <mika.kahola@xxxxxxxxx> > Cc: intel-gfx <intel-gfx@xxxxxxxxxxxxxxxxxxxxx> > Subject: Re: [PATCH i-g-t v2] tests/kms_plane_lowres: Plane visibility after > atomic modesets > > Hi Mika, > Thanks for respinning! > > On 23 November 2016 at 11:49, Mika Kahola <mika.kahola@xxxxxxxxx> wrote: > > +bool kmstest_plane_visible(void) > > +{ > > + char tmp[256]; > > + FILE *fid; > > + bool visible = false; > > + struct kmstest_resolution resolution; > > + const char *mode = "r"; > > + int ret; > > + > > + fid = igt_debugfs_fopen("i915_display_info", mode); > > + > > + igt_assert(fid != NULL); > > This, however, breaks non-Intel drivers: we declare DRIVER_ANY, but later > assert that we can open this file. Maybe what would be better would be to have: > void igt_assert_plane_visible(plane_id, should_be_visible) which would then > internally do an igt_assert() on the result, or > igt_skip() if that file is not present. That's a good point. For non-Intel drivers 'i915_display_info' file doesn't exist. The test could skip if we cannot find the file but then the test would skip all non-Intel drivers. For non-Intel drivers, is there any way to check (debugfs files?) if the resolution change has succeeded and check the plane visibility? > > Also, it seems kind of fragile, as it only looks for the first plane marked 'OVL'. > What happens if we have multiple overlay planes? I had an idea of testing with one plane only but it wouldn't be a major change to use as many OVL planes as we have available. > > Regardless, I think it's very close, and am happy to test a v3 on non-i915. Thanks > a lot for the rework! I'll respin another round! Cheers, Mika > > Cheers, > Daniel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx