ASoC: SOF: mediatek: Add machine driver dsp ops for mt8195 platform #3239

yaochunhung · 2021-10-26T06:40:20Z

Hi,
This patch is to support dsp ops callback to select and register machine driver.

dbaluta · 2021-10-26T08:31:59Z

sound/soc/sof/mediatek/mt8195/mt8195.c

+{
+	struct snd_sof_pdata *sof_pdata = sdev->pdata;
+	struct snd_soc_acpi_mach *mach;
+


My only concern here is that we aren't really using ACPI right?

yes, I don't use acpi and don't provide id for it.

plbossart · 2021-10-26T18:47:01Z

sound/soc/sof/mediatek/mt8195/mt8195.c

+	/* machine driver */
+	.machine_select = mt8195_machine_select,
+	.machine_register = sof_machine_register,
+


same comment as @dbaluta, in theory you should have a device tree device for the machine driver?

machine driver name "mt8195_mt6359_rt1019_rt5682" is defined in
(1). https://patchwork.kernel.org/project/linux-mediatek/patch/20210819084144.18483-9-trevor.wu@mediatek.com/
(2). for non-sof device tree binding :
https://patchwork.kernel.org/project/linux-mediatek/patch/20210818103215.9186-12-trevor.wu@mediatek.com/
(3). for sof dsp device tree binding :
https://patchwork.kernel.org/project/linux-mediatek/patch/20211025105635.30625-3-yc.hung@mediatek.com/
Thanks.

if you specify a driver for the machine, the SOF core will create a platform device.

But if you also have a DT binding for the machine driver, then you will create a second platform device, no?

Something seems wrong here?

@plbossart Sorry for confusion, currently we only use SOF core to create platform device for machine (dts binding is item (3) above.). We use mt8195_machine_select to get driver name then create it. We don't use device tree binding for "mt8195_mt6359_rt1019_rt5682" for SOF,

item(3) adsp: adsp@10803000 { compatible = "mediatek,mt8195-dsp"; ... sound { mediatek,dptx-codec = <&dp_tx>; mediatek,hdmi-codec = <&hdmi0>; mediatek,platform = <&afe>; };

The non-sof device tree binding(item (2) above) is used without SOF support and it will create platform device via device tree. Thanks.

item(2) sound: mt8195-sound { compatible = "mediatek,mt8195_mt6359_rt1019_rt5682"; mediatek,platform = <&afe>; pinctrl-names = "default";

Sorry @yaochunhung I am really confused. In previous PRs you described how the same card can include SOF and non-SOF managed streams. But above you seem to describe a case where the card is created in two different ways.

I don't get how this might work.

@ujfalusi Thanks for your detail descriptions. I will base on your suggestion to modify the DSP node as property of sound card node as below,

&sound { pinctrl-names = "default"; pinctrl-0 = <&aud_pins_default>; mediatek,adsp = <&adsp>; mediatek,platform =<&afe>; status = "okay"; }

where adsp is DSP node for dsp driver and afe is regular(non-sof) node for regular platform driver. Therefore, machine register won't be needed in this patch. But I still keep the machine_select to get sof_tplg_filename. Please feel free to tell me if I am misunderstanding. The machine driver code is in the link https://patchwork.kernel.org/project/linux-mediatek/patch/20211129141057.12422-4-trevor.wu@mediatek.com . Thanks.

@ujfalusi @plbossart Thanks for review. The machine driver is applied to mark/for-next branch as the link https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?id=3d00d2c07f04f47aa4228700b440ac47abf13853. Please help to check if you have any concern or suggestions about this PR. Thanks.

I agree with @dbaluta and @plbossart that the machine_select should not be used, but you should be using the of match data in a similar way iMX is doing.

@dbaluta I check this function sof_machine_check and snd_sof_machine_select should return mach to avoid fail. Could you please advice how to avoid it without providing machine_select callback on iMX? Otherwise, I think sof_tplg_filename is also needed, but I don't know how to assign it without call machine_select . Thanks.

@yaochunhung I think this might be useful. https://www.spinics.net/lists/devicetree/msg433908.html

But I hit a roadblock and didn't get a chance to find a better solution.

I think for now at least we can have a default_tplg_filename in sof_dev_desc and select it based on probed platform.

lyakh · 2021-11-26T11:29:47Z

sound/soc/sof/mediatek/mt8195/mt8195.c

+	struct snd_soc_acpi_mach *mach;
+
+	mach = &sof_mt8195_mach;
+	if (!mach) {


this is impossible - as long as sof_mt8195_mach is a "normal real" object

@lyakh Thanks. I will remove it.

Add dsp ops callback to machine select to get sof topolgy file name Signed-off-by: YC Hung <yc.hung@mediatek.com>

yaochunhung · 2021-12-17T01:01:48Z

@plbossart @ujfalusi @lyakh Thanks for review, please help to check if there are any suggestions or concerns about this PR. Thanks a lot.

yaochunhung · 2022-01-04T00:45:09Z

@plbossart @ujfalusi @lyakh @dbaluta Friendly reminders. Do you have a chance to look at this patch and give comments if any? Thanks a lot.

ujfalusi · 2022-01-04T08:43:13Z

sound/soc/sof/mediatek/mt8195/mt8195.c

+	/* machine driver */
+	.machine_select = mt8195_machine_select,
+	.machine_register = sof_machine_register,
+


I agree with @dbaluta and @plbossart that the machine_select should not be used, but you should be using the of match data in a similar way iMX is doing.

plbossart · 2022-05-17T21:56:13Z

@yaochunhung is this still relevant? Last update was in January 2022.

chunxu-li · 2022-05-19T06:48:44Z

@plbossart
It looks like the machine_select callback is not optional, the callback must be implemented, otherwise sof can only support NOCODEC_MODE as the restrictions in function sof_machine_check .

ujfalusi · 2022-05-19T10:51:42Z

@chunxu-li, but how iMX is working? Is it using nocodec? @dbaluta, can you clarify this?

yaochunhung · 2022-06-08T02:48:01Z

@ujfalusi I think @dbaluta provide IMX's solution like #3239 (comment). Thanks.

plbossart · 2022-08-09T08:16:42Z

@yaochunhung this PR has been marked as 'Unclear' for several weeks. Unless there's an update and/or a need for comments, we should close this PR. You can always resubmit a new one later, it's just not efficient to leave such PRs dangling without any progress.

yaochunhung · 2022-08-09T08:57:56Z

@plbossart I will close this PR. Thanks

yaochunhung requested review from bardliao, dbaluta, kv2019i, lgirdwood, plbossart and ranj063 as code owners October 26, 2021 06:40

dbaluta reviewed Oct 26, 2021

View reviewed changes

plbossart reviewed Oct 26, 2021

View reviewed changes

yaochunhung force-pushed the mt8195-mach-ops branch from f36194f to 803ee10 Compare November 22, 2021 06:48

lyakh reviewed Nov 26, 2021

View reviewed changes

ASoC: SOF: mediatek: Add machine driver dsp ops for mt8195 platform

e5c7d59

Add dsp ops callback to machine select to get sof topolgy file name Signed-off-by: YC Hung <yc.hung@mediatek.com>

yaochunhung force-pushed the mt8195-mach-ops branch from 803ee10 to e5c7d59 Compare November 29, 2021 12:58

ujfalusi requested changes Jan 4, 2022

View reviewed changes

plbossart added the Unclear No agreement on problem statement and resolution label Jun 21, 2022

yaochunhung closed this Aug 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ASoC: SOF: mediatek: Add machine driver dsp ops for mt8195 platform #3239

ASoC: SOF: mediatek: Add machine driver dsp ops for mt8195 platform #3239

yaochunhung commented Oct 26, 2021

dbaluta Oct 26, 2021

yaochunhung Oct 26, 2021

plbossart Oct 26, 2021

yaochunhung Oct 27, 2021 •

edited

Loading

plbossart Oct 27, 2021

yaochunhung Oct 28, 2021 •

edited

Loading

plbossart Nov 3, 2021

yaochunhung Nov 29, 2021 •

edited

Loading

yaochunhung Dec 7, 2021

ujfalusi Jan 4, 2022

yaochunhung Jan 4, 2022

dbaluta Jan 12, 2022

lyakh Nov 26, 2021

yaochunhung Nov 29, 2021

yaochunhung commented Dec 17, 2021

yaochunhung commented Jan 4, 2022 •

edited

Loading

ujfalusi Jan 4, 2022

plbossart commented May 17, 2022

chunxu-li commented May 19, 2022

ujfalusi commented May 19, 2022

yaochunhung commented Jun 8, 2022

plbossart commented Aug 9, 2022

yaochunhung commented Aug 9, 2022

ASoC: SOF: mediatek: Add machine driver dsp ops for mt8195 platform #3239

ASoC: SOF: mediatek: Add machine driver dsp ops for mt8195 platform #3239

Conversation

yaochunhung commented Oct 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaochunhung Oct 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaochunhung Oct 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaochunhung Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaochunhung commented Dec 17, 2021

yaochunhung commented Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

plbossart commented May 17, 2022

chunxu-li commented May 19, 2022

ujfalusi commented May 19, 2022

yaochunhung commented Jun 8, 2022

plbossart commented Aug 9, 2022

yaochunhung commented Aug 9, 2022

yaochunhung Oct 27, 2021 •

edited

Loading

yaochunhung Oct 28, 2021 •

edited

Loading

yaochunhung Nov 29, 2021 •

edited

Loading

yaochunhung commented Jan 4, 2022 •

edited

Loading