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