On Tue, 14 Mar 2017, "Pandiyan, Dhinakaran" <[email protected]> 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 <[email protected]> >> > --- >> > 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 <[email protected]> >> >> > > 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 >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
