[PATCH 00/11] drm/i915: adjusted_mode.clock vs. port_clock v3

[PATCH 00/11] drm/i915: adjusted_mode.clock vs. port_clock v3

[ville.syrjala]

Another day, another attempt.

This series replaces the earlier attempt:
[PATCH 0/8] drm/i915: adjusted_mode.clock vs. port_clock v2 

The main new idea is to make it the encoders' responsibility to calculate
adjusted_mode.clock. In order to do that I had to add DP M/N extraction and
i9xx_crtc_clock_get() had to be improved so that it can dig out the PCH DPLL
frequency. The rest is more or less the same old.

As a bonus we now get to cross check the FDI's idea of dotclock against the
encoder's idea.

Also now CTG too can calculate the dotclock from the DP port_clock. I don't
think that could have worked before as the DPLL was running at the DP link
frequency, so we read that out into adjusted_mode.clock and then compared
it with whatever the user requested. Not the same thing at all. But now it
should just work (tm).

I gave it a quick whirl on my ILK eDP + DP, and IVB HDMI and DP. Neither
machine started to yell at me, so I think it's in a semi-decent shape at
least. Unfortunately I don't have a CTG w/ DP so I can't test that part,
nor do I have any SDVO/DVO hardware.

[PATCH 01/11] drm/i915: Don't factor in pixel multplier when deriving dotclock from link clock and M/N values

[ville.syrjala]

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

We feed the non-multiplied clock to intel_link_compute_m_n(), so the
opposite operation should use the same order of operations. So we just
multiply by pixel_multiplier in the end now.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d88057e..b45c6e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7340,20 +7340,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	int link_freq, repeat;
+	int link_freq;
 	u64 clock;
 	u32 link_m, link_n;
 
-	repeat = pipe_config->pixel_multiplier;
-
 	/*
 	 * The calculation for the data clock is:
-	 * pixel_clock = ((m/n)*(link_clock * nr_lanes * repeat))/bpp
+	 * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp
 	 * But we want to avoid losing precison if possible, so:
-	 * pixel_clock = ((m * link_clock * nr_lanes * repeat)/(n*bpp))
+	 * pixel_clock = ((m * link_clock * nr_lanes)/(n*bpp))
 	 *
 	 * and the link clock is simpler:
-	 * link_clock = (m * link_clock * repeat) / n
+	 * link_clock = (m * link_clock) / n
 	 */
 
 	/*
@@ -7375,10 +7373,11 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	if (!link_m || !link_n)
 		return;
 
-	clock = ((u64)link_m * (u64)link_freq * (u64)repeat);
+	clock = ((u64)link_m * (u64)link_freq);
 	do_div(clock, link_n);
 
-	pipe_config->adjusted_mode.clock = clock;
+	pipe_config->adjusted_mode.clock = clock *
+		pipe_config->pixel_multiplier;
 }
 
 /** Returns the currently programmed mode of the given pipe. */
-- 
1.8.1.5

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

[PATCH v2 02/11] drm/i915: Make adjusted_mode.clock non-pixel multiplied

[ville.syrjala]

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

It would be easier if adjusted_mode.clock would be the pipe pixel clock,
and it actually is, except for the cases where pixel_multiplier > 1.

So let's change intel_sdvo to use port_clock as the multiplied clock,
and then we can leave adjusted_mode.clock as pipe pixel clock.

v2: Improve port_clock documentation
    Rebased on top of SDVO pixel_multiplier fixes

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++----
 drivers/gpu/drm/i915/intel_drv.h     | 5 ++++-
 drivers/gpu/drm/i915/intel_sdvo.c    | 4 +---
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b45c6e6..2aac205 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4062,7 +4062,6 @@ retry:
 	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
 
 	fdi_dotclock = adjusted_mode->clock;
-	fdi_dotclock /= pipe_config->pixel_multiplier;
 
 	lane = ironlake_get_lanes_required(fdi_dotclock, link_bw,
 					   pipe_config->pipe_bpp);
@@ -7376,8 +7375,7 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	clock = ((u64)link_m * (u64)link_freq);
 	do_div(clock, link_n);
 
-	pipe_config->adjusted_mode.clock = clock *
-		pipe_config->pixel_multiplier;
+	pipe_config->adjusted_mode.clock = clock;
 }
 
 /** Returns the currently programmed mode of the given pipe. */
@@ -8322,7 +8320,8 @@ encoder_retry:
 	/* Set default port clock if not overwritten by the encoder. Needs to be
 	 * done afterwards in case the encoder adjusts the mode. */
 	if (!pipe_config->port_clock)
-		pipe_config->port_clock = pipe_config->adjusted_mode.clock;
+		pipe_config->port_clock = pipe_config->adjusted_mode.clock *
+			pipe_config->pixel_multiplier;
 
 	ret = intel_crtc_compute_config(to_intel_crtc(crtc), pipe_config);
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ea97c23..dbf04be 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -212,6 +212,8 @@ struct intel_crtc_config {
 	unsigned long quirks;
 
 	struct drm_display_mode requested_mode;
+	/* Actual pipe timings ie. what we program into the pipe timing
+	 * registers. adjusted_mode.clock is the pipe pixel clock. */
 	struct drm_display_mode adjusted_mode;
 	/* Whether to set up the PCH/FDI. Note that we never allow sharing
 	 * between pch encoders and cpu encoders. */
@@ -266,7 +268,8 @@ struct intel_crtc_config {
 
 	/*
 	 * Frequence the dpll for the port should run at. Differs from the
-	 * adjusted dotclock e.g. for DP or 12bpc hdmi mode.
+	 * adjusted dotclock e.g. for DP or 12bpc hdmi mode. This is also
+	 * already multiplied by pixel_multiplier.
 	 */
 	int port_clock;
 
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 85037b9..74042c6 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1059,7 +1059,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
 
 static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_config *pipe_config)
 {
-	unsigned dotclock = pipe_config->adjusted_mode.clock;
+	unsigned dotclock = pipe_config->port_clock;
 	struct dpll *clock = &pipe_config->dpll;
 
 	/* SDVO TV has fixed PLL values depend on its clock range,
@@ -1124,7 +1124,6 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 	 */
 	pipe_config->pixel_multiplier =
 		intel_sdvo_get_pixel_multiplier(adjusted_mode);
-	adjusted_mode->clock *= pipe_config->pixel_multiplier;
 
 	if (intel_sdvo->color_range_auto) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
@@ -1212,7 +1211,6 @@ static void intel_sdvo_mode_set(struct intel_encoder *intel_encoder)
 	 * adjusted_mode.
 	 */
 	intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
-	input_dtd.part1.clock /= crtc->config.pixel_multiplier;
 
 	if (intel_sdvo->is_tv || intel_sdvo->is_lvds)
 		input_dtd.part2.sdvo_flags = intel_sdvo->dtd_sdvo_flags;
-- 
1.8.1.5

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

Re: [PATCH v2 02/11] drm/i915: Make adjusted_mode.clock non-pixel multiplied

[Jani Nikula]

On Fri, 06 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> It would be easier if adjusted_mode.clock would be the pipe pixel clock,
> and it actually is, except for the cases where pixel_multiplier > 1.
>
> So let's change intel_sdvo to use port_clock as the multiplied clock,
> and then we can leave adjusted_mode.clock as pipe pixel clock.
>
> v2: Improve port_clock documentation
>     Rebased on top of SDVO pixel_multiplier fixes

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 +++----
>  drivers/gpu/drm/i915/intel_drv.h     | 5 ++++-
>  drivers/gpu/drm/i915/intel_sdvo.c    | 4 +---
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b45c6e6..2aac205 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4062,7 +4062,6 @@ retry:
>  	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
>  
>  	fdi_dotclock = adjusted_mode->clock;
> -	fdi_dotclock /= pipe_config->pixel_multiplier;
>  
>  	lane = ironlake_get_lanes_required(fdi_dotclock, link_bw,
>  					   pipe_config->pipe_bpp);
> @@ -7376,8 +7375,7 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	clock = ((u64)link_m * (u64)link_freq);
>  	do_div(clock, link_n);
>  
> -	pipe_config->adjusted_mode.clock = clock *
> -		pipe_config->pixel_multiplier;
> +	pipe_config->adjusted_mode.clock = clock;
>  }
>  
>  /** Returns the currently programmed mode of the given pipe. */
> @@ -8322,7 +8320,8 @@ encoder_retry:
>  	/* Set default port clock if not overwritten by the encoder. Needs to be
>  	 * done afterwards in case the encoder adjusts the mode. */
>  	if (!pipe_config->port_clock)
> -		pipe_config->port_clock = pipe_config->adjusted_mode.clock;
> +		pipe_config->port_clock = pipe_config->adjusted_mode.clock *
> +			pipe_config->pixel_multiplier;
>  
>  	ret = intel_crtc_compute_config(to_intel_crtc(crtc), pipe_config);
>  	if (ret < 0) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ea97c23..dbf04be 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -212,6 +212,8 @@ struct intel_crtc_config {
>  	unsigned long quirks;
>  
>  	struct drm_display_mode requested_mode;
> +	/* Actual pipe timings ie. what we program into the pipe timing
> +	 * registers. adjusted_mode.clock is the pipe pixel clock. */
>  	struct drm_display_mode adjusted_mode;
>  	/* Whether to set up the PCH/FDI. Note that we never allow sharing
>  	 * between pch encoders and cpu encoders. */
> @@ -266,7 +268,8 @@ struct intel_crtc_config {
>  
>  	/*
>  	 * Frequence the dpll for the port should run at. Differs from the
> -	 * adjusted dotclock e.g. for DP or 12bpc hdmi mode.
> +	 * adjusted dotclock e.g. for DP or 12bpc hdmi mode. This is also
> +	 * already multiplied by pixel_multiplier.
>  	 */
>  	int port_clock;
>  
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 85037b9..74042c6 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1059,7 +1059,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
>  
>  static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_config *pipe_config)
>  {
> -	unsigned dotclock = pipe_config->adjusted_mode.clock;
> +	unsigned dotclock = pipe_config->port_clock;
>  	struct dpll *clock = &pipe_config->dpll;
>  
>  	/* SDVO TV has fixed PLL values depend on its clock range,
> @@ -1124,7 +1124,6 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  	 */
>  	pipe_config->pixel_multiplier =
>  		intel_sdvo_get_pixel_multiplier(adjusted_mode);
> -	adjusted_mode->clock *= pipe_config->pixel_multiplier;
>  
>  	if (intel_sdvo->color_range_auto) {
>  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
> @@ -1212,7 +1211,6 @@ static void intel_sdvo_mode_set(struct intel_encoder *intel_encoder)
>  	 * adjusted_mode.
>  	 */
>  	intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
> -	input_dtd.part1.clock /= crtc->config.pixel_multiplier;
>  
>  	if (intel_sdvo->is_tv || intel_sdvo->is_lvds)
>  		input_dtd.part2.sdvo_flags = intel_sdvo->dtd_sdvo_flags;
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH v2 03/11] drm/i915: Add support for pipe_bpp readout

[ville.syrjala]

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

On CTG+ read out the pipe bpp setting from hardware and fill it into
pipe config. Also check it appropriately.

v2: Don't do the pipe_bpp extraction inside the PCH only code block on
    ILK+.
    Avoid the PIPECONF read as we already have read it for the
    PIPECONF_EANBLE check.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 17 +++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 060ea50..9305fb6 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1268,6 +1268,23 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	switch (temp & TRANS_DDI_BPC_MASK) {
+	case TRANS_DDI_BPC_6:
+		pipe_config->pipe_bpp = 18;
+		break;
+	case TRANS_DDI_BPC_8:
+		pipe_config->pipe_bpp = 24;
+		break;
+	case TRANS_DDI_BPC_10:
+		pipe_config->pipe_bpp = 30;
+		break;
+	case TRANS_DDI_BPC_12:
+		pipe_config->pipe_bpp = 36;
+		break;
+	default:
+		break;
+	}
 }
 
 static void intel_ddi_destroy(struct drm_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2aac205..35ad910 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4999,6 +4999,22 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
+	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
+		switch (tmp & PIPECONF_BPC_MASK) {
+		case PIPECONF_6BPC:
+			pipe_config->pipe_bpp = 18;
+			break;
+		case PIPECONF_8BPC:
+			pipe_config->pipe_bpp = 24;
+			break;
+		case PIPECONF_10BPC:
+			pipe_config->pipe_bpp = 30;
+			break;
+		default:
+			break;
+		}
+	}
+
 	intel_get_pipe_timings(crtc, pipe_config);
 
 	i9xx_get_pfit_config(crtc, pipe_config);
@@ -5899,6 +5915,23 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
+	switch (tmp & PIPECONF_BPC_MASK) {
+	case PIPECONF_6BPC:
+		pipe_config->pipe_bpp = 18;
+		break;
+	case PIPECONF_8BPC:
+		pipe_config->pipe_bpp = 24;
+		break;
+	case PIPECONF_10BPC:
+		pipe_config->pipe_bpp = 30;
+		break;
+	case PIPECONF_12BPC:
+		pipe_config->pipe_bpp = 36;
+		break;
+	default:
+		break;
+	}
+
 	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
 		struct intel_shared_dpll *pll;
 
@@ -8630,6 +8663,9 @@ intel_pipe_config_compare(struct drm_device *dev,
 	PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
 	PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
 
+	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
+		PIPE_CONF_CHECK_I(pipe_bpp);
+
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
 #undef PIPE_CONF_CHECK_FLAGS
-- 
1.8.1.5

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

Re: [PATCH v2 03/11] drm/i915: Add support for pipe_bpp readout

[Jani Nikula]

On Fri, 06 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On CTG+ read out the pipe bpp setting from hardware and fill it into
> pipe config. Also check it appropriately.
>
> v2: Don't do the pipe_bpp extraction inside the PCH only code block on
>     ILK+.
>     Avoid the PIPECONF read as we already have read it for the
>     PIPECONF_EANBLE check.

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c     | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 060ea50..9305fb6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1268,6 +1268,23 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	switch (temp & TRANS_DDI_BPC_MASK) {
> +	case TRANS_DDI_BPC_6:
> +		pipe_config->pipe_bpp = 18;
> +		break;
> +	case TRANS_DDI_BPC_8:
> +		pipe_config->pipe_bpp = 24;
> +		break;
> +	case TRANS_DDI_BPC_10:
> +		pipe_config->pipe_bpp = 30;
> +		break;
> +	case TRANS_DDI_BPC_12:
> +		pipe_config->pipe_bpp = 36;
> +		break;
> +	default:
> +		break;
> +	}
>  }
>  
>  static void intel_ddi_destroy(struct drm_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2aac205..35ad910 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4999,6 +4999,22 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  	if (!(tmp & PIPECONF_ENABLE))
>  		return false;
>  
> +	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> +		switch (tmp & PIPECONF_BPC_MASK) {
> +		case PIPECONF_6BPC:
> +			pipe_config->pipe_bpp = 18;
> +			break;
> +		case PIPECONF_8BPC:
> +			pipe_config->pipe_bpp = 24;
> +			break;
> +		case PIPECONF_10BPC:
> +			pipe_config->pipe_bpp = 30;
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
>  	intel_get_pipe_timings(crtc, pipe_config);
>  
>  	i9xx_get_pfit_config(crtc, pipe_config);
> @@ -5899,6 +5915,23 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  	if (!(tmp & PIPECONF_ENABLE))
>  		return false;
>  
> +	switch (tmp & PIPECONF_BPC_MASK) {
> +	case PIPECONF_6BPC:
> +		pipe_config->pipe_bpp = 18;
> +		break;
> +	case PIPECONF_8BPC:
> +		pipe_config->pipe_bpp = 24;
> +		break;
> +	case PIPECONF_10BPC:
> +		pipe_config->pipe_bpp = 30;
> +		break;
> +	case PIPECONF_12BPC:
> +		pipe_config->pipe_bpp = 36;
> +		break;
> +	default:
> +		break;
> +	}
> +
>  	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
>  		struct intel_shared_dpll *pll;
>  
> @@ -8630,6 +8663,9 @@ intel_pipe_config_compare(struct drm_device *dev,
>  	PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
>  
> +	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
> +		PIPE_CONF_CHECK_I(pipe_bpp);
> +
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
>  #undef PIPE_CONF_CHECK_FLAGS
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 04/11] drm/i915: Add state readout and checking for has_dp_encoder and dp_m_n

[ville.syrjala]

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

Add functions to read out the CPU and PCH transcoder M/N values,
and use them to fill out the pipe config dp_m_n information. And
while at it populate has_dp_encoder too.

Also refactor ironlake_get_fdi_m_n_config() to simply call the new
intel_cpu_transcoder_get_m_n() function.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 76 +++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_dp.c      |  4 ++
 drivers/gpu/drm/i915/intel_drv.h     |  2 +
 3 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 35ad910..3bfde06 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5862,20 +5862,64 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	return ret;
 }
 
-static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
-					struct intel_crtc_config *pipe_config)
+static void intel_pch_transcoder_get_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe = crtc->pipe;
+
+	m_n->link_m = I915_READ(PCH_TRANS_LINK_M1(pipe));
+	m_n->link_n = I915_READ(PCH_TRANS_LINK_N1(pipe));
+	m_n->gmch_m = I915_READ(PCH_TRANS_DATA_M1(pipe))
+		& ~TU_SIZE_MASK;
+	m_n->gmch_n = I915_READ(PCH_TRANS_DATA_N1(pipe));
+	m_n->tu = ((I915_READ(PCH_TRANS_DATA_M1(pipe))
+		    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+}
+
+static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
+					 enum transcoder transcoder,
+					 struct intel_link_m_n *m_n)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum transcoder transcoder = pipe_config->cpu_transcoder;
+	enum pipe pipe = crtc->pipe;
+
+	if (INTEL_INFO(dev)->gen >= 5) {
+		m_n->link_m = I915_READ(PIPE_LINK_M1(transcoder));
+		m_n->link_n = I915_READ(PIPE_LINK_N1(transcoder));
+		m_n->gmch_m = I915_READ(PIPE_DATA_M1(transcoder))
+			& ~TU_SIZE_MASK;
+		m_n->gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
+		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
+			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+	} else {
+		m_n->link_m = I915_READ(PIPE_LINK_M_G4X(pipe));
+		m_n->link_n = I915_READ(PIPE_LINK_N_G4X(pipe));
+		m_n->gmch_m = I915_READ(PIPE_DATA_M_G4X(pipe))
+			& ~TU_SIZE_MASK;
+		m_n->gmch_n = I915_READ(PIPE_DATA_N_G4X(pipe));
+		m_n->tu = ((I915_READ(PIPE_DATA_M_G4X(pipe))
+			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+	}
+}
+
+void intel_dp_get_m_n(struct intel_crtc *crtc,
+		      struct intel_crtc_config *pipe_config)
+{
+	if (crtc->config.has_pch_encoder)
+		intel_pch_transcoder_get_m_n(crtc, &pipe_config->dp_m_n);
+	else
+		intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
+					     &pipe_config->dp_m_n);
+}
 
-	pipe_config->fdi_m_n.link_m = I915_READ(PIPE_LINK_M1(transcoder));
-	pipe_config->fdi_m_n.link_n = I915_READ(PIPE_LINK_N1(transcoder));
-	pipe_config->fdi_m_n.gmch_m = I915_READ(PIPE_DATA_M1(transcoder))
-					& ~TU_SIZE_MASK;
-	pipe_config->fdi_m_n.gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
-	pipe_config->fdi_m_n.tu = ((I915_READ(PIPE_DATA_M1(transcoder))
-				   & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
+					struct intel_crtc_config *pipe_config)
+{
+	intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
+				     &pipe_config->fdi_m_n);
 }
 
 static void ironlake_get_pfit_config(struct intel_crtc *crtc,
@@ -8245,6 +8289,11 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->fdi_m_n.gmch_m, pipe_config->fdi_m_n.gmch_n,
 		      pipe_config->fdi_m_n.link_m, pipe_config->fdi_m_n.link_n,
 		      pipe_config->fdi_m_n.tu);
+	DRM_DEBUG_KMS("dp: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
+		      pipe_config->has_dp_encoder,
+		      pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n,
+		      pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n,
+		      pipe_config->dp_m_n.tu);
 	DRM_DEBUG_KMS("requested mode:\n");
 	drm_mode_debug_printmodeline(&pipe_config->requested_mode);
 	DRM_DEBUG_KMS("adjusted mode:\n");
@@ -8614,6 +8663,13 @@ intel_pipe_config_compare(struct drm_device *dev,
 	PIPE_CONF_CHECK_I(fdi_m_n.link_n);
 	PIPE_CONF_CHECK_I(fdi_m_n.tu);
 
+	PIPE_CONF_CHECK_I(has_dp_encoder);
+	PIPE_CONF_CHECK_I(dp_m_n.gmch_m);
+	PIPE_CONF_CHECK_I(dp_m_n.gmch_n);
+	PIPE_CONF_CHECK_I(dp_m_n.link_m);
+	PIPE_CONF_CHECK_I(dp_m_n.link_n);
+	PIPE_CONF_CHECK_I(dp_m_n.tu);
+
 	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
 	PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
 	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c192dbb..be0449b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1399,6 +1399,10 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 
 	pipe_config->adjusted_mode.flags |= flags;
 
+	pipe_config->has_dp_encoder = true;
+
+	intel_dp_get_m_n(crtc, pipe_config);
+
 	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
 		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
 			pipe_config->port_clock = 162000;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dbf04be..a4e8689 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -801,5 +801,7 @@ extern void hsw_pc8_disable_interrupts(struct drm_device *dev);
 extern void hsw_pc8_restore_interrupts(struct drm_device *dev);
 extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
+extern void intel_dp_get_m_n(struct intel_crtc *crtc,
+			     struct intel_crtc_config *pipe_config);
 
 #endif /* __INTEL_DRV_H__ */
-- 
1.8.1.5

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

[PATCH v2] drm/i915: Add state readout and checking for has_dp_encoder and dp_m_n

[ville.syrjala]

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

Add functions to read out the CPU and PCH transcoder M/N values,
and use them to fill out the pipe config dp_m_n information. And
while at it populate has_dp_encoder too.

Also refactor ironlake_get_fdi_m_n_config() to simply call the new
intel_cpu_transcoder_get_m_n() function.

v2: Remember the DDI

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 14 +++++++
 drivers/gpu/drm/i915/intel_display.c | 76 +++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_dp.c      |  4 ++
 drivers/gpu/drm/i915/intel_drv.h     |  2 +
 4 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1803cff..b47dc4e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1285,6 +1285,20 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
 	default:
 		break;
 	}
+
+	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
+	case TRANS_DDI_MODE_SELECT_HDMI:
+	case TRANS_DDI_MODE_SELECT_DVI:
+	case TRANS_DDI_MODE_SELECT_FDI:
+		break;
+	case TRANS_DDI_MODE_SELECT_DP_SST:
+	case TRANS_DDI_MODE_SELECT_DP_MST:
+		pipe_config->has_dp_encoder = true;
+		intel_dp_get_m_n(intel_crtc, pipe_config);
+		break;
+	default:
+		break;
+	}
 }
 
 static void intel_ddi_destroy(struct drm_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 935f913..c9d3f54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5863,20 +5863,64 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	return ret;
 }
 
