| Message ID | 1448957286-25655-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 1 December 2015 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Two things: > - Somehow the kernel's mode list changed with our EDID. No idea > whether that's the right thing here since I'm not really an EDID > expert. But then again the testcase wants to check that the > injection works, not validate the kernel's parser. If only checking that the injection worked, perhaps it would be more robust just to check that at least one of the expected modes is present, in any position in the array. Ideally this would be a mode that isn't included in the default modes when a connector is enabled. > > - We need to disable the forcing _before_ we reprobe to check whether > everything is back to normal: With the EDID gone but the connection > still force to on the kernel will fall back to a default low-res > mode list, making the testcase fail. This should be taken into account with the value of start_n_modes, since this is calculated after the connector is enabled. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > tests/kms_force_connector.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c > index 838a3b0ae2e6..5f3f4a5400f5 100644 > --- a/tests/kms_force_connector.c > +++ b/tests/kms_force_connector.c > @@ -153,20 +153,22 @@ int main(int argc, char **argv) > CHECK_MODE(temp->modes[1], 1280, 720, 60); > CHECK_MODE(temp->modes[2], 1024, 768, 60); > CHECK_MODE(temp->modes[3], 800, 600, 60); > - CHECK_MODE(temp->modes[4], 640, 480, 60); > + CHECK_MODE(temp->modes[4], 800, 600, 56); > + CHECK_MODE(temp->modes[5], 848, 480, 60); > + CHECK_MODE(temp->modes[6], 640, 480, 60); > > drmModeFreeConnector(temp); > > /* remove edid */ > kmstest_force_edid(drm_fd, vga_connector, NULL, 0); > + kmstest_force_connector(drm_fd, vga_connector, > + FORCE_CONNECTOR_UNSPECIFIED); > temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); > /* the connector should now have the same number of modes that > * it started with */ > igt_assert_eq(temp->count_modes, start_n_modes); > drmModeFreeConnector(temp); > > - kmstest_force_connector(drm_fd, vga_connector, > - FORCE_CONNECTOR_UNSPECIFIED); > } > > igt_fixture { > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Dec 01, 2015 at 10:11:14AM +0000, Thomas Wood wrote: > On 1 December 2015 at 08:08, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Two things: > > - Somehow the kernel's mode list changed with our EDID. No idea > > whether that's the right thing here since I'm not really an EDID > > expert. But then again the testcase wants to check that the > > injection works, not validate the kernel's parser. > > If only checking that the injection worked, perhaps it would be more > robust just to check that at least one of the expected modes is > present, in any position in the array. Ideally this would be a mode > that isn't included in the default modes when a connector is enabled. One option would be to check for the 1920x1080 mode only. That's the preferred one, so really must be first. And it's way more high-res than any of the default modes we inject (those top out at 1024x768). Ok with that? > > - We need to disable the forcing _before_ we reprobe to check whether > > everything is back to normal: With the EDID gone but the connection > > still force to on the kernel will fall back to a default low-res > > mode list, making the testcase fail. > > This should be taken into account with the value of start_n_modes, > since this is calculated after the connector is enabled. Oops, I did accidentally break this in my GetConnectorCurrent patch, since there I consolidated them all to avoid overhead. And didn't notice because of the earlier breakage. I'll respin these two and shift the 2nd change to the 2nd patch. -Daniel
diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c index 838a3b0ae2e6..5f3f4a5400f5 100644 --- a/tests/kms_force_connector.c +++ b/tests/kms_force_connector.c @@ -153,20 +153,22 @@ int main(int argc, char **argv) CHECK_MODE(temp->modes[1], 1280, 720, 60); CHECK_MODE(temp->modes[2], 1024, 768, 60); CHECK_MODE(temp->modes[3], 800, 600, 60); - CHECK_MODE(temp->modes[4], 640, 480, 60); + CHECK_MODE(temp->modes[4], 800, 600, 56); + CHECK_MODE(temp->modes[5], 848, 480, 60); + CHECK_MODE(temp->modes[6], 640, 480, 60); drmModeFreeConnector(temp); /* remove edid */ kmstest_force_edid(drm_fd, vga_connector, NULL, 0); + kmstest_force_connector(drm_fd, vga_connector, + FORCE_CONNECTOR_UNSPECIFIED); temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); /* the connector should now have the same number of modes that * it started with */ igt_assert_eq(temp->count_modes, start_n_modes); drmModeFreeConnector(temp); - kmstest_force_connector(drm_fd, vga_connector, - FORCE_CONNECTOR_UNSPECIFIED); } igt_fixture {
Two things: - Somehow the kernel's mode list changed with our EDID. No idea whether that's the right thing here since I'm not really an EDID expert. But then again the testcase wants to check that the injection works, not validate the kernel's parser. - We need to disable the forcing _before_ we reprobe to check whether everything is back to normal: With the EDID gone but the connection still force to on the kernel will fall back to a default low-res mode list, making the testcase fail. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- tests/kms_force_connector.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)