[PATCH] drm/i915/display: Optimize panel power-on wait time
Jani Nikula
jani.nikula at linux.intel.com
Tue Jul 1 09:28:41 UTC 2025
On Mon, 30 Jun 2025, Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian at intel.com> wrote:
> The current wait_panel_on() uses intel_de_wait() with a long timeout
> (5000ms), which is suboptimal on Xe platforms where the underlying
> xe_mmio_wait32() employs an exponential backoff strategy. This leads
> to unnecessary delays during resume or power-on when the panel becomes
> ready earlier than the full timeout.
>
> This patch splits the total wait time into two pases
> - First wait for the typical panel-on time(180ms)
> - If panel is not ready , continue polling in short 20ms intervals
> until the maximum timeout (5000ms) is reached
I'm *very* reluctant to merge any new custom wait hacks. I'm in the
process of *removing* a plethora of them [1][2][3].
I think the question is, should xe_mmio_wait32() (and the i915
counterpart) and the intel_de_wait*() functions be migrated to an
interface similar to read_poll_timeout(), i.e. provide sleep and timeout
separately, no exponential backoff. And implement the waits using
read_poll_timeout(), or whatever Ville ends up with [4].
[1] https://lore.kernel.org/r/cover.1750959689.git.jani.nikula@intel.com
[2] https://lore.kernel.org/r/20250626192632.2330349-1-jani.nikula@intel.com
[3] https://lore.kernel.org/r/cover.1751023767.git.jani.nikula@intel.com
[4] https://lore.kernel.org/r/aF67cxjlfWiWMx-4@intel.com
> Signed-off-by: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian at intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_pps.c | 61 +++++++++++++++++++++++-
> 1 file changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index bff81fb5c316..57a062c8038d 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -598,8 +598,67 @@ void intel_pps_check_power_unlocked(struct intel_dp *intel_dp)
> #define IDLE_CYCLE_MASK (PP_ON | PP_SEQUENCE_MASK | PP_CYCLE_DELAY_ACTIVE | PP_SEQUENCE_STATE_MASK)
> #define IDLE_CYCLE_VALUE (0 | PP_SEQUENCE_NONE | 0 | PP_SEQUENCE_STATE_OFF_IDLE)
>
> +#define PANEL_TYPICAL_ON_TIME_MS (180)
> +#define PANEL_MAXIMUM_ON_TIME_MS (5000)
> +
> static void intel_pps_verify_state(struct intel_dp *intel_dp);
>
> +/*
> + * Panel power-on typically completes within ~200ms. Using a large timeout
> + * (5000ms) with intel_de_wait() results in unnecessary delays,
> + * especially under Xe, where xe_mmio_wait32() uses an exponential backoff.
> + *
> + * To optimize resume and power-on latency, we first wait for the typical
> + * completion window, then perform short polling loops thereafter.
> + * This reduces worst-case latency while still ensuring correctness.
> + */
> +static void wait_panel_on_status(struct intel_dp *intel_dp)
> +{
> + struct intel_display *display = to_intel_display(intel_dp);
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + i915_reg_t pp_stat_reg, pp_ctrl_reg;
> + u32 mask = IDLE_ON_MASK;
> + u32 value = IDLE_ON_VALUE;
> + int elapsed = PANEL_TYPICAL_ON_TIME_MS;
> +
> + lockdep_assert_held(&display->pps.mutex);
> +
> + intel_pps_verify_state(intel_dp);
> +
> + pp_stat_reg = _pp_stat_reg(intel_dp);
> + pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
> +
> + drm_dbg_kms(display->drm,
> + "[ENCODER:%d:%s] %s mask: 0x%08x value: 0x%08x PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
> + dig_port->base.base.base.id, dig_port->base.base.name,
> + pps_name(intel_dp),
> + mask, value,
> + intel_de_read(display, pp_stat_reg),
> + intel_de_read(display, pp_ctrl_reg));
> +
> + /* Wait for typical panel on time first */
> + if (intel_de_wait(display, pp_stat_reg, mask, value, PANEL_TYPICAL_ON_TIME_MS) == 0)
> + goto panel_wait_complete;
> +
> + /* Wait for maxtime in 20ms intervals */
> + while (elapsed < PANEL_MAXIMUM_ON_TIME_MS) {
> + if (intel_de_wait(display, pp_stat_reg, mask, value, 20) == 0)
> + goto panel_wait_complete;
> +
> + elapsed += 20;
> + }
That's just not how time works in the kernel. Please let's not reinvent
wheels.
BR,
Jani.
> +
> + drm_err(display->drm,
> + "[ENCODER:%d:%s] %s panel status timeout: PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
> + dig_port->base.base.base.id, dig_port->base.base.name,
> + pps_name(intel_dp),
> + intel_de_read(display, pp_stat_reg),
> + intel_de_read(display, pp_ctrl_reg));
> +
> +panel_wait_complete:
> + drm_dbg_kms(display->drm, "Wait complete\n");
> +}
> +
> static void wait_panel_status(struct intel_dp *intel_dp,
> u32 mask, u32 value)
> {
> @@ -642,7 +701,7 @@ static void wait_panel_on(struct intel_dp *intel_dp)
> "[ENCODER:%d:%s] %s wait for panel power on\n",
> dig_port->base.base.base.id, dig_port->base.base.name,
> pps_name(intel_dp));
> - wait_panel_status(intel_dp, IDLE_ON_MASK, IDLE_ON_VALUE);
> + wait_panel_on_status(intel_dp);
> }
>
> static void wait_panel_off(struct intel_dp *intel_dp)
--
Jani Nikula, Intel
More information about the Intel-gfx
mailing list