[Intel-gfx] [PATCH v3] drm/i915/psr: Lockless version of psr_wait_for_idle

[Shaikh, Azhar]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of
>Shaikh, Azhar
>Sent: Friday, June 22, 2018 10:43 AM
>To: Vyas, Tarun <tarun.vyas at intel.com>; intel-gfx at lists.freedesktop.org
>Cc: Pandiyan, Dhinakaran <dhinakaran.pandiyan at intel.com>; Vivi, Rodrigo
><rodrigo.vivi at intel.com>
>Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/psr: Lockless version of
>psr_wait_for_idle
>
>
>
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On
>>Behalf Of Tarun Vyas
>>Sent: Friday, June 22, 2018 1:59 AM
>>To: intel-gfx at lists.freedesktop.org
>>Cc: Pandiyan, Dhinakaran <dhinakaran.pandiyan at intel.com>; Vivi, Rodrigo
>><rodrigo.vivi at intel.com>
>>Subject: [Intel-gfx] [PATCH v3] drm/i915/psr: Lockless version of
>>psr_wait_for_idle
>>
>>This is a lockless version of the exisiting psr_wait_for_idle().
>>We want to wait for PSR to idle out inside intel_pipe_update_start.
>>At the time of a pipe update, we should never race with any psr enable
>>or disable code, which is a part of crtc enable/disable. So, we can
>>live w/o taking any psr locks at all.
>>The follow up patch will use this lockless wait inside pipe_update_
>>start to wait for PSR to idle out before checking for vblank evasion.
>>
>>Even if psr is never enabled, psr2_enabled will be false and this
>>function will wait for PSR1 to idle out, which should just return
>>immediately, so a very short
>>(~1-2 usec) wait for cases where PSR is disabled.
>>
>>v2: Add comment to explain the 25msec timeout (DK)
>>
>>v3: Rename psr_wait_for_idle to __psr_wait_for_idle_locked to avoid
>>    naming conflicts and propagate err (if any) to the caller (Chris)
>>
>>Signed-off-by: Tarun Vyas <tarun.vyas at intel.com>
>>---
>> drivers/gpu/drm/i915/intel_drv.h |  1 +
>>drivers/gpu/drm/i915/intel_psr.c |
>>25 +++++++++++++++++++++++--
>> 2 files changed, 24 insertions(+), 2 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>b/drivers/gpu/drm/i915/intel_drv.h
>>index 578346b8d7e2..9cb2b8afdd3e 100644
>>--- a/drivers/gpu/drm/i915/intel_drv.h
>>+++ b/drivers/gpu/drm/i915/intel_drv.h
>>@@ -1920,6 +1920,7 @@ void intel_psr_compute_config(struct intel_dp
>>*intel_dp,
>> 			      struct intel_crtc_state *crtc_state);  void
>>intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
>>void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
>>psr_iir);
>>+int intel_psr_wait_for_idle(struct drm_i915_private *dev_priv);
>>
>> /* intel_runtime_pm.c */
>> int intel_power_domains_init(struct drm_i915_private *); diff --git
>>a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>>index aea81ace854b..41e6962923ae 100644
>>--- a/drivers/gpu/drm/i915/intel_psr.c
>>+++ b/drivers/gpu/drm/i915/intel_psr.c
>>@@ -757,7 +757,28 @@ void intel_psr_disable(struct intel_dp *intel_dp,
>> 	cancel_work_sync(&dev_priv->psr.work);
>> }
>>
>>-static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
>>+int intel_psr_wait_for_idle(struct drm_i915_private *dev_priv) {
>
>
>I think you should upload this patch and
>https://patchwork.freedesktop.org/patch/231033/  as a series.
>intel_psr_wait_for_idle_lockless() does not get called anywhere in this patch.

intel_psr_wait_for_idle() and not intel_psr_wait_for_idle_lockless() [ created a new function name trying to comment on v2 version of this patch ;) ]

>
>>+	i915_reg_t reg;
>>+	u32 mask;
>>+
>>+	if (dev_priv->psr.psr2_enabled) {
>>+		reg = EDP_PSR2_STATUS;
>>+		mask = EDP_PSR2_STATUS_STATE_MASK;
>>+	} else {
>>+		reg = EDP_PSR_STATUS;
>>+		mask = EDP_PSR_STATUS_STATE_MASK;
>>+	}
>>+
>>+	/*
>>+	 * The  25 msec timeout accounts for a frame @ 60Hz refresh rate,
>>+	 * exit training an aux handshake time.
>>+	 */
>>+	return intel_wait_for_register(dev_priv, reg, mask,
>>+				       EDP_PSR_STATUS_STATE_IDLE, 25); }
>>+
>>+static bool __psr_wait_for_idle_locked(struct drm_i915_private
>>+*dev_priv)
>> {
>> 	struct intel_dp *intel_dp;
>> 	i915_reg_t reg;
>>@@ -803,7 +824,7 @@ static void intel_psr_work(struct work_struct *work)
>> 	 * PSR might take some time to get fully disabled
>> 	 * and be ready for re-enable.
>> 	 */
>>-	if (!psr_wait_for_idle(dev_priv))
>>+	if (!__psr_wait_for_idle_locked(dev_priv))
>> 		goto unlock;
>>
>> 	/*
>>--
>>2.13.5
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx at lists.freedesktop.org
>>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>Regards,
>Azhar Shaikh
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx