split CFL, CML, CNL firmware names #1393

plbossart · 2019-10-29T14:44:55Z

we need to split those firmwares due to signature issues
this replaces #1278

libinyang

LGTM

plbossart · 2019-10-29T15:25:34Z

@xiulipan you'll need to change the CI settings, we still check for CNL firmware on CML hardware:

2019-10-29 15:01:56 UTC [REM_INFO]: Checking file: /lib/firmware/intel/sof/sof-cnl.ri
2019-10-29 15:01:56 UTC [REM_INFO]: Found file: /lib/firmware/intel/sof/sof-cnl.ri

ranj063 · 2019-10-29T16:20:54Z

@plbossart do we also need the corresponding changes in sof-pci-dev.c to add a cml_desc?

plbossart · 2019-10-29T17:02:52Z

the changes are already there:

static const struct sof_dev_desc cml_desc = {
	.machines		= snd_soc_acpi_intel_cnl_machines,		.machines		= snd_soc_acpi_intel_cnl_machines,
	.resindex_lpe_base	= 0,		.resindex_lpe_base	= 0,
	.resindex_pcicfg_base	= -1,		.resindex_pcicfg_base	= -1,
	.resindex_imr_base	= -1,		.resindex_imr_base	= -1,
	.irqindex_host_ipc	= -1,		.irqindex_host_ipc	= -1,
	.resindex_dma_base	= -1,		.resindex_dma_base	= -1,
	.chip_info = &cnl_chip_info,		.chip_info = &cnl_chip_info,
	.default_fw_path = "intel/sof",		.default_fw_path = "intel/sof",
	.default_tplg_path = "intel/sof-tplg",		.default_tplg_path = "intel/sof-tplg",
	.nocodec_fw_filename = "sof-cnl.ri",		.nocodec_fw_filename = "sof-cml.ri",

ranj063 · 2019-10-29T17:04:12Z

	.machines		= snd_soc_acpi_intel_cnl_machines,		.machines		= snd_soc_acpi_intel_cnl_machines,

@plbossart this is what Im confused about. Shouldnt this be snd_soc_acpi_intel_cml_machines with your new addition?

plbossart · 2019-10-29T17:05:11Z

	.machines		= snd_soc_acpi_intel_cnl_machines,		.machines		= snd_soc_acpi_intel_cnl_machines,
@plbossart this is what Im confused about. Shouldnt this be snd_soc_acpi_intel_cml_machines with your new addition?

ah yes, that's just wrong. Thanks for spotting this.

sound/soc/sof/sof-pci-dev.c

RanderWang · 2019-10-30T01:29:36Z

@plbossart , do we need to split cml from cnl ? please check our sdw table https://github.com/thesofproject/linux/blob/integration/soundwire-latest/sound/soc/intel/common/soc-acpi-intel-cnl-match.c#L68. we could just split cnl table to cnl & cml table, and I still consider cml as a cnl platform

xiulipan · 2019-10-30T02:06:56Z

@plbossart Thanks for the note. We will try to come up with a smooth transition method.

plbossart · 2019-10-30T20:16:35Z

@fredoh9 the CI tests should break on CFL, will need your help to add sof-cnl.ri -> sof-cfl.ri symlink

RanderWang

LGTM

kv2019i · 2019-10-31T12:06:20Z

sound/soc/intel/common/soc-acpi-intel-cfl-match.c

+struct snd_soc_acpi_mach snd_soc_acpi_intel_cfl_machines[] = {
+	{},
+};
+EXPORT_SYMBOL_GPL(snd_soc_acpi_intel_cfl_machines);


Putting an empty table looks a bit odd, but given there will be entryes, ++votes still.

plbossart · 2019-10-31T15:43:03Z

@fredoh9 can you help recheck the CFL and CML HDA cases. I see messages such as

[    9.302490 &lt;    0.226217&gt;] sof-audio-pci 0000:00:1f.3: Direct firmware load for intel/sof/(null) failed with error -2
[    9.302497 &lt;    0.000007&gt;] sof-audio-pci 0000:00:1f.3: error: request firmware intel/sof/(null) failed err: -2
[    9.302589 &lt;    0.000092&gt;] sof-audio-pci 0000:00:1f.3: error: failed to load DSP firmware -2
[    9.302656 &lt;    0.000067&gt;] sof-audio-pci 0000:00:1f.3: error: sof_probe_work failed err: -2

Not sure if my fallback worked here.

ranj063 · 2019-10-31T15:52:30Z

sound/soc/sof/intel/hda.c

 			mach = pdata->desc->machines;
-			pdata->fw_filename = mach->sof_fw_filename;
+			if (mach)


@plbossart Im not sure an empty struct is equivalent to NULL. Maybe check for ARRAY_SIZE() is a better option?

yes, that was my fear when I wrote the code, but then we also test for the NULL case when we reach the last empty structure in the ACPI table, when we scan for possible machines. That's why I'd want someone to test on CFL w/ HDA.

@plbossart one way to make sure that this is null is not to set the "machines" field until we have entries in the cfl table.
But in my multi-client series, I fixed this problem by adding a default_fw_filename field to desc because the firmware is loaded long before the acpi match is done by the audio client. But thats for another day.

right, and for now I am stuck due to this other issue:
#1417

whoever invented signed firmware should buy free beer for life to all developers.

if (mach->id[0]) is what we used for acpi scans, so fixed the code to use the same thing

Due to firmware manifest/signature differences, we have to use different firmware names, so split CNL machine table in three (CNL, CFL, CML). The CFL table is currently empty since all known platforms use HDaudio, but let's plan ahead. Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The manifest information is different between CNL, CML and CFL platforms hence we need to load different files. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

We have platforms such as CFL with no known I2S codec being used, and the ACPI tables are currently empty, so fall-back to using the firmware filename used in nocodec mode Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

plbossart · 2019-11-01T01:37:04Z

@fredoh9 @xiulipan CFL and CML HDA failed with firmware name changes.

RanderWang · 2019-11-01T02:41:44Z

@plbossart I got a CFL device and test this PR. " if (mach)" always return true, and " if (mach->id[0])" works. How about change it to

			if (mach->sof_fw_filename)
				pdata->fw_filename = mach->sof_fw_filename;
			else
				pdata->fw_filename =
					pdata->desc->nocodec_fw_filename;

xiulipan · 2019-11-01T03:57:57Z

@plbossart We are working on this. I hope we do not get some sudden change to broke previous version.
But if we find we can not give the support before next weed, we may get some hard switch.

plbossart · 2019-11-01T14:16:39Z

@plbossart I got a CFL device and test this PR. " if (mach)" always return true, and " if (mach->id[0])" works. How about change it to
			if (mach->sof_fw_filename)
				pdata->fw_filename = mach->sof_fw_filename;
			else
				pdata->fw_filename =
					pdata->desc->nocodec_fw_filename;

I guess that would probably work. I still prefer mach->id[0] for consistency with the code in soc-acpi.h

plbossart requested review from bardliao, lgirdwood, libinyang and lyakh as code owners October 29, 2019 14:44

plbossart mentioned this pull request Oct 29, 2019

ASoC: SOF: Intel: Fix CFL and CML FW binary names. #1278

Closed

jajanusz previously approved these changes Oct 29, 2019

View reviewed changes

libinyang previously approved these changes Oct 29, 2019

View reviewed changes

RanderWang reviewed Oct 30, 2019

View reviewed changes

sound/soc/sof/sof-pci-dev.c Show resolved Hide resolved

plbossart dismissed stale reviews from libinyang and jajanusz via 6d7c562 October 30, 2019 20:14

plbossart force-pushed the fix/firmware-names branch from d4c5beb to 6d7c562 Compare October 30, 2019 20:14

RanderWang previously approved these changes Oct 31, 2019

View reviewed changes

kv2019i previously approved these changes Oct 31, 2019

View reviewed changes

ranj063 reviewed Oct 31, 2019

View reviewed changes

plbossart and others added 3 commits October 31, 2019 20:03

plbossart dismissed stale reviews from kv2019i and RanderWang via 3e3415d November 1, 2019 01:03

plbossart force-pushed the fix/firmware-names branch from 6d7c562 to 3e3415d Compare November 1, 2019 01:03

ranj063 approved these changes Nov 1, 2019

View reviewed changes

plbossart merged commit 1cae799 into thesofproject:topic/sof-dev Nov 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

split CFL, CML, CNL firmware names #1393

split CFL, CML, CNL firmware names #1393

plbossart commented Oct 29, 2019

libinyang left a comment

plbossart commented Oct 29, 2019

ranj063 commented Oct 29, 2019

plbossart commented Oct 29, 2019

ranj063 commented Oct 29, 2019

plbossart commented Oct 29, 2019

RanderWang commented Oct 30, 2019

xiulipan commented Oct 30, 2019

plbossart commented Oct 30, 2019

RanderWang left a comment

kv2019i Oct 31, 2019

plbossart commented Oct 31, 2019

ranj063 Oct 31, 2019

plbossart Oct 31, 2019 •

edited

Loading

ranj063 Oct 31, 2019

plbossart Oct 31, 2019

plbossart Nov 1, 2019

plbossart commented Nov 1, 2019

RanderWang commented Nov 1, 2019

xiulipan commented Nov 1, 2019

plbossart commented Nov 1, 2019

split CFL, CML, CNL firmware names #1393

split CFL, CML, CNL firmware names #1393

Conversation

plbossart commented Oct 29, 2019

libinyang left a comment

Choose a reason for hiding this comment

plbossart commented Oct 29, 2019

ranj063 commented Oct 29, 2019

plbossart commented Oct 29, 2019

ranj063 commented Oct 29, 2019

plbossart commented Oct 29, 2019

RanderWang commented Oct 30, 2019

xiulipan commented Oct 30, 2019

plbossart commented Oct 30, 2019

RanderWang left a comment

Choose a reason for hiding this comment

kv2019i Oct 31, 2019

Choose a reason for hiding this comment

plbossart commented Oct 31, 2019

ranj063 Oct 31, 2019

Choose a reason for hiding this comment

plbossart Oct 31, 2019 • edited Loading

Choose a reason for hiding this comment

ranj063 Oct 31, 2019

Choose a reason for hiding this comment

plbossart Oct 31, 2019

Choose a reason for hiding this comment

plbossart Nov 1, 2019

Choose a reason for hiding this comment

plbossart commented Nov 1, 2019

RanderWang commented Nov 1, 2019

xiulipan commented Nov 1, 2019

plbossart commented Nov 1, 2019

plbossart Oct 31, 2019 •

edited

Loading