| Message ID | 20230803075028.32170-1-trevor.wu@mediatek.com |
|---|---|
| State | Accepted |
| Commit | 1d54134df47684ee29d4d4bbe28174a4282389e0 |
| Headers | show |
| Series | ASoC: SOF: mediatek: mt8186 modify dram type as non-cache | expand |
Hi Trevor, On 8/3/23 10:50, Trevor Wu wrote: > To prevent incorrect access between the host and DSP sides, we need to > modify DRAM as a non-cache memory type. Additionally, we can retrieve > the size of shared DMA from the device tree. > > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com> > Reviewed-by: Yaochun Hung <yc.hung@mediatek.com> > Reviewed-by: Kuan-Hsun Cheng <Allen-KH.Cheng@mediatek.com> > --- > sound/soc/sof/mediatek/mt8186/mt8186.c | 40 +++++++++++++++----------- > 1 file changed, 23 insertions(+), 17 deletions(-) > > diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.c b/sound/soc/sof/mediatek/mt8186/mt8186.c > index 3e0ea0e109e2..f587edf9e0a7 100644 > --- a/sound/soc/sof/mediatek/mt8186/mt8186.c > +++ b/sound/soc/sof/mediatek/mt8186/mt8186.c > @@ -111,6 +111,14 @@ static int platform_parse_resource(struct platform_device *pdev, void *data) > > dev_dbg(dev, "DMA %pR\n", &res); > > + adsp->pa_shared_dram = (phys_addr_t)res.start; > + adsp->shared_size = resource_size(&res); > + if (adsp->pa_shared_dram & DRAM_REMAP_MASK) { > + dev_err(dev, "adsp shared dma memory(%#x) is not 4K-aligned\n", > + (u32)adsp->pa_shared_dram); > + return -EINVAL; > + } > + Would it be better to just realign to the next 4k boundary ? Or, isn't it more usual to use dma_coerce_mask_and_coherent ? > ret = of_reserved_mem_device_init(dev); > if (ret) { > dev_err(dev, "of_reserved_mem_device_init failed\n"); > @@ -244,23 +252,18 @@ static int adsp_shared_base_ioremap(struct platform_device *pdev, void *data) > { > struct device *dev = &pdev->dev; > struct mtk_adsp_chip_info *adsp = data; > - u32 shared_size; > > /* remap shared-dram base to be non-cachable */ > - shared_size = TOTAL_SIZE_SHARED_DRAM_FROM_TAIL; > - adsp->pa_shared_dram = adsp->pa_dram + adsp->dramsize - shared_size; > - if (adsp->va_dram) { > - adsp->shared_dram = adsp->va_dram + DSP_DRAM_SIZE - shared_size; > - } else { > - adsp->shared_dram = devm_ioremap(dev, adsp->pa_shared_dram, > - shared_size); > - if (!adsp->shared_dram) { > - dev_err(dev, "ioremap failed for shared DRAM\n"); > - return -ENOMEM; > - } > + adsp->shared_dram = devm_ioremap(dev, adsp->pa_shared_dram, > + adsp->shared_size); You cannot use dma_alloc_coherent ? This should take care of all the cache maintainance for you. > + if (!adsp->shared_dram) { > + dev_err(dev, "failed to ioremap base %pa size %#x\n", > + adsp->shared_dram, adsp->shared_size); > + return -ENOMEM; > } > - dev_dbg(dev, "shared-dram vbase=%p, phy addr :%pa, size=%#x\n", > - adsp->shared_dram, &adsp->pa_shared_dram, shared_size); > + > + dev_dbg(dev, "shared-dram vbase=%p, phy addr :%pa, size=%#x\n", > + adsp->shared_dram, &adsp->pa_shared_dram, adsp->shared_size); > > return 0; > } > @@ -307,9 +310,12 @@ static int mt8186_dsp_probe(struct snd_sof_dev *sdev) > return -ENOMEM; > } > > - sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, > - priv->adsp->pa_dram, > - priv->adsp->dramsize); > + priv->adsp->va_sram = sdev->bar[SOF_FW_BLK_TYPE_IRAM]; > + > + sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap(sdev->dev, > + priv->adsp->pa_dram, > + priv->adsp->dramsize); > + Same here > if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) { > dev_err(sdev->dev, "failed to ioremap base %pa size %#x\n", > &priv->adsp->pa_dram, priv->adsp->dramsize); Regards, Eugen
On Thu, 2023-08-03 at 11:04 +0300, Eugen Hristev wrote: > Hi Trevor, > > On 8/3/23 10:50, Trevor Wu wrote: > > To prevent incorrect access between the host and DSP sides, we need > > to > > modify DRAM as a non-cache memory type. Additionally, we can > > retrieve > > the size of shared DMA from the device tree. > > > > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com> > > Reviewed-by: Yaochun Hung <yc.hung@mediatek.com> > > Reviewed-by: Kuan-Hsun Cheng <Allen-KH.Cheng@mediatek.com> > > --- > > sound/soc/sof/mediatek/mt8186/mt8186.c | 40 +++++++++++++++---- > > ------- > > 1 file changed, 23 insertions(+), 17 deletions(-) > > > > diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.c > > b/sound/soc/sof/mediatek/mt8186/mt8186.c > > index 3e0ea0e109e2..f587edf9e0a7 100644 > > --- a/sound/soc/sof/mediatek/mt8186/mt8186.c > > +++ b/sound/soc/sof/mediatek/mt8186/mt8186.c > > @@ -111,6 +111,14 @@ static int platform_parse_resource(struct > > platform_device *pdev, void *data) > > > > dev_dbg(dev, "DMA %pR\n", &res); > > > > + adsp->pa_shared_dram = (phys_addr_t)res.start; > > + adsp->shared_size = resource_size(&res); > > + if (adsp->pa_shared_dram & DRAM_REMAP_MASK) { > > + dev_err(dev, "adsp shared dma memory(%#x) is not 4K- > > aligned\n", > > + (u32)adsp->pa_shared_dram); > > + return -EINVAL; > > + } > > + > > Would it be better to just realign to the next 4k boundary ? > Or, isn't it more usual to use dma_coerce_mask_and_coherent ? Hi Eugen, The 4k boundary checking serves as a sentinel to ensure that the memory we assign on the AP side can be utilized on the DSP side. If we handle it in the way you suggested, it would mean that some reserved memory will never be used. > > ret = of_reserved_mem_device_init(dev); > > if (ret) { > > dev_err(dev, "of_reserved_mem_device_init failed\n"); > > @@ -244,23 +252,18 @@ static int adsp_shared_base_ioremap(struct > > platform_device *pdev, void *data) > > { > > struct device *dev = &pdev->dev; > > struct mtk_adsp_chip_info *adsp = data; > > - u32 shared_size; > > > > /* remap shared-dram base to be non-cachable */ > > - shared_size = TOTAL_SIZE_SHARED_DRAM_FROM_TAIL; > > - adsp->pa_shared_dram = adsp->pa_dram + adsp->dramsize - > > shared_size; > > - if (adsp->va_dram) { > > - adsp->shared_dram = adsp->va_dram + DSP_DRAM_SIZE - > > shared_size; > > - } else { > > - adsp->shared_dram = devm_ioremap(dev, adsp- > > >pa_shared_dram, > > - shared_size); > > - if (!adsp->shared_dram) { > > - dev_err(dev, "ioremap failed for shared > > DRAM\n"); > > - return -ENOMEM; > > - } > > + adsp->shared_dram = devm_ioremap(dev, adsp->pa_shared_dram, > > + adsp->shared_size); > > You cannot use dma_alloc_coherent ? This should take care of all the > cache maintainance for you. The purpose of this patch is to align the implementation on mt8195[1]. In fact, the usage of "adsp->shared_dram" has been discontinued. I will create a follow-up patch to remove this part. > > > + if (!adsp->shared_dram) { > > + dev_err(dev, "failed to ioremap base %pa size %#x\n", > > + adsp->shared_dram, adsp->shared_size); > > + return -ENOMEM; > > } > > - dev_dbg(dev, "shared-dram vbase=%p, phy addr :%pa, size=%#x\n", > > - adsp->shared_dram, &adsp->pa_shared_dram, shared_size); > > + > > + dev_dbg(dev, "shared-dram vbase=%p, phy addr > > :%pa, size=%#x\n", > > + adsp->shared_dram, &adsp->pa_shared_dram, adsp- > > >shared_size); > > > > return 0; > > } > > @@ -307,9 +310,12 @@ static int mt8186_dsp_probe(struct snd_sof_dev > > *sdev) > > return -ENOMEM; > > } > > > > - sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, > > - priv->adsp- > > >pa_dram, > > - priv->adsp- > > >dramsize); > > + priv->adsp->va_sram = sdev->bar[SOF_FW_BLK_TYPE_IRAM]; > > + > > + sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap(sdev->dev, > > + priv->adsp- > > >pa_dram, > > + priv->adsp- > > >dramsize); > > + > > Same here There are two separate memories. However, the memory being referred to here is not the one we set for of_reserved_mem_device_init(). [1] https://lore.kernel.org/all/20220606210212.146626-5-pierre-louis.bossart@linux.intel.com/ Thanks, Trevor
On Thu, 03 Aug 2023 15:50:28 +0800, Trevor Wu wrote: > To prevent incorrect access between the host and DSP sides, we need to > modify DRAM as a non-cache memory type. Additionally, we can retrieve > the size of shared DMA from the device tree. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: SOF: mediatek: mt8186 modify dram type as non-cache commit: 1d54134df47684ee29d4d4bbe28174a4282389e0 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
diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.c b/sound/soc/sof/mediatek/mt8186/mt8186.c index 3e0ea0e109e2..f587edf9e0a7 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186.c +++ b/sound/soc/sof/mediatek/mt8186/mt8186.c @@ -111,6 +111,14 @@ static int platform_parse_resource(struct platform_device *pdev, void *data) dev_dbg(dev, "DMA %pR\n", &res); + adsp->pa_shared_dram = (phys_addr_t)res.start; + adsp->shared_size = resource_size(&res); + if (adsp->pa_shared_dram & DRAM_REMAP_MASK) { + dev_err(dev, "adsp shared dma memory(%#x) is not 4K-aligned\n", + (u32)adsp->pa_shared_dram); + return -EINVAL; + } + ret = of_reserved_mem_device_init(dev); if (ret) { dev_err(dev, "of_reserved_mem_device_init failed\n"); @@ -244,23 +252,18 @@ static int adsp_shared_base_ioremap(struct platform_device *pdev, void *data) { struct device *dev = &pdev->dev; struct mtk_adsp_chip_info *adsp = data; - u32 shared_size; /* remap shared-dram base to be non-cachable */ - shared_size = TOTAL_SIZE_SHARED_DRAM_FROM_TAIL; - adsp->pa_shared_dram = adsp->pa_dram + adsp->dramsize - shared_size; - if (adsp->va_dram) { - adsp->shared_dram = adsp->va_dram + DSP_DRAM_SIZE - shared_size; - } else { - adsp->shared_dram = devm_ioremap(dev, adsp->pa_shared_dram, - shared_size); - if (!adsp->shared_dram) { - dev_err(dev, "ioremap failed for shared DRAM\n"); - return -ENOMEM; - } + adsp->shared_dram = devm_ioremap(dev, adsp->pa_shared_dram, + adsp->shared_size); + if (!adsp->shared_dram) { + dev_err(dev, "failed to ioremap base %pa size %#x\n", + adsp->shared_dram, adsp->shared_size); + return -ENOMEM; } - dev_dbg(dev, "shared-dram vbase=%p, phy addr :%pa, size=%#x\n", - adsp->shared_dram, &adsp->pa_shared_dram, shared_size); + + dev_dbg(dev, "shared-dram vbase=%p, phy addr :%pa, size=%#x\n", + adsp->shared_dram, &adsp->pa_shared_dram, adsp->shared_size); return 0; } @@ -307,9 +310,12 @@ static int mt8186_dsp_probe(struct snd_sof_dev *sdev) return -ENOMEM; } - sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, - priv->adsp->pa_dram, - priv->adsp->dramsize); + priv->adsp->va_sram = sdev->bar[SOF_FW_BLK_TYPE_IRAM]; + + sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap(sdev->dev, + priv->adsp->pa_dram, + priv->adsp->dramsize); + if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) { dev_err(sdev->dev, "failed to ioremap base %pa size %#x\n", &priv->adsp->pa_dram, priv->adsp->dramsize);