Re: [PATCH v11 0/7] Qualcomm's lpass-hdmi ASoC driver to support audio over dp port

From: Srinivas Kandagatla
Date: Wed Jan 13 2021 - 21:11:25 EST

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@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/

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