diff mbox

[i-g-t,1/3] tests/kms_force_connector: Fixes

Message ID 1448965460-32257-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Dec. 1, 2015, 10:24 a.m. UTC
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.

v2: Only check the preferred mode for more future-proofing (Thomas).

Cc: Thomas Wood <thomas.wood@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/kms_force_connector.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Thomas Wood Dec. 1, 2015, 10:33 a.m. UTC | #1
On 1 December 2015 at 10:24, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Forcing connector state is a basic piece of our test infrastructure
> that we use in all the kms_ tests. It allows us to run tests even if
> no outputs are connected.
>
> They're also really fast, so perfect candidates for inclusion into the
> BAT set.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/.gitignore                  |   2 +-
>  tests/Makefile.sources            |   2 +-
>  tests/kms_force_connector.c       | 189 --------------------------------------
>  tests/kms_force_connector_basic.c | 189 ++++++++++++++++++++++++++++++++++++++

Would it be worth renaming the subtests instead, in case there are
future tests that would make sense in kms_force_connector but that
shouldn't be included in the BAT list?


>  4 files changed, 191 insertions(+), 191 deletions(-)
>  delete mode 100644 tests/kms_force_connector.c
>  create mode 100644 tests/kms_force_connector_basic.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 43d63d3abac9..900c681caec9 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -138,7 +138,7 @@ kms_fence_pin_leak
>  kms_flip
>  kms_flip_event_leak
>  kms_flip_tiling
> -kms_force_connector
> +kms_force_connector_basic
>  kms_frontbuffer_tracking
>  kms_legacy_colorkey
>  kms_mmio_vs_cs_flip
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index b70bca060253..63de439d8717 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -163,7 +163,7 @@ TESTS_progs = \
>         gen7_forcewake_mt \
>         kms_3d \
>         kms_fence_pin_leak \
> -       kms_force_connector \
> +       kms_force_connector_basic \
>         kms_pwrite_crc \
>         kms_sink_crc_basic \
>         prime_udl \
> diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c
> deleted file mode 100644
> index 637f625a852f..000000000000
> --- a/tests/kms_force_connector.c
> +++ /dev/null
> @@ -1,189 +0,0 @@
> -/*
> - * Copyright © 2014 Intel Corporation
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> - * IN THE SOFTWARE.
> - *
> - */
> -
> -#include "igt.h"
> -
> -IGT_TEST_DESCRIPTION("Check the debugfs force connector/edid features work"
> -                    " correctly.");
> -
> -#define CHECK_MODE(m, h, w, r) \
> -       igt_assert_eq(m.hdisplay, h); igt_assert_eq(m.vdisplay, w); \
> -       igt_assert_eq(m.vrefresh, r);
> -
> -static void reset_connectors(void)
> -{
> -       int drm_fd = 0;
> -       drmModeRes *res;
> -       drmModeConnector *connector = NULL;
> -
> -       drm_fd = drm_open_driver_master(DRIVER_INTEL);
> -       res = drmModeGetResources(drm_fd);
> -
> -       for (int i = 0; i < res->count_connectors; i++) {
> -
> -               connector = drmModeGetConnectorCurrent(drm_fd,
> -                                                      res->connectors[i]);
> -
> -               kmstest_force_connector(drm_fd, connector,
> -                                       FORCE_CONNECTOR_UNSPECIFIED);
> -
> -               kmstest_force_edid(drm_fd, connector, NULL, 0);
> -
> -               drmModeFreeConnector(connector);
> -       }
> -}
> -
> -static int opt_handler(int opt, int opt_index, void *data)
> -{
> -       switch (opt) {
> -       case 'r':
> -               reset_connectors();
> -               exit(0);
> -               break;
> -       }
> -
> -       return 0;
> -}
> -
> -int main(int argc, char **argv)
> -{
> -       /* force the VGA output and test that it worked */
> -       int drm_fd = 0;
> -       drmModeRes *res;
> -       drmModeConnector *vga_connector = NULL, *temp;
> -       int start_n_modes, start_connection;
> -       struct option long_opts[] = {
> -               {"reset", 0, 0, 'r'},
> -               {0, 0, 0, 0}
> -       };
> -       const char *help_str =
> -              "  --reset\t\tReset all connector force states and edid.\n";
> -
> -       igt_subtest_init_parse_opts(&argc, argv, "", long_opts, help_str,
> -                                   opt_handler, NULL);
> -
> -       igt_fixture {
> -               drm_fd = drm_open_driver_master(DRIVER_INTEL);
> -               res = drmModeGetResources(drm_fd);
> -
> -               /* find the vga connector */
> -               for (int i = 0; i < res->count_connectors; i++) {
> -
> -                       vga_connector = drmModeGetConnectorCurrent(drm_fd,
> -                                                                  res->connectors[i]);
> -
> -                       if (vga_connector->connector_type == DRM_MODE_CONNECTOR_VGA) {
> -                               start_n_modes = vga_connector->count_modes;
> -                               start_connection = vga_connector->connection;
> -                               break;
> -                       }
> -
> -                       drmModeFreeConnector(vga_connector);
> -
> -                       vga_connector = NULL;
> -               }
> -
> -               igt_require(vga_connector);
> -               igt_skip_on(vga_connector->connection == DRM_MODE_CONNECTED);
> -       }
> -
> -       igt_subtest("force-connector-state") {
> -               igt_display_t display;
> -
> -               /* force the connector on and check the reported values */
> -               kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_ON);
> -               temp = drmModeGetConnectorCurrent(drm_fd,
> -                                                 vga_connector->connector_id);
> -               igt_assert_eq(temp->connection, DRM_MODE_CONNECTED);
> -               igt_assert_lt(0, temp->count_modes);
> -               drmModeFreeConnector(temp);
> -
> -               /* attempt to use the display */
> -               kmstest_set_vt_graphics_mode();
> -               igt_display_init(&display, drm_fd);
> -               igt_display_commit(&display);
> -               igt_display_fini(&display);
> -
> -
> -               /* force the connector off */
> -               kmstest_force_connector(drm_fd, vga_connector,
> -                                       FORCE_CONNECTOR_OFF);
> -               temp = drmModeGetConnectorCurrent(drm_fd,
> -                                                 vga_connector->connector_id);
> -               igt_assert_eq(temp->connection, DRM_MODE_DISCONNECTED);
> -               igt_assert_eq(0, temp->count_modes);
> -               drmModeFreeConnector(temp);
> -
> -               /* check that the previous state is restored */
> -               kmstest_force_connector(drm_fd, vga_connector,
> -                                       FORCE_CONNECTOR_UNSPECIFIED);
> -               temp = drmModeGetConnectorCurrent(drm_fd,
> -                                                 vga_connector->connector_id);
> -               igt_assert_eq(temp->connection, start_connection);
> -               drmModeFreeConnector(temp);
> -       }
> -
> -       igt_subtest("force-edid") {
> -               kmstest_force_connector(drm_fd, vga_connector,
> -                                       FORCE_CONNECTOR_ON);
> -               temp = drmModeGetConnectorCurrent(drm_fd,
> -                                                 vga_connector->connector_id);
> -               drmModeFreeConnector(temp);
> -
> -               /* test edid forcing */
> -               kmstest_force_edid(drm_fd, vga_connector,
> -                                  igt_kms_get_base_edid(), EDID_LENGTH);
> -               temp = drmModeGetConnectorCurrent(drm_fd,
> -                                                 vga_connector->connector_id);
> -
> -               igt_debug("num_conn %i\n", temp->count_modes);
> -
> -               CHECK_MODE(temp->modes[0], 1920, 1080, 60);
> -               /* Don't check non-preferred modes to avoid to tight coupling
> -                * with the in-kernel EDID parser. */
> -
> -               drmModeFreeConnector(temp);
> -
> -               /* remove edid */
> -               kmstest_force_edid(drm_fd, vga_connector, NULL, 0);
> -               kmstest_force_connector(drm_fd, vga_connector,
> -                                       FORCE_CONNECTOR_UNSPECIFIED);
> -               temp = drmModeGetConnectorCurrent(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);
> -
> -       }
> -
> -       igt_fixture {
> -               drmModeFreeConnector(vga_connector);
> -               close(drm_fd);
> -
> -               reset_connectors();
> -       }
> -
> -       igt_exit();
> -}
> diff --git a/tests/kms_force_connector_basic.c b/tests/kms_force_connector_basic.c
> new file mode 100644
> index 000000000000..637f625a852f
> --- /dev/null
> +++ b/tests/kms_force_connector_basic.c
> @@ -0,0 +1,189 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt.h"
> +
> +IGT_TEST_DESCRIPTION("Check the debugfs force connector/edid features work"
> +                    " correctly.");
> +
> +#define CHECK_MODE(m, h, w, r) \
> +       igt_assert_eq(m.hdisplay, h); igt_assert_eq(m.vdisplay, w); \
> +       igt_assert_eq(m.vrefresh, r);
> +
> +static void reset_connectors(void)
> +{
> +       int drm_fd = 0;
> +       drmModeRes *res;
> +       drmModeConnector *connector = NULL;
> +
> +       drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +       res = drmModeGetResources(drm_fd);
> +
> +       for (int i = 0; i < res->count_connectors; i++) {
> +
> +               connector = drmModeGetConnectorCurrent(drm_fd,
> +                                                      res->connectors[i]);
> +
> +               kmstest_force_connector(drm_fd, connector,
> +                                       FORCE_CONNECTOR_UNSPECIFIED);
> +
> +               kmstest_force_edid(drm_fd, connector, NULL, 0);
> +
> +               drmModeFreeConnector(connector);
> +       }
> +}
> +
> +static int opt_handler(int opt, int opt_index, void *data)
> +{
> +       switch (opt) {
> +       case 'r':
> +               reset_connectors();
> +               exit(0);
> +               break;
> +       }
> +
> +       return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +       /* force the VGA output and test that it worked */
> +       int drm_fd = 0;
> +       drmModeRes *res;
> +       drmModeConnector *vga_connector = NULL, *temp;
> +       int start_n_modes, start_connection;
> +       struct option long_opts[] = {
> +               {"reset", 0, 0, 'r'},
> +               {0, 0, 0, 0}
> +       };
> +       const char *help_str =
> +              "  --reset\t\tReset all connector force states and edid.\n";
> +
> +       igt_subtest_init_parse_opts(&argc, argv, "", long_opts, help_str,
> +                                   opt_handler, NULL);
> +
> +       igt_fixture {
> +               drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +               res = drmModeGetResources(drm_fd);
> +
> +               /* find the vga connector */
> +               for (int i = 0; i < res->count_connectors; i++) {
> +
> +                       vga_connector = drmModeGetConnectorCurrent(drm_fd,
> +                                                                  res->connectors[i]);
> +
> +                       if (vga_connector->connector_type == DRM_MODE_CONNECTOR_VGA) {
> +                               start_n_modes = vga_connector->count_modes;
> +                               start_connection = vga_connector->connection;
> +                               break;
> +                       }
> +
> +                       drmModeFreeConnector(vga_connector);
> +
> +                       vga_connector = NULL;
> +               }
> +
> +               igt_require(vga_connector);
> +               igt_skip_on(vga_connector->connection == DRM_MODE_CONNECTED);
> +       }
> +
> +       igt_subtest("force-connector-state") {
> +               igt_display_t display;
> +
> +               /* force the connector on and check the reported values */
> +               kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_ON);
> +               temp = drmModeGetConnectorCurrent(drm_fd,
> +                                                 vga_connector->connector_id);
> +               igt_assert_eq(temp->connection, DRM_MODE_CONNECTED);
> +               igt_assert_lt(0, temp->count_modes);
> +               drmModeFreeConnector(temp);
> +
> +               /* attempt to use the display */
> +               kmstest_set_vt_graphics_mode();
> +               igt_display_init(&display, drm_fd);
> +               igt_display_commit(&display);
> +               igt_display_fini(&display);
> +
> +
> +               /* force the connector off */
> +               kmstest_force_connector(drm_fd, vga_connector,
> +                                       FORCE_CONNECTOR_OFF);
> +               temp = drmModeGetConnectorCurrent(drm_fd,
> +                                                 vga_connector->connector_id);
> +               igt_assert_eq(temp->connection, DRM_MODE_DISCONNECTED);
> +               igt_assert_eq(0, temp->count_modes);
> +               drmModeFreeConnector(temp);
> +
> +               /* check that the previous state is restored */
> +               kmstest_force_connector(drm_fd, vga_connector,
> +                                       FORCE_CONNECTOR_UNSPECIFIED);
> +               temp = drmModeGetConnectorCurrent(drm_fd,
> +                                                 vga_connector->connector_id);
> +               igt_assert_eq(temp->connection, start_connection);
> +               drmModeFreeConnector(temp);
> +       }
> +
> +       igt_subtest("force-edid") {
> +               kmstest_force_connector(drm_fd, vga_connector,
> +                                       FORCE_CONNECTOR_ON);
> +               temp = drmModeGetConnectorCurrent(drm_fd,
> +                                                 vga_connector->connector_id);
> +               drmModeFreeConnector(temp);
> +
> +               /* test edid forcing */
> +               kmstest_force_edid(drm_fd, vga_connector,
> +                                  igt_kms_get_base_edid(), EDID_LENGTH);
> +               temp = drmModeGetConnectorCurrent(drm_fd,
> +                                                 vga_connector->connector_id);
> +
> +               igt_debug("num_conn %i\n", temp->count_modes);
> +
> +               CHECK_MODE(temp->modes[0], 1920, 1080, 60);
> +               /* Don't check non-preferred modes to avoid to tight coupling
> +                * with the in-kernel EDID parser. */
> +
> +               drmModeFreeConnector(temp);
> +
> +               /* remove edid */
> +               kmstest_force_edid(drm_fd, vga_connector, NULL, 0);
> +               kmstest_force_connector(drm_fd, vga_connector,
> +                                       FORCE_CONNECTOR_UNSPECIFIED);
> +               temp = drmModeGetConnectorCurrent(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);
> +
> +       }
> +
> +       igt_fixture {
> +               drmModeFreeConnector(vga_connector);
> +               close(drm_fd);
> +
> +               reset_connectors();
> +       }
> +
> +       igt_exit();
> +}
> --
> 2.1.0
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Jani Nikula Dec. 1, 2015, 11:12 a.m. UTC | #2
On Tue, 01 Dec 2015, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Somehow the kernel's mode list changed with our EDID.

