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

[Shankar, Uma]

>-----Original Message-----
>From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
>Sent: Tuesday, October 13, 2015 9:17 PM
>To: Shankar, Uma
>Cc: intel-gfx at 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 at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch