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

[Jani Nikula]

On Tue, 14 Mar 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan at intel.com> wrote:
> On Tue, 2017-03-14 at 17:47 -0300, Paulo Zanoni wrote:
>> Em Ter, 2017-03-07 às 16:12 -0800, Dhinakaran Pandiyan escreveu:
>> > 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);
>> > +		if (IS_BROXTON(dev_priv))
>> 
>> SKL also documents this in the page that explains the cdclk freq change
>> sequences. The funny thing is that the minimum CDCLK for SKL seems to
>> be 308.57, so that's still bigger than 96*2... Anyway, having this for
>> completeness would probably be good, just in case I'm missing some
>> detail that's important here.
>> 
>> I'd like to see the SKL addition, but I can live without it in case you
>> have some better argument, so if you don't send a new version, here's
>> it:
>> 
>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> 
>> 
>
> I did not include SKL as the lowest cdclk freq. it supports was higher
> than 2 * 96MHz. But, I agree it's good to include it for clarity. I'll
> send another version.
>
>
>> Perhaps it would also be interesting to have some sort of macro to
>> identify the platform(s) that need the magic *2 calculation. A more
>> confusion-proof version of this function would look like this:
>> 
>> if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
>> 	if (HAS_2_PIXELS_PER_CLOCK(dev_priv))

I didn't check the spec (where's the fun in that?!) about the
terminology it uses, but isn't that just double data rate, or DDR?

BR,
Jani.


>> 		pixel_rate = max(2 * 2 * 96000, pixel_rate);
>> 	else
>> 		pixel_rate = max(2 * 96000, pixel_rate);
>> 
>> Maybe we'd be able to use the macro in other places too (I haven't
>> checked). The new macro would definitely be part of a separate patch on
>> top of these.
>> 
>> 
>> 
>
> Agreed. Using a macro makes it cleaner, will send a patch for this too.
>
> -DK
>
>> > +			pixel_rate = max(2 * 96000, pixel_rate);
>> > +	}
>> > +
>> >  	return pixel_rate;
>> >  }
>> >  
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center