[PATCH] drm/i915/bxt: Fix DSI HW state readout

[PATCH] drm/i915/bxt: Fix DSI HW state readout

[Imre Deak]

Currently the machine hangs during booting while accessing the
BXT_MIPI_PORT_CTRL register during pipe HW state readout. After some
experimentation I found that the hang is caused by the DSI PLL being
disabled, or it being enabled but with an incorrect divider
configuration. Enabling the PLL got rid of the boot problem, so fix
this by checking the PLL enabled state/configuration before attempting
to read out the HW state.

Ville also found this dependency specified in BSpec, so I added a
reference to that too.

CC: Shashank Sharma <shashank.sharma@intel.com>
CC: Uma Shankar <uma.shankar@intel.com>
CC: Jani Nikula <jani.nikula@intel.com>
Fixes: c6c794a2fc5e ("drm/i915/bxt: Initialize MIPI DSI for BXT")
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
 drivers/gpu/drm/i915/intel_dsi.c     |  8 ++++++++
 drivers/gpu/drm/i915/intel_dsi.h     |  1 +
 drivers/gpu/drm/i915/intel_dsi_pll.c | 37 ++++++++++++++++++++++++++++++++++++
 5 files changed, 57 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f3ba43c..c839ce9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7811,9 +7811,11 @@ enum skl_disp_power_wells {
 #define  BXT_DSIC_16X_BY2		(1 << 10)
 #define  BXT_DSIC_16X_BY3		(2 << 10)
 #define  BXT_DSIC_16X_BY4		(3 << 10)
+#define  BXT_DSIC_16X_MASK		(3 << 10)
 #define  BXT_DSIA_16X_BY2		(1 << 8)
 #define  BXT_DSIA_16X_BY3		(2 << 8)
 #define  BXT_DSIA_16X_BY4		(3 << 8)
+#define  BXT_DSIA_16X_MASK		(3 << 8)
 #define  BXT_DSI_FREQ_SEL_SHIFT		8
 #define  BXT_DSI_FREQ_SEL_MASK		(0xF << BXT_DSI_FREQ_SEL_SHIFT)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 009b03b..d5dc6b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -36,6 +36,7 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "intel_dsi.h"
 #include "i915_trace.h"
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -9856,6 +9857,14 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
 
 	pipe_config->has_dsi_encoder = false;
 
+	/*
+	 * The PLL needs to be enabled with a valid divider configuration,
+	 * otherwise accessing DSI registers will hang the machine.
+	 * See BSpec North Display Engine registers/MIPI[BXT].
+	 */
+	if (!intel_dsi_pll_is_enabled(dev_priv))
+		return false;
+
 	for_each_port_masked(port, BIT(PORT_A) | BIT(PORT_C)) {
 		if (port == PORT_A)
 			cpu_transcoder = TRANSCODER_DSI_A;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 96ea3f7..656807f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -680,6 +680,14 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("\n");
 
+	/*
+	 * On Broxton the PLL needs to be enabled with a valid divider
+	 * configuration, otherwise accessing DSI registers will hang the
+	 * machine. See BSpec North Display Engine registers/MIPI[BXT].
+	 */
+	if (IS_BROXTON(dev_priv) && !intel_dsi_pll_is_enabled(dev_priv))
+		return false;
+
 	power_domain = intel_display_port_power_domain(encoder);
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index e582ef8..ec58ead 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -126,6 +126,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 	return container_of(encoder, struct intel_dsi, base.base);
 }
 
+bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
 extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
 extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
 extern u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index e3e343c..e5b0625 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -192,6 +192,33 @@ static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+static bool bxt_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
+{
+	bool enabled;
+	u32 val;
+	u32 mask;
+
+	mask = BXT_DSI_PLL_DO_ENABLE | BXT_DSI_PLL_LOCKED;
+	val = I915_READ(BXT_DSI_PLL_ENABLE);
+	enabled = (val & mask) == mask;
+
+	if (!enabled)
+		return false;
+
+	/*
+	 * Both dividers must be programmed with valid values even if only one
+	 * of the PLL is used. See BSpec/Broxton Clocks.
+	 */
+	val = I915_READ(BXT_DSI_PLL_CTL);
+	if (!(val & BXT_DSIA_16X_MASK) || !(val & BXT_DSIC_16X_MASK)) {
+		DRM_DEBUG_DRIVER("PLL is enabled with invalid divider settings "
+				 "(%08x)\n", val);
+		enabled = false;
+	}
+
+	return enabled;
+}
+
 static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -486,6 +513,16 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
 	DRM_DEBUG_KMS("DSI PLL locked\n");
 }
 
+bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
+{
+	if (IS_BROXTON(dev_priv))
+		return bxt_dsi_pll_is_enabled(dev_priv);
+
+	MISSING_CASE(INTEL_DEVID(dev_priv));
+
+	return false;
+}
+
 void intel_enable_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
-- 
2.5.0

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

[PATCH v2] drm/i915/bxt: Fix DSI HW state readout

[Imre Deak]

Currently the machine hangs during booting while accessing the
BXT_MIPI_PORT_CTRL register during pipe HW state readout. After some
experimentation I found that the hang is caused by the DSI PLL being
disabled, or it being enabled but with an incorrect divider
configuration. Enabling the PLL got rid of the boot problem, so fix
this by checking the PLL enabled state/configuration before attempting
to read out the HW state.

The DSI_PLL_ENABLE register is in the always-on power well, while the
BXT_DSI_PLL_CTL is in power well 0. This isn't exactly matched by the
transcoder power domain, but what we really need is just a runtime PM
reference, which is provided by any power domain.

Ville also found this dependency specified in BSpec, so I added a
reference to that too.

v2:
- Make sure we hold a power reference while accessing the PLL registers.

CC: Shashank Sharma <shashank.sharma@intel.com>
CC: Uma Shankar <uma.shankar@intel.com>
CC: Jani Nikula <jani.nikula@intel.com>
Fixes: c6c794a2fc5e ("drm/i915/bxt: Initialize MIPI DSI for BXT")
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_dsi.c     |  9 +++++++++
 drivers/gpu/drm/i915/intel_dsi.h     |  1 +
 drivers/gpu/drm/i915/intel_dsi_pll.c | 37 ++++++++++++++++++++++++++++++++++++
 5 files changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f3ba43c..c839ce9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7811,9 +7811,11 @@ enum skl_disp_power_wells {
 #define  BXT_DSIC_16X_BY2		(1 << 10)
 #define  BXT_DSIC_16X_BY3		(2 << 10)
 #define  BXT_DSIC_16X_BY4		(3 << 10)
+#define  BXT_DSIC_16X_MASK		(3 << 10)
 #define  BXT_DSIA_16X_BY2		(1 << 8)
 #define  BXT_DSIA_16X_BY3		(2 << 8)
 #define  BXT_DSIA_16X_BY4		(3 << 8)
+#define  BXT_DSIA_16X_MASK		(3 << 8)
 #define  BXT_DSI_FREQ_SEL_SHIFT		8
 #define  BXT_DSI_FREQ_SEL_MASK		(0xF << BXT_DSI_FREQ_SEL_SHIFT)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 009b03b..6fd94c4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -36,6 +36,7 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "intel_dsi.h"
 #include "i915_trace.h"
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -9852,10 +9853,12 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
 	enum intel_display_power_domain power_domain;
 	enum port port;
 	enum transcoder cpu_transcoder;
+	bool pll_enabled;
 	u32 tmp;
 
 	pipe_config->has_dsi_encoder = false;
 
+	pll_enabled = false;
 	for_each_port_masked(port, BIT(PORT_A) | BIT(PORT_C)) {
 		if (port == PORT_A)
 			cpu_transcoder = TRANSCODER_DSI_A;
@@ -9867,6 +9870,16 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
 			continue;
 		*power_domain_mask |= BIT(power_domain);
 
+		/*
+		 * The PLL needs to be enabled with a valid divider
+		 * configuration, otherwise accessing DSI registers will hang
+		 * the machine. See BSpec North Display Engine
+		 * registers/MIPI[BXT].
+		 */
+		pll_enabled = pll_enabled || intel_dsi_pll_is_enabled(dev_priv);
+		if (!pll_enabled)
+			break;
+
 		/* XXX: this works for video mode only */
 		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
 		if (!(tmp & DPI_ENABLE))
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 96ea3f7..0de74e1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -684,6 +684,14 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
 
+	/*
+	 * On Broxton the PLL needs to be enabled with a valid divider
+	 * configuration, otherwise accessing DSI registers will hang the
+	 * machine. See BSpec North Display Engine registers/MIPI[BXT].
+	 */
+	if (IS_BROXTON(dev_priv) && !intel_dsi_pll_is_enabled(dev_priv))
+		goto out_put_power;
+
 	/* XXX: this only works for one DSI output */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		i915_reg_t ctrl_reg = IS_BROXTON(dev) ?
@@ -726,6 +734,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 		break;
 	}
 
+out_put_power:
 	intel_display_power_put(dev_priv, power_domain);
 
 	return active;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index e582ef8..ec58ead 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -126,6 +126,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 	return container_of(encoder, struct intel_dsi, base.base);
 }
 
+bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
 extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
 extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
 extern u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index e3e343c..e5b0625 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -192,6 +192,33 @@ static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+static bool bxt_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
+{
+	bool enabled;
+	u32 val;
+	u32 mask;
+
+	mask = BXT_DSI_PLL_DO_ENABLE | BXT_DSI_PLL_LOCKED;
+	val = I915_READ(BXT_DSI_PLL_ENABLE);
+	enabled = (val & mask) == mask;
+
+	if (!enabled)
+		return false;
+
+	/*
+	 * Both dividers must be programmed with valid values even if only one
+	 * of the PLL is used. See BSpec/Broxton Clocks.
+	 */
+	val = I915_READ(BXT_DSI_PLL_CTL);
+	if (!(val & BXT_DSIA_16X_MASK) || !(val & BXT_DSIC_16X_MASK)) {
+		DRM_DEBUG_DRIVER("PLL is enabled with invalid divider settings "
+				 "(%08x)\n", val);
+		enabled = false;
+	}
+
+	return enabled;
+}
+
 static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -486,6 +513,16 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
 	DRM_DEBUG_KMS("DSI PLL locked\n");
 }
 
+bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
+{
+	if (IS_BROXTON(dev_priv))
+		return bxt_dsi_pll_is_enabled(dev_priv);
+
+	MISSING_CASE(INTEL_DEVID(dev_priv));
+
+	return false;
+}
+
 void intel_enable_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
-- 
2.5.0

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

Re: [PATCH v2] drm/i915/bxt: Fix DSI HW state readout

[Jani Nikula]

On Wed, 23 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> Currently the machine hangs during booting while accessing the
> BXT_MIPI_PORT_CTRL register during pipe HW state readout. After some
> experimentation I found that the hang is caused by the DSI PLL being
> disabled, or it being enabled but with an incorrect divider
> configuration. Enabling the PLL got rid of the boot problem, so fix
> this by checking the PLL enabled state/configuration before attempting
> to read out the HW state.
>
> The DSI_PLL_ENABLE register is in the always-on power well, while the
> BXT_DSI_PLL_CTL is in power well 0. This isn't exactly matched by the
> transcoder power domain, but what we really need is just a runtime PM
> reference, which is provided by any power domain.
>
> Ville also found this dependency specified in BSpec, so I added a
> reference to that too.
>
> v2:
> - Make sure we hold a power reference while accessing the PLL registers.
>
> CC: Shashank Sharma <shashank.sharma@intel.com>
> CC: Uma Shankar <uma.shankar@intel.com>
> CC: Jani Nikula <jani.nikula@intel.com>
> Fixes: c6c794a2fc5e ("drm/i915/bxt: Initialize MIPI DSI for BXT")

I figured this might blow something up now, but then again we needed to
do it to find out all the ways it would blow up. Apologies and
thanks. ;)

> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.c     |  9 +++++++++
>  drivers/gpu/drm/i915/intel_dsi.h     |  1 +
>  drivers/gpu/drm/i915/intel_dsi_pll.c | 37 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 62 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f3ba43c..c839ce9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7811,9 +7811,11 @@ enum skl_disp_power_wells {
>  #define  BXT_DSIC_16X_BY2		(1 << 10)
>  #define  BXT_DSIC_16X_BY3		(2 << 10)
>  #define  BXT_DSIC_16X_BY4		(3 << 10)
> +#define  BXT_DSIC_16X_MASK		(3 << 10)
>  #define  BXT_DSIA_16X_BY2		(1 << 8)
>  #define  BXT_DSIA_16X_BY3		(2 << 8)
>  #define  BXT_DSIA_16X_BY4		(3 << 8)
> +#define  BXT_DSIA_16X_MASK		(3 << 8)
>  #define  BXT_DSI_FREQ_SEL_SHIFT		8
>  #define  BXT_DSI_FREQ_SEL_MASK		(0xF << BXT_DSI_FREQ_SEL_SHIFT)
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 009b03b..6fd94c4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -36,6 +36,7 @@
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include "intel_dsi.h"
>  #include "i915_trace.h"
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -9852,10 +9853,12 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
>  	enum intel_display_power_domain power_domain;
>  	enum port port;
>  	enum transcoder cpu_transcoder;
> +	bool pll_enabled;
>  	u32 tmp;
>  
>  	pipe_config->has_dsi_encoder = false;
>  
> +	pll_enabled = false;

It seems to me you could just bail out early here if the dsi pll is
disabled.

I'm guessing you put the check in the loop because of the power domain
reference. Two thoughts on that:

a) BXT_DSI_PLL_ENABLE is in always on power. If you dropped the paranoia
in bxt_dsi_pll_is_enabled() about valid BXT_DSI_PLL_CTL values, you
could simplify code here and in intel_dsi_get_hw_state().

b) BXT_DSI_PLL_CTL is in PG0. Here, you implicitly trust the transcoder
power domain to match that. In intel_dsi_get_hw_state() you implicitly
trust the DSI port power domain to match that. They both do, but this
seems a somewhat coincidental choice.

>  	for_each_port_masked(port, BIT(PORT_A) | BIT(PORT_C)) {
>  		if (port == PORT_A)
>  			cpu_transcoder = TRANSCODER_DSI_A;
> @@ -9867,6 +9870,16 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
>  			continue;
>  		*power_domain_mask |= BIT(power_domain);
>  
> +		/*
> +		 * The PLL needs to be enabled with a valid divider
> +		 * configuration, otherwise accessing DSI registers will hang
> +		 * the machine. See BSpec North Display Engine
> +		 * registers/MIPI[BXT].
> +		 */
> +		pll_enabled = pll_enabled || intel_dsi_pll_is_enabled(dev_priv);
> +		if (!pll_enabled)
> +			break;
> +

I guess if we want to keep the paranoia and we are happy with how the
power domain references map to the power well of BXT_DSI_PLL_CTL, the
simplest you could do is throw away the local variable and just do this:

		if (!intel_dsi_pll_is_enabled(dev_priv))
			continue;

This potentially leads to an extra loop here, but at least the code gets
simpler and easier to grasp.

>  		/* XXX: this works for video mode only */
>  		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
>  		if (!(tmp & DPI_ENABLE))
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 96ea3f7..0de74e1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -684,6 +684,14 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  		return false;
>  
> +	/*
> +	 * On Broxton the PLL needs to be enabled with a valid divider
> +	 * configuration, otherwise accessing DSI registers will hang the
> +	 * machine. See BSpec North Display Engine registers/MIPI[BXT].
> +	 */
> +	if (IS_BROXTON(dev_priv) && !intel_dsi_pll_is_enabled(dev_priv))
> +		goto out_put_power;
> +
>  	/* XXX: this only works for one DSI output */
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		i915_reg_t ctrl_reg = IS_BROXTON(dev) ?
> @@ -726,6 +734,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  		break;
>  	}
>  
> +out_put_power:
>  	intel_display_power_put(dev_priv, power_domain);
>  
>  	return active;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index e582ef8..ec58ead 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -126,6 +126,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>  	return container_of(encoder, struct intel_dsi, base.base);
>  }
>  
> +bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
>  extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
>  extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
>  extern u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp);
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index e3e343c..e5b0625 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -192,6 +192,33 @@ static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
>  	mutex_unlock(&dev_priv->sb_lock);
>  }
>  
> +static bool bxt_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
> +{
> +	bool enabled;
> +	u32 val;
> +	u32 mask;
> +
> +	mask = BXT_DSI_PLL_DO_ENABLE | BXT_DSI_PLL_LOCKED;
> +	val = I915_READ(BXT_DSI_PLL_ENABLE);
> +	enabled = (val & mask) == mask;
> +
> +	if (!enabled)
> +		return false;
> +
> +	/*
> +	 * Both dividers must be programmed with valid values even if only one
> +	 * of the PLL is used. See BSpec/Broxton Clocks.
> +	 */
> +	val = I915_READ(BXT_DSI_PLL_CTL);
> +	if (!(val & BXT_DSIA_16X_MASK) || !(val & BXT_DSIC_16X_MASK)) {
> +		DRM_DEBUG_DRIVER("PLL is enabled with invalid divider settings "
> +				 "(%08x)\n", val);
> +		enabled = false;
> +	}

Do we really need this level of paranoia?

> +
> +	return enabled;
> +}
> +
>  static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -486,6 +513,16 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
>  	DRM_DEBUG_KMS("DSI PLL locked\n");
>  }
>  
> +bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
> +{
> +	if (IS_BROXTON(dev_priv))
> +		return bxt_dsi_pll_is_enabled(dev_priv);
> +
> +	MISSING_CASE(INTEL_DEVID(dev_priv));
> +
> +	return false;
> +}
> +
>  void intel_enable_dsi_pll(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;

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

Re: [PATCH v2] drm/i915/bxt: Fix DSI HW state readout

[Imre Deak]

On to, 2016-03-24 at 11:15 +0200, Jani Nikula wrote:
> On Wed, 23 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> > Currently the machine hangs during booting while accessing the
> > BXT_MIPI_PORT_CTRL register during pipe HW state readout. After
> > some
> > experimentation I found that the hang is caused by the DSI PLL
> > being
> > disabled, or it being enabled but with an incorrect divider
> > configuration. Enabling the PLL got rid of the boot problem, so fix
> > this by checking the PLL enabled state/configuration before
> > attempting
> > to read out the HW state.
> > 
> > The DSI_PLL_ENABLE register is in the always-on power well, while
> > the
> > BXT_DSI_PLL_CTL is in power well 0. This isn't exactly matched by
> > the
> > transcoder power domain, but what we really need is just a runtime
> > PM
> > reference, which is provided by any power domain.
> > 
> > Ville also found this dependency specified in BSpec, so I added a
> > reference to that too.
> > 
> > v2:
> > - Make sure we hold a power reference while accessing the PLL
> > registers.
> > 
> > CC: Shashank Sharma <shashank.sharma@intel.com>
> > CC: Uma Shankar <uma.shankar@intel.com>
> > CC: Jani Nikula <jani.nikula@intel.com>
> > Fixes: c6c794a2fc5e ("drm/i915/bxt: Initialize MIPI DSI for BXT")
> 
> I figured this might blow something up now, but then again we needed
> to
> do it to find out all the ways it would blow up. Apologies and
> thanks. ;)
> 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  2 ++
> >  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
> >  drivers/gpu/drm/i915/intel_dsi.c     |  9 +++++++++
> >  drivers/gpu/drm/i915/intel_dsi.h     |  1 +
> >  drivers/gpu/drm/i915/intel_dsi_pll.c | 37
> > ++++++++++++++++++++++++++++++++++++
> >  5 files changed, 62 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index f3ba43c..c839ce9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7811,9 +7811,11 @@ enum skl_disp_power_wells {
> >  #define  BXT_DSIC_16X_BY2		(1 << 10)
> >  #define  BXT_DSIC_16X_BY3		(2 << 10)
> >  #define  BXT_DSIC_16X_BY4		(3 << 10)
> > +#define  BXT_DSIC_16X_MASK		(3 << 10)
> >  #define  BXT_DSIA_16X_BY2		(1 << 8)
> >  #define  BXT_DSIA_16X_BY3		(2 << 8)
> >  #define  BXT_DSIA_16X_BY4		(3 << 8)
> > +#define  BXT_DSIA_16X_MASK		(3 << 8)
> >  #define  BXT_DSI_FREQ_SEL_SHIFT		8
> >  #define  BXT_DSI_FREQ_SEL_MASK		(0xF <<
> > BXT_DSI_FREQ_SEL_SHIFT)
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 009b03b..6fd94c4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -36,6 +36,7 @@
> >  #include "intel_drv.h"
> >  #include <drm/i915_drm.h>
> >  #include "i915_drv.h"
> > +#include "intel_dsi.h"
> >  #include "i915_trace.h"
> >  #include <drm/drm_atomic.h>
> >  #include <drm/drm_atomic_helper.h>
> > @@ -9852,10 +9853,12 @@ static bool
> > bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
> >  	enum intel_display_power_domain power_domain;
> >  	enum port port;
> >  	enum transcoder cpu_transcoder;
> > +	bool pll_enabled;
> >  	u32 tmp;
> >  
> >  	pipe_config->has_dsi_encoder = false;
> >  
> > +	pll_enabled = false;
> 
> It seems to me you could just bail out early here if the dsi pll is
> disabled.
>
> I'm guessing you put the check in the loop because of the power
> domain reference.

Yes.

> Two thoughts on that:
> 
> a) BXT_DSI_PLL_ENABLE is in always on power.

Well, that means always-on, whenever the PCI device is on, that is we
still need to hold a runtime PM reference, which is provided by any
display power domain reference.

> If you dropped the paranoia
> in bxt_dsi_pll_is_enabled() about valid BXT_DSI_PLL_CTL values, you
> could simplify code here and in intel_dsi_get_hw_state().

I think it's better to check the dividers too. If the dividers are
incorrect that machine will hang even though the PLL is enabled. Ville
told me that he saw a similar misconfiguration by BIOS on CHV/VLV, so
it's not unimaginable that it could also happen on BXT. 

> b) BXT_DSI_PLL_CTL is in PG0. Here, you implicitly trust the
> transcoder
> power domain to match that. In intel_dsi_get_hw_state() you
> implicitly
> trust the DSI port power domain to match that. They both do, but this
> seems a somewhat coincidental choice.

I'm actually not sure if power well 0 is simply the same as the always-
on well, there is no separate always-on well shown in BSpec "Broxton
Display Connections". In any case we don't handle either of  these
wells manually (on-demand), it's only during runtime suspend when they
are powered off. So to keep them on we need a runtime PM reference and
that is guaranteed to be provided by any power domain ref. 

> >  	for_each_port_masked(port, BIT(PORT_A) | BIT(PORT_C)) {
> >  		if (port == PORT_A)
> >  			cpu_transcoder = TRANSCODER_DSI_A;
> > @@ -9867,6 +9870,16 @@ static bool
> > bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
> >  			continue;
> >  		*power_domain_mask |= BIT(power_domain);
> >  
> > +		/*
> > +		 * The PLL needs to be enabled with a valid
> > divider
> > +		 * configuration, otherwise accessing DSI
> > registers will hang
> > +		 * the machine. See BSpec North Display Engine
> > +		 * registers/MIPI[BXT].
> > +		 */
> > +		pll_enabled = pll_enabled ||
> > intel_dsi_pll_is_enabled(dev_priv);
> > +		if (!pll_enabled)
> > +			break;
> > +
> 
> I guess if we want to keep the paranoia and we are happy with how the
> power domain references map to the power well of BXT_DSI_PLL_CTL, the
> simplest you could do is throw away the local variable and just do
> this:
> 
> 		if (!intel_dsi_pll_is_enabled(dev_priv))
> 			continue;
> 
> This potentially leads to an extra loop here, but at least the code
> gets
> simpler and easier to grasp.

Yea, it will result in an extra HW access, but I can simplify things as
you suggested.

> >  		/* XXX: this works for video mode only */
> >  		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
> >  		if (!(tmp & DPI_ENABLE))
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index 96ea3f7..0de74e1 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -684,6 +684,14 @@ static bool intel_dsi_get_hw_state(struct
> > intel_encoder *encoder,
> >  	if (!intel_display_power_get_if_enabled(dev_priv,
> > power_domain))
> >  		return false;
> >  
> > +	/*
> > +	 * On Broxton the PLL needs to be enabled with a valid
> > divider
> > +	 * configuration, otherwise accessing DSI registers will
> > hang the
> > +	 * machine. See BSpec North Display Engine
> > registers/MIPI[BXT].
> > +	 */
> > +	if (IS_BROXTON(dev_priv) &&
> > !intel_dsi_pll_is_enabled(dev_priv))
> > +		goto out_put_power;
> > +
> >  	/* XXX: this only works for one DSI output */
> >  	for_each_dsi_port(port, intel_dsi->ports) {
> >  		i915_reg_t ctrl_reg = IS_BROXTON(dev) ?
> > @@ -726,6 +734,7 @@ static bool intel_dsi_get_hw_state(struct
> > intel_encoder *encoder,
> >  		break;
> >  	}
> >  
> > +out_put_power:
> >  	intel_display_power_put(dev_priv, power_domain);
> >  
> >  	return active;
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> > b/drivers/gpu/drm/i915/intel_dsi.h
> > index e582ef8..ec58ead 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.h
> > +++ b/drivers/gpu/drm/i915/intel_dsi.h
> > @@ -126,6 +126,7 @@ static inline struct intel_dsi
> > *enc_to_intel_dsi(struct drm_encoder *encoder)
> >  	return container_of(encoder, struct intel_dsi, base.base);
> >  }
> >  
> > +bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
> >  extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
> >  extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
> >  extern u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int
> > pipe_bpp);
> > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c
> > b/drivers/gpu/drm/i915/intel_dsi_pll.c
> > index e3e343c..e5b0625 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> > @@ -192,6 +192,33 @@ static void vlv_disable_dsi_pll(struct
> > intel_encoder *encoder)
> >  	mutex_unlock(&dev_priv->sb_lock);
> >  }
> >  
> > +static bool bxt_dsi_pll_is_enabled(struct drm_i915_private
> > *dev_priv)
> > +{
> > +	bool enabled;
> > +	u32 val;
> > +	u32 mask;
> > +
> > +	mask = BXT_DSI_PLL_DO_ENABLE | BXT_DSI_PLL_LOCKED;
> > +	val = I915_READ(BXT_DSI_PLL_ENABLE);
> > +	enabled = (val & mask) == mask;
> > +
> > +	if (!enabled)
> > +		return false;
> > +
> > +	/*
> > +	 * Both dividers must be programmed with valid values even
> > if only one
> > +	 * of the PLL is used. See BSpec/Broxton Clocks.
> > +	 */
> > +	val = I915_READ(BXT_DSI_PLL_CTL);
> > +	if (!(val & BXT_DSIA_16X_MASK) || !(val &
> > BXT_DSIC_16X_MASK)) {
> > +		DRM_DEBUG_DRIVER("PLL is enabled with invalid
> > divider settings "
> > +				 "(%08x)\n", val);
> > +		enabled = false;
> > +	}
> 
> Do we really need this level of paranoia?

Yes, based on my test I think so, see above.

> > +
> > +	return enabled;
> > +}
> > +
> >  static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
> >  {
> >  	struct drm_i915_private *dev_priv = encoder->base.dev-
> > >dev_private;
> > @@ -486,6 +513,16 @@ static void bxt_enable_dsi_pll(struct
> > intel_encoder *encoder)
> >  	DRM_DEBUG_KMS("DSI PLL locked\n");
> >  }
> >  
> > +bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
> > +{
> > +	if (IS_BROXTON(dev_priv))
> > +		return bxt_dsi_pll_is_enabled(dev_priv);
> > +
> > +	MISSING_CASE(INTEL_DEVID(dev_priv));
> > +
> > +	return false;
> > +}
> > +
> >  void intel_enable_dsi_pll(struct intel_encoder *encoder)
> >  {
> >  	struct drm_device *dev = encoder->base.dev;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3] drm/i915/bxt: Fix DSI HW state readout

[Imre Deak]

Currently the machine hangs during booting while accessing the
BXT_MIPI_PORT_CTRL register during pipe HW state readout. After some
experimentation I found that the hang is caused by the DSI PLL being
disabled, or it being enabled but with an incorrect divider
configuration. Enabling the PLL got rid of the boot problem, so fix
this by checking the PLL enabled state/configuration before attempting
to read out the HW state.

The DSI_PLL_ENABLE register is in the always-on power well, while the
BXT_DSI_PLL_CTL is in power well 0. This isn't exactly matched by the
transcoder power domain, but what we really need is just a runtime PM
reference, which is provided by any power domain.

Ville also found this dependency specified in BSpec, so I added a
reference to that too.

v2:
- Make sure we hold a power reference while accessing the PLL registers.
v3: (Jani)
- Simplify check in bxt_get_dsi_transcoder_state()
- Add comment explaining why we check for valid dividers in
  bxt_dsi_pll_is_enabled()

CC: Shashank Sharma <shashank.sharma@intel.com>
CC: Uma Shankar <uma.shankar@intel.com>
CC: Jani Nikula <jani.nikula@intel.com>
Fixes: c6c794a2fc5e ("drm/i915/bxt: Initialize MIPI DSI for BXT")
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++
 drivers/gpu/drm/i915/intel_dsi.c     |  9 ++++++++
 drivers/gpu/drm/i915/intel_dsi.h     |  1 +
 drivers/gpu/drm/i915/intel_dsi_pll.c | 40 ++++++++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f3ba43c..c839ce9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7811,9 +7811,11 @@ enum skl_disp_power_wells {
 #define  BXT_DSIC_16X_BY2		(1 << 10)
 #define  BXT_DSIC_16X_BY3		(2 << 10)
 #define  BXT_DSIC_16X_BY4		(3 << 10)
+#define  BXT_DSIC_16X_MASK		(3 << 10)
 #define  BXT_DSIA_16X_BY2		(1 << 8)
 #define  BXT_DSIA_16X_BY3		(2 << 8)
 #define  BXT_DSIA_16X_BY4		(3 << 8)
+#define  BXT_DSIA_16X_MASK		(3 << 8)
 #define  BXT_DSI_FREQ_SEL_SHIFT		8
 #define  BXT_DSI_FREQ_SEL_MASK		(0xF << BXT_DSI_FREQ_SEL_SHIFT)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 009b03b..e05393e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -36,6 +36,7 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "intel_dsi.h"
 #include "i915_trace.h"
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -9867,6 +9868,16 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
 			continue;
 		*power_domain_mask |= BIT(power_domain);
 
+		/*
+		 * The PLL needs to be enabled with a valid divider
+		 * configuration, otherwise accessing DSI registers will hang
+		 * the machine. See BSpec North Display Engine
+		 * registers/MIPI[BXT]. We can break out here early, since we
+		 * need the same DSI PLL to be enabled for both DSI ports.
+		 */
+		if (!intel_dsi_pll_is_enabled(dev_priv))
+			break;
+
 		/* XXX: this works for video mode only */
 		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
 		if (!(tmp & DPI_ENABLE))
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 96ea3f7..0de74e1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -684,6 +684,14 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
 
+	/*
+	 * On Broxton the PLL needs to be enabled with a valid divider
+	 * configuration, otherwise accessing DSI registers will hang the
+	 * machine. See BSpec North Display Engine registers/MIPI[BXT].
+	 */
+	if (IS_BROXTON(dev_priv) && !intel_dsi_pll_is_enabled(dev_priv))
+		goto out_put_power;
+
 	/* XXX: this only works for one DSI output */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		i915_reg_t ctrl_reg = IS_BROXTON(dev) ?
@@ -726,6 +734,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 		break;
 	}
 
+out_put_power:
 	intel_display_power_put(dev_priv, power_domain);
 
 	return active;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index e582ef8..ec58ead 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -126,6 +126,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 	return container_of(encoder, struct intel_dsi, base.base);
 }
 
+bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
 extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
 extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
 extern u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index e3e343c..ce688f9 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -192,6 +192,36 @@ static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+static bool bxt_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
+{
+	bool enabled;
+	u32 val;
+	u32 mask;
+
+	mask = BXT_DSI_PLL_DO_ENABLE | BXT_DSI_PLL_LOCKED;
+	val = I915_READ(BXT_DSI_PLL_ENABLE);
+	enabled = (val & mask) == mask;
+
+	if (!enabled)
+		return false;
+
+	/*
+	 * Both dividers must be programmed with valid values even if only one
+	 * of the PLL is used, see BSpec/Broxton Clocks. Check this here for
+	 * paranoia, since BIOS is known to misconfigure PLLs in this way at
+	 * times, and since accessing DSI registers with invalid dividers
+	 * causes a system hang.
+	 */
+	val = I915_READ(BXT_DSI_PLL_CTL);
+	if (!(val & BXT_DSIA_16X_MASK) || !(val & BXT_DSIC_16X_MASK)) {
+		DRM_DEBUG_DRIVER("PLL is enabled with invalid divider settings "
+				 "(%08x)\n", val);
+		enabled = false;
+	}
+
+	return enabled;
+}
+
 static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -486,6 +516,16 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
 	DRM_DEBUG_KMS("DSI PLL locked\n");
 }
 
+bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
+{
+	if (IS_BROXTON(dev_priv))
+		return bxt_dsi_pll_is_enabled(dev_priv);
+
+	MISSING_CASE(INTEL_DEVID(dev_priv));
+
+	return false;
+}
+
 void intel_enable_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
-- 
2.5.0

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

Re: [PATCH v3] drm/i915/bxt: Fix DSI HW state readout

[Jani Nikula]

On Thu, 24 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> Currently the machine hangs during booting while accessing the
> BXT_MIPI_PORT_CTRL register during pipe HW state readout. After some
> experimentation I found that the hang is caused by the DSI PLL being
> disabled, or it being enabled but with an incorrect divider
> configuration. Enabling the PLL got rid of the boot problem, so fix
> this by checking the PLL enabled state/configuration before attempting
> to read out the HW state.
>
> The DSI_PLL_ENABLE register is in the always-on power well, while the
> BXT_DSI_PLL_CTL is in power well 0. This isn't exactly matched by the
> transcoder power domain, but what we really need is just a runtime PM
> reference, which is provided by any power domain.
>
> Ville also found this dependency specified in BSpec, so I added a
> reference to that too.
>
> v2:
> - Make sure we hold a power reference while accessing the PLL registers.
> v3: (Jani)
> - Simplify check in bxt_get_dsi_transcoder_state()
> - Add comment explaining why we check for valid dividers in
>   bxt_dsi_pll_is_enabled()
>
> CC: Shashank Sharma <shashank.sharma@intel.com>
> CC: Uma Shankar <uma.shankar@intel.com>
> CC: Jani Nikula <jani.nikula@intel.com>
> Fixes: c6c794a2fc5e ("drm/i915/bxt: Initialize MIPI DSI for BXT")
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Thanks for fixing this.

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


> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++
>  drivers/gpu/drm/i915/intel_dsi.c     |  9 ++++++++
>  drivers/gpu/drm/i915/intel_dsi.h     |  1 +
>  drivers/gpu/drm/i915/intel_dsi_pll.c | 40 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 63 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f3ba43c..c839ce9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7811,9 +7811,11 @@ enum skl_disp_power_wells {
>  #define  BXT_DSIC_16X_BY2		(1 << 10)
>  #define  BXT_DSIC_16X_BY3		(2 << 10)
>  #define  BXT_DSIC_16X_BY4		(3 << 10)
> +#define  BXT_DSIC_16X_MASK		(3 << 10)
>  #define  BXT_DSIA_16X_BY2		(1 << 8)
>  #define  BXT_DSIA_16X_BY3		(2 << 8)
>  #define  BXT_DSIA_16X_BY4		(3 << 8)
> +#define  BXT_DSIA_16X_MASK		(3 << 8)
>  #define  BXT_DSI_FREQ_SEL_SHIFT		8
>  #define  BXT_DSI_FREQ_SEL_MASK		(0xF << BXT_DSI_FREQ_SEL_SHIFT)
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 009b03b..e05393e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -36,6 +36,7 @@
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include "intel_dsi.h"
>  #include "i915_trace.h"
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -9867,6 +9868,16 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
>  			continue;
>  		*power_domain_mask |= BIT(power_domain);
>  
> +		/*
> +		 * The PLL needs to be enabled with a valid divider
> +		 * configuration, otherwise accessing DSI registers will hang
> +		 * the machine. See BSpec North Display Engine
> +		 * registers/MIPI[BXT]. We can break out here early, since we
> +		 * need the same DSI PLL to be enabled for both DSI ports.
> +		 */
> +		if (!intel_dsi_pll_is_enabled(dev_priv))
> +			break;
> +
>  		/* XXX: this works for video mode only */
>  		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
>  		if (!(tmp & DPI_ENABLE))
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 96ea3f7..0de74e1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -684,6 +684,14 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  		return false;
>  
> +	/*
> +	 * On Broxton the PLL needs to be enabled with a valid divider
> +	 * configuration, otherwise accessing DSI registers will hang the
> +	 * machine. See BSpec North Display Engine registers/MIPI[BXT].
> +	 */
> +	if (IS_BROXTON(dev_priv) && !intel_dsi_pll_is_enabled(dev_priv))
> +		goto out_put_power;
> +
>  	/* XXX: this only works for one DSI output */
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		i915_reg_t ctrl_reg = IS_BROXTON(dev) ?
> @@ -726,6 +734,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  		break;
>  	}
>  
> +out_put_power:
>  	intel_display_power_put(dev_priv, power_domain);
>  
>  	return active;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index e582ef8..ec58ead 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -126,6 +126,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>  	return container_of(encoder, struct intel_dsi, base.base);
>  }
>  
> +bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
>  extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
>  extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
>  extern u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp);
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index e3e343c..ce688f9 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -192,6 +192,36 @@ static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
>  	mutex_unlock(&dev_priv->sb_lock);
>  }
>  
> +static bool bxt_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
> +{
> +	bool enabled;
> +	u32 val;
> +	u32 mask;
> +
> +	mask = BXT_DSI_PLL_DO_ENABLE | BXT_DSI_PLL_LOCKED;
> +	val = I915_READ(BXT_DSI_PLL_ENABLE);
> +	enabled = (val & mask) == mask;
> +
> +	if (!enabled)
> +		return false;
> +
> +	/*
> +	 * Both dividers must be programmed with valid values even if only one
> +	 * of the PLL is used, see BSpec/Broxton Clocks. Check this here for
> +	 * paranoia, since BIOS is known to misconfigure PLLs in this way at
> +	 * times, and since accessing DSI registers with invalid dividers
> +	 * causes a system hang.
> +	 */
> +	val = I915_READ(BXT_DSI_PLL_CTL);
> +	if (!(val & BXT_DSIA_16X_MASK) || !(val & BXT_DSIC_16X_MASK)) {
> +		DRM_DEBUG_DRIVER("PLL is enabled with invalid divider settings "
> +				 "(%08x)\n", val);
> +		enabled = false;
> +	}
> +
> +	return enabled;
> +}
> +
>  static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -486,6 +516,16 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
>  	DRM_DEBUG_KMS("DSI PLL locked\n");
>  }
>  
> +bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
> +{
> +	if (IS_BROXTON(dev_priv))
> +		return bxt_dsi_pll_is_enabled(dev_priv);
> +
> +	MISSING_CASE(INTEL_DEVID(dev_priv));
> +
> +	return false;
> +}
> +
>  void intel_enable_dsi_pll(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;

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

Re: [PATCH v3] drm/i915/bxt: Fix DSI HW state readout

[Sharma, Shashank]

reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

Regards
Shashank
-----Original Message-----
From: Deak, Imre 
Sent: Thursday, March 24, 2016 4:12 PM
To: intel-gfx@lists.freedesktop.org
Cc: Ville Syrjälä; Sharma, Shashank; Shankar, Uma; Nikula, Jani
Subject: [PATCH v3] drm/i915/bxt: Fix DSI HW state readout

Currently the machine hangs during booting while accessing the BXT_MIPI_PORT_CTRL register during pipe HW state readout. After some experimentation I found that the hang is caused by the DSI PLL being disabled, or it being enabled but with an incorrect divider configuration. Enabling the PLL got rid of the boot problem, so fix this by checking the PLL enabled state/configuration before attempting to read out the HW state.

The DSI_PLL_ENABLE register is in the always-on power well, while the BXT_DSI_PLL_CTL is in power well 0. This isn't exactly matched by the transcoder power domain, but what we really need is just a runtime PM reference, which is provided by any power domain.

Ville also found this dependency specified in BSpec, so I added a reference to that too.

v2:
- Make sure we hold a power reference while accessing the PLL registers.
v3: (Jani)
- Simplify check in bxt_get_dsi_transcoder_state()
- Add comment explaining why we check for valid dividers in
  bxt_dsi_pll_is_enabled()

CC: Shashank Sharma <shashank.sharma@intel.com>
CC: Uma Shankar <uma.shankar@intel.com>
CC: Jani Nikula <jani.nikula@intel.com>
Fixes: c6c794a2fc5e ("drm/i915/bxt: Initialize MIPI DSI for BXT")
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++
 drivers/gpu/drm/i915/intel_dsi.c     |  9 ++++++++
 drivers/gpu/drm/i915/intel_dsi.h     |  1 +
 drivers/gpu/drm/i915/intel_dsi_pll.c | 40 ++++++++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f3ba43c..c839ce9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7811,9 +7811,11 @@ enum skl_disp_power_wells {
 #define  BXT_DSIC_16X_BY2		(1 << 10)
 #define  BXT_DSIC_16X_BY3		(2 << 10)
 #define  BXT_DSIC_16X_BY4		(3 << 10)
+#define  BXT_DSIC_16X_MASK		(3 << 10)
 #define  BXT_DSIA_16X_BY2		(1 << 8)
 #define  BXT_DSIA_16X_BY3		(2 << 8)
 #define  BXT_DSIA_16X_BY4		(3 << 8)
+#define  BXT_DSIA_16X_MASK		(3 << 8)
 #define  BXT_DSI_FREQ_SEL_SHIFT		8
 #define  BXT_DSI_FREQ_SEL_MASK		(0xF << BXT_DSI_FREQ_SEL_SHIFT)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 009b03b..e05393e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -36,6 +36,7 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "intel_dsi.h"
 #include "i915_trace.h"
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -9867,6 +9868,16 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
 			continue;
 		*power_domain_mask |= BIT(power_domain);
 
+		/*
+		 * The PLL needs to be enabled with a valid divider
+		 * configuration, otherwise accessing DSI registers will hang
+		 * the machine. See BSpec North Display Engine
+		 * registers/MIPI[BXT]. We can break out here early, since we
+		 * need the same DSI PLL to be enabled for both DSI ports.
+		 */
+		if (!intel_dsi_pll_is_enabled(dev_priv))
+			break;
+
 		/* XXX: this works for video mode only */
 		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
 		if (!(tmp & DPI_ENABLE))
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 96ea3f7..0de74e1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -684,6 +684,14 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
 
+	/*
+	 * On Broxton the PLL needs to be enabled with a valid divider
+	 * configuration, otherwise accessing DSI registers will hang the
+	 * machine. See BSpec North Display Engine registers/MIPI[BXT].
+	 */
+	if (IS_BROXTON(dev_priv) && !intel_dsi_pll_is_enabled(dev_priv))
+		goto out_put_power;
+
 	/* XXX: this only works for one DSI output */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		i915_reg_t ctrl_reg = IS_BROXTON(dev) ?
@@ -726,6 +734,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 		break;
 	}
 
+out_put_power:
 	intel_display_power_put(dev_priv, power_domain);
 
 	return active;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index e582ef8..ec58ead 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -126,6 +126,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 	return container_of(encoder, struct intel_dsi, base.base);  }
 
+bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
 extern void intel_enable_dsi_pll(struct intel_encoder *encoder);  extern void intel_disable_dsi_pll(struct intel_encoder *encoder);  extern u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp); diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index e3e343c..ce688f9 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -192,6 +192,36 @@ static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
+static bool bxt_dsi_pll_is_enabled(struct drm_i915_private *dev_priv) {
+	bool enabled;
+	u32 val;
+	u32 mask;
+
+	mask = BXT_DSI_PLL_DO_ENABLE | BXT_DSI_PLL_LOCKED;
+	val = I915_READ(BXT_DSI_PLL_ENABLE);
+	enabled = (val & mask) == mask;
+
+	if (!enabled)
+		return false;
+
+	/*
+	 * Both dividers must be programmed with valid values even if only one
+	 * of the PLL is used, see BSpec/Broxton Clocks. Check this here for
+	 * paranoia, since BIOS is known to misconfigure PLLs in this way at
+	 * times, and since accessing DSI registers with invalid dividers
+	 * causes a system hang.
+	 */
+	val = I915_READ(BXT_DSI_PLL_CTL);
+	if (!(val & BXT_DSIA_16X_MASK) || !(val & BXT_DSIC_16X_MASK)) {
+		DRM_DEBUG_DRIVER("PLL is enabled with invalid divider settings "
+				 "(%08x)\n", val);
+		enabled = false;
+	}
+
+	return enabled;
+}
+
 static void bxt_disable_dsi_pll(struct intel_encoder *encoder)  {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private; @@ -486,6 +516,16 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
 	DRM_DEBUG_KMS("DSI PLL locked\n");
 }
 
+bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv) {
+	if (IS_BROXTON(dev_priv))
+		return bxt_dsi_pll_is_enabled(dev_priv);
+
+	MISSING_CASE(INTEL_DEVID(dev_priv));
+
+	return false;
+}
+
 void intel_enable_dsi_pll(struct intel_encoder *encoder)  {
 	struct drm_device *dev = encoder->base.dev;
--
2.5.0

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

✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix DSI HW state readout (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/bxt: Fix DSI HW state readout (rev2)
URL   : https://patchwork.freedesktop.org/series/4832/
State : failure

== Summary ==

Series 4832v2 drm/i915/bxt: Fix DSI HW state readout
http://patchwork.freedesktop.org/api/1.0/series/4832/revisions/2/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test kms_force_connector_basic:
        Subgroup force-edid:
                pass       -> SKIP       (snb-x220t)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c:
                pass       -> SKIP       (hsw-brixbox)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (byt-nuc) UNSTABLE

bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:192  pass:171  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:192  pass:153  dwarn:2   dfail:0   fail:0   skip:37 
byt-nuc          total:192  pass:157  dwarn:0   dfail:0   fail:0   skip:35 
hsw-brixbox      total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
hsw-gt2          total:192  pass:175  dwarn:0   dfail:0   fail:0   skip:17 
ivb-t430s        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
skl-i7k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
snb-x220t        total:192  pass:157  dwarn:0   dfail:0   fail:1   skip:34 

Results at /archive/results/CI_IGT_test/Patchwork_1700/

8f2e9bbc15607f56261aefd7a1f8caf6909c6b9d drm-intel-nightly: 2016y-03m-23d-17h-03m-10s UTC integration manifest
79a5fd5ff300da5306de558ec9835cb8d42cc3b8 drm/i915/bxt: Fix DSI HW state readout

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

✓ Fi.CI.BAT: success for drm/i915/bxt: Fix DSI HW state readout (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/bxt: Fix DSI HW state readout (rev3)
URL   : https://patchwork.freedesktop.org/series/4832/
State : success

== Summary ==

Series 4832v3 drm/i915/bxt: Fix DSI HW state readout
http://patchwork.freedesktop.org/api/1.0/series/4832/revisions/3/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (hsw-gt2)

bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:192  pass:171  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:192  pass:155  dwarn:0   dfail:0   fail:0   skip:37 
byt-nuc          total:192  pass:156  dwarn:1   dfail:0   fail:0   skip:35 
hsw-brixbox      total:192  pass:170  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:192  pass:175  dwarn:0   dfail:0   fail:0   skip:17 
ivb-t430s        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
skl-i7k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:192  pass:158  dwarn:0   dfail:0   fail:0   skip:34 
snb-x220t        total:192  pass:158  dwarn:0   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1703/

79ee42317266a82b932a39e9567244ed91dd27d6 drm-intel-nightly: 2016y-03m-24d-10h-26m-54s UTC integration manifest
8e94bd5898ec48db5dcfed6dbbe1cf1e9ac2499f drm/i915/bxt: Fix DSI HW state readout

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

Re: ✓ Fi.CI.BAT: success for drm/i915/bxt: Fix DSI HW state readout (rev3)

[Imre Deak]

On to, 2016-03-24 at 11:32 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/bxt: Fix DSI HW state readout (rev3)
> URL   : https://patchwork.freedesktop.org/series/4832/
> State : success
> 
> == Summary ==
> 
> Series 4832v3 drm/i915/bxt: Fix DSI HW state readout
> http://patchwork.freedesktop.org/api/1.0/series/4832/revisions/3/mbox
> /
> 
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 dmesg-warn -> PASS       (bsw-nuc-2)
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-c:
>                 incomplete -> PASS       (hsw-gt2)

Thanks for the review, I pushed the patch to -dinq.

--Imre

> bdw-
> nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:1
> 2 
> bdw-
> ultra        total:192  pass:171  dwarn:0   dfail:0   fail:0   skip:2
> 1 
> bsw-nuc-
> 2        total:192  pass:155  dwarn:0   dfail:0   fail:0   skip:37 
> byt-
> nuc          total:192  pass:156  dwarn:1   dfail:0   fail:0   skip:3
> 5 
> hsw-
> brixbox      total:192  pass:170  dwarn:0   dfail:0   fail:0   skip:2
> 2 
> hsw-
> gt2          total:192  pass:175  dwarn:0   dfail:0   fail:0   skip:1
> 7 
> ivb-
> t430s        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:2
> 5 
> skl-i7k-
> 2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
> skl-
> nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:1
> 1 
> snb-
> dellxps      total:192  pass:158  dwarn:0   dfail:0   fail:0   skip:3
> 4 
> snb-
> x220t        total:192  pass:158  dwarn:0   dfail:0   fail:1   skip:3
> 3 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1703/
> 
> 79ee42317266a82b932a39e9567244ed91dd27d6 drm-intel-nightly: 2016y-
> 03m-24d-10h-26m-54s UTC integration manifest
> 8e94bd5898ec48db5dcfed6dbbe1cf1e9ac2499f drm/i915/bxt: Fix DSI HW
> state readout
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx