Re: [PATCH v1 9/9] ASoC: Intel: Boards: add support for HDA codecs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index ceb105c..d44cf1e 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -107,6 +107,7 @@ config SND_SOC_INTEL_SKYLAKE
   	select SND_HDA_DSP_LOADER
   	select SND_SOC_TOPOLOGY
   	select SND_SOC_INTEL_SST
+	select SND_SOC_HDAC_HDA if
SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH

This looks odd, it beats the purpose of the clean split between platform
and machine Kconfig added in 2017. You should select SND_SOC_HDAC_HDA in
the config for SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH

If I move into the machine driver Kconfig, then it will be loaded after
the SKL platform driver loads and so symbol dependency will create an issue.

hdac_hda needs to be loaded before the SKL driver, so that it can
get the ops from library. So I will have to load it manually in the platform
driver code. Let me try that, if it does not work, I will come back on this
topic.

I don't believe the Kconfig selection has any bearing on how the codecs are loaded. In fact the existing I2S-based machine drivers are a counter-example, the machine drivers depend on a codec driver in Kconfig, but the latter needs to be loaded first so that the machine driver finds the relevant DAIs.

+static const struct snd_kcontrol_new skl_hda_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Headphone"),
+	SOC_DAPM_PIN_SWITCH("Headset Mic"),
+};
+
+static const struct snd_soc_dapm_widget skl_hda_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone", NULL),
+	SND_SOC_DAPM_MIC("Headset Mic", NULL),
+	SND_SOC_DAPM_SPK("Codec Speaker", NULL),
+	SND_SOC_DAPM_MIC("Codec Mic", NULL),
+};

what about all the other outputs, e.g. line out? And how does this work
with pin retasking?

Good point.

The hdac_hda is just a wrapper around the legacy codec driver
and so it relies on the functionality of the legacy HDA codec driver
for all the functionality including pin re-tasking.

The widget names that you see above is just to complete the
DAPM route. Based on your comment I am planning to rename it as
following

Analog In Endpoint
Analog Output Endpoint
Digital In Endpoint
Digital Out Endpoint

and will connect it to the Codec Pins.

Also I think it makes sense to rename the codec Pin names accordingly

Codec Analog Input Pin
Codec Analog Output Pin
Codec Digital Input Pin
Codec Digital Output Pin

Humm, what if you have more than one analog input? It's almost as if this list should be created dynamically based on what is exposed by the codec, I don't see how a static list will cover all configurations.


+	err = snd_hdac_ext_bus_device_init(bus, addr, hdev);
+	if (err < 0)
+		return err;
+
+	if ((res & 0xFFFF0000) != 0x80860000)
+		hdev->type = HDA_DEV_LEGACY;

I don't get what this test does?

I am using the legacy HDA codec drivers only for the HDA codecs.
For iDisp I will continue to use the ASoC hdac_hdmi driver, which
is using ASOC BUS.

To make it more clear I will convert the if condition into inline
function with proper name.

As written in my previous email, this is fine for now but long term we'd want to avoid having duplication of functionality. Fixing issues in two locations is not efficient, I vote to deprecate hdac_hdmi at some point.

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux