[igt-dev] [PATCH i-g-t 4/4] tests/kms_dp_aux_dev: Add drm_dp_aux test

[Ville Syrjälä]

On Tue, Dec 03, 2019 at 04:32:15PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-12-03 15:35:55)
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Add a basic test for /dev/drm_dp_aux. We just try to read 16 bytes
> > from each connector's aux device, and if succesful we do a few
> > quick sanity checks on the data.
> > 
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  tests/Makefile.sources |   1 +
> >  tests/kms_dp_aux_dev.c | 117 +++++++++++++++++++++++++++++++++++++++++
> >  tests/meson.build      |   1 +
> >  3 files changed, 119 insertions(+)
> >  create mode 100644 tests/kms_dp_aux_dev.c
> > 
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 17980ed607d1..1d37228655e4 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -42,6 +42,7 @@ TESTS_progs = \
> >         kms_cursor_crc \
> >         kms_cursor_edge_walk \
> >         kms_cursor_legacy \
> > +       kms_dp_aux_dev \
> >         kms_dp_dsc \
> >         kms_dp_tiled_display \
> >         kms_draw_crc \
> > diff --git a/tests/kms_dp_aux_dev.c b/tests/kms_dp_aux_dev.c
> > new file mode 100644
> > index 000000000000..8db85899cf4d
> > --- /dev/null
> > +++ b/tests/kms_dp_aux_dev.c
> > @@ -0,0 +1,117 @@
> > +/*
> > + * Copyright © 2019 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 "config.h"
> > +
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +#include <dirent.h>
> > +
> > +#include "igt.h"
> > +#include "igt_kms.h"
> > +
> > +IGT_TEST_DESCRIPTION("Test that /dev/drm_dp_aux reads work");
> > +
> > +static bool test(int drm_fd, uint32_t connector_id)
> > +{
> > +       drmModeConnector *connector;
> > +       DIR *dir;
> > +       int dir_fd;
> > +
> > +       connector = drmModeGetConnectorCurrent(drm_fd, connector_id);
> > +       dir_fd = igt_connector_sysfs_open(drm_fd, connector);
> > +       drmModeFreeConnector(connector);
> > +       igt_assert(dir_fd >= 0);
> > +
> > +       dir = fdopendir(dir_fd);
> > +       igt_assert(dir);
> > +
> > +       for (;;) {
> > +               struct dirent *ent;
> > +               char path[5 + sizeof(ent->d_name)];
> > +               uint8_t buf[16];
> > +               int fd, ret;
> > +
> > +               ent = readdir(dir);
> > +               if (!ent)
> > +                       break;
> > +
> > +               if (strncmp(ent->d_name, "drm_dp_aux", 10))
> > +                       continue;
> 
> So find the drm_dp_aux name for this connector and open it in /dev/

I was pondering if we have a good way to get the actual device node.

Ideas that came to mind:
a) assume it's there with the kernel provided name
b) get the major:minor from sysfs and trawl though 
   the whole of /dev looking for it
c) ask udev?

I went with a) since I was too lazy to try the other options.

> 
> > +               snprintf(path, sizeof(path), "/dev/%s", ent->d_name);
> > +
> > +               fd = open(path, O_RDONLY);
> > +               igt_assert(fd >= 0);
> > +
> > +               ret = read(fd, buf, sizeof(buf));
> > +               igt_assert(ret == sizeof(buf) || errno == ETIMEDOUT);
> > +
> > +               igt_info("%s: %s\n", path,
> > +                        ret > 0 ? "success" : "timed out");
> 
> Expect to be able to read 16 bytes.
> 
> > +
> > +               close(fd);
> > +
> > +               closedir(dir);
> > +               close(dir_fd);
> > +
> > +               if (ret > 0) {
> > +                       /* DPCD rev sanity check */
> > +                       igt_assert(buf[0] == 0x10 ||
> > +                                  buf[0] == 0x11 ||
> > +                                  buf[0] == 0x12 ||
> > +                                  buf[0] == 0x13 ||
> > +                                  buf[0] == 0x14);
> > +                       /* DPCD max lane count sanity check */
> > +                       igt_assert((buf[2] & 0x1f) == 0x01 ||
> > +                                  (buf[2] & 0x1f) == 0x02 ||
> > +                                  (buf[2] & 0x1f) == 0x04);
> 
> I believe you.
> 
> Might be worth using igt_assert_f() as I doubt these will make much
> sense if they ever fail.

Yeah, I guess I can add a bit of extra paint on top.

> 
> > +               }
> > +
> > +               return ret > 0;
> 
> Only care about drm_dp_aux, and there should only be one.
> > +       }
> > +
> > +       closedir(dir);
> > +       close(dir_fd);
> > +       return false;
> > +}
> > +
> > +igt_simple_main
> > +{
> > +       int valid_connectors = 0;
> > +       drmModeRes *res;
> > +       int drm_fd;
> > +
> > +       drm_fd = drm_open_driver_master(DRIVER_ANY);
> > +
> > +       res = drmModeGetResources(drm_fd);
> > +       igt_require(res);
> > +
> > +       for (int i = 0; i < res->count_connectors; i++)
> > +               valid_connectors += test(drm_fd, res->connectors[i]);
> > +       igt_require(valid_connectors);
> > +
> > +       drmModeFreeResources(res);
> 
> I was able to follow along, so
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Thanks.

-- 
Ville Syrjälä
Intel