Re: [Intel-gfx] [PATCH v2] drm/i915/dsi: Use unconditional msleep() instead of intel_dsi_msleep()
On Tue, Apr 25, 2023 at 09:44:41PM +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.
>
> Checking the Windows driver shows that it does always honor
> the VBT delays independent of the version of the VBT sequences.
>
> 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.
>
> And now testing on a Nextbook Ares 8A shows that panel_on_delay
> must always be honored otherwise the panel will not turn on.
>
> Instead of only always using regular msleep() for panel_on_delay
> do as Windows does and always use regular msleep() everywhere
> were intel_dsi_msleep() is used and drop the intel_dsi_msleep()
> helper.
>
> Changes in v2:
> - Replace all intel_dsi_msleep() calls instead of just
> the intel_dsi_msleep(panel_on_delay) call
>
> 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
Thanks. Added cc:stable and pushed to drm-intel-next.
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 2 +-
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 11 --
> drivers/gpu/drm/i915/display/intel_dsi_vbt.h | 1 -
> drivers/gpu/drm/i915/display/vlv_dsi.c | 22 +---
> 4 files changed, 6 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index fc0eaf40dc94..6dd942522021 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1211,7 +1211,7 @@ static void gen11_dsi_powerup_panel(struct
> intel_encoder *encoder)
>
> /* panel power on related mipi dsi vbt sequences */
> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
> - intel_dsi_msleep(intel_dsi, intel_dsi->panel_on_delay);
> + msleep(intel_dsi->panel_on_delay);
> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
> intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index 2cbc1292ab38..f102c13cb959 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -762,17 +762,6 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi
> *intel_dsi,
> gpiod_set_value_cansleep(intel_dsi->gpio_backlight, 0);
> }
>
> -void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
> -{
> - struct intel_connector *connector = intel_dsi->attached_connector;
> -
> - /* For v3 VBTs in vid-mode the delays are part of the VBT sequences */
> - if (is_vid_mode(intel_dsi) && connector->panel.vbt.dsi.seq_version >= 3)
> - return;
> -
> - msleep(msec);
> -}
> -
> void intel_dsi_log_params(struct intel_dsi *intel_dsi)
> {
> struct drm_i915_private *i915 = to_i915(intel_dsi->base.base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
> index dc642c1fe7ef..468d873fab1a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
> @@ -16,7 +16,6 @@ void intel_dsi_vbt_gpio_init(struct intel_dsi *intel_dsi,
> bool panel_is_on);
> void intel_dsi_vbt_gpio_cleanup(struct intel_dsi *intel_dsi);
> void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
>enum mipi_seq seq_id);
> -void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec);
> void intel_dsi_log_params(struct intel_dsi *intel_dsi);
>
> #endif /* __INTEL_DSI_VBT_H__ */
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c
> b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 2289f6b1b4eb..37efeab52581 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -783,7 +783,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;
> @@ -831,21 +830,10 @@ static void int
[Intel-gfx] [PATCH v2] drm/i915/dsi: Use unconditional msleep() instead of intel_dsi_msleep()
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.
Checking the Windows driver shows that it does always honor
the VBT delays independent of the version of the VBT sequences.
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.
And now testing on a Nextbook Ares 8A shows that panel_on_delay
must always be honored otherwise the panel will not turn on.
Instead of only always using regular msleep() for panel_on_delay
do as Windows does and always use regular msleep() everywhere
were intel_dsi_msleep() is used and drop the intel_dsi_msleep()
helper.
Changes in v2:
- Replace all intel_dsi_msleep() calls instead of just
the intel_dsi_msleep(panel_on_delay) call
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
---
drivers/gpu/drm/i915/display/icl_dsi.c | 2 +-
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 11 --
drivers/gpu/drm/i915/display/intel_dsi_vbt.h | 1 -
drivers/gpu/drm/i915/display/vlv_dsi.c | 22 +---
4 files changed, 6 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
b/drivers/gpu/drm/i915/display/icl_dsi.c
index fc0eaf40dc94..6dd942522021 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1211,7 +1211,7 @@ static void gen11_dsi_powerup_panel(struct intel_encoder
*encoder)
/* panel power on related mipi dsi vbt sequences */
intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
- intel_dsi_msleep(intel_dsi, intel_dsi->panel_on_delay);
+ msleep(intel_dsi->panel_on_delay);
intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
index 2cbc1292ab38..f102c13cb959 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
@@ -762,17 +762,6 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi
*intel_dsi,
gpiod_set_value_cansleep(intel_dsi->gpio_backlight, 0);
}
-void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
-{
- struct intel_connector *connector = intel_dsi->attached_connector;
-
- /* For v3 VBTs in vid-mode the delays are part of the VBT sequences */
- if (is_vid_mode(intel_dsi) && connector->panel.vbt.dsi.seq_version >= 3)
- return;
-
- msleep(msec);
-}
-
void intel_dsi_log_params(struct intel_dsi *intel_dsi)
{
struct drm_i915_private *i915 = to_i915(intel_dsi->base.base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
index dc642c1fe7ef..468d873fab1a 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
@@ -16,7 +16,6 @@ void intel_dsi_vbt_gpio_init(struct intel_dsi *intel_dsi,
bool panel_is_on);
void intel_dsi_vbt_gpio_cleanup(struct intel_dsi *intel_dsi);
void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
enum mipi_seq seq_id);
-void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec);
void intel_dsi_log_params(struct intel_dsi *intel_dsi);
#endif /* __INTEL_DSI_VBT_H__ */
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c
b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 2289f6b1b4eb..37efeab52581 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -783,7 +783,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;
@@ -831,21 +830,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);
