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@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
