Re: [PATCH v2] ASoC: rt1316: Add RT1316 SDCA vendor-specific driver

[Jaroslav Kysela <perex@perex.cz>]

Dne 18. 02. 21 v 15:49 Pierre-Louis Bossart napsal(a):
> 
> 
> On 2/18/21 3:44 AM, Jaroslav Kysela wrote:
>> Dne 18. 02. 21 v 10:12 shumingf@realtek.com napsal(a):
>>
>>> +	SND_SOC_DAPM_SWITCH("DAC L", SND_SOC_NOPM, 0, 0, &rt1316_sto_dac_l),
>>> +	SND_SOC_DAPM_SWITCH("DAC R", SND_SOC_NOPM, 0, 0, &rt1316_sto_dac_r),
>>
>> Truly, I don't understand the reason to have a separate L/R switch when we can
>> map this functionality to one stereo (multichannel) control.
>>
>> It's an issue for all ASoC drivers. We should consider to be more strict for
>> the new ones.
> 
> At the same time we have to recognize that the L/R notion only makes 
> sense at the input to the amplifier. The amplifier may recombine 
> channels to deal with orientation/posture or simply select a specific 
> input, and drive different speakers (e.g. tweeter/woofer). Dac L and R 
> are often an abuse of language when the system have multi-way speakers. 
> Exhibit A for this is the TigerLake device with 2 RT1316 and 4 speakers. 
> L/R don't make sense to describe amplifier outputs and speaker position.

My point is a bit different. If the channels are supposed to be used together
(which usually mean a kind of the stereo operation in this case), it does not
make much sense to split this control to separate single channels. It's just a
waste of resources.

The current patch code:

one channel control "DAC L"
one channel control "DAC R"

The one control:

two channels control "DAC"

From the user space POV, the only difference is the value write operation
(both channels are set using one ioctl).

> There's also a difficult balance to be found between exposing all the 
> capabilities of the device, and making integration and userspace 
> simpler. I2C/IS2 and SoundWire devices tend to expose more controls than 
> HDaudio ones, and that was driven by a desire to optimize as much as 
> possible. Some devices are designed with limited number of controls, 
> others provide hooks to tweak everything in the system by exposing 
> literally have thousands of controls. I don't think we should pick and 
> choose which controls we want to expose, that's the codec vendor's job 
> IMHO (or the device class definition when standard and applicable)

The problem with ASoC tree is that many of those controls are not supposed to
be configured/used by the end user, but in UCM or other higher level layer
configuration, because they're a part of the hw/driver setup.

I think that we should classify those controls so the standard user space
tools can hide them, but it's another problem.

					Jaroslav

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