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

Arkadiusz Hiler
2019-05-31T13:18:26Z

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.

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