[PATCH 1/3] drm/i915: get basic encoder state before reading CRTC state

[PATCH 1/3] drm/i915: get basic encoder state before reading CRTC state

[Jesse Barnes]

In DDI configs, we need to get the encoder to CRTC mapping early on so
we can read out and calculate the clock state correctly, as it depends
on the port.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 59 +++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 74137d5..92f46ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9476,6 +9476,22 @@ check_crtc_state(struct drm_device *dev)
 		     "crtc's computed enabled state doesn't match tracked enabled state "
 		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
 
+
+		list_for_each_entry(encoder, &dev->mode_config.encoder_list,
+				    base.head) {
+			/* Get encoder->crtc mapping */
+			struct intel_crtc *tmp_crtc;
+			enum pipe pipe;
+			if (encoder->get_hw_state(encoder, &pipe)) {
+				tmp_crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+				if (tmp_crtc == crtc) {
+					encoder->base.crtc = &tmp_crtc->base;
+					encoder->get_config(encoder,
+							    &pipe_config);
+				}
+			}
+		}
+
 		active = dev_priv->display.get_pipe_config(crtc,
 							   &pipe_config);
 
@@ -11109,9 +11125,26 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	int i;
 
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
-			    base.head) {
+			    base.head)
 		memset(&crtc->config, 0, sizeof(crtc->config));
 
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
+			    base.head) {
+		pipe = 0;
+
+		if (encoder->get_hw_state(encoder, &pipe)) {
+			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+			encoder->base.crtc = &crtc->base;
+			drm_mode_debug_printmodeline(&crtc->config.adjusted_mode);
+		} else {
+			encoder->base.crtc = NULL;
+		}
+
+		encoder->connectors_active = false;
+	}
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
+			    base.head) {
 		crtc->active = dev_priv->display.get_pipe_config(crtc,
 								 &crtc->config);
 
@@ -11145,22 +11178,18 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
 			    base.head) {
-		pipe = 0;
-
-		if (encoder->get_hw_state(encoder, &pipe)) {
-			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
-			encoder->base.crtc = &crtc->base;
+		if (encoder->base.crtc) {
+			crtc = to_intel_crtc(encoder->base.crtc);
 			encoder->get_config(encoder, &crtc->config);
-		} else {
-			encoder->base.crtc = NULL;
-		}
 
-		encoder->connectors_active = false;
-		DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
-			      encoder->base.base.id,
-			      drm_get_encoder_name(&encoder->base),
-			      encoder->base.crtc ? "enabled" : "disabled",
-			      pipe_name(pipe));
+			DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c, dp: %s\n",
+				      encoder->base.base.id,
+				      drm_get_encoder_name(&encoder->base),
+				      encoder->base.crtc ? "enabled" : "disabled",
+				      pipe_name(crtc->pipe),
+				      crtc->config.has_dp_encoder ? "yes" : "no");
+			drm_mode_debug_printmodeline(&crtc->config.adjusted_mode);
+		}
 	}
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list,
-- 
1.8.3.2

[PATCH 2/3] drm/i915: clock readout support for DDI

[Jesse Barnes]

Read out and calculate the port and pixel clocks on DDI configs as well.
This means we have to grab the DP divider values and look at the port
mapping to figure out which clock select reg to read out.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h      |  6 ++++
 drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 69 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 4 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a699efd..644e4f9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5318,8 +5318,13 @@
 #define  WRPLL_PLL_SELECT_LCPLL_2700	(0x03<<28)
 /* WRPLL divider programming */
 #define  WRPLL_DIVIDER_REFERENCE(x)	((x)<<0)
+#define  WRPLL_DIVIDER_REF_MASK		(0xff)
 #define  WRPLL_DIVIDER_POST(x)		((x)<<8)
+#define  WRPLL_DIVIDER_POST_MASK	(0x3f<<8)
+#define  WRPLL_DIVIDER_POST_SHIFT	8
 #define  WRPLL_DIVIDER_FEEDBACK(x)	((x)<<16)
+#define  WRPLL_DIVIDER_FB_SHIFT		16
+#define  WRPLL_DIVIDER_FB_MASK		(0xff<<16)
 
 /* Port clock selection */
 #define PORT_CLK_SEL_A			0x46100
@@ -5332,6 +5337,7 @@
 #define  PORT_CLK_SEL_WRPLL1		(4<<29)
 #define  PORT_CLK_SEL_WRPLL2		(5<<29)
 #define  PORT_CLK_SEL_NONE		(7<<29)
+#define  PORT_CLK_SEL_MASK		(7<<29)
 
 /* Transcoder clock selection */
 #define TRANS_CLK_SEL_A			0x46140
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1488b28..f3d7b42 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -413,7 +413,7 @@ static void intel_ddi_mode_set(struct intel_encoder *encoder)
 	}
 }
 
-static struct intel_encoder *
+struct intel_encoder *
 intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 92f46ad..7a9ff57 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -48,6 +48,8 @@ static void i9xx_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 void haswell_ddi_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);
@@ -6994,10 +6996,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
 		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
 					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
-
 		ironlake_get_fdi_m_n_config(crtc, pipe_config);
 	}
 
+	intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
+				     &pipe_config->dp_m_n);
+	haswell_ddi_clock_get(crtc, pipe_config);
+
 	intel_get_pipe_timings(crtc, pipe_config);
 
 	pfit_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
@@ -8034,6 +8039,68 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 					 &pipe_config->fdi_m_n);
 }
 
+#define LC_FREQ 2700
+
+static int intel_ddi_calc_wrpll_link(u32 wrpll)
+{
+	int n, p, r;
+
+	r = wrpll & WRPLL_DIVIDER_REF_MASK;
+	p = (wrpll & WRPLL_DIVIDER_POST_MASK) >> WRPLL_DIVIDER_POST_SHIFT;
+	n = (wrpll & WRPLL_DIVIDER_FB_MASK) >> WRPLL_DIVIDER_FB_SHIFT;
+
+	return (LC_FREQ * n) / (p * r);
+}
+
+static void haswell_ddi_clock_get(struct intel_crtc *crtc,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct intel_encoder *intel_encoder =
+		intel_ddi_get_crtc_encoder(&crtc->base);
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	int link_clock;
+	u32 val, pll;
+
+	val = I915_READ(PORT_CLK_SEL(port));
+	switch (val & PORT_CLK_SEL_MASK) {
+	case PORT_CLK_SEL_LCPLL_810:
+		link_clock = 81000;
+		break;
+	case PORT_CLK_SEL_LCPLL_1350:
+		link_clock = 135000;
+		break;
+	case PORT_CLK_SEL_LCPLL_2700:
+		link_clock = 270000;
+		break;
+	case PORT_CLK_SEL_WRPLL1:
+		pll = I915_READ(WRPLL_CTL1);
+		link_clock = intel_ddi_calc_wrpll_link(pll);
+		break;
+	case PORT_CLK_SEL_WRPLL2:
+		pll = I915_READ(WRPLL_CTL2);
+		link_clock = intel_ddi_calc_wrpll_link(pll);
+		break;
+	case PORT_CLK_SEL_SPLL:
+		link_clock = 135000;
+		break;
+	default:
+		WARN(1, "bad port clock sel\n");
+		return;
+	}
+
+	if (crtc->config.has_pch_encoder)
+		pipe_config->adjusted_mode.crtc_clock =
+			intel_dotclock_calculate(link_clock, &pipe_config->fdi_m_n);
+	else
+		pipe_config->adjusted_mode.crtc_clock =
+			intel_dotclock_calculate(link_clock, &pipe_config->dp_m_n);
+
+	pipe_config->port_clock = link_clock * 2;
+	pipe_config->adjusted_mode.crtc_clock *= 2;
+
+}
+
 /** Returns the currently programmed mode of the given pipe. */
 struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 					     struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 46aea6c..7bfc19a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -604,6 +604,7 @@ void intel_prepare_ddi(struct drm_device *dev);
 void hsw_fdi_link_train(struct drm_crtc *crtc);
 void intel_ddi_init(struct drm_device *dev, enum port port);
 enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
+struct intel_encoder *intel_ddi_get_crtc_encoder(struct drm_crtc *crtc);
 bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
 int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv);
 void intel_ddi_pll_init(struct drm_device *dev);
-- 
1.8.3.2

[PATCH 3/3] drm/i915: always check clocks when comparing pipe configs

[Jesse Barnes]

Now that we have DDI support, we can check these all the time.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7a9ff57..0efa8b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9418,10 +9418,8 @@ intel_pipe_config_compare(struct drm_device *dev,
 	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
 		PIPE_CONF_CHECK_I(pipe_bpp);
 
-	if (!HAS_DDI(dev)) {
-		PIPE_CONF_CHECK_CLOCK_FUZZY(adjusted_mode.crtc_clock);
-		PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
-	}
+	PIPE_CONF_CHECK_CLOCK_FUZZY(adjusted_mode.crtc_clock);
+	PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
 
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
-- 
1.8.3.2

Re: [PATCH 1/3] drm/i915: get basic encoder state before reading CRTC state

[Ville Syrjälä]

On Fri, Jan 17, 2014 at 01:16:56PM -0800, Jesse Barnes wrote:
> In DDI configs, we need to get the encoder to CRTC mapping early on so
> we can read out and calculate the clock state correctly, as it depends
> on the port.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 59 +++++++++++++++++++++++++++---------
>  1 file changed, 44 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 74137d5..92f46ad 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9476,6 +9476,22 @@ check_crtc_state(struct drm_device *dev)
>  		     "crtc's computed enabled state doesn't match tracked enabled state "
>  		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
>  
> +
> +		list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> +				    base.head) {
> +			/* Get encoder->crtc mapping */
> +			struct intel_crtc *tmp_crtc;
> +			enum pipe pipe;
> +			if (encoder->get_hw_state(encoder, &pipe)) {
> +				tmp_crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> +				if (tmp_crtc == crtc) {
> +					encoder->base.crtc = &tmp_crtc->base;
> +					encoder->get_config(encoder,
> +							    &pipe_config);
> +				}
> +			}
> +		}
> +
>  		active = dev_priv->display.get_pipe_config(crtc,
>  							   &pipe_config);
>  

NAK. This will break clock readout for every other platform. For most 
things we read out the .port_clock from the DPLL in .get_pipe_config()
(eDP port A being the excption where intel_dp_get_config() also fills
out .port_clock), and then the encoder .get_config() massages .port_clock
appropriately to figure out what .crtc_clock was supposed to be.

Maybe I'm missing something, but can't you just follow the eDP port A
route and simply stick your intel_ddi_clock_get() into
intel_ddi_get_config(), so that intel_ddi_get_config() is always
responsible for filling out both .port_clock and .crtc_clock?

> @@ -11109,9 +11125,26 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  	int i;
>  
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> -			    base.head) {
> +			    base.head)
>  		memset(&crtc->config, 0, sizeof(crtc->config));
>  
> +	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> +			    base.head) {
> +		pipe = 0;
> +
> +		if (encoder->get_hw_state(encoder, &pipe)) {
> +			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> +			encoder->base.crtc = &crtc->base;
> +			drm_mode_debug_printmodeline(&crtc->config.adjusted_mode);
> +		} else {
> +			encoder->base.crtc = NULL;
> +		}
> +
> +		encoder->connectors_active = false;
> +	}
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> +			    base.head) {
>  		crtc->active = dev_priv->display.get_pipe_config(crtc,
>  								 &crtc->config);
>  
> @@ -11145,22 +11178,18 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
>  			    base.head) {
> -		pipe = 0;
> -
> -		if (encoder->get_hw_state(encoder, &pipe)) {
> -			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> -			encoder->base.crtc = &crtc->base;
> +		if (encoder->base.crtc) {
> +			crtc = to_intel_crtc(encoder->base.crtc);
>  			encoder->get_config(encoder, &crtc->config);
> -		} else {
> -			encoder->base.crtc = NULL;
> -		}
>  
> -		encoder->connectors_active = false;
> -		DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
> -			      encoder->base.base.id,
> -			      drm_get_encoder_name(&encoder->base),
> -			      encoder->base.crtc ? "enabled" : "disabled",
> -			      pipe_name(pipe));
> +			DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c, dp: %s\n",
> +				      encoder->base.base.id,
> +				      drm_get_encoder_name(&encoder->base),
> +				      encoder->base.crtc ? "enabled" : "disabled",
> +				      pipe_name(crtc->pipe),
> +				      crtc->config.has_dp_encoder ? "yes" : "no");
> +			drm_mode_debug_printmodeline(&crtc->config.adjusted_mode);
> +		}
>  	}
>  
>  	list_for_each_entry(connector, &dev->mode_config.connector_list,
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/3] drm/i915: get basic encoder state before reading CRTC state

[Jesse Barnes]

On Sat, 18 Jan 2014 16:01:06 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Fri, Jan 17, 2014 at 01:16:56PM -0800, Jesse Barnes wrote:
> > In DDI configs, we need to get the encoder to CRTC mapping early on so
> > we can read out and calculate the clock state correctly, as it depends
> > on the port.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 59 +++++++++++++++++++++++++++---------
> >  1 file changed, 44 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 74137d5..92f46ad 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9476,6 +9476,22 @@ check_crtc_state(struct drm_device *dev)
> >  		     "crtc's computed enabled state doesn't match tracked enabled state "
> >  		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
> >  
> > +
> > +		list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> > +				    base.head) {
> > +			/* Get encoder->crtc mapping */
> > +			struct intel_crtc *tmp_crtc;
> > +			enum pipe pipe;
> > +			if (encoder->get_hw_state(encoder, &pipe)) {
> > +				tmp_crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > +				if (tmp_crtc == crtc) {
> > +					encoder->base.crtc = &tmp_crtc->base;
> > +					encoder->get_config(encoder,
> > +							    &pipe_config);
> > +				}
> > +			}
> > +		}
> > +
> >  		active = dev_priv->display.get_pipe_config(crtc,
> >  							   &pipe_config);
> >  
> 
> NAK. This will break clock readout for every other platform. For most 
> things we read out the .port_clock from the DPLL in .get_pipe_config()
> (eDP port A being the excption where intel_dp_get_config() also fills
> out .port_clock), and then the encoder .get_config() massages .port_clock
> appropriately to figure out what .crtc_clock was supposed to be.
> 
> Maybe I'm missing something, but can't you just follow the eDP port A
> route and simply stick your intel_ddi_clock_get() into
> intel_ddi_get_config(), so that intel_ddi_get_config() is always
> responsible for filling out both .port_clock and .crtc_clock?
> 

Yeah I knew this would cause trouble.  I might be able to get the clock
later, but I just need to make sure the CRTC to encoder mapping is set
up when I need it.

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