-static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
-					struct intel_crtc_config *pipe_config)
+static void intel_pch_transcoder_get_m_n(struct intel_crtc *crtc,
+					 struct intel_link_m_n *m_n)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe = crtc->pipe;
+
+	m_n->link_m = I915_READ(PCH_TRANS_LINK_M1(pipe));
+	m_n->link_n = I915_READ(PCH_TRANS_LINK_N1(pipe));
+	m_n->gmch_m = I915_READ(PCH_TRANS_DATA_M1(pipe))
+		& ~TU_SIZE_MASK;
+	m_n->gmch_n = I915_READ(PCH_TRANS_DATA_N1(pipe));
+	m_n->tu = ((I915_READ(PCH_TRANS_DATA_M1(pipe))
+		    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+}
+
+static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
+					 enum transcoder transcoder,
+					 struct intel_link_m_n *m_n)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum transcoder transcoder = pipe_config->cpu_transcoder;
+	enum pipe pipe = crtc->pipe;
+
+	if (INTEL_INFO(dev)->gen >= 5) {
+		m_n->link_m = I915_READ(PIPE_LINK_M1(transcoder));
+		m_n->link_n = I915_READ(PIPE_LINK_N1(transcoder));
+		m_n->gmch_m = I915_READ(PIPE_DATA_M1(transcoder))
+			& ~TU_SIZE_MASK;
+		m_n->gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
+		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
+			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+	} else {
+		m_n->link_m = I915_READ(PIPE_LINK_M_G4X(pipe));
+		m_n->link_n = I915_READ(PIPE_LINK_N_G4X(pipe));
+		m_n->gmch_m = I915_READ(PIPE_DATA_M_G4X(pipe))
+			& ~TU_SIZE_MASK;
+		m_n->gmch_n = I915_READ(PIPE_DATA_N_G4X(pipe));
+		m_n->tu = ((I915_READ(PIPE_DATA_M_G4X(pipe))
+			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+	}
+}
+
+void intel_dp_get_m_n(struct intel_crtc *crtc,
+		      struct intel_crtc_config *pipe_config)
+{
+	if (crtc->config.has_pch_encoder)
+		intel_pch_transcoder_get_m_n(crtc, &pipe_config->dp_m_n);
+	else
+		intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
+					     &pipe_config->dp_m_n);
+}
 
-	pipe_config->fdi_m_n.link_m = I915_READ(PIPE_LINK_M1(transcoder));
-	pipe_config->fdi_m_n.link_n = I915_READ(PIPE_LINK_N1(transcoder));
-	pipe_config->fdi_m_n.gmch_m = I915_READ(PIPE_DATA_M1(transcoder))
-					& ~TU_SIZE_MASK;
-	pipe_config->fdi_m_n.gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
-	pipe_config->fdi_m_n.tu = ((I915_READ(PIPE_DATA_M1(transcoder))
-				   & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
+					struct intel_crtc_config *pipe_config)
+{
+	intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
+				     &pipe_config->fdi_m_n);
 }
 
 static void ironlake_get_pfit_config(struct intel_crtc *crtc,
@@ -8247,6 +8291,11 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->fdi_m_n.gmch_m, pipe_config->fdi_m_n.gmch_n,
 		      pipe_config->fdi_m_n.link_m, pipe_config->fdi_m_n.link_n,
 		      pipe_config->fdi_m_n.tu);
+	DRM_DEBUG_KMS("dp: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
+		      pipe_config->has_dp_encoder,
+		      pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n,
+		      pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n,
+		      pipe_config->dp_m_n.tu);
 	DRM_DEBUG_KMS("requested mode:\n");
 	drm_mode_debug_printmodeline(&pipe_config->requested_mode);
 	DRM_DEBUG_KMS("adjusted mode:\n");
@@ -8617,6 +8666,13 @@ intel_pipe_config_compare(struct drm_device *dev,
 	PIPE_CONF_CHECK_I(fdi_m_n.link_n);
 	PIPE_CONF_CHECK_I(fdi_m_n.tu);
 
+	PIPE_CONF_CHECK_I(has_dp_encoder);
+	PIPE_CONF_CHECK_I(dp_m_n.gmch_m);
+	PIPE_CONF_CHECK_I(dp_m_n.gmch_n);
+	PIPE_CONF_CHECK_I(dp_m_n.link_m);
+	PIPE_CONF_CHECK_I(dp_m_n.link_n);
+	PIPE_CONF_CHECK_I(dp_m_n.tu);
+
 	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
 	PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
 	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8c70a83..9ba697c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1444,6 +1444,10 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 
 	pipe_config->adjusted_mode.flags |= flags;
 
+	pipe_config->has_dp_encoder = true;
+
+	intel_dp_get_m_n(crtc, pipe_config);
+
 	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
 		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
 			pipe_config->port_clock = 162000;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a8ce0b7..a509dd6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -803,5 +803,7 @@ extern void hsw_pc8_restore_interrupts(struct drm_device *dev);
 extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 extern void i915_disable_vga_mem(struct drm_device *dev);
+extern void intel_dp_get_m_n(struct intel_crtc *crtc,
+			     struct intel_crtc_config *pipe_config);
 
 #endif /* __INTEL_DRV_H__ */
-- 
1.8.1.5

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

Re: [PATCH v2] drm/i915: Add state readout and checking for has_dp_encoder and dp_m_n

[Jani Nikula]

On Tue, 10 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add functions to read out the CPU and PCH transcoder M/N values,
> and use them to fill out the pipe config dp_m_n information. And
> while at it populate has_dp_encoder too.
>
> Also refactor ironlake_get_fdi_m_n_config() to simply call the new
> intel_cpu_transcoder_get_m_n() function.
>
> v2: Remember the DDI

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c     | 14 +++++++
>  drivers/gpu/drm/i915/intel_display.c | 76 +++++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_dp.c      |  4 ++
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>  4 files changed, 86 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 1803cff..b47dc4e 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1285,6 +1285,20 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
>  	default:
>  		break;
>  	}
> +
> +	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> +	case TRANS_DDI_MODE_SELECT_HDMI:
> +	case TRANS_DDI_MODE_SELECT_DVI:
> +	case TRANS_DDI_MODE_SELECT_FDI:
> +		break;
> +	case TRANS_DDI_MODE_SELECT_DP_SST:
> +	case TRANS_DDI_MODE_SELECT_DP_MST:
> +		pipe_config->has_dp_encoder = true;
> +		intel_dp_get_m_n(intel_crtc, pipe_config);
> +		break;
> +	default:
> +		break;
> +	}
>  }
>  
>  static void intel_ddi_destroy(struct drm_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 935f913..c9d3f54 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5863,20 +5863,64 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	return ret;
>  }
>  
> -static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
> -					struct intel_crtc_config *pipe_config)
> +static void intel_pch_transcoder_get_m_n(struct intel_crtc *crtc,
> +					 struct intel_link_m_n *m_n)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = crtc->pipe;
> +
> +	m_n->link_m = I915_READ(PCH_TRANS_LINK_M1(pipe));
> +	m_n->link_n = I915_READ(PCH_TRANS_LINK_N1(pipe));
> +	m_n->gmch_m = I915_READ(PCH_TRANS_DATA_M1(pipe))
> +		& ~TU_SIZE_MASK;
> +	m_n->gmch_n = I915_READ(PCH_TRANS_DATA_N1(pipe));
> +	m_n->tu = ((I915_READ(PCH_TRANS_DATA_M1(pipe))
> +		    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> +}
> +
> +static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
> +					 enum transcoder transcoder,
> +					 struct intel_link_m_n *m_n)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum transcoder transcoder = pipe_config->cpu_transcoder;
> +	enum pipe pipe = crtc->pipe;
> +
> +	if (INTEL_INFO(dev)->gen >= 5) {
> +		m_n->link_m = I915_READ(PIPE_LINK_M1(transcoder));
> +		m_n->link_n = I915_READ(PIPE_LINK_N1(transcoder));
> +		m_n->gmch_m = I915_READ(PIPE_DATA_M1(transcoder))
> +			& ~TU_SIZE_MASK;
> +		m_n->gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
> +		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
> +			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> +	} else {
> +		m_n->link_m = I915_READ(PIPE_LINK_M_G4X(pipe));
> +		m_n->link_n = I915_READ(PIPE_LINK_N_G4X(pipe));
> +		m_n->gmch_m = I915_READ(PIPE_DATA_M_G4X(pipe))
> +			& ~TU_SIZE_MASK;
> +		m_n->gmch_n = I915_READ(PIPE_DATA_N_G4X(pipe));
> +		m_n->tu = ((I915_READ(PIPE_DATA_M_G4X(pipe))
> +			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> +	}
> +}
> +
> +void intel_dp_get_m_n(struct intel_crtc *crtc,
> +		      struct intel_crtc_config *pipe_config)
> +{
> +	if (crtc->config.has_pch_encoder)
> +		intel_pch_transcoder_get_m_n(crtc, &pipe_config->dp_m_n);
> +	else
> +		intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
> +					     &pipe_config->dp_m_n);
> +}
>  
> -	pipe_config->fdi_m_n.link_m = I915_READ(PIPE_LINK_M1(transcoder));
> -	pipe_config->fdi_m_n.link_n = I915_READ(PIPE_LINK_N1(transcoder));
> -	pipe_config->fdi_m_n.gmch_m = I915_READ(PIPE_DATA_M1(transcoder))
> -					& ~TU_SIZE_MASK;
> -	pipe_config->fdi_m_n.gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
> -	pipe_config->fdi_m_n.tu = ((I915_READ(PIPE_DATA_M1(transcoder))
> -				   & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> +static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
> +					struct intel_crtc_config *pipe_config)
> +{
> +	intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
> +				     &pipe_config->fdi_m_n);
>  }
>  
>  static void ironlake_get_pfit_config(struct intel_crtc *crtc,
> @@ -8247,6 +8291,11 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  		      pipe_config->fdi_m_n.gmch_m, pipe_config->fdi_m_n.gmch_n,
>  		      pipe_config->fdi_m_n.link_m, pipe_config->fdi_m_n.link_n,
>  		      pipe_config->fdi_m_n.tu);
> +	DRM_DEBUG_KMS("dp: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
> +		      pipe_config->has_dp_encoder,
> +		      pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n,
> +		      pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n,
> +		      pipe_config->dp_m_n.tu);
>  	DRM_DEBUG_KMS("requested mode:\n");
>  	drm_mode_debug_printmodeline(&pipe_config->requested_mode);
>  	DRM_DEBUG_KMS("adjusted mode:\n");
> @@ -8617,6 +8666,13 @@ intel_pipe_config_compare(struct drm_device *dev,
>  	PIPE_CONF_CHECK_I(fdi_m_n.link_n);
>  	PIPE_CONF_CHECK_I(fdi_m_n.tu);
>  
> +	PIPE_CONF_CHECK_I(has_dp_encoder);
> +	PIPE_CONF_CHECK_I(dp_m_n.gmch_m);
> +	PIPE_CONF_CHECK_I(dp_m_n.gmch_n);
> +	PIPE_CONF_CHECK_I(dp_m_n.link_m);
> +	PIPE_CONF_CHECK_I(dp_m_n.link_n);
> +	PIPE_CONF_CHECK_I(dp_m_n.tu);
> +
>  	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
>  	PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
>  	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8c70a83..9ba697c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1444,6 +1444,10 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  
>  	pipe_config->adjusted_mode.flags |= flags;
>  
> +	pipe_config->has_dp_encoder = true;
> +
> +	intel_dp_get_m_n(crtc, pipe_config);
> +
>  	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
>  		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
>  			pipe_config->port_clock = 162000;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a8ce0b7..a509dd6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -803,5 +803,7 @@ extern void hsw_pc8_restore_interrupts(struct drm_device *dev);
>  extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>  extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>  extern void i915_disable_vga_mem(struct drm_device *dev);
> +extern void intel_dp_get_m_n(struct intel_crtc *crtc,
> +			     struct intel_crtc_config *pipe_config);
>  
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 05/11] drm/i915: Make intel_fuzzy_clock_check() take in arbitrary clocks

[ville.syrjala]

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

We want to do fuzzy clock checks for other things besides
adjusted_mode.clock, so just pass two two clocks to compare
to intel_fuzzy_clock_check().

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3bfde06..b3049a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8590,13 +8590,9 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
 
 }
 
-static bool intel_fuzzy_clock_check(struct intel_crtc_config *cur,
-				    struct intel_crtc_config *new)
+static bool intel_fuzzy_clock_check(int clock1, int clock2)
 {
-	int clock1, clock2, diff;
-
-	clock1 = cur->adjusted_mode.clock;
-	clock2 = new->adjusted_mode.clock;
+	int diff;
 
 	if (clock1 == clock2)
 		return true;
@@ -8728,7 +8724,8 @@ intel_pipe_config_compare(struct drm_device *dev,
 #undef PIPE_CONF_QUIRK
 
 	if (!IS_HASWELL(dev)) {
-		if (!intel_fuzzy_clock_check(current_config, pipe_config)) {
+		if (!intel_fuzzy_clock_check(current_config->adjusted_mode.clock,
+					     pipe_config->adjusted_mode.clock)) {
 			DRM_ERROR("mismatch in clock (expected %d, found %d)\n",
 				  current_config->adjusted_mode.clock,
 				  pipe_config->adjusted_mode.clock);
-- 
1.8.1.5

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

Re: [PATCH 05/11] drm/i915: Make intel_fuzzy_clock_check() take in arbitrary clocks

[Daniel Vetter]

On Fri, Sep 06, 2013 at 11:29:02PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We want to do fuzzy clock checks for other things besides
> adjusted_mode.clock, so just pass two two clocks to compare
> to intel_fuzzy_clock_check().
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, I've merged up to this one, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 06/11] drm/i915: Add intel_dotclock_calculate()

[ville.syrjala]

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

Extract the code to calculate the dotclock from the link clock and M/N
values into a new function from ironlake_crtc_clock_get().

The new function can be used to calculate the dotclock for both FDI and
DP cases.

Also simplify the code a bit along the way.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b3049a6..c393c8e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7410,16 +7410,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 	pipe_config->adjusted_mode.clock = clock.dot;
 }
 
-static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
-				    struct intel_crtc_config *pipe_config)
+int intel_dotclock_calculate(int link_freq,
+			     const struct intel_link_m_n *m_n)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	int link_freq;
-	u64 clock;
-	u32 link_m, link_n;
-
 	/*
 	 * The calculation for the data clock is:
 	 * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp
@@ -7430,6 +7423,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	 * link_clock = (m * link_clock) / n
 	 */
 
+	if (!m_n->link_n)
+		return 0;
+
+	return div_u64((u64)m_n->link_m * link_freq, m_n->link_n);
+}
+
+static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
+				    struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	int link_freq;
+
 	/*
 	 * We need to get the FDI or DP link clock here to derive
 	 * the M/N dividers.
@@ -7438,21 +7443,10 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	 * For DP, it's either 1.62GHz or 2.7GHz.
 	 * We do our calculations in 10*MHz since we don't need much precison.
 	 */
-	if (pipe_config->has_pch_encoder)
-		link_freq = intel_fdi_link_freq(dev) * 10000;
-	else
-		link_freq = pipe_config->port_clock;
-
-	link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder));
-	link_n = I915_READ(PIPE_LINK_N1(cpu_transcoder));
-
-	if (!link_m || !link_n)
-		return;
-
-	clock = ((u64)link_m * (u64)link_freq);
-	do_div(clock, link_n);
+	link_freq = intel_fdi_link_freq(dev) * 10000;
 
