Hi, On Wed, Jan 13, 2021 at 10:24:49PM +0000, Srinivas Kandagatla wrote: > On 13/01/2021 20:08, Stephan Gerhold wrote: > > 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@xxxxxxxxxx> > > > Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> > > > > 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/ > I fixed that one a bit differently in "ASoC: hdmi-codec: Fix return value in hdmi_codec_set_jack()" https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=for-5.11&id=2a0435df963f996ca870a2ef1cbf1773dc0ea25a It might be useful to implement the jack detection in adv7511, but I guess we would also need some changes in sound/soc/qcom/apq8016_sbc.c to setup SND_JACK_LINEOUT. And somehow there would need to be a way to differentiate between the normal audio jack and the "HDMI jack". > 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. > > [...] > > We should probably consolidate these defines to a single lpass.h file in > include/dt-bindings/ and not split them into soc specific. > I agree, especially in case more special DAIs are added in the future. However, in my patch I tried to avoid doing that for the following two reasons: - Changing dt-bindings after they are added is generally discouraged. but more importantly: - lpass-ipq806x.c does not seem to have PRIMARY, SECONDARY, ... but something completely different. I know nothing about that platform so I would not know how to handle it. I will post my patch and then you can decide. Thanks, Stephan