Re: [PATCH 00/35] ASoC: Intel: Clenaup SST initialization - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Cezary Rojewski <cezary.rojewski@intel.com>,
	"Wasko, Michal" <michal.wasko@linux.intel.com>,
	Mark Brown <broonie@kernel.org>
Cc: "Wasko, Michal" <michal.wasko@intel.com>,
	lgirdwood@gmail.com, alsa-devel@alsa-project.org, tiwai@suse.com,
	"Kaczmarski, Filip" <filip.kaczmarski@intel.com>
Subject: Re: [PATCH 00/35] ASoC: Intel: Clenaup SST initialization
Date: Mon, 26 Aug 2019 16:57:23 -0500	[thread overview]
Message-ID: <47683473-c177-055b-03fe-2f01feeb4016@linux.intel.com> (raw)
In-Reply-To: <a95ac4d7-1c6a-ac19-59b3-cfb9ab058520@intel.com>

On 8/26/19 3:08 PM, Cezary Rojewski wrote:
> On 2019-08-26 18:51, Pierre-Louis Bossart wrote:
>>
>>
>> On 8/26/19 2:24 AM, Wasko, Michal wrote:
>>>
>>> On 8/25/2019 1:06 PM, Cezary Rojewski wrote:
>>>> On 2019-08-24 15:51, Cezary Rojewski wrote:
>>>>> On 2019-08-23 23:39, Mark Brown wrote:
>>>>>> On Fri, Aug 23, 2019 at 03:12:18PM -0500, Pierre-Louis Bossart wrote:
>>>>>>> On 8/23/19 1:44 PM, Cezary Rojewski wrote:
>>>>>>
>>>>>>>> Wasn't lying about FW version being unreliable. Let's say vendor
>>>>>>>> receives quick FW drop with new RCR.. such eng drop may carry 
>>>>>>>> invalid
>>>>>>>> numbers such as 0.0.0.0..
>>>>>>>> In general, I try to avoid relying on FW version whenever 
>>>>>>>> possible. It
>>>>>>>> can be dumped for debug reasons, true, but to be relied on? Not 
>>>>>>>> really.
>>>>>>
>>>>>>> Goodness, that's really bad. I didn't realize this.
>>>>>>
>>>>>> At a previous employer I modified our build stamping
>>>>>> infrastructure to also include both a timestamp and a serialized
>>>>>> build number in the version number since one of my colleagues was
>>>>>> fond of sending people prereleases of what he was working on to
>>>>>> other people with identical version numbers on different
>>>>>> binaries leading to much confusion and checksumming.  You do see
>>>>>> a lot of things with those serialized version numbers, especially
>>>>>> SVN based projects.
>>>>>>
>>>>>>>> Personally, I'm against all hardcodes and would simply recommend 
>>>>>>>> all
>>>>>>>> user to redirect their symlinks when they do switch kernel - 
>>>>>>>> along with
>>>>>>>> dumping warning/ error message in dmesg. Hardcodes bring 
>>>>>>>> problems with
>>>>>>>> forward compatibility and that's why host should offload them 
>>>>>>>> away to
>>>>>>>> FW.
>>>>>>
>>>>>>> Cezary, I know you are not responsible for all this, but at this 
>>>>>>> point if we
>>>>>>> (Intel) can't guarantee any sort of interoperability with both 
>>>>>>> firmware and
>>>>>>> topology we should make it clear that this driver is not 
>>>>>>> recommended unless
>>>>>>> specific versions of the firmware/topology are used, and as a 
>>>>>>> consequence
>>>>>>> the typical client distros and desktop/laptop users should use 
>>>>>>> HDaudio
>>>>>>> legacy or SOF (for DMICs)
>>>>>>
>>>>>> Not the most elegent solution but I'm wondering if keeping a copy
>>>>>> of the driver as is around and using new locations for the fixed
>>>>>> firmware might be the safest way to handle this.  We could have a
>>>>>> wrapper which tries to load the newer firmware and uses the fixed
>>>>>> driver code if that's there, otherwise tries the old driver with
>>>>>> the existing firmware paths.  This is obviously a horror show and
>>>>>> leaves the old code sitting there but given the mistakes that
>>>>>> have been made the whole situation looks like a house of cards.
>>>>>>
>>>>>
>>>>> Thanks for the feedback Mark. While I'm not yet on the "SOF will 
>>>>> fix this" train, I'm keen to agree to leaving this entirely to SOF 
>>>>> if it comes down to us duplicating /skylake.
>>>>>
>>>>> However, we are not going to give up that easily. I'll see if some 
>>>>> "golden config" hardcodes can't be provided in some legacy.c file 
>>>>> which would be fetched if initial setup fails. E.g.: 2cores, 3ssps, 
>>>>> 1PAGE_SIZE per trace buffer.. and such. There are quite a few 
>>>>> factors to take into consideration though. If "asking" user via 
>>>>> dmesg to upgrade the firmware if his/her setup contains obsolete 
>>>>> binary is really not an option, then some magic words got to be 
>>>>> involved.
>>>>>
>>>>> Czarek
>>>>
>>>> On the second thought what if instead of duplicating kernel code, 
>>>> binaries would be duplicated?
>>>> I.e. rather than targeting /intel/dsp_fw_cnl.bin, _new_ /skylake 
>>>> would be expecting /intel/dsp_fw_cnl_release.bin? Same with topology 
>>>> binaries.
>>>> In such case, we "only" need to figure out how to propagate new 
>>>> files to Linux distos so whenever someone updates their kernel, new 
>>>> binaries are already present in their /lib/firmware.
>>>>
>>>> If such option is valid, we can postpone /skylake upgrade till 5.4 
>>>> merging window closes and the patches (rough estimation is 150) 
>>>> would descend upon alsa-devel in time between 5.4 and 5.5.
>>>
>>> If the driver and FW update will be within the same kernel release 
>>> then IMHO
>>> there should be no compatibility problem between those two 
>>> components, right?
>>> This way kernel users willing to stick to old FW can stay on older 
>>> kernel version while
>>> others can update and receive all the latest FW functionality that 
>>> was developed and enabled.
>>
>> I am not comfortable with precluding a kernel update because of a 
>> single firmware file. There are all sort of reasons for updating a 
>> kernel, security, sideband attacks and Android CDD compatibility being 
>> the most obvious ones.
>>
>>>
>>> In terms of FW topology compatibility there is an option to read from 
>>> topology manifest
>>> a FW version that it was build for and in  case if it does not match 
>>> FW version present on
>>> the platform then print warning that the FW topology binary should be 
>>> rebuild for current
>>> FW version (x.x.x.x).
>>
>> Can you provide a pointer on how the FW version is embedded in a 
>> .conf/.tplg file? I see a couple where that information does not seem 
>> present.
>>
>>> The above approach at the end may be less confusing then source code 
>>> or binary duplication.
>>
> 
> Indeed. Our existing topology skips that part of internal .xml and thus 
> such information is not propagated to kernel.
> 
> Pierre, how about the binary-duplication - as described above? Btw, 
> that's not a single firmware file ^)^ We would immediately update all of 
> them, together with topologies.

I didn't understand how you would select the new firmwares? Some code 
change needs to happen as well?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel