All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI Uma Shankar
@ 2015-10-12 17:23   ` Ville Syrjälä
  2015-10-13 11:03     ` Shankar, Uma
  2016-01-18 13:09   ` Mika Kahola
  1 sibling, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2015-10-12 17:23 UTC (permalink / raw)
  To: Uma Shankar; +Cc: shobhit.kumar, intel-gfx

On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote:
> For BXT DSI, vtotal, vactive, hactive registers are different.
> Making changes to intel_crtc_mode_get() and get_pipe_timings(),
> to read the correct registers for BXT DSI.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   48 +++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 75c60b8..2717823 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(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;
> +	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>  	uint32_t tmp;
>  
>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>  	}
>  
> +	if (IS_BROXTON(dev) && is_dsi) {
> +		struct intel_encoder *encoder;
> +
> +		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +			struct intel_dsi *intel_dsi =
> +				enc_to_intel_dsi(&encoder->base);
> +			enum port port;
> +
> +			for_each_dsi_port(port, intel_dsi->ports) {
> +				pipe_config->base.adjusted_mode.crtc_hdisplay =
> +					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +				pipe_config->base.adjusted_mode.crtc_vdisplay =
> +					I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +				pipe_config->base.adjusted_mode.crtc_vtotal =
> +					I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +			}
> +		}
> +
> +	}

I think I already asked this earlier when this patch was posted; Don't
the normal pipe timing registers contain the same values? And if so, why
would you need to read them out from the DSI speciific registers?

> +
>  	tmp = I915_READ(PIPESRC(crtc->pipe));
>  	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
>  	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> @@ -10664,6 +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
>  	int vsync = I915_READ(VSYNC(cpu_transcoder));
>  	enum pipe pipe = intel_crtc->pipe;
> +	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>  
>  	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
>  	if (!mode)
> @@ -10684,15 +10706,35 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
>  
>  	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
> -	mode->hdisplay = (htot & 0xffff) + 1;
>  	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
>  	mode->hsync_start = (hsync & 0xffff) + 1;
>  	mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
> -	mode->vdisplay = (vtot & 0xffff) + 1;
> -	mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
>  	mode->vsync_start = (vsync & 0xffff) + 1;
>  	mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
>  
> +	if (IS_BROXTON(dev) && is_dsi) {
> +		struct intel_encoder *encoder;
> +
> +		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
> +			struct intel_dsi *intel_dsi =
> +				enc_to_intel_dsi(&encoder->base);
> +			enum port port;
> +
> +			for_each_dsi_port(port, intel_dsi->ports) {
> +				mode->vtotal =
> +					I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +				mode->hdisplay =
> +					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +				mode->vdisplay =
> +					I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +			}
> +		}
> +	} else {
> +		mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> +		mode->hdisplay = (htot & 0xffff) + 1;
> +		mode->vdisplay = (vtot & 0xffff) + 1;
> +	}
> +
>  	drm_mode_set_name(mode);
>  
>  	return mode;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [BXT DSI timing fixes v1 0/3] BXT DSI mode timing fixes
@ 2015-10-12 17:25 Uma Shankar
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 1/3] drm/i915/: DSI mode setting fix Uma Shankar
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Uma Shankar @ 2015-10-12 17:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: shobhit.kumar

This patch series adds handling for getting mode and timing parameters
for BXT DSI.

MIPI DSI for BXT uses a separate trasncoder, hence special handling is
needed to address this. 

Floating version 1 of the patches to get the feedback and address the
BXT DSI HW devaition and corresponding SW design.

Uma Shankar (3):
  drm/i915/: DSI mode setting fix
  drm/i915/bxt: Get pipe timing for BXT DSI
  drm/i915/bxt: Fixed dsi enc disable and  blank at bootup

 drivers/gpu/drm/i915/i915_drv.h      |    3 +
 drivers/gpu/drm/i915/intel_display.c |  161 +++++++++++++++++++++++++++-------
 2 files changed, 134 insertions(+), 30 deletions(-)

-- 
1.7.9.5

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [BXT DSI timing fixes v1 1/3] drm/i915/: DSI mode setting fix
  2015-10-12 17:25 [BXT DSI timing fixes v1 0/3] BXT DSI mode timing fixes Uma Shankar
@ 2015-10-12 17:25 ` Uma Shankar
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI Uma Shankar
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup Uma Shankar
  2 siblings, 0 replies; 16+ messages in thread
From: Uma Shankar @ 2015-10-12 17:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: shobhit.kumar

Fixed dsi crtc state. Updated the get config function
and handled the DSI and DDI encoder cases.

BXT DSI have to be handled differently from rest of the encoders.
Reading the port control register to determine if DSI is enabled.
Generalizing it for all existing platforms.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   94 ++++++++++++++++++++++++----------
 1 file changed, 67 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cddb0c6..75c60b8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -44,6 +44,7 @@
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_rect.h>
 #include <linux/dma_remapping.h>
+#include "intel_dsi.h"
 
 /* Primary plane formats for gen <= 3 */
 static const uint32_t i8xx_primary_formats[] = {
@@ -9788,46 +9789,85 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum intel_display_power_domain pfit_domain;
-	uint32_t tmp;
+	uint32_t tmp = 0;
+	bool is_dsi = false;
+	bool dsi_enc_enabled = false;
+	u32 port_ctrl = 0;
 
 	if (!intel_display_power_is_enabled(dev_priv,
-					 POWER_DOMAIN_PIPE(crtc->pipe)))
+				POWER_DOMAIN_PIPE(crtc->pipe)))
 		return false;
 
 	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
 	pipe_config->shared_dpll = DPLL_ID_PRIVATE;
 