-	pipe_config->adjusted_mode.clock = clock;
+	pipe_config->adjusted_mode.clock =
+		intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
 }
 
 /** Returns the currently programmed mode of the given pipe. */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4e8689..4cf8898 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -803,5 +803,7 @@ extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 extern void intel_dp_get_m_n(struct intel_crtc *crtc,
 			     struct intel_crtc_config *pipe_config);
+extern int intel_dotclock_calculate(int link_freq,
+				    const struct intel_link_m_n *m_n);
 
 #endif /* __INTEL_DRV_H__ */
-- 
1.8.1.5

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

Re: [PATCH 06/11] drm/i915: Add intel_dotclock_calculate()

[Jani Nikula]

On Fri, 06 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extract the code to calculate the dotclock from the link clock and M/N
> values into a new function from ironlake_crtc_clock_get().
>
> The new function can be used to calculate the dotclock for both FDI and
> DP cases.
>
> Also simplify the code a bit along the way.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++++---------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  2 files changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b3049a6..c393c8e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7410,16 +7410,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  	pipe_config->adjusted_mode.clock = clock.dot;
>  }
>  
> -static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> -				    struct intel_crtc_config *pipe_config)
> +int intel_dotclock_calculate(int link_freq,
> +			     const struct intel_link_m_n *m_n)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> -	int link_freq;
> -	u64 clock;
> -	u32 link_m, link_n;
> -
>  	/*
>  	 * The calculation for the data clock is:
>  	 * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp
> @@ -7430,6 +7423,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	 * link_clock = (m * link_clock) / n
>  	 */
>  
> +	if (!m_n->link_n)
> +		return 0;
> +
> +	return div_u64((u64)m_n->link_m * link_freq, m_n->link_n);
> +}
> +
> +static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> +				    struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	int link_freq;
> +
>  	/*
>  	 * We need to get the FDI or DP link clock here to derive
>  	 * the M/N dividers.
> @@ -7438,21 +7443,10 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	 * For DP, it's either 1.62GHz or 2.7GHz.
>  	 * We do our calculations in 10*MHz since we don't need much precison.
>  	 */
> -	if (pipe_config->has_pch_encoder)
> -		link_freq = intel_fdi_link_freq(dev) * 10000;
> -	else
> -		link_freq = pipe_config->port_clock;

The new code loses this distinction. I don't know if it matters. Please
explain.

> -
> -	link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder));
> -	link_n = I915_READ(PIPE_LINK_N1(cpu_transcoder));
> -
> -	if (!link_m || !link_n)
> -		return;
> -
> -	clock = ((u64)link_m * (u64)link_freq);
> -	do_div(clock, link_n);
> +	link_freq = intel_fdi_link_freq(dev) * 10000;
>  
> -	pipe_config->adjusted_mode.clock = clock;
> +	pipe_config->adjusted_mode.clock =
> +		intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);

I'm not sure if the fear in unwarranted, but can we always be sure
fdi_m_n has been set?

Jani.


>  }
>  
>  /** Returns the currently programmed mode of the given pipe. */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a4e8689..4cf8898 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -803,5 +803,7 @@ extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>  extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>  extern void intel_dp_get_m_n(struct intel_crtc *crtc,
>  			     struct intel_crtc_config *pipe_config);
> +extern int intel_dotclock_calculate(int link_freq,
> +				    const struct intel_link_m_n *m_n);
>  
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 06/11] drm/i915: Add intel_dotclock_calculate()

[Ville Syrjälä]

On Fri, Sep 13, 2013 at 03:30:51PM +0300, Jani Nikula wrote:
> On Fri, 06 Sep 2013, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Extract the code to calculate the dotclock from the link clock and M/N
> > values into a new function from ironlake_crtc_clock_get().
> >
> > The new function can be used to calculate the dotclock for both FDI and
> > DP cases.
> >
> > Also simplify the code a bit along the way.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++++---------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
> >  2 files changed, 19 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index b3049a6..c393c8e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7410,16 +7410,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  	pipe_config->adjusted_mode.clock = clock.dot;
> >  }
> >  
> > -static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> > -				    struct intel_crtc_config *pipe_config)
> > +int intel_dotclock_calculate(int link_freq,
> > +			     const struct intel_link_m_n *m_n)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > -	int link_freq;
> > -	u64 clock;
> > -	u32 link_m, link_n;
> > -
> >  	/*
> >  	 * The calculation for the data clock is:
> >  	 * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp
> > @@ -7430,6 +7423,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> >  	 * link_clock = (m * link_clock) / n
> >  	 */
> >  
> > +	if (!m_n->link_n)
> > +		return 0;
> > +
> > +	return div_u64((u64)m_n->link_m * link_freq, m_n->link_n);
> > +}
> > +
> > +static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> > +				    struct intel_crtc_config *pipe_config)
> > +{
> > +	struct drm_device *dev = crtc->base.dev;
> > +	int link_freq;
> > +
> >  	/*
> >  	 * We need to get the FDI or DP link clock here to derive
> >  	 * the M/N dividers.
> > @@ -7438,21 +7443,10 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> >  	 * For DP, it's either 1.62GHz or 2.7GHz.
> >  	 * We do our calculations in 10*MHz since we don't need much precison.
> >  	 */
> > -	if (pipe_config->has_pch_encoder)
> > -		link_freq = intel_fdi_link_freq(dev) * 10000;
> > -	else
> > -		link_freq = pipe_config->port_clock;
> 
> The new code loses this distinction. I don't know if it matters. Please
> explain.
> 
> > -
> > -	link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder));
> > -	link_n = I915_READ(PIPE_LINK_N1(cpu_transcoder));
> > -
> > -	if (!link_m || !link_n)
> > -		return;
> > -
> > -	clock = ((u64)link_m * (u64)link_freq);
> > -	do_div(clock, link_n);
> > +	link_freq = intel_fdi_link_freq(dev) * 10000;
> >  
> > -	pipe_config->adjusted_mode.clock = clock;
> > +	pipe_config->adjusted_mode.clock =
> > +		intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
> 
> I'm not sure if the fear in unwarranted, but can we always be sure
> fdi_m_n has been set?

I seem to have jumped the gun a bit with this patch. Patch 9 will make
it so that we don't call ironlake_crtc_clock_get() except in the PCH
encoder case (could use a rename of the function at that point maybe).
Before patch 9 we still call this guy for non-PCH cases and then it
might fall apart a bit. As the dp_m_n extraction patch appears before
this patch in the series, I think I can just make a slightly fixed
version of this patch. Stay tuned for v2...

-- 
Ville Syrjälä
Intel OTC

[PATCH v2] drm/i915: Add intel_dotclock_calculate()

[ville.syrjala]

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

Extract the code to calculate the dotclock from the link clock and M/N
values into a new function from ironlake_crtc_clock_get().

The new function can be used to calculate the dotclock for both FDI and
DP cases.

Also simplify the code a bit along the way.

v2: Don't forget about non-pch encoders in ironlake_crtc_clock_get()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c0ee41c..13dea9b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7405,16 +7405,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 	pipe_config->adjusted_mode.clock = clock.dot;
 }
 
-static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
-				    struct intel_crtc_config *pipe_config)
+int intel_dotclock_calculate(int link_freq,
+			     const struct intel_link_m_n *m_n)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	int link_freq;
-	u64 clock;
-	u32 link_m, link_n;
-
 	/*
 	 * The calculation for the data clock is:
 	 * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp
@@ -7425,6 +7418,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	 * link_clock = (m * link_clock) / n
 	 */
 
+	if (!m_n->link_n)
+		return 0;
+
+	return div_u64((u64)m_n->link_m * link_freq, m_n->link_n);
+}
+
+static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
+				    struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	int link_freq;
+
 	/*
 	 * We need to get the FDI or DP link clock here to derive
 	 * the M/N dividers.
@@ -7433,21 +7438,17 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	 * For DP, it's either 1.62GHz or 2.7GHz.
 	 * We do our calculations in 10*MHz since we don't need much precison.
 	 */
-	if (pipe_config->has_pch_encoder)
+	if (pipe_config->has_pch_encoder) {
 		link_freq = intel_fdi_link_freq(dev) * 10000;
-	else
-		link_freq = pipe_config->port_clock;
 
-	link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder));
-	link_n = I915_READ(PIPE_LINK_N1(cpu_transcoder));
-
-	if (!link_m || !link_n)
-		return;
-
-	clock = ((u64)link_m * (u64)link_freq);
-	do_div(clock, link_n);
+		pipe_config->adjusted_mode.clock =
+			intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
+	} else {
+		link_freq = pipe_config->port_clock;
 
-	pipe_config->adjusted_mode.clock = clock;
+		pipe_config->adjusted_mode.clock =
+			intel_dotclock_calculate(link_freq, &pipe_config->dp_m_n);
+	}
 }
 
 /** Returns the currently programmed mode of the given pipe. */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c058f1b..6b97ac1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -805,5 +805,7 @@ extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 extern void i915_disable_vga_mem(struct drm_device *dev);
 extern void intel_dp_get_m_n(struct intel_crtc *crtc,
 			     struct intel_crtc_config *pipe_config);
+extern int intel_dotclock_calculate(int link_freq,
+				    const struct intel_link_m_n *m_n);
 
 #endif /* __INTEL_DRV_H__ */
-- 
1.8.1.5

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

Re: [PATCH v2] drm/i915: Add intel_dotclock_calculate()

[Jani Nikula]

On Fri, 13 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extract the code to calculate the dotclock from the link clock and M/N
> values into a new function from ironlake_crtc_clock_get().
>
> The new function can be used to calculate the dotclock for both FDI and
> DP cases.
>
> Also simplify the code a bit along the way.
>
> v2: Don't forget about non-pch encoders in ironlake_crtc_clock_get()
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  2 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c0ee41c..13dea9b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7405,16 +7405,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  	pipe_config->adjusted_mode.clock = clock.dot;
>  }
>  
> -static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> -				    struct intel_crtc_config *pipe_config)
> +int intel_dotclock_calculate(int link_freq,
> +			     const struct intel_link_m_n *m_n)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> -	int link_freq;
> -	u64 clock;
> -	u32 link_m, link_n;
> -
>  	/*
>  	 * The calculation for the data clock is:
>  	 * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp
> @@ -7425,6 +7418,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	 * link_clock = (m * link_clock) / n
>  	 */
>  
> +	if (!m_n->link_n)
> +		return 0;
> +
> +	return div_u64((u64)m_n->link_m * link_freq, m_n->link_n);
> +}
> +
> +static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> +				    struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	int link_freq;
> +
>  	/*
>  	 * We need to get the FDI or DP link clock here to derive
>  	 * the M/N dividers.
> @@ -7433,21 +7438,17 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	 * For DP, it's either 1.62GHz or 2.7GHz.
>  	 * We do our calculations in 10*MHz since we don't need much precison.
>  	 */
> -	if (pipe_config->has_pch_encoder)
> +	if (pipe_config->has_pch_encoder) {
>  		link_freq = intel_fdi_link_freq(dev) * 10000;
> -	else
> -		link_freq = pipe_config->port_clock;
>  
> -	link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder));
> -	link_n = I915_READ(PIPE_LINK_N1(cpu_transcoder));
> -
> -	if (!link_m || !link_n)
> -		return;
> -
> -	clock = ((u64)link_m * (u64)link_freq);
> -	do_div(clock, link_n);
> +		pipe_config->adjusted_mode.clock =
> +			intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
> +	} else {
> +		link_freq = pipe_config->port_clock;
>  
> -	pipe_config->adjusted_mode.clock = clock;
> +		pipe_config->adjusted_mode.clock =
> +			intel_dotclock_calculate(link_freq, &pipe_config->dp_m_n);
> +	}
>  }
>  
>  /** Returns the currently programmed mode of the given pipe. */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c058f1b..6b97ac1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -805,5 +805,7 @@ extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>  extern void i915_disable_vga_mem(struct drm_device *dev);
>  extern void intel_dp_get_m_n(struct intel_crtc *crtc,
>  			     struct intel_crtc_config *pipe_config);
> +extern int intel_dotclock_calculate(int link_freq,
> +				    const struct intel_link_m_n *m_n);
>  
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 1.8.1.5
>

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

Re: [PATCH v2] drm/i915: Add intel_dotclock_calculate()

[Daniel Vetter]

On Mon, Sep 16, 2013 at 02:14:47PM +0300, Jani Nikula wrote:
> On Fri, 13 Sep 2013, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Extract the code to calculate the dotclock from the link clock and M/N
> > values into a new function from ironlake_crtc_clock_get().
> >
> > The new function can be used to calculate the dotclock for both FDI and
> > DP cases.
> >
> > Also simplify the code a bit along the way.
> >
> > v2: Don't forget about non-pch encoders in ironlake_crtc_clock_get()
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
> >  2 files changed, 24 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c0ee41c..13dea9b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7405,16 +7405,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  	pipe_config->adjusted_mode.clock = clock.dot;
> >  }
> >  
> > -static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> > -				    struct intel_crtc_config *pipe_config)
> > +int intel_dotclock_calculate(int link_freq,
> > +			     const struct intel_link_m_n *m_n)

intel_dotclock_calculate is an awfully generic name for something which
computes the dotclock for an fdi/dp link ... Maybe intel_dotclock_from_m_n
instead?

Patch merged since I don't want to block this any longer (and maybe it
makes more sense in the end, haven't checked).

Cheers, Daniel

> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > -	int link_freq;
> > -	u64 clock;
> > -	u32 link_m, link_n;
> > -
> >  	/*
> >  	 * The calculation for the data clock is:
> >  	 * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp
> > @@ -7425,6 +7418,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> >  	 * link_clock = (m * link_clock) / n
> >  	 */
> >  
> > +	if (!m_n->link_n)
> > +		return 0;
> > +
> > +	return div_u64((u64)m_n->link_m * link_freq, m_n->link_n);
> > +}
> > +
> > +static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> > +				    struct intel_crtc_config *pipe_config)
> > +{
> > +	struct drm_device *dev = crtc->base.dev;
> > +	int link_freq;
> > +
> >  	/*
> >  	 * We need to get the FDI or DP link clock here to derive
> >  	 * the M/N dividers.
> > @@ -7433,21 +7438,17 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> >  	 * For DP, it's either 1.62GHz or 2.7GHz.
> >  	 * We do our calculations in 10*MHz since we don't need much precison.
> >  	 */
> > -	if (pipe_config->has_pch_encoder)
> > +	if (pipe_config->has_pch_encoder) {
> >  		link_freq = intel_fdi_link_freq(dev) * 10000;
> > -	else
> > -		link_freq = pipe_config->port_clock;
> >  
> > -	link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder));
> > -	link_n = I915_READ(PIPE_LINK_N1(cpu_transcoder));
> > -
> > -	if (!link_m || !link_n)
> > -		return;
> > -
> > -	clock = ((u64)link_m * (u64)link_freq);
> > -	do_div(clock, link_n);
> > +		pipe_config->adjusted_mode.clock =
> > +			intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
> > +	} else {
> > +		link_freq = pipe_config->port_clock;
> >  
> > -	pipe_config->adjusted_mode.clock = clock;
> > +		pipe_config->adjusted_mode.clock =
> > +			intel_dotclock_calculate(link_freq, &pipe_config->dp_m_n);
> > +	}
> >  }
> >  
> >  /** Returns the currently programmed mode of the given pipe. */
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index c058f1b..6b97ac1 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -805,5 +805,7 @@ extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
> >  extern void i915_disable_vga_mem(struct drm_device *dev);
> >  extern void intel_dp_get_m_n(struct intel_crtc *crtc,
> >  			     struct intel_crtc_config *pipe_config);
> > +extern int intel_dotclock_calculate(int link_freq,
> > +				    const struct intel_link_m_n *m_n);
> >  
> >  #endif /* __INTEL_DRV_H__ */
> > -- 
> > 1.8.1.5
> >
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v2] drm/i915: Add intel_dotclock_calculate()

[Ville Syrjälä]

On Mon, Sep 16, 2013 at 10:41:38PM +0200, Daniel Vetter wrote:
> On Mon, Sep 16, 2013 at 02:14:47PM +0300, Jani Nikula wrote:
> > On Fri, 13 Sep 2013, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Extract the code to calculate the dotclock from the link clock and M/N
> > > values into a new function from ironlake_crtc_clock_get().
> > >
> > > The new function can be used to calculate the dotclock for both FDI and
> > > DP cases.
> > >
> > > Also simplify the code a bit along the way.
> > >
> > > v2: Don't forget about non-pch encoders in ironlake_crtc_clock_get()
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> > 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++------------------
> > >  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
> > >  2 files changed, 24 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index c0ee41c..13dea9b 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -7405,16 +7405,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> > >  	pipe_config->adjusted_mode.clock = clock.dot;
> > >  }
> > >  
> > > -static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> > > -				    struct intel_crtc_config *pipe_config)
> > > +int intel_dotclock_calculate(int link_freq,
> > > +			     const struct intel_link_m_n *m_n)
> 
> intel_dotclock_calculate is an awfully generic name for something which
> computes the dotclock for an fdi/dp link ... Maybe intel_dotclock_from_m_n
> instead?
> 
> Patch merged since I don't want to block this any longer (and maybe it
> makes more sense in the end, haven't checked).

Probably doesn't. It should be obvious by now that I suck at naming.

-- 
Ville Syrjälä
Intel OTC

[PATCH 07/11] drm/i915: Make i9xx_crtc_clock_get() use dpll_hw_state

[ville.syrjala]

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

We already extract the DPLL state to pipe_config, so let's make use of
it in i9xx_crtc_clock_get() and avoid the register reads.

This will also make the function closer to being useable with PCH DPLL
since the registers for those live in a different address.

Also kill the useless adjusted_mode.clock zeroing. It's already zero at
this point.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c393c8e..754de85 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7331,14 +7331,14 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = pipe_config->cpu_transcoder;
-	u32 dpll = I915_READ(DPLL(pipe));
+	u32 dpll = pipe_config->dpll_hw_state.dpll;
 	u32 fp;
 	intel_clock_t clock;
 
 	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
