Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Jaroslav Kysela <perex@perex.cz>,
	ALSA development <alsa-devel@alsa-project.org>
Cc: Takashi Iwai <tiwai@suse.de>,
	Cezary Rojewski <cezary.rojewski@intel.com>
Subject: Re: [alsa-devel] [PATCH v3] ALSA: hda: add Intel DSP configuration / probe code
Date: Mon, 7 Oct 2019 09:05:17 -0500
Message-ID: <fd997c93-c7c9-6ede-90c2-a94df93a613e@linux.intel.com> (raw)
In-Reply-To: <20191006152232.17701-1-perex@perex.cz>



On 10/6/19 10:22 AM, Jaroslav Kysela wrote:
> For distributions, we need one place where we can decide
> which driver will be activated for the auto-configation of the
> Intel's HDA hardware with DSP. Actually, we cover three drivers:
> 
> * Legacy HDA
> * Intel SST
> * Intel Sound Open Firmware (SOF)
> 
> All those drivers registers similar PCI IDs, so the first
> driver probed from the PCI stack can win. But... it is not
> guaranteed that the correct driver wins.
> 
> This commit changes Intel's NHLT ACPI module to a common
> DSP probe module for the Intel's hardware. All above sound
> drivers calls this code. The user can force another behaviour
> using the module parameter 'dsp_driver' located in
> the 'snd-intel-dspcfg' module.
> 
> This change allows to add specific dmi checks for the specific
> systems. The examples are taken from the pull request:
> 
>    https://github.com/thesofproject/linux/pull/927
> 
> Tested on Lenovo Carbon X1 7th gen.

Thanks Jaroslav, I like the ideas in the patch, the flags+DMI table is 
quite elegant.

We will need to do additional checks for the quirks, e.g. I know there 
is a 'Phaser' Chromebook with HDaudio and I don't recall if they use 
DMICs. I also don't know if we always have the NHTL information when the 
legacy BIOS is used.

It's likely that we will have multiple iterations before getting this 
right. And we'll have to add SoundWire support as well (which isn't that 
hard, I already have all the ACPI parsing needed to detect if a link is 
enabled).

Some additional comments below.

> +module_param(dsp_driver, int, 0444);
> +MODULE_PARM_DESC(dsp_driver, "Force the DSP driver for Intel DSP (0=auto, 1=legacy, 2=SST, 3=SOF)");
> +
> +#define FLAG_SST		(1<<0)
> +#define FLAG_SOF		(1<<1)
> +#define FLAG_SOF_ONLY_IF_DMIC	(1<<16)
> +
> +struct config_entry {
> +	u32 flags;
> +	u16 device;
> +	const struct dmi_system_id *dmi_table;
> +};
> +
> +/*
> + * configuration table - the order of entries is important!

It's not really the order but the exclusion between SST and SOF for the 
same PCI ID?

> + */
> +static const struct config_entry config_table[] = {
> +/* Cometlake-LP */
> +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CML_LP)
> +	{
> +		/* prefer SST */
> +		.flags = FLAG_SST,
> +		.device = 0x02c8,
> +	},
> +#elif IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_LP)
> +	{
> +		.flags = FLAG_SOF,

need to add FLAG_SOF_ONLY_IF_DMIC

> +		.device = 0x02c8,
> +	},
> +#endif
> +/* Cometlake-H */
> +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CML_H)
> +	{
> +		.flags = FLAG_SST,
> +		.device = 0x06c8,
> +	},
> +#elif IS_ENABLED(CONFIG_SND_SOC_SOF_COMETLAKE_H)
> +	{
> +		.flags = FLAG_SOF,

| FLAG_SOF_ONLY_IF_DMIC

> +		.device = 0x06c8,
> +	},
> +#endif
> +/* Merrifield */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_MERRIFIELD)
> +	{
> +		.flags = FLAG_SOF,
> +		.device = 0x119a,
> +	},
> +#endif
> +/* Broxton-T */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
> +	{
> +		.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,
> +		.device = 0x1a98,
> +	},
> +#endif
> +/* Geminilake */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_GEMINILAKE)
> +	{
> +		.flags = FLAG_SOF,

can we have more than one table per PCI ID? e.g. in this case it'd be 
good to have the DMIC case separate from Google.

> +		.device = 0x3198,
> +		.dmi_table = (const struct dmi_system_id []) {
> +			{
> +				.ident = "Google Chromebooks",
> +				.matches = {
> +					DMI_MATCH(DMI_SYS_VENDOR, "Google"),
> +				}
> +			},
> +			{}
> +		}
> +	},
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_GLK)

should it be elif, as done for CometLake/CML?

> +	{
> +		.flags = FLAG_SST,
> +		.device = 0x3198,
> +	},
> +#endif
> +/* Icelake */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_ICELAKE)
> +	{
> +		.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,
> +		.device = 0x34c8,
> +	},
> +#endif
> +/* Elkhart Lake */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_ELKHARTLAKE)
> +	{
> +		.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,
> +		.device = 0x4b55,
> +	},
> +#endif
> +/* Appololake (Broxton-P) */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
> +	{
> +		.flags = FLAG_SOF,
> +		.device = 0x5a98,
> +		.dmi_table = (const struct dmi_system_id []) {
> +			{
> +				.ident = "Up Squared",
> +				.matches = {
> +					DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
> +					DMI_MATCH(DMI_BOARD_NAME, "UP-APL01"),
> +				}
> +			},
> +			{}
> +		}
> +	},
> +#endif
> +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_APL)

elif?

> +	{
> +		.flags = FLAG_SST,
> +		.device = 0x5a98,
> +	},
> +#endif
> +/* Cannonlake */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_CANNONLAKE)
> +	{
> +		.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,
> +		.device = 0x9dc8,
> +	},
> +#endif
> +/* Sunrise Point-LP */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_SKYLAKE)
> +	{
> +		.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,
> +		.device = 0x9d70,
> +	},
> +#endif
> +/* Kabylake-LP */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_KABYLAKE)
> +	{
> +		.flags = FLAG_SOF | FLAG_SOF_ONLY_IF_DMIC,
> +		.device = 0x9d71,
> +	},
> +#endif

SKL and SKL are not supported by SOF for now, and SST doesn't support 
HDaudio+DMIC combinations

This should be FLAG_SST but only for Google Chromebooks, e.g.


		.flags = FLAG_SST,
		.device = 0x9d70 or 0x9d71
		.dmi_table = (const struct dmi_system_id []) {
			{
				.ident = "Google Chromebooks",
				.matches = {
					DMI_MATCH(DMI_SYS_VENDOR, "Google"),
				}
			},
			{}
		}
	},

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

  parent reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-06 15:22 Jaroslav Kysela
2019-10-07  1:50 ` Takashi Iwai
2019-10-07 16:13   ` Jaroslav Kysela
2019-10-07 14:05 ` Pierre-Louis Bossart [this message]
2019-10-07 16:03   ` Jaroslav Kysela
2019-10-07 16:44     ` Pierre-Louis Bossart
2019-10-08 18:44     ` Pierre-Louis Bossart
2019-10-09  6:26       ` Jaroslav Kysela

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fd997c93-c7c9-6ede-90c2-a94df93a613e@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=cezary.rojewski@intel.com \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git