-	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
-	if (tmp & TRANS_DDI_FUNC_ENABLE) {
-		enum pipe trans_edp_pipe;
-		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
-		default:
-			WARN(1, "unknown pipe linked to edp transcoder\n");
-		case TRANS_DDI_EDP_INPUT_A_ONOFF:
-		case TRANS_DDI_EDP_INPUT_A_ON:
-			trans_edp_pipe = PIPE_A;
-			break;
-		case TRANS_DDI_EDP_INPUT_B_ONOFF:
-			trans_edp_pipe = PIPE_B;
-			break;
-		case TRANS_DDI_EDP_INPUT_C_ONOFF:
-			trans_edp_pipe = PIPE_C;
-			break;
+	is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
+
+	/*
+	 * Check if encoder is enabled or not
+	 * Separate implementation for DSI and DDI encoders.
+	 */
+	if (is_dsi) {
+		struct intel_encoder *encoder;
+
+		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+			struct intel_dsi *intel_dsi =
+				enc_to_intel_dsi(&encoder->base);
+			enum port port;
+
+			for_each_dsi_port(port, intel_dsi->ports) {
+				if (IS_BROXTON(dev))
+					port_ctrl = BXT_MIPI_PORT_CTRL(port);
+				else if (IS_VALLEYVIEW(dev))
+					port_ctrl = MIPI_PORT_CTRL(port);
+
+				tmp = I915_READ(port_ctrl);
+				if (tmp & DPI_ENABLE) {
+					dsi_enc_enabled = true;
+					break;
+				}
+			}
+
+			if (dsi_enc_enabled)
+				break;
 		}
+		if (!dsi_enc_enabled)
+			return false;
+	} else {
 
-		if (trans_edp_pipe == crtc->pipe)
-			pipe_config->cpu_transcoder = TRANSCODER_EDP;
-	}
+		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
+		if (tmp & TRANS_DDI_FUNC_ENABLE) {
+			enum pipe trans_edp_pipe;
 
-	if (!intel_display_power_is_enabled(dev_priv,
+			switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
+			default:
+				WARN(1, "unknown pipe linked to edp transcoder\n");
+			case TRANS_DDI_EDP_INPUT_A_ONOFF:
+			case TRANS_DDI_EDP_INPUT_A_ON:
+				trans_edp_pipe = PIPE_A;
+				break;
+			case TRANS_DDI_EDP_INPUT_B_ONOFF:
+				trans_edp_pipe = PIPE_B;
+				break;
+			case TRANS_DDI_EDP_INPUT_C_ONOFF:
+				trans_edp_pipe = PIPE_C;
+				break;
+			}
+
+			if (trans_edp_pipe == crtc->pipe)
+				pipe_config->cpu_transcoder = TRANSCODER_EDP;
+		}
+
+		if (!intel_display_power_is_enabled(dev_priv,
 			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
-		return false;
+			return false;
 
-	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
-	if (!(tmp & PIPECONF_ENABLE))
-		return false;
+		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
+		if (!(tmp & PIPECONF_ENABLE))
+			return false;
 
-	haswell_get_ddi_port_state(crtc, pipe_config);
+		haswell_get_ddi_port_state(crtc, pipe_config);
+	}
 
 	intel_get_pipe_timings(crtc, pipe_config);
 
-- 
1.7.9.5

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI
  2015-10-12 17:25 [BXT DSI timing fixes v1 0/3] BXT DSI mode timing fixes Uma Shankar
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 1/3] drm/i915/: DSI mode setting fix Uma Shankar
@ 2015-10-12 17:25 ` Uma Shankar
  2015-10-12 17:23   ` Ville Syrjälä
  2016-01-18 13:09   ` Mika Kahola
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup Uma Shankar
  2 siblings, 2 replies; 16+ messages in thread
From: Uma Shankar @ 2015-10-12 17:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: shobhit.kumar

For BXT DSI, vtotal, vactive, hactive registers are different.
Making changes to intel_crtc_mode_get() and get_pipe_timings(),
to read the correct registers for BXT DSI.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   48 +++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 75c60b8..2717823 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(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;
+	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
 	uint32_t tmp;
 
 	tmp = I915_READ(HTOTAL(cpu_transcoder));
@@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
 		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
 	}
 
+	if (IS_BROXTON(dev) && is_dsi) {
+		struct intel_encoder *encoder;
+
+		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+			struct intel_dsi *intel_dsi =
+				enc_to_intel_dsi(&encoder->base);
+			enum port port;
+
+			for_each_dsi_port(port, intel_dsi->ports) {
+				pipe_config->base.adjusted_mode.crtc_hdisplay =
+					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
+				pipe_config->base.adjusted_mode.crtc_vdisplay =
+					I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
+				pipe_config->base.adjusted_mode.crtc_vtotal =
+					I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
+			}
+		}
+
+	}
+
 	tmp = I915_READ(PIPESRC(crtc->pipe));
 	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
 	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
@@ -10664,6 +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 	int vtot = I915_READ(VTOTAL(cpu_transcoder));
 	int vsync = I915_READ(VSYNC(cpu_transcoder));
 	enum pipe pipe = intel_crtc->pipe;
+	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
 
 	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
 	if (!mode)
@@ -10684,15 +10706,35 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
 
 	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
-	mode->hdisplay = (htot & 0xffff) + 1;
 	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
 	mode->hsync_start = (hsync & 0xffff) + 1;
 	mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
-	mode->vdisplay = (vtot & 0xffff) + 1;
-	mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
 	mode->vsync_start = (vsync & 0xffff) + 1;
 	mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
 
+	if (IS_BROXTON(dev) && is_dsi) {
+		struct intel_encoder *encoder;
+
+		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
+			struct intel_dsi *intel_dsi =
+				enc_to_intel_dsi(&encoder->base);
+			enum port port;
+
+			for_each_dsi_port(port, intel_dsi->ports) {
+				mode->vtotal =
+					I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
+				mode->hdisplay =
+					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
+				mode->vdisplay =
+					I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
+			}
+		}
+	} else {
+		mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
+		mode->hdisplay = (htot & 0xffff) + 1;
+		mode->vdisplay = (vtot & 0xffff) + 1;
+	}
+
 	drm_mode_set_name(mode);
 
 	return mode;
-- 
1.7.9.5

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup
  2015-10-12 17:25 [BXT DSI timing fixes v1 0/3] BXT DSI mode timing fixes Uma Shankar
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 1/3] drm/i915/: DSI mode setting fix Uma Shankar
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI Uma Shankar
@ 2015-10-12 17:25 ` Uma Shankar
  2015-10-13 15:47   ` Daniel Vetter
  2016-01-18 13:07   ` Mika Kahola
  2 siblings, 2 replies; 16+ messages in thread
From: Uma Shankar @ 2015-10-12 17:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: shobhit.kumar

During bootup, mipi display was blanking in BXT. This is because during
driver load, though encoder, connector were active but crtc returned
inactive. This caused sanitize function to disable the DSI panel. In AOS,
this is fine since HWC will do a modeset and crtc, connector, encoder
mapping will be restored. But in Charging OS, no modeset is called, it
just calls DPMS ON/OFF. Hence display doesn't come in COS. This is needed
even for seamless booting to Android without a blank.

This is fine on BYT/CHT since transcoder is common b/w all encoders. But
for BXT, there is a separate mipi transcoder. Hence this needs special
handling for BXT DSI.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    3 +++
 drivers/gpu/drm/i915/intel_display.c |   27 +++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bf14096..ae790ec 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1948,6 +1948,9 @@ struct drm_i915_private {
 	/* perform PHY state sanity checks? */
 	bool chv_phy_assert[2];
 
+	/* To check if DSI is initializing at bootup */
+	bool dsi_enumerating;
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2717823..fe4f4f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7711,6 +7711,9 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
 	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
 	uint32_t tmp;
 
+	if (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)
+		is_dsi = true;
+
 	tmp = I915_READ(HTOTAL(cpu_transcoder));
 	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
 	pipe_config->base.adjusted_mode.crtc_htotal = ((tmp >> 16) & 0xffff) + 1;
@@ -9825,10 +9828,20 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
 
 	/*
-	 * Check if encoder is enabled or not
+	 * Check if encoder is enabled or not.
 	 * Separate implementation for DSI and DDI encoders.
+	 * For first time during driver init, encoder association
+	 * would not have happened and this function will return
+	 * false. This will cause encoder to be disabled causing
+	 * a blank, till user space does a modeset. In order to avoid
+	 * this, if DSI is enabled in VBT, for first iteration, this
+	 * will return true since BIOS would have initialized MIPI.
+	 * This is needed for seamless booting without blanking and
+	 * for Charging OS scenario. Since DSI is the first display in
+	 * setup_outputs, hence first crtc will be associated with mipi
+	 * display
 	 */
-	if (is_dsi) {
+	if (is_dsi || (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)) {
 		struct intel_encoder *encoder;
 
 		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
@@ -9852,8 +9865,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 			if (dsi_enc_enabled)
 				break;
 		}
-		if (!dsi_enc_enabled)
-			return false;
+
+		 if (!dsi_enc_enabled) {
+			if (!dev_priv->dsi_enumerating)
+				return false;
+		}
 	} else {
 
 		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
@@ -9921,6 +9937,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->pixel_multiplier = 1;
 	}
 
+	dev_priv->dsi_enumerating = false;
+
 	return true;
 }
 
@@ -14877,6 +14895,7 @@ void intel_modeset_init(struct drm_device *dev)
 	enum pipe pipe;
 	struct intel_crtc *crtc;
 
+	dev_priv->dsi_enumerating = true;
 	drm_mode_config_init(dev);
 
 	dev->mode_config.min_width = 0;
-- 
1.7.9.5

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI
  2015-10-12 17:23   ` Ville Syrjälä
@ 2015-10-13 11:03     ` Shankar, Uma
  2015-10-13 11:24       ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Shankar, Uma @ 2015-10-13 11:03 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Kumar, Shobhit, intel-gfx



>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Monday, October 12, 2015 10:54 PM
>To: Shankar, Uma
>Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
>Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe
>timing for BXT DSI
>
>On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote:
>> For BXT DSI, vtotal, vactive, hactive registers are different.
>> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to
>> read the correct registers for BXT DSI.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c |   48
>+++++++++++++++++++++++++++++++---
>>  1 file changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 75c60b8..2717823 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(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;
>> +	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>>  	uint32_t tmp;
>>
>>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
>> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc
>*crtc,
>>  		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>>  	}
>>
>> +	if (IS_BROXTON(dev) && is_dsi) {
>> +		struct intel_encoder *encoder;
>> +
>> +		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>> +			struct intel_dsi *intel_dsi =
>> +				enc_to_intel_dsi(&encoder->base);
>> +			enum port port;
>> +
>> +			for_each_dsi_port(port, intel_dsi->ports) {
>> +				pipe_config->base.adjusted_mode.crtc_hdisplay
>=
>> +
>	I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
>> +				pipe_config->base.adjusted_mode.crtc_vdisplay
>=
>> +
>	I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
>> +				pipe_config->base.adjusted_mode.crtc_vtotal =
>> +
>	I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
>> +			}
>> +		}
>> +
>> +	}
>
>I think I already asked this earlier when this patch was posted; Don't the normal
>pipe timing registers contain the same values? And if so, why would you need to
>read them out from the DSI speciific registers?
>

Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI for BXT. Hence getting the data from
DSI specific registers for BXT. Separate MIPI transcoder is used for BXT.

Regards,
Uma Shankar