-		fp = I915_READ(FP0(pipe));
+		fp = pipe_config->dpll_hw_state.fp0;
 	else
-		fp = I915_READ(FP1(pipe));
+		fp = pipe_config->dpll_hw_state.fp1;
 
 	clock.m1 = (fp & FP_M1_DIV_MASK) >> FP_M1_DIV_SHIFT;
 	if (IS_PINEVIEW(dev)) {
@@ -7369,7 +7369,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 		default:
 			DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed "
 				  "mode\n", (int)(dpll & DPLL_MODE_MASK));
-			pipe_config->adjusted_mode.clock = 0;
 			return;
 		}
 
-- 
1.8.1.5

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

Re: [PATCH 07/11] drm/i915: Make i9xx_crtc_clock_get() use dpll_hw_state

[Jani Nikula]

On Fri, 06 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We already extract the DPLL state to pipe_config, so let's make use of
> it in i9xx_crtc_clock_get() and avoid the register reads.

What about the calls through intel_dvo_init/intel_lvds_init ->
intel_crtc_mode_get -> i9xx_crtc_clock_get?

Side note, we should s/intel_crtc_mode_get/i9xx_crtc_mode_get/

Jani.



>
> This will also make the function closer to being useable with PCH DPLL
> since the registers for those live in a different address.
>
> Also kill the useless adjusted_mode.clock zeroing. It's already zero at
> this point.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c393c8e..754de85 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7331,14 +7331,14 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int pipe = pipe_config->cpu_transcoder;
> -	u32 dpll = I915_READ(DPLL(pipe));
> +	u32 dpll = pipe_config->dpll_hw_state.dpll;
>  	u32 fp;
>  	intel_clock_t clock;
>  
>  	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
> -		fp = I915_READ(FP0(pipe));
> +		fp = pipe_config->dpll_hw_state.fp0;
>  	else
> -		fp = I915_READ(FP1(pipe));
> +		fp = pipe_config->dpll_hw_state.fp1;
>  
>  	clock.m1 = (fp & FP_M1_DIV_MASK) >> FP_M1_DIV_SHIFT;
>  	if (IS_PINEVIEW(dev)) {
> @@ -7369,7 +7369,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  		default:
>  			DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed "
>  				  "mode\n", (int)(dpll & DPLL_MODE_MASK));
> -			pipe_config->adjusted_mode.clock = 0;
>  			return;
>  		}
>  
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 07/11] drm/i915: Make i9xx_crtc_clock_get() use dpll_hw_state

[Ville Syrjälä]

On Fri, Sep 13, 2013 at 03:40:55PM +0300, Jani Nikula wrote:
> On Fri, 06 Sep 2013, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We already extract the DPLL state to pipe_config, so let's make use of
> > it in i9xx_crtc_clock_get() and avoid the register reads.
> 
> What about the calls through intel_dvo_init/intel_lvds_init ->
> intel_crtc_mode_get -> i9xx_crtc_clock_get?

Crap. Who put that there damnit! I guess I need to shovel the PLL
register reads into intel_crtc_mode_get() then.

> Side note, we should s/intel_crtc_mode_get/i9xx_crtc_mode_get/

Dunno.

> 
> Jani.
> 
> 
> 
> >
> > This will also make the function closer to being useable with PCH DPLL
> > since the registers for those live in a different address.
> >
> > Also kill the useless adjusted_mode.clock zeroing. It's already zero at
> > this point.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c393c8e..754de85 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7331,14 +7331,14 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int pipe = pipe_config->cpu_transcoder;
> > -	u32 dpll = I915_READ(DPLL(pipe));
> > +	u32 dpll = pipe_config->dpll_hw_state.dpll;
> >  	u32 fp;
> >  	intel_clock_t clock;
> >  
> >  	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
> > -		fp = I915_READ(FP0(pipe));
> > +		fp = pipe_config->dpll_hw_state.fp0;
> >  	else
> > -		fp = I915_READ(FP1(pipe));
> > +		fp = pipe_config->dpll_hw_state.fp1;
> >  
> >  	clock.m1 = (fp & FP_M1_DIV_MASK) >> FP_M1_DIV_SHIFT;
> >  	if (IS_PINEVIEW(dev)) {
> > @@ -7369,7 +7369,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  		default:
> >  			DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed "
> >  				  "mode\n", (int)(dpll & DPLL_MODE_MASK));
> > -			pipe_config->adjusted_mode.clock = 0;
> >  			return;
> >  		}
> >  
> > -- 
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC

[PATCH v2] drm/i915: Make i9xx_crtc_clock_get() use dpll_hw_state

[ville.syrjala]

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

We already extract the DPLL state to pipe_config, so let's make use of
it in i9xx_crtc_clock_get() and avoid the register reads.

This will also make the function closer to being useable with PCH DPLL
since the registers for those live in a different address.

Also kill the useless adjusted_mode.clock zeroing. It's already zero at
this point.

v2: Read out DPLL state in intel_crtc_mode_get()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 13dea9b..ad7f8a7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7326,14 +7326,14 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = pipe_config->cpu_transcoder;
-	u32 dpll = I915_READ(DPLL(pipe));
+	u32 dpll = pipe_config->dpll_hw_state.dpll;
 	u32 fp;
 	intel_clock_t clock;
 
 	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
-		fp = I915_READ(FP0(pipe));
+		fp = pipe_config->dpll_hw_state.fp0;
 	else
-		fp = I915_READ(FP1(pipe));
+		fp = pipe_config->dpll_hw_state.fp1;
 
 	clock.m1 = (fp & FP_M1_DIV_MASK) >> FP_M1_DIV_SHIFT;
 	if (IS_PINEVIEW(dev)) {
@@ -7364,7 +7364,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 		default:
 			DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed "
 				  "mode\n", (int)(dpll & DPLL_MODE_MASK));
-			pipe_config->adjusted_mode.clock = 0;
 			return;
 		}
 
@@ -7464,6 +7463,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 	int hsync = I915_READ(HSYNC(cpu_transcoder));
 	int vtot = I915_READ(VTOTAL(cpu_transcoder));
 	int vsync = I915_READ(VSYNC(cpu_transcoder));
+	enum pipe pipe = intel_crtc->pipe;
 
 	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
 	if (!mode)
@@ -7476,8 +7476,11 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 	 * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
 	 * to use a real value here instead.
 	 */
-	pipe_config.cpu_transcoder = (enum transcoder) intel_crtc->pipe;
+	pipe_config.cpu_transcoder = (enum transcoder) pipe;
 	pipe_config.pixel_multiplier = 1;
+	pipe_config.dpll_hw_state.dpll = I915_READ(DPLL(pipe));
+	pipe_config.dpll_hw_state.fp0 = I915_READ(FP0(pipe));
+	pipe_config.dpll_hw_state.fp1 = I915_READ(FP1(pipe));
 	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
 
 	mode->clock = pipe_config.adjusted_mode.clock;
-- 
1.8.1.5

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

Re: [PATCH v2] drm/i915: Make i9xx_crtc_clock_get() use dpll_hw_state

[Jani Nikula]

On Fri, 13 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We already extract the DPLL state to pipe_config, so let's make use of
> it in i9xx_crtc_clock_get() and avoid the register reads.
>
> This will also make the function closer to being useable with PCH DPLL
> since the registers for those live in a different address.
>
> Also kill the useless adjusted_mode.clock zeroing. It's already zero at
> this point.
>
> v2: Read out DPLL state in intel_crtc_mode_get()

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 13dea9b..ad7f8a7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7326,14 +7326,14 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int pipe = pipe_config->cpu_transcoder;
> -	u32 dpll = I915_READ(DPLL(pipe));
> +	u32 dpll = pipe_config->dpll_hw_state.dpll;
>  	u32 fp;
>  	intel_clock_t clock;
>  
>  	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
> -		fp = I915_READ(FP0(pipe));
> +		fp = pipe_config->dpll_hw_state.fp0;
>  	else
> -		fp = I915_READ(FP1(pipe));
> +		fp = pipe_config->dpll_hw_state.fp1;
>  
>  	clock.m1 = (fp & FP_M1_DIV_MASK) >> FP_M1_DIV_SHIFT;
>  	if (IS_PINEVIEW(dev)) {
> @@ -7364,7 +7364,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  		default:
>  			DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed "
>  				  "mode\n", (int)(dpll & DPLL_MODE_MASK));
> -			pipe_config->adjusted_mode.clock = 0;
>  			return;
>  		}
>  
> @@ -7464,6 +7463,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	int hsync = I915_READ(HSYNC(cpu_transcoder));
>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
>  	int vsync = I915_READ(VSYNC(cpu_transcoder));
> +	enum pipe pipe = intel_crtc->pipe;
>  
>  	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
>  	if (!mode)
> @@ -7476,8 +7476,11 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	 * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
>  	 * to use a real value here instead.
>  	 */
> -	pipe_config.cpu_transcoder = (enum transcoder) intel_crtc->pipe;
> +	pipe_config.cpu_transcoder = (enum transcoder) pipe;
>  	pipe_config.pixel_multiplier = 1;
> +	pipe_config.dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> +	pipe_config.dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> +	pipe_config.dpll_hw_state.fp1 = I915_READ(FP1(pipe));
>  	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
>  
>  	mode->clock = pipe_config.adjusted_mode.clock;
> -- 
> 1.8.1.5
>

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

[PATCH 08/11] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

[ville.syrjala]

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

Just add the 120MHz reference clock case, and magically the
function appears to be ready to for PCH DPLLs.

Now, 120MHz might not always be correct, but we're already using
hardocoded values for other platforms, so the situation isn't
getting much worse.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 754de85..d89ea94 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7372,7 +7372,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 			return;
 		}
 
-		if (IS_PINEVIEW(dev))
+		if (HAS_PCH_SPLIT(dev))
+			i9xx_clock(120000, &clock);
+		else if (IS_PINEVIEW(dev))
 			pineview_clock(96000, &clock);
 		else
 			i9xx_clock(96000, &clock);
-- 
1.8.1.5

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

Re: [PATCH 08/11] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

[Daniel Vetter]

On Fri, Sep 06, 2013 at 11:29:05PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Just add the 120MHz reference clock case, and magically the
> function appears to be ready to for PCH DPLLs.
> 
> Now, 120MHz might not always be correct, but we're already using
> hardocoded values for other platforms, so the situation isn't
> getting much worse.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 754de85..d89ea94 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7372,7 +7372,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  			return;
>  		}
>  
> -		if (IS_PINEVIEW(dev))
> +		if (HAS_PCH_SPLIT(dev))
> +			i9xx_clock(120000, &clock);

Check out ironlake_get_refclk for the more correct answer. You can check
for the ssc case by checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN bit
in the dpll register.

One lofty idea I've had is that we'll track the dpll refclock in the pipe
config struct too. Then we can also double-check it when reconstructing
clocks. Finally I think the clock reconstruction dance should use the bits
in the dpll register and not magic knowledge ....

But that's a entire patch series of its own, probably part of the big
clock limits computation rework. So for this I'd just add a check for SSC
and snatch the SSC clock compuatation from ironlake_get_refclk.

Cheers, Daniel
> +		else if (IS_PINEVIEW(dev))
>  			pineview_clock(96000, &clock);
>  		else
>  			i9xx_clock(96000, &clock);
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v2] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

[ville.syrjala]

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

Add the 120MHz refernce clock case for PCH DPLLs.

Also determine the reference clock frequency more accurately by
checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN refclk input
mode. The gen2 code already checked it, but it stil assumed a
fixed 66MHz refclk. Instead we need to consult the VBT for the
real value.

v2: Fix refclk for SSC panel case

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 754de85..4f07292 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7324,6 +7324,22 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 	mutex_unlock(&crtc->mutex);
 }
 
+static int i9xx_pll_refclk(struct drm_device *dev,
+			   const struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 dpll = pipe_config->dpll_hw_state.dpll;
+
+	if ((dpll & PLL_REF_INPUT_MASK) == PLLB_REF_INPUT_SPREADSPECTRUMIN)
+		return dev_priv->vbt.lvds_ssc_freq * 1000;
+	else if (HAS_PCH_SPLIT(dev))
+		return 120000;
+	else if (!IS_GEN2(dev))
+		return 96000;
+	else
+		return 48000;
+}
+
 /* Returns the clock of the currently programmed mode of the given pipe. */
 static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 				struct intel_crtc_config *pipe_config)
@@ -7334,6 +7350,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 	u32 dpll = pipe_config->dpll_hw_state.dpll;
 	u32 fp;
 	intel_clock_t clock;
+	int refclk = i9xx_pll_refclk(dev, pipe_config);
 
 	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
 		fp = pipe_config->dpll_hw_state.fp0;
@@ -7373,9 +7390,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 		}
 
 		if (IS_PINEVIEW(dev))
-			pineview_clock(96000, &clock);
+			pineview_clock(refclk, &clock);
 		else
-			i9xx_clock(96000, &clock);
+			i9xx_clock(refclk, &clock);
 	} else {
 		bool is_lvds = (pipe == 1) && (I915_READ(LVDS) & LVDS_PORT_EN);
 
@@ -7383,13 +7400,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 			clock.p1 = ffs((dpll & DPLL_FPA01_P1_POST_DIV_MASK_I830_LVDS) >>
 				       DPLL_FPA01_P1_POST_DIV_SHIFT);
 			clock.p2 = 14;
-
-			if ((dpll & PLL_REF_INPUT_MASK) ==
-			    PLLB_REF_INPUT_SPREADSPECTRUMIN) {
-				/* XXX: might not be 66MHz */
-				i9xx_clock(66000, &clock);
-			} else
-				i9xx_clock(48000, &clock);
 		} else {
 			if (dpll & PLL_P1_DIVIDE_BY_TWO)
 				clock.p1 = 2;
@@ -7401,9 +7411,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 				clock.p2 = 4;
 			else
 				clock.p2 = 2;
-
-			i9xx_clock(48000, &clock);
 		}
+
+		i9xx_clock(refclk, &clock);
 	}
 
 	pipe_config->adjusted_mode.clock = clock.dot;
-- 
1.8.1.5

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

Re: [PATCH v2] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

[Jani Nikula]

On Mon, 09 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add the 120MHz refernce clock case for PCH DPLLs.
>
> Also determine the reference clock frequency more accurately by
> checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN refclk input
> mode. The gen2 code already checked it, but it stil assumed a
> fixed 66MHz refclk. Instead we need to consult the VBT for the
> real value.
>
> v2: Fix refclk for SSC panel case
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 754de85..4f07292 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7324,6 +7324,22 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>  	mutex_unlock(&crtc->mutex);
>  }
>  
> +static int i9xx_pll_refclk(struct drm_device *dev,
> +			   const struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 dpll = pipe_config->dpll_hw_state.dpll;
> +
> +	if ((dpll & PLL_REF_INPUT_MASK) == PLLB_REF_INPUT_SPREADSPECTRUMIN)

This seems wrong for at least gen3 and vlv. And it's a bit scary to go
change gen2 but oh well...

Jani.

> +		return dev_priv->vbt.lvds_ssc_freq * 1000;
> +	else if (HAS_PCH_SPLIT(dev))
> +		return 120000;
> +	else if (!IS_GEN2(dev))
> +		return 96000;
> +	else
> +		return 48000;
> +}
> +
>  /* Returns the clock of the currently programmed mode of the given pipe. */
>  static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  				struct intel_crtc_config *pipe_config)
> @@ -7334,6 +7350,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  	u32 dpll = pipe_config->dpll_hw_state.dpll;
>  	u32 fp;
>  	intel_clock_t clock;
> +	int refclk = i9xx_pll_refclk(dev, pipe_config);
>  
>  	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
>  		fp = pipe_config->dpll_hw_state.fp0;
> @@ -7373,9 +7390,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  		}
>  
>  		if (IS_PINEVIEW(dev))
> -			pineview_clock(96000, &clock);
> +			pineview_clock(refclk, &clock);
>  		else
> -			i9xx_clock(96000, &clock);
> +			i9xx_clock(refclk, &clock);
>  	} else {
>  		bool is_lvds = (pipe == 1) && (I915_READ(LVDS) & LVDS_PORT_EN);
>  
> @@ -7383,13 +7400,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  			clock.p1 = ffs((dpll & DPLL_FPA01_P1_POST_DIV_MASK_I830_LVDS) >>
>  				       DPLL_FPA01_P1_POST_DIV_SHIFT);
>  			clock.p2 = 14;
> -
> -			if ((dpll & PLL_REF_INPUT_MASK) ==
> -			    PLLB_REF_INPUT_SPREADSPECTRUMIN) {
> -				/* XXX: might not be 66MHz */
> -				i9xx_clock(66000, &clock);
> -			} else
> -				i9xx_clock(48000, &clock);
>  		} else {
>  			if (dpll & PLL_P1_DIVIDE_BY_TWO)
>  				clock.p1 = 2;
> @@ -7401,9 +7411,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  				clock.p2 = 4;
>  			else
>  				clock.p2 = 2;
> -
> -			i9xx_clock(48000, &clock);
>  		}
> +
> +		i9xx_clock(refclk, &clock);
>  	}
>  
>  	pipe_config->adjusted_mode.clock = clock.dot;
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v2] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

[Ville Syrjälä]

On Fri, Sep 13, 2013 at 04:04:03PM +0300, Jani Nikula wrote:
> On Mon, 09 Sep 2013, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Add the 120MHz refernce clock case for PCH DPLLs.
> >
> > Also determine the reference clock frequency more accurately by
> > checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN refclk input
> > mode. The gen2 code already checked it, but it stil assumed a
> > fixed 66MHz refclk. Instead we need to consult the VBT for the
> > real value.
> >
> > v2: Fix refclk for SSC panel case
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++-----------
> >  1 file changed, 21 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 754de85..4f07292 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7324,6 +7324,22 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> >  	mutex_unlock(&crtc->mutex);
> >  }
> >  
> > +static int i9xx_pll_refclk(struct drm_device *dev,
> > +			   const struct intel_crtc_config *pipe_config)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	u32 dpll = pipe_config->dpll_hw_state.dpll;
> > +
> > +	if ((dpll & PLL_REF_INPUT_MASK) == PLLB_REF_INPUT_SPREADSPECTRUMIN)
> 
> This seems wrong for at least gen3 and vlv. And it's a bit scary to go
> change gen2 but oh well...

Why? i8xx_update_pll(), i9xx_update_pll() and ironlake_compute_dpll()
all have the same logic for setting that bit.

For VLV I agree. But the clock readout there is totally busted anyway,
so I don't care at this point.

> 
> Jani.
> 
> > +		return dev_priv->vbt.lvds_ssc_freq * 1000;
> > +	else if (HAS_PCH_SPLIT(dev))
> > +		return 120000;
> > +	else if (!IS_GEN2(dev))
> > +		return 96000;
> > +	else
> > +		return 48000;
> > +}
> > +
> >  /* Returns the clock of the currently programmed mode of the given pipe. */
> >  static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  				struct intel_crtc_config *pipe_config)
> > @@ -7334,6 +7350,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  	u32 dpll = pipe_config->dpll_hw_state.dpll;
> >  	u32 fp;
> >  	intel_clock_t clock;
> > +	int refclk = i9xx_pll_refclk(dev, pipe_config);
> >  
> >  	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
> >  		fp = pipe_config->dpll_hw_state.fp0;
> > @@ -7373,9 +7390,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  		}
> >  
> >  		if (IS_PINEVIEW(dev))
> > -			pineview_clock(96000, &clock);
> > +			pineview_clock(refclk, &clock);
> >  		else
> > -			i9xx_clock(96000, &clock);
> > +			i9xx_clock(refclk, &clock);
> >  	} else {
> >  		bool is_lvds = (pipe == 1) && (I915_READ(LVDS) & LVDS_PORT_EN);
> >  
> > @@ -7383,13 +7400,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  			clock.p1 = ffs((dpll & DPLL_FPA01_P1_POST_DIV_MASK_I830_LVDS) >>
> >  				       DPLL_FPA01_P1_POST_DIV_SHIFT);
> >  			clock.p2 = 14;
> > -
> > -			if ((dpll & PLL_REF_INPUT_MASK) ==
> > -			    PLLB_REF_INPUT_SPREADSPECTRUMIN) {
> > -				/* XXX: might not be 66MHz */
> > -				i9xx_clock(66000, &clock);
> > -			} else
> > -				i9xx_clock(48000, &clock);
> >  		} else {
> >  			if (dpll & PLL_P1_DIVIDE_BY_TWO)
> >  				clock.p1 = 2;
> > @@ -7401,9 +7411,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >  				clock.p2 = 4;
> >  			else
> >  				clock.p2 = 2;
> > -
> > -			i9xx_clock(48000, &clock);
> >  		}
> > +
> > +		i9xx_clock(refclk, &clock);
> >  	}
> >  
> >  	pipe_config->adjusted_mode.clock = clock.dot;
> > -- 
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v2] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

[Jani Nikula]

On Fri, 13 Sep 2013, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Sep 13, 2013 at 04:04:03PM +0300, Jani Nikula wrote:
>> On Mon, 09 Sep 2013, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Add the 120MHz refernce clock case for PCH DPLLs.
>> >
>> > Also determine the reference clock frequency more accurately by
>> > checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN refclk input
>> > mode. The gen2 code already checked it, but it stil assumed a
>> > fixed 66MHz refclk. Instead we need to consult the VBT for the
>> > real value.
>> >
>> > v2: Fix refclk for SSC panel case
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++-----------
>> >  1 file changed, 21 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > index 754de85..4f07292 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -7324,6 +7324,22 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>> >  	mutex_unlock(&crtc->mutex);
>> >  }
>> >  
>> > +static int i9xx_pll_refclk(struct drm_device *dev,
>> > +			   const struct intel_crtc_config *pipe_config)
>> > +{
>> > +	struct drm_i915_private *dev_priv = dev->dev_private;
>> > +	u32 dpll = pipe_config->dpll_hw_state.dpll;
>> > +
>> > +	if ((dpll & PLL_REF_INPUT_MASK) == PLLB_REF_INPUT_SPREADSPECTRUMIN)
>> 
>> This seems wrong for at least gen3 and vlv. And it's a bit scary to go
>> change gen2 but oh well...
>
> Why? i8xx_update_pll(), i9xx_update_pll() and ironlake_compute_dpll()
> all have the same logic for setting that bit.

My Super Reliable(tm) gen3 spec says reserved for that value
there. *shrug*.

> For VLV I agree. But the clock readout there is totally busted anyway,
> so I don't care at this point.

Maybe Daniel can copy-paste something along those lines in the commit
message. Or not. *shrug. :)

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


>
>> 
>> Jani.
>> 
>> > +		return dev_priv->vbt.lvds_ssc_freq * 1000;
>> > +	else if (HAS_PCH_SPLIT(dev))
>> > +		return 120000;
>> > +	else if (!IS_GEN2(dev))
>> > +		return 96000;
>> > +	else
>> > +		return 48000;
>> > +}
>> > +
>> >  /* Returns the clock of the currently programmed mode of the given pipe. */
>> >  static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>> >  				struct intel_crtc_config *pipe_config)
>> > @@ -7334,6 +7350,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>> >  	u32 dpll = pipe_config->dpll_hw_state.dpll;
>> >  	u32 fp;
>> >  	intel_clock_t clock;
>> > +	int refclk = i9xx_pll_refclk(dev, pipe_config);
>> >  
>> >  	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
>> >  		fp = pipe_config->dpll_hw_state.fp0;
>> > @@ -7373,9 +7390,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>> >  		}
>> >  
>> >  		if (IS_PINEVIEW(dev))
>> > -			pineview_clock(96000, &clock);
>> > +			pineview_clock(refclk, &clock);
>> >  		else
>> > -			i9xx_clock(96000, &clock);
>> > +			i9xx_clock(refclk, &clock);
>> >  	} else {
>> >  		bool is_lvds = (pipe == 1) && (I915_READ(LVDS) & LVDS_PORT_EN);
>> >  
>> > @@ -7383,13 +7400,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>> >  			clock.p1 = ffs((dpll & DPLL_FPA01_P1_POST_DIV_MASK_I830_LVDS) >>
>> >  				       DPLL_FPA01_P1_POST_DIV_SHIFT);
>> >  			clock.p2 = 14;
>> > -
>> > -			if ((dpll & PLL_REF_INPUT_MASK) ==
>> > -			    PLLB_REF_INPUT_SPREADSPECTRUMIN) {
>> > -				/* XXX: might not be 66MHz */
>> > -				i9xx_clock(66000, &clock);
>> > -			} else
>> > -				i9xx_clock(48000, &clock);
>> >  		} else {
>> >  			if (dpll & PLL_P1_DIVIDE_BY_TWO)
>> >  				clock.p1 = 2;
>> > @@ -7401,9 +7411,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>> >  				clock.p2 = 4;
>> >  			else
>> >  				clock.p2 = 2;
>> > -
>> > -			i9xx_clock(48000, &clock);
>> >  		}
>> > +
>> > +		i9xx_clock(refclk, &clock);
>> >  	}
>> >  
>> >  	pipe_config->adjusted_mode.clock = clock.dot;
>> > -- 
>> > 1.8.1.5
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>
> -- 
> Ville Syrjälä
> Intel OTC

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

Re: [PATCH v2] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

[Ville Syrjälä]

On Fri, Sep 13, 2013 at 04:47:53PM +0300, Jani Nikula wrote:
> On Fri, 13 Sep 2013, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Sep 13, 2013 at 04:04:03PM +0300, Jani Nikula wrote:
> >> On Mon, 09 Sep 2013, ville.syrjala@linux.intel.com wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > Add the 120MHz refernce clock case for PCH DPLLs.
> >> >
> >> > Also determine the reference clock frequency more accurately by
> >> > checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN refclk input
> >> > mode. The gen2 code already checked it, but it stil assumed a
> >> > fixed 66MHz refclk. Instead we need to consult the VBT for the
> >> > real value.
> >> >
> >> > v2: Fix refclk for SSC panel case
> >> >
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++-----------
> >> >  1 file changed, 21 insertions(+), 11 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> > index 754de85..4f07292 100644
> >> > --- a/drivers/gpu/drm/i915/intel_display.c
> >> > +++ b/drivers/gpu/drm/i915/intel_display.c
> >> > @@ -7324,6 +7324,22 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> >> >  	mutex_unlock(&crtc->mutex);
> >> >  }
> >> >  
> >> > +static int i9xx_pll_refclk(struct drm_device *dev,
> >> > +			   const struct intel_crtc_config *pipe_config)
> >> > +{
> >> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> > +	u32 dpll = pipe_config->dpll_hw_state.dpll;
> >> > +
> >> > +	if ((dpll & PLL_REF_INPUT_MASK) == PLLB_REF_INPUT_SPREADSPECTRUMIN)
> >> 
> >> This seems wrong for at least gen3 and vlv. And it's a bit scary to go
> >> change gen2 but oh well...
> >
> > Why? i8xx_update_pll(), i9xx_update_pll() and ironlake_compute_dpll()
> > all have the same logic for setting that bit.
> 
> My Super Reliable(tm) gen3 spec says reserved for that value
> there. *shrug*.

Looks like it's there only for pipe B and even then only for certain
platforms. But if we managed to program it w/ an invalid value I suspect
things fall apart in other ways.

> 
> > For VLV I agree. But the clock readout there is totally busted anyway,
> > so I don't care at this point.
> 
> Maybe Daniel can copy-paste something along those lines in the commit
> message. Or not. *shrug. :)
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> >
> >> 
> >> Jani.
> >> 
> >> > +		return dev_priv->vbt.lvds_ssc_freq * 1000;
> >> > +	else if (HAS_PCH_SPLIT(dev))
> >> > +		return 120000;
> >> > +	else if (!IS_GEN2(dev))
> >> > +		return 96000;
> >> > +	else
> >> > +		return 48000;
> >> > +}
> >> > +
> >> >  /* Returns the clock of the currently programmed mode of the given pipe. */
> >> >  static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >> >  				struct intel_crtc_config *pipe_config)
> >> > @@ -7334,6 +7350,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >> >  	u32 dpll = pipe_config->dpll_hw_state.dpll;
> >> >  	u32 fp;
> >> >  	intel_clock_t clock;
> >> > +	int refclk = i9xx_pll_refclk(dev, pipe_config);
> >> >  
> >> >  	if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
> >> >  		fp = pipe_config->dpll_hw_state.fp0;
> >> > @@ -7373,9 +7390,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >> >  		}
> >> >  
> >> >  		if (IS_PINEVIEW(dev))
> >> > -			pineview_clock(96000, &clock);
> >> > +			pineview_clock(refclk, &clock);
> >> >  		else
> >> > -			i9xx_clock(96000, &clock);
> >> > +			i9xx_clock(refclk, &clock);
> >> >  	} else {
> >> >  		bool is_lvds = (pipe == 1) && (I915_READ(LVDS) & LVDS_PORT_EN);
> >> >  
> >> > @@ -7383,13 +7400,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >> >  			clock.p1 = ffs((dpll & DPLL_FPA01_P1_POST_DIV_MASK_I830_LVDS) >>
> >> >  				       DPLL_FPA01_P1_POST_DIV_SHIFT);
> >> >  			clock.p2 = 14;
> >> > -
> >> > -			if ((dpll & PLL_REF_INPUT_MASK) ==
> >> > -			    PLLB_REF_INPUT_SPREADSPECTRUMIN) {
> >> > -				/* XXX: might not be 66MHz */
> >> > -				i9xx_clock(66000, &clock);
> >> > -			} else
> >> > -				i9xx_clock(48000, &clock);
> >> >  		} else {
> >> >  			if (dpll & PLL_P1_DIVIDE_BY_TWO)
> >> >  				clock.p1 = 2;
> >> > @@ -7401,9 +7411,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> >> >  				clock.p2 = 4;
> >> >  			else
> >> >  				clock.p2 = 2;
> >> > -
> >> > -			i9xx_clock(48000, &clock);
> >> >  		}
> >> > +
> >> > +		i9xx_clock(refclk, &clock);
> >> >  	}
> >> >  
> >> >  	pipe_config->adjusted_mode.clock = clock.dot;
> >> > -- 
> >> > 1.8.1.5
> >> >
> >> > _______________________________________________
> >> > Intel-gfx mailing list
> >> > Intel-gfx@lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Technology Center
> >
> > -- 
> > Ville Syrjälä
> > Intel OTC
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v2] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

[Daniel Vetter]

On Fri, Sep 13, 2013 at 04:47:53PM +0300, Jani Nikula wrote:
> On Fri, 13 Sep 2013, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Sep 13, 2013 at 04:04:03PM +0300, Jani Nikula wrote:
> >> On Mon, 09 Sep 2013, ville.syrjala@linux.intel.com wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > Add the 120MHz refernce clock case for PCH DPLLs.
> >> >
> >> > Also determine the reference clock frequency more accurately by
> >> > checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN refclk input
> >> > mode. The gen2 code already checked it, but it stil assumed a
> >> > fixed 66MHz refclk. Instead we need to consult the VBT for the
> >> > real value.
> >> >
> >> > v2: Fix refclk for SSC panel case
> >> >
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++-----------
> >> >  1 file changed, 21 insertions(+), 11 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> > index 754de85..4f07292 100644
> >> > --- a/drivers/gpu/drm/i915/intel_display.c
> >> > +++ b/drivers/gpu/drm/i915/intel_display.c
> >> > @@ -7324,6 +7324,22 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> >> >  	mutex_unlock(&crtc->mutex);
> >> >  }
> >> >  
> >> > +static int i9xx_pll_refclk(struct drm_device *dev,
> >> > +			   const struct intel_crtc_config *pipe_config)
> >> > +{
> >> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> > +	u32 dpll = pipe_config->dpll_hw_state.dpll;
> >> > +
> >> > +	if ((dpll & PLL_REF_INPUT_MASK) == PLLB_REF_INPUT_SPREADSPECTRUMIN)
> >> 
> >> This seems wrong for at least gen3 and vlv. And it's a bit scary to go
> >> change gen2 but oh well...
> >
> > Why? i8xx_update_pll(), i9xx_update_pll() and ironlake_compute_dpll()
> > all have the same logic for setting that bit.
> 
> My Super Reliable(tm) gen3 spec says reserved for that value
> there. *shrug*.
> 
> > For VLV I agree. But the clock readout there is totally busted anyway,
> > so I don't care at this point.
> 
> Maybe Daniel can copy-paste something along those lines in the commit
> message. Or not. *shrug. :)

SSC refclocks is only used on lvds on those platforms, and lo and behold
lvds is restricted to pipe B on gen2/3. Magic!

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 09/11] drm/i915: Fix port_clock and adjusted_mode.clock readout all over

[ville.syrjala]

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

Now that adjusted_mode.clock no longer contains the pixel_multiplier, we
can kill the get_clock() callback and instead do the clock readout
in get_pipe_config().

Also i9xx_crtc_clock_get() can now extract the frequency of the PCH
DPLL, so use it to populate port_clock accurately for PCH encoders.
For DP in port A the encoder is still responsible for filling in
port_clock. The FDI adjusted_mode.clock extraction is kept in place
for some extra sanity checking, but we no longer need to pretend it's
also the port_clock.

In the encoder get_config() functions fill out adjusted_mode.clock
based on port_clock and other details such as the DP M/N values,
HDMI 12bpc and SDVO pixel_multiplier. For PCH encoders we will then
do an extra sanity check to make sure the dotclock we derived from
the FDI configuratiuon matches the one we derive from port_clock.

DVO doesn't exist on PCH platforms, so it doesn't need to anything
but assign adjusted_mode.clock=port_clock. And DDI is HSW only, so
none of the changes apply there.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_crt.c     |  8 ++++++
 drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_dp.c      | 11 ++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_dvo.c     |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c    | 11 +++++++++
 drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++++++
 8 files changed, 72 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 769c138..09fc308 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -369,7 +369,6 @@ struct drm_i915_display_funcs {
 	 * fills out the pipe-config with the hw state. */
 	bool (*get_pipe_config)(struct intel_crtc *,
 				struct intel_crtc_config *);
-	void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *);
 	int (*crtc_mode_set)(struct drm_crtc *crtc,
 			     int x, int y,
 			     struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index ea9022e..be99cf2 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -89,6 +89,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_crt *crt = intel_encoder_to_crt(encoder);
 	u32 tmp, flags = 0;
+	int dotclock;
 
 	tmp = I915_READ(crt->adpa_reg);
 
@@ -103,6 +104,13 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 /* Note: The caller is required to filter out dpms modes not supported by the
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d89ea94..26f0269 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5047,6 +5047,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 						     DPLL_PORTB_READY_MASK);
 	}
 
+	i9xx_crtc_clock_get(crtc, pipe_config);
+
 	return true;
 }
 
@@ -6007,6 +6009,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->pixel_multiplier =
 			((tmp & PLL_REF_SDVO_HDMI_MULTIPLIER_MASK)
 			 >> PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT) + 1;
+
+		ironlake_crtc_clock_get(crtc, pipe_config);
 	} else {
 		pipe_config->pixel_multiplier = 1;
 	}
@@ -7408,7 +7412,12 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 		}
 	}
 
-	pipe_config->adjusted_mode.clock = clock.dot;
+	/*
+	 * This value includes pixel_multiplier. We will use
+	 * port_clock to compute adjusted_mode.clock in the
+	 * encoder's get_config() function.
+	 */
+	pipe_config->port_clock = clock.dot;
 }
 
 int intel_dotclock_calculate(int link_freq,
@@ -7436,6 +7445,9 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	int link_freq;
 
+	/* read out port_clock from the DPLL */
+	i9xx_crtc_clock_get(crtc, pipe_config);
+
 	/*
 	 * We need to get the FDI or DP link clock here to derive
 	 * the M/N dividers.
@@ -7446,6 +7458,12 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	 */
 	link_freq = intel_fdi_link_freq(dev) * 10000;
 
+	/*
+	 * This value does not include pixel_multiplier.
+	 * We will check that port_clock and adjusted_mode.clock
+	 * agree once we know their relationship in the encoder's
+	 * get_config() function.
+	 */
 	pipe_config->adjusted_mode.clock =
 		intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
 }
@@ -8859,9 +8877,6 @@ check_crtc_state(struct drm_device *dev)
 				encoder->get_config(encoder, &pipe_config);
 		}
 
-		if (dev_priv->display.get_clock)
-			dev_priv->display.get_clock(crtc, &pipe_config);
-
 		WARN(crtc->active != active,
 		     "crtc active state doesn't match with hw state "
 		     "(expected %i, found %i)\n", crtc->active, active);
@@ -8936,6 +8951,18 @@ intel_modeset_check_state(struct drm_device *dev)
 	check_shared_dpll_state(dev);
 }
 
+void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
+				     int dotclock)
+{
+	/*
+	 * FDI already provided one idea for the dotclock.
+	 * Yell if the encoder disagrees.
+	 */
+	WARN(!intel_fuzzy_clock_check(pipe_config->adjusted_mode.clock, dotclock),
+	     "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
+	     pipe_config->adjusted_mode.clock, dotclock);
+}
+
 static int __intel_set_mode(struct drm_crtc *crtc,
 			    struct drm_display_mode *mode,
 			    int x, int y, struct drm_framebuffer *fb)
@@ -9887,7 +9914,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
-		dev_priv->display.get_clock = ironlake_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
@@ -9895,7 +9921,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
-		dev_priv->display.get_clock = i9xx_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -9903,7 +9928,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = i9xx_update_plane;
 	} else {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
-		dev_priv->display.get_clock = i9xx_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -10517,15 +10541,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			      pipe);
 	}
 
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
-			    base.head) {
-		if (!crtc->active)
-			continue;
-		if (dev_priv->display.get_clock)
-			dev_priv->display.get_clock(crtc,
-						    &crtc->config);
-	}
-
 	list_for_each_entry(connector, &dev->mode_config.connector_list,
 			    base.head) {
 		if (connector->get_hw_state(connector)) {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index be0449b..509a9e4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1372,6 +1372,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
+	int dotclock;
 
 	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
 		tmp = I915_READ(intel_dp->output_reg);
@@ -1403,12 +1404,20 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 
 	intel_dp_get_m_n(crtc, pipe_config);
 
-	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
+	if (port == PORT_A) {
 		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
 			pipe_config->port_clock = 162000;
 		else
 			pipe_config->port_clock = 270000;
 	}
+
+	dotclock = intel_dotclock_calculate(pipe_config->port_clock,
+					    &pipe_config->dp_m_n);
+
+	if (HAS_PCH_SPLIT(dev_priv->dev) && port != PORT_A)
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 static bool is_edp_psr(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4cf8898..6e1204f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -805,5 +805,7 @@ extern void intel_dp_get_m_n(struct intel_crtc *crtc,
 			     struct intel_crtc_config *pipe_config);
 extern int intel_dotclock_calculate(int link_freq,
 				    const struct intel_link_m_n *m_n);
+extern void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
+					    int dotclock);
 
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 406303b..92487ce 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -153,6 +153,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	pipe_config->adjusted_mode.clock = pipe_config->port_clock;
 }
 
 static void intel_disable_dvo(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4148cc8..56e6191 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -713,6 +713,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	u32 tmp, flags = 0;
+	int dotclock;
 
 	tmp = I915_READ(intel_hdmi->hdmi_reg);
 
@@ -727,6 +728,16 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	if (pipe_config->pipe_bpp == 12*3)
+		dotclock = pipe_config->port_clock * 2 / 3;
+	else
+		dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 static void intel_enable_hdmi(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 74042c6..7148fdd 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1319,6 +1319,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
 	struct intel_sdvo_dtd dtd;
 	int encoder_pixel_multiplier = 0;
+	int dotclock;
 	u32 flags = 0, sdvox;
 	u8 val;
 	bool ret;
@@ -1357,6 +1358,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 			 >> SDVO_PORT_MULTIPLY_SHIFT) + 1;
 	}
 
+	dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier;
+
+	if (HAS_PCH_SPLIT(dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
+
 	/* Cross check the port pixel multiplier with the sdvo encoder state. */
 	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_CLOCK_RATE_MULT,
 				 &val, 1)) {
-- 
1.8.1.5

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

Re: [PATCH 09/11] drm/i915: Fix port_clock and adjusted_mode.clock readout all over

[Daniel Vetter]

On Fri, Sep 06, 2013 at 11:29:06PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that adjusted_mode.clock no longer contains the pixel_multiplier, we
> can kill the get_clock() callback and instead do the clock readout
> in get_pipe_config().
> 
> Also i9xx_crtc_clock_get() can now extract the frequency of the PCH
> DPLL, so use it to populate port_clock accurately for PCH encoders.
> For DP in port A the encoder is still responsible for filling in
> port_clock. The FDI adjusted_mode.clock extraction is kept in place
> for some extra sanity checking, but we no longer need to pretend it's
> also the port_clock.
> 
> In the encoder get_config() functions fill out adjusted_mode.clock
> based on port_clock and other details such as the DP M/N values,
> HDMI 12bpc and SDVO pixel_multiplier. For PCH encoders we will then
> do an extra sanity check to make sure the dotclock we derived from
> the FDI configuratiuon matches the one we derive from port_clock.
> 
> DVO doesn't exist on PCH platforms, so it doesn't need to anything
> but assign adjusted_mode.clock=port_clock. And DDI is HSW only, so
> none of the changes apply there.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 -
>  drivers/gpu/drm/i915/intel_crt.c     |  8 ++++++
>  drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_dp.c      | 11 ++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_dvo.c     |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c    | 11 +++++++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++++++
>  8 files changed, 72 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 769c138..09fc308 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -369,7 +369,6 @@ struct drm_i915_display_funcs {
>  	 * fills out the pipe-config with the hw state. */
>  	bool (*get_pipe_config)(struct intel_crtc *,
>  				struct intel_crtc_config *);
> -	void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *);
>  	int (*crtc_mode_set)(struct drm_crtc *crtc,
>  			     int x, int y,
>  			     struct drm_framebuffer *old_fb);
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index ea9022e..be99cf2 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -89,6 +89,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>  	struct intel_crt *crt = intel_encoder_to_crt(encoder);
>  	u32 tmp, flags = 0;
> +	int dotclock;
>  
>  	tmp = I915_READ(crt->adpa_reg);
>  
> @@ -103,6 +104,13 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	dotclock = pipe_config->port_clock;
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  /* Note: The caller is required to filter out dpms modes not supported by the
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d89ea94..26f0269 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5047,6 +5047,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  						     DPLL_PORTB_READY_MASK);
>  	}
>  
> +	i9xx_crtc_clock_get(crtc, pipe_config);
> +
>  	return true;
>  }
>  
> @@ -6007,6 +6009,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier =
>  			((tmp & PLL_REF_SDVO_HDMI_MULTIPLIER_MASK)
>  			 >> PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT) + 1;
> +
> +		ironlake_crtc_clock_get(crtc, pipe_config);
>  	} else {
>  		pipe_config->pixel_multiplier = 1;
>  	}
> @@ -7408,7 +7412,12 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  		}
>  	}
>  
> -	pipe_config->adjusted_mode.clock = clock.dot;
> +	/*
> +	 * This value includes pixel_multiplier. We will use
> +	 * port_clock to compute adjusted_mode.clock in the
> +	 * encoder's get_config() function.
> +	 */
> +	pipe_config->port_clock = clock.dot;
>  }
>  
>  int intel_dotclock_calculate(int link_freq,
> @@ -7436,6 +7445,9 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	int link_freq;
>  
> +	/* read out port_clock from the DPLL */
> +	i9xx_crtc_clock_get(crtc, pipe_config);
> +
>  	/*
>  	 * We need to get the FDI or DP link clock here to derive
>  	 * the M/N dividers.
> @@ -7446,6 +7458,12 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	 */
>  	link_freq = intel_fdi_link_freq(dev) * 10000;
>  
> +	/*
> +	 * This value does not include pixel_multiplier.
> +	 * We will check that port_clock and adjusted_mode.clock
> +	 * agree once we know their relationship in the encoder's
> +	 * get_config() function.
> +	 */
>  	pipe_config->adjusted_mode.clock =
>  		intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
>  }
> @@ -8859,9 +8877,6 @@ check_crtc_state(struct drm_device *dev)
>  				encoder->get_config(encoder, &pipe_config);
>  		}
>  
> -		if (dev_priv->display.get_clock)
> -			dev_priv->display.get_clock(crtc, &pipe_config);
> -
>  		WARN(crtc->active != active,
>  		     "crtc active state doesn't match with hw state "
>  		     "(expected %i, found %i)\n", crtc->active, active);
> @@ -8936,6 +8951,18 @@ intel_modeset_check_state(struct drm_device *dev)
>  	check_shared_dpll_state(dev);
>  }
>  
> +void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> +				     int dotclock)
> +{
> +	/*
> +	 * FDI already provided one idea for the dotclock.
> +	 * Yell if the encoder disagrees.
> +	 */
> +	WARN(!intel_fuzzy_clock_check(pipe_config->adjusted_mode.clock, dotclock),
> +	     "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
> +	     pipe_config->adjusted_mode.clock, dotclock);
> +}
> +
>  static int __intel_set_mode(struct drm_crtc *crtc,
>  			    struct drm_display_mode *mode,
>  			    int x, int y, struct drm_framebuffer *fb)
> @@ -9887,7 +9914,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = ironlake_update_plane;
>  	} else if (HAS_PCH_SPLIT(dev)) {
>  		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
> -		dev_priv->display.get_clock = ironlake_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
>  		dev_priv->display.crtc_enable = ironlake_crtc_enable;
>  		dev_priv->display.crtc_disable = ironlake_crtc_disable;
> @@ -9895,7 +9921,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = ironlake_update_plane;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> -		dev_priv->display.get_clock = i9xx_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
>  		dev_priv->display.crtc_enable = valleyview_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -9903,7 +9928,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = i9xx_update_plane;
>  	} else {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> -		dev_priv->display.get_clock = i9xx_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
>  		dev_priv->display.crtc_enable = i9xx_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -10517,15 +10541,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			      pipe);
>  	}
>  
> -	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> -			    base.head) {
> -		if (!crtc->active)
> -			continue;
> -		if (dev_priv->display.get_clock)
> -			dev_priv->display.get_clock(crtc,
> -						    &crtc->config);
> -	}
> -
>  	list_for_each_entry(connector, &dev->mode_config.connector_list,
>  			    base.head) {
>  		if (connector->get_hw_state(connector)) {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index be0449b..509a9e4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1372,6 +1372,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> +	int dotclock;
>  
>  	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
>  		tmp = I915_READ(intel_dp->output_reg);
> @@ -1403,12 +1404,20 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  
>  	intel_dp_get_m_n(crtc, pipe_config);
>  
> -	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
> +	if (port == PORT_A) {
>  		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
>  			pipe_config->port_clock = 162000;
>  		else
>  			pipe_config->port_clock = 270000;
>  	}
> +
> +	dotclock = intel_dotclock_calculate(pipe_config->port_clock,
> +					    &pipe_config->dp_m_n);
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev) && port != PORT_A)
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  static bool is_edp_psr(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4cf8898..6e1204f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -805,5 +805,7 @@ extern void intel_dp_get_m_n(struct intel_crtc *crtc,
>  			     struct intel_crtc_config *pipe_config);
>  extern int intel_dotclock_calculate(int link_freq,
>  				    const struct intel_link_m_n *m_n);
> +extern void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> +					    int dotclock);
>  
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 406303b..92487ce 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -153,6 +153,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	pipe_config->adjusted_mode.clock = pipe_config->port_clock;
>  }
>  
>  static void intel_disable_dvo(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4148cc8..56e6191 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -713,6 +713,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>  	u32 tmp, flags = 0;
> +	int dotclock;
>  
>  	tmp = I915_READ(intel_hdmi->hdmi_reg);
>  
> @@ -727,6 +728,16 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	if (pipe_config->pipe_bpp == 12*3)

I think you need to check for the 12BPC bit in the hdmi/sdvo reg here, you
can't rely on the pipe bpc value. At least on fdi configs we might end up
with sending a differnt pipe bpp (mayb just 10bpc) accross the wire than
what the port will output (if we aim for the 12bpc mode).
-Daniel

> +		dotclock = pipe_config->port_clock * 2 / 3;
> +	else
> +		dotclock = pipe_config->port_clock;
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  static void intel_enable_hdmi(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 74042c6..7148fdd 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1319,6 +1319,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
>  	struct intel_sdvo_dtd dtd;
>  	int encoder_pixel_multiplier = 0;
> +	int dotclock;
>  	u32 flags = 0, sdvox;
>  	u8 val;
>  	bool ret;
> @@ -1357,6 +1358,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  			 >> SDVO_PORT_MULTIPLY_SHIFT) + 1;
>  	}
>  
> +	dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier;
> +
> +	if (HAS_PCH_SPLIT(dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
> +
>  	/* Cross check the port pixel multiplier with the sdvo encoder state. */
>  	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_CLOCK_RATE_MULT,
>  				 &val, 1)) {
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v2] drm/i915: Fix port_clock and adjusted_mode.clock readout all over

[ville.syrjala]

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

Now that adjusted_mode.clock no longer contains the pixel_multiplier, we
can kill the get_clock() callback and instead do the clock readout
in get_pipe_config().

Also i9xx_crtc_clock_get() can now extract the frequency of the PCH
DPLL, so use it to populate port_clock accurately for PCH encoders.
For DP in port A the encoder is still responsible for filling in
port_clock. The FDI adjusted_mode.clock extraction is kept in place
for some extra sanity checking, but we no longer need to pretend it's
also the port_clock.

In the encoder get_config() functions fill out adjusted_mode.clock
based on port_clock and other details such as the DP M/N values,
HDMI 12bpc and SDVO pixel_multiplier. For PCH encoders we will then
do an extra sanity check to make sure the dotclock we derived from
the FDI configuratiuon matches the one we derive from port_clock.

DVO doesn't exist on PCH platforms, so it doesn't need to anything
but assign adjusted_mode.clock=port_clock. And DDI is HSW only, so
none of the changes apply there.

v2: Use hdmi_reg color format to detect 12bpc HDMI case

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 -
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_crt.c     |  8 ++++++
 drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_dp.c      | 11 ++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_dvo.c     |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c    | 11 +++++++++
 drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++++++
 9 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 769c138..09fc308 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -369,7 +369,6 @@ struct drm_i915_display_funcs {
 	 * fills out the pipe-config with the hw state. */
 	bool (*get_pipe_config)(struct intel_crtc *,
 				struct intel_crtc_config *);
-	void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *);
 	int (*crtc_mode_set)(struct drm_crtc *crtc,
 			     int x, int y,
 			     struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c7f2da3..ace9443 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2071,6 +2071,7 @@
 
 /* Gen 4 SDVO/HDMI bits: */
 #define   SDVO_COLOR_FORMAT_8bpc		(0 << 26)
+#define   SDVO_COLOR_FORMAT_MASK		(7 << 26)
 #define   SDVO_ENCODING_SDVO			(0 << 10)
 #define   SDVO_ENCODING_HDMI			(2 << 10)
 #define   HDMI_MODE_SELECT_HDMI			(1 << 9) /* HDMI only */
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index ea9022e..be99cf2 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -89,6 +89,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_crt *crt = intel_encoder_to_crt(encoder);
 	u32 tmp, flags = 0;
+	int dotclock;
 
 	tmp = I915_READ(crt->adpa_reg);
 
@@ -103,6 +104,13 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 /* Note: The caller is required to filter out dpms modes not supported by the
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d89ea94..26f0269 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5047,6 +5047,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 						     DPLL_PORTB_READY_MASK);
 	}
 
+	i9xx_crtc_clock_get(crtc, pipe_config);
+
 	return true;
 }
 
@@ -6007,6 +6009,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->pixel_multiplier =
 			((tmp & PLL_REF_SDVO_HDMI_MULTIPLIER_MASK)
 			 >> PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT) + 1;
+
+		ironlake_crtc_clock_get(crtc, pipe_config);
 	} else {
 		pipe_config->pixel_multiplier = 1;
 	}
@@ -7408,7 +7412,12 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 		}
 	}
 
-	pipe_config->adjusted_mode.clock = clock.dot;
+	/*
+	 * This value includes pixel_multiplier. We will use
+	 * port_clock to compute adjusted_mode.clock in the
+	 * encoder's get_config() function.
+	 */
+	pipe_config->port_clock = clock.dot;
 }
 
 int intel_dotclock_calculate(int link_freq,
@@ -7436,6 +7445,9 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	int link_freq;
 
+	/* read out port_clock from the DPLL */
+	i9xx_crtc_clock_get(crtc, pipe_config);
+
 	/*
 	 * We need to get the FDI or DP link clock here to derive
 	 * the M/N dividers.
@@ -7446,6 +7458,12 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	 */
 	link_freq = intel_fdi_link_freq(dev) * 10000;
 
+	/*
+	 * This value does not include pixel_multiplier.
+	 * We will check that port_clock and adjusted_mode.clock
+	 * agree once we know their relationship in the encoder's
+	 * get_config() function.
+	 */
 	pipe_config->adjusted_mode.clock =
 		intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
 }
@@ -8859,9 +8877,6 @@ check_crtc_state(struct drm_device *dev)
 				encoder->get_config(encoder, &pipe_config);
 		}
 
-		if (dev_priv->display.get_clock)
-			dev_priv->display.get_clock(crtc, &pipe_config);
-
 		WARN(crtc->active != active,
 		     "crtc active state doesn't match with hw state "
 		     "(expected %i, found %i)\n", crtc->active, active);
@@ -8936,6 +8951,18 @@ intel_modeset_check_state(struct drm_device *dev)
 	check_shared_dpll_state(dev);
 }
 
+void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
+				     int dotclock)
+{
+	/*
+	 * FDI already provided one idea for the dotclock.
+	 * Yell if the encoder disagrees.
+	 */
+	WARN(!intel_fuzzy_clock_check(pipe_config->adjusted_mode.clock, dotclock),
+	     "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
+	     pipe_config->adjusted_mode.clock, dotclock);
+}
+
 static int __intel_set_mode(struct drm_crtc *crtc,
 			    struct drm_display_mode *mode,
 			    int x, int y, struct drm_framebuffer *fb)
@@ -9887,7 +9914,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
-		dev_priv->display.get_clock = ironlake_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
@@ -9895,7 +9921,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
-		dev_priv->display.get_clock = i9xx_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -9903,7 +9928,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = i9xx_update_plane;
 	} else {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
-		dev_priv->display.get_clock = i9xx_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -10517,15 +10541,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			      pipe);
 	}
 
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
-			    base.head) {
-		if (!crtc->active)
-			continue;
-		if (dev_priv->display.get_clock)
-			dev_priv->display.get_clock(crtc,
-						    &crtc->config);
-	}
-
 	list_for_each_entry(connector, &dev->mode_config.connector_list,
 			    base.head) {
 		if (connector->get_hw_state(connector)) {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index be0449b..509a9e4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1372,6 +1372,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
+	int dotclock;
 
 	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
 		tmp = I915_READ(intel_dp->output_reg);
@@ -1403,12 +1404,20 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 
 	intel_dp_get_m_n(crtc, pipe_config);
 
-	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
+	if (port == PORT_A) {
 		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
 			pipe_config->port_clock = 162000;
 		else
 			pipe_config->port_clock = 270000;
 	}
+
+	dotclock = intel_dotclock_calculate(pipe_config->port_clock,
+					    &pipe_config->dp_m_n);
+
+	if (HAS_PCH_SPLIT(dev_priv->dev) && port != PORT_A)
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 static bool is_edp_psr(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4cf8898..6e1204f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -805,5 +805,7 @@ extern void intel_dp_get_m_n(struct intel_crtc *crtc,
 			     struct intel_crtc_config *pipe_config);
 extern int intel_dotclock_calculate(int link_freq,
 				    const struct intel_link_m_n *m_n);
+extern void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
+					    int dotclock);
 
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 406303b..92487ce 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -153,6 +153,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	pipe_config->adjusted_mode.clock = pipe_config->port_clock;
 }
 
 static void intel_disable_dvo(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4148cc8..16daa72 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -713,6 +713,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	u32 tmp, flags = 0;
+	int dotclock;
 
 	tmp = I915_READ(intel_hdmi->hdmi_reg);
 
@@ -727,6 +728,16 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	if ((tmp & SDVO_COLOR_FORMAT_MASK) == HDMI_COLOR_FORMAT_12bpc)
+		dotclock = pipe_config->port_clock * 2 / 3;
+	else
+		dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 static void intel_enable_hdmi(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 74042c6..7148fdd 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1319,6 +1319,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
 	struct intel_sdvo_dtd dtd;
 	int encoder_pixel_multiplier = 0;
+	int dotclock;
 	u32 flags = 0, sdvox;
 	u8 val;
 	bool ret;
@@ -1357,6 +1358,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 			 >> SDVO_PORT_MULTIPLY_SHIFT) + 1;
 	}
 
+	dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier;
+
+	if (HAS_PCH_SPLIT(dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
+
 	/* Cross check the port pixel multiplier with the sdvo encoder state. */
 	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_CLOCK_RATE_MULT,
 				 &val, 1)) {
-- 
1.8.1.5

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

[PATCH v3] drm/i915: Fix port_clock and adjusted_mode.clock readout all over

[ville.syrjala]

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

Now that adjusted_mode.clock no longer contains the pixel_multiplier, we
can kill the get_clock() callback and instead do the clock readout
in get_pipe_config().

Also i9xx_crtc_clock_get() can now extract the frequency of the PCH
DPLL, so use it to populate port_clock accurately for PCH encoders.
For DP in port A the encoder is still responsible for filling in
port_clock. The FDI adjusted_mode.clock extraction is kept in place
for some extra sanity checking, but we no longer need to pretend it's
also the port_clock.

In the encoder get_config() functions fill out adjusted_mode.clock
based on port_clock and other details such as the DP M/N values,
HDMI 12bpc and SDVO pixel_multiplier. For PCH encoders we will then
do an extra sanity check to make sure the dotclock we derived from
the FDI configuratiuon matches the one we derive from port_clock.

DVO doesn't exist on PCH platforms, so it doesn't need to anything
but assign adjusted_mode.clock=port_clock. And DDI is HSW only, so
none of the changes apply there.

v2: Use hdmi_reg color format to detect 12bpc HDMI case
v3: Set adjusted_mode.clock for LVDS too

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 -
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_crt.c     |  8 ++++++
 drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_dp.c      | 11 ++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_dvo.c     |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c    | 11 +++++++++
 drivers/gpu/drm/i915/intel_lvds.c    |  8 ++++++
 drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++++++
 10 files changed, 81 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 769c138..09fc308 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -369,7 +369,6 @@ struct drm_i915_display_funcs {
 	 * fills out the pipe-config with the hw state. */
 	bool (*get_pipe_config)(struct intel_crtc *,
 				struct intel_crtc_config *);
-	void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *);
 	int (*crtc_mode_set)(struct drm_crtc *crtc,
 			     int x, int y,
 			     struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c7f2da3..ace9443 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2071,6 +2071,7 @@
 
 /* Gen 4 SDVO/HDMI bits: */
 #define   SDVO_COLOR_FORMAT_8bpc		(0 << 26)
+#define   SDVO_COLOR_FORMAT_MASK		(7 << 26)
 #define   SDVO_ENCODING_SDVO			(0 << 10)
 #define   SDVO_ENCODING_HDMI			(2 << 10)
 #define   HDMI_MODE_SELECT_HDMI			(1 << 9) /* HDMI only */
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index ea9022e..be99cf2 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -89,6 +89,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_crt *crt = intel_encoder_to_crt(encoder);
 	u32 tmp, flags = 0;
+	int dotclock;
 
 	tmp = I915_READ(crt->adpa_reg);
 
@@ -103,6 +104,13 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 /* Note: The caller is required to filter out dpms modes not supported by the
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4f07292..31478b7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5047,6 +5047,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 						     DPLL_PORTB_READY_MASK);
 	}
 
+	i9xx_crtc_clock_get(crtc, pipe_config);
+
 	return true;
 }
 
@@ -6007,6 +6009,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->pixel_multiplier =
 			((tmp & PLL_REF_SDVO_HDMI_MULTIPLIER_MASK)
 			 >> PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT) + 1;
+
+		ironlake_crtc_clock_get(crtc, pipe_config);
 	} else {
 		pipe_config->pixel_multiplier = 1;
 	}
@@ -7416,7 +7420,12 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 		i9xx_clock(refclk, &clock);
 	}
 
-	pipe_config->adjusted_mode.clock = clock.dot;
+	/*
+	 * This value includes pixel_multiplier. We will use
+	 * port_clock to compute adjusted_mode.clock in the
+	 * encoder's get_config() function.
+	 */
+	pipe_config->port_clock = clock.dot;
 }
 
 int intel_dotclock_calculate(int link_freq,
@@ -7444,6 +7453,9 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	int link_freq;
 
+	/* read out port_clock from the DPLL */
+	i9xx_crtc_clock_get(crtc, pipe_config);
+
 	/*
 	 * We need to get the FDI or DP link clock here to derive
 	 * the M/N dividers.
@@ -7454,6 +7466,12 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
 	 */
 	link_freq = intel_fdi_link_freq(dev) * 10000;
 
+	/*
+	 * This value does not include pixel_multiplier.
+	 * We will check that port_clock and adjusted_mode.clock
+	 * agree once we know their relationship in the encoder's
+	 * get_config() function.
+	 */
 	pipe_config->adjusted_mode.clock =
 		intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
 }
@@ -8867,9 +8885,6 @@ check_crtc_state(struct drm_device *dev)
 				encoder->get_config(encoder, &pipe_config);
 		}
 
-		if (dev_priv->display.get_clock)
-			dev_priv->display.get_clock(crtc, &pipe_config);
-
 		WARN(crtc->active != active,
 		     "crtc active state doesn't match with hw state "
 		     "(expected %i, found %i)\n", crtc->active, active);
@@ -8944,6 +8959,18 @@ intel_modeset_check_state(struct drm_device *dev)
 	check_shared_dpll_state(dev);
 }
 
+void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
+				     int dotclock)
+{
+	/*
+	 * FDI already provided one idea for the dotclock.
+	 * Yell if the encoder disagrees.
+	 */
+	WARN(!intel_fuzzy_clock_check(pipe_config->adjusted_mode.clock, dotclock),
+	     "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
+	     pipe_config->adjusted_mode.clock, dotclock);
+}
+
 static int __intel_set_mode(struct drm_crtc *crtc,
 			    struct drm_display_mode *mode,
 			    int x, int y, struct drm_framebuffer *fb)
@@ -9895,7 +9922,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
-		dev_priv->display.get_clock = ironlake_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
@@ -9903,7 +9929,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
-		dev_priv->display.get_clock = i9xx_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -9911,7 +9936,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = i9xx_update_plane;
 	} else {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
-		dev_priv->display.get_clock = i9xx_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -10525,15 +10549,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			      pipe);
 	}
 
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
-			    base.head) {
-		if (!crtc->active)
-			continue;
-		if (dev_priv->display.get_clock)
-			dev_priv->display.get_clock(crtc,
-						    &crtc->config);
-	}
-
 	list_for_each_entry(connector, &dev->mode_config.connector_list,
 			    base.head) {
 		if (connector->get_hw_state(connector)) {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index be0449b..509a9e4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1372,6 +1372,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
+	int dotclock;
 
 	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
 		tmp = I915_READ(intel_dp->output_reg);
@@ -1403,12 +1404,20 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 
 	intel_dp_get_m_n(crtc, pipe_config);
 
-	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
+	if (port == PORT_A) {
 		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
 			pipe_config->port_clock = 162000;
 		else
 			pipe_config->port_clock = 270000;
 	}
+
+	dotclock = intel_dotclock_calculate(pipe_config->port_clock,
+					    &pipe_config->dp_m_n);
+
+	if (HAS_PCH_SPLIT(dev_priv->dev) && port != PORT_A)
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 static bool is_edp_psr(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4cf8898..6e1204f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -805,5 +805,7 @@ extern void intel_dp_get_m_n(struct intel_crtc *crtc,
 			     struct intel_crtc_config *pipe_config);
 extern int intel_dotclock_calculate(int link_freq,
 				    const struct intel_link_m_n *m_n);
+extern void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
+					    int dotclock);
 
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 406303b..92487ce 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -153,6 +153,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	pipe_config->adjusted_mode.clock = pipe_config->port_clock;
 }
 
 static void intel_disable_dvo(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4148cc8..16daa72 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -713,6 +713,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	u32 tmp, flags = 0;
+	int dotclock;
 
 	tmp = I915_READ(intel_hdmi->hdmi_reg);
 
@@ -727,6 +728,16 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	if ((tmp & SDVO_COLOR_FORMAT_MASK) == HDMI_COLOR_FORMAT_12bpc)
+		dotclock = pipe_config->port_clock * 2 / 3;
+	else
+		dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 static void intel_enable_hdmi(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 831a5c0..05e5485 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -92,6 +92,7 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 lvds_reg, tmp, flags = 0;
+	int dotclock;
 
 	if (HAS_PCH_SPLIT(dev))
 		lvds_reg = PCH_LVDS;
@@ -116,6 +117,13 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
 
 		pipe_config->gmch_pfit.control |= tmp & PANEL_8TO6_DITHER_ENABLE;
 	}
+
+	dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 /* The LVDS pin pair needs to be on before the DPLLs are enabled.
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 74042c6..7148fdd 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1319,6 +1319,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
 	struct intel_sdvo_dtd dtd;
 	int encoder_pixel_multiplier = 0;
+	int dotclock;
 	u32 flags = 0, sdvox;
 	u8 val;
 	bool ret;
@@ -1357,6 +1358,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 			 >> SDVO_PORT_MULTIPLY_SHIFT) + 1;
 	}
 
+	dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier;
+
+	if (HAS_PCH_SPLIT(dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
+
 	/* Cross check the port pixel multiplier with the sdvo encoder state. */
 	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_CLOCK_RATE_MULT,
 				 &val, 1)) {
-- 
1.8.1.5

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

[PATCH v4] drm/i915: Fix port_clock and adjusted_mode.clock readout all over

[ville.syrjala]

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

Now that adjusted_mode.clock no longer contains the pixel_multiplier, we
can kill the get_clock() callback and instead do the clock readout
in get_pipe_config().

Also i9xx_crtc_clock_get() can now extract the frequency of the PCH
DPLL, so use it to populate port_clock accurately for PCH encoders.
For DP in port A the encoder is still responsible for filling in
port_clock. The FDI adjusted_mode.clock extraction is kept in place
for some extra sanity checking, but we no longer need to pretend it's
also the port_clock.

In the encoder get_config() functions fill out adjusted_mode.clock
based on port_clock and other details such as the DP M/N values,
HDMI 12bpc and SDVO pixel_multiplier. For PCH encoders we will then
do an extra sanity check to make sure the dotclock we derived from
the FDI configuratiuon matches the one we derive from port_clock.

DVO doesn't exist on PCH platforms, so it doesn't need to anything
but assign adjusted_mode.clock=port_clock. And DDI is HSW only, so
none of the changes apply there.

v2: Use hdmi_reg color format to detect 12bpc HDMI case
v3: Set adjusted_mode.clock for LVDS too
v4: Rename ironlake_crtc_clock_get to ironlake_pch_clock_get,
    eliminate the useless link_freq variable.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 -
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_crt.c     |  8 ++++
 drivers/gpu/drm/i915/intel_display.c | 74 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_dp.c      | 11 +++++-
 drivers/gpu/drm/i915/intel_drv.h     |  2 +
 drivers/gpu/drm/i915/intel_dvo.c     |  2 +
 drivers/gpu/drm/i915/intel_hdmi.c    | 11 ++++++
 drivers/gpu/drm/i915/intel_lvds.c    |  8 ++++
 drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++++
 10 files changed, 86 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7caf71d..8b16d47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -371,7 +371,6 @@ struct drm_i915_display_funcs {
 	 * fills out the pipe-config with the hw state. */
 	bool (*get_pipe_config)(struct intel_crtc *,
 				struct intel_crtc_config *);
-	void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *);
 	int (*crtc_mode_set)(struct drm_crtc *crtc,
 			     int x, int y,
 			     struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index bcee89b..384adfb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2071,6 +2071,7 @@
 
 /* Gen 4 SDVO/HDMI bits: */
 #define   SDVO_COLOR_FORMAT_8bpc		(0 << 26)
+#define   SDVO_COLOR_FORMAT_MASK		(7 << 26)
 #define   SDVO_ENCODING_SDVO			(0 << 10)
 #define   SDVO_ENCODING_HDMI			(2 << 10)
 #define   HDMI_MODE_SELECT_HDMI			(1 << 9) /* HDMI only */
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index f5f89c3..6f101d5 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -89,6 +89,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_crt *crt = intel_encoder_to_crt(encoder);
 	u32 tmp, flags = 0;
+	int dotclock;
 
 	tmp = I915_READ(crt->adpa_reg);
 
@@ -103,6 +104,13 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 /* Note: The caller is required to filter out dpms modes not supported by the
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fed3804..42c7dc6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -47,8 +47,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
 
 static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 				struct intel_crtc_config *pipe_config);
-static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
-				    struct intel_crtc_config *pipe_config);
+static void ironlake_pch_clock_get(struct intel_crtc *crtc,
+				   struct intel_crtc_config *pipe_config);
 
 static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
 			  int x, int y, struct drm_framebuffer *old_fb);
@@ -5045,6 +5045,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 						     DPLL_PORTB_READY_MASK);
 	}
 
+	i9xx_crtc_clock_get(crtc, pipe_config);
+
 	return true;
 }
 
@@ -6004,6 +6006,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->pixel_multiplier =
 			((tmp & PLL_REF_SDVO_HDMI_MULTIPLIER_MASK)
 			 >> PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT) + 1;
+
+		ironlake_pch_clock_get(crtc, pipe_config);
 	} else {
 		pipe_config->pixel_multiplier = 1;
 	}
@@ -7411,7 +7415,12 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 		i9xx_clock(refclk, &clock);
 	}
 
-	pipe_config->adjusted_mode.clock = clock.dot;
+	/*
+	 * This value includes pixel_multiplier. We will use
+	 * port_clock to compute adjusted_mode.clock in the
+	 * encoder's get_config() function.
+	 */
+	pipe_config->port_clock = clock.dot;
 }
 
 int intel_dotclock_calculate(int link_freq,
@@ -7433,31 +7442,23 @@ int intel_dotclock_calculate(int link_freq,
 	return div_u64((u64)m_n->link_m * link_freq, m_n->link_n);
 }
 
-static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
-				    struct intel_crtc_config *pipe_config)
+static void ironlake_pch_clock_get(struct intel_crtc *crtc,
+				   struct intel_crtc_config *pipe_config)
 {
 	struct drm_device *dev = crtc->base.dev;
-	int link_freq;
+
+	/* read out port_clock from the DPLL */
+	i9xx_crtc_clock_get(crtc, pipe_config);
 
 	/*
-	 * We need to get the FDI or DP link clock here to derive
-	 * the M/N dividers.
-	 *
-	 * For FDI, we read it from the BIOS or use a fixed 2.7GHz.
-	 * For DP, it's either 1.62GHz or 2.7GHz.
-	 * We do our calculations in 10*MHz since we don't need much precison.
+	 * This value does not include pixel_multiplier.
+	 * We will check that port_clock and adjusted_mode.clock
+	 * agree once we know their relationship in the encoder's
+	 * get_config() function.
 	 */
-	if (pipe_config->has_pch_encoder) {
-		link_freq = intel_fdi_link_freq(dev) * 10000;
-
-		pipe_config->adjusted_mode.clock =
-			intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
-	} else {
-		link_freq = pipe_config->port_clock;
-
-		pipe_config->adjusted_mode.clock =
-			intel_dotclock_calculate(link_freq, &pipe_config->dp_m_n);
-	}
+	pipe_config->adjusted_mode.clock =
+		intel_dotclock_calculate(intel_fdi_link_freq(dev) * 10000,
+					 &pipe_config->fdi_m_n);
 }
 
 /** Returns the currently programmed mode of the given pipe. */
@@ -8873,9 +8874,6 @@ check_crtc_state(struct drm_device *dev)
 				encoder->get_config(encoder, &pipe_config);
 		}
 
-		if (dev_priv->display.get_clock)
-			dev_priv->display.get_clock(crtc, &pipe_config);
-
 		WARN(crtc->active != active,
 		     "crtc active state doesn't match with hw state "
 		     "(expected %i, found %i)\n", crtc->active, active);
@@ -8950,6 +8948,18 @@ intel_modeset_check_state(struct drm_device *dev)
 	check_shared_dpll_state(dev);
 }
 
+void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
+				     int dotclock)
+{
+	/*
+	 * FDI already provided one idea for the dotclock.
+	 * Yell if the encoder disagrees.
+	 */
+	WARN(!intel_fuzzy_clock_check(pipe_config->adjusted_mode.clock, dotclock),
+	     "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
+	     pipe_config->adjusted_mode.clock, dotclock);
+}
+
 static int __intel_set_mode(struct drm_crtc *crtc,
 			    struct drm_display_mode *mode,
 			    int x, int y, struct drm_framebuffer *fb)
