diff mbox series

ASoC: core: Change device numbering

Message ID 20250127144445.2739017-1-amadeuszx.slawinski@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series ASoC: core: Change device numbering | expand

Commit Message

Amadeusz Sławiński Jan. 27, 2025, 2:44 p.m. UTC
Currently ASoC cards when enumerating create CPUs rtds first and CODECs
rtds second. This causes device number on cards to not start from 0, but
from number of present CPUs. During that it does count number of rtds
and uses it as device number visible in userspace.

This patch changes device visible to userspace, when listing cards:

Before:
card 0: hdaudioB0D0 [hdaudioB0D0], device 1: HDAudio Analog (*) []
card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI1 (*) []
card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI2 (*) []
card 1: hdaudioB0D2 [hdaudioB0D2], device 3: HDMI3 (*) []

After:
card 0: hdaudioB0D0 [hdaudioB0D0], device 0: HDAudio Analog (*) []
card 1: hdaudioB0D2 [hdaudioB0D2], device 0: HDMI1 (*) []
card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI2 (*) []
card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI3 (*) []

It is done by skipping back end devices and only counting front end
ones.

Now there are few concerns I have:
- while rtd->id is not used much, few drivers seem to be using it as
  index into a table, above may break this use (although
  "include/sound/simple_card_utils.h: * the ID stored in rtd->id may not be a valid array index."
  suggests that maybe it is a bad idea anyway, but I'm not sure how
  generic that comment is)
- this will break user scripts, with hardcoded device IDs
- this will also break some UCMs with hardcoded IDs

Now my main question is, if such patch would even be considered?
Perhaps device IDs are not considered as "stable" interface and can be
changed and my above worries are unnecessary.

Patch is a result of discussion from:
https://github.com/alsa-project/alsa-ucm-conf/pull/499
and as such I may consider others ways of fixing the problem.
---
 sound/soc/soc-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Amadeusz Sławiński Jan. 27, 2025, 2:45 p.m. UTC | #1
On 1/27/2025 3:44 PM, Amadeusz Sławiński wrote:
> Currently ASoC cards when enumerating create CPUs rtds first and CODECs
> rtds second. This causes device number on cards to not start from 0, but
> from number of present CPUs. During that it does count number of rtds
> and uses it as device number visible in userspace.
> 
> This patch changes device visible to userspace, when listing cards:
> 
> Before:
> card 0: hdaudioB0D0 [hdaudioB0D0], device 1: HDAudio Analog (*) []
> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI1 (*) []
> card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI2 (*) []
> card 1: hdaudioB0D2 [hdaudioB0D2], device 3: HDMI3 (*) []
> 
> After:
> card 0: hdaudioB0D0 [hdaudioB0D0], device 0: HDAudio Analog (*) []
> card 1: hdaudioB0D2 [hdaudioB0D2], device 0: HDMI1 (*) []
> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI2 (*) []
> card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI3 (*) []
> 
> It is done by skipping back end devices and only counting front end
> ones.
> 
> Now there are few concerns I have:
> - while rtd->id is not used much, few drivers seem to be using it as
>    index into a table, above may break this use (although
>    "include/sound/simple_card_utils.h: * the ID stored in rtd->id may not be a valid array index."
>    suggests that maybe it is a bad idea anyway, but I'm not sure how
>    generic that comment is)
> - this will break user scripts, with hardcoded device IDs
> - this will also break some UCMs with hardcoded IDs
> 
> Now my main question is, if such patch would even be considered?
> Perhaps device IDs are not considered as "stable" interface and can be
> changed and my above worries are unnecessary.
> 
> Patch is a result of discussion from:
> https://github.com/alsa-project/alsa-ucm-conf/pull/499
> and as such I may consider others ways of fixing the problem.

And it should've been RFC in topic... :( Sorry about that.
Jaroslav Kysela Jan. 27, 2025, 2:54 p.m. UTC | #2
On 27. 01. 25 15:45, Amadeusz Sławiński wrote:
> On 1/27/2025 3:44 PM, Amadeusz Sławiński wrote:
>> Currently ASoC cards when enumerating create CPUs rtds first and CODECs
>> rtds second. This causes device number on cards to not start from 0, but
>> from number of present CPUs. During that it does count number of rtds
>> and uses it as device number visible in userspace.
>>
>> This patch changes device visible to userspace, when listing cards:
>>
>> Before:
>> card 0: hdaudioB0D0 [hdaudioB0D0], device 1: HDAudio Analog (*) []
>> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI1 (*) []
>> card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI2 (*) []
>> card 1: hdaudioB0D2 [hdaudioB0D2], device 3: HDMI3 (*) []
>>
>> After:
>> card 0: hdaudioB0D0 [hdaudioB0D0], device 0: HDAudio Analog (*) []
>> card 1: hdaudioB0D2 [hdaudioB0D2], device 0: HDMI1 (*) []
>> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI2 (*) []
>> card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI3 (*) []
>>
>> It is done by skipping back end devices and only counting front end
>> ones.
>>
>> Now there are few concerns I have:
>> - while rtd->id is not used much, few drivers seem to be using it as
>>     index into a table, above may break this use (although
>>     "include/sound/simple_card_utils.h: * the ID stored in rtd->id may not be a valid array index."
>>     suggests that maybe it is a bad idea anyway, but I'm not sure how
>>     generic that comment is)
>> - this will break user scripts, with hardcoded device IDs
>> - this will also break some UCMs with hardcoded IDs
>>
>> Now my main question is, if such patch would even be considered?
>> Perhaps device IDs are not considered as "stable" interface and can be
>> changed and my above worries are unnecessary.
>>
>> Patch is a result of discussion from:
>> https://github.com/alsa-project/alsa-ucm-conf/pull/499
>> and as such I may consider others ways of fixing the problem.
> 
> And it should've been RFC in topic... :( Sorry about that.

Looking to UCM configs, most of ASoC cards have PCM devices starting from 
zero. So this id is not used for all ASoC cards.

Also, a bit off-topic, but the driver name (hdaudioB?D?) for this particular 
driver should be corrected, too. It should be like 'hda-avs-dsp' or so. If I 
am not wrong, the SST driver name was 'hda-dsp'.

					Jaroslav
Cezary Rojewski Jan. 29, 2025, 9:25 a.m. UTC | #3
On 2025-01-27 3:54 PM, Jaroslav Kysela wrote:
> On 27. 01. 25 15:45, Amadeusz Sławiński wrote:
>> On 1/27/2025 3:44 PM, Amadeusz Sławiński wrote:
>>> Currently ASoC cards when enumerating create CPUs rtds first and CODECs
>>> rtds second. This causes device number on cards to not start from 0, but
>>> from number of present CPUs. During that it does count number of rtds
>>> and uses it as device number visible in userspace.
>>>
>>> This patch changes device visible to userspace, when listing cards:
>>>
>>> Before:
>>> card 0: hdaudioB0D0 [hdaudioB0D0], device 1: HDAudio Analog (*) []
>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI1 (*) []
>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI2 (*) []
>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 3: HDMI3 (*) []
>>>
>>> After:
>>> card 0: hdaudioB0D0 [hdaudioB0D0], device 0: HDAudio Analog (*) []
>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 0: HDMI1 (*) []
>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI2 (*) []
>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI3 (*) []
>>>
>>> It is done by skipping back end devices and only counting front end
>>> ones.
>>>
>>> Now there are few concerns I have:
>>> - while rtd->id is not used much, few drivers seem to be using it as
>>>     index into a table, above may break this use (although
>>>     "include/sound/simple_card_utils.h: * the ID stored in rtd->id 
>>> may not be a valid array index."
>>>     suggests that maybe it is a bad idea anyway, but I'm not sure how
>>>     generic that comment is)
>>> - this will break user scripts, with hardcoded device IDs
>>> - this will also break some UCMs with hardcoded IDs
>>>
>>> Now my main question is, if such patch would even be considered?
>>> Perhaps device IDs are not considered as "stable" interface and can be
>>> changed and my above worries are unnecessary.
>>>
>>> Patch is a result of discussion from:
>>> https://github.com/alsa-project/alsa-ucm-conf/pull/499
>>> and as such I may consider others ways of fixing the problem.
>>
>> And it should've been RFC in topic... :( Sorry about that.
> 
> Looking to UCM configs, most of ASoC cards have PCM devices starting 
> from zero. So this id is not used for all ASoC cards.

Most of the ASoC does not utilize topology and there is no strict 
guideline: initialize FE first, BE last. What Amadeusz proposes is to 
skip BEs when counting the 'devices' for the userspace as these are not 
touchable by them anyway. I believe this is a good direction and does 
not limit one's action when playing with the ASoC-topology feature.

> Also, a bit off-topic, but the driver name (hdaudioB?D?) for this 
> particular driver should be corrected, too. It should be like 'hda-avs- 
> dsp' or so. If I am not wrong, the SST driver name was 'hda-dsp'.

We had a discussion or two within the team and yes, we do agree that a 
more user-friendly pattern should be provided. Currently card names are 
mostly based on machine board device (platform_device) name. There is no 
strong technical argument for that - the development was/is simply 
focused on bringing new functionality and we did not prioritize the card 
naming.

The task touches all interfaces though, not just HDA. We would like to 
streamline or fix the naming for all the interfaces. This time we will 
specifically request a review for that : )


Kind regards,
Czarek
Amadeusz Sławiński Jan. 29, 2025, 2:51 p.m. UTC | #4
On 1/29/2025 10:25 AM, Cezary Rojewski wrote:
> On 2025-01-27 3:54 PM, Jaroslav Kysela wrote:
>> On 27. 01. 25 15:45, Amadeusz Sławiński wrote:
>>> On 1/27/2025 3:44 PM, Amadeusz Sławiński wrote:
>>>> Currently ASoC cards when enumerating create CPUs rtds first and CODECs
>>>> rtds second. This causes device number on cards to not start from 0, 
>>>> but
>>>> from number of present CPUs. During that it does count number of rtds
>>>> and uses it as device number visible in userspace.
>>>>
>>>> This patch changes device visible to userspace, when listing cards:
>>>>
>>>> Before:
>>>> card 0: hdaudioB0D0 [hdaudioB0D0], device 1: HDAudio Analog (*) []
>>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI1 (*) []
>>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI2 (*) []
>>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 3: HDMI3 (*) []
>>>>
>>>> After:
>>>> card 0: hdaudioB0D0 [hdaudioB0D0], device 0: HDAudio Analog (*) []
>>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 0: HDMI1 (*) []
>>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI2 (*) []
>>>> card 1: hdaudioB0D2 [hdaudioB0D2], device 2: HDMI3 (*) []
>>>>
>>>> It is done by skipping back end devices and only counting front end
>>>> ones.
>>>>
>>>> Now there are few concerns I have:
>>>> - while rtd->id is not used much, few drivers seem to be using it as
>>>>     index into a table, above may break this use (although
>>>>     "include/sound/simple_card_utils.h: * the ID stored in rtd->id 
>>>> may not be a valid array index."
>>>>     suggests that maybe it is a bad idea anyway, but I'm not sure how
>>>>     generic that comment is)
>>>> - this will break user scripts, with hardcoded device IDs
>>>> - this will also break some UCMs with hardcoded IDs
>>>>
>>>> Now my main question is, if such patch would even be considered?
>>>> Perhaps device IDs are not considered as "stable" interface and can be
>>>> changed and my above worries are unnecessary.
>>>>
>>>> Patch is a result of discussion from:
>>>> https://github.com/alsa-project/alsa-ucm-conf/pull/499
>>>> and as such I may consider others ways of fixing the problem.
>>>
>>> And it should've been RFC in topic... :( Sorry about that.
>>
>> Looking to UCM configs, most of ASoC cards have PCM devices starting 
>> from zero. So this id is not used for all ASoC cards.
> 
> Most of the ASoC does not utilize topology and there is no strict 
> guideline: initialize FE first, BE last. What Amadeusz proposes is to 
> skip BEs when counting the 'devices' for the userspace as these are not 
> touchable by them anyway. I believe this is a good direction and does 
> not limit one's action when playing with the ASoC-topology feature.
> 

And it seems that apparently other topology users set .use_dai_pcm_id in 
struct snd_soc_component_driver. Which tells the code to use PCM ID 
values from topology as device IDs. Although the more I look at this the 
more this looks to be a workaround due to how ASoC counts devices, which 
this RFC fixes. But at least for now we are also looking to set this 
flag, as to avoid potentially impacting ASoC core.

>> Also, a bit off-topic, but the driver name (hdaudioB?D?) for this 
>> particular driver should be corrected, too. It should be like 'hda- 
>> avs- dsp' or so. If I am not wrong, the SST driver name was 'hda-dsp'.
> 
> We had a discussion or two within the team and yes, we do agree that a 
> more user-friendly pattern should be provided. Currently card names are 
> mostly based on machine board device (platform_device) name. There is no 
> strong technical argument for that - the development was/is simply 
> focused on bringing new functionality and we did not prioritize the card 
> naming.
> 
> The task touches all interfaces though, not just HDA. We would like to 
> streamline or fix the naming for all the interfaces. This time we will 
> specifically request a review for that : )

And we had yet another discussion today ;) Are there some guidelines 
upstream, about how cards should be named? Currently we need names for: 
HDA, HDMI, DMIC & I2S cards. Here are examples from running systems:
HDA:
card 0: hdaudioB0D0 [hdaudioB0D0], device 1: HDAudio Analog (*) []
HDMI:
card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI 0 (*) []
DMIC:
card 2: avsdmic [avs_dmic], device 2: Digital Microphone (*) []
I2S:
card 3: avsrt274 [avs_rt274], device 1: Audio (*) []

Additionally is there a way to alias card name? We would like to still 
allow users to use current names, to keep backward compatibility at 
least for some time.
Jaroslav Kysela Jan. 31, 2025, 9:03 a.m. UTC | #5
On 29. 01. 25 15:51, Amadeusz Sławiński wrote:
> On 1/29/2025 10:25 AM, Cezary Rojewski wrote:
>> On 2025-01-27 3:54 PM, Jaroslav Kysela wrote:
>>> On 27. 01. 25 15:45, Amadeusz Sławiński wrote:

...

>>> Also, a bit off-topic, but the driver name (hdaudioB?D?) for this
>>> particular driver should be corrected, too. It should be like 'hda-
>>> avs- dsp' or so. If I am not wrong, the SST driver name was 'hda-dsp'.
>>
>> We had a discussion or two within the team and yes, we do agree that a
>> more user-friendly pattern should be provided. Currently card names are
>> mostly based on machine board device (platform_device) name. There is no
>> strong technical argument for that - the development was/is simply
>> focused on bringing new functionality and we did not prioritize the card
>> naming.
>>
>> The task touches all interfaces though, not just HDA. We would like to
>> streamline or fix the naming for all the interfaces. This time we will
>> specifically request a review for that : )
> 
> And we had yet another discussion today ;) Are there some guidelines
> upstream, about how cards should be named? Currently we need names for:
I am not aware of any documentation, but it should be user friendly and 
meaningful. I am talking about driver name not other UI parts, but they should 
be probably corrected, too.

