[PATCH v2 drm/i915: rawclk/cdclk stuff (v2)

[PATCH v2 drm/i915: rawclk/cdclk stuff (v2)

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Here are the remainder of my cdclk/rawclk cleanup patches. Previous time I
posted these was [1]. Bunch of them got already r-b'd by Jani. I also did
the PWM DIV_ROUND_CLOSEST() changes as Jani suggested last time around.

Entire series is available here:
git://github.com/vsyrjala/linux.git rawclk_freq_10

Ville Syrjälä (6):
  drm/i915: Store rawclk_freq in dev_priv
  drm/i915: Rename s/i9xx/g4x/ in DP code
  drm/i915: Use g4x_get_aux_clock_divider() for VLV/CHV
  drm/i915: Read out hrawclk from CCK on vlv/chv
  drm/i915: Clean up .get_aux_clock_divider() functions
  drm/i915: Use DIV_ROUND_CLOSEST for PWM calculations

 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 56 +++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dp.c      | 59 ++++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 drivers/gpu/drm/i915/intel_panel.c   | 50 +++++++++++++++---------------
 6 files changed, 87 insertions(+), 82 deletions(-)

-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/6] drm/i915: Store rawclk_freq in dev_priv

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Generalize rawclk handling by storing it in dev_priv.

Presumably our hrawclk readout works at least for CTG and ELK
since we've been using it for DP AUX on those platforms. There
are no real docs anymore after configdb vanished, so the only
reference is the public CTG GMCH spec. What bits are listed in
that doc match our code. The ELK GMCH spec have no relevant
details unfortunately.

The PNV situation is less clear. Starting from
commit aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
we assume that the CTG/ELK hrawclk readout works for PNV as well.
At least the results *seem* reasonable for one PNV machine (Lenovo
Ideapad S10-3t). Sadly the PNV GMCH spec doesn't have the goods on
the relevant register either.

So let's keep assuming it works for PNV,ELK,CTG and read it out on
those platforms. G33 also has hrawclk according to some notes
in BSpec, but we don't actually need it for anything, so let's not
even try to read it out there.

v2: Rebase due to IS_VALLYVIEW vs. IS_CHERRYVIEW split
    Use KHz() all over, and kill off a few useless temp variables

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_dp.c      | 16 +++++------
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 drivers/gpu/drm/i915/intel_panel.c   | 42 +++++++++++++--------------
 5 files changed, 62 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2cb0a411c10e..b0e7f35a8be4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1837,6 +1837,7 @@ struct drm_i915_private {
 	unsigned int skl_boot_cdclk;
 	unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq;
 	unsigned int max_dotclk_freq;
+	unsigned int rawclk_freq;
 	unsigned int hpll_freq;
 	unsigned int czclk_freq;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 44fcff0343f2..330528c1fb28 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -169,49 +169,61 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
 	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
 }
 
-int
-intel_pch_rawclk(struct drm_device *dev)
+static int
+intel_pch_rawclk(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	WARN_ON(!HAS_PCH_SPLIT(dev));
+	return (I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK) * 1000;
+}
 
-	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
+static int
+intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
+{
+	return 200000;
 }
 
-/* hrawclock is 1/4 the FSB frequency */
-int intel_hrawclk(struct drm_device *dev)
+static int
+intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t clkcfg;
 
-	/* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */
-	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
-		return 200;
-
+	/* hrawclock is 1/4 the FSB frequency */
 	clkcfg = I915_READ(CLKCFG);
 	switch (clkcfg & CLKCFG_FSB_MASK) {
 	case CLKCFG_FSB_400:
-		return 100;
+		return 100000;
 	case CLKCFG_FSB_533:
-		return 133;
+		return 133333;
 	case CLKCFG_FSB_667:
-		return 166;
+		return 166667;
 	case CLKCFG_FSB_800:
-		return 200;
+		return 200000;
 	case CLKCFG_FSB_1067:
-		return 266;
+		return 266667;
 	case CLKCFG_FSB_1333:
-		return 333;
+		return 333333;
 	/* these two are just a guess; one of them might be right */
 	case CLKCFG_FSB_1600:
 	case CLKCFG_FSB_1600_ALT:
-		return 400;
+		return 400000;
 	default:
-		return 133;
+		return 133333;
 	}
 }
 
+static void intel_update_rawclk(struct drm_i915_private *dev_priv)
+{
+	if (HAS_PCH_SPLIT(dev_priv))
+		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
+	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		dev_priv->rawclk_freq = intel_vlv_hrawclk(dev_priv);
+	else if (IS_G4X(dev_priv) || IS_PINEVIEW(dev_priv))
+		dev_priv->rawclk_freq = intel_g4x_hrawclk(dev_priv);
+	else
+		return; /* no rawclk on other platforms, or no need to know it */
+
+	DRM_DEBUG_DRIVER("rawclk rate: %d kHz\n", dev_priv->rawclk_freq);
+}
+
 static void intel_update_czclk(struct drm_i915_private *dev_priv)
 {
 	if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
@@ -15617,6 +15629,7 @@ void intel_modeset_init(struct drm_device *dev)
 	}
 
 	intel_update_czclk(dev_priv);
+	intel_update_rawclk(dev_priv);
 	intel_update_cdclk(dev);
 
 	intel_shared_dpll_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f272b3541e00..6afdfa720974 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -674,13 +674,13 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 
 	/*
 	 * The clock divider is based off the hrawclk, and would like to run at
 	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
 	 */
-	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
+	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 }
 
 static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
@@ -692,12 +692,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	if (index)
 		return 0;
 
-	if (intel_dig_port->port == PORT_A) {
+	if (intel_dig_port->port == PORT_A)
 		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
-
-	} else {
-		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
-	}
+	else
+		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 }
 
 static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
@@ -718,7 +716,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 		default: return 0;
 		}
 	} else  {
-		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
+		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 	}
 }
 
@@ -5254,7 +5252,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pp_on, pp_off, pp_div, port_sel = 0;
-	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
+	int div = dev_priv->rawclk_freq / 1000;
 	i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	const struct edp_power_seq *seq = &intel_dp->pps_delays;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cb413e246267..e7485ec34f63 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1085,8 +1085,6 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
 extern const struct drm_plane_funcs intel_plane_funcs;
 unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
 bool intel_has_pending_fb_unpin(struct drm_device *dev);
-int intel_pch_rawclk(struct drm_device *dev);
-int intel_hrawclk(struct drm_device *dev);
 void intel_mark_busy(struct drm_device *dev);
 void intel_mark_idle(struct drm_device *dev);
 void intel_crtc_restore_mode(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 21ee6477bf98..5cf377507162 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1251,16 +1251,14 @@ static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 static u32 spt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
-	u32 mul, clock;
+	u32 mul;
 
 	if (I915_READ(SOUTH_CHICKEN1) & SPT_PWM_GRANULARITY)
 		mul = 128;
 	else
 		mul = 16;
 
-	clock = MHz(24);
-
-	return clock / (pwm_freq_hz * mul);
+	return MHz(24) / (pwm_freq_hz * mul);
 }
 
 /*
@@ -1292,10 +1290,9 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
  */
 static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
-	struct drm_device *dev = connector->base.dev;
-	int clock = MHz(intel_pch_rawclk(dev));
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 
-	return clock / (pwm_freq_hz * 128);
+	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
 }
 
 /*
@@ -1308,14 +1305,13 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
  */
 static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
-	struct drm_device *dev = connector->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	int clock;
 
-	if (IS_PINEVIEW(dev))
-		clock = MHz(intel_hrawclk(dev));
+	if (IS_PINEVIEW(dev_priv))
+		clock = KHz(dev_priv->rawclk_freq);
 	else
-		clock = 1000 * dev_priv->cdclk_freq;
+		clock = KHz(dev_priv->cdclk_freq);
 
 	return clock / (pwm_freq_hz * 32);
 }
@@ -1332,9 +1328,9 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	int clock;
 
 	if (IS_G4X(dev_priv))
-		clock = MHz(intel_hrawclk(dev));
+		clock = KHz(dev_priv->rawclk_freq);
 	else
-		clock = 1000 * dev_priv->cdclk_freq;
+		clock = KHz(dev_priv->cdclk_freq);
 
 	return clock / (pwm_freq_hz * 128);
 }
@@ -1346,19 +1342,21 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
  */
 static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
-	struct drm_device *dev = connector->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int clock;
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+	int mul, clock;
 
 	if ((I915_READ(CBR1_VLV) & CBR_PWM_CLOCK_MUX_SELECT) == 0) {
-		if (IS_CHERRYVIEW(dev))
-			return KHz(19200) / (pwm_freq_hz * 16);
+		if (IS_CHERRYVIEW(dev_priv))
+			clock = KHz(19200);
 		else
-			return MHz(25) / (pwm_freq_hz * 16);
+			clock = MHz(25);
+		mul = 16;
 	} else {
-		clock = intel_hrawclk(dev);
-		return MHz(clock) / (pwm_freq_hz * 128);
+		clock = KHz(dev_priv->rawclk_freq);
+		mul = 128;
 	}
+
+	return clock / (pwm_freq_hz * mul);
 }
 
 static u32 get_backlight_max_vbt(struct intel_connector *connector)
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/6] drm/i915: Store rawclk_freq in dev_priv

[Jani Nikula]

On Wed, 02 Mar 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Generalize rawclk handling by storing it in dev_priv.
>
> Presumably our hrawclk readout works at least for CTG and ELK
> since we've been using it for DP AUX on those platforms. There
> are no real docs anymore after configdb vanished, so the only
> reference is the public CTG GMCH spec. What bits are listed in
> that doc match our code. The ELK GMCH spec have no relevant
> details unfortunately.
>
> The PNV situation is less clear. Starting from
> commit aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> we assume that the CTG/ELK hrawclk readout works for PNV as well.
> At least the results *seem* reasonable for one PNV machine (Lenovo
> Ideapad S10-3t). Sadly the PNV GMCH spec doesn't have the goods on
> the relevant register either.
>
> So let's keep assuming it works for PNV,ELK,CTG and read it out on
> those platforms. G33 also has hrawclk according to some notes
> in BSpec, but we don't actually need it for anything, so let's not
> even try to read it out there.

While the above is useful information, I don't think it's specifically
related to the changes in this patch. The only thing that changes here
is that the rawclk is read and stored once, instead of being read every
time.

There are a few nitpicks below, but I don't insist on doing anything
with them.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> v2: Rebase due to IS_VALLYVIEW vs. IS_CHERRYVIEW split
>     Use KHz() all over, and kill off a few useless temp variables
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_dp.c      | 16 +++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 --
>  drivers/gpu/drm/i915/intel_panel.c   | 42 +++++++++++++--------------
>  5 files changed, 62 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2cb0a411c10e..b0e7f35a8be4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1837,6 +1837,7 @@ struct drm_i915_private {
>  	unsigned int skl_boot_cdclk;
>  	unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq;
>  	unsigned int max_dotclk_freq;
> +	unsigned int rawclk_freq;
>  	unsigned int hpll_freq;
>  	unsigned int czclk_freq;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 44fcff0343f2..330528c1fb28 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -169,49 +169,61 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
>  	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
>  }
>  
> -int
> -intel_pch_rawclk(struct drm_device *dev)
> +static int
> +intel_pch_rawclk(struct drm_i915_private *dev_priv)

Now that you make these static, it feels a bit silly to have both intel
and platform in the name. But meh, no need to resend over that.

>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	WARN_ON(!HAS_PCH_SPLIT(dev));
> +	return (I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK) * 1000;
> +}
>  
> -	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
> +static int
> +intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> +{
> +	return 200000;
>  }
>  
> -/* hrawclock is 1/4 the FSB frequency */
> -int intel_hrawclk(struct drm_device *dev)
> +static int
> +intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t clkcfg;
>  
> -	/* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */
> -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> -		return 200;
> -
> +	/* hrawclock is 1/4 the FSB frequency */
>  	clkcfg = I915_READ(CLKCFG);
>  	switch (clkcfg & CLKCFG_FSB_MASK) {
>  	case CLKCFG_FSB_400:
> -		return 100;
> +		return 100000;
>  	case CLKCFG_FSB_533:
> -		return 133;
> +		return 133333;
>  	case CLKCFG_FSB_667:
> -		return 166;
> +		return 166667;
>  	case CLKCFG_FSB_800:
> -		return 200;
> +		return 200000;
>  	case CLKCFG_FSB_1067:
> -		return 266;
> +		return 266667;
>  	case CLKCFG_FSB_1333:
> -		return 333;
> +		return 333333;
>  	/* these two are just a guess; one of them might be right */
>  	case CLKCFG_FSB_1600:
>  	case CLKCFG_FSB_1600_ALT:
> -		return 400;
> +		return 400000;
>  	default:
> -		return 133;
> +		return 133333;
>  	}
>  }
>  
> +static void intel_update_rawclk(struct drm_i915_private *dev_priv)
> +{
> +	if (HAS_PCH_SPLIT(dev_priv))
> +		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
> +	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> +		dev_priv->rawclk_freq = intel_vlv_hrawclk(dev_priv);
> +	else if (IS_G4X(dev_priv) || IS_PINEVIEW(dev_priv))
> +		dev_priv->rawclk_freq = intel_g4x_hrawclk(dev_priv);
> +	else
> +		return; /* no rawclk on other platforms, or no need to know it */
> +
> +	DRM_DEBUG_DRIVER("rawclk rate: %d kHz\n", dev_priv->rawclk_freq);
> +}
> +
>  static void intel_update_czclk(struct drm_i915_private *dev_priv)
>  {
>  	if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> @@ -15617,6 +15629,7 @@ void intel_modeset_init(struct drm_device *dev)
>  	}
>  
>  	intel_update_czclk(dev_priv);
> +	intel_update_rawclk(dev_priv);
>  	intel_update_cdclk(dev);
>  
>  	intel_shared_dpll_init(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f272b3541e00..6afdfa720974 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -674,13 +674,13 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>  static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>  
>  	/*
>  	 * The clock divider is based off the hrawclk, and would like to run at
>  	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
>  	 */

You forget to fix the comment here, but I see that you fix it in a later
patch... (You know I'm just commenting to show that I've actually read
the patch properly. ;)

> -	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
> +	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>  }
>  
>  static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> @@ -692,12 +692,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  	if (index)
>  		return 0;
>  
> -	if (intel_dig_port->port == PORT_A) {
> +	if (intel_dig_port->port == PORT_A)
>  		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
> -
> -	} else {
> -		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> -	}
> +	else
> +		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>  }
>  
>  static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> @@ -718,7 +716,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  		default: return 0;
>  		}
>  	} else  {
> -		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> +		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>  	}
>  }
>  
> @@ -5254,7 +5252,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 pp_on, pp_off, pp_div, port_sel = 0;
> -	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
> +	int div = dev_priv->rawclk_freq / 1000;
>  	i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	const struct edp_power_seq *seq = &intel_dp->pps_delays;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cb413e246267..e7485ec34f63 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1085,8 +1085,6 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
>  extern const struct drm_plane_funcs intel_plane_funcs;
>  unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
>  bool intel_has_pending_fb_unpin(struct drm_device *dev);
> -int intel_pch_rawclk(struct drm_device *dev);
> -int intel_hrawclk(struct drm_device *dev);
>  void intel_mark_busy(struct drm_device *dev);
>  void intel_mark_idle(struct drm_device *dev);
>  void intel_crtc_restore_mode(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 21ee6477bf98..5cf377507162 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1251,16 +1251,14 @@ static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  static u32 spt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> -	u32 mul, clock;
> +	u32 mul;
>  
>  	if (I915_READ(SOUTH_CHICKEN1) & SPT_PWM_GRANULARITY)
>  		mul = 128;
>  	else
>  		mul = 16;
>  
> -	clock = MHz(24);
> -
> -	return clock / (pwm_freq_hz * mul);
> +	return MHz(24) / (pwm_freq_hz * mul);

Unrelated change, really.

>  }
>  
>  /*
> @@ -1292,10 +1290,9 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>   */
>  static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
> -	struct drm_device *dev = connector->base.dev;
> -	int clock = MHz(intel_pch_rawclk(dev));
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  
> -	return clock / (pwm_freq_hz * 128);
> +	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
>  }
>  
>  /*
> @@ -1308,14 +1305,13 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>   */
>  static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
> -	struct drm_device *dev = connector->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	int clock;
>  
> -	if (IS_PINEVIEW(dev))
> -		clock = MHz(intel_hrawclk(dev));
> +	if (IS_PINEVIEW(dev_priv))
> +		clock = KHz(dev_priv->rawclk_freq);
>  	else
> -		clock = 1000 * dev_priv->cdclk_freq;
> +		clock = KHz(dev_priv->cdclk_freq);
>  
>  	return clock / (pwm_freq_hz * 32);
>  }
> @@ -1332,9 +1328,9 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	int clock;
>  
>  	if (IS_G4X(dev_priv))
> -		clock = MHz(intel_hrawclk(dev));
> +		clock = KHz(dev_priv->rawclk_freq);
>  	else
> -		clock = 1000 * dev_priv->cdclk_freq;
> +		clock = KHz(dev_priv->cdclk_freq);
>  
>  	return clock / (pwm_freq_hz * 128);
>  }
> @@ -1346,19 +1342,21 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>   */
>  static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
> -	struct drm_device *dev = connector->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int clock;
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	int mul, clock;
>  
>  	if ((I915_READ(CBR1_VLV) & CBR_PWM_CLOCK_MUX_SELECT) == 0) {
> -		if (IS_CHERRYVIEW(dev))
> -			return KHz(19200) / (pwm_freq_hz * 16);
> +		if (IS_CHERRYVIEW(dev_priv))
> +			clock = KHz(19200);
>  		else
> -			return MHz(25) / (pwm_freq_hz * 16);
> +			clock = MHz(25);
> +		mul = 16;
>  	} else {
> -		clock = intel_hrawclk(dev);
> -		return MHz(clock) / (pwm_freq_hz * 128);
> +		clock = KHz(dev_priv->rawclk_freq);
> +		mul = 128;
>  	}
> +
> +	return clock / (pwm_freq_hz * mul);
>  }
>  
>  static u32 get_backlight_max_vbt(struct intel_connector *connector)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/6] drm/i915: Store rawclk_freq in dev_priv

[Ville Syrjälä]

On Thu, Mar 03, 2016 at 10:47:38AM +0200, Jani Nikula wrote:
> On Wed, 02 Mar 2016, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Generalize rawclk handling by storing it in dev_priv.
> >
> > Presumably our hrawclk readout works at least for CTG and ELK
> > since we've been using it for DP AUX on those platforms. There
> > are no real docs anymore after configdb vanished, so the only
> > reference is the public CTG GMCH spec. What bits are listed in
> > that doc match our code. The ELK GMCH spec have no relevant
> > details unfortunately.
> >
> > The PNV situation is less clear. Starting from
> > commit aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> > we assume that the CTG/ELK hrawclk readout works for PNV as well.
> > At least the results *seem* reasonable for one PNV machine (Lenovo
> > Ideapad S10-3t). Sadly the PNV GMCH spec doesn't have the goods on
> > the relevant register either.
> >
> > So let's keep assuming it works for PNV,ELK,CTG and read it out on
> > those platforms. G33 also has hrawclk according to some notes
> > in BSpec, but we don't actually need it for anything, so let's not
> > even try to read it out there.
> 
> While the above is useful information, I don't think it's specifically
> related to the changes in this patch. The only thing that changes here
> is that the rawclk is read and stored once, instead of being read every
> time.

Sure. But I wanted those considerations on some record in case someone
ever wonders about this code.

> 
> There are a few nitpicks below, but I don't insist on doing anything
> with them.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> >
> > v2: Rebase due to IS_VALLYVIEW vs. IS_CHERRYVIEW split
> >     Use KHz() all over, and kill off a few useless temp variables
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  1 +
> >  drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++--------------
> >  drivers/gpu/drm/i915/intel_dp.c      | 16 +++++------
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 --
> >  drivers/gpu/drm/i915/intel_panel.c   | 42 +++++++++++++--------------
> >  5 files changed, 62 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2cb0a411c10e..b0e7f35a8be4 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1837,6 +1837,7 @@ struct drm_i915_private {
> >  	unsigned int skl_boot_cdclk;
> >  	unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq;
> >  	unsigned int max_dotclk_freq;
> > +	unsigned int rawclk_freq;
> >  	unsigned int hpll_freq;
> >  	unsigned int czclk_freq;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 44fcff0343f2..330528c1fb28 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -169,49 +169,61 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> >  	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> >  }
> >  
> > -int
> > -intel_pch_rawclk(struct drm_device *dev)
> > +static int
> > +intel_pch_rawclk(struct drm_i915_private *dev_priv)
> 
> Now that you make these static, it feels a bit silly to have both intel
> and platform in the name. But meh, no need to resend over that.

Yeah, you commented on that before too I think, and I just forgot to
change it when I realized I should resend.

> 
> >  {
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > -	WARN_ON(!HAS_PCH_SPLIT(dev));
> > +	return (I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK) * 1000;
> > +}
> >  
> > -	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
> > +static int
> > +intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> > +{
> > +	return 200000;
> >  }
> >  
> > -/* hrawclock is 1/4 the FSB frequency */
> > -int intel_hrawclk(struct drm_device *dev)
> > +static int
> > +intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
> >  {
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	uint32_t clkcfg;
> >  
> > -	/* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */
> > -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> > -		return 200;
> > -
> > +	/* hrawclock is 1/4 the FSB frequency */
> >  	clkcfg = I915_READ(CLKCFG);
> >  	switch (clkcfg & CLKCFG_FSB_MASK) {
> >  	case CLKCFG_FSB_400:
> > -		return 100;
> > +		return 100000;
> >  	case CLKCFG_FSB_533:
> > -		return 133;
> > +		return 133333;
> >  	case CLKCFG_FSB_667:
> > -		return 166;
> > +		return 166667;
> >  	case CLKCFG_FSB_800:
> > -		return 200;
> > +		return 200000;
> >  	case CLKCFG_FSB_1067:
> > -		return 266;
> > +		return 266667;
> >  	case CLKCFG_FSB_1333:
> > -		return 333;
> > +		return 333333;
> >  	/* these two are just a guess; one of them might be right */
> >  	case CLKCFG_FSB_1600:
> >  	case CLKCFG_FSB_1600_ALT:
> > -		return 400;
> > +		return 400000;
> >  	default:
> > -		return 133;
> > +		return 133333;
> >  	}
> >  }
> >  
> > +static void intel_update_rawclk(struct drm_i915_private *dev_priv)
> > +{
> > +	if (HAS_PCH_SPLIT(dev_priv))
> > +		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
> > +	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > +		dev_priv->rawclk_freq = intel_vlv_hrawclk(dev_priv);
> > +	else if (IS_G4X(dev_priv) || IS_PINEVIEW(dev_priv))
> > +		dev_priv->rawclk_freq = intel_g4x_hrawclk(dev_priv);
> > +	else
> > +		return; /* no rawclk on other platforms, or no need to know it */
> > +
> > +	DRM_DEBUG_DRIVER("rawclk rate: %d kHz\n", dev_priv->rawclk_freq);
> > +}
> > +
> >  static void intel_update_czclk(struct drm_i915_private *dev_priv)
> >  {
> >  	if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> > @@ -15617,6 +15629,7 @@ void intel_modeset_init(struct drm_device *dev)
> >  	}
> >  
> >  	intel_update_czclk(dev_priv);
> > +	intel_update_rawclk(dev_priv);
> >  	intel_update_cdclk(dev);
> >  
> >  	intel_shared_dpll_init(dev);
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index f272b3541e00..6afdfa720974 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -674,13 +674,13 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> >  static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >  {
> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > -	struct drm_device *dev = intel_dig_port->base.base.dev;
> > +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> >  
> >  	/*
> >  	 * The clock divider is based off the hrawclk, and would like to run at
> >  	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
> >  	 */
> 
> You forget to fix the comment here, but I see that you fix it in a later
> patch... (You know I'm just commenting to show that I've actually read
> the patch properly. ;)

I think you had the exact same comment last time around :)

> 
> > -	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
> > +	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >  }
> >  
> >  static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > @@ -692,12 +692,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >  	if (index)
> >  		return 0;
> >  
> > -	if (intel_dig_port->port == PORT_A) {
> > +	if (intel_dig_port->port == PORT_A)
> >  		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
> > -
> > -	} else {
> > -		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> > -	}
> > +	else
> > +		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >  }
> >  
> >  static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > @@ -718,7 +716,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >  		default: return 0;
> >  		}
> >  	} else  {
> > -		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> > +		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >  	}
> >  }
> >  
> > @@ -5254,7 +5252,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	u32 pp_on, pp_off, pp_div, port_sel = 0;
> > -	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
> > +	int div = dev_priv->rawclk_freq / 1000;
> >  	i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
> >  	enum port port = dp_to_dig_port(intel_dp)->port;
> >  	const struct edp_power_seq *seq = &intel_dp->pps_delays;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index cb413e246267..e7485ec34f63 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1085,8 +1085,6 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
> >  extern const struct drm_plane_funcs intel_plane_funcs;
> >  unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
> >  bool intel_has_pending_fb_unpin(struct drm_device *dev);
> > -int intel_pch_rawclk(struct drm_device *dev);
> > -int intel_hrawclk(struct drm_device *dev);
> >  void intel_mark_busy(struct drm_device *dev);
> >  void intel_mark_idle(struct drm_device *dev);
> >  void intel_crtc_restore_mode(struct drm_crtc *crtc);
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 21ee6477bf98..5cf377507162 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1251,16 +1251,14 @@ static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  static u32 spt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > -	u32 mul, clock;
> > +	u32 mul;
> >  
> >  	if (I915_READ(SOUTH_CHICKEN1) & SPT_PWM_GRANULARITY)
> >  		mul = 128;
> >  	else
> >  		mul = 16;
> >  
> > -	clock = MHz(24);
> > -
> > -	return clock / (pwm_freq_hz * mul);
> > +	return MHz(24) / (pwm_freq_hz * mul);
> 
> Unrelated change, really.

You complained about this sort of stuff last time in the functions that
used rawclk/cdclk, so after I changed those the inconsistency drove me
mad and I had to change everything else as well.