@@ -9901,7 +9911,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
-		dev_priv->display.get_clock = ironlake_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
@@ -9909,7 +9918,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
-		dev_priv->display.get_clock = i9xx_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -9917,7 +9925,6 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.update_plane = i9xx_update_plane;
 	} else {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
-		dev_priv->display.get_clock = i9xx_crtc_clock_get;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -10536,15 +10543,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			      pipe);
 	}
 
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
-			    base.head) {
-		if (!crtc->active)
-			continue;
-		if (dev_priv->display.get_clock)
-			dev_priv->display.get_clock(crtc,
-						    &crtc->config);
-	}
-
 	list_for_each_entry(connector, &dev->mode_config.connector_list,
 			    base.head) {
 		if (connector->get_hw_state(connector)) {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9ba697c..a7fa6fd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1417,6 +1417,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
+	int dotclock;
 
 	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
 		tmp = I915_READ(intel_dp->output_reg);
@@ -1448,12 +1449,20 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 
 	intel_dp_get_m_n(crtc, pipe_config);
 
-	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
+	if (port == PORT_A) {
 		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
 			pipe_config->port_clock = 162000;
 		else
 			pipe_config->port_clock = 270000;
 	}
+
+	dotclock = intel_dotclock_calculate(pipe_config->port_clock,
+					    &pipe_config->dp_m_n);
+
+	if (HAS_PCH_SPLIT(dev_priv->dev) && port != PORT_A)
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 static bool is_edp_psr(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6b97ac1..338222c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -807,5 +807,7 @@ extern void intel_dp_get_m_n(struct intel_crtc *crtc,
 			     struct intel_crtc_config *pipe_config);
 extern int intel_dotclock_calculate(int link_freq,
 				    const struct intel_link_m_n *m_n);
+extern void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
+					    int dotclock);
 
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 55cec38..ff86c36 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -153,6 +153,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	pipe_config->adjusted_mode.clock = pipe_config->port_clock;
 }
 
 static void intel_disable_dvo(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 70c716e..17b2d7e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -713,6 +713,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	u32 tmp, flags = 0;
+	int dotclock;
 
 	tmp = I915_READ(intel_hdmi->hdmi_reg);
 
@@ -727,6 +728,16 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 		flags |= DRM_MODE_FLAG_NVSYNC;
 
 	pipe_config->adjusted_mode.flags |= flags;
+
+	if ((tmp & SDVO_COLOR_FORMAT_MASK) == HDMI_COLOR_FORMAT_12bpc)
+		dotclock = pipe_config->port_clock * 2 / 3;
+	else
+		dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 static void intel_enable_hdmi(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 831a5c0..05e5485 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -92,6 +92,7 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 lvds_reg, tmp, flags = 0;
+	int dotclock;
 
 	if (HAS_PCH_SPLIT(dev))
 		lvds_reg = PCH_LVDS;
@@ -116,6 +117,13 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
 
 		pipe_config->gmch_pfit.control |= tmp & PANEL_8TO6_DITHER_ENABLE;
 	}
+
+	dotclock = pipe_config->port_clock;
+
+	if (HAS_PCH_SPLIT(dev_priv->dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
 }
 
 /* The LVDS pin pair needs to be on before the DPLLs are enabled.
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index ebfd513..91aea9e 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1325,6 +1325,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
 	struct intel_sdvo_dtd dtd;
 	int encoder_pixel_multiplier = 0;
+	int dotclock;
 	u32 flags = 0, sdvox;
 	u8 val;
 	bool ret;
@@ -1363,6 +1364,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 			 >> SDVO_PORT_MULTIPLY_SHIFT) + 1;
 	}
 
+	dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier;
+
+	if (HAS_PCH_SPLIT(dev))
+		ironlake_check_encoder_dotclock(pipe_config, dotclock);
+
+	pipe_config->adjusted_mode.clock = dotclock;
+
 	/* Cross check the port pixel multiplier with the sdvo encoder state. */
 	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_CLOCK_RATE_MULT,
 				 &val, 1)) {
-- 
1.8.1.5

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

Re: [PATCH v4] drm/i915: Fix port_clock and adjusted_mode.clock readout all over

[Jani Nikula]

On Fri, 13 Sep 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that adjusted_mode.clock no longer contains the pixel_multiplier, we
> can kill the get_clock() callback and instead do the clock readout
> in get_pipe_config().
>
> Also i9xx_crtc_clock_get() can now extract the frequency of the PCH
> DPLL, so use it to populate port_clock accurately for PCH encoders.
> For DP in port A the encoder is still responsible for filling in
> port_clock. The FDI adjusted_mode.clock extraction is kept in place
> for some extra sanity checking, but we no longer need to pretend it's
> also the port_clock.
>
> In the encoder get_config() functions fill out adjusted_mode.clock
> based on port_clock and other details such as the DP M/N values,
> HDMI 12bpc and SDVO pixel_multiplier. For PCH encoders we will then
> do an extra sanity check to make sure the dotclock we derived from
> the FDI configuratiuon matches the one we derive from port_clock.
>
> DVO doesn't exist on PCH platforms, so it doesn't need to anything
> but assign adjusted_mode.clock=port_clock. And DDI is HSW only, so
> none of the changes apply there.
>
> v2: Use hdmi_reg color format to detect 12bpc HDMI case
> v3: Set adjusted_mode.clock for LVDS too
> v4: Rename ironlake_crtc_clock_get to ironlake_pch_clock_get,
>     eliminate the useless link_freq variable.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Daniel, I think the patch is right, but I feel it deserves another set
of eyeballs.

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


> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 -
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_crt.c     |  8 ++++
>  drivers/gpu/drm/i915/intel_display.c | 74 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_dp.c      | 11 +++++-
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>  drivers/gpu/drm/i915/intel_dvo.c     |  2 +
>  drivers/gpu/drm/i915/intel_hdmi.c    | 11 ++++++
>  drivers/gpu/drm/i915/intel_lvds.c    |  8 ++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++++
>  10 files changed, 86 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7caf71d..8b16d47 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -371,7 +371,6 @@ struct drm_i915_display_funcs {
>  	 * fills out the pipe-config with the hw state. */
>  	bool (*get_pipe_config)(struct intel_crtc *,
>  				struct intel_crtc_config *);
> -	void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *);
>  	int (*crtc_mode_set)(struct drm_crtc *crtc,
>  			     int x, int y,
>  			     struct drm_framebuffer *old_fb);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bcee89b..384adfb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2071,6 +2071,7 @@
>  
>  /* Gen 4 SDVO/HDMI bits: */
>  #define   SDVO_COLOR_FORMAT_8bpc		(0 << 26)
> +#define   SDVO_COLOR_FORMAT_MASK		(7 << 26)
>  #define   SDVO_ENCODING_SDVO			(0 << 10)
>  #define   SDVO_ENCODING_HDMI			(2 << 10)
>  #define   HDMI_MODE_SELECT_HDMI			(1 << 9) /* HDMI only */
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index f5f89c3..6f101d5 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -89,6 +89,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>  	struct intel_crt *crt = intel_encoder_to_crt(encoder);
>  	u32 tmp, flags = 0;
> +	int dotclock;
>  
>  	tmp = I915_READ(crt->adpa_reg);
>  
> @@ -103,6 +104,13 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	dotclock = pipe_config->port_clock;
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  /* Note: The caller is required to filter out dpms modes not supported by the
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fed3804..42c7dc6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -47,8 +47,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
>  
>  static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  				struct intel_crtc_config *pipe_config);
> -static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> -				    struct intel_crtc_config *pipe_config);
> +static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> +				   struct intel_crtc_config *pipe_config);
>  
>  static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
>  			  int x, int y, struct drm_framebuffer *old_fb);
> @@ -5045,6 +5045,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  						     DPLL_PORTB_READY_MASK);
>  	}
>  
> +	i9xx_crtc_clock_get(crtc, pipe_config);
> +
>  	return true;
>  }
>  
> @@ -6004,6 +6006,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier =
>  			((tmp & PLL_REF_SDVO_HDMI_MULTIPLIER_MASK)
>  			 >> PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT) + 1;
> +
> +		ironlake_pch_clock_get(crtc, pipe_config);
>  	} else {
>  		pipe_config->pixel_multiplier = 1;
>  	}
> @@ -7411,7 +7415,12 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  		i9xx_clock(refclk, &clock);
>  	}
>  
> -	pipe_config->adjusted_mode.clock = clock.dot;
> +	/*
> +	 * This value includes pixel_multiplier. We will use
> +	 * port_clock to compute adjusted_mode.clock in the
> +	 * encoder's get_config() function.
> +	 */
> +	pipe_config->port_clock = clock.dot;
>  }
>  
>  int intel_dotclock_calculate(int link_freq,
> @@ -7433,31 +7442,23 @@ int intel_dotclock_calculate(int link_freq,
>  	return div_u64((u64)m_n->link_m * link_freq, m_n->link_n);
>  }
>  
> -static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
> -				    struct intel_crtc_config *pipe_config)
> +static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> +				   struct intel_crtc_config *pipe_config)
>  {
>  	struct drm_device *dev = crtc->base.dev;
> -	int link_freq;
> +
> +	/* read out port_clock from the DPLL */
> +	i9xx_crtc_clock_get(crtc, pipe_config);
>  
>  	/*
> -	 * We need to get the FDI or DP link clock here to derive
> -	 * the M/N dividers.
> -	 *
> -	 * For FDI, we read it from the BIOS or use a fixed 2.7GHz.
> -	 * For DP, it's either 1.62GHz or 2.7GHz.
> -	 * We do our calculations in 10*MHz since we don't need much precison.
> +	 * This value does not include pixel_multiplier.
> +	 * We will check that port_clock and adjusted_mode.clock
> +	 * agree once we know their relationship in the encoder's
> +	 * get_config() function.
>  	 */
> -	if (pipe_config->has_pch_encoder) {
> -		link_freq = intel_fdi_link_freq(dev) * 10000;
> -
> -		pipe_config->adjusted_mode.clock =
> -			intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
> -	} else {
> -		link_freq = pipe_config->port_clock;
> -
> -		pipe_config->adjusted_mode.clock =
> -			intel_dotclock_calculate(link_freq, &pipe_config->dp_m_n);
> -	}
> +	pipe_config->adjusted_mode.clock =
> +		intel_dotclock_calculate(intel_fdi_link_freq(dev) * 10000,
> +					 &pipe_config->fdi_m_n);
>  }
>  
>  /** Returns the currently programmed mode of the given pipe. */
> @@ -8873,9 +8874,6 @@ check_crtc_state(struct drm_device *dev)
>  				encoder->get_config(encoder, &pipe_config);
>  		}
>  
> -		if (dev_priv->display.get_clock)
> -			dev_priv->display.get_clock(crtc, &pipe_config);
> -
>  		WARN(crtc->active != active,
>  		     "crtc active state doesn't match with hw state "
>  		     "(expected %i, found %i)\n", crtc->active, active);
> @@ -8950,6 +8948,18 @@ intel_modeset_check_state(struct drm_device *dev)
>  	check_shared_dpll_state(dev);
>  }
>  
> +void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> +				     int dotclock)
> +{
> +	/*
> +	 * FDI already provided one idea for the dotclock.
> +	 * Yell if the encoder disagrees.
> +	 */
> +	WARN(!intel_fuzzy_clock_check(pipe_config->adjusted_mode.clock, dotclock),
> +	     "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
> +	     pipe_config->adjusted_mode.clock, dotclock);
> +}
> +
>  static int __intel_set_mode(struct drm_crtc *crtc,
>  			    struct drm_display_mode *mode,
>  			    int x, int y, struct drm_framebuffer *fb)
> @@ -9901,7 +9911,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = ironlake_update_plane;
>  	} else if (HAS_PCH_SPLIT(dev)) {
>  		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
> -		dev_priv->display.get_clock = ironlake_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
>  		dev_priv->display.crtc_enable = ironlake_crtc_enable;
>  		dev_priv->display.crtc_disable = ironlake_crtc_disable;
> @@ -9909,7 +9918,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = ironlake_update_plane;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> -		dev_priv->display.get_clock = i9xx_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
>  		dev_priv->display.crtc_enable = valleyview_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -9917,7 +9925,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = i9xx_update_plane;
>  	} else {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> -		dev_priv->display.get_clock = i9xx_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
>  		dev_priv->display.crtc_enable = i9xx_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -10536,15 +10543,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			      pipe);
>  	}
>  
> -	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> -			    base.head) {
> -		if (!crtc->active)
> -			continue;
> -		if (dev_priv->display.get_clock)
> -			dev_priv->display.get_clock(crtc,
> -						    &crtc->config);
> -	}
> -
>  	list_for_each_entry(connector, &dev->mode_config.connector_list,
>  			    base.head) {
>  		if (connector->get_hw_state(connector)) {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9ba697c..a7fa6fd 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1417,6 +1417,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> +	int dotclock;
>  
>  	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
>  		tmp = I915_READ(intel_dp->output_reg);
> @@ -1448,12 +1449,20 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  
>  	intel_dp_get_m_n(crtc, pipe_config);
>  
> -	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
> +	if (port == PORT_A) {
>  		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
>  			pipe_config->port_clock = 162000;
>  		else
>  			pipe_config->port_clock = 270000;
>  	}
> +
> +	dotclock = intel_dotclock_calculate(pipe_config->port_clock,
> +					    &pipe_config->dp_m_n);
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev) && port != PORT_A)
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  static bool is_edp_psr(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6b97ac1..338222c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -807,5 +807,7 @@ extern void intel_dp_get_m_n(struct intel_crtc *crtc,
>  			     struct intel_crtc_config *pipe_config);
>  extern int intel_dotclock_calculate(int link_freq,
>  				    const struct intel_link_m_n *m_n);
> +extern void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> +					    int dotclock);
>  
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 55cec38..ff86c36 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -153,6 +153,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	pipe_config->adjusted_mode.clock = pipe_config->port_clock;
>  }
>  
>  static void intel_disable_dvo(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70c716e..17b2d7e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -713,6 +713,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>  	u32 tmp, flags = 0;
> +	int dotclock;
>  
>  	tmp = I915_READ(intel_hdmi->hdmi_reg);
>  
> @@ -727,6 +728,16 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	if ((tmp & SDVO_COLOR_FORMAT_MASK) == HDMI_COLOR_FORMAT_12bpc)
> +		dotclock = pipe_config->port_clock * 2 / 3;
> +	else
> +		dotclock = pipe_config->port_clock;
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  static void intel_enable_hdmi(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 831a5c0..05e5485 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -92,6 +92,7 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 lvds_reg, tmp, flags = 0;
> +	int dotclock;
>  
>  	if (HAS_PCH_SPLIT(dev))
>  		lvds_reg = PCH_LVDS;
> @@ -116,6 +117,13 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
>  
>  		pipe_config->gmch_pfit.control |= tmp & PANEL_8TO6_DITHER_ENABLE;
>  	}
> +
> +	dotclock = pipe_config->port_clock;
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  /* The LVDS pin pair needs to be on before the DPLLs are enabled.
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index ebfd513..91aea9e 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1325,6 +1325,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
>  	struct intel_sdvo_dtd dtd;
>  	int encoder_pixel_multiplier = 0;
> +	int dotclock;
>  	u32 flags = 0, sdvox;
>  	u8 val;
>  	bool ret;
> @@ -1363,6 +1364,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  			 >> SDVO_PORT_MULTIPLY_SHIFT) + 1;
>  	}
>  
> +	dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier;
> +
> +	if (HAS_PCH_SPLIT(dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
> +
>  	/* Cross check the port pixel multiplier with the sdvo encoder state. */
>  	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_CLOCK_RATE_MULT,
>  				 &val, 1)) {
> -- 
> 1.8.1.5
>

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

[PATCH v2 10/11] drm/i915: Add PIPE_CONF_CHECK_CLOCK_FUZZY()

[ville.syrjala]

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

Add a new pipe config check macro PIPE_CONF_CHECK_CLOCK_FUZZY() to make
it trivial and error proof to compare clocks in a fuzzy manner.

v2: Drop extra curly braces

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 26f0269..5117954 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8659,6 +8659,15 @@ intel_pipe_config_compare(struct drm_device *dev,
 		return false; \
 	}
 
+#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) \
+	if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
+		DRM_ERROR("mismatch in " #name " " \
+			  "(expected %i, found %i)\n", \
+			  current_config->name, \
+			  pipe_config->name); \
+		return false; \
+	}
+
 #define PIPE_CONF_QUIRK(quirk)	\
 	((current_config->quirks | pipe_config->quirks) & (quirk))
 
@@ -8731,21 +8740,15 @@ intel_pipe_config_compare(struct drm_device *dev,
 	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
 		PIPE_CONF_CHECK_I(pipe_bpp);
 
+	if (!IS_HASWELL(dev))
+		PIPE_CONF_CHECK_CLOCK_FUZZY(adjusted_mode.clock);
+
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
 #undef PIPE_CONF_CHECK_FLAGS
+#undef PIPE_CONF_CHECK_CLOCK_FUZZY
 #undef PIPE_CONF_QUIRK
 
-	if (!IS_HASWELL(dev)) {
-		if (!intel_fuzzy_clock_check(current_config->adjusted_mode.clock,
-					     pipe_config->adjusted_mode.clock)) {
-			DRM_ERROR("mismatch in clock (expected %d, found %d)\n",
-				  current_config->adjusted_mode.clock,
-				  pipe_config->adjusted_mode.clock);
-			return false;
-		}
-	}
-
 	return true;
 }
 
-- 
1.8.1.5

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

[PATCH v2 11/11] drm/i915: Add fuzzy clock check for port_clock

[ville.syrjala]

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

Check and dump for port_clock.

v2: Also dump port_clock

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5117954..5f49685 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8311,6 +8311,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	drm_mode_debug_printmodeline(&pipe_config->requested_mode);
 	DRM_DEBUG_KMS("adjusted mode:\n");
 	drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
+	DRM_DEBUG_KMS("port clock: %d\n", pipe_config->port_clock);
 	DRM_DEBUG_KMS("gmch pfit: control: 0x%08x, ratios: 0x%08x, lvds border: 0x%08x\n",
 		      pipe_config->gmch_pfit.control,
 		      pipe_config->gmch_pfit.pgm_ratios,
@@ -8740,8 +8741,10 @@ intel_pipe_config_compare(struct drm_device *dev,
 	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
 		PIPE_CONF_CHECK_I(pipe_bpp);
 
-	if (!IS_HASWELL(dev))
+	if (!IS_HASWELL(dev)) {
 		PIPE_CONF_CHECK_CLOCK_FUZZY(adjusted_mode.clock);
+		PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
+	}
 
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
-- 
1.8.1.5

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

Re: [PATCH v2 11/11] drm/i915: Add fuzzy clock check for port_clock

[Daniel Vetter]

On Fri, Sep 06, 2013 at 11:29:08PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check and dump for port_clock.
> 
> v2: Also dump port_clock
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, merged the remaining patches from this series, hopefully without
fumbling the job. Will continue with the next one.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/11] drm/i915: adjusted_mode.clock vs. port_clock v3

[Daniel Vetter]

On Fri, Sep 06, 2013 at 11:28:57PM +0300, ville.syrjala@linux.intel.com wrote:
> Another day, another attempt.
> 
> This series replaces the earlier attempt:
> [PATCH 0/8] drm/i915: adjusted_mode.clock vs. port_clock v2 
> 
> The main new idea is to make it the encoders' responsibility to calculate
> adjusted_mode.clock. In order to do that I had to add DP M/N extraction and
> i9xx_crtc_clock_get() had to be improved so that it can dig out the PCH DPLL
> frequency. The rest is more or less the same old.
> 
> As a bonus we now get to cross check the FDI's idea of dotclock against the
> encoder's idea.
> 
> Also now CTG too can calculate the dotclock from the DP port_clock. I don't
> think that could have worked before as the DPLL was running at the DP link
> frequency, so we read that out into adjusted_mode.clock and then compared
> it with whatever the user requested. Not the same thing at all. But now it
> should just work (tm).
> 
> I gave it a quick whirl on my ILK eDP + DP, and IVB HDMI and DP. Neither
> machine started to yell at me, so I think it's in a semi-decent shape at
> least. Unfortunately I don't have a CTG w/ DP so I can't test that part,
> nor do I have any SDVO/DVO hardware.

Ok, I've read through this a bit and commented on the oddball cases where
I expect pipe config mismatches might happen. Otherwise I guess we can
just merge this and then fix any oddball fallout - it's early for 3.13
after all ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch