Re: [PATCH v5] drm/i915/display: Optimize panel power-on wait time

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 04 Aug 2025, Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com> wrote:
> The current wait_panel_status() uses intel_de_wait(),
> which internally on Xe platforms calls  xe_mmio_wait32().
> xe_mmio_wait32() increases poll interval exponentially.
>
> This exponential poll interval increase causes unnessory delays
> during resume or power-on when the panel becomes ready earlier,
> but polling is delayed due to backoff.
>
> Replace intel_de_wait() with read_poll_timeout() +
> intel_de_read() to actively poll the register at a fixed 10ms interval
> up to a 5 second timeout. This allows poll to exit
> early  when panel is ready.
>
> Changes in v2:
> Replaced  two-phase intel_de_wait() with  read_poll_timeout()
>  + intel_de_read()
> Changes in v3:
>  - Add poll_interval_ms argument  'wait_panel_status' function.
>  - Modify 'wait_panel_status' callers with proper poll interval
> Changes in v4:
>  - Change 'wait_panel_off' poll interval to 10ms
> Changes in v5:
>  - Dropped  poll_interval_ms parameter,use fixed polling
>    interval of 10ms (Jani Nikula)
>
> Signed-off-by: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_pps.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index b64d0b30f5b1..47e274a6fd72 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <linux/debugfs.h>
> +#include <linux/iopoll.h>
>  
>  #include <drm/drm_print.h>
>  
> @@ -608,6 +609,8 @@ static void wait_panel_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;
> +	int ret;
> +	u32 val;
>  
>  	lockdep_assert_held(&display->pps.mutex);
>  
> @@ -624,7 +627,18 @@ static void wait_panel_status(struct intel_dp *intel_dp,
>  		    intel_de_read(display, pp_stat_reg),
>  		    intel_de_read(display, pp_ctrl_reg));
>  
> -	if (intel_de_wait(display, pp_stat_reg, mask, value, 5000))
> +	ret = read_poll_timeout(intel_de_read, val,
> +				(val & mask) == value,
> +				10 * 1000, 5000 * 1000, true,
> +				display, pp_stat_reg);
> +
> +	if (ret)
> +		goto panel_wait_err;
> +
> +	drm_dbg_kms(display->drm, "Wait complete\n");
> +	return;
> +
> +panel_wait_err:

What I meant was, you'd have

	if (ret)
>  		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,
> @@ -632,7 +646,6 @@ static void wait_panel_status(struct intel_dp *intel_dp,
>  			intel_de_read(display, pp_stat_reg),
>  			intel_de_read(display, pp_ctrl_reg));

But with the goto, the drm_err() is now incorrectly indented.

BR,
Jani.

>  
> -	drm_dbg_kms(display->drm, "Wait complete\n");
>  }
>  
>  static void wait_panel_on(struct intel_dp *intel_dp)

-- 
Jani Nikula, Intel