diff mbox series

ASoC: qcom: Fix number of HDMI RDMA channels on sc7180

Message ID 20210115203329.846824-1-swboyd@chromium.org (mailing list archive)
State Accepted
Commit 7dfe20ee92f681ab1342015254ddb77a18f40cdb
Headers show
Series ASoC: qcom: Fix number of HDMI RDMA channels on sc7180 | expand

Commit Message

Stephen Boyd Jan. 15, 2021, 8:33 p.m. UTC 
Suspending/resuming with an HDMI dongle attached leads to crashes from
an audio regmap.

 Unable to handle kernel paging request at virtual address ffffffc018068000
 Mem abort info:
   ESR = 0x96000047
   EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
 Data abort info:
   ISV = 0, ISS = 0x00000047
   CM = 0, WnR = 1
 swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000081b12000
 [ffffffc018068000] pgd=0000000275d14003, pud=0000000275d14003, pmd=000000026365d003, pte=0000000000000000
 Internal error: Oops: 96000047 [#1] PREEMPT SMP
 Call trace:
  regmap_mmio_write32le+0x2c/0x40
  regmap_mmio_write+0x48/0x6c
  _regmap_bus_reg_write+0x34/0x44
  _regmap_write+0x100/0x150
  regcache_default_sync+0xc0/0x138
  regcache_sync+0x188/0x26c
  lpass_platform_pcmops_resume+0x48/0x54 [snd_soc_lpass_platform]
  snd_soc_component_resume+0x28/0x40
  soc_resume_deferred+0x6c/0x178
  process_one_work+0x208/0x3c8
  worker_thread+0x23c/0x3e8
  kthread+0x144/0x178
  ret_from_fork+0x10/0x18
 Code: d503201f d50332bf f94002a8 8b344108 (b9000113)

I can reliably reproduce this problem by running 'tail' on the registers
file in debugfs for the hdmi regmap.

 # tail /sys/kernel/debug/regmap/62d87000.lpass-lpass_hdmi/registers
 [   84.658733] Unable to handle kernel paging request at virtual address ffffffd0128e800c

This crash happens because we're trying to read registers from the
regmap beyond the length of the mapping created by ioremap().

The number of hdmi_rdma_channels determines the size of the regmap via
this code in sound/soc/qcom/lpass-cpu.c:

  lpass_hdmi_regmap_config.max_register = LPAIF_HDMI_RDMAPER_REG(variant, variant->hdmi_rdma_channels);

According to debugfs the size of the regmap is 0x68010 but according to
the DTS file posted in [1] the size is only 0x68000 (see the first reg
property of the lpass_cpu node). Let's change the number of channels to
be 3 instead of 4 so the math works out to have a max register of
0x67010, nicely fitting inside of the region size of 0x68000.

Note: I tried to bump up the size of the register region to the next
page to include the 0x68010 register but then the tail command caused
SErrors with an async abort, implying that the register region doesn't
exist or it isn't clocked because the bus is telling us that the
register read failed. I reduce the number of channels and played audio
through the HDMI channel and it kept working so I think this is correct.

Fixes: 2ad63dc8df6b ("ASoC: qcom: sc7180: Add support for audio over DP")
Link: https://lore.kernel.org/r/1601448168-18396-2-git-send-email-srivasam@codeaurora.org [1]
Cc: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
Cc: Srinivasa Rao <srivasam@codeaurora.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Cheng-Yi Chiang <cychiang@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 sound/soc/qcom/lpass-sc7180.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 5c8fe583cce542aa0b84adc939ce85293de36e5e

Comments

Mark Brown Jan. 21, 2021, 7:39 p.m. UTC | #1 
On Fri, 15 Jan 2021 12:33:29 -0800, Stephen Boyd wrote:
> Suspending/resuming with an HDMI dongle attached leads to crashes from
> an audio regmap.
> 
>  Unable to handle kernel paging request at virtual address ffffffc018068000
>  Mem abort info:
>    ESR = 0x96000047
>    EC = 0x25: DABT (current EL), IL = 32 bits
>    SET = 0, FnV = 0
>    EA = 0, S1PTW = 0
>  Data abort info:
>    ISV = 0, ISS = 0x00000047
>    CM = 0, WnR = 1
>  swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000081b12000
>  [ffffffc018068000] pgd=0000000275d14003, pud=0000000275d14003, pmd=000000026365d003, pte=0000000000000000
>  Internal error: Oops: 96000047 [#1] PREEMPT SMP
>  Call trace:
>   regmap_mmio_write32le+0x2c/0x40
>   regmap_mmio_write+0x48/0x6c
>   _regmap_bus_reg_write+0x34/0x44
>   _regmap_write+0x100/0x150
>   regcache_default_sync+0xc0/0x138
>   regcache_sync+0x188/0x26c
>   lpass_platform_pcmops_resume+0x48/0x54 [snd_soc_lpass_platform]
>   snd_soc_component_resume+0x28/0x40
>   soc_resume_deferred+0x6c/0x178
>   process_one_work+0x208/0x3c8
>   worker_thread+0x23c/0x3e8
>   kthread+0x144/0x178
>   ret_from_fork+0x10/0x18
>  Code: d503201f d50332bf f94002a8 8b344108 (b9000113)
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: qcom: Fix number of HDMI RDMA channels on sc7180
      commit: 7dfe20ee92f681ab1342015254ddb77a18f40cdb

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Srinivasa Rao Mandadapu Jan. 22, 2021, 11:26 a.m. UTC | #2 
Hi Mark and Boyd,

Thanks for your time on this issue.

In my opinion, It 's better not to apply this patch.

I will post patch with changing size in sc7180.dtsi file.


On 1/22/2021 1:09 AM, Mark Brown wrote:
> On Fri, 15 Jan 2021 12:33:29 -0800, Stephen Boyd wrote:
>> Suspending/resuming with an HDMI dongle attached leads to crashes from
>> an audio regmap.
>>
>>   Unable to handle kernel paging request at virtual address ffffffc018068000
>>   Mem abort info:
>>     ESR = 0x96000047
>>     EC = 0x25: DABT (current EL), IL = 32 bits
>>     SET = 0, FnV = 0
>>     EA = 0, S1PTW = 0
>>   Data abort info:
>>     ISV = 0, ISS = 0x00000047
>>     CM = 0, WnR = 1
>>   swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000081b12000
>>   [ffffffc018068000] pgd=0000000275d14003, pud=0000000275d14003, pmd=000000026365d003, pte=0000000000000000
>>   Internal error: Oops: 96000047 [#1] PREEMPT SMP
>>   Call trace:
>>    regmap_mmio_write32le+0x2c/0x40
>>    regmap_mmio_write+0x48/0x6c
>>    _regmap_bus_reg_write+0x34/0x44
>>    _regmap_write+0x100/0x150
>>    regcache_default_sync+0xc0/0x138
>>    regcache_sync+0x188/0x26c
>>    lpass_platform_pcmops_resume+0x48/0x54 [snd_soc_lpass_platform]
>>    snd_soc_component_resume+0x28/0x40
>>    soc_resume_deferred+0x6c/0x178
>>    process_one_work+0x208/0x3c8
>>    worker_thread+0x23c/0x3e8
>>    kthread+0x144/0x178
>>    ret_from_fork+0x10/0x18
>>   Code: d503201f d50332bf f94002a8 8b344108 (b9000113)
>>
>> [...]
> Applied to
>
>     https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
>
> Thanks!
>
> [1/1] ASoC: qcom: Fix number of HDMI RDMA channels on sc7180
>        commit: 7dfe20ee92f681ab1342015254ddb77a18f40cdb
>
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
>
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
>
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
>
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
>
> Thanks,
> Mark
Mark Brown Jan. 22, 2021, 1:39 p.m. UTC | #3 
On Fri, Jan 22, 2021 at 04:56:21PM +0530, Srinivasa Rao Mandadapu wrote:
> Hi Mark and Boyd,

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

> Thanks for your time on this issue.
> 
> In my opinion, It 's better not to apply this patch.
> 
> I will post patch with changing size in sc7180.dtsi file.

We can always do both...  If you think the patch should be reverted then
please submit a patch doing that (including a changelog which will
explain why), but note that the DT is in theory an ABI and it's possible
that people won't upgrade their DTs if the fix is in there.
diff mbox series

Patch

diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
index 85db650c2169..3127b6022ea3 100644
--- a/sound/soc/qcom/lpass-sc7180.c
+++ b/sound/soc/qcom/lpass-sc7180.c
@@ -174,7 +174,7 @@  static struct lpass_variant sc7180_data = {
 	.rdma_channels		= 5,
 	.hdmi_rdma_reg_base		= 0x64000,
 	.hdmi_rdma_reg_stride	= 0x1000,
-	.hdmi_rdma_channels		= 4,
+	.hdmi_rdma_channels		= 3,
 	.dmactl_audif_start	= 1,
 	.wrdma_reg_base		= 0x18000,
 	.wrdma_reg_stride	= 0x1000,