[igt-dev] [PATCH i-g-t 1/3] lib/igt_kms: generate an EDID suitable for DP audio
Ser, Simon
simon.ser at intel.com
Wed Jun 12 10:54:16 UTC 2019
On Wed, 2019-06-12 at 13:31 +0300, Martin Peres wrote:
> On 06/06/2019 14:48, Simon Ser wrote:
> > Prior to this commit, we were using the Chamelium's default EDID for DP audio.
> > This relies on the fact that this EDID supports audio and has correct audio
> > paremeters for our testing.
> >
> > Generating our own EDID is less error-prone and will allow us to test different
> > audio parameters.
> >
> > v2:
> > - Replace {HDMI,DP}_AUDIO_EDID_LENGTH with AUDIO_EDID_LENGTH (Arek)
> > - Don't hardcode EDIDs array length (Arek)
> >
> > Signed-off-by: Simon Ser <simon.ser at intel.com>
> > ---
> > lib/igt_kms.c | 46 +++++++++++++++++++++++++++++++++--------
> > lib/igt_kms.h | 3 ++-
> > tests/kms_chamelium.c | 10 ++++++---
> > tests/kms_hdmi_inject.c | 2 +-
> > 4 files changed, 47 insertions(+), 14 deletions(-)
> >
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index d7d711a72d27..011b064579a8 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -183,9 +183,9 @@ const unsigned char *igt_kms_get_alt_edid(void)
> > }
> >
> > static void
> > -generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
> > - struct cea_sad *sad,
> > - struct cea_speaker_alloc *speaker_alloc)
> > +generate_audio_edid(unsigned char raw_edid[static AUDIO_EDID_LENGTH],
> > + bool with_vsd, struct cea_sad *sad,
>
> with_vsd is not exactly super clear as to what it does, right? How about
> with_hdmi_audio_vsd?
Makes sense.
> > + struct cea_speaker_alloc *speaker_alloc)
> > {
> > struct edid *edid;
> > struct edid_ext *edid_ext;
> > @@ -210,10 +210,12 @@ generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
> > cea_data_size += edid_cea_data_block_set_sad(block, sad, 1);
> >
> > /* A Vendor Specific Data block is needed for HDMI audio */
> > - block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> > - vsd = cea_vsd_get_hdmi_default(&vsd_size);
> > - cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
> > - vsd_size);
> > + if (with_vsd) {
> > + block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> > + vsd = cea_vsd_get_hdmi_default(&vsd_size);
> > + cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
> > + vsd_size);
> > + }
> >
> > /* Speaker Allocation Data block */
> > block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> > @@ -233,7 +235,33 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
> > {
> > int channels;
> > uint8_t sampling_rates, sample_sizes;
> > - static unsigned char raw_edid[HDMI_AUDIO_EDID_LENGTH] = {0};
> > + static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
> > + struct cea_sad sad = {0};
> > + struct cea_speaker_alloc speaker_alloc = {0};
> > +
> > + /* Initialize the Short Audio Descriptor for PCM */
> > + channels = 2;
> > + sampling_rates = CEA_SAD_SAMPLING_RATE_32KHZ |
> > + CEA_SAD_SAMPLING_RATE_44KHZ |
> > + CEA_SAD_SAMPLING_RATE_48KHZ;
> > + sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
> > + CEA_SAD_SAMPLE_SIZE_20 |
> > + CEA_SAD_SAMPLE_SIZE_24;
> > + cea_sad_init_pcm(&sad, channels, sampling_rates, sample_sizes);
> > +
> > + /* Initialize the Speaker Allocation Data */
> > + speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
> > +
> > + generate_audio_edid(raw_edid, true, &sad, &speaker_alloc);
> > +
> > + return raw_edid;
> > +}
> > +
> > +const unsigned char *igt_kms_get_dp_audio_edid(void)
> > +{
> > + int channels;
> > + uint8_t sampling_rates, sample_sizes;
> > + static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
> > struct cea_sad sad = {0};
> > struct cea_speaker_alloc speaker_alloc = {0};
> >
> > @@ -250,7 +278,7 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
> > /* Initialize the Speaker Allocation Data */
> > speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
> >
> > - generate_hdmi_audio_edid(raw_edid, &sad, &speaker_alloc);
> > + generate_audio_edid(raw_edid, false, &sad, &speaker_alloc);
> >
> > return raw_edid;
> > }
> > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > index 4ac28131b6d9..5df9f18c0bd1 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -757,10 +757,11 @@ struct cea_sad;
> > struct cea_speaker_alloc;
> >
> > #define EDID_LENGTH 128
> > -#define HDMI_AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
> > +#define AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
> > const unsigned char *igt_kms_get_base_edid(void);
> > const unsigned char *igt_kms_get_alt_edid(void);
> > const unsigned char *igt_kms_get_hdmi_audio_edid(void);
> > +const unsigned char *igt_kms_get_dp_audio_edid(void);
> >
> > struct udev_monitor *igt_watch_hotplug(void);
> > bool igt_hotplug_detected(struct udev_monitor *mon,
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 52e3f0953621..3beea0c623bd 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -39,7 +39,9 @@ enum test_edid {
> > TEST_EDID_BASE,
> > TEST_EDID_ALT,
> > TEST_EDID_HDMI_AUDIO,
> > + TEST_EDID_DP_AUDIO,
>
> Move TEST_EDID_COUNT here, so as it will automatically get the right
> value when you change the amount of tests.
The issue with this approach is that I'll then need to handle the
invalid TEST_EDID_COUNT enum item in each switch using this enum. Also
from a semantic point-of-view, the count is not a valid enum item.
I also agree that having to keep the count in sync is annoying. Both
solutions are annoying. :(
I haven't found a perfect solution for this issue yet. Suggestions
welcome, if you have any.
> With this particular change done, this is :
> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
>
> The rename is up to you.
>
> > };
> > +#define TEST_EDID_COUNT 5
> >
> > typedef struct {
> > struct chamelium *chamelium;
> > @@ -49,7 +51,7 @@ typedef struct {
> >
> > int drm_fd;
> >
> > - int edids[4];
> > + int edids[TEST_EDID_COUNT];
> > } data_t;
> >
> > #define HOTPLUG_TIMEOUT 20 /* seconds */
> > @@ -1997,6 +1999,8 @@ static const unsigned char *get_edid(enum test_edid edid)
> > return igt_kms_get_alt_edid();
> > case TEST_EDID_HDMI_AUDIO:
> > return igt_kms_get_hdmi_audio_edid();
> > + case TEST_EDID_DP_AUDIO:
> > + return igt_kms_get_dp_audio_edid();
> > }
> > assert(0); /* unreachable */
> > }
> > @@ -2031,7 +2035,7 @@ igt_main
> > &data.port_count);
> >
> > data.edids[TEST_EDID_CHAMELIUM_DEFAULT] = CHAMELIUM_DEFAULT_EDID;
> > - for (i = 1; i < sizeof(data.edids) / sizeof(data.edids[0]); ++i) {
> > + for (i = 1; i < TEST_EDID_COUNT; ++i) {
> > data.edids[i] = chamelium_new_edid(data.chamelium,
> > get_edid(i));
> > }
> > @@ -2113,7 +2117,7 @@ igt_main
> > * Use the Chamelium's default EDID for DP audio. */
> > connector_subtest("dp-audio", DisplayPort)
> > test_display_audio(&data, port, "HDMI",
> > - TEST_EDID_CHAMELIUM_DEFAULT);
> > + TEST_EDID_DP_AUDIO);
> > }
> >
> > igt_subtest_group {
> > diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> > index 754a7407b441..8c0d1333db19 100644
> > --- a/tests/kms_hdmi_inject.c
> > +++ b/tests/kms_hdmi_inject.c
> > @@ -149,7 +149,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
> > struct kmstest_connector_config config;
> >
> > edid = igt_kms_get_hdmi_audio_edid();
> > - length = HDMI_AUDIO_EDID_LENGTH;
> > + length = AUDIO_EDID_LENGTH;
> >
> > kmstest_force_edid(drm_fd, connector, edid, length);
> >
> >
More information about the igt-dev
mailing list