On Fri, 2017-03-03 at 14:23 +0200, Ander Conselvan De Oliveira wrote:
> On Tue, 2017-02-28 at 18:57 -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. BXT and GLK both have cdclk frequencies that are less han 192 MHz,
> > so apply the check conditionally for these platforms.
> >
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_cdclk.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 8fc0f72..89027fa 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1444,6 +1444,8 @@ static int intel_min_cdclk(struct drm_atomic_state *state)
> > struct intel_crtc_state *crtc_state;
> >
> > crtc_state = to_intel_crtc_state(cstate);
> > + if (!crtc_state->has_audio)
> > + continue;
> >
> > /* According to BSpec, "Do not use DisplayPort with CDCLK less
> > * than 432 MHz, audio enabled, port width x4, and link rate
> > @@ -1452,7 +1454,6 @@ static int intel_min_cdclk(struct drm_atomic_state *state)
> > * for GLK is at 316.8 MHz
> > */
> > if (intel_crtc_has_dp_encoder(crtc_state) &&
> > - crtc_state->has_audio &&
> > crtc_state->port_clock >= 540000 &&
> > crtc_state->lane_count == 4) {
> > if (IS_GEMINILAKE(dev_priv))
> > @@ -1460,6 +1461,13 @@ static int intel_min_cdclk(struct drm_atomic_state *state)
> > else if (IS_BROADWELL(dev_priv) || IS_GEN9(dev_priv))
> > min_cdclk = 432000;
> > }
> > +
> > + /* 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.
> > + */
> > + if (IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv))
> > + min_cdclk = max(min_cdclk, 2 * 96000);
>
> Hmm, my assumption from reviewing patch 1 of "this returns a valid cdclk" is
> broken here. Maybe with the idea of splitting the calc_cdclk_state() in two from
> my comment to Paulo's series this could be just:
>
> min_cdclk = max(min_cdcdlk, choose_cdclk(2 * 96000));
>
> ?
>
In this case, we'd have to call <platform>_calc_cdclk() twice? Once for
finding a valid min. cdclk and then again to find the right cdclk for a
given pixel rate.
> On second thought, it does make sense for this kind of platform specific
> restrictions to be dealt with in platform specific hook, so duplicating that in
> calc_cdclk_state() is not that bad?
>
I thought of that, but with comments and long conditional checks, it'd
look ugly to duplicate this code in bdw, skl, bxt, glk specific hooks.
-DK
> Ander
>
>
> > }
> >
> > return min_cdclk;
> _______________________________________________
> 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
