Messages in this thread | | | Subject | Re: [PATCH 0/2] ASoC: qdsp6: fix default FE dais and routings. | From | Srinivas Kandagatla <> | Date | Mon, 20 Apr 2020 09:32:18 +0100 |
| |
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.
--srini
> > 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 >>>>
| |