| Message ID | 20200305145314.32579-1-cezary.rojewski@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ASoC: Intel: Skylake: Fix HDaudio and Dmic | expand |
Hi Cezary, Thank you for this series. You should have mentioned that this fixes an issue that's been around since September 2018, and for which there's currently no solution on KabyLake/AmberLake platforms - PCI_DEVICE(0x8086, 0x9D71) https://bugzilla.kernel.org/show_bug.cgi?id=201251 > This patchset has been prepared internally for topmost linux-stable 5.5 > and 4.20 (no 4.19 as skl_hda_dsp did not exist there yet). > > Apart from our RVPs, we have run tests also on: > - KBL Lenovo Carbon X1 > - SKL Dell XPS 9350 > - WHL Acer Swift 5 > > Honestly, I'd see HDaudio related patches being backport as low as 4.20 > (although some changes had to be adjusted due to base differences > between 4.20 and 5.5, can share these too). One could argue HDA + Dmic > configuration should be available on 4.19 too - it's an LTS after all. > However, that time, some changes could be counted as "feature" rather > than fixes. Awaiting your replies and thoughts on that. > > In consequence, I've appended "Fixes" only for last two patches for now > - once decisions are made, can append adequate tags wherever necessary. There's been a couple of accidental regressions already on stable, now fixed, and my understanding is that the bar for inclusion is higher, so let's assume this counts as a new feature: it never worked before with an upstream kernel and distributions haven't had access to topology files either. If a specific distribution wants to backport on top of a -stable version, that's entirely possible but that would be their decision. They would not only need to update the kernel but topology and UCM as well. One comment though: in the absence of blacklist/voodoo magic, this solution will not be selected with GLK+/WHL, the default with dmics is SOF. Even on KBL, the legacy driver would be selected, we only select the SST driver for SKL and KBL Chromebooks. You would have to modify the dsp-config stuff [1] to load the skl driver on KBL, which likely requires a FLAG_SST_ONLY_IF_DMIC definition, and probably add a Kconfig since SOF will at some point support KBL, and we want to prevent conflicts between PCI drivers registered for the same ID. The order in that table might be enough though, it's much better than the selection based on a Makefile inclusion. Patches 6/7 don't seem to be related to DMICs, so we should probably discuss them separately. Thanks -Pierre [1] https://elixir.bootlin.com/linux/latest/source/sound/hda/intel-dsp-config.c#L118
On Fri, Mar 06, 2020 at 02:48:39PM -0600, Pierre-Louis Bossart wrote: > There's been a couple of accidental regressions already on stable, now > fixed, and my understanding is that the bar for inclusion is higher, so > let's assume this counts as a new feature: it never worked before with an There's been no change in the requirements for stable, those regressions have come from patches that have been pulled in automatically by the bot Sasha runs which doesn't pay attention to the requirements but tries to identify patches automatically.
On 2020-03-09 12:38, Mark Brown wrote: > On Fri, Mar 06, 2020 at 02:48:39PM -0600, Pierre-Louis Bossart wrote: > >> There's been a couple of accidental regressions already on stable, now >> fixed, and my understanding is that the bar for inclusion is higher, so >> let's assume this counts as a new feature: it never worked before with an > > There's been no change in the requirements for stable, those regressions > have come from patches that have been pulled in automatically by the bot > Sasha runs which doesn't pay attention to the requirements but tries to > identify patches automatically. > Mark, what's your take on backporting these to 4.19 LTS? Do we abandon the subject and "just" merge (once reviewed/ approved) into 5.5? I believe addressing all the issues mentioned on 4.19 is important.
On Mon, Mar 09, 2020 at 03:02:25PM +0100, Cezary Rojewski wrote: > Mark, what's your take on backporting these to 4.19 LTS? Do we abandon the > subject and "just" merge (once reviewed/ approved) into 5.5? I believe > addressing all the issues mentioned on 4.19 is important. I didn't actually look at the patches since by the time I went to look at them it was clear that there was going to be a new version. Pierre was saying that they added new functionality which would generally not be suitable. See Documentation/process/stable-kernel-rules.rst for the official rules.
On 2020-03-09 17:54, Mark Brown wrote: > On Mon, Mar 09, 2020 at 03:02:25PM +0100, Cezary Rojewski wrote: > >> Mark, what's your take on backporting these to 4.19 LTS? Do we abandon the >> subject and "just" merge (once reviewed/ approved) into 5.5? I believe >> addressing all the issues mentioned on 4.19 is important. > > I didn't actually look at the patches since by the time I went to look > at them it was clear that there was going to be a new version. Pierre > was saying that they added new functionality which would generally not > be suitable. > > See Documentation/process/stable-kernel-rules.rst for the official > rules. > Ok, sure. Should the 'Fixes' be appended regardless or leave it as is?
On Mon, Mar 09, 2020 at 06:48:35PM +0100, Cezary Rojewski wrote: > On 2020-03-09 17:54, Mark Brown wrote: > > I didn't actually look at the patches since by the time I went to look > > at them it was clear that there was going to be a new version. Pierre > > was saying that they added new functionality which would generally not > > be suitable. > Ok, sure. Should the 'Fixes' be appended regardless or leave it as is? There's never any harm in adding them.
> I didn't actually look at the patches since by the time I went to look > at them it was clear that there was going to be a new version. Pierre > was saying that they added new functionality which would generally not > be suitable. I am ok with the patches, as long as "[PATCH 5/7] ASoC: Intel: skl_hda_dsp: Enable Dmic configuration" is dropped as discussed. I don't know if that requires a v2 or if Mark you can just drop this one patch? So for all Patches except Patch5: Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Thanks Cezary!
On 2020-03-10 17:03, Pierre-Louis Bossart wrote: >> I didn't actually look at the patches since by the time I went to look >> at them it was clear that there was going to be a new version. Pierre >> was saying that they added new functionality which would generally not >> be suitable. > > I am ok with the patches, as long as "[PATCH 5/7] ASoC: Intel: > skl_hda_dsp: Enable Dmic configuration" is dropped as discussed. > > I don't know if that requires a v2 or if Mark you can just drop this one > patch? > > So for all Patches except Patch5: > > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > > Thanks Cezary! > Thanks for quick review and upstream! In fact, I wasn't even able to append missing 'Fixes' here and there. While indeed on 4.19 hdadsp machine driver does not exist so these patches can count as "new feature", for some other kernels such as 5.4 that ain't a case. Not very familiar with the cherry-pick mechanism for stable kernels, but is it possible for patches that ain't flagged with 'Fixes' tag to get backported? Having all of these at least on 5.4 is in my opinion desirable. Czarek
On Wed, Mar 11, 2020 at 04:49:41PM +0100, Cezary Rojewski wrote: > Not very familiar with the cherry-pick mechanism for stable kernels, but is > it possible for patches that ain't flagged with 'Fixes' tag to get > backported? Having all of these at least on 5.4 is in my opinion desirable. The document I pointed you at for the stable process includes information on how to submit patches for stable via e-mail.