Re: [BXT DSI timing fixes v1 3/3] drm/i915/bxt: Fixed dsi enc disable and blank at bootup

[Daniel Vetter <daniel@ffwll.ch>]

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