Messages in this thread | | | Subject | Re: [PATCH v11 0/7] Qualcomm's lpass-hdmi ASoC driver to support audio over dp port | From | Srinivas Kandagatla <> | Date | Wed, 13 Jan 2021 22:24:49 +0000 |
| |
Hi Stephan,
On 13/01/2021 20:08, Stephan Gerhold wrote: > Hi Srinivas, > > On Thu, Oct 08, 2020 at 06:37:40AM +0100, Srinivas Kandagatla wrote: >> On 08/10/2020 06:16, Srinivasa Rao Mandadapu wrote: >>> These patches are to support audio over DP port on Qualcomm's SC7180 LPASS >>> Asoc. It includes machine driver, cpu driver, platform driver updates for >>> HDMI path support, device tree documention, lpass variant structure >>> optimization and configuration changes. >>> These patches depends on the DP patch series >>> https://patchwork.kernel.org/project/dri-devel/list/?series=332029 >>> https://lore.kernel.org/patchwork/project/lkml/list/?series=464856 >>> >>> changes since V10: >>> -- Moved hdmi regmap functions from lpass-hdmi.c to lpass-cpu.c >>> -- Moved QCOM_REGMAP_FIELD_ALLOC macro from lpass-hdmi.c to lpass.h >>> changes since V9: >>> -- Removed unused structures lpass_hdmi.h >>> changes since V8: >>> -- Removed redundant structure wrapper for reg map field memebrs >>> -- Updated lpass_hdmi_regmap_volatile API with appropriate registers as true >>> and others as false. >>> changes since V7: >>> -- Fixed typo errors >>> -- Created Separate patch for buffer size change >>> changes since V6: >>> -- Removed compile time define flag, which used for enabling >>> HDMI code, based on corresponding config param is included. >>> -- Updated reg map alloc API with reg map bulk API. >>> -- Removed unnecessary line splits >>> changes since V5: >>> -- Removed unused struct regmap *map in lpass_platform_alloc_hdmidmactl_fields. >>> -- DMA alloc and free API signature change in lpass-apq8016.c, lpass-ipq806x.c >>> -- Keeping API "irqreturn_t lpass_platform_hdmiif_irq" under ifdef macro >>> Changes Since v4: >>> -- Updated with single compatible node for both I2S and HDMI. >>> Changes Since v3: >>> -- Removed id in lpass variant structure and used snd_soc_dai_driver id. >>> Changes Since v2: >>> -- Audio buffer size(i.e. LPASS_PLATFORM_BUFFER_SIZE) in lpass-platform.c increased. >>> Changes Since v1: >>> -- Commit messages are updated >>> -- Addressed Rob Herring review comments >>> >>> V Sujith Kumar Reddy (7): >>> ASoC: Add sc7180-lpass binding header hdmi define >>> ASoC: dt-bindings: Add dt binding for lpass hdmi >>> Asoc:qcom:lpass-cpu:Update dts property read API >>> Asoc: qcom: lpass:Update lpaif_dmactl members order >>> ASoC: qcom: Add support for lpass hdmi driver >>> Asoc: qcom: lpass-platform : Increase buffer size >>> ASoC: qcom: sc7180: Add support for audio over DP >>> >>> .../devicetree/bindings/sound/qcom,lpass-cpu.yaml | 74 ++-- >>> include/dt-bindings/sound/sc7180-lpass.h | 1 + >>> sound/soc/qcom/Kconfig | 5 + >>> sound/soc/qcom/Makefile | 2 + >>> sound/soc/qcom/lpass-apq8016.c | 4 +- >>> sound/soc/qcom/lpass-cpu.c | 249 ++++++++++++- >>> sound/soc/qcom/lpass-hdmi.c | 258 ++++++++++++++ >>> sound/soc/qcom/lpass-hdmi.h | 102 ++++++ >>> sound/soc/qcom/lpass-ipq806x.c | 4 +- >>> sound/soc/qcom/lpass-lpaif-reg.h | 49 ++- >>> sound/soc/qcom/lpass-platform.c | 395 +++++++++++++++++---- >>> sound/soc/qcom/lpass-sc7180.c | 116 +++++- >>> sound/soc/qcom/lpass.h | 124 ++++++- >>> 13 files changed, 1240 insertions(+), 143 deletions(-) >>> create mode 100644 sound/soc/qcom/lpass-hdmi.c >>> create mode 100644 sound/soc/qcom/lpass-hdmi.h >>> >> >> Tested this series on DragonBoard 410c >> >> Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > I spent quite some time today trying to track down another regression > for MSM8916/DragonBoard 410c audio in 5.10 and identified this patch > series as the cause. So I'm very surprised that you successfully tested > this on DB410c. > > Attempting to play HDMI audio results in: > > ADV7533: lpass_platform_pcmops_hw_params: invalid interface: 3 > ADV7533: lpass_platform_pcmops_trigger: invalid 3 interface > apq8016-lpass-cpu 7708000.audio-controller: ASoC: error at soc_component_trigger on 7708000.audio-controller: -22 > > Attempting to record analog audio results in: > > Unable to handle kernel NULL pointer dereference at virtual address 00000000000001e4 > Internal error: Oops: 96000004 [#1] PREEMPT SMP > CPU: 1 PID: 1568 Comm: arecord Not tainted 5.11.0-rc3 #20 > Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) > pc : regmap_write+0x14/0x80 > lr : lpass_platform_pcmops_open+0xd8/0x210 [snd_soc_lpass_platform] > Call trace: > regmap_write+0x14/0x80 > lpass_platform_pcmops_open+0xd8/0x210 [snd_soc_lpass_platform] > snd_soc_component_open+0x2c/0x94 > ... > > Looking at the changes in "ASoC: qcom: Add support for lpass hdmi driver" > there is a quite obvious mistake there. lpass.h now contains >
We did hit these two bugs recently while June was testing a platform based of 410c.
we had to these 2 fixes in his dev branch
https://paste.ubuntu.com/p/MCbpBgH7JV/
and a hack: https://paste.ubuntu.com/p/GYDSDmJt7Y/
I got side tracked with other stuff, so I could not cleanup the lpass hack patch to send it!
With this two patches June was able to test all the usecases for 410c.
> #include <dt-bindings/sound/sc7180-lpass.h> > > and then the SC7810 DAI IDs are hardcoded all over lpass-cpu.c and > lpass-platform.c. But apq8016 and ipq806x have completely different > DAI IDs so now MI2S_QUATERNARY (HDMI) is invalid and > MI2S_TERTIARY (= LPASS_DP_RX in sc7180-lpass.h) is treated as HDMI port. > Yes, this basically overwritten some of the defines. Specially MI2S_TERTIARY and MI2S_QUATERNARY
We should probably consolidate these defines to a single lpass.h file in include/dt-bindings/ and not split them into soc specific.
> Effectively LPASS is broken on all platforms except SC7810. > > I have a patch prepared to fix this (will send it tomorrow), > but I wonder how you have tested this successfully on DB410c. :) My tests are pretty basic headset/speaker playback cases, this basically tests PRIMARY and SECONDARY MI2S and not MI2S_TERTIARY or QUAT MI2S.
June reported this to me some time during Christmas.
Look forward to review your patches!
--srini
> > Stephan >
| |