lkml.org 
[lkml]   [2021]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v11 0/7] Qualcomm's lpass-hdmi ASoC driver to support audio over dp port
From
Date
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
>

\
 
 \ /
  Last update: 2021-01-14 03:11    [W:0.061 / U:0.644 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site