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

[Dibin Moolakadan Subrahmanian]

On 12-08-2025 23:42, Jani Nikula wrote:

On Thu, 07 Aug 2025, Dibin Moolakadan Subrahmanian 
<dibin.moolakadan.subrahman...@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)
Changes in v6:
  - Removed goto in error path
Signed-off-by: Dibin Moolakadan Subrahmanian 
<dibin.moolakadan.subrahman...@intel.com>
---
  drivers/gpu/drm/i915/display/intel_pps.c | 12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pps.c 
b/drivers/gpu/drm/i915/display/intel_pps.c
index b64d0b30f5b1..b84eb43bd2d0 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,13 +627,20 @@ 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,

Otherwise looks good, but did you have some rationale for the
sleep_before_read == true?

Yes, this is intentional — the status register is taking time in all cases.
(panel power off time- 82ms, panel power cycle- 0.074 ms,panel power on-327 ms)

BR,
Jani.

+                               display, pp_stat_reg);
+
+       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,
                        pps_name(intel_dp),
                        intel_de_read(display, pp_stat_reg),
                        intel_de_read(display, pp_ctrl_reg));
+               return;
+       }

drm_dbg_kms(display->drm, "Wait complete\n");

  }