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

[Takashi Iwai]

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

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.


thanks,

Takashi