[Intel-gfx] [PATCH v2] drm/i915: Limit audio CDCLK>=2*BCLK constraint back to GLK only

[Kai Vehmanen]

Hi,

On Thu, 2 Jan 2020, Rodrigo Vivi wrote:

> On Tue, Dec 31, 2019 at 04:00:07PM +0200, Kai Vehmanen wrote:
>> Revert changes done in commit f6ec9483091f ("drm/i915: extend audio
>> CDCLK>=2*BCLK constraint to more platforms"). Audio drivers
[...]
>>  		/* Force CDCLK to 2*BCLK as long as we need audio powered. */
>> -		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>> +		if (IS_GEMINILAKE(dev_priv))
> 
> I believe for correctness we should at least say this is for display_10 but
> since we don't have display gen defined probably the right thing to do here
> is to at least replace this per:
> 
> IS_GEN(dev_priv, 10) || IS_GEMINILAKE(dev_priv)

I checked the cdclk tables for CNL in intel_cdclk.c and minimum CDCLK
is 168kHz, so it is similar (>BCLK and close to 2*BCLK) as ICL and 
others and the workaround is not needed.

I do agree this still smells funny, but basicly my naive attempt to align 
with the spec failed in wider testing and it seems this original solution 
to have the WA for GLK only, is the least bad option at this point.

Possible longer term solutions for this: 
   (i) more clock configurations allowing to bump the freq without
       a mode change on all platforms
   (ii) avoid all HDA communication at probe time and only initialize
  	the HDA connection when a monitor is connected 
   (iii) guarantee min cdclk to be sufficient for HDA communication

Closing on feasibility of (i) and (iii) is going to be a longer 
discussion.

The (ii) option would be quite a big change on audio side and might
potentially require changes to drm_audio_component.h (and impact other
drivers). To me, this feels wrong, the HDA bus supports discovery of
codecs, so we should be able to use it as with any HDA codec, including
graphics. Unless we hit deadends with (i) and (iii), I'd rather 
not pursue this path.

Br, Kai