On Fri, Jun 09, 2017 at 10:32:54PM +0300, Ville Syrjälä wrote:
> On Fri, Jun 09, 2017 at 10:02:09PM +0300, Imre Deak wrote:
> > On Fri, Jun 09, 2017 at 05:34:55PM +0300, Ville Syrjälä wrote:
> > > On Thu, Jun 08, 2017 at 03:03:13PM -0700, Rodrigo Vivi wrote:
> > > > All the low level cdclk bits are present, so let's add the required
> > > > hooks to reconfigure cdclk on the fly.
> > > >
> > > > v2: Rebase due to cnl_sanitize_cdclk()
> > > > v3: Rebased by Rodrigo on top of Ville's cdclk rework.
> > > > v4: Rebase moving cnl_calc_cdclk up to follow same order
> > > > as previous platforms.
> > > > v2: Squash drm/i915/cnl: Adjust min pixel rate. to address
> > > > the current limitation where CDCLK cannot be set to 168MHz
> > > > if audio is used with 96MHz. (Imre)
> > > >
> > > > Cc: Imre Deak <imre.deak@xxxxxxxxx>
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > >
> > > > Cannonlake also needs to adjust the minimal pixel rate
> > > > as gen9 platforms. Specially for the Azalia audio case.
> > > >
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> > > > Cc: Sanyog Kale <sanyog.r.kale@xxxxxxxxx>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_cdclk.c | 56 ++++++++++++++++++++++++++++++++++++--
> > > > 1 file changed, 53 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > index 35a1432..1d7cfc5 100644
> > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > @@ -1400,6 +1400,16 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> > > > bxt_set_cdclk(dev_priv, &cdclk_state);
> > > > }
> > > >
> > > > +static int cnl_calc_cdclk(int max_pixclk)
> > > > +{
> > > > + if (max_pixclk > 336000)
> > > > + return 528000;
> > > > + else if (max_pixclk > 168000)
> > > > + return 336000;
> > > > + else
> > > > + return 168000;
> > > > +}
> > > > +
> > > > static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv,
> > > > struct intel_cdclk_state *cdclk_state)
> > > > {
> > > > @@ -1641,7 +1651,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> > > >
> > > > cdclk_state = dev_priv->cdclk.hw;
> > > >
> > > > - cdclk_state.cdclk = 168000;
> > > > + cdclk_state.cdclk = cnl_calc_cdclk(0);
> > > > cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> > > >
> > > > cnl_set_cdclk(dev_priv, &cdclk_state);
> > > > @@ -1768,7 +1778,7 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state)
> > > >
> > > > pixel_rate = crtc_state->pixel_rate;
> > > >
> > > > - if (IS_BROADWELL(dev_priv) || IS_GEN9(dev_priv))
> > > > + if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
> > > > pixel_rate =
> > > > bdw_adjust_min_pipe_pixel_rate(crtc_state,
> > > > pixel_rate);
> > >
> > > I think you might also have to adjust some of the clock limits
> > > within bdw_adjust_min_pipe_pixel_rate().
> >
> > Yes, we missed this. The DP/HBR2/4-lanes/audio workaround on CNL is the
> > same as on GLK. So in bdw_adjust_min_pipe_pixel_rate() we could use the
> > same limits as on GLK, except I don't understand the 2x multiplier there
> > in
> >
> > pixel_rate = max(2 * 316800, pixel_rate);
> >
> > and
> >
> > pixel_rate = max(2 * 2 * 96000, pixel_rate);
> >
> > This pixel_rate value will be compared against CDCLK limit values in
> > glk_calc_cdclk()
>
> It's going to compare against 2*cdclk. But that doesn't mean it isn't
> extremely confusing. To make it more sane, I think we should probably
> s/min_pixclk/min_cdclk/ and actually track the "min acceptable cdclk"
> rather than "the pixel rate corresponding to the min acceptable cdclk".
Arr, correct. And now I found in BSpec how to calculate cdclk for a
given dotclock. So yes, the GLK part is correct, sorry for the noise.
However. For CNL the target CDCLK calculation should be the same as for
GLK, based on the "GEN10 Resolution Support". So as an example there if
the max pixel rate is 299MHz we can use a CDCLK of 168MHz. With the
current code we'd use a CDCLK of 336MHz. Is that correct?
>
> > not CD2XCLK limit values. Removing the 2x multiplier
> > from the above pixel_rate values wouldn't change anything on GLK AFAICS
> > and it would allow us to use the same code to calculate these on GLK
> > and CNL (and simplify the second pixel_rate calculation).
> >
> > --Imre
> >
> > >
> > > > @@ -1929,6 +1939,40 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
> > > > return 0;
> > > > }
> > > >
> > > > +static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
> > > > +{
> > > > + struct drm_i915_private *dev_priv = to_i915(state->dev);
> > > > + struct intel_atomic_state *intel_state =
> > > > + to_intel_atomic_state(state);
> > > > + int max_pixclk = intel_max_pixel_rate(state);
> > > > + int cdclk, vco;
> > > > +
> > > > + cdclk = cnl_calc_cdclk(max_pixclk);
> > > > + vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
> > > > +
> > > > + if (cdclk > dev_priv->max_cdclk_freq) {
> > > > + DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> > > > + cdclk, dev_priv->max_cdclk_freq);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + intel_state->cdclk.logical.vco = vco;
> > > > + intel_state->cdclk.logical.cdclk = cdclk;
> > > > +
> > > > + if (!intel_state->active_crtcs) {
> > > > + cdclk = cnl_calc_cdclk(0);
> > > > + vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
> > > > +
> > > > + intel_state->cdclk.actual.vco = vco;
> > > > + intel_state->cdclk.actual.cdclk = cdclk;
> > > > + } else {
> > > > + intel_state->cdclk.actual =
> > > > + intel_state->cdclk.logical;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > > > {
> > > > int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > > > @@ -1960,7 +2004,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > > > */
> > > > void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
> > > > {
> > > > - if (IS_GEN9_BC(dev_priv)) {
> > > > + if (IS_CANNONLAKE(dev_priv)) {
> > > > + dev_priv->max_cdclk_freq = 528000;
> > > > + } else if (IS_GEN9_BC(dev_priv)) {
> > > > u32 limit = I915_READ(SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
> > > > int max_cdclk, vco;
> > > >
> > > > @@ -2157,6 +2203,10 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> > > > dev_priv->display.set_cdclk = skl_set_cdclk;
> > > > dev_priv->display.modeset_calc_cdclk =
> > > > skl_modeset_calc_cdclk;
> > > > + } else if (IS_CANNONLAKE(dev_priv)) {
> > > > + dev_priv->display.set_cdclk = cnl_set_cdclk;
> > > > + dev_priv->display.modeset_calc_cdclk =
> > > > + cnl_modeset_calc_cdclk;
> > > > }
> > > >
> > > > if (IS_CANNONLAKE(dev_priv))
> > > > --
> > > > 1.9.1
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
