[Intel-gfx] [PATCH] drm/i915/vlv: add VLV specific clock_get function v3

[Chris Wilson]

On Wed, Sep 25, 2013 at 11:00:20PM +0200, Daniel Vetter wrote:
> On Wed, Sep 25, 2013 at 10:38:33AM -0700, Jesse Barnes wrote:
> > On Mon, 23 Sep 2013 21:01:44 +0300
> > Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> > 
> > > On Fri, Sep 20, 2013 at 11:29:32AM -0700, Jesse Barnes wrote:
> > > > Calculation is a little different than other platforms.
> > > > 
> > > > v2: update to use port_clock instead
> > > >     rebase on top of Ville's changes
> > > > v3: update to new port_clock semantics - don't divide by
> > > >     pixel_multiplier (Ville)
> > > > 
> > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=67345
> > > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c |   33 ++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 32 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 7eecf37..e5c9c1c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -5048,6 +5048,34 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> > > >  			I915_READ(LVDS) & LVDS_BORDER_ENABLE;
> > > >  }
> > > >  
> > > > +static void vlv_crtc_clock_get(struct intel_crtc *crtc,
> > > > +			       struct intel_crtc_config *pipe_config)
> > > > +{
> > > > +	struct drm_device *dev = crtc->base.dev;
> > > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > > +	int pipe = pipe_config->cpu_transcoder;
> > > > +	intel_clock_t clock;
> > > > +	u32 mdiv;
> > > > +	int refclk = 100000, fastclk, update_rate;
> > > > +
> > > > +	mutex_lock(&dev_priv->dpio_lock);
> > > > +	mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe));
> > > > +	mutex_unlock(&dev_priv->dpio_lock);
> > > > +
> > > > +	clock.m1 = (mdiv >> DPIO_M1DIV_SHIFT) & 7;
> > > > +	clock.m2 = mdiv & DPIO_M2DIV_MASK;
> > > > +	clock.n = (mdiv >> DPIO_N_SHIFT) & 0xf;
> > > > +	clock.p1 = (mdiv >> DPIO_P1_SHIFT) & 7;
> > > > +	clock.p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f;
> > > > +
> > > > +	update_rate = refclk / clock.n;
> > > > +	clock.vco = update_rate * clock.m1 * clock.m2;
> > > > +	fastclk = clock.vco / clock.p1 / clock.p2;
> > > > +	clock.dot = (2 * fastclk);
> > > > +
> > > > +	pipe_config->port_clock = clock.dot / 10;
> > > 
> > > Looks like it should get roughly the right answer, but I don't see much
> > > point in all the intermediate results.
> > > 
> > > If you want to keep some of them for clarity, then I think this should
> > > be enough:
> > > 
> > > clock.vco = refclk * clock.m / clock.n;
> > > clock.dot = clock.vco / clock.p; /* fast clock */
> > > pipe_config->port_clock = clock.dot / 5;
> > > 
> > > Although calling the fast clock "dot" is a bit wrong, but I think it's
> > > fine here, especially as it matches what I have in mind for
> > > vlv_find_best_dpll(). I had to rewrite that sucker a bit to make it
> > > compatible with my brain ;) I'll send a patch for that ASAP.
> > 
> > The other advantage of having it all spelled out like this is that it
> > matches the freq calculation spreadsheet I sourced it from.
> > 
> > So I'd prefer to keep this as is, if I can have your r-b anyway. :)
> 
> Queued for -next with Ville's irc-r-b, thanks for the patch.
> -Daniel

Did Jesse post a delta update to this, as #67345 still warns with v3.

For the record, this works - can't see why it would make a difference at
the moment though (probably some type is too small or loss of precision):

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5741d48..5219fdc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5083,10 +5083,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 {
        struct drm_device *dev = crtc->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
+       const int refclk = 100000;
        int pipe = pipe_config->cpu_transcoder;
        intel_clock_t clock;
        u32 mdiv;
-       int refclk = 100000, fastclk, update_rate;
 
        mutex_lock(&dev_priv->dpio_lock);
        mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe));
@@ -5098,12 +5098,16 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
        clock.p1 = (mdiv >> DPIO_P1_SHIFT) & 7;
        clock.p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f;
 
-       update_rate = refclk / clock.n;
-       clock.vco = update_rate * clock.m1 * clock.m2;
-       fastclk = clock.vco / clock.p1 / clock.p2;
-       clock.dot = (2 * fastclk);
+       clock.vco = refclk * clock.m1 * clock.m2 / clock.n;
+       clock.dot = 2 * clock.vco / (clock.p1 * clock.p2);
 
        pipe_config->port_clock = clock.dot / 10;
 }

-- 
Chris Wilson, Intel Open Source Technology Centre