The driver name is more related to the kernel module name and it should just 
identify the affected driver not runtime (like addressing). Runtime 
descriptors should be put to the components string.

> HDA, HDMI, DMIC & I2S cards. Here are examples from running systems:
> HDA:
> card 0: hdaudioB0D0 [hdaudioB0D0], device 1: HDAudio Analog (*) []
> HDMI:
> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI 0 (*) []
> DMIC:
> card 2: avsdmic [avs_dmic], device 2: Digital Microphone (*) []
> I2S:
> card 3: avsrt274 [avs_rt274], device 1: Audio (*) []

You should probably show all card info strings ('cat /proc/asound/cards' or 
'alsactl info 0').

There are basically four strings:

1) driver name (used for alsa-lib / ucm configs)
2) card short name (for UI)
3) card long name (for UI)
4) card components (more precise driver/hw identification)

The components string can be printed using 'amixer -c 0 info' or 'alsactl info 
0' command.

I think that in your case, you just set the short name which is copied to the 
driver name in the ASoC core. The avs_dmic/avs_rt274 names seems fine, but 
only the hdaudioB?C? names are really mess (including card short name).

> Additionally is there a way to alias card name? We would like to still
> allow users to use current names, to keep backward compatibility at
> least for some time.

We don't have this possibility in the kernel API. For user space (alsa-lib, 
UCM) we can use symlinks. As I already suggested, it would be probably best to 
create CONFIG_XXXXX_OBSOLETE option which can be turned on by default with 
proper description and let distribution maintainers to turn it off when 
alsa-lib / UCM / whatever related configs are updated in the user space.

					Jaroslav
Amadeusz Sławiński Jan. 31, 2025, 12:41 p.m. UTC | #6
On 1/31/2025 10:03 AM, Jaroslav Kysela wrote:
> On 29. 01. 25 15:51, Amadeusz Sławiński wrote:
>> On 1/29/2025 10:25 AM, Cezary Rojewski wrote:
>>> On 2025-01-27 3:54 PM, Jaroslav Kysela wrote:
>>>> On 27. 01. 25 15:45, Amadeusz Sławiński wrote:
> 
> ...
> 
>>>> Also, a bit off-topic, but the driver name (hdaudioB?D?) for this
>>>> particular driver should be corrected, too. It should be like 'hda-
>>>> avs- dsp' or so. If I am not wrong, the SST driver name was 'hda-dsp'.
>>>
>>> We had a discussion or two within the team and yes, we do agree that a
>>> more user-friendly pattern should be provided. Currently card names are
>>> mostly based on machine board device (platform_device) name. There is no
>>> strong technical argument for that - the development was/is simply
>>> focused on bringing new functionality and we did not prioritize the card
>>> naming.
>>>
>>> The task touches all interfaces though, not just HDA. We would like to
>>> streamline or fix the naming for all the interfaces. This time we will
>>> specifically request a review for that : )
>>
>> And we had yet another discussion today ;) Are there some guidelines
>> upstream, about how cards should be named? Currently we need names for:
> I am not aware of any documentation, but it should be user friendly and 
> meaningful. I am talking about driver name not other UI parts, but they 
> should be probably corrected, too.
> 
> The driver name is more related to the kernel module name and it should 
> just identify the affected driver not runtime (like addressing). Runtime 
> descriptors should be put to the components string.
> 
>> HDA, HDMI, DMIC & I2S cards. Here are examples from running systems:
>> HDA:
>> card 0: hdaudioB0D0 [hdaudioB0D0], device 1: HDAudio Analog (*) []
>> HDMI:
>> card 1: hdaudioB0D2 [hdaudioB0D2], device 1: HDMI 0 (*) []
>> DMIC:
>> card 2: avsdmic [avs_dmic], device 2: Digital Microphone (*) []
>> I2S:
>> card 3: avsrt274 [avs_rt274], device 1: Audio (*) []
> 
> You should probably show all card info strings ('cat /proc/asound/cards' 
> or 'alsactl info 0').
> 
> There are basically four strings:
> 
> 1) driver name (used for alsa-lib / ucm configs)
> 2) card short name (for UI)
> 3) card long name (for UI)
> 4) card components (more precise driver/hw identification)
> 
> The components string can be printed using 'amixer -c 0 info' or 
> 'alsactl info 0' command.
> 
> I think that in your case, you just set the short name which is copied 
> to the driver name in the ASoC core. The avs_dmic/avs_rt274 names seems 
> fine, but only the hdaudioB?C? names are really mess (including card 
> short name).
> 
So, as I understand you want something like the following and to 
differentiate cards in UCM using component?