>> +
>>  	tmp = I915_READ(PIPESRC(crtc->pipe));
>>  	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
>>  	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6
>> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct
>drm_device *dev,
>>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
>>  	int vsync = I915_READ(VSYNC(cpu_transcoder));
>>  	enum pipe pipe = intel_crtc->pipe;
>> +	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>>
>>  	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
>>  	if (!mode)
>> @@ -10684,15 +10706,35 @@ struct drm_display_mode
>*intel_crtc_mode_get(struct drm_device *dev,
>>  	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
>>
>>  	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
>> -	mode->hdisplay = (htot & 0xffff) + 1;
>>  	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
>>  	mode->hsync_start = (hsync & 0xffff) + 1;
>>  	mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
>> -	mode->vdisplay = (vtot & 0xffff) + 1;
>> -	mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
>>  	mode->vsync_start = (vsync & 0xffff) + 1;
>>  	mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
>>
>> +	if (IS_BROXTON(dev) && is_dsi) {
>> +		struct intel_encoder *encoder;
>> +
>> +		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
>> +			struct intel_dsi *intel_dsi =
>> +				enc_to_intel_dsi(&encoder->base);
>> +			enum port port;
>> +
>> +			for_each_dsi_port(port, intel_dsi->ports) {
>> +				mode->vtotal =
>> +
>	I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
>> +				mode->hdisplay =
>> +
>	I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
>> +				mode->vdisplay =
>> +
>	I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
>> +			}
>> +		}
>> +	} else {
>> +		mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
>> +		mode->hdisplay = (htot & 0xffff) + 1;
>> +		mode->vdisplay = (vtot & 0xffff) + 1;
>> +	}
>> +
>>  	drm_mode_set_name(mode);
>>
>>  	return mode;
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI
  2015-10-13 11:03     ` Shankar, Uma
@ 2015-10-13 11:24       ` Ville Syrjälä
  2015-10-13 13:24         ` Shankar, Uma
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2015-10-13 11:24 UTC (permalink / raw)
  To: Shankar, Uma; +Cc: Kumar, Shobhit, intel-gfx

On Tue, Oct 13, 2015 at 11:03:27AM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >Sent: Monday, October 12, 2015 10:54 PM
> >To: Shankar, Uma
> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe
> >timing for BXT DSI
> >
> >On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote:
> >> For BXT DSI, vtotal, vactive, hactive registers are different.
> >> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to
> >> read the correct registers for BXT DSI.
> >>
> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c |   48
> >+++++++++++++++++++++++++++++++---
> >>  1 file changed, 45 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 75c60b8..2717823 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(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;
> >> +	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
> >>  	uint32_t tmp;
> >>
> >>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
> >> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc
> >*crtc,
> >>  		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
> >>  	}
> >>
> >> +	if (IS_BROXTON(dev) && is_dsi) {
> >> +		struct intel_encoder *encoder;
> >> +
> >> +		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> >> +			struct intel_dsi *intel_dsi =
> >> +				enc_to_intel_dsi(&encoder->base);
> >> +			enum port port;
> >> +
> >> +			for_each_dsi_port(port, intel_dsi->ports) {
> >> +				pipe_config->base.adjusted_mode.crtc_hdisplay
> >=
> >> +
> >	I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> >> +				pipe_config->base.adjusted_mode.crtc_vdisplay
> >=
> >> +
> >	I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> >> +				pipe_config->base.adjusted_mode.crtc_vtotal =
> >> +
> >	I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> >> +			}
> >> +		}
> >> +
> >> +	}
> >
> >I think I already asked this earlier when this patch was posted; Don't the normal
> >pipe timing registers contain the same values? And if so, why would you need to
> >read them out from the DSI speciific registers?
> >
> 
> Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI for BXT. Hence getting the data from
> DSI specific registers for BXT. Separate MIPI transcoder is used for BXT.

So calling intel_set_pipe_timings() is pointless (apart from PIPESRC)?

If that's the case then it seems the readback should also read all of
it from the DSI registers instead of reading some from the pipe timings
and some from DSI registers.

> 
> Regards,
> Uma Shankar
> 
> >> +
> >>  	tmp = I915_READ(PIPESRC(crtc->pipe));
> >>  	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> >>  	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6
> >> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct
> >drm_device *dev,
> >>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
> >>  	int vsync = I915_READ(VSYNC(cpu_transcoder));
> >>  	enum pipe pipe = intel_crtc->pipe;
> >> +	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
> >>
> >>  	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> >>  	if (!mode)
> >> @@ -10684,15 +10706,35 @@ struct drm_display_mode
> >*intel_crtc_mode_get(struct drm_device *dev,
> >>  	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
> >>
> >>  	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
> >> -	mode->hdisplay = (htot & 0xffff) + 1;
> >>  	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
> >>  	mode->hsync_start = (hsync & 0xffff) + 1;
> >>  	mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
> >> -	mode->vdisplay = (vtot & 0xffff) + 1;
> >> -	mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> >>  	mode->vsync_start = (vsync & 0xffff) + 1;
> >>  	mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
> >>
> >> +	if (IS_BROXTON(dev) && is_dsi) {
> >> +		struct intel_encoder *encoder;
> >> +
> >> +		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
> >> +			struct intel_dsi *intel_dsi =
> >> +				enc_to_intel_dsi(&encoder->base);
> >> +			enum port port;
> >> +
> >> +			for_each_dsi_port(port, intel_dsi->ports) {
> >> +				mode->vtotal =
> >> +
> >	I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> >> +				mode->hdisplay =
> >> +
> >	I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> >> +				mode->vdisplay =
> >> +
> >	I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> >> +			}
> >> +		}
> >> +	} else {
> >> +		mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> >> +		mode->hdisplay = (htot & 0xffff) + 1;
> >> +		mode->vdisplay = (vtot & 0xffff) + 1;
> >> +	}
> >> +
> >>  	drm_mode_set_name(mode);
> >>
> >>  	return mode;
> >> --
> >> 1.7.9.5
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >--
> >Ville Syrjälä
> >Intel OTC

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI
  2015-10-13 11:24       ` Ville Syrjälä
@ 2015-10-13 13:24         ` Shankar, Uma
  2015-10-13 15:45           ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Shankar, Uma @ 2015-10-13 13:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Kumar, Shobhit, intel-gfx



>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, October 13, 2015 4:54 PM
>To: Shankar, Uma
>Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
>Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe
>timing for BXT DSI
>
>On Tue, Oct 13, 2015 at 11:03:27AM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>> >Sent: Monday, October 12, 2015 10:54 PM
>> >To: Shankar, Uma
>> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
>> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt:
>> >Get pipe timing for BXT DSI
>> >
>> >On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote:
>> >> For BXT DSI, vtotal, vactive, hactive registers are different.
>> >> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to
>> >> read the correct registers for BXT DSI.
>> >>
>> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/intel_display.c |   48
>> >+++++++++++++++++++++++++++++++---
>> >>  1 file changed, 45 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> >> b/drivers/gpu/drm/i915/intel_display.c
>> >> index 75c60b8..2717823 100644
>> >> --- a/drivers/gpu/drm/i915/intel_display.c
>> >> +++ b/drivers/gpu/drm/i915/intel_display.c
>> >> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(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;
>> >> +	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>> >>  	uint32_t tmp;
>> >>
>> >>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
>> >> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct
>> >> intel_crtc
>> >*crtc,
>> >>  		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>> >>  	}
>> >>
>> >> +	if (IS_BROXTON(dev) && is_dsi) {
>> >> +		struct intel_encoder *encoder;
>> >> +
>> >> +		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>> >> +			struct intel_dsi *intel_dsi =
>> >> +				enc_to_intel_dsi(&encoder->base);
>> >> +			enum port port;
>> >> +
>> >> +			for_each_dsi_port(port, intel_dsi->ports) {
>> >> +				pipe_config->base.adjusted_mode.crtc_hdisplay
>> >=
>> >> +
>> >	I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
>> >> +				pipe_config->base.adjusted_mode.crtc_vdisplay
>> >=
>> >> +
>> >	I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
>> >> +				pipe_config->base.adjusted_mode.crtc_vtotal =
>> >> +
>> >	I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
>> >> +			}
>> >> +		}
>> >> +
>> >> +	}
>> >
>> >I think I already asked this earlier when this patch was posted;
>> >Don't the normal pipe timing registers contain the same values? And
>> >if so, why would you need to read them out from the DSI speciific registers?
>> >
>>
>> Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI
>> for BXT. Hence getting the data from DSI specific registers for BXT. Separate
>MIPI transcoder is used for BXT.
>
>So calling intel_set_pipe_timings() is pointless (apart from PIPESRC)?
>
>If that's the case then it seems the readback should also read all of it from the
>DSI registers instead of reading some from the pipe timings and some from DSI
>registers.
>

Yes set_pipe_timings is doing only the pipesrc programming which is useful for DSI, rest is not needed.

Readback can be rectified to read all other details as well from DSI specific registers apart from the ones
currently being used.

>>
>> Regards,
>> Uma Shankar
>>
>> >> +
>> >>  	tmp = I915_READ(PIPESRC(crtc->pipe));
>> >>  	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
>> >>  	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6
>> >> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct
>> >drm_device *dev,
>> >>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
>> >>  	int vsync = I915_READ(VSYNC(cpu_transcoder));
>> >>  	enum pipe pipe = intel_crtc->pipe;
>> >> +	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>> >>
>> >>  	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
>> >>  	if (!mode)
>> >> @@ -10684,15 +10706,35 @@ struct drm_display_mode
>> >*intel_crtc_mode_get(struct drm_device *dev,
>> >>  	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
>> >>
>> >>  	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
>> >> -	mode->hdisplay = (htot & 0xffff) + 1;
>> >>  	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
>> >>  	mode->hsync_start = (hsync & 0xffff) + 1;
>> >>  	mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
>> >> -	mode->vdisplay = (vtot & 0xffff) + 1;
>> >> -	mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
>> >>  	mode->vsync_start = (vsync & 0xffff) + 1;
>> >>  	mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
>> >>
>> >> +	if (IS_BROXTON(dev) && is_dsi) {
>> >> +		struct intel_encoder *encoder;
>> >> +
>> >> +		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
>> >> +			struct intel_dsi *intel_dsi =
>> >> +				enc_to_intel_dsi(&encoder->base);
>> >> +			enum port port;
>> >> +
>> >> +			for_each_dsi_port(port, intel_dsi->ports) {
>> >> +				mode->vtotal =
>> >> +
>> >	I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
>> >> +				mode->hdisplay =
>> >> +
>> >	I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
>> >> +				mode->vdisplay =
>> >> +
>> >	I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
>> >> +			}
>> >> +		}
>> >> +	} else {
>> >> +		mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
>> >> +		mode->hdisplay = (htot & 0xffff) + 1;
>> >> +		mode->vdisplay = (vtot & 0xffff) + 1;
>> >> +	}
>> >> +
>> >>  	drm_mode_set_name(mode);
>> >>
>> >>  	return mode;
>> >> --
>> >> 1.7.9.5
>> >>
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> >--
>> >Ville Syrjälä
>> >Intel OTC
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI
  2015-10-13 13:24         ` Shankar, Uma
@ 2015-10-13 15:45           ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2015-10-13 15:45 UTC (permalink / raw)
  To: Shankar, Uma; +Cc: Kumar, Shobhit, intel-gfx

On Tue, Oct 13, 2015 at 01:24:27PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >Sent: Tuesday, October 13, 2015 4:54 PM
> >To: Shankar, Uma
> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe
> >timing for BXT DSI
> >
> >On Tue, Oct 13, 2015 at 11:03:27AM +0000, Shankar, Uma wrote:
> >>
> >>
> >> >-----Original Message-----
> >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >> >Sent: Monday, October 12, 2015 10:54 PM
> >> >To: Shankar, Uma
> >> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
> >> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 2/3] drm/i915/bxt:
> >> >Get pipe timing for BXT DSI
> >> >
> >> >On Mon, Oct 12, 2015 at 10:55:02PM +0530, Uma Shankar wrote:
> >> >> For BXT DSI, vtotal, vactive, hactive registers are different.
> >> >> Making changes to intel_crtc_mode_get() and get_pipe_timings(), to
> >> >> read the correct registers for BXT DSI.
> >> >>
> >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> >> >> ---
> >> >>  drivers/gpu/drm/i915/intel_display.c |   48
> >> >+++++++++++++++++++++++++++++++---
> >> >>  1 file changed, 45 insertions(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> >> b/drivers/gpu/drm/i915/intel_display.c
> >> >> index 75c60b8..2717823 100644
> >> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> >> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(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;
> >> >> +	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
> >> >>  	uint32_t tmp;
> >> >>
> >> >>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
> >> >> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct
> >> >> intel_crtc
> >> >*crtc,
> >> >>  		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
> >> >>  	}
> >> >>
> >> >> +	if (IS_BROXTON(dev) && is_dsi) {
> >> >> +		struct intel_encoder *encoder;
> >> >> +
> >> >> +		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> >> >> +			struct intel_dsi *intel_dsi =
> >> >> +				enc_to_intel_dsi(&encoder->base);
> >> >> +			enum port port;
> >> >> +
> >> >> +			for_each_dsi_port(port, intel_dsi->ports) {
> >> >> +				pipe_config->base.adjusted_mode.crtc_hdisplay
> >> >=
> >> >> +
> >> >	I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> >> >> +				pipe_config->base.adjusted_mode.crtc_vdisplay
> >> >=
> >> >> +
> >> >	I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> >> >> +				pipe_config->base.adjusted_mode.crtc_vtotal =
> >> >> +
> >> >	I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> >> >> +			}
> >> >> +		}
> >> >> +
> >> >> +	}
> >> >
> >> >I think I already asked this earlier when this patch was posted;
> >> >Don't the normal pipe timing registers contain the same values? And
> >> >if so, why would you need to read them out from the DSI speciific registers?
> >> >
> >>
> >> Normal pipe timing registers like HTOTAL etc. is not used by MIPI DSI
> >> for BXT. Hence getting the data from DSI specific registers for BXT. Separate
> >MIPI transcoder is used for BXT.
> >
> >So calling intel_set_pipe_timings() is pointless (apart from PIPESRC)?
> >
> >If that's the case then it seems the readback should also read all of it from the
> >DSI registers instead of reading some from the pipe timings and some from DSI
> >registers.
> >
> 
> Yes set_pipe_timings is doing only the pipesrc programming which is useful for DSI, rest is not needed.
> 
> Readback can be rectified to read all other details as well from DSI specific registers apart from the ones
> currently being used.

