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

Ser, Simon
2019-06-03T12:56:58Z

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
> >