I do not undestand what you're trying to say here.

BR,
Jani.
Daniel Vetter Dec. 1, 2015, 1:38 p.m. UTC | #3
On Tue, Dec 01, 2015 at 01:12:43PM +0200, Jani Nikula wrote:
> On Tue, 01 Dec 2015, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Somehow the kernel's mode list changed with our EDID.
> 
> I do not undestand what you're trying to say here.

The EDID we injected stayed the same, but the kernel started listing more
modes for it. I didn't do a bisect nor analyse the root-cause since that
would mean decoding the edid by hand to double-check what's going on.

Suggestions for better phrasing?
-Daniel
Daniel Vetter Dec. 1, 2015, 1:39 p.m. UTC | #4
On Tue, Dec 01, 2015 at 10:33:22AM +0000, Thomas Wood wrote:
> On 1 December 2015 at 10:24, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Forcing connector state is a basic piece of our test infrastructure
> > that we use in all the kms_ tests. It allows us to run tests even if
> > no outputs are connected.
> >
> > They're also really fast, so perfect candidates for inclusion into the
> > BAT set.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  tests/.gitignore                  |   2 +-
> >  tests/Makefile.sources            |   2 +-
> >  tests/kms_force_connector.c       | 189 --------------------------------------
> >  tests/kms_force_connector_basic.c | 189 ++++++++++++++++++++++++++++++++++++++
> 
> Would it be worth renaming the subtests instead, in case there are
> future tests that would make sense in kms_force_connector but that
> shouldn't be included in the BAT list?

If we in the future can inject hdmi screens (or maybe even DP) I think
that should still stay a BAT testcase since it's basic test
infrastructure. Same reason for why all the pipe CRC and sink CRC tests
are in BAT in their entirety - if any of that fails large amounts of
testcase results will be highly suspect. That's why I opted to rename the
test binary, not the subtests.
-Daniel

> 
> 
> >  4 files changed, 191 insertions(+), 191 deletions(-)
> >  delete mode 100644 tests/kms_force_connector.c
> >  create mode 100644 tests/kms_force_connector_basic.c
> >
> > diff --git a/tests/.gitignore b/tests/.gitignore
> > index 43d63d3abac9..900c681caec9 100644
> > --- a/tests/.gitignore
> > +++ b/tests/.gitignore
> > @@ -138,7 +138,7 @@ kms_fence_pin_leak
> >  kms_flip
> >  kms_flip_event_leak
> >  kms_flip_tiling
> > -kms_force_connector
> > +kms_force_connector_basic
> >  kms_frontbuffer_tracking
> >  kms_legacy_colorkey
> >  kms_mmio_vs_cs_flip
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index b70bca060253..63de439d8717 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -163,7 +163,7 @@ TESTS_progs = \
> >         gen7_forcewake_mt \
> >         kms_3d \
> >         kms_fence_pin_leak \
> > -       kms_force_connector \
> > +       kms_force_connector_basic \
> >         kms_pwrite_crc \
> >         kms_sink_crc_basic \
> >         prime_udl \
> > diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c
> > deleted file mode 100644
> > index 637f625a852f..000000000000
> > --- a/tests/kms_force_connector.c
> > +++ /dev/null
> > @@ -1,189 +0,0 @@
> > -/*
> > - * Copyright © 2014 Intel Corporation
> > - *
> > - * Permission is hereby granted, free of charge, to any person obtaining a
> > - * copy of this software and associated documentation files (the "Software"),
> > - * to deal in the Software without restriction, including without limitation
> > - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > - * and/or sell copies of the Software, and to permit persons to whom the
> > - * Software is furnished to do so, subject to the following conditions:
> > - *
> > - * The above copyright notice and this permission notice (including the next
> > - * paragraph) shall be included in all copies or substantial portions of the
> > - * Software.
> > - *
> > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > - * IN THE SOFTWARE.
> > - *
> > - */
> > -
> > -#include "igt.h"
> > -
> > -IGT_TEST_DESCRIPTION("Check the debugfs force connector/edid features work"
> > -                    " correctly.");
> > -
> > -#define CHECK_MODE(m, h, w, r) \
> > -       igt_assert_eq(m.hdisplay, h); igt_assert_eq(m.vdisplay, w); \
> > -       igt_assert_eq(m.vrefresh, r);
> > -
> > -static void reset_connectors(void)
> > -{
> > -       int drm_fd = 0;
> > -       drmModeRes *res;
> > -       drmModeConnector *connector = NULL;
> > -
> > -       drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > -       res = drmModeGetResources(drm_fd);
> > -
> > -       for (int i = 0; i < res->count_connectors; i++) {
> > -
> > -               connector = drmModeGetConnectorCurrent(drm_fd,
> > -                                                      res->connectors[i]);
> > -
> > -               kmstest_force_connector(drm_fd, connector,
> > -                                       FORCE_CONNECTOR_UNSPECIFIED);
> > -
> > -               kmstest_force_edid(drm_fd, connector, NULL, 0);
> > -
> > -               drmModeFreeConnector(connector);
> > -       }
> > -}
> > -
> > -static int opt_handler(int opt, int opt_index, void *data)
> > -{
> > -       switch (opt) {
> > -       case 'r':
> > -               reset_connectors();
> > -               exit(0);
> > -               break;
> > -       }
> > -
> > -       return 0;
> > -}
> > -
> > -int main(int argc, char **argv)
> > -{
> > -       /* force the VGA output and test that it worked */
> > -       int drm_fd = 0;
> > -       drmModeRes *res;
> > -       drmModeConnector *vga_connector = NULL, *temp;
> > -       int start_n_modes, start_connection;
> > -       struct option long_opts[] = {
> > -               {"reset", 0, 0, 'r'},
> > -               {0, 0, 0, 0}
> > -       };
> > -       const char *help_str =
> > -              "  --reset\t\tReset all connector force states and edid.\n";
> > -
> > -       igt_subtest_init_parse_opts(&argc, argv, "", long_opts, help_str,
> > -                                   opt_handler, NULL);
> > -
> > -       igt_fixture {
> > -               drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > -               res = drmModeGetResources(drm_fd);
> > -
> > -               /* find the vga connector */
> > -               for (int i = 0; i < res->count_connectors; i++) {
> > -
> > -                       vga_connector = drmModeGetConnectorCurrent(drm_fd,
> > -                                                                  res->connectors[i]);
> > -
> > -                       if (vga_connector->connector_type == DRM_MODE_CONNECTOR_VGA) {
> > -                               start_n_modes = vga_connector->count_modes;
> > -                               start_connection = vga_connector->connection;
> > -                               break;
> > -                       }
> > -
> > -                       drmModeFreeConnector(vga_connector);
> > -
> > -                       vga_connector = NULL;
> > -               }
> > -
> > -               igt_require(vga_connector);
> > -               igt_skip_on(vga_connector->connection == DRM_MODE_CONNECTED);
> > -       }
> > -
> > -       igt_subtest("force-connector-state") {
> > -               igt_display_t display;
> > -
> > -               /* force the connector on and check the reported values */
> > -               kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_ON);
> > -               temp = drmModeGetConnectorCurrent(drm_fd,
> > -                                                 vga_connector->connector_id);
> > -               igt_assert_eq(temp->connection, DRM_MODE_CONNECTED);
> > -               igt_assert_lt(0, temp->count_modes);
> > -               drmModeFreeConnector(temp);
> > -
> > -               /* attempt to use the display */
> > -               kmstest_set_vt_graphics_mode();
> > -               igt_display_init(&display, drm_fd);
> > -               igt_display_commit(&display);
> > -               igt_display_fini(&display);
> > -
> > -
> > -               /* force the connector off */
> > -               kmstest_force_connector(drm_fd, vga_connector,
> > -                                       FORCE_CONNECTOR_OFF);
> > -               temp = drmModeGetConnectorCurrent(drm_fd,
> > -                                                 vga_connector->connector_id);
> > -               igt_assert_eq(temp->connection, DRM_MODE_DISCONNECTED);
> > -               igt_assert_eq(0, temp->count_modes);
> > -               drmModeFreeConnector(temp);
> > -
> > -               /* check that the previous state is restored */
> > -               kmstest_force_connector(drm_fd, vga_connector,
> > -                                       FORCE_CONNECTOR_UNSPECIFIED);
> > -               temp = drmModeGetConnectorCurrent(drm_fd,
> > -                                                 vga_connector->connector_id);
> > -               igt_assert_eq(temp->connection, start_connection);
> > -               drmModeFreeConnector(temp);
> > -       }
> > -
> > -       igt_subtest("force-edid") {
> > -               kmstest_force_connector(drm_fd, vga_connector,
> > -                                       FORCE_CONNECTOR_ON);
> > -               temp = drmModeGetConnectorCurrent(drm_fd,
> > -                                                 vga_connector->connector_id);
> > -               drmModeFreeConnector(temp);
> > -
> > -               /* test edid forcing */
> > -               kmstest_force_edid(drm_fd, vga_connector,
> > -                                  igt_kms_get_base_edid(), EDID_LENGTH);
> > -               temp = drmModeGetConnectorCurrent(drm_fd,
> > -                                                 vga_connector->connector_id);
> > -
> > -               igt_debug("num_conn %i\n", temp->count_modes);
> > -
> > -               CHECK_MODE(temp->modes[0], 1920, 1080, 60);
> > -               /* Don't check non-preferred modes to avoid to tight coupling
> > -                * with the in-kernel EDID parser. */
> > -
> > -               drmModeFreeConnector(temp);
> > -
> > -               /* remove edid */
> > -               kmstest_force_edid(drm_fd, vga_connector, NULL, 0);
> > -               kmstest_force_connector(drm_fd, vga_connector,
> > -                                       FORCE_CONNECTOR_UNSPECIFIED);
> > -               temp = drmModeGetConnectorCurrent(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);
> > -
> > -       }
> > -
> > -       igt_fixture {
> > -               drmModeFreeConnector(vga_connector);
> > -               close(drm_fd);
> > -
> > -               reset_connectors();
> > -       }
> > -
> > -       igt_exit();
> > -}
> > diff --git a/tests/kms_force_connector_basic.c b/tests/kms_force_connector_basic.c
> > new file mode 100644
> > index 000000000000..637f625a852f
> > --- /dev/null
> > +++ b/tests/kms_force_connector_basic.c
> > @@ -0,0 +1,189 @@
> > +/*
> > + * Copyright © 2014 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + */
> > +
> > +#include "igt.h"
> > +
> > +IGT_TEST_DESCRIPTION("Check the debugfs force connector/edid features work"
> > +                    " correctly.");
> > +
> > +#define CHECK_MODE(m, h, w, r) \
> > +       igt_assert_eq(m.hdisplay, h); igt_assert_eq(m.vdisplay, w); \
> > +       igt_assert_eq(m.vrefresh, r);
> > +
> > +static void reset_connectors(void)
> > +{
> > +       int drm_fd = 0;
> > +       drmModeRes *res;
> > +       drmModeConnector *connector = NULL;
> > +
> > +       drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > +       res = drmModeGetResources(drm_fd);
> > +
> > +       for (int i = 0; i < res->count_connectors; i++) {
> > +
> > +               connector = drmModeGetConnectorCurrent(drm_fd,
> > +                                                      res->connectors[i]);
> > +
> > +               kmstest_force_connector(drm_fd, connector,
> > +                                       FORCE_CONNECTOR_UNSPECIFIED);
> > +
> > +               kmstest_force_edid(drm_fd, connector, NULL, 0);
> > +
> > +               drmModeFreeConnector(connector);
> > +       }
> > +}
> > +
> > +static int opt_handler(int opt, int opt_index, void *data)
> > +{
> > +       switch (opt) {
> > +       case 'r':
> > +               reset_connectors();
> > +               exit(0);
> > +               break;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +       /* force the VGA output and test that it worked */
> > +       int drm_fd = 0;
> > +       drmModeRes *res;
> > +       drmModeConnector *vga_connector = NULL, *temp;
> > +       int start_n_modes, start_connection;
> > +       struct option long_opts[] = {
> > +               {"reset", 0, 0, 'r'},
> > +               {0, 0, 0, 0}
> > +       };
> > +       const char *help_str =
> > +              "  --reset\t\tReset all connector force states and edid.\n";
> > +
> > +       igt_subtest_init_parse_opts(&argc, argv, "", long_opts, help_str,
> > +                                   opt_handler, NULL);
> > +
> > +       igt_fixture {
> > +               drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > +               res = drmModeGetResources(drm_fd);
> > +
> > +               /* find the vga connector */
> > +               for (int i = 0; i < res->count_connectors; i++) {
> > +
> > +                       vga_connector = drmModeGetConnectorCurrent(drm_fd,
> > +                                                                  res->connectors[i]);
> > +
> > +                       if (vga_connector->connector_type == DRM_MODE_CONNECTOR_VGA) {
> > +                               start_n_modes = vga_connector->count_modes;
> > +                               start_connection = vga_connector->connection;
> > +                               break;
> > +                       }
> > +
> > +                       drmModeFreeConnector(vga_connector);
> > +
> > +                       vga_connector = NULL;
> > +               }
> > +
> > +               igt_require(vga_connector);
> > +               igt_skip_on(vga_connector->connection == DRM_MODE_CONNECTED);
> > +       }
> > +
> > +       igt_subtest("force-connector-state") {
> > +               igt_display_t display;
> > +
> > +               /* force the connector on and check the reported values */
> > +               kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_ON);
> > +               temp = drmModeGetConnectorCurrent(drm_fd,
> > +                                                 vga_connector->connector_id);
> > +               igt_assert_eq(temp->connection, DRM_MODE_CONNECTED);
> > +               igt_assert_lt(0, temp->count_modes);
> > +               drmModeFreeConnector(temp);
> > +
> > +               /* attempt to use the display */
> > +               kmstest_set_vt_graphics_mode();
> > +               igt_display_init(&display, drm_fd);
> > +               igt_display_commit(&display);
> > +               igt_display_fini(&display);
> > +
> > +
> > +               /* force the connector off */
> > +               kmstest_force_connector(drm_fd, vga_connector,
> > +                                       FORCE_CONNECTOR_OFF);
> > +               temp = drmModeGetConnectorCurrent(drm_fd,
> > +                                                 vga_connector->connector_id);
> > +               igt_assert_eq(temp->connection, DRM_MODE_DISCONNECTED);
> > +               igt_assert_eq(0, temp->count_modes);
> > +               drmModeFreeConnector(temp);
> > +
> > +               /* check that the previous state is restored */
> > +               kmstest_force_connector(drm_fd, vga_connector,
> > +                                       FORCE_CONNECTOR_UNSPECIFIED);
> > +               temp = drmModeGetConnectorCurrent(drm_fd,
> > +                                                 vga_connector->connector_id);
> > +               igt_assert_eq(temp->connection, start_connection);
> > +               drmModeFreeConnector(temp);
> > +       }
> > +
> > +       igt_subtest("force-edid") {
> > +               kmstest_force_connector(drm_fd, vga_connector,
> > +                                       FORCE_CONNECTOR_ON);
> > +               temp = drmModeGetConnectorCurrent(drm_fd,
> > +                                                 vga_connector->connector_id);
> > +               drmModeFreeConnector(temp);
> > +
> > +               /* test edid forcing */
> > +               kmstest_force_edid(drm_fd, vga_connector,
> > +                                  igt_kms_get_base_edid(), EDID_LENGTH);
> > +               temp = drmModeGetConnectorCurrent(drm_fd,
> > +                                                 vga_connector->connector_id);
> > +
> > +               igt_debug("num_conn %i\n", temp->count_modes);
> > +
> > +               CHECK_MODE(temp->modes[0], 1920, 1080, 60);
> > +               /* Don't check non-preferred modes to avoid to tight coupling
> > +                * with the in-kernel EDID parser. */
> > +
> > +               drmModeFreeConnector(temp);
> > +
> > +               /* remove edid */
> > +               kmstest_force_edid(drm_fd, vga_connector, NULL, 0);
> > +               kmstest_force_connector(drm_fd, vga_connector,
> > +                                       FORCE_CONNECTOR_UNSPECIFIED);
> > +               temp = drmModeGetConnectorCurrent(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);
> > +
> > +       }
> > +
> > +       igt_fixture {
> > +               drmModeFreeConnector(vga_connector);
> > +               close(drm_fd);
> > +
> > +               reset_connectors();
> > +       }
> > +
> > +       igt_exit();
> > +}
> > --
> > 2.1.0
> >
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
Jani Nikula Dec. 2, 2015, 11:50 a.m. UTC | #5
On Tue, 01 Dec 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Dec 01, 2015 at 01:12:43PM +0200, Jani Nikula wrote:
>> On Tue, 01 Dec 2015, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> > Somehow the kernel's mode list changed with our EDID.
>> 
>> I do not undestand what you're trying to say here.
>
> The EDID we injected stayed the same, but the kernel started listing more
> modes for it. I didn't do a bisect nor analyse the root-cause since that
> would mean decoding the edid by hand to double-check what's going on.
>
> Suggestions for better phrasing?

Your first sentence above.

BR,
Jani.
Daniel Vetter Dec. 4, 2015, 9:51 a.m. UTC | #6
On Wed, Dec 02, 2015 at 01:50:36PM +0200, Jani Nikula wrote:
> On Tue, 01 Dec 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Dec 01, 2015 at 01:12:43PM +0200, Jani Nikula wrote:
> >> On Tue, 01 Dec 2015, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >> > Somehow the kernel's mode list changed with our EDID.
> >> 
> >> I do not undestand what you're trying to say here.
> >
> > The EDID we injected stayed the same, but the kernel started listing more
> > modes for it. I didn't do a bisect nor analyse the root-cause since that
> > would mean decoding the edid by hand to double-check what's going on.
> >
> > Suggestions for better phrasing?
> 
> Your first sentence above.

Done, and pushed all three patches to master.
-Daniel
diff mbox

Patch

diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c
index 838a3b0ae2e6..99a5649ec0c3 100644
--- a/tests/kms_force_connector.c
+++ b/tests/kms_force_connector.c
@@ -150,10 +150,8 @@  int main(int argc, char **argv)
 					   vga_connector->connector_id);
 
 		CHECK_MODE(temp->modes[0], 1920, 1080, 60);
-		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);
+		/* Don't check non-preferred modes to avoid to tight coupling
+		 * with the in-kernel EDID parser. */
 
 		drmModeFreeConnector(temp);