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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@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?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx