Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

Tvrtko Ursulin
2019-06-05T10:51:43Z

On 04/06/2019 15:06, Ser, Simon wrote:
> On Tue, 2019-06-04 at 13:59 +0100, Tvrtko Ursulin wrote:
>> On 04/06/2019 13:26, Peres, Martin wrote:
>>> On 04/06/2019 14:22, Ser, Simon wrote:
>>>> On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
>>>>> On 27/05/2019 15:34, Simon Ser wrote:
>>>>>> This commit adds a flatline test alongside the existing frequencies test.
>>>>>>
>>>>>> The test sends a constant value and checks that the amplitude is correct. A
>>>>>> window is used to check that each sample is within acceptable bounds. The test
>>>>>> is stopped as soon as 3 audio pages pass the test.
>>>>>>
>>>>>> Signed-off-by: Simon Ser <simon.ser@intel.com>
>>>>>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>>>>> ---
>>>>>> tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 101 insertions(+)
>>>>>>
>>>>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>>>>> index 40ca93687c20..451a616f1a2e 100644
>>>>>> --- a/tests/kms_chamelium.c
>>>>>> +++ b/tests/kms_chamelium.c
>>>>>> @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>>>>>> /* A streak of 3 gives confidence that the signal is good. */
>>>>>> #define MIN_STREAK 3
>>>>>>
>>>>>> +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
>>>>>
>>>>> I assume the test is making triple sure it only ever outputs this signal
>>>>> to connectors connected to Chamelium, in all possible scenarios? (I am
>>>>> thinking it could be dangerous to some amps/speakers if by some kind of
>>>>> accident.)
>>>>
>>>> Not at all. The signal is sent to all HDMI/DP ports.
>>>>
>>>> I have to check whether it's easy to match ALSA outputs to monitor
>>>> names.
>>>>
>>>> Martin, is this a concern?
>>>
>>> This is true that a non-zero constant voltage could be damaging for
>>> speakers as it can make them overheat without us hearing anything
>>> (constant position == no sound heard, but Ohm's law still applies). It
>>> would take longer than 1s though... On top of this, all speakers (except
>>> subwoofers) have high-pass filters that should remove the DC-offset so
>>> all we should be left with is a nice pop which might or might not be
>>> loud depending on how powerful the speakers are and how loud their
>>> settings are. Multi-kW systems definitely don't like them, but how
>>> likely is it that people would run IGT on it? :D
>>
>> Why would all speakers have high-pass filters? I would be surprised if
>> full range ones do, and N-way definitely do not in their totality.
>> Considering the range of laptop speakers, monitor speakers, TVs, etc,
>> all of varying quality, I'd be quite conservative and cautious before
>> outputting even one second of +90% DC signal. I know it is just IGT and
>> so extremely unlikely that if anything fries it would be anyone apart
>> from us frying our own stuff, but, still, do we have to use DC as test
>> signal?
>
> DC is just so simple. It also allows us to check that all samples are
> in a very precise range.

Why is this test important? Is there some path in i915 which can mess up
with individual sample levels? Like, it will lower 127 to 100, but won't
affect 126? (In signed 8-bit for simplicity.)

> Would it be fine to use a 25% DC?

Must be better by definition but it would be best to ask ALSA folks.

>>> That being said, if we can associate the alsa output to a certain
>>> connector (the one we are reading the sound from), then it would
>>> actually be a good thing to test the sound on this connector only, since
>>> it would allow us to verify that the mapping is indeed correct!
>>
>> Yeah.. and why do we even test this in such detail? I mean outputting
>> different signals and stuff.
>
> Because audio could be broken in a lot of different ways.
I don't understand if this "broken in a lot of different ways" is
something i915 can affect or it more belongs in ALSA testing?

>> Why not just send a sine wave beep or
>> something and check it was captured? I did not figure out from the
>> commit message why DC.
>
> We already have sine wave tests, but sine waves make it complicated to
> check that amplitude is correct for all samples.

Yeah, I don't get it as said above.

Regards,

Tvrtko