> 
> >  }
> >  
> >  /*
> > @@ -1292,10 +1290,9 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >   */
> >  static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> > -	struct drm_device *dev = connector->base.dev;
> > -	int clock = MHz(intel_pch_rawclk(dev));
> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  
> > -	return clock / (pwm_freq_hz * 128);
> > +	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
> >  }
> >  
> >  /*
> > @@ -1308,14 +1305,13 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >   */
> >  static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> > -	struct drm_device *dev = connector->base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	int clock;
> >  
> > -	if (IS_PINEVIEW(dev))
> > -		clock = MHz(intel_hrawclk(dev));
> > +	if (IS_PINEVIEW(dev_priv))
> > +		clock = KHz(dev_priv->rawclk_freq);
> >  	else
> > -		clock = 1000 * dev_priv->cdclk_freq;
> > +		clock = KHz(dev_priv->cdclk_freq);
> >  
> >  	return clock / (pwm_freq_hz * 32);
> >  }
> > @@ -1332,9 +1328,9 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  	int clock;
> >  
> >  	if (IS_G4X(dev_priv))
> > -		clock = MHz(intel_hrawclk(dev));
> > +		clock = KHz(dev_priv->rawclk_freq);
> >  	else
> > -		clock = 1000 * dev_priv->cdclk_freq;
> > +		clock = KHz(dev_priv->cdclk_freq);
> >  
> >  	return clock / (pwm_freq_hz * 128);
> >  }
> > @@ -1346,19 +1342,21 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >   */
> >  static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> > -	struct drm_device *dev = connector->base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	int clock;
> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > +	int mul, clock;
> >  
> >  	if ((I915_READ(CBR1_VLV) & CBR_PWM_CLOCK_MUX_SELECT) == 0) {
> > -		if (IS_CHERRYVIEW(dev))
> > -			return KHz(19200) / (pwm_freq_hz * 16);
> > +		if (IS_CHERRYVIEW(dev_priv))
> > +			clock = KHz(19200);
> >  		else
> > -			return MHz(25) / (pwm_freq_hz * 16);
> > +			clock = MHz(25);
> > +		mul = 16;
> >  	} else {
> > -		clock = intel_hrawclk(dev);
> > -		return MHz(clock) / (pwm_freq_hz * 128);
> > +		clock = KHz(dev_priv->rawclk_freq);
> > +		mul = 128;
> >  	}
> > +
> > +	return clock / (pwm_freq_hz * mul);
> >  }
> >  
> >  static u32 get_backlight_max_vbt(struct intel_connector *connector)
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/6] drm/i915: Rename s/i9xx/g4x/ in DP code

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

g4x is the first platform with DP support, so let's name the relevant
functions as g4x_ instead i9xx_ to avoid confusion.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6afdfa720974..83fada3e4cec 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -671,7 +671,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	return status;
 }
 
-static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
+static uint32_t g4x_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
@@ -735,10 +735,10 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	return index ? 0 : 1;
 }
 
-static uint32_t i9xx_get_aux_send_ctl(struct intel_dp *intel_dp,
-				      bool has_aux_irq,
-				      int send_bytes,
-				      uint32_t aux_clock_divider)
+static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
+				     bool has_aux_irq,
+				     int send_bytes,
+				     uint32_t aux_clock_divider)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -5852,12 +5852,12 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	else if (HAS_PCH_SPLIT(dev))
 		intel_dp->get_aux_clock_divider = ilk_get_aux_clock_divider;
 	else
-		intel_dp->get_aux_clock_divider = i9xx_get_aux_clock_divider;
+		intel_dp->get_aux_clock_divider = g4x_get_aux_clock_divider;
 
 	if (INTEL_INFO(dev)->gen >= 9)
 		intel_dp->get_aux_send_ctl = skl_get_aux_send_ctl;
 	else
-		intel_dp->get_aux_send_ctl = i9xx_get_aux_send_ctl;
+		intel_dp->get_aux_send_ctl = g4x_get_aux_send_ctl;
 
 	if (HAS_DDI(dev))
 		intel_dp->prepare_link_retrain = intel_ddi_prepare_link_retrain;
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/6] drm/i915: Use g4x_get_aux_clock_divider() for VLV/CHV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

With the hrawclk frequency cached in dev_priv, we can simply use
g4x_get_aux_clock_divider() for VLV/CHV.

v2: Rebase due to IS_VALLYVIEW vs. IS_CHERRYVIEW split

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 83fada3e4cec..b0ad6201680d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -720,11 +720,6 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	}
 }
 
-static uint32_t vlv_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
-{
-	return index ? 0 : 100;
-}
-
 static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	/*
@@ -5845,8 +5840,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	/* intel_dp vfuncs */
 	if (INTEL_INFO(dev)->gen >= 9)
 		intel_dp->get_aux_clock_divider = skl_get_aux_clock_divider;
-	else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
-		intel_dp->get_aux_clock_divider = vlv_get_aux_clock_divider;
 	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		intel_dp->get_aux_clock_divider = hsw_get_aux_clock_divider;
 	else if (HAS_PCH_SPLIT(dev))
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/6] drm/i915: Read out hrawclk from CCK on vlv/chv

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we assume that hrawclk is 200MHz on VLV/CHV. That should
be true always, but just to avoid such asumptions we can read out the
actual frequency from CCK.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 1 +
 drivers/gpu/drm/i915/intel_display.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 71abf5725495..c4606c7ad8d6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -786,6 +786,7 @@ enum skl_disp_power_wells {
 #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
 #define CCK_CZ_CLOCK_CONTROL			0x62
 #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
+#define CCK_DISPLAY_REF_CLOCK_CONTROL		0x6c
 #define  CCK_TRUNK_FORCE_ON			(1 << 17)
 #define  CCK_TRUNK_FORCE_OFF			(1 << 16)
 #define  CCK_FREQUENCY_STATUS			(0x1f << 8)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 330528c1fb28..f5a757bec3f7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -178,7 +178,8 @@ intel_pch_rawclk(struct drm_i915_private *dev_priv)
 static int
 intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
 {
-	return 200000;
+	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
+				      CCK_DISPLAY_REF_CLOCK_CONTROL);
 }
 
 static int
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Read out hrawclk from CCK on vlv/chv

[Jani Nikula]

On Wed, 02 Mar 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we assume that hrawclk is 200MHz on VLV/CHV. That should
> be true always, but just to avoid such asumptions we can read out the
> actual frequency from CCK.

Okay, so I don't want to spend forever looking for the spec section
where I can verify this... please just pinpoint me the location. ;)

And whenever I see "that should be true always", I feel like there
should be a WARN_ON(*gasp* it's not true this time!)...

BR,
Jani.


>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 1 +
>  drivers/gpu/drm/i915/intel_display.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 71abf5725495..c4606c7ad8d6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -786,6 +786,7 @@ enum skl_disp_power_wells {
>  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
>  #define CCK_CZ_CLOCK_CONTROL			0x62
>  #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
> +#define CCK_DISPLAY_REF_CLOCK_CONTROL		0x6c
>  #define  CCK_TRUNK_FORCE_ON			(1 << 17)
>  #define  CCK_TRUNK_FORCE_OFF			(1 << 16)
>  #define  CCK_FREQUENCY_STATUS			(0x1f << 8)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 330528c1fb28..f5a757bec3f7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -178,7 +178,8 @@ intel_pch_rawclk(struct drm_i915_private *dev_priv)
>  static int
>  intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
>  {
> -	return 200000;
> +	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
> +				      CCK_DISPLAY_REF_CLOCK_CONTROL);
>  }
>  
>  static int

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Read out hrawclk from CCK on vlv/chv

[Ville Syrjälä]

On Thu, Mar 03, 2016 at 10:51:23AM +0200, Jani Nikula wrote:
> On Wed, 02 Mar 2016, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently we assume that hrawclk is 200MHz on VLV/CHV. That should
> > be true always, but just to avoid such asumptions we can read out the
> > actual frequency from CCK.
> 
> Okay, so I don't want to spend forever looking for the spec section
> where I can verify this... please just pinpoint me the location. ;)

I've asked Imre to have a look at this since he should have all the
docs around I think, so you don't have to if you don't want to

If you do want to dig at it you'll want to look at the
VLV/CHV Display Cluster HAS and the VLV North Clock HAS/CHV Clock HAS.

> 
> And whenever I see "that should be true always", I feel like there
> should be a WARN_ON(*gasp* it's not true this time!)...

I guess we could, but at least for the places where we use this, it
wouldn't actually matter if it's not true.

> 
> BR,
> Jani.
> 
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      | 1 +
> >  drivers/gpu/drm/i915/intel_display.c | 3 ++-
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 71abf5725495..c4606c7ad8d6 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -786,6 +786,7 @@ enum skl_disp_power_wells {
> >  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
> >  #define CCK_CZ_CLOCK_CONTROL			0x62
> >  #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
> > +#define CCK_DISPLAY_REF_CLOCK_CONTROL		0x6c
> >  #define  CCK_TRUNK_FORCE_ON			(1 << 17)
> >  #define  CCK_TRUNK_FORCE_OFF			(1 << 16)
> >  #define  CCK_FREQUENCY_STATUS			(0x1f << 8)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 330528c1fb28..f5a757bec3f7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -178,7 +178,8 @@ intel_pch_rawclk(struct drm_i915_private *dev_priv)
> >  static int
> >  intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> >  {
> > -	return 200000;
> > +	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
> > +				      CCK_DISPLAY_REF_CLOCK_CONTROL);
> >  }
> >  
> >  static int
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Read out hrawclk from CCK on vlv/chv

[Imre Deak]

On Wed, 2016-03-02 at 17:22 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we assume that hrawclk is 200MHz on VLV/CHV. That should
> be true always, but just to avoid such asumptions we can read out the
> actual frequency from CCK.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Matches both the VLV and CHV clock spec, so:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 1 +
>  drivers/gpu/drm/i915/intel_display.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 71abf5725495..c4606c7ad8d6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -786,6 +786,7 @@ enum skl_disp_power_wells {
>  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
>  #define CCK_CZ_CLOCK_CONTROL			0x62
>  #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
> +#define CCK_DISPLAY_REF_CLOCK_CONTROL		0x6c
>  #define  CCK_TRUNK_FORCE_ON			(1 << 17)
>  #define  CCK_TRUNK_FORCE_OFF			(1 << 16)
>  #define  CCK_FREQUENCY_STATUS			(0x1f << 8)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 330528c1fb28..f5a757bec3f7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -178,7 +178,8 @@ intel_pch_rawclk(struct drm_i915_private
> *dev_priv)
>  static int
>  intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
>  {
> -	return 200000;
> +	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
> +				      CCK_DISPLAY_REF_CLOCK_CONTROL)
> ;
>  }
>  
>  static int
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/6] drm/i915: Clean up .get_aux_clock_divider() functions

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that the mess with AUX clock divder rounding is sorted out and
we have both cdclk and rawclk cached in dev_priv, we can clean up
the .get_aux_clock_divider() functions a bit.

The main thing here is just calling ilk_get_aux_clock_divider()
from hsw_get_aux_clock_divider() except for the LPT:H special
case.

We could go further and call g4x_get_aux_clock_divider() from
ilk_get_aux_clock_divider() for the PCH ports, but I'm sure Jani
would object, so leave that be.

While at it repeat the comment where the AUX clock comes from
in ilk_get_aux_clock_divider().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b0ad6201680d..c24d1a6ad912 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -676,22 +676,29 @@ static uint32_t g4x_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 
+	if (index)
+		return 0;
+
 	/*
 	 * The clock divider is based off the hrawclk, and would like to run at
-	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
+	 * 2MHz.  So, take the hrawclk value and divide by 2000 and use that
 	 */
-	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
+	return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 }
 
 static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = intel_dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 
 	if (index)
 		return 0;
 
+	/*
+	 * The clock divider is based off the cdclk or PCH rawclk, and would
+	 * like to run at 2MHz.  So, take the cdclk or PCH rawclk value and
+	 * divide by 2000 and use that
+	 */
 	if (intel_dig_port->port == PORT_A)
 		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
 	else
@@ -701,23 +708,18 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = intel_dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 
-	if (intel_dig_port->port == PORT_A) {
-		if (index)
-			return 0;
-		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
-	} else if (HAS_PCH_LPT_H(dev_priv)) {
+	if (intel_dig_port->port != PORT_A && HAS_PCH_LPT_H(dev_priv)) {
 		/* Workaround for non-ULT HSW */
 		switch (index) {
 		case 0: return 63;
 		case 1: return 72;
 		default: return 0;
 		}
-	} else  {
-		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 	}
+
+	return ilk_get_aux_clock_divider(intel_dp, index);
 }
 
 static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/6] drm/i915: Use DIV_ROUND_CLOSEST for PWM calculations

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Supposedly we would want to get the PWM output as close as possible to
the target, so let's round to closest.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_panel.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 5cf377507162..0fe059bc7d80 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1240,7 +1240,7 @@ static void intel_backlight_device_unregister(struct intel_connector *connector)
  */
 static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
