[Intel-gfx] [PATCH v2 2/2] drm/i915: Implement cdclk restrictions based on Azalia BCLK

[Pandiyan, Dhinakaran]

On Thu, 2017-03-30 at 15:44 +0300, Ville Syrjälä wrote:
> On Thu, Mar 30, 2017 at 02:17:15PM +0200, Takashi Iwai wrote:
> > On Thu, 30 Mar 2017 13:42:09 +0200,
> > Ville Syrjälä wrote:
> > > 
> > > On Wed, Mar 29, 2017 at 09:16:36PM +0000, Pandiyan, Dhinakaran wrote:
> > > > On Wed, 2017-03-29 at 11:50 +0300, Ville Syrjälä wrote:
> > > > > On Tue, Mar 07, 2017 at 04:12:52PM -0800, Dhinakaran Pandiyan wrote:
> > > > > > According to BSpec, "The CD clock frequency must be at least twice the
> > > > > > frequency of the Azalia BCLK." and BCLK is configured to 96 MHz by
> > > > > > default. This check is needed because BXT and GLK support cdclk
> > > > > > frequencies less than 192 MHz.
> > > > > > 
> > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_cdclk.c | 12 ++++++++++++
> > > > > >  1 file changed, 12 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > > > index e8c1181..7b1ac1d 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > > > @@ -1458,6 +1458,18 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > > > > >  			pixel_rate = max(432000, pixel_rate);
> > > > > >  	}
> > > > > >  
> > > > > > +	/* According to BSpec, "The CD clock frequency must be at least twice
> > > > > > +	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
> > > > > > +	 * The check for GLK has to be adjusted as the platform can output
> > > > > > +	 * two pixels per clock.
> > > > > > +	 */
> > > > > > +	if (crtc_state->has_audio) {
> > > > > > +		if (IS_GEMINILAKE(dev_priv))
> > > > > > +			pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > > > > 
> > > > > BTW that x2 factor for GLK looks wrong. It should be /2.
> > > > > 
> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=100439
> > > > > 
> > > > 
> > > > BSpec[1] says cdclk needs to be > 2*96 MHz (azalia bclk). To set a cdclk
> > > > > 192 MHz on GLK, we need to pass in max_pixclk = 2*192 MHz. But with a
> > > > factor of /2, we can end up picking 158.4 MHz or 79.2 MHz for cdclk,
> > > > which are lower than 192 MHz.
> > > > 
> > > > 
> > > > 	static int glk_calc_cdclk(int max_pixclk)
> > > > 	{
> > > > 	        if (max_pixclk > 2 * 158400)
> > > >         	        return 316800;
> > > > 	        else if (max_pixclk > 2 * 79200)
> > > >         	        return 158400;
> > > >         	else
> > > >         	        return 79200;
> > > > 	}
> > > > 
> > > > [1] Geminilake Clocks
> > > > "158.4 MHz CD (cannot be used when audio is enabled and Azalia BCLK is
> > > > 96 MHz)
> > > > 316.8 MHz CD
> > > > 79.2 MHz CD (exclusively for resolutions up to 1080p in low power single
> > > > pipe eDP/MIPI configurations, no audio support)"
> > > 
> > > Argh. So it's really about the cdclk freq vs. bclk. Apparently the fact
> > > that the pipe can output two pixels per clock doesn't extend to the HDA
> > > side. OK, then the code looks correct.
> > > 
> > > But how do we know the bclk is really 96Mhz? Or are we just making a
> > > worst case estimate here? Base on the hda spec I think bclk should typically
> > > be 24 MHz, but of course the hda spec is so old that it probably has
> > > little to do with today's realities. I guess ideally we'd like the audio
> > > driver to tell us what the frequency is, or we'd dig that up from
> > > somewhere ourselves.
> > > 
> > > Hmm. AUD_FREQ_CNTRL seems to have something. 96 vs. 48 MHz. But I'm not sure
> > > we could trust that at boot time. Not sure if the audio driver will make
> > > its own choice if BCLK somewhere. Can any alsa folks help us?
> > 
> > Well, I'm not sure which BCLK is referred in this context, but if it's
> > about HD-audio (aka Azalia) BCLK, it's always 24MHz.  It's the
> > specification, AFAIK.
> 
> That's what I read also. But considering the HDA spec is so dated
> (doesn't even have the MST stuff in it) I can't be sure it matches
> any current realities.
> 
> The spec quote from DK clearly shows that the CDCLK programming section
> of our spec clearly seems to make provisions for a 96MHz Azalia BCLK.
> 
> If I look at the AUD_FREQ_CNTRL register I see the following:
> "Audio BCLK Frequency Control
> ...
> [4] 96MHz BCLK
> Default Value: 	1b
> Indicates that iDISPLAY Audio Link will run at 96MHz. This bit is
> defaulted to 1. BIOS or System Software must pre-program B96 before the
> iDISPLAY Audio Link is brought out from reset.
> [3] 48MHz BCLK
> Default Value: 	0b
> Indicates that iDISPLAY Audio Link will run at 48MHz. This bit is
> defaulted to 0. BIOS or System Software must pre-program B96 before the
> iDISPLAY Audio Link is brought out from reset."
> 
> Whether this iDISPLAY BCLK has anything to do with the Azalia BCLK
> I don't know. Maybe they changed it from the standard 24 MHz since it's
> all internal now and thus no need to interface with external parts?
> 

Libin was working on a patch that was reading the bclk frequency, looks
like he has submitted this https://patchwork.kernel.org/patch/9608123/
We could add a new acomp->audio_ops function to get the exact bclk
frequency from the audio driver.

-DK