On Wed, 13 Aug 2025, Dibin Moolakadan Subrahmanian
<dibin.moolakadan.subrahman...@intel.com> wrote:
> 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)
Thanks for the patch, pushed to drm-intel-next.
BR,
Jani.
>
>> 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");
>>> }
--
Jani Nikula, Intel
