[lkml]   [2020]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 0/2] ASoC: qdsp6: fix default FE dais and routings.

On 17/04/2020 16:35, Stephan Gerhold wrote:
> On Fri, Apr 17, 2020 at 02:02:08PM +0100, Srinivas Kandagatla wrote:
>> On 17/04/2020 12:24, Stephan Gerhold wrote:
>>> Hi Srini,
>>> On Wed, Mar 11, 2020 at 06:04:20PM +0000, Srinivas Kandagatla wrote:
>>>> QDSP6 Frontend dais can be configured to work in rx or tx or both rx/tx mode,
>>>> however the default routing do not honour this DT configuration making sound
>>>> card fail to probe. FE dais are also not fully honouring device tree configuration.
>>>> Fix both of them.
>>> I discovered this patch set when QDSP6 audio stopped working after
>>> upgrading to Linux 5.7-rc1. As far as I understand, device tree bindings
>>> should attempt to be backwards compatible wherever possible.
>>> This isn't the case here, although this is not the reason for my mail.
>>> (I don't mind updating my device tree, especially since it is not
>>> upstream yet...)
>>> I have a general question about the design here.
>>> I understand the original motivation for this patch set: Attempting to
>>> configure a TX/RX-only DAI was not possible due to the default routing.
>>> In my opinion this is only relevant for the compressed DAI case.
>>> If we ignore the compressed DAIs for a moment (which can be
>>> unidirectional only), I think we shouldn't care how userspace uses the
>>> available FE/MultiMedia DAIs. We have this huge routing matrix in q6routing,
>>> with 800+ mixers that can be configured in any way possible from userspace.
>>> In "ASoC: qdsp6: q6asm-dai: only enable dais from device tree" you mention:
>>>> This can lead to un-necessary dais in the system which will never be
>>>> used. So honour whats specfied in device tree.
>>> but IMO the FE DAIs are a negligible overhead compared to the routing
>>> matrix and the many BE DAIs that are really never going to be used
>>> (because nothing is physically connected to the ports).
>> Two things, one unnecessary mixers, second thing is we need to know how many
>> FE dais are in the system, which should be derived from the number of dai
>> child nodes. These can potentially be SoC specific or firmware specific.
> So there are SoCs/firmwares that just support e.g. MultiMedia1-4 and not
> all 8 MultiMedia FE DAIs?

This all depends on vendor customization to the firmware.
Normally Q6ASM supports up to 8 concurrent streams.

>> My plan is to cleanup the BE DAIs as well!, any patches welcome!
>>> Even if you restrict FE DAIs to RX/TX only, or disable them entirely,
>> I think this is mistake from myside. Alteast according to bindings direction
>> property is only "Required for Compress offload dais", code should have
>> explicitly ignored it. Here is change that should fix it.
> This would make the MultiMedia1-3 bi-directional in sdm845-db845c,
> but MultiMedia5-8 would still be disabled.
> My question here would then be similar as above:
> Is this an arbitrary selection of a reasonable amount of FE DAIs,
> or actually based on some firmware limitations?
Yes, it is purely firmware limitation.

> As I described in the rest of my mail (below your diff),
> before this patch set it was simple to just expose all 8 FE DAIs.
> At least on MSM8916 all of them work in exactly the same way,
> there is no difference between any of them.
> If we list what is working in SoC/firmware in the device tree,
> would I just list all 8 FE DAIs?

That is the direction, which should also include any dai specific
properties like compressed case.

> Basically I'm still trying to understand why we limit the number of
> FE/MultiMedia DAIs that we expose, when all of them would be working
> fine. :)
I don't think we need to limit this from Linux side, but Its important
to take care of the max allowed Q6ASM dais w.r.t DSP.


> Thanks,
> Stephan
>> --------------------------->cut<---------------------------------
>> diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c
>> b/sound/soc/qcom/qdsp6/q6asm-dai.c
>> index 125af00bba53..31f46b25978e 100644
>> --- a/sound/soc/qcom/qdsp6/q6asm-dai.c
>> +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
>> @@ -1067,6 +1067,11 @@ static int of_q6asm_parse_dai_data(struct device
>> *dev,
>> dai_drv = &pdata->dais[idx++];
>> *dai_drv = q6asm_fe_dais_template[id];
>> + if (of_property_read_bool(node, "is-compress-dai"))
>> + dai_drv->compress_new = snd_soc_new_compress;
>> + else
>> + continue;
>> +
>> ret = of_property_read_u32(node, "direction", &dir);
>> if (ret)
>> continue;
>> @@ -1076,8 +1081,6 @@ static int of_q6asm_parse_dai_data(struct device *dev,
>> else if (dir == Q6ASM_DAI_TX)
>> dai_drv->playback = empty_stream;
>> - if (of_property_read_bool(node, "is-compress-dai"))
>> - dai_drv->compress_new = snd_soc_new_compress;
>> }
>> return 0;
>> --------------------------->cut<---------------------------------
>> Thanks,
>> srini
>>> all the routing mixers still exist for them. They will just result in
>>> configurations that are not usable in any way. IMO the only thing we
>>> gain by restricting the FE DAIs is that the available mixers no longer
>>> match possible configurations.
>>> Before this patch set I used a slightly different approach in my device
>>> tree for MSM8916: I kept all FE DAIs bi-directional, and added DAI links
>>> for all of them. This means that I actually had 8 bi-directional PCM
>>> devices in userspace.
>>> I didn't use all of them - my ALSA UCM configuration only uses
>>> MultiMedia1 for playback and MultiMedia2 for capture.
>>> However, some other userspace (let's say Android) could have chosen
>>> different FE DAIs for whatever reason. We have the overhead for the
>>> routing matrix anyway, so we might as well expose it in my opinion.
>>> My question is: In what way are the FE DAIs really board-specific?
>>> If we expose only some FE DAIs with intended usage per board,
>>> e.g. MultiMedia1 for HDMI, MultiMedia2 for slimbus playback,
>>> MultiMedia3 for slimbus capture,
>>> I could almost argue that we don't need DPCM at all.
>>> The FE DAIs are always going to be used for the same backend anyway.
>>> This is a bit exaggerated - for example if you have a single compress
>>> DAI per board you probably intend to use it for both HDMI/slimbus.
>>> But this is the feeling I get if we configure the FE DAIs separately
>>> for each board.
>>> I wonder if we should leave configuration of the FE DAIs up to userspace
>>> (e.g. ALSA UCM), and expose the same full set of FE DAIs for each board.
>>> I think this is mostly a matter of convention for configuring FE DAIs
>>> in the device tree - I have some ideas how to make that work
>>> with the existing device tree bindings and for compressed DAIs.
>>> But this mail is already long enough as-is. ;)
>>> I also don't mind if we keep everything as-is
>>> - I just wanted to share what I have been thinking about.
>>> What do you think?
>>> Thanks for reading! ;)
>>> Stephan
>>>> Originally issue was reported by Vinod Koul
>>>> Srinivas Kandagatla (2):
>>>> ASoC: qdsp6: q6asm-dai: only enable dais from device tree
>>>> ASoC: qdsp6: q6routing: remove default routing
>>>> sound/soc/qcom/qdsp6/q6asm-dai.c | 30 +++++++++++++++++++++++-------
>>>> sound/soc/qcom/qdsp6/q6routing.c | 19 -------------------
>>>> 2 files changed, 23 insertions(+), 26 deletions(-)
>>>> --
>>>> 2.21.0

 \ /
  Last update: 2020-04-20 10:33    [W:0.062 / U:0.856 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site