Re: [igt-dev] [PATCH i-g-t] lib/igt_eld: introduce eld_is_supported

["Ser, Simon" <simon.ser@intel.com>]

On Wed, 2019-09-11 at 14:05 +0100, Chris Wilson wrote:
> Quoting Simon Ser (2019-09-11 14:00:24)
> > We've seen cases in which /proc/asound doesn't exist (e.g. with the new SOF
> > framework). We've also seen cases in which no soundcard is exposed by ALSA (see
> > bugzilla link). In both of these cases, skipping the tests depending on ELD
> > makes more sense and makes it clearer what happens.
> > 
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102370
> > ---
> >  lib/igt_eld.c           | 30 ++++++++++++++++++++++++++++++
> >  lib/igt_eld.h           |  1 +
> >  tests/kms_chamelium.c   |  2 ++
> >  tests/kms_hdmi_inject.c |  2 ++
> >  4 files changed, 35 insertions(+)
> > 
> > diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> > index 16c4ac06c6f6..8fdc6e8e8f0b 100644
> > --- a/lib/igt_eld.c
> > +++ b/lib/igt_eld.c
> > @@ -26,6 +26,7 @@
> >  #include "config.h"
> > 
> >  #include <dirent.h>
> > +#include <errno.h>
> >  #include <stdint.h>
> >  #include <stdio.h>
> >  #include <string.h>
> > @@ -262,3 +263,32 @@ bool eld_has_igt(void)
> >         struct eld_entry eld;
> >         return eld_get_igt(&eld);
> >  }
> > +
> > +/** eld_is_supported: check whether the ALSA procfs is enabled and audio cards
> > + * are found */
> > +bool eld_is_supported(void)
> 
> eld? I don't see where you confirm that these sound cards could only be
> provided by hmdi/eld. So alsa_is_supported() ?

Well, hmm, indeed. We need to perform three checks here:

1. Does the ALSA driver support the ALSA procfs? (e.g. SOF doesn't,
yet)
2. Does the ALSA driver detect any card?
3. Does the ALSA driver support ELDs? (only Intel HDA supports it)

This patch only does (1) and (2). I'll send a v2 to check for (3). This
can be done by making sure some files in the sound card directory are
prefixed with "eld" (these exist even if ELDs are empty).

> > +{
> > +       FILE *f;
> > +       char buf[1024];
> > +
> > +       f = fopen("/proc/asound/cards", "r");
> > +       if (f == NULL) {
> > +               if (errno == ENOENT) {
> > +                       igt_debug("/proc/asound doesn't exist\n");
> > +                       return false;
> > +               }
> > +               igt_assert_f(false,
> > +                            "Failed to open /proc/asound/cards "
> > +                            "(but the file exists)\n");
> > +       }
> > +
> > +       igt_assert_f(fgets(buf, sizeof(buf), f) != NULL,
> > +                    "fgets failed\n");
> > +       igt_assert_f(buf[0] != '\0', "/proc/asound/cards is empty\n");
> > +       fclose(f);
> > +
> > +       bool has_card = strstr(buf, "no soundcards") == NULL;
> 
> C99 is not yet default.

Oh, right

> Would strncmp be more succinct?

There are some little "---" around the text [1], so this wouldn't work.

[1]: https://bugs.freedesktop.org/show_bug.cgi?id=102370#c42

> Principle holds,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev