[Intel-gfx] [PATCH i-g-t 1/2] lib: add functions to change connector states

[Daniel Vetter]

On Mon, May 19, 2014 at 03:57:36PM +0100, Thomas Wood wrote:
> On 19 May 2014 15:28, Daniel Vetter <daniel at ffwll.ch> wrote:
> > On Mon, May 19, 2014 at 02:42:07PM +0100, Thomas Wood wrote:
> >> Add a function to force a particular state on a connector and a
> >> convenience function to find and set the state on the VGA connector.
> >>
> >> Signed-off-by: Thomas Wood <thomas.wood at intel.com>
> >> ---
> >>  lib/igt_kms.c               | 78 ++++++++++++++++++++++++++++++++++++++++
> >>  lib/igt_kms.h               | 12 +++++++
> >>  tests/Makefile.sources      |  1 +
> >>  tests/kms_force_connector.c | 87 +++++++++++++++++++++++++++++++++++++++++++++
> >>  4 files changed, 178 insertions(+)
> >>  create mode 100644 tests/kms_force_connector.c
> >>
> >> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> >> index d00250d..90c3e56 100644
> >> --- a/lib/igt_kms.c
> >> +++ b/lib/igt_kms.c
> >> @@ -384,6 +384,61 @@ err1:
> >>       return -1;
> >>  }
> >>
> >> +static int get_card_number(int fd)
> >> +{
> >> +     struct stat buf;
> >> +
> >> +     /* find the minor number of the device */
> >> +     fstat(fd, &buf);
> >> +
> >> +     return minor(buf.st_rdev);
> >> +}
> >> +
> >> +/**
> >> + * kmstest_force_connector:
> >> + * @fd: drm file descriptor
> >> + * @connector: connector
> >> + * @state: state to force on @connector
> >> + *
> >> + * Force the specified state on the specified connector.
> >> + */
> >> +void kmstest_force_connector(int drm_fd, drmModeConnector *connector, enum
> >> +                          force_connector state)
> >
> > Hm, for the interface I've thought more about
> > kmsttest_force_connector(drm_fd, connector, edid) since most often we
> > don't just want to force the state, but set a specific EDID (for a
> > high-res mode or something similar).
> 
> Many tests don't need a specific mode, so the default ones are fine.
> Obviously in those cases they may just be able to pass NULL for the
> edid in the prototype you suggested, but the main thing here is to be
> able to enable the connector and use it. Another consideration when
> setting edid would be to prevent arbitrary edid values being set on a
> connector that actually has a display connected.

Yeah, sanity checks are missing, too. Wrt the edid I've thought we could
just add some random 1080p panel with a pile of modes to keep things
interesting. 1024x756 is kinda low-res nowadays, so not really that
interesting for testing.

Same with hdmi/dp (if we get around to that): Imo the default injection
should be a really big panel so that we have lots of upscaled modes and
good chances to stress-test the watermark code a bit.
> 
> >
> >> +{
> >> +     char *path;
> >> +     const char *value;
> >> +     int sysfs_fd, ret;
> >> +
> >> +     switch (state) {
> >> +     case FORCE_CONNECTOR_UNSPECIFIED:
> >> +             value = "unspecified";
> >> +             break;
> >> +     case FORCE_CONNECTOR_ON:
> >> +             value = "on";
> >> +             break;
> >> +     case FORCE_CONNECTOR_DIGITAL:
> >> +             value = "digital";
> >> +             break;
> >> +     case FORCE_CONNECTOR_OFF:
> >> +             value = "off";
> >> +             break;
> >> +     }
> >> +
> >> +     asprintf(&path, "/sys/class/drm/card%d-%s-%d/force",
> >> +              get_card_number(drm_fd),
> >> +              kmstest_connector_type_str(connector->connector_type),
> >> +              connector->connector_type_id);
> >> +     sysfs_fd = open(path, O_WRONLY | O_TRUNC);
> >> +     free(path);
> >> +
> >> +     igt_assert(sysfs_fd != -1);
> >> +
> >> +     ret = write(sysfs_fd, value, strlen(value) + 1);
> >> +     close(sysfs_fd);
> >> +
> >> +     igt_assert(ret != -1);
> >> +}
> >
> > We need an exit handler to clean up this mess again ;-)
> >
> >> +
> >>  void kmstest_free_connector_config(struct kmstest_connector_config *config)
> >>  {
> >>       drmModeFreeCrtc(config->crtc);
> >> @@ -1047,3 +1102,26 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe)
> >>
> >>       igt_assert(drmWaitVBlank(drm_fd, &wait_vbl) == 0);
> >>  }
> >> +
> >> +void igt_set_vga_connector_state(int drm_fd, enum force_connector state)
> >
> > Hm, for the higher-level library I've thought we'd just force everything
> > we reaosonable can, so I'd drop the vga here and also the state. So maybe
> > just igt_enable_fake_connectors(drm_fd, flags);
> 
> How would the state be reset? Using an exit handler?

Yeah, if the low-level code has some bookmarking + cleanup handlers the
high-level doesn't need to do much. Maybe we could expose a general reset
function, which could be used by tests to reset injection state if they
have special needs.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch