Re: [PATCH 1/7] ASoC: Intel: Skylake: Remove superfluous chip initialization - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Cezary Rojewski <cezary.rojewski@intel.com>, alsa-devel@alsa-project.org
Cc: vkoul@kernel.org, broonie@kernel.org, tiwai@suse.com,
	lgirdwood@gmail.com
Subject: Re: [PATCH 1/7] ASoC: Intel: Skylake: Remove superfluous chip initialization
Date: Mon, 9 Mar 2020 11:48:01 -0500	[thread overview]
Message-ID: <cd43e35c-bd32-37d0-7379-1113912d0fd5@linux.intel.com> (raw)
In-Reply-To: <8dc83af4-c4af-1bdd-5d59-f13598037f96@intel.com>

On 3/9/20 8:57 AM, Cezary Rojewski wrote:
> On 2020-03-06 21:52, Pierre-Louis Bossart wrote:
>> On 3/5/20 8:53 AM, Cezary Rojewski wrote:
>>> Skylake driver does the controller init operation twice:
>>> - first during probe (only to stop it just before scheduling probe_work)
>>> - and during said probe_work where the actual correct sequence is
>>> executed
>>>
>>> To properly complete boot sequence when iDisp codec is present, bus
>>> initialization has to be called only after _i915_init() finishes.
>>> With additional _reset_list preceding _i915_init(), iDisp codec never
>>> gets the chance to enumerate on the link. Remove the superfluous
>>> initialization to address the issue.
>>
>> Have you tested with with DRM built-in and as a module? that was 
>> enough to trigger race conditions in the past on Dell XPS9350.
>>
> 
> DRM is quite a tree, you got to be more specific. Tested with i915=m and 
> DRM=m. I hope we mean the same thing when mentioning 'race'. There is an 
> obvious initialization race between hda bus drv and i915 which requires 
> one to follow a tight operation order in order to not lose i915 codec on 
> hda link and thus be able to enumerate it properly.

I meant CONFIG_DRM=m, yes, thanks for the clarification.

With the DRM as module, it took more time to establish the 
communication. That's probably changed if we do all the inits in a 
workqueue now.

> 
> On top of that, as you mentioned (by the link) this series addresses 
> missing DMIC configuration in conjunction with HDA +/- iDisp AND shields 
> against no-NHLT configuration. On Dell XPS 9350 lack on of NHLT was the 
> biggest problem - that's why I'd like that issue not to be forgotten about.

yes, but we don't want the driver to be auto-selected on SKL w/o DMICs, 
since it'd break existing devices who don't have a topology file installed.