| Message ID | 1488337038-2906-2-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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)); ? 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? Ander > } > > return min_cdclk;
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@intel.com> > > --- > > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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); } return min_cdclk;
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@intel.com> --- drivers/gpu/drm/i915/intel_cdclk.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)