[igt-dev] [PATCH i-g-t 4/5] lib/igt_eld: allow retrieving an ELD entry per monitor name

[Ser, Simon]

On Mon, 2019-06-03 at 16:04 +0300, Martin Peres wrote:
> On 31/05/2019 14:21, Simon Ser wrote:
> > The previous function eld_has_igt (1) assumed the monitor name was always "IGT"
> > and (2) didn't provide a way to access the ELD data.
> > 
> > (1) is an issue for EDIDs that don't have "IGT" as the monitor name. This is
> > the case for the Chamelium default EDID, but this will also become an issue
> > with MST where each monitor will need to have a unique name (to be able to tell
> > them apart).
> 
> I dislike the idea of not checking that the EDID has been generated by
> IGT, but I understand that you might want some freedom there.
> 
> How about checking that the monitor name STARTS with IGT.
> 
> Given that Arek is also complaining about something like in patch 5, how
> about merging 1-4, then reviewing/merging your DP patches, then
> re-spinning the last two patches of this series?

Good idea!

> Martin
> 
> > (2) makes it impossible to check ELD audio parameters.
> > 
> > This commit fixes both (1) and (2).
> > 
> > Signed-off-by: Simon Ser <simon.ser at intel.com>
> > ---
> >  lib/igt_eld.c           | 12 +++++-------
> >  lib/igt_eld.h           |  2 +-
> >  tests/kms_hdmi_inject.c |  8 +++-----
> >  3 files changed, 9 insertions(+), 13 deletions(-)
> > 
> > diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> > index 732bbabd2d7c..53655c5e18f1 100644
> > --- a/lib/igt_eld.c
> > +++ b/lib/igt_eld.c
> > @@ -171,16 +171,14 @@ static bool eld_parse_entry(const char *path, struct eld_entry *eld)
> >  	return monitor_present;
> >  }
> >  
> > -/** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by
> > - * parsing ELD entries */
> > -bool eld_has_igt(void)
> > +/** eld_from_monitor_name: retrieve an ELD entry from a monitor name */
> > +bool eld_from_monitor_name(struct eld_entry *eld, const char *monitor_name)
> >  {
> >  	DIR *dir;
> >  	struct dirent *dirent;
> >  	int i;
> >  	char card[64];
> >  	char path[PATH_MAX];
> > -	struct eld_entry eld;
> >  
> >  	for (i = 0; i < 8; i++) {
> >  		snprintf(card, sizeof(card), "/proc/asound/card%d", i);
> > @@ -195,16 +193,16 @@ bool eld_has_igt(void)
> >  
> >  			snprintf(path, sizeof(path), "%s/%s", card,
> >  				 dirent->d_name);
> > -			if (!eld_parse_entry(path, &eld)) {
> > +			if (!eld_parse_entry(path, eld)) {
> >  				continue;
> >  			}
> >  
> > -			if (!eld.valid) {
> > +			if (!eld->valid) {
> >  				igt_debug("Skipping invalid ELD: %s\n", path);
> >  				continue;
> >  			}
> >  
> > -			if (strcmp(eld.monitor_name, "IGT") == 0) {
> > +			if (strcmp(eld->monitor_name, monitor_name) == 0) {
> >  				closedir(dir);
> >  				return true;
> >  			}
> > diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> > index e16187884d4b..1f34f784749b 100644
> > --- a/lib/igt_eld.h
> > +++ b/lib/igt_eld.h
> > @@ -49,6 +49,6 @@ struct eld_entry {
> >  	struct eld_sad sads[ELD_SADS_CAP];
> >  };
> >  
> > -bool eld_has_igt(void);
> > +bool eld_from_monitor_name(struct eld_entry *eld, const char *monitor_name);
> >  
> >  #endif
> > diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> > index eba25046cead..31ddda731d7b 100644
> > --- a/tests/kms_hdmi_inject.c
> > +++ b/tests/kms_hdmi_inject.c
> > @@ -146,6 +146,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
> >  	int fb_id, cid, ret, crtc_mask = -1;
> >  	struct igt_fb fb;
> >  	struct kmstest_connector_config config;
> > +	struct eld_entry eld;
> >  
> >  	kmstest_edid_add_audio(igt_kms_get_base_edid(), EDID_LENGTH, &edid,
> >  			       &length);
> > @@ -178,11 +179,8 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
> >  
> >  	igt_assert(ret == 0);
> >  
> > -	/*
> > -	 * Test if we have /proc/asound/HDMI/eld#0.0 and is its contents are
> > -	 * valid.
> > -	 */
> > -	igt_assert(eld_has_igt());
> > +	/* Check whether ALSA has properly detected the audio-capable EDID */
> > +	igt_assert(eld_from_monitor_name(&eld, "IGT"));
> >  
> >  	igt_remove_fb(drm_fd, &fb);
> >  
> >