| Message ID | 20180622085925.3216-1-tarun.vyas@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of >Tarun Vyas >Sent: Friday, June 22, 2018 1:59 AM >To: intel-gfx@lists.freedesktop.org >Cc: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo ><rodrigo.vivi@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@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. >+ 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@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx Regards, Azhar Shaikh
>-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of >Shaikh, Azhar >Sent: Friday, June 22, 2018 10:43 AM >To: Vyas, Tarun <tarun.vyas@intel.com>; intel-gfx@lists.freedesktop.org >Cc: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo ><rodrigo.vivi@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@lists.freedesktop.org] On >>Behalf Of Tarun Vyas >>Sent: Friday, June 22, 2018 1:59 AM >>To: intel-gfx@lists.freedesktop.org >>Cc: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo >><rodrigo.vivi@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@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@lists.freedesktop.org >>https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >Regards, >Azhar Shaikh >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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) +{ + 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; /*
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@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(-)