[igt-dev] [PATCH i-g-t 3/3] tests/kms_chamelium: add an audio EDID test

[Ser, Simon]

On Wed, 2019-06-12 at 15:24 +0300, Martin Peres wrote:
> On 12/06/2019 14:35, Ser, Simon wrote:
> > On Wed, 2019-06-12 at 13:47 +0300, Martin Peres wrote:
> > > On 06/06/2019 14:48, Simon Ser wrote:
> > > > This test enables a Chamelium port with an audio-friendly EDID, and then checks
> > > > that the EDID-Like Data exposed by ALSA matches our expectations.
> > > > Signed-off-by: Simon Ser <simon.ser at intel.com>
> > > > ---
> > > >  tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 58 insertions(+)
> > > > 
> > > > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > > > index 3beea0c623bd..1a5271bb770f 100644
> > > > --- a/tests/kms_chamelium.c
> > > > +++ b/tests/kms_chamelium.c
> > > > @@ -28,6 +28,7 @@
> > > >  #include "igt.h"
> > > >  #include "igt_vc4.h"
> > > >  #include "igt_edid.h"
> > > > +#include "igt_eld.h"
> > > >  
> > > >  #include <fcntl.h>
> > > >  #include <pthread.h>
> > > > @@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
> > > >  	free(alsa);
> > > >  }
> > > >  
> > > > +static void
> > > > +test_display_audio_edid(data_t *data, struct chamelium_port *port,
> > > > +			enum test_edid edid)
> > > > +{
> > > > +	igt_output_t *output;
> > > > +	igt_plane_t *primary;
> > > > +	struct igt_fb fb;
> > > > +	drmModeModeInfo *mode;
> > > > +	drmModeConnector *connector;
> > > > +	int fb_id;
> > > > +	struct eld_entry eld;
> > > > +	struct eld_sad *sad;
> > > > +
> > > > +	reset_state(data, port);
> > > > +
> > > > +	output = prepare_output(data, port, edid);
> > > > +	connector = chamelium_port_get_connector(data->chamelium, port, false);
> > > > +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> > > > +	igt_assert(primary);
> > > > +
> > > > +	/* Enable the output because audio cannot be played on inactive
> > > > +	 * connectors. */
> > > > +	igt_assert(connector->count_modes > 0);
> > > > +	mode = &connector->modes[0];
> > > > +
> > > > +	fb_id = igt_create_color_pattern_fb(data->drm_fd,
> > > > +					    mode->hdisplay, mode->vdisplay,
> > > > +					    DRM_FORMAT_XRGB8888,
> > > > +					    LOCAL_DRM_FORMAT_MOD_NONE,
> > > > +					    0, 0, 0, &fb);
> > > > +	igt_assert(fb_id > 0);
> > > > +
> > > > +	enable_output(data, port, output, mode, &fb);
> > > > +
> > > > +	igt_assert(eld_get_igt(&eld));
> > > > +	igt_assert(eld.sads_len == 1);
> > > > +
> > > > +	sad = &eld.sads[0];
> > > > +	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
> > > > +	igt_assert(sad->channels == 2);
> > > > +	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
> > > > +		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
> > > > +	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
> > > > +		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));
> > > 
> > > If we have only one tests for one EDID, I would suggest trying more
> > > coding_types and channels to make it less typical (we want to detect
> > > hardcoded values).
> > 
> > So, the issue is that right now the EDID matches the capabilities of
> > the Chamelium receiver. I'm not too comfortable with advertising an
> > EDID with more than what the Chamelium supports for audio tests.
> 
> Well, we are not going to send anything to the chamelium, so not sure
> why you are afraid of anything :s

The same EDID is used for audio tests.

> > I agree that we need more EDIDs with different params. I have plans to
> > send a patch for this.
> 
> OK!
> 
> > > With channels = 8 and a couple of other coding_types, the patch is:
> > > Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
> > > 
> > > Any plans on generating broken ELDs and seeing how Alsa parses that?
> > 
> > Yeah, broken EDIDs/ELDs would be interesting, at least to make sure it
> > doesn't freak out completely.
> 
> Exactly! :)
> 
> > > Martin
> > > > +
> > > > +	igt_remove_fb(data->drm_fd, &fb);
> > > > +
> > > > +	drmModeFreeConnector(connector);
> > > > +}
> > > >  
> > > >  static void randomize_plane_stride(data_t *data,
> > > >  				   uint32_t width, uint32_t height,
> > > > @@ -2118,6 +2168,10 @@ igt_main
> > > >  		connector_subtest("dp-audio", DisplayPort)
> > > >  			test_display_audio(&data, port, "HDMI",
> > > >  					   TEST_EDID_DP_AUDIO);
> > > > +
> > > > +		connector_subtest("dp-audio-edid", DisplayPort)
> > > > +			test_display_audio_edid(&data, port,
> > > > +						TEST_EDID_DP_AUDIO);
> > > >  	}
> > > >  
> > > >  	igt_subtest_group {
> > > > @@ -2269,6 +2323,10 @@ igt_main
> > > >  		connector_subtest("hdmi-audio", HDMIA)
> > > >  			test_display_audio(&data, port, "HDMI",
> > > >  					   TEST_EDID_HDMI_AUDIO);
> > > > +
> > > > +		connector_subtest("hdmi-audio-edid", HDMIA)
> > > > +			test_display_audio_edid(&data, port,
> > > > +						TEST_EDID_HDMI_AUDIO);
> > > >  	}
> > > >  
> > > >  	igt_subtest_group {
> > > >