diff mbox series

[v7,6/9] ASoC: Intel: bxt-da7219-max98357a: common hdmi codec support

Message ID 20191023090331.10531-7-kai.vehmanen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series adapt SOF to use snd-hda-codec-hdmi | expand

Commit Message

Kai Vehmanen Oct. 23, 2019, 9:03 a.m. UTC
Add support for using snd-hda-codec-hdmi driver for HDMI/DP
instead of ASoC hdac-hdmi. This is aligned with how other
HDA codecs are already handled.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/intel/boards/Makefile               |  2 +-
 sound/soc/intel/boards/bxt_da7219_max98357a.c | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Takashi Iwai Oct. 28, 2019, 4:58 p.m. UTC | #1
On Wed, 23 Oct 2019 11:03:28 +0200,
Kai Vehmanen wrote:
> 
> @@ -4,7 +4,7 @@ snd-soc-sst-byt-rt5640-mach-objs := byt-rt5640.o
>  snd-soc-sst-byt-max98090-mach-objs := byt-max98090.o
>  snd-soc-sst-bdw-rt5677-mach-objs := bdw-rt5677.o
>  snd-soc-sst-broadwell-objs := broadwell.o
> -snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o
> +snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o hda_dsp_common.o

Hrm, this can be a problem.  I see there are multiple drivers that are
built with this object.  When they are built as modules and more than
one module get loaded on a system, it'll lead to a conflict because
both modules try to put the same stuff.

So, hda_dsp_common.o should be in the common helper module that is
used by both drivers, or we need other trick.

But I'm not entirely sure whether this is true on the recent kernel
build.  At least it *was* a problem in the past.

In anyway, please try to load the two modules on your system and check
whether the module loading works.


thanks,

Takashi
Pierre-Louis Bossart Oct. 28, 2019, 5:24 p.m. UTC | #2
On 10/28/19 11:58 AM, Takashi Iwai wrote:
> On Wed, 23 Oct 2019 11:03:28 +0200,
> Kai Vehmanen wrote:
>>
>> @@ -4,7 +4,7 @@ snd-soc-sst-byt-rt5640-mach-objs := byt-rt5640.o
>>   snd-soc-sst-byt-max98090-mach-objs := byt-max98090.o
>>   snd-soc-sst-bdw-rt5677-mach-objs := bdw-rt5677.o
>>   snd-soc-sst-broadwell-objs := broadwell.o
>> -snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o
>> +snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o hda_dsp_common.o
> 
> Hrm, this can be a problem.  I see there are multiple drivers that are
> built with this object.  When they are built as modules and more than
> one module get loaded on a system, it'll lead to a conflict because
> both modules try to put the same stuff.
> 
> So, hda_dsp_common.o should be in the common helper module that is
> used by both drivers, or we need other trick.
> 
> But I'm not entirely sure whether this is true on the recent kernel
> build.  At least it *was* a problem in the past.
> 
> In anyway, please try to load the two modules on your system and check
> whether the module loading works.

along the same lines, we have a Kconfig issue that I didn't see earlier.
These bxt parts will only compile with the Skylake driver with the 
upstream code, the following test applies:

if SND_SOC_INTEL_APL

In the SOF tree, we have a different test:

if SND_SOC_INTEL_APL || (SND_SOC_SOF_APOLLOLAKE && SND_SOC_SOF_HDA_LINK)

I kept this change back on purpose since we can only use APL+SOF on 
chromebooks with an experimental CoreBoot and only on pre-production 
hardware, but what this means is that GLK devices wouldn't work with 
SOF...I'll add this fix to my Kconfig update series.

Also I wonder if anyone tested bxt_rt298 with SOF? I see a topology for 
it but I've never seen any test results from anyone.


> 
> 
> thanks,
> 
> Takashi
>
Kai Vehmanen Oct. 28, 2019, 5:33 p.m. UTC | #3
Hi,

thanks for the review!

On Mon, 28 Oct 2019, Takashi Iwai wrote:

> On Wed, 23 Oct 2019 11:03:28 +0200,
> Kai Vehmanen wrote:
> > -snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o
> > +snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o hda_dsp_common.o
> 
> Hrm, this can be a problem.  I see there are multiple drivers that are
> built with this object.  When they are built as modules and more than
> one module get loaded on a system, it'll lead to a conflict because
> both modules try to put the same stuff.
[...]
> But I'm not entirely sure whether this is true on the recent kernel
> build.  At least it *was* a problem in the past.
> 
> In anyway, please try to load the two modules on your system and check
> whether the module loading works.

hmm, did a quick test and no problems seen, all modules load fine.

Another option is to go back to solution in v6 of the series and
have the helper functions inlines. But it seems v7 works as well, so maybe 
we'll stick with this.

Br, Kai
Kai Vehmanen Oct. 28, 2019, 5:55 p.m. UTC | #4
Hey,

On Mon, 28 Oct 2019, Pierre-Louis Bossart wrote:
> > But I'm not entirely sure whether this is true on the recent kernel
> > build.  At least it *was* a problem in the past.
> > 
> > In anyway, please try to load the two modules on your system and check
> > whether the module loading works.
> 
> along the same lines, we have a Kconfig issue that I didn't see earlier.
> These bxt parts will only compile with the Skylake driver with the upstream
> code, the following test applies:
> 
> if SND_SOC_INTEL_APL
[...]
> I kept this change back on purpose since we can only use APL+SOF on
> chromebooks with an experimental CoreBoot and only on pre-production hardware,
> but what this means is that GLK devices wouldn't work with SOF...I'll add this
> fix to my Kconfig update series.

ack, that's a good catch. I'll keep this machine driver change in
the series anyways (I want to include all machine drivers that are in 
upstream kernel and have the HDMI setup code).

> Also I wonder if anyone tested bxt_rt298 with SOF? I see a topology for it but
> I've never seen any test results from anyone.

No info about this. The patch in my series for bxt_rt298 is a mechanical 
update (it has the HDMI code for hdac-hdmi and it can be used with SOF/GLK 
-> driver updated in the series).

Br, Kai
Takashi Iwai Oct. 28, 2019, 6:04 p.m. UTC | #5
On Mon, 28 Oct 2019 18:33:44 +0100,
Kai Vehmanen wrote:
> 
> Hi,
> 
> thanks for the review!
> 
> On Mon, 28 Oct 2019, Takashi Iwai wrote:
> 
> > On Wed, 23 Oct 2019 11:03:28 +0200,
> > Kai Vehmanen wrote:
> > > -snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o
> > > +snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o hda_dsp_common.o
> > 
> > Hrm, this can be a problem.  I see there are multiple drivers that are
> > built with this object.  When they are built as modules and more than
> > one module get loaded on a system, it'll lead to a conflict because
> > both modules try to put the same stuff.
> [...]
> > But I'm not entirely sure whether this is true on the recent kernel
> > build.  At least it *was* a problem in the past.
> > 
> > In anyway, please try to load the two modules on your system and check
> > whether the module loading works.
> 
> hmm, did a quick test and no problems seen, all modules load fine.
> 
> Another option is to go back to solution in v6 of the series and
> have the helper functions inlines. But it seems v7 works as well, so maybe 
> we'll stick with this.

Hrm, then it might be built-in + module mixed case.  If that works, it
implies that my concern can be ignored.  Let's see.


thanks,

Takashi
diff mbox series

Patch

diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index 52e990b16b0d..0cf4a984f083 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -4,7 +4,7 @@  snd-soc-sst-byt-rt5640-mach-objs := byt-rt5640.o
 snd-soc-sst-byt-max98090-mach-objs := byt-max98090.o
 snd-soc-sst-bdw-rt5677-mach-objs := bdw-rt5677.o
 snd-soc-sst-broadwell-objs := broadwell.o
-snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o
+snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o hda_dsp_common.o
 snd-soc-sst-bxt-rt298-objs := bxt_rt298.o
 snd-soc-sst-glk-rt5682_max98357a-objs := glk_rt5682_max98357a.o
 snd-soc-sst-bytcr-rt5640-objs := bytcr_rt5640.o
diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c
index ac1dea5f9d11..5873abb46441 100644
--- a/sound/soc/intel/boards/bxt_da7219_max98357a.c
+++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c
@@ -21,6 +21,7 @@ 
 #include "../../codecs/da7219.h"
 #include "../../codecs/da7219-aad.h"
 #include "../common/soc-intel-quirks.h"
+#include "hda_dsp_common.h"
 
 #define BXT_DIALOG_CODEC_DAI	"da7219-hifi"
 #define BXT_MAXIM_CODEC_DAI	"HiFi"
@@ -38,6 +39,7 @@  struct bxt_hdmi_pcm {
 
 struct bxt_card_private {
 	struct list_head hdmi_pcm_list;
+	bool common_hdmi_codec_drv;
 };
 
 enum {
@@ -615,6 +617,13 @@  static int bxt_card_late_probe(struct snd_soc_card *card)
 		snd_soc_dapm_add_routes(&card->dapm, broxton_map,
 					ARRAY_SIZE(broxton_map));
 
+	pcm = list_first_entry(&ctx->hdmi_pcm_list, struct bxt_hdmi_pcm,
+			       head);
+	component = pcm->codec_dai->component;
+
+	if (ctx->common_hdmi_codec_drv)
+		return hda_dsp_hdmi_build_controls(card, component);
+
 	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
 		component = pcm->codec_dai->component;
 		snprintf(jack_name, sizeof(jack_name),
@@ -720,6 +729,8 @@  static int broxton_audio_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ctx->common_hdmi_codec_drv = mach->mach_params.common_hdmi_codec_drv;
+
 	return devm_snd_soc_register_card(&pdev->dev, &broxton_audio_card);
 }