Re: [Intel-gfx] [PATCH] drm/i915/dsi: Always use unconditional msleep() for the panel_on_delay

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sat, Apr 22, 2023 at 08:43:59PM +0200, Hans de Goede wrote:
> The intel_dsi_msleep() helper skips sleeping if the MIPI-sequences have
> a version of 3 or newer and the panel is in vid-mode.
> 
> This is based on the big comment around line 730 which starts with
> "Panel enable/disable sequences from the VBT spec.", where
> the "v3 video mode seq" column does not have any wait t# entries.
> 
> Commit 6fdb335f1c9c ("drm/i915/dsi: Use unconditional msleep for
> the panel_on_delay when there is no reset-deassert MIPI-sequence")
> switched to a direct msleep() instead of intel_dsi_msleep()
> when there is no MIPI_SEQ_DEASSERT_RESET sequence, to fix
> the panel on an Acer Aspire Switch 10 E SW3-016 not turning on.
> 
> This was done under the assumption that when there is a v3
> MIPI_SEQ_DEASSERT_RESET sequence it will take care of any
> necessary delays.
> 
> On the Nextbook Ares 8A (a Cherry Trail device like the Acer SW3-016)
> there is a MIPI_SEQ_DEASSERT_RESET sequence, yet panel_on_delay
> must still be honored otherwise the panel will not turn on.
> 
> Switch to always using an unconditional msleep() for
> the panel_on_delay instead of making this depend on
> the presence of a MIPI_SEQ_DEASSERT_RESET sequence.

I just checked what Windows does, and at least for icl+ it
always waits for the panel power delays regardless of what
the VBT MIPI sequences are doing.

So I suspect we should just get rid of intel_dsi_msleep()
entirely and do what the power sequencing delays tell us.
Anything else is untested territory. If the VBT actually
wanted us to skip the delays then it should really be
setting them to zero.

> 
> Fixes: 6fdb335f1c9c ("drm/i915/dsi: Use unconditional msleep for the panel_on_delay when there is no reset-deassert MIPI-sequence")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/display/vlv_dsi.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index a6d6d8b33f3f..1b87f8f5f7fd 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -788,7 +788,6 @@ static void intel_dsi_pre_enable(struct intel_atomic_state *state,
>  {
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
> -	struct intel_connector *connector = to_intel_connector(conn_state->connector);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum pipe pipe = crtc->pipe;
>  	enum port port;
> @@ -836,21 +835,10 @@ static void intel_dsi_pre_enable(struct intel_atomic_state *state,
>  	if (!IS_GEMINILAKE(dev_priv))
>  		intel_dsi_prepare(encoder, pipe_config);
>  
> +	/* Give the panel time to power-on and then deassert its reset */
>  	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
> -
> -	/*
> -	 * Give the panel time to power-on and then deassert its reset.
> -	 * Depending on the VBT MIPI sequences version the deassert-seq
> -	 * may contain the necessary delay, intel_dsi_msleep() will skip
> -	 * the delay in that case. If there is no deassert-seq, then an
> -	 * unconditional msleep is used to give the panel time to power-on.
> -	 */
> -	if (connector->panel.vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET]) {
> -		intel_dsi_msleep(intel_dsi, intel_dsi->panel_on_delay);
> -		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
> -	} else {
> -		msleep(intel_dsi->panel_on_delay);
> -	}
> +	msleep(intel_dsi->panel_on_delay);
> +	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
>  
>  	if (IS_GEMINILAKE(dev_priv)) {
>  		glk_cold_boot = glk_dsi_enable_io(encoder);
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel