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

[Matt Roper]

On Fri, Jan 03, 2020 at 05:28:24PM +0200, Kai Vehmanen wrote:
> 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.

Agreed; GLK's minimum cdclk goes down to 79.2 mhz so it makes sense that
it needs to be handled differently than CNL (and beyond).  I.e., this
isn't a pure revert of the original patch.

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

The only CI shard failure was an unrelated GEM false positive, so added
a Fixes: tag referencing the original patch and applied to dinq.  Thanks
for the patch!


Matt

> 
> 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795