[PATCH 0/2] ASoC: SOF: add mt8188 audio support
This series adds mt8188 audio support and dbg_dump callback for mt8186 and mt8188.
Trevor Wu (2): ASoC: SOF: mediatek: add mt8188 audio support ASoC: SOF: mediatek: add adsp debug dump
sound/soc/sof/mediatek/mt8186/mt8186.c | 83 +++++++++++++++++++++++++- sound/soc/sof/mediatek/mt8186/mt8186.h | 5 ++ 2 files changed, 87 insertions(+), 1 deletion(-)
Add mt8188 dai driver and specify of_machine to support mt8188 audio.
Signed-off-by: Trevor Wu trevor.wu@mediatek.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Yaochun Hung yc.hung@mediatek.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/mediatek/mt8186/mt8186.c | 61 +++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.c b/sound/soc/sof/mediatek/mt8186/mt8186.c index 419913c8474d..3a9c81418c1f 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186.c +++ b/sound/soc/sof/mediatek/mt8186/mt8186.c @@ -594,7 +594,65 @@ static const struct sof_dev_desc sof_of_mt8186_desc = { .ops = &sof_mt8186_ops, };
+/* + * DL2, DL3, UL4, UL5 are registered as SOF FE, so creating the corresponding + * SOF BE to complete the pipeline. + */ +static struct snd_soc_dai_driver mt8188_dai[] = { +{ + .name = "SOF_DL2", + .playback = { + .channels_min = 1, + .channels_max = 2, + }, +}, +{ + .name = "SOF_DL3", + .playback = { + .channels_min = 1, + .channels_max = 2, + }, +}, +{ + .name = "SOF_UL4", + .capture = { + .channels_min = 1, + .channels_max = 2, + }, +}, +{ + .name = "SOF_UL5", + .capture = { + .channels_min = 1, + .channels_max = 2, + }, +}, +}; + +/* mt8188 ops */ +static struct snd_sof_dsp_ops sof_mt8188_ops; + +static int sof_mt8188_ops_init(struct snd_sof_dev *sdev) +{ + /* common defaults */ + memcpy(&sof_mt8188_ops, &sof_mt8186_ops, sizeof(struct snd_sof_dsp_ops)); + + sof_mt8188_ops.drv = mt8188_dai; + sof_mt8186_ops.num_drv = ARRAY_SIZE(mt8188_dai); + + return 0; +} + +static struct snd_sof_of_mach sof_mt8188_machs[] = { + { + .compatible = "mediatek,mt8188", + .sof_tplg_filename = "sof-mt8188.tplg", + }, + {} +}; + static const struct sof_dev_desc sof_of_mt8188_desc = { + .of_machines = sof_mt8188_machs, .ipc_supported_mask = BIT(SOF_IPC), .ipc_default = SOF_IPC, .default_fw_path = { @@ -607,7 +665,8 @@ static const struct sof_dev_desc sof_of_mt8188_desc = { [SOF_IPC] = "sof-mt8188.ri", }, .nocodec_tplg_filename = "sof-mt8188-nocodec.tplg", - .ops = &sof_mt8186_ops, + .ops = &sof_mt8188_ops, + .ops_init = sof_mt8188_ops_init, };
static const struct of_device_id sof_of_mt8186_ids[] = {
Il 15/05/23 07:25, Trevor Wu ha scritto:
Add mt8188 dai driver and specify of_machine to support mt8188 audio.
Signed-off-by: Trevor Wu trevor.wu@mediatek.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Yaochun Hung yc.hung@mediatek.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com
sound/soc/sof/mediatek/mt8186/mt8186.c | 61 +++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.c b/sound/soc/sof/mediatek/mt8186/mt8186.c index 419913c8474d..3a9c81418c1f 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186.c +++ b/sound/soc/sof/mediatek/mt8186/mt8186.c @@ -594,7 +594,65 @@ static const struct sof_dev_desc sof_of_mt8186_desc = { .ops = &sof_mt8186_ops, };
+/*
- DL2, DL3, UL4, UL5 are registered as SOF FE, so creating the corresponding
- SOF BE to complete the pipeline.
- */
+static struct snd_soc_dai_driver mt8188_dai[] = { +{
- .name = "SOF_DL2",
- .playback = {
.channels_min = 1,.channels_max = 2,- },
+}, +{
- .name = "SOF_DL3",
- .playback = {
.channels_min = 1,.channels_max = 2,- },
+}, +{
- .name = "SOF_UL4",
- .capture = {
.channels_min = 1,.channels_max = 2,- },
+}, +{
- .name = "SOF_UL5",
- .capture = {
.channels_min = 1,.channels_max = 2,- },
+}, +};
+/* mt8188 ops */ +static struct snd_sof_dsp_ops sof_mt8188_ops;
+static int sof_mt8188_ops_init(struct snd_sof_dev *sdev) +{
- /* common defaults */
- memcpy(&sof_mt8188_ops, &sof_mt8186_ops, sizeof(struct snd_sof_dsp_ops));
Never use sizeof(type), always use destination var size! Anyway, there's more.
I don't think we need to perform this memcpy: we'll never see an instance of both mt8186 and mt8188 drivers on the same machine, so you can safely just use sof_mt8186_ops for mt8188...
- sof_mt8188_ops.drv = mt8188_dai;
...which obviously means that this becomes
sof_mt8186_ops.drv = mt8188_dai;
and....
- sof_mt8186_ops.num_drv = ARRAY_SIZE(mt8188_dai);
- return 0;
+}
+static struct snd_sof_of_mach sof_mt8188_machs[] = {
- {
.compatible = "mediatek,mt8188",.sof_tplg_filename = "sof-mt8188.tplg",- },
- {}
+};
- static const struct sof_dev_desc sof_of_mt8188_desc = {
- .of_machines = sof_mt8188_machs, .ipc_supported_mask = BIT(SOF_IPC), .ipc_default = SOF_IPC, .default_fw_path = {
@@ -607,7 +665,8 @@ static const struct sof_dev_desc sof_of_mt8188_desc = { [SOF_IPC] = "sof-mt8188.ri", }, .nocodec_tplg_filename = "sof-mt8188-nocodec.tplg",
- .ops = &sof_mt8186_ops,
- .ops = &sof_mt8188_ops,
...this keeps being sof_mt8186_ops as well.
.ops_init = sof_mt8188_ops_init, };
static const struct of_device_id sof_of_mt8186_ids[] = {
Regards, Angelo
On Mon, May 15, 2023 at 01:25:44PM +0200, AngeloGioacchino Del Regno wrote:
Il 15/05/23 07:25, Trevor Wu ha scritto:
+{
- /* common defaults */
- memcpy(&sof_mt8188_ops, &sof_mt8186_ops, sizeof(struct snd_sof_dsp_ops));
Never use sizeof(type), always use destination var size! Anyway, there's more.
I don't think we need to perform this memcpy: we'll never see an instance of both mt8186 and mt8188 drivers on the same machine, so you can safely just use sof_mt8186_ops for mt8188...
- sof_mt8188_ops.drv = mt8188_dai;
...which obviously means that this becomes
sof_mt8186_ops.drv = mt8188_dai;
This does have the issue that it then means the ops struct isn't const which isn't ideal. It's also not the end of the world though so I don't have super strong feelings.
On 5/15/23 10:05, Mark Brown wrote:
On Mon, May 15, 2023 at 01:25:44PM +0200, AngeloGioacchino Del Regno wrote:
Il 15/05/23 07:25, Trevor Wu ha scritto:
+{
- /* common defaults */
- memcpy(&sof_mt8188_ops, &sof_mt8186_ops, sizeof(struct snd_sof_dsp_ops));
Never use sizeof(type), always use destination var size! Anyway, there's more.
I don't think we need to perform this memcpy: we'll never see an instance of both mt8186 and mt8188 drivers on the same machine, so you can safely just use sof_mt8186_ops for mt8188...
- sof_mt8188_ops.drv = mt8188_dai;
...which obviously means that this becomes
sof_mt8186_ops.drv = mt8188_dai;
This does have the issue that it then means the ops struct isn't const which isn't ideal. It's also not the end of the world though so I don't have super strong feelings.
We do the same for Intel devices, we have a common structure which is copied and only the members that differ in specific SOCs are updated. You're right that it's not constant, but it avoids copy-paste of a rather large structure just to change a couple of lines.
On Mon, 2023-05-15 at 10:28 -0500, Pierre-Louis Bossart wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content.
On 5/15/23 10:05, Mark Brown wrote:
On Mon, May 15, 2023 at 01:25:44PM +0200, AngeloGioacchino Del Regno wrote:
Il 15/05/23 07:25, Trevor Wu ha scritto:
+{
- /* common defaults */
- memcpy(&sof_mt8188_ops, &sof_mt8186_ops, sizeof(struct
snd_sof_dsp_ops));
Never use sizeof(type), always use destination var size! Anyway, there's more.
OK, I will use sizeof(sof_mt8188_ops) instead.
I don't think we need to perform this memcpy: we'll never see an instance of both mt8186 and mt8188 drivers on the same machine, so you can safely just use sof_mt8186_ops for mt8188...
- sof_mt8188_ops.drv = mt8188_dai;
...which obviously means that this becomes sof_mt8186_ops.drv = mt8188_dai;
This does have the issue that it then means the ops struct isn't const which isn't ideal. It's also not the end of the world though so I don't have super strong feelings.
We do the same for Intel devices, we have a common structure which is copied and only the members that differ in specific SOCs are updated. You're right that it's not constant, but it avoids copy-paste of a rather large structure just to change a couple of lines.
Currently, I prefer to follow the same implementation as Intel devices. It's easier to see a different ops exists for mt8188 in sof_of_mt8188_desc and it really avoids copy-paste of a large structure.
Additionally, I found a typo in the next line.
sof_mt8186_ops.num_drv = ARRAY_SIZE(mt8188_dai); ^ This should be sof_mt8188_ops. I will correct it in V2.
Thanks, Trevor
On Mon, 2023-05-15 at 10:28 -0500, Pierre-Louis Bossart wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content.
On 5/15/23 10:05, Mark Brown wrote:
On Mon, May 15, 2023 at 01:25:44PM +0200, AngeloGioacchino Del Regno wrote:
Il 15/05/23 07:25, Trevor Wu ha scritto:
+{
- /* common defaults */
- memcpy(&sof_mt8188_ops, &sof_mt8186_ops, sizeof(struct
snd_sof_dsp_ops));
Never use sizeof(type), always use destination var size! Anyway, there's more.
OK, I will use sizeof(sof_mt8188_ops) instead.
I don't think we need to perform this memcpy: we'll never see an instance of both mt8186 and mt8188 drivers on the same machine, so you can safely just use sof_mt8186_ops for mt8188...
- sof_mt8188_ops.drv = mt8188_dai;
...which obviously means that this becomes sof_mt8186_ops.drv = mt8188_dai;
This does have the issue that it then means the ops struct isn't const which isn't ideal. It's also not the end of the world though so I don't have super strong feelings.
We do the same for Intel devices, we have a common structure which is copied and only the members that differ in specific SOCs are updated. You're right that it's not constant, but it avoids copy-paste of a rather large structure just to change a couple of lines.
Currently, I prefer to follow the same implementation as Intel devices. It's easier to see a different ops exists for mt8188 in sof_of_mt8188_desc and it really avoids copy-paste of a large structure.
Additionally, I found a typo in the next line.
sof_mt8186_ops.num_drv = ARRAY_SIZE(mt8188_dai); ^ This should be sof_mt8188_ops. I will correct it in V2.
Thanks, Trevor
Add mt8188 and mt8186 .dbg_dump callback to print some information when DSP panic occurs.
Signed-off-by: Trevor Wu trevor.wu@mediatek.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Yaochun Hung yc.hung@mediatek.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/mediatek/mt8186/mt8186.c | 22 ++++++++++++++++++++++ sound/soc/sof/mediatek/mt8186/mt8186.h | 5 +++++ 2 files changed, 27 insertions(+)
diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.c b/sound/soc/sof/mediatek/mt8186/mt8186.c index 3a9c81418c1f..bb59952885f6 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186.c +++ b/sound/soc/sof/mediatek/mt8186/mt8186.c @@ -24,6 +24,7 @@ #include "../../sof-of-dev.h" #include "../../sof-audio.h" #include "../adsp_helper.h" +#include "../mtk-adsp-common.h" #include "mt8186.h" #include "mt8186-clk.h"
@@ -473,6 +474,26 @@ static snd_pcm_uframes_t mt8186_pcm_pointer(struct snd_sof_dev *sdev, return pos; }
+static void mt8186_adsp_dump(struct snd_sof_dev *sdev, u32 flags) +{ + u32 dbg_pc, dbg_data, dbg_inst, dbg_ls0stat, dbg_status, faultinfo; + + /* dump debug registers */ + dbg_pc = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGPC); + dbg_data = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGDATA); + dbg_inst = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGINST); + dbg_ls0stat = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGLS0STAT); + dbg_status = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PDEBUGSTATUS); + faultinfo = snd_sof_dsp_read(sdev, DSP_REG_BAR, DSP_PFAULTINFO); + + dev_info(sdev->dev, "adsp dump : pc %#x, data %#x, dbg_inst %#x,", + dbg_pc, dbg_data, dbg_inst); + dev_info(sdev->dev, "ls0stat %#x, status %#x, faultinfo %#x", + dbg_ls0stat, dbg_status, faultinfo); + + mtk_adsp_dump(sdev, flags); +} + static struct snd_soc_dai_driver mt8186_dai[] = { { .name = "SOF_DL1", @@ -555,6 +576,7 @@ static struct snd_sof_dsp_ops sof_mt8186_ops = { .num_drv = ARRAY_SIZE(mt8186_dai),
/* Debug information */ + .dbg_dump = mt8186_adsp_dump, .debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
/* PM */ diff --git a/sound/soc/sof/mediatek/mt8186/mt8186.h b/sound/soc/sof/mediatek/mt8186/mt8186.h index 5b521c60b4e3..91323f492a1e 100644 --- a/sound/soc/sof/mediatek/mt8186/mt8186.h +++ b/sound/soc/sof/mediatek/mt8186/mt8186.h @@ -38,6 +38,11 @@ struct snd_sof_dev; #define DSP_MBOX3_IRQ_EN BIT(3) #define DSP_MBOX4_IRQ_EN BIT(4) #define DSP_PDEBUGPC 0x013C +#define DSP_PDEBUGDATA 0x0140 +#define DSP_PDEBUGINST 0x0144 +#define DSP_PDEBUGLS0STAT 0x0148 +#define DSP_PDEBUGSTATUS 0x014C +#define DSP_PFAULTINFO 0x0150 #define ADSP_CK_EN 0x1000 #define CORE_CLK_EN BIT(0) #define COREDBG_EN BIT(1)
participants (5)
-
AngeloGioacchino Del Regno -
Mark Brown -
Pierre-Louis Bossart -
Trevor Wu -
Trevor Wu (吳文良)