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

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

On Fri, 2019-05-31 at 16:18 +0300, Arkadiusz Hiler wrote:
> On Fri, May 31, 2019 at 02:22:00PM +0300, 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.
> > 
> > Remove some high sampling rates from get_hdmi_audio_edid, since the Chamelium
> > doesn't support them anyway. This makes the SAD check uniform for all ports.
> > 
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > ---
> >  tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 64 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 49c3de0b7beb..16d3a773bb2a 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>
> > @@ -1426,6 +1427,60 @@ 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;
> > +	const char *monitor_name;
> > +	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);
> > +
> > +	monitor_name = "IGT";
> > +	if (edid == TEST_EDID_CHAMELIUM_DEFAULT)
> > +		monitor_name = "AthenaDP";
> 
> I am starting to grow less and less fond of using the default
> Chamelium's EDID. This will just make us build up on simillar checks
> over time. Let's just define our own copy and then use values from it
> directly.
> 
> "monitor_name" should probably be named manufacturer_id and be obtained
> by chamelium_get_manufacturer_id(edid_id) or smth.
> 
> And since you were talking about MST in one of the other patches, let's
> talk about MST :-) This would make it more future proof (WIP was
> defining a copy of EDID for each port using manufacturer id in form of
> "I-$CHAM_PORT_ID"). chamelium_get_manufacturer_id() can be extended later
> to also take chamelium_port as parameter.

I now have a patch for another series that adds a DisplayPort EDID
(depends on Speaker Allocation Data blocks, so can't be added to this
one).

> Other than that the series looks good.
> 
> > +
> > +	igt_assert(eld_from_monitor_name(&eld, monitor_name));
> > +	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));
> > +
> > +	igt_remove_fb(data->drm_fd, &fb);
> > +
> > +	drmModeFreeConnector(connector);
> > +}
> >  
> >  static void randomize_plane_stride(data_t *data,
> >  				   uint32_t width, uint32_t height,
> > @@ -2005,11 +2060,7 @@ static unsigned const char *get_hdmi_audio_edid(void)
> >  	channels = 2; /* TODO: speaker alloc blocks for > 2 channels */
> >  	sampling_rates = CEA_SAD_SAMPLING_RATE_32KHZ |
> >  			 CEA_SAD_SAMPLING_RATE_44KHZ |
> > -			 CEA_SAD_SAMPLING_RATE_48KHZ |
> > -			 CEA_SAD_SAMPLING_RATE_88KHZ |
> > -			 CEA_SAD_SAMPLING_RATE_96KHZ |
> > -			 CEA_SAD_SAMPLING_RATE_176KHZ |
> > -			 CEA_SAD_SAMPLING_RATE_192KHZ;
> > +			 CEA_SAD_SAMPLING_RATE_48KHZ;
> >  	sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
> >  		       CEA_SAD_SAMPLE_SIZE_20 |
> >  		       CEA_SAD_SAMPLE_SIZE_24;
> > @@ -2174,6 +2225,10 @@ igt_main
> >  		connector_subtest("dp-audio", DisplayPort)
> >  			test_display_audio(&data, port, "HDMI",
> >  					   TEST_EDID_CHAMELIUM_DEFAULT);
> > +
> > +		connector_subtest("dp-audio-edid", DisplayPort)
> > +			test_display_audio_edid(&data, port,
> > +						TEST_EDID_CHAMELIUM_DEFAULT);
> >  	}
> >  
> >  	igt_subtest_group {
> > @@ -2325,6 +2380,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 {
> > -- 
> > 2.21.0
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev