diff mbox

drm/i915/vlv: add VLV specific clock_get function v3

Message ID 1379701772-12818-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes Sept. 20, 2013, 6:29 p.m. UTC
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@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |   33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

Comments

Ville Syrjälä Sept. 23, 2013, 6:01 p.m. UTC | #1
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@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.

> +}
> +
>  static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  				 struct intel_crtc_config *pipe_config)
>  {
> @@ -5113,7 +5141,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  						     DPLL_PORTB_READY_MASK);
>  	}
>  
> -	i9xx_crtc_clock_get(crtc, pipe_config);
> +	if (IS_VALLEYVIEW(dev))
> +		vlv_crtc_clock_get(crtc, pipe_config);
> +	else
> +		i9xx_crtc_clock_get(crtc, pipe_config);
>  
>  	return true;
>  }
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jesse Barnes Sept. 25, 2013, 5:38 p.m. UTC | #2
On Mon, 23 Sep 2013 21:01:44 +0300
Ville Syrjälä <ville.syrjala@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@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. :)

Thanks,
Daniel Vetter Sept. 25, 2013, 9 p.m. UTC | #3
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@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@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
diff mbox

Patch

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;
+}
+
 static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 				 struct intel_crtc_config *pipe_config)
 {
@@ -5113,7 +5141,10 @@  static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 						     DPLL_PORTB_READY_MASK);
 	}
 
-	i9xx_crtc_clock_get(crtc, pipe_config);
+	if (IS_VALLEYVIEW(dev))
+		vlv_crtc_clock_get(crtc, pipe_config);
+	else
+		i9xx_crtc_clock_get(crtc, pipe_config);
 
 	return true;
 }