[alsa-devel] [PATCH] ASoC: change 'HDMI/DP, pcm=' to 'HDMI/DP, pcm=' Jack control names

[Jaroslav Kysela]

Dne 26. 10. 19 v 9:37 Takashi Iwai napsal(a):
> On Fri, 25 Oct 2019 23:03:26 +0200,
> Jaroslav Kysela wrote:
>>
>> Dne 25. 10. 19 v 20:02 Kai Vehmanen napsal(a):
>>> Hi Jaroslav and all,
>>>
>>> On Fri, 25 Oct 2019, Jaroslav Kysela wrote:
>>>
>>>> the single user. Another problem is that we are not able to review all those
>>>> mistakes at the merge time. It is not a complain but a true fact.
>>>
>>> but the strings are in kernel patches, so even if all UCM files don't
>>> go through the list, we can always review when the strings are added
>>> in kernel, right?
>>
>> My point is that we already did this incomplete review (the wrong
>> strings are in the current kernel). We cannot prevent to avoid those
>> code merges, we are just human. I just don't think that the driver /
>> control names should be part of the don't-break-the-userspace policy.
> 
> It's a similar situation like the long-time discussion of tracing:
> when the kernel broke latencytop by changing the tracing format, we
> had to revert it in the end although the tracing format itself isn't
> strictly a "standard kernel ABI".  The consensus is: if upgrading the
> kernel breaks anything *significant*, it's a regression and no-go.
> It's not about whether it's a part of ABI or not.
> 
> In our particular case, the strings you wanted to fix are the ones
> that are actually hard-coded by the UCM profiles that are known to be
> really used on major systems.  That's the only reason of NAK.  If it
> were for some other minor kcontrol elements, it would have been OK.
> 
> Kai's work to integrate SOF to the legacy HDMI driver would be also OK
> because it provides the compatibility mode.  That is, we have some
> excuse that it's not us but users (distros) who actually breaks by
> choosing the kernel configuration explicitly (and even there can be a
> workaround with a module option).

We can add another kernel option for this fix, too. If you like to move
in this direction, I'll modify my patch.

The question is, if the kernel should provide a hint to the user space (UCM), 
that something *significant* changed. Perhaps, the component field in the 
control API might be used for this purpose as I already proposed. In this way, 
we can support both kernels (with old and new control names).

> Or, in general, if a fix is mandatory due to any critical problem
> (leading to a system crash or such) and there is no other way, we'll
> have to adapt it.  But our case is, again, not that category.  It was
> merely a cleanup work.
> 
>>>> I would be really curious what will happen when we change those strings ;-)
>>>
>>> I can share some experiences that happen on Linux distros with Pulseaudio:
>>>
>>> - if mixer control name is changed/missing that us used in
>>>     a UCM enable sequence, the enable sequence will fail and PA will
>>>     not choose that device
>>> 	-> e.g. when wrong HDMI control names are in the UCM file, HDMI
>>> 	output stops working
>>> - if mixer control for a Jack is changed, PA will not listen
>>>     for ALSA kctl events
>>> 	-> HDMI connection is silently missed and HDMI is not listed
>>> 	as a possible output
>>>
>>> .. i.e. in both cases HDMI is essentially broken if you update to
>>> a kernel that updates the strings but don't update user-space in sync.
>>
>> Yes, it's true. But usually, users do only upgrade. If we resolve the
>> UCM configs before the kernel change, the impact is just very low.
> 
> Well, we can't define users' behavior at all.  Upgrading only the
> kernel while keeping the old user-space is a very common practice on
> openSUSE Leap systems, for example.  (Or imagine centos user who wants
> to try a newer kernel.)
> 
>>> I wonder if one option would be to expose "legacy string" aliases,
>>> allowing to make changes but keep existing user-space happy. With my HDMI
>>> codec change, the controls are simply different, so this was not an
>>> option and I had to opt for keeping the whole driver around.
>>
>> It seems that we may need to add conditions to the UCM syntax. There
>> are several ways to do it. For your specific purpose it may look like
>> "if the control exist, use this config" or so.
>>
>> Another approach might be to add something to the decision code which
>> top UCM config file should be loaded. Actually, we rely on the card
>> name / long_name only. It seems that the probe might be extended here.
> 
> Yes, currently a UCM profile is very hard-coded, hence it's quite
> tightly coupled with the kernel driver implementation details.  It
> makes the UCM parser code implementation easier, while it induces this
> kind of incompatibility if we want to change something in the kernel
> side...
> 
> Another (rather tangential) improvement would be to introduce some
> validator in the kernel or in the UCM side to check whether the given
> string names are OK or not.  A generic kcontrol element validator
> might be worth not only for UCM but in general, because lots of ASoC
> drivers tend to use any funky string names, and currently we accept
> them without strict checks.

Yes, we need another conformance tool for the control interface and do more 
checking when the patches are accepted. I agree that the ASoC tree is too much 
benevolent in this area.

					Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.