# aplay -l
**** List of PLAYBACK Hardware Devices ****
card 0: avshdaudio [avs_hdaudio], device 1: HDAudio Analog (*) []
   Subdevices: 1/1
   Subdevice #0: subdevice #0
card 1: avshdaudio_1 [avs_hdaudio], device 1: HDMI 1 (*) []
   Subdevices: 1/1
   Subdevice #0: subdevice #0
card 1: avshdaudio_1 [avs_hdaudio], device 2: HDMI 2 (*) []
   Subdevices: 1/1
   Subdevice #0: subdevice #0
card 1: avshdaudio_1 [avs_hdaudio], device 3: HDMI 3 (*) []
   Subdevices: 1/1
   Subdevice #0: subdevice #0
root@test-ThinkPad-X1-Carbon-6th:/home/test# cat /proc/asound/cards
  0 [avshdaudio     ]: avs_hdaudio - avs_hdaudio
                       LENOVO-20KH006LPB-ThinkPadX1Carbon6th
  1 [avshdaudio_1   ]: avs_hdaudio - avs_hdaudio
                       LENOVO-20KH006LPB-ThinkPadX1Carbon6th
# alsactl info 0
#
# Sound card
#
- card: 0
   id: avshdaudio
   name: avs_hdaudio
   longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
   driver_name: avs_hdaudio
   mixer_name: Realtek ALC285
   components: HDA:10ec0285,17aa225c,00100002
   controls_count: 14
   pcm:
     - stream: PLAYBACK
       devices:
         - device: 1
           id: HDAudio Analog (*)
           name:
           subdevices:
             - subdevice: 0
               name: subdevice #0
     - stream: CAPTURE
       devices:
         - device: 1
           id: HDAudio Analog (*)
           name:
           subdevices:
             - subdevice: 0
               name: subdevice #0
alsactl: rawmidi_device_list:105: snd_ctl_rawmidi_next_device
# alsactl info 1
#
# Sound card
#
- card: 1
   id: avshdaudio_1
   name: avs_hdaudio
   longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
   driver_name: avs_hdaudio
   mixer_name: Intel Kabylake HDMI
   components: HDA:8086280b,80860101,00100000
   controls_count: 21
   pcm:
     - stream: PLAYBACK
       devices:
         - device: 1
           id: HDMI 1 (*)
           name:
           subdevices:
             - subdevice: 0
               name: subdevice #0
         - device: 2
           id: HDMI 2 (*)
           name:
           subdevices:
             - subdevice: 0
               name: subdevice #0
         - device: 3
           id: HDMI 3 (*)
           name:
           subdevices:
             - subdevice: 0
               name: subdevice #0
alsactl: rawmidi_device_list:105: snd_ctl_rawmidi_next_device
Jaroslav Kysela Jan. 31, 2025, 4:18 p.m. UTC | #7
On 31. 01. 25 13:41, Amadeusz Sławiński wrote:

> So, as I understand you want something like the following and to
> differentiate cards in UCM using component?

> - card: 0
>     id: avshdaudio
>     name: avs_hdaudio
>     longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>     driver_name: avs_hdaudio
>     mixer_name: Realtek ALC285
>     components: HDA:10ec0285,17aa225c,00100002

The short name (name:) may be better like 'AVS HD-Audio' (almost all ASoC 
drivers suffers here), but the proposed driver_name is fine from my view.

					Jaroslav
Amadeusz Sławiński Feb. 11, 2025, 10:53 a.m. UTC | #8
On 1/31/2025 5:18 PM, Jaroslav Kysela wrote:
> On 31. 01. 25 13:41, Amadeusz Sławiński wrote:
> 
>> So, as I understand you want something like the following and to
>> differentiate cards in UCM using component?
> 
>> - card: 0
>>     id: avshdaudio
>>     name: avs_hdaudio
>>     longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>>     driver_name: avs_hdaudio
>>     mixer_name: Realtek ALC285
>>     components: HDA:10ec0285,17aa225c,00100002
> 
> The short name (name:) may be better like 'AVS HD-Audio' (almost all 
> ASoC drivers suffers here), but the proposed driver_name is fine from my 
> view.
> 

We had some more discussions, what about something like:

card X: HDAudio [AVS HD-Audio], device X: HDAudio Analog () []
card X: HDMI [AVS HDMI], device X: HDMI 1 () []
card X: ALC274 [AVS I2S ALC274], device X: Audio () []

- card: X
   id: HDAudio
   name: AVS HD-Audio
   longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
   driver_name: avs_hdaudio
   mixer_name: Realtek ALC285
   components: HDA:10ec0285,17aa225c,00100002
   controls_count: 14

- card: X
   id: HDMI
   name: AVS HDMI
   longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
   driver_name: avs_hdaudio
   mixer_name: Intel Kabylake HDMI
   components: HDA:8086280b,80860101,00100000
   controls_count: 21

- card: X
   id: ALC274
   name: AVS I2S ALC274
   longname: 
IntelCorporation-CannonLakeClientPlatform-0.1-CannonLakeYLPDDR4RVP
   driver_name: avs_rt274
   mixer_name:
   components:
   controls_count: 17

it feels like it is more descriptive.
Jaroslav Kysela Feb. 11, 2025, 1:30 p.m. UTC | #9
On 11. 02. 25 11:53, Amadeusz Sławiński wrote:
> On 1/31/2025 5:18 PM, Jaroslav Kysela wrote:
>> On 31. 01. 25 13:41, Amadeusz Sławiński wrote:
>>
>>> So, as I understand you want something like the following and to
>>> differentiate cards in UCM using component?
>>
>>> - card: 0
>>>      id: avshdaudio
>>>      name: avs_hdaudio
>>>      longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>>>      driver_name: avs_hdaudio
>>>      mixer_name: Realtek ALC285
>>>      components: HDA:10ec0285,17aa225c,00100002
>>
>> The short name (name:) may be better like 'AVS HD-Audio' (almost all
>> ASoC drivers suffers here), but the proposed driver_name is fine from my
>> view.
>>
> 
> We had some more discussions, what about something like:
> 
> card X: HDAudio [AVS HD-Audio], device X: HDAudio Analog () []
> card X: HDMI [AVS HDMI], device X: HDMI 1 () []
> card X: ALC274 [AVS I2S ALC274], device X: Audio () []
> 
> - card: X
>     id: HDAudio
>     name: AVS HD-Audio
>     longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>     driver_name: avs_hdaudio
>     mixer_name: Realtek ALC285
>     components: HDA:10ec0285,17aa225c,00100002
>     controls_count: 14
> 
> - card: X
>     id: HDMI
>     name: AVS HDMI
>     longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>     driver_name: avs_hdaudio
>     mixer_name: Intel Kabylake HDMI
>     components: HDA:8086280b,80860101,00100000
>     controls_count: 21
> 
> - card: X
>     id: ALC274
>     name: AVS I2S ALC274
>     longname:
> IntelCorporation-CannonLakeClientPlatform-0.1-CannonLakeYLPDDR4RVP
>     driver_name: avs_rt274
>     mixer_name:
>     components:
>     controls_count: 17
> 
> it feels like it is more descriptive.

It looks much better for driver name and card (short) name. It also shows, how 
the DMI auto-generated long names are really broken and confusing for users.

				Jaroslav
Takashi Iwai Feb. 11, 2025, 1:33 p.m. UTC | #10
On Tue, 11 Feb 2025 14:30:42 +0100,
Jaroslav Kysela wrote:
> 
> On 11. 02. 25 11:53, Amadeusz Sławiński wrote:
> > On 1/31/2025 5:18 PM, Jaroslav Kysela wrote:
> >> On 31. 01. 25 13:41, Amadeusz Sławiński wrote:
> >> 
> >>> So, as I understand you want something like the following and to
> >>> differentiate cards in UCM using component?
> >> 
> >>> - card: 0
> >>>      id: avshdaudio
> >>>      name: avs_hdaudio
> >>>      longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
> >>>      driver_name: avs_hdaudio
> >>>      mixer_name: Realtek ALC285
> >>>      components: HDA:10ec0285,17aa225c,00100002
> >> 
> >> The short name (name:) may be better like 'AVS HD-Audio' (almost all
> >> ASoC drivers suffers here), but the proposed driver_name is fine from my
> >> view.
> >> 
> > 
> > We had some more discussions, what about something like:
> > 
> > card X: HDAudio [AVS HD-Audio], device X: HDAudio Analog () []
> > card X: HDMI [AVS HDMI], device X: HDMI 1 () []
> > card X: ALC274 [AVS I2S ALC274], device X: Audio () []
> > 
> > - card: X
> >     id: HDAudio
> >     name: AVS HD-Audio
> >     longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
> >     driver_name: avs_hdaudio
> >     mixer_name: Realtek ALC285
> >     components: HDA:10ec0285,17aa225c,00100002
> >     controls_count: 14
> > 
> > - card: X
> >     id: HDMI
> >     name: AVS HDMI
> >     longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
> >     driver_name: avs_hdaudio
> >     mixer_name: Intel Kabylake HDMI
> >     components: HDA:8086280b,80860101,00100000
> >     controls_count: 21
> > 
> > - card: X
> >     id: ALC274
> >     name: AVS I2S ALC274
> >     longname:
> > IntelCorporation-CannonLakeClientPlatform-0.1-CannonLakeYLPDDR4RVP
> >     driver_name: avs_rt274
> >     mixer_name:
> >     components:
> >     controls_count: 17
> > 
> > it feels like it is more descriptive.
> 
> It looks much better for driver name and card (short) name. It also
> shows, how the DMI auto-generated long names are really broken and
> confusing for users.

Agreed, let's go ahead with this.


thanks,

Takashi
Amadeusz Sławiński Feb. 11, 2025, 1:56 p.m. UTC | #11
On 2/11/2025 2:30 PM, Jaroslav Kysela wrote:
> On 11. 02. 25 11:53, Amadeusz Sławiński wrote:
>> On 1/31/2025 5:18 PM, Jaroslav Kysela wrote:
>>> On 31. 01. 25 13:41, Amadeusz Sławiński wrote:
>>>

>>
>> We had some more discussions, what about something like:
>>
>> card X: HDAudio [AVS HD-Audio], device X: HDAudio Analog () []
>> card X: HDMI [AVS HDMI], device X: HDMI 1 () []
>> card X: ALC274 [AVS I2S ALC274], device X: Audio () []
>>
>> - card: X
>>     id: HDAudio
>>     name: AVS HD-Audio
>>     longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>>     driver_name: avs_hdaudio
>>     mixer_name: Realtek ALC285
>>     components: HDA:10ec0285,17aa225c,00100002
>>     controls_count: 14
>>
>> - card: X
>>     id: HDMI
>>     name: AVS HDMI
>>     longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>>     driver_name: avs_hdaudio
>>     mixer_name: Intel Kabylake HDMI
>>     components: HDA:8086280b,80860101,00100000
>>     controls_count: 21
>>
>> - card: X
>>     id: ALC274
>>     name: AVS I2S ALC274
>>     longname:
>> IntelCorporation-CannonLakeClientPlatform-0.1-CannonLakeYLPDDR4RVP
>>     driver_name: avs_rt274
>>     mixer_name:
>>     components:
>>     controls_count: 17
>>
>> it feels like it is more descriptive.
> 
> It looks much better for driver name and card (short) name. It also 
> shows, how the DMI auto-generated long names are really broken and 
> confusing for users.
> 

Well, those can be set to something better, it's just that ASoC defaults 
to DMI if there is no longname provided. So if you have any suggestions, 
we can do it while also changing card names.
But do note that it allows for differentiating between different 
machines using same card in case we need to do something custom for UCM 
on one of them:
2680 
access("/usr/share/alsa/ucm2/conf.d/avs_hdaudio/LENOVO-20KH006LPB-ThinkPadX1Carbon6th.conf", 
R_OK) = -1 ENOENT (No such file or directory)
2680  access("/usr/share/alsa/ucm2/conf.d/avs_hdaudio/avs_hdaudio.conf", 
R_OK) = -1 ENOENT (No such file or directory)
so while not pretty I'm not sure if we can do any better, without 
breaking above possibility. And in most cases UCM based on driver name 
should be enough, which looks pretty enough.
Jaroslav Kysela Feb. 11, 2025, 2:50 p.m. UTC | #12
On 11. 02. 25 14:56, Amadeusz Sławiński wrote:
> On 2/11/2025 2:30 PM, Jaroslav Kysela wrote:
>> On 11. 02. 25 11:53, Amadeusz Sławiński wrote:
>>> On 1/31/2025 5:18 PM, Jaroslav Kysela wrote:
>>>> On 31. 01. 25 13:41, Amadeusz Sławiński wrote:
>>>>
> 
>>>
>>> We had some more discussions, what about something like:
>>>
>>> card X: HDAudio [AVS HD-Audio], device X: HDAudio Analog () []
>>> card X: HDMI [AVS HDMI], device X: HDMI 1 () []
>>> card X: ALC274 [AVS I2S ALC274], device X: Audio () []
>>>
>>> - card: X
>>>      id: HDAudio
>>>      name: AVS HD-Audio
>>>      longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>>>      driver_name: avs_hdaudio
>>>      mixer_name: Realtek ALC285
>>>      components: HDA:10ec0285,17aa225c,00100002
>>>      controls_count: 14
>>>
>>> - card: X
>>>      id: HDMI
>>>      name: AVS HDMI
>>>      longname: LENOVO-20KH006LPB-ThinkPadX1Carbon6th
>>>      driver_name: avs_hdaudio
>>>      mixer_name: Intel Kabylake HDMI
>>>      components: HDA:8086280b,80860101,00100000
>>>      controls_count: 21
>>>
>>> - card: X
>>>      id: ALC274
>>>      name: AVS I2S ALC274
>>>      longname:
>>> IntelCorporation-CannonLakeClientPlatform-0.1-CannonLakeYLPDDR4RVP
>>>      driver_name: avs_rt274
>>>      mixer_name:
>>>      components:
>>>      controls_count: 17
>>>
>>> it feels like it is more descriptive.
>>
>> It looks much better for driver name and card (short) name. It also
>> shows, how the DMI auto-generated long names are really broken and
>> confusing for users.
>>
> 
> Well, those can be set to something better, it's just that ASoC defaults
> to DMI if there is no longname provided. So if you have any suggestions,
> we can do it while also changing card names.
> But do note that it allows for differentiating between different
> machines using same card in case we need to do something custom for UCM
> on one of them:
> 2680
> access("/usr/share/alsa/ucm2/conf.d/avs_hdaudio/LENOVO-20KH006LPB-ThinkPadX1Carbon6th.conf",
> R_OK) = -1 ENOENT (No such file or directory)
> 2680  access("/usr/share/alsa/ucm2/conf.d/avs_hdaudio/avs_hdaudio.conf",
> R_OK) = -1 ENOENT (No such file or directory)
> so while not pretty I'm not sure if we can do any better, without
> breaking above possibility. And in most cases UCM based on driver name
> should be enough, which looks pretty enough.

UCM can do DMI matching itself now like (copied from acp5x.conf):

....
If.jupiter {
	Condition {
		Type String
		String1 "Jupiter"
		String2 "${sys:devices/virtual/dmi/id/product_name}"
	}
	True {
                 ...
....

The long name path lookup is a relict from the past. ASoC core just duplicates 
information which can be already obtained in the user space. I'm going to 
rewrite the machine specific links and use the UCM like matching here. With 
this, we can turn long name lookups in /usr/share/alsa/ucm2/ucm.conf off.

					Jaroslav
Amadeusz Sławiński Feb. 11, 2025, 3:12 p.m. UTC | #13
>>
>> Well, those can be set to something better, it's just that ASoC defaults
>> to DMI if there is no longname provided. So if you have any suggestions,
>> we can do it while also changing card names.
>> But do note that it allows for differentiating between different
>> machines using same card in case we need to do something custom for UCM
>> on one of them:
>> 2680
>> access("/usr/share/alsa/ucm2/conf.d/avs_hdaudio/LENOVO-20KH006LPB- 
>> ThinkPadX1Carbon6th.conf",
>> R_OK) = -1 ENOENT (No such file or directory)
>> 2680  access("/usr/share/alsa/ucm2/conf.d/avs_hdaudio/avs_hdaudio.conf",
>> R_OK) = -1 ENOENT (No such file or directory)
>> so while not pretty I'm not sure if we can do any better, without
>> breaking above possibility. And in most cases UCM based on driver name
>> should be enough, which looks pretty enough.
> 
> UCM can do DMI matching itself now like (copied from acp5x.conf):
> 
> ....
> If.jupiter {
>      Condition {
>          Type String
>          String1 "Jupiter"
>          String2 "${sys:devices/virtual/dmi/id/product_name}"
>      }
>      True {
>                  ...
> ....
> 
> The long name path lookup is a relict from the past. ASoC core just 
> duplicates information which can be already obtained in the user space. 
> I'm going to rewrite the machine specific links and use the UCM like 
> matching here. With this, we can turn long name lookups in /usr/share/ 
> alsa/ucm2/ucm.conf off.

Oh, that's nice, should I then also set longname = shortname for our 
cards? I would like to avoid a need to change things again in this area 
in the future ;)
Jaroslav Kysela Feb. 12, 2025, 1:39 p.m. UTC | #14
On 11. 02. 25 16:12, Amadeusz Sławiński wrote:
> 
>>>
>>> Well, those can be set to something better, it's just that ASoC defaults
>>> to DMI if there is no longname provided. So if you have any suggestions,
>>> we can do it while also changing card names.
>>> But do note that it allows for differentiating between different
>>> machines using same card in case we need to do something custom for UCM
>>> on one of them:
>>> 2680
>>> access("/usr/share/alsa/ucm2/conf.d/avs_hdaudio/LENOVO-20KH006LPB-
>>> ThinkPadX1Carbon6th.conf",
>>> R_OK) = -1 ENOENT (No such file or directory)
>>> 2680  access("/usr/share/alsa/ucm2/conf.d/avs_hdaudio/avs_hdaudio.conf",
>>> R_OK) = -1 ENOENT (No such file or directory)
>>> so while not pretty I'm not sure if we can do any better, without
>>> breaking above possibility. And in most cases UCM based on driver name
>>> should be enough, which looks pretty enough.
>>
>> UCM can do DMI matching itself now like (copied from acp5x.conf):
>>
>> ....
>> If.jupiter {
>>       Condition {
>>           Type String
>>           String1 "Jupiter"
>>           String2 "${sys:devices/virtual/dmi/id/product_name}"
>>       }
>>       True {
>>                   ...
>> ....
>>
>> The long name path lookup is a relict from the past. ASoC core just
>> duplicates information which can be already obtained in the user space.
>> I'm going to rewrite the machine specific links and use the UCM like
>> matching here. With this, we can turn long name lookups in /usr/share/
>> alsa/ucm2/ucm.conf off.
> 
> Oh, that's nice, should I then also set longname = shortname for our
> cards?
I think so. USB and legacy HDA cards add the device path (sysfs) to the long 
name or PCI device address like:

Lenovo ThinkPad Thunderbolt 4 Dock USB at usb-0000:00:14.0-4.4.4.4, full speed
HDA NVidia at 0x99000000 irq 19

But if you don't expect multiple instances, I think that name without this 
additional identification is enough.

						Jaroslav
diff mbox series

Patch

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 3c6d8aef41309..ba257396b2063 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -558,7 +558,8 @@  static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 	 */
 	rtd->card	= card;
 	rtd->dai_link	= dai_link;
-	rtd->id		= card->num_rtd++;
+	if (!rtd->dai_link->no_pcm)
+		rtd->id		= card->num_rtd++;
 	rtd->pmdown_time = pmdown_time;			/* default power off timeout */
 
 	/* see for_each_card_rtds */