Also this kind of is_dsi check is a bit a hack and won't really work for
fastboot case (intel_pipe_has_type doesn't work at that point yet).
Instead you need to push this into a dsi-specific readout function.
-Daniel

> 
> >>
> >> Regards,
> >> Uma Shankar
> >>
> >> >> +
> >> >>  	tmp = I915_READ(PIPESRC(crtc->pipe));
> >> >>  	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> >> >>  	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1; @@ -10664,6
> >> >> +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct
> >> >drm_device *dev,
> >> >>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
> >> >>  	int vsync = I915_READ(VSYNC(cpu_transcoder));
> >> >>  	enum pipe pipe = intel_crtc->pipe;
> >> >> +	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
> >> >>
> >> >>  	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> >> >>  	if (!mode)
> >> >> @@ -10684,15 +10706,35 @@ struct drm_display_mode
> >> >*intel_crtc_mode_get(struct drm_device *dev,
> >> >>  	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
> >> >>
> >> >>  	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
> >> >> -	mode->hdisplay = (htot & 0xffff) + 1;
> >> >>  	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
> >> >>  	mode->hsync_start = (hsync & 0xffff) + 1;
> >> >>  	mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
> >> >> -	mode->vdisplay = (vtot & 0xffff) + 1;
> >> >> -	mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> >> >>  	mode->vsync_start = (vsync & 0xffff) + 1;
> >> >>  	mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
> >> >>
> >> >> +	if (IS_BROXTON(dev) && is_dsi) {
> >> >> +		struct intel_encoder *encoder;
> >> >> +
> >> >> +		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
> >> >> +			struct intel_dsi *intel_dsi =
> >> >> +				enc_to_intel_dsi(&encoder->base);
> >> >> +			enum port port;
> >> >> +
> >> >> +			for_each_dsi_port(port, intel_dsi->ports) {
> >> >> +				mode->vtotal =
> >> >> +
> >> >	I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> >> >> +				mode->hdisplay =
> >> >> +
> >> >	I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> >> >> +				mode->vdisplay =
> >> >> +
> >> >	I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> >> >> +			}
> >> >> +		}
> >> >> +	} else {
> >> >> +		mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> >> >> +		mode->hdisplay = (htot & 0xffff) + 1;
> >> >> +		mode->vdisplay = (vtot & 0xffff) + 1;
> >> >> +	}
> >> >> +
> >> >>  	drm_mode_set_name(mode);
> >> >>
> >> >>  	return mode;
> >> >> --
> >> >> 1.7.9.5
> >> >>
> >> >> _______________________________________________
> >> >> Intel-gfx mailing list
> >> >> Intel-gfx@lists.freedesktop.org
> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >
> >> >--
> >> >Ville Syrjälä
> >> >Intel OTC
> >
> >--
> >Ville Syrjälä
> >Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup Uma Shankar
@ 2015-10-13 15:47   ` Daniel Vetter
  2015-10-14 10:20     ` Shankar, Uma
  2016-01-18 13:07   ` Mika Kahola
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-10-13 15:47 UTC (permalink / raw)
  To: Uma Shankar; +Cc: shobhit.kumar, intel-gfx

On Mon, Oct 12, 2015 at 10:55:03PM +0530, Uma Shankar wrote:
> During bootup, mipi display was blanking in BXT. This is because during
> driver load, though encoder, connector were active but crtc returned
> inactive. This caused sanitize function to disable the DSI panel. In AOS,
> this is fine since HWC will do a modeset and crtc, connector, encoder
> mapping will be restored. But in Charging OS, no modeset is called, it
> just calls DPMS ON/OFF. Hence display doesn't come in COS. This is needed
> even for seamless booting to Android without a blank.
> 
> This is fine on BYT/CHT since transcoder is common b/w all encoders. But
> for BXT, there is a separate mipi transcoder. Hence this needs special
> handling for BXT DSI.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    3 +++
>  drivers/gpu/drm/i915/intel_display.c |   27 +++++++++++++++++++++++----
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bf14096..ae790ec 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1948,6 +1948,9 @@ struct drm_i915_private {
>  	/* perform PHY state sanity checks? */
>  	bool chv_phy_assert[2];
>  
> +	/* To check if DSI is initializing at bootup */
> +	bool dsi_enumerating;

See my other comment, you're working around the broken design of patch 2
here. Special casing fastboot is a big no-no which needs some really good
reasons. An example is the special edp clock readout code in the encoder
callbacks we have for ilk-ivb cpu edp.
-Daniel

> +
>  	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2717823..fe4f4f3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7711,6 +7711,9 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>  	uint32_t tmp;
>  
> +	if (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)
> +		is_dsi = true;
> +
>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
>  	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
>  	pipe_config->base.adjusted_mode.crtc_htotal = ((tmp >> 16) & 0xffff) + 1;
> @@ -9825,10 +9828,20 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>  
>  	/*
> -	 * Check if encoder is enabled or not
> +	 * Check if encoder is enabled or not.
>  	 * Separate implementation for DSI and DDI encoders.
> +	 * For first time during driver init, encoder association
> +	 * would not have happened and this function will return
> +	 * false. This will cause encoder to be disabled causing
> +	 * a blank, till user space does a modeset. In order to avoid
> +	 * this, if DSI is enabled in VBT, for first iteration, this
> +	 * will return true since BIOS would have initialized MIPI.
> +	 * This is needed for seamless booting without blanking and
> +	 * for Charging OS scenario. Since DSI is the first display in
> +	 * setup_outputs, hence first crtc will be associated with mipi
> +	 * display
>  	 */
> -	if (is_dsi) {
> +	if (is_dsi || (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)) {
>  		struct intel_encoder *encoder;
>  
>  		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> @@ -9852,8 +9865,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  			if (dsi_enc_enabled)
>  				break;
>  		}
> -		if (!dsi_enc_enabled)
> -			return false;
> +
> +		 if (!dsi_enc_enabled) {
> +			if (!dev_priv->dsi_enumerating)
> +				return false;
> +		}
>  	} else {
>  
>  		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> @@ -9921,6 +9937,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier = 1;
>  	}
>  
> +	dev_priv->dsi_enumerating = false;
> +
>  	return true;
>  }
>  
> @@ -14877,6 +14895,7 @@ void intel_modeset_init(struct drm_device *dev)
>  	enum pipe pipe;
>  	struct intel_crtc *crtc;
>  
> +	dev_priv->dsi_enumerating = true;
>  	drm_mode_config_init(dev);
>  
>  	dev->mode_config.min_width = 0;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup
  2015-10-13 15:47   ` Daniel Vetter
@ 2015-10-14 10:20     ` Shankar, Uma
  2015-10-14 12:49       ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Shankar, Uma @ 2015-10-14 10:20 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Kumar, Shobhit, intel-gfx



>-----Original Message-----
>From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
>Sent: Tuesday, October 13, 2015 9:17 PM
>To: Shankar, Uma
>Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
>Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc
>disable and blank at bootup
>
>On Mon, Oct 12, 2015 at 10:55:03PM +0530, Uma Shankar wrote:
>> During bootup, mipi display was blanking in BXT. This is because
>> during driver load, though encoder, connector were active but crtc
>> returned inactive. This caused sanitize function to disable the DSI
>> panel. In AOS, this is fine since HWC will do a modeset and crtc,
>> connector, encoder mapping will be restored. But in Charging OS, no
>> modeset is called, it just calls DPMS ON/OFF. Hence display doesn't
>> come in COS. This is needed even for seamless booting to Android without a
>blank.
>>
>> This is fine on BYT/CHT since transcoder is common b/w all encoders.
>> But for BXT, there is a separate mipi transcoder. Hence this needs
>> special handling for BXT DSI.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h      |    3 +++
>>  drivers/gpu/drm/i915/intel_display.c |   27 +++++++++++++++++++++++----
>>  2 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h index bf14096..ae790ec 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1948,6 +1948,9 @@ struct drm_i915_private {
>>  	/* perform PHY state sanity checks? */
>>  	bool chv_phy_assert[2];
>>
>> +	/* To check if DSI is initializing at bootup */
>> +	bool dsi_enumerating;
>
>See my other comment, you're working around the broken design of patch 2
>here. Special casing fastboot is a big no-no which needs some really good
>reasons. An example is the special edp clock readout code in the encoder
>callbacks we have for ilk-ivb cpu edp.
>-Daniel
>

I agree this is not a clean solution but was not getting any better ideas. As part of driver load and initialization, readout_state returns 
encoder, connector as active but all crtc return inactive. This make sanitize encoder to consider this as inconsistent hw state and it goes
ahead and disables encoder, thereby disabling the BIOS/GOP Initialization.

After this only way to recover is a modeset which doesn't come in cases like Charging OS in Android. Also this will create issues in having
a seamless boot without a blank (a must have for Android devices).

For EDP case, we can read DDI_FUNC_CTL and get a pipe number and match it with crtc->pipe to detect EDP is on that pipe and map it to crtc.

In DSI there is no such mechanism to detect this. Can you suggest some pointers as to how to approach this issue. 

Thanks & Regards,
Uma Shankar
>> +
>>  	/*
>>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>>  	 * will be rejected. Instead look for a better place.
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 2717823..fe4f4f3 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7711,6 +7711,9 @@ static void intel_get_pipe_timings(struct intel_crtc
>*crtc,
>>  	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>>  	uint32_t tmp;
>>
>> +	if (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)
>> +		is_dsi = true;
>> +
>>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
>>  	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
>>  	pipe_config->base.adjusted_mode.crtc_htotal = ((tmp >> 16) & 0xffff)
>> + 1; @@ -9825,10 +9828,20 @@ static bool haswell_get_pipe_config(struct
>intel_crtc *crtc,
>>  	is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>>
>>  	/*
>> -	 * Check if encoder is enabled or not
>> +	 * Check if encoder is enabled or not.
>>  	 * Separate implementation for DSI and DDI encoders.
>> +	 * For first time during driver init, encoder association
>> +	 * would not have happened and this function will return
>> +	 * false. This will cause encoder to be disabled causing
>> +	 * a blank, till user space does a modeset. In order to avoid
>> +	 * this, if DSI is enabled in VBT, for first iteration, this
>> +	 * will return true since BIOS would have initialized MIPI.
>> +	 * This is needed for seamless booting without blanking and
>> +	 * for Charging OS scenario. Since DSI is the first display in
>> +	 * setup_outputs, hence first crtc will be associated with mipi
>> +	 * display
>>  	 */
>> -	if (is_dsi) {
>> +	if (is_dsi || (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi))
>> +{
>>  		struct intel_encoder *encoder;
>>
>>  		for_each_encoder_on_crtc(dev, &crtc->base, encoder) { @@ -
>9852,8
>> +9865,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>  			if (dsi_enc_enabled)
>>  				break;
>>  		}
>> -		if (!dsi_enc_enabled)
>> -			return false;
>> +
>> +		 if (!dsi_enc_enabled) {
>> +			if (!dev_priv->dsi_enumerating)
>> +				return false;
>> +		}
>>  	} else {
>>
>>  		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
>> @@ -9921,6 +9937,8 @@ static bool haswell_get_pipe_config(struct intel_crtc
>*crtc,
>>  		pipe_config->pixel_multiplier = 1;
>>  	}
>>
>> +	dev_priv->dsi_enumerating = false;
>> +
>>  	return true;
>>  }
>>
>> @@ -14877,6 +14895,7 @@ void intel_modeset_init(struct drm_device *dev)
>>  	enum pipe pipe;
>>  	struct intel_crtc *crtc;
>>
>> +	dev_priv->dsi_enumerating = true;
>>  	drm_mode_config_init(dev);
>>
>>  	dev->mode_config.min_width = 0;
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup
  2015-10-14 10:20     ` Shankar, Uma
@ 2015-10-14 12:49       ` Daniel Vetter
  2015-10-14 16:37         ` Shankar, Uma
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-10-14 12:49 UTC (permalink / raw)
  To: Shankar, Uma; +Cc: Kumar, Shobhit, intel-gfx

On Wed, Oct 14, 2015 at 10:20:32AM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> >Sent: Tuesday, October 13, 2015 9:17 PM
> >To: Shankar, Uma
> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc
> >disable and blank at bootup
> >
> >On Mon, Oct 12, 2015 at 10:55:03PM +0530, Uma Shankar wrote:
> >> During bootup, mipi display was blanking in BXT. This is because
> >> during driver load, though encoder, connector were active but crtc
> >> returned inactive. This caused sanitize function to disable the DSI
> >> panel. In AOS, this is fine since HWC will do a modeset and crtc,
> >> connector, encoder mapping will be restored. But in Charging OS, no
> >> modeset is called, it just calls DPMS ON/OFF. Hence display doesn't
> >> come in COS. This is needed even for seamless booting to Android without a
> >blank.
> >>
> >> This is fine on BYT/CHT since transcoder is common b/w all encoders.
> >> But for BXT, there is a separate mipi transcoder. Hence this needs
> >> special handling for BXT DSI.
> >>
> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_drv.h      |    3 +++
> >>  drivers/gpu/drm/i915/intel_display.c |   27 +++++++++++++++++++++++----
> >>  2 files changed, 26 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> b/drivers/gpu/drm/i915/i915_drv.h index bf14096..ae790ec 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -1948,6 +1948,9 @@ struct drm_i915_private {
> >>  	/* perform PHY state sanity checks? */
> >>  	bool chv_phy_assert[2];
> >>
> >> +	/* To check if DSI is initializing at bootup */
> >> +	bool dsi_enumerating;
> >
> >See my other comment, you're working around the broken design of patch 2
> >here. Special casing fastboot is a big no-no which needs some really good
> >reasons. An example is the special edp clock readout code in the encoder
> >callbacks we have for ilk-ivb cpu edp.
> >-Daniel
> >
> 
> I agree this is not a clean solution but was not getting any better
> ideas. As part of driver load and initialization, readout_state returns
> encoder, connector as active but all crtc return inactive. This make
> sanitize encoder to consider this as inconsistent hw state and it goes
> ahead and disables encoder, thereby disabling the BIOS/GOP
> Initialization.
> 
> After this only way to recover is a modeset which doesn't come in cases
> like Charging OS in Android. Also this will create issues in having a
> seamless boot without a blank (a must have for Android devices).
> 
> For EDP case, we can read DDI_FUNC_CTL and get a pipe number and match
> it with crtc->pipe to detect EDP is on that pipe and map it to crtc.
> 
> In DSI there is no such mechanism to detect this. Can you suggest some
> pointers as to how to approach this issue. 

MIPI_CTRL(port) has BXT_PIPE_SELECT_A/C bits. Which btw we don't ever seem
to check anywhere in intel_dsi_compute_config, which is a pretty stellar
bug that running kms_flip even once should have caught.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup
  2015-10-14 12:49       ` Daniel Vetter
@ 2015-10-14 16:37         ` Shankar, Uma
  2015-10-14 17:18           ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Shankar, Uma @ 2015-10-14 16:37 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Kumar, Shobhit, intel-gfx



>-----Original Message-----
>From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
>Sent: Wednesday, October 14, 2015 6:20 PM
>To: Shankar, Uma
>Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Kumar, Shobhit
>Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc
>disable and blank at bootup
>
>On Wed, Oct 14, 2015 at 10:20:32AM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of
>> >Daniel Vetter
>> >Sent: Tuesday, October 13, 2015 9:17 PM
>> >To: Shankar, Uma
>> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
>> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 3/3] drm/i915/bxt:
>> >Fixed dsi enc disable and blank at bootup
>> >
>> >On Mon, Oct 12, 2015 at 10:55:03PM +0530, Uma Shankar wrote:
>> >> During bootup, mipi display was blanking in BXT. This is because
>> >> during driver load, though encoder, connector were active but crtc
>> >> returned inactive. This caused sanitize function to disable the DSI
>> >> panel. In AOS, this is fine since HWC will do a modeset and crtc,
>> >> connector, encoder mapping will be restored. But in Charging OS, no
>> >> modeset is called, it just calls DPMS ON/OFF. Hence display doesn't
>> >> come in COS. This is needed even for seamless booting to Android
>> >> without a
>> >blank.
>> >>
>> >> This is fine on BYT/CHT since transcoder is common b/w all encoders.
>> >> But for BXT, there is a separate mipi transcoder. Hence this needs
>> >> special handling for BXT DSI.
>> >>
>> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/i915_drv.h      |    3 +++
>> >>  drivers/gpu/drm/i915/intel_display.c |   27 +++++++++++++++++++++++----
>> >>  2 files changed, 26 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> >> b/drivers/gpu/drm/i915/i915_drv.h index bf14096..ae790ec 100644
>> >> --- a/drivers/gpu/drm/i915/i915_drv.h
>> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> >> @@ -1948,6 +1948,9 @@ struct drm_i915_private {
>> >>  	/* perform PHY state sanity checks? */
>> >>  	bool chv_phy_assert[2];
>> >>
>> >> +	/* To check if DSI is initializing at bootup */
>> >> +	bool dsi_enumerating;
>> >
>> >See my other comment, you're working around the broken design of
>> >patch 2 here. Special casing fastboot is a big no-no which needs some
>> >really good reasons. An example is the special edp clock readout code
>> >in the encoder callbacks we have for ilk-ivb cpu edp.
>> >-Daniel
>> >
>>
>> I agree this is not a clean solution but was not getting any better
>> ideas. As part of driver load and initialization, readout_state
>> returns encoder, connector as active but all crtc return inactive.
>> This make sanitize encoder to consider this as inconsistent hw state
>> and it goes ahead and disables encoder, thereby disabling the BIOS/GOP
>> Initialization.
>>
>> After this only way to recover is a modeset which doesn't come in
>> cases like Charging OS in Android. Also this will create issues in
>> having a seamless boot without a blank (a must have for Android devices).
>>
>> For EDP case, we can read DDI_FUNC_CTL and get a pipe number and match
>> it with crtc->pipe to detect EDP is on that pipe and map it to crtc.
>>
>> In DSI there is no such mechanism to detect this. Can you suggest some
>> pointers as to how to approach this issue.
>
>MIPI_CTRL(port) has BXT_PIPE_SELECT_A/C bits. Which btw we don't ever seem
>to check anywhere in intel_dsi_compute_config, which is a pretty stellar bug
>that running kms_flip even once should have caught.
>-Daniel
>--

These fields were removed from the port control register in the bspec updates. These fields remained in the code
and should be cleaned up and removed. As per latest bspec status, there is no pipe info in the port control for BXT DSI.

I am not sure, we could add something like below in compute_config to update transcoder info:

if (IS_BROXTON(dev)) {
+               if (intel_dsi->ports & (1 << PORT_A))
+                       config->cpu_transcoder = TRANSCODER_MIPI_A;
+               else
+                       config->cpu_transcoder = TRANSCODER_MIPI_C;
+       }

Thoughts ?

Regards,
Uma Shankar

>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup
  2015-10-14 16:37         ` Shankar, Uma
@ 2015-10-14 17:18           ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2015-10-14 17:18 UTC (permalink / raw)
  To: Shankar, Uma; +Cc: Kumar, Shobhit, intel-gfx

On Wed, Oct 14, 2015 at 04:37:27PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> >Sent: Wednesday, October 14, 2015 6:20 PM
> >To: Shankar, Uma
> >Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Kumar, Shobhit
> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc
> >disable and blank at bootup
> >
> >On Wed, Oct 14, 2015 at 10:20:32AM +0000, Shankar, Uma wrote:
> >>
> >>
> >> >-----Original Message-----
> >> >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of
> >> >Daniel Vetter
> >> >Sent: Tuesday, October 13, 2015 9:17 PM
> >> >To: Shankar, Uma
> >> >Cc: intel-gfx@lists.freedesktop.org; Kumar, Shobhit
> >> >Subject: Re: [Intel-gfx] [BXT DSI timing fixes v1 3/3] drm/i915/bxt:
> >> >Fixed dsi enc disable and blank at bootup
> >> >
> >> >On Mon, Oct 12, 2015 at 10:55:03PM +0530, Uma Shankar wrote:
> >> >> During bootup, mipi display was blanking in BXT. This is because
> >> >> during driver load, though encoder, connector were active but crtc
> >> >> returned inactive. This caused sanitize function to disable the DSI
> >> >> panel. In AOS, this is fine since HWC will do a modeset and crtc,
> >> >> connector, encoder mapping will be restored. But in Charging OS, no
> >> >> modeset is called, it just calls DPMS ON/OFF. Hence display doesn't
> >> >> come in COS. This is needed even for seamless booting to Android
> >> >> without a
> >> >blank.
> >> >>
> >> >> This is fine on BYT/CHT since transcoder is common b/w all encoders.
> >> >> But for BXT, there is a separate mipi transcoder. Hence this needs
> >> >> special handling for BXT DSI.
> >> >>
> >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> >> ---
> >> >>  drivers/gpu/drm/i915/i915_drv.h      |    3 +++
> >> >>  drivers/gpu/drm/i915/intel_display.c |   27 +++++++++++++++++++++++----
> >> >>  2 files changed, 26 insertions(+), 4 deletions(-)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> >> b/drivers/gpu/drm/i915/i915_drv.h index bf14096..ae790ec 100644
> >> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> >> @@ -1948,6 +1948,9 @@ struct drm_i915_private {
> >> >>  	/* perform PHY state sanity checks? */
> >> >>  	bool chv_phy_assert[2];
> >> >>
> >> >> +	/* To check if DSI is initializing at bootup */
> >> >> +	bool dsi_enumerating;
> >> >
> >> >See my other comment, you're working around the broken design of
> >> >patch 2 here. Special casing fastboot is a big no-no which needs some
> >> >really good reasons. An example is the special edp clock readout code
> >> >in the encoder callbacks we have for ilk-ivb cpu edp.
> >> >-Daniel
> >> >
> >>
> >> I agree this is not a clean solution but was not getting any better
> >> ideas. As part of driver load and initialization, readout_state
> >> returns encoder, connector as active but all crtc return inactive.
> >> This make sanitize encoder to consider this as inconsistent hw state
> >> and it goes ahead and disables encoder, thereby disabling the BIOS/GOP
> >> Initialization.
> >>
> >> After this only way to recover is a modeset which doesn't come in
> >> cases like Charging OS in Android. Also this will create issues in
> >> having a seamless boot without a blank (a must have for Android devices).
> >>
> >> For EDP case, we can read DDI_FUNC_CTL and get a pipe number and match
> >> it with crtc->pipe to detect EDP is on that pipe and map it to crtc.
> >>
> >> In DSI there is no such mechanism to detect this. Can you suggest some
> >> pointers as to how to approach this issue.
> >
> >MIPI_CTRL(port) has BXT_PIPE_SELECT_A/C bits. Which btw we don't ever seem
> >to check anywhere in intel_dsi_compute_config, which is a pretty stellar bug
> >that running kms_flip even once should have caught.
> >-Daniel
> >--
> 
> These fields were removed from the port control register in the bspec updates. These fields remained in the code
> and should be cleaned up and removed. As per latest bspec status, there is no pipe info in the port control for BXT DSI.
> 
> I am not sure, we could add something like below in compute_config to update transcoder info:
> 
> if (IS_BROXTON(dev)) {
> +               if (intel_dsi->ports & (1 << PORT_A))
> +                       config->cpu_transcoder = TRANSCODER_MIPI_A;
> +               else
> +                       config->cpu_transcoder = TRANSCODER_MIPI_C;
> +       }
> 
> Thoughts ?

That doesn't fix the hw state readout really - we need to fix that to
reflect reality. If dsi is enabled at boot then the corresponding crtc
should state that it's active. So on bxt we also need to check the dsi
port registers besides all the other transcoders. Maybe it makes sense to
extend the cpu_transcoder enum to include MIPI_A/C, but I'm not sure about
that.

But the bug here is in haswell_get_pipe_config. And probably we should
split that out into a broxton_get_pipe_config if the dsi stuff gets too
complicated.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup Uma Shankar
  2015-10-13 15:47   ` Daniel Vetter
@ 2016-01-18 13:07   ` Mika Kahola
  1 sibling, 0 replies; 16+ messages in thread
From: Mika Kahola @ 2016-01-18 13:07 UTC (permalink / raw)
  To: Uma Shankar; +Cc: shobhit.kumar, intel-gfx

I applied this patch for testing BXT-M I received this error message

[   16.276906] Hardware name: Intel Corp. Broxton M/RVP, BIOS
BXTM_IFWI_X64_R_2015_49_2_03 11/25/2015
[   16.286793] task: ffff8801795a2640 ti: ffff880178300000 task.ti:
ffff880178300000
[   16.295047] RIP: 0010:[<ffffffffa01154e3>]  [<ffffffffa01154e3>]
skl_update_pipe_wm+0xc3/0x870 [i915]
[   16.305284] RSP: 0018:ffff8801783036c0  EFLAGS: 00010246
[   16.311143] RAX: 0000000000000000 RBX: ffff880178317000 RCX:
0000000000000000
[   16.319013] RDX: 0000000000000000 RSI: ffff880178317760 RDI:
ffff880179660000
[   16.326882] RBP: ffff880178303760 R08: ffff88017966b8ec R09:
ffff880178317000
[   16.334754] R10: 00000000000001fc R11: 0000000000000000 R12:
ffff880178286000
[   16.342624] R13: ffff880178286000 R14: ffff88007af90800 R15:
ffff880179660000
[   16.350496] FS:  00007f2f0d3a1880(0000) GS:ffff88017fd00000(0000)
knlGS:0000000000000000
[   16.359421] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   16.365758] CR2: 00007fa7544dd020 CR3: 00000001782df000 CR4:
00000000003406e0
[   16.373629] Stack:
[   16.375842]  0000000000000006 ffff880178286000 ffffffff818996ae
0000000000000005
[   16.384013]  ffff8801795a2640 0000000000000007 0000000000000006
0000000000000002
[   16.392201]  ffff88007af90b08 ffff88017966b8ec 0000000000000000
ffffffff8189967b
[   16.400379] Call Trace:
[   16.403083]  [<ffffffff818996ae>] ? mutex_unlock+0xe/0x10
[   16.409042]  [<ffffffff8189967b>] ? __mutex_unlock_slowpath
+0x11b/0x140
[   16.416382]  [<ffffffffa01160ef>] skl_update_wm+0x16f/0x690 [i915]
[   16.423201]  [<ffffffff818996ae>] ? mutex_unlock+0xe/0x10
[   16.429216]  [<ffffffffa018d540>] ? intel_fbc_disable_crtc+0xa0/0xb0
[i915]
[   16.436943]  [<ffffffffa01176ae>] intel_update_watermarks+0x1e/0x30
[i915]
[   16.444583]  [<ffffffffa017e4ea>] intel_crtc_disable_noatomic
+0xca/0x140 [i915]
[   16.452703]  [<ffffffffa0187e07>] intel_modeset_setup_hw_state
+0xe27/0xe30 [i915]
[   16.461015]  [<ffffffffa018a935>] intel_modeset_init+0x1575/0x1920
[i915]
[   16.468569]  [<ffffffffa01c1dc3>] i915_driver_load+0x1283/0x15e0
[i915]
[   16.475894]  [<ffffffffa006d34f>] drm_dev_register+0x6f/0xb0 [drm]
[   16.482731]  [<ffffffffa006f96a>] drm_get_pci_dev+0x10a/0x1d0 [drm]
[   16.489648]  [<ffffffff8189ba11>] ? _raw_spin_unlock_irqrestore
+0x51/0x70
[   16.497177]  [<ffffffffa0103259>] i915_pci_probe+0x49/0x50 [i915]
[   16.503907]  [<ffffffff814388d0>] pci_device_probe+0x80/0xf0
[   16.510156]  [<ffffffff8150def8>] driver_probe_device+0x168/0x3d0
[   16.516881]  [<ffffffff8150e1c6>] __driver_attach+0x66/0x90
[   16.523030]  [<ffffffff8150e160>] ? driver_probe_device+0x3d0/0x3d0
[   16.529946]  [<ffffffff8150bc9b>] bus_for_each_dev+0x5b/0xa0
[   16.536190]  [<ffffffff8150d8ce>] driver_attach+0x1e/0x20
[   16.542146]  [<ffffffff8150d2b1>] bus_add_driver+0x151/0x270
[   16.548389]  [<ffffffff8150ed7c>] driver_register+0x8c/0xd0
[   16.554540]  [<ffffffff81436f60>] __pci_register_driver+0x60/0x70
[   16.561276]  [<ffffffffa006fa88>] drm_pci_init+0x58/0xf0 [drm]
[   16.567714]  [<ffffffff810cd1fd>] ? trace_hardirqs_on+0xd/0x10
[   16.574149]  [<ffffffffa0237000>] ? 0xffffffffa0237000
[   16.579855]  [<ffffffffa0237094>] i915_init+0x94/0x9b [i915]
[   16.586101]  [<ffffffff81000436>] do_one_initcall+0x106/0x1d0
[   16.592445]  [<ffffffff810e937e>] ? rcu_read_lock_sched_held
+0x6e/0xa0
[   16.599647]  [<ffffffff811da001>] ? kmem_cache_alloc_trace
+0x1c1/0x230
[   16.606853]  [<ffffffff8118c25d>] do_init_module+0x60/0x1ea
[   16.613003]  [<ffffffff81110760>] load_module+0x1df0/0x25a0
[   16.619151]  [<ffffffff8110c880>] ? store_uevent+0x40/0x40
[   16.625201]  [<ffffffff8110d065>] ? copy_module_from_fd.isra.38
+0xa5/0x140
[   16.632787]  [<ffffffff811110ff>] SyS_finit_module+0x8f/0xa0
[   16.639030]  [<ffffffff8189c29b>] entry_SYSCALL_64_fastpath+0x16/0x73
[   16.646134] Code: 00 00 74 08 49 39 cc 74 10 83 c2 01 48 8b 71 10 48
39 f0 48 8d 4e f0 75 e2 89 d0 41 0f af c2 eb 02 31 c0 8b 8f e0 b8 00 00
3 
[   16.667331] RIP  [<ffffffffa01154e3>] skl_update_pipe_wm+0xc3/0x870
[i915]
[   16.674965]  RSP <ffff8801783036c0>
[   16.681229] ---[ end trace 42f41653f5df7793 ]---

-Mika-

On Mon, 2015-10-12 at 22:55 +0530, Uma Shankar wrote:
> During bootup, mipi display was blanking in BXT. This is because during
> driver load, though encoder, connector were active but crtc returned
> inactive. This caused sanitize function to disable the DSI panel. In AOS,
> this is fine since HWC will do a modeset and crtc, connector, encoder
> mapping will be restored. But in Charging OS, no modeset is called, it
> just calls DPMS ON/OFF. Hence display doesn't come in COS. This is needed
> even for seamless booting to Android without a blank.
> 
> This is fine on BYT/CHT since transcoder is common b/w all encoders. But
> for BXT, there is a separate mipi transcoder. Hence this needs special
> handling for BXT DSI.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    3 +++
>  drivers/gpu/drm/i915/intel_display.c |   27 +++++++++++++++++++++++----
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bf14096..ae790ec 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1948,6 +1948,9 @@ struct drm_i915_private {
>  	/* perform PHY state sanity checks? */
>  	bool chv_phy_assert[2];
>  
> +	/* To check if DSI is initializing at bootup */
> +	bool dsi_enumerating;
> +
>  	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2717823..fe4f4f3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7711,6 +7711,9 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>  	uint32_t tmp;
>  
> +	if (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)
> +		is_dsi = true;
> +
>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
>  	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
>  	pipe_config->base.adjusted_mode.crtc_htotal = ((tmp >> 16) & 0xffff) + 1;
> @@ -9825,10 +9828,20 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>  
>  	/*
> -	 * Check if encoder is enabled or not
> +	 * Check if encoder is enabled or not.
>  	 * Separate implementation for DSI and DDI encoders.
> +	 * For first time during driver init, encoder association
> +	 * would not have happened and this function will return
> +	 * false. This will cause encoder to be disabled causing
> +	 * a blank, till user space does a modeset. In order to avoid
> +	 * this, if DSI is enabled in VBT, for first iteration, this
> +	 * will return true since BIOS would have initialized MIPI.
> +	 * This is needed for seamless booting without blanking and
> +	 * for Charging OS scenario. Since DSI is the first display in
> +	 * setup_outputs, hence first crtc will be associated with mipi
> +	 * display
>  	 */
> -	if (is_dsi) {
> +	if (is_dsi || (dev_priv->dsi_enumerating && dev_priv->vbt.has_mipi)) {
>  		struct intel_encoder *encoder;
>  
>  		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> @@ -9852,8 +9865,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  			if (dsi_enc_enabled)
>  				break;
>  		}
> -		if (!dsi_enc_enabled)
> -			return false;
> +
> +		 if (!dsi_enc_enabled) {
> +			if (!dev_priv->dsi_enumerating)
> +				return false;
> +		}
>  	} else {
>  
>  		tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> @@ -9921,6 +9937,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier = 1;
>  	}
>  
> +	dev_priv->dsi_enumerating = false;
> +
>  	return true;
>  }
>  
> @@ -14877,6 +14895,7 @@ void intel_modeset_init(struct drm_device *dev)
>  	enum pipe pipe;
>  	struct intel_crtc *crtc;
>  
> +	dev_priv->dsi_enumerating = true;
>  	drm_mode_config_init(dev);
>  
>  	dev->mode_config.min_width = 0;


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI
  2015-10-12 17:25 ` [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI Uma Shankar
  2015-10-12 17:23   ` Ville Syrjälä
@ 2016-01-18 13:09   ` Mika Kahola
  1 sibling, 0 replies; 16+ messages in thread
From: Mika Kahola @ 2016-01-18 13:09 UTC (permalink / raw)
  To: Uma Shankar; +Cc: shobhit.kumar, intel-gfx

On Mon, 2015-10-12 at 22:55 +0530, Uma Shankar wrote:
> For BXT DSI, vtotal, vactive, hactive registers are different.
> Making changes to intel_crtc_mode_get() and get_pipe_timings(),
> to read the correct registers for BXT DSI.
> 
Tested-by: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   48 +++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 75c60b8..2717823 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7708,6 +7708,7 @@ static void intel_get_pipe_timings(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;
> +	bool is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>  	uint32_t tmp;
>  
>  	tmp = I915_READ(HTOTAL(cpu_transcoder));
> @@ -7736,6 +7737,26 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>  	}
>  
> +	if (IS_BROXTON(dev) && is_dsi) {
> +		struct intel_encoder *encoder;
> +
> +		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +			struct intel_dsi *intel_dsi =
> +				enc_to_intel_dsi(&encoder->base);
> +			enum port port;
> +
> +			for_each_dsi_port(port, intel_dsi->ports) {
> +				pipe_config->base.adjusted_mode.crtc_hdisplay =
> +					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +				pipe_config->base.adjusted_mode.crtc_vdisplay =
> +					I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +				pipe_config->base.adjusted_mode.crtc_vtotal =
> +					I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +			}
> +		}
> +
> +	}
> +
>  	tmp = I915_READ(PIPESRC(crtc->pipe));
>  	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
>  	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> @@ -10664,6 +10685,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
>  	int vsync = I915_READ(VSYNC(cpu_transcoder));
>  	enum pipe pipe = intel_crtc->pipe;
> +	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>  
>  	mode = kzalloc(sizeof(*mode), GFP_KERNEL);
>  	if (!mode)
> @@ -10684,15 +10706,35 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
>  
>  	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
> -	mode->hdisplay = (htot & 0xffff) + 1;
>  	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
>  	mode->hsync_start = (hsync & 0xffff) + 1;
>  	mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
> -	mode->vdisplay = (vtot & 0xffff) + 1;
> -	mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
>  	mode->vsync_start = (vsync & 0xffff) + 1;
>  	mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
>  
> +	if (IS_BROXTON(dev) && is_dsi) {
> +		struct intel_encoder *encoder;
> +
> +		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
> +			struct intel_dsi *intel_dsi =
> +				enc_to_intel_dsi(&encoder->base);
> +			enum port port;
> +
> +			for_each_dsi_port(port, intel_dsi->ports) {
> +				mode->vtotal =
> +					I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +				mode->hdisplay =
> +					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +				mode->vdisplay =
> +					I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +			}
> +		}
> +	} else {
> +		mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> +		mode->hdisplay = (htot & 0xffff) + 1;
> +		mode->vdisplay = (vtot & 0xffff) + 1;
> +	}
> +
>  	drm_mode_set_name(mode);
>  
>  	return mode;


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-01-18 13:08 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-12 17:25 [BXT DSI timing fixes v1 0/3] BXT DSI mode timing fixes Uma Shankar
2015-10-12 17:25 ` [BXT DSI timing fixes v1 1/3] drm/i915/: DSI mode setting fix Uma Shankar
2015-10-12 17:25 ` [BXT DSI timing fixes v1 2/3] drm/i915/bxt: Get pipe timing for BXT DSI Uma Shankar
2015-10-12 17:23   ` Ville Syrjälä
2015-10-13 11:03     ` Shankar, Uma
2015-10-13 11:24       ` Ville Syrjälä
2015-10-13 13:24         ` Shankar, Uma
2015-10-13 15:45           ` Daniel Vetter
2016-01-18 13:09   ` Mika Kahola
2015-10-12 17:25 ` [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup Uma Shankar
2015-10-13 15:47   ` Daniel Vetter
2015-10-14 10:20     ` Shankar, Uma
2015-10-14 12:49       ` Daniel Vetter
2015-10-14 16:37         ` Shankar, Uma
2015-10-14 17:18           ` Daniel Vetter
2016-01-18 13:07   ` Mika Kahola

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.