-	return KHz(19200) / pwm_freq_hz;
+	return DIV_ROUND_CLOSEST(KHz(19200), pwm_freq_hz);
 }
 
 /*
@@ -1258,7 +1258,7 @@ static u32 spt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	else
 		mul = 16;
 
-	return MHz(24) / (pwm_freq_hz * mul);
+	return DIV_ROUND_CLOSEST(MHz(24), pwm_freq_hz * mul);
 }
 
 /*
@@ -1281,7 +1281,7 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	else
 		clock = MHz(24); /* LPT:LP */
 
-	return clock / (pwm_freq_hz * mul);
+	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * mul);
 }
 
 /*
@@ -1292,7 +1292,7 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 
-	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
+	return DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), pwm_freq_hz * 128);
 }
 
 /*
@@ -1313,7 +1313,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	else
 		clock = KHz(dev_priv->cdclk_freq);
 
-	return clock / (pwm_freq_hz * 32);
+	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * 32);
 }
 
 /*
@@ -1332,7 +1332,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	else
 		clock = KHz(dev_priv->cdclk_freq);
 
-	return clock / (pwm_freq_hz * 128);
+	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * 128);
 }
 
 /*
@@ -1356,7 +1356,7 @@ static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 		mul = 128;
 	}
 
-	return clock / (pwm_freq_hz * mul);
+	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * mul);
 }
 
 static u32 get_backlight_max_vbt(struct intel_connector *connector)
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Use DIV_ROUND_CLOSEST for PWM calculations

[Jani Nikula]

On Wed, 02 Mar 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Supposedly we would want to get the PWM output as close as possible to
> the target, so let's round to closest.
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>

I have zero recollection of asking this change, but since it makes sense
I won't deny it was me.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 5cf377507162..0fe059bc7d80 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1240,7 +1240,7 @@ static void intel_backlight_device_unregister(struct intel_connector *connector)
>   */
>  static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
> -	return KHz(19200) / pwm_freq_hz;
> +	return DIV_ROUND_CLOSEST(KHz(19200), pwm_freq_hz);
>  }
>  
>  /*
> @@ -1258,7 +1258,7 @@ static u32 spt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	else
>  		mul = 16;
>  
> -	return MHz(24) / (pwm_freq_hz * mul);
> +	return DIV_ROUND_CLOSEST(MHz(24), pwm_freq_hz * mul);
>  }
>  
>  /*
> @@ -1281,7 +1281,7 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	else
>  		clock = MHz(24); /* LPT:LP */
>  
> -	return clock / (pwm_freq_hz * mul);
> +	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * mul);
>  }
>  
>  /*
> @@ -1292,7 +1292,7 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  
> -	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
> +	return DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), pwm_freq_hz * 128);
>  }
>  
>  /*
> @@ -1313,7 +1313,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	else
>  		clock = KHz(dev_priv->cdclk_freq);
>  
> -	return clock / (pwm_freq_hz * 32);
> +	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * 32);
>  }
>  
>  /*
> @@ -1332,7 +1332,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	else
>  		clock = KHz(dev_priv->cdclk_freq);
>  
> -	return clock / (pwm_freq_hz * 128);
> +	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * 128);
>  }
>  
>  /*
> @@ -1356,7 +1356,7 @@ static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  		mul = 128;
>  	}
>  
> -	return clock / (pwm_freq_hz * mul);
> +	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * mul);
>  }
>  
>  static u32 get_backlight_max_vbt(struct intel_connector *connector)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Use DIV_ROUND_CLOSEST for PWM calculations

[Ville Syrjälä]

On Thu, Mar 03, 2016 at 10:25:06AM +0200, Jani Nikula wrote:
> On Wed, 02 Mar 2016, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Supposedly we would want to get the PWM output as close as possible to
> > the target, so let's round to closest.
> >
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> 
> I have zero recollection of asking this change, but since it makes sense
> I won't deny it was me.

https://lists.freedesktop.org/archives/intel-gfx/2015-December/081700.html

> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_panel.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 5cf377507162..0fe059bc7d80 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1240,7 +1240,7 @@ static void intel_backlight_device_unregister(struct intel_connector *connector)
> >   */
> >  static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> > -	return KHz(19200) / pwm_freq_hz;
> > +	return DIV_ROUND_CLOSEST(KHz(19200), pwm_freq_hz);
> >  }
> >  
> >  /*
> > @@ -1258,7 +1258,7 @@ static u32 spt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  	else
> >  		mul = 16;
> >  
> > -	return MHz(24) / (pwm_freq_hz * mul);
> > +	return DIV_ROUND_CLOSEST(MHz(24), pwm_freq_hz * mul);
> >  }
> >  
> >  /*
> > @@ -1281,7 +1281,7 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  	else
> >  		clock = MHz(24); /* LPT:LP */
> >  
> > -	return clock / (pwm_freq_hz * mul);
> > +	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * mul);
> >  }
> >  
> >  /*
> > @@ -1292,7 +1292,7 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  
> > -	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
> > +	return DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), pwm_freq_hz * 128);
> >  }
> >  
> >  /*
> > @@ -1313,7 +1313,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  	else
> >  		clock = KHz(dev_priv->cdclk_freq);
> >  
> > -	return clock / (pwm_freq_hz * 32);
> > +	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * 32);
> >  }
> >  
> >  /*
> > @@ -1332,7 +1332,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  	else
> >  		clock = KHz(dev_priv->cdclk_freq);
> >  
> > -	return clock / (pwm_freq_hz * 128);
> > +	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * 128);
> >  }
> >  
> >  /*
> > @@ -1356,7 +1356,7 @@ static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  		mul = 128;
> >  	}
> >  
> > -	return clock / (pwm_freq_hz * mul);
> > +	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * mul);
> >  }
> >  
> >  static u32 get_backlight_max_vbt(struct intel_connector *connector)
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [v2,1/6] drm/i915: Store rawclk_freq in dev_priv

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/6] drm/i915: Store rawclk_freq in dev_priv
URL   : https://patchwork.freedesktop.org/series/4024/
State : failure

== Summary ==

Series 4024v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/4024/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup force-load-detect:
                skip       -> PASS       (ivb-t430s)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                dmesg-warn -> PASS       (hsw-brixbox)
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (hsw-gt2)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                fail       -> DMESG-FAIL (snb-x220t)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (snb-x220t)

bdw-nuci7        total:169  pass:158  dwarn:0   dfail:0   fail:0   skip:11 
bdw-ultra        total:169  pass:155  dwarn:0   dfail:0   fail:0   skip:14 
bsw-nuc-2        total:169  pass:138  dwarn:0   dfail:0   fail:1   skip:30 
byt-nuc          total:169  pass:144  dwarn:0   dfail:0   fail:0   skip:25 
hsw-brixbox      total:169  pass:154  dwarn:0   dfail:0   fail:0   skip:15 
hsw-gt2          total:169  pass:157  dwarn:2   dfail:0   fail:0   skip:10 
ivb-t430s        total:169  pass:154  dwarn:0   dfail:0   fail:0   skip:15 
skl-i5k-2        total:169  pass:153  dwarn:0   dfail:0   fail:0   skip:16 
skl-i7k-2        total:169  pass:153  dwarn:0   dfail:0   fail:0   skip:16 
snb-dellxps      total:169  pass:145  dwarn:1   dfail:0   fail:0   skip:23 
snb-x220t        total:169  pass:146  dwarn:0   dfail:1   fail:0   skip:22 

Results at /archive/results/CI_IGT_test/Patchwork_1518/

db506392f6706faffdc965c53c4cdea58cc16a02 drm-intel-nightly: 2016y-03m-02d-13h-47m-11s UTC integration manifest
86d1897d65543d83ea61d353402ea528bd09c833 drm/i915: Use DIV_ROUND_CLOSEST for PWM calculations
9740675dbf1fd7a0645acc20cacba3dffbdd0681 drm/i915: Clean up .get_aux_clock_divider() functions
cdce73816277bbb1d86f5240086c49381d8f478d drm/i915: Read out hrawclk from CCK on vlv/chv
f5b631d4e2f01063d9df36fac1549071151346f6 drm/i915: Use g4x_get_aux_clock_divider() for VLV/CHV
f9b4fa5dd87d16b63f5f9033d9af6bd5590aab60 drm/i915: Rename s/i9xx/g4x/ in DP code
f2f8477cf45930518f6a3feb29b15eece5a299cd drm/i915: Store rawclk_freq in dev_priv

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for series starting with [v2,1/6] drm/i915: Store rawclk_freq in dev_priv

[Ville Syrjälä]

On Wed, Mar 02, 2016 at 04:55:52PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [v2,1/6] drm/i915: Store rawclk_freq in dev_priv
> URL   : https://patchwork.freedesktop.org/series/4024/
> State : failure
> 
> == Summary ==
> 
> Series 4024v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/4024/revisions/1/mbox/
> 
> Test kms_force_connector_basic:
>         Subgroup force-load-detect:
>                 skip       -> PASS       (ivb-t430s)
> Test kms_pipe_crc_basic:
>         Subgroup nonblocking-crc-pipe-b-frame-sequence:
>                 dmesg-warn -> PASS       (hsw-brixbox)
>         Subgroup suspend-read-crc-pipe-a:
>                 incomplete -> PASS       (hsw-gt2)
>         Subgroup suspend-read-crc-pipe-c:
>                 dmesg-warn -> PASS       (bsw-nuc-2)
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 fail       -> DMESG-FAIL (snb-x220t)

[  304.946208] Device suspended during HW access
...
[  304.946557]  [<ffffffffa03bcc6c>] gen6_write32+0x1dc/0x270 [i915]
[  304.946651]  [<ffffffffa0366a68>] _ilk_disable_lp_wm+0x98/0xd0 [i915]

which is a bit odd considering the test itself always fails to autosuspend
the device on this particular machine.

Anyway, part of this bug:
https://bugs.freedesktop.org/show_bug.cgi?id=94349

>         Subgroup basic-rte:
>                 dmesg-warn -> PASS       (snb-x220t)
> 
> bdw-nuci7        total:169  pass:158  dwarn:0   dfail:0   fail:0   skip:11 
> bdw-ultra        total:169  pass:155  dwarn:0   dfail:0   fail:0   skip:14 
> bsw-nuc-2        total:169  pass:138  dwarn:0   dfail:0   fail:1   skip:30 
> byt-nuc          total:169  pass:144  dwarn:0   dfail:0   fail:0   skip:25 
> hsw-brixbox      total:169  pass:154  dwarn:0   dfail:0   fail:0   skip:15 
> hsw-gt2          total:169  pass:157  dwarn:2   dfail:0   fail:0   skip:10 
> ivb-t430s        total:169  pass:154  dwarn:0   dfail:0   fail:0   skip:15 
> skl-i5k-2        total:169  pass:153  dwarn:0   dfail:0   fail:0   skip:16 
> skl-i7k-2        total:169  pass:153  dwarn:0   dfail:0   fail:0   skip:16 
> snb-dellxps      total:169  pass:145  dwarn:1   dfail:0   fail:0   skip:23 
> snb-x220t        total:169  pass:146  dwarn:0   dfail:1   fail:0   skip:22 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1518/
> 
> db506392f6706faffdc965c53c4cdea58cc16a02 drm-intel-nightly: 2016y-03m-02d-13h-47m-11s UTC integration manifest
> 86d1897d65543d83ea61d353402ea528bd09c833 drm/i915: Use DIV_ROUND_CLOSEST for PWM calculations
> 9740675dbf1fd7a0645acc20cacba3dffbdd0681 drm/i915: Clean up .get_aux_clock_divider() functions
> cdce73816277bbb1d86f5240086c49381d8f478d drm/i915: Read out hrawclk from CCK on vlv/chv
> f5b631d4e2f01063d9df36fac1549071151346f6 drm/i915: Use g4x_get_aux_clock_divider() for VLV/CHV
> f9b4fa5dd87d16b63f5f9033d9af6bd5590aab60 drm/i915: Rename s/i9xx/g4x/ in DP code
> f2f8477cf45930518f6a3feb29b15eece5a299cd drm/i915: Store rawclk_freq in dev_priv

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 drm/i915: rawclk/cdclk stuff (v2)

[Ville Syrjälä]

On Wed, Mar 02, 2016 at 05:22:12PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Here are the remainder of my cdclk/rawclk cleanup patches. Previous time I
> posted these was [1]. Bunch of them got already r-b'd by Jani. I also did
> the PWM DIV_ROUND_CLOSEST() changes as Jani suggested last time around.
> 
> Entire series is available here:
> git://github.com/vsyrjala/linux.git rawclk_freq_10
> 
> Ville Syrjälä (6):
>   drm/i915: Store rawclk_freq in dev_priv
>   drm/i915: Rename s/i9xx/g4x/ in DP code
>   drm/i915: Use g4x_get_aux_clock_divider() for VLV/CHV
>   drm/i915: Read out hrawclk from CCK on vlv/chv
>   drm/i915: Clean up .get_aux_clock_divider() functions
>   drm/i915: Use DIV_ROUND_CLOSEST for PWM calculations

Series pushed to dinq. Thanks for the reviews.

> 
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 56 +++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_dp.c      | 59 ++++++++++++++++--------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 --
>  drivers/gpu/drm/i915/intel_panel.c   | 50 +++++++++++++++---------------
>  6 files changed, 87 insertions(+), 82 deletions(-)
> 
> -- 
> 2.4.10

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx