| Message ID | 20190306064728.8234-8-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/9] drm/i915/psr: Remove PSR2 FIXME | expand |
On Tue, 2019-03-05 at 22:47 -0800, José Roberto de Souza wrote: > If PSR is active when pipe CRC is enabled the CRC calculations will > be inhibit by the transition to low power states that PSR brings. The MMIO write to enable CRCs should bring the hardware out from PSR, but I can imagine some initial CRCs are going to be corrupt or unavailable. > So lets for a PSR exit and as soon as pipe CRC is enabled it will There is a missing word. > block PSR activation avoid CRC timeouts when running IGT tests. This surely has fdo bugs, please include them in the commit message. > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++++-------- > ---- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index d3e3996551c6..5d66e7313c75 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -452,6 +452,7 @@ static void hsw_activate_psr1(struct intel_dp > *intel_dp) > * frames, we'll go with 9 frames for now > */ > idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency > + 1); > + > val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > > val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > @@ -851,6 +852,20 @@ void intel_psr_disable(struct intel_dp > *intel_dp, > cancel_work_sync(&dev_priv->psr.work); > } > > +static void psr_force_hw_tracking_exit(struct drm_i915_private > *dev_priv) > +{ > + /* > + * Display WA #0884: all > + * This documented WA for bxt can be safely applied > + * broadly so we can force HW tracking to exit PSR > + * instead of disabling and re-enabling. > + * Workaround tells us to write 0 to CUR_SURFLIVE_A, > + * but it makes more sense write to the current active > + * pipe. > + */ > + I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > +} > + > /** > * intel_psr_update - Update PSR state > * @intel_dp: Intel DP > @@ -875,8 +890,13 @@ void intel_psr_update(struct intel_dp *intel_dp, > enable = crtc_state->has_psr && psr_global_enabled(psr->debug); > psr2_enable = intel_psr2_enabled(dev_priv, crtc_state); > > - if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) > + if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) > { PSR2 is enabled, then user requests CRCs, the mode_changed commit switches to PSR1. The above condition isn't true in that case. Also, since the CRC workaround is done before enabling pipe CRC, isn't there a possibility that the idle frame counter times out and activates PSR1 before CRC is enabled? > + /* Force a PSR exit when enabling CRC to avoid CRC > timeouts */ > + if (crtc_state->crc_enabled && psr->enabled) > + psr_force_hw_tracking_exit(dev_priv); The patch fixes a PSR1 issue, why isn't there any reference to PSR1 anywhere? > + > goto unlock; > + } > > if (psr->enabled) > intel_psr_disable_locked(intel_dp); > @@ -1146,18 +1166,8 @@ void intel_psr_flush(struct drm_i915_private > *dev_priv, > dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits; > > /* By definition flush = invalidate + flush */ > - if (frontbuffer_bits) { > - /* > - * Display WA #0884: all > - * This documented WA for bxt can be safely applied > - * broadly so we can force HW tracking to exit PSR > - * instead of disabling and re-enabling. > - * Workaround tells us to write 0 to CUR_SURFLIVE_A, > - * but it makes more sense write to the current active > - * pipe. > - */ > - I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > - } > + if (frontbuffer_bits) > + psr_force_hw_tracking_exit(dev_priv); > > if (!dev_priv->psr.active && !dev_priv- > >psr.busy_frontbuffer_bits) > schedule_work(&dev_priv->psr.work);
On Thu, 2019-03-07 at 13:25 -0800, Dhinakaran Pandiyan wrote: > On Tue, 2019-03-05 at 22:47 -0800, José Roberto de Souza wrote: > > If PSR is active when pipe CRC is enabled the CRC calculations will > > be inhibit by the transition to low power states that PSR brings. > The MMIO write to enable CRCs should bring the hardware out from PSR, > but I can imagine some initial CRCs are going to be corrupt or > unavailable. That do not happen, if PSR is active and CRC is configured PSR will remain active and no CRC will be calculated. > > > So lets for a PSR exit and as soon as pipe CRC is enabled it will > There is a missing word. Thanks "So lets force a PSR exit and as soon as pipe CRC is enabled it will block PSR activation and avoid CRC timeouts when running IGT tests." > > > block PSR activation avoid CRC timeouts when running IGT tests. > > This surely has fdo bugs, please include them in the commit message. I did this patch because of the regressions that I got from CI in my testings, the only bug that I found related is https://bugs.freedesktop.org/show_bug.cgi?id=107814 but we don't have PSR enabled by default on HSW. > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++++-------- > > ---- > > 1 file changed, 23 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index d3e3996551c6..5d66e7313c75 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -452,6 +452,7 @@ static void hsw_activate_psr1(struct intel_dp > > *intel_dp) > > * frames, we'll go with 9 frames for now > > */ > > idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency > > + 1); > > + > > val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > > > > val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > > @@ -851,6 +852,20 @@ void intel_psr_disable(struct intel_dp > > *intel_dp, > > cancel_work_sync(&dev_priv->psr.work); > > } > > > > +static void psr_force_hw_tracking_exit(struct drm_i915_private > > *dev_priv) > > +{ > > + /* > > + * Display WA #0884: all > > + * This documented WA for bxt can be safely applied > > + * broadly so we can force HW tracking to exit PSR > > + * instead of disabling and re-enabling. > > + * Workaround tells us to write 0 to CUR_SURFLIVE_A, > > + * but it makes more sense write to the current active > > + * pipe. > > + */ > > + I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > > +} > > + > > /** > > * intel_psr_update - Update PSR state > > * @intel_dp: Intel DP > > @@ -875,8 +890,13 @@ void intel_psr_update(struct intel_dp > > *intel_dp, > > enable = crtc_state->has_psr && psr_global_enabled(psr->debug); > > psr2_enable = intel_psr2_enabled(dev_priv, crtc_state); > > > > - if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) > > + if (enable == psr->enabled && psr2_enable == psr- > > >psr2_enabled) > > { > > PSR2 is enabled, then user requests CRCs, the mode_changed commit > switches to PSR1. The above condition isn't true in that case. Not sure if I understood the question but if PSR2 is enabled and user request CRC it will switch to PSR1 already forcing a PSR exit so we don't need to call psr_force_hw_tracking_exit() in this case. > > Also, since the CRC workaround is done before enabling pipe CRC, > isn't > there a possibility that the idle frame counter times out and > activates > PSR1 before CRC is enabled? There still the possibility since syscalls can be preempted too but this is a huge improvement over current scenario, now it will give at least the time of 6 idle frames between intel_crtc_crc_setup_workarounds() and the I915_WRITE() to the PIPE CRC registers and it happens right after intel_crtc_crc_setup_workarounds() returns. > > > + /* Force a PSR exit when enabling CRC to avoid CRC > > timeouts */ > > + if (crtc_state->crc_enabled && psr->enabled) > > + psr_force_hw_tracking_exit(dev_priv); > The patch fixes a PSR1 issue, why isn't there any reference to PSR1 > anywhere? fair, going to update the commit message. > > > > + > > goto unlock; > > + } > > > > if (psr->enabled) > > intel_psr_disable_locked(intel_dp); > > @@ -1146,18 +1166,8 @@ void intel_psr_flush(struct drm_i915_private > > *dev_priv, > > dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits; > > > > /* By definition flush = invalidate + flush */ > > - if (frontbuffer_bits) { > > - /* > > - * Display WA #0884: all > > - * This documented WA for bxt can be safely applied > > - * broadly so we can force HW tracking to exit PSR > > - * instead of disabling and re-enabling. > > - * Workaround tells us to write 0 to CUR_SURFLIVE_A, > > - * but it makes more sense write to the current active > > - * pipe. > > - */ > > - I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > > - } > > + if (frontbuffer_bits) > > + psr_force_hw_tracking_exit(dev_priv); > > > > if (!dev_priv->psr.active && !dev_priv- > > > psr.busy_frontbuffer_bits) > > schedule_work(&dev_priv->psr.work);
On Thu, 2019-03-07 at 13:57 -0800, Souza, Jose wrote: > On Thu, 2019-03-07 at 13:25 -0800, Dhinakaran Pandiyan wrote: > > On Tue, 2019-03-05 at 22:47 -0800, José Roberto de Souza wrote: > > > If PSR is active when pipe CRC is enabled the CRC calculations > > > will > > > be inhibit by the transition to low power states that PSR brings. > > > > The MMIO write to enable CRCs should bring the hardware out from > > PSR, > > but I can imagine some initial CRCs are going to be corrupt or > > unavailable. > > That do not happen, if PSR is active and CRC is configured PSR will > remain active and no CRC will be calculated. Odd, MMIO writes should trigger DC6 exit, which always results in PSR exit. If the hardware isnt't in DC6, then I can imagine CRC enabling not causing a PSR exit. > > > > > > So lets for a PSR exit and as soon as pipe CRC is enabled it will > > > > There is a missing word. > > Thanks > > "So lets force a PSR exit and as soon as pipe CRC is enabled it will > block PSR activation and avoid CRC timeouts when running IGT tests." > > > > > > > block PSR activation avoid CRC timeouts when running IGT tests. > > > > This surely has fdo bugs, please include them in the commit > > message. > > I did this patch because of the regressions that I got from CI in my > testings, Can you include the test name and platform in the commit message? > the only bug that I found related is > https://bugs.freedesktop.org/show_bug.cgi?id=107814 but we don't have > PSR enabled by default on HSW. Jani S had reported the issue, Cc'ing him. > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++++---- > > > ---- > > > ---- > > > 1 file changed, 23 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > > b/drivers/gpu/drm/i915/intel_psr.c > > > index d3e3996551c6..5d66e7313c75 100644 > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > @@ -452,6 +452,7 @@ static void hsw_activate_psr1(struct intel_dp > > > *intel_dp) > > > * frames, we'll go with 9 frames for now > > > */ > > > idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency > > > + 1); > > > + > > > val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > > > > > > val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > > > @@ -851,6 +852,20 @@ void intel_psr_disable(struct intel_dp > > > *intel_dp, > > > cancel_work_sync(&dev_priv->psr.work); > > > } > > > > > > +static void psr_force_hw_tracking_exit(struct drm_i915_private > > > *dev_priv) > > > +{ > > > + /* > > > + * Display WA #0884: all > > > + * This documented WA for bxt can be safely applied > > > + * broadly so we can force HW tracking to exit PSR > > > + * instead of disabling and re-enabling. > > > + * Workaround tells us to write 0 to CUR_SURFLIVE_A, > > > + * but it makes more sense write to the current active > > > + * pipe. > > > + */ > > > + I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > > > +} > > > + > > > /** > > > * intel_psr_update - Update PSR state > > > * @intel_dp: Intel DP > > > @@ -875,8 +890,13 @@ void intel_psr_update(struct intel_dp > > > *intel_dp, > > > enable = crtc_state->has_psr && psr_global_enabled(psr->debug); > > > psr2_enable = intel_psr2_enabled(dev_priv, crtc_state); > > > > > > - if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) > > > + if (enable == psr->enabled && psr2_enable == psr- > > > > psr2_enabled) > > > > > > { > > > > PSR2 is enabled, then user requests CRCs, the mode_changed commit > > switches to PSR1. The above condition isn't true in that case. > > Not sure if I understood the question but if PSR2 is enabled and user > request CRC it will switch to PSR1 already forcing a PSR exit so we > don't need to call psr_force_hw_tracking_exit() in this case. The PSR2->PSR1 switching gives us the same window before CRC enabling as force_hw_tracking_exit(). My question was about if that window is sufficient. > > > > > Also, since the CRC workaround is done before enabling pipe CRC, > > isn't > > there a possibility that the idle frame counter times out and > > activates > > PSR1 before CRC is enabled? > > There still the possibility since syscalls can be preempted too but > this is a huge improvement over current scenario, now it will give at > least the time of 6 idle frames between > intel_crtc_crc_setup_workarounds() and the I915_WRITE() to the PIPE > CRC > registers and it happens right after > intel_crtc_crc_setup_workarounds() > returns. Yeah, it should be okay. Please document in the commit message the idea is that we rely on the idle frame counter to have not expired before CRC is enabled and we are only kicking the hardware out of PSR1. Since what this patch does is different from what psr_exit() implements, I think it makes sense to clarify what exit means. With the commit message fixed, this looks okay to me but let's see if the timeouts go away. Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > > > + /* Force a PSR exit when enabling CRC to avoid CRC > > > timeouts */ > > > + if (crtc_state->crc_enabled && psr->enabled) > > > + psr_force_hw_tracking_exit(dev_priv); > > > > The patch fixes a PSR1 issue, why isn't there any reference to PSR1 > > anywhere? > > fair, going to update the commit message. > > > > > > > > + > > > goto unlock; > > > + } > > > > > > if (psr->enabled) > > > intel_psr_disable_locked(intel_dp); > > > @@ -1146,18 +1166,8 @@ void intel_psr_flush(struct > > > drm_i915_private > > > *dev_priv, > > > dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits; > > > > > > /* By definition flush = invalidate + flush */ > > > - if (frontbuffer_bits) { > > > - /* > > > - * Display WA #0884: all > > > - * This documented WA for bxt can be safely applied > > > - * broadly so we can force HW tracking to exit PSR > > > - * instead of disabling and re-enabling. > > > - * Workaround tells us to write 0 to CUR_SURFLIVE_A, > > > - * but it makes more sense write to the current active > > > - * pipe. > > > - */ > > > - I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > > > - } > > > + if (frontbuffer_bits) > > > + psr_force_hw_tracking_exit(dev_priv); > > > > > > if (!dev_priv->psr.active && !dev_priv- > > > > psr.busy_frontbuffer_bits) > > > > > > schedule_work(&dev_priv->psr.work);
On Thu, 2019-03-07 at 14:30 -0800, Dhinakaran Pandiyan wrote: > On Thu, 2019-03-07 at 13:57 -0800, Souza, Jose wrote: > > On Thu, 2019-03-07 at 13:25 -0800, Dhinakaran Pandiyan wrote: > > > On Tue, 2019-03-05 at 22:47 -0800, José Roberto de Souza wrote: > > > > If PSR is active when pipe CRC is enabled the CRC calculations > > > > will > > > > be inhibit by the transition to low power states that PSR > > > > brings. > > > > > > The MMIO write to enable CRCs should bring the hardware out from > > > PSR, > > > but I can imagine some initial CRCs are going to be corrupt or > > > unavailable. > > > > That do not happen, if PSR is active and CRC is configured PSR will > > remain active and no CRC will be calculated. > > Odd, MMIO writes should trigger DC6 exit, which always results in PSR > exit. If the hardware isnt't in DC6, then I can imagine CRC enabling > not causing a PSR exit. > > > > So lets for a PSR exit and as soon as pipe CRC is enabled it > > > > will > > > > > > There is a missing word. > > > > Thanks > > > > "So lets force a PSR exit and as soon as pipe CRC is enabled it > > will > > block PSR activation and avoid CRC timeouts when running IGT > > tests." > > > > > > > > block PSR activation avoid CRC timeouts when running IGT tests. > > > > > > This surely has fdo bugs, please include them in the commit > > > message. > > > > I did this patch because of the regressions that I got from CI in > > my > > testings, > Can you include the test name and platform in the commit message? Done > > > the only bug that I found related is > > https://bugs.freedesktop.org/show_bug.cgi?id=107814 but we don't > > have > > PSR enabled by default on HSW. > > Jani S had reported the issue, Cc'ing him. As said the issue don't look related to this. > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++++---- > > > > ---- > > > > ---- > > > > 1 file changed, 23 insertions(+), 13 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > > > b/drivers/gpu/drm/i915/intel_psr.c > > > > index d3e3996551c6..5d66e7313c75 100644 > > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > > @@ -452,6 +452,7 @@ static void hsw_activate_psr1(struct > > > > intel_dp > > > > *intel_dp) > > > > * frames, we'll go with 9 frames for now > > > > */ > > > > idle_frames = max(idle_frames, dev_priv- > > > > >psr.sink_sync_latency > > > > + 1); > > > > + > > > > val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > > > > > > > > val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > > > > @@ -851,6 +852,20 @@ void intel_psr_disable(struct intel_dp > > > > *intel_dp, > > > > cancel_work_sync(&dev_priv->psr.work); > > > > } > > > > > > > > +static void psr_force_hw_tracking_exit(struct drm_i915_private > > > > *dev_priv) > > > > +{ > > > > + /* > > > > + * Display WA #0884: all > > > > + * This documented WA for bxt can be safely applied > > > > + * broadly so we can force HW tracking to exit PSR > > > > + * instead of disabling and re-enabling. > > > > + * Workaround tells us to write 0 to CUR_SURFLIVE_A, > > > > + * but it makes more sense write to the current active > > > > + * pipe. > > > > + */ > > > > + I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > > > > +} > > > > + > > > > /** > > > > * intel_psr_update - Update PSR state > > > > * @intel_dp: Intel DP > > > > @@ -875,8 +890,13 @@ void intel_psr_update(struct intel_dp > > > > *intel_dp, > > > > enable = crtc_state->has_psr && psr_global_enabled(psr- > > > > >debug); > > > > psr2_enable = intel_psr2_enabled(dev_priv, crtc_state); > > > > > > > > - if (enable == psr->enabled && psr2_enable == psr- > > > > >psr2_enabled) > > > > + if (enable == psr->enabled && psr2_enable == psr- > > > > > psr2_enabled) > > > > > > > > { > > > > > > PSR2 is enabled, then user requests CRCs, the mode_changed commit > > > switches to PSR1. The above condition isn't true in that case. > > > > Not sure if I understood the question but if PSR2 is enabled and > > user > > request CRC it will switch to PSR1 already forcing a PSR exit so we > > don't need to call psr_force_hw_tracking_exit() in this case. > The PSR2->PSR1 switching gives us the same window before CRC enabling > as force_hw_tracking_exit(). My question was about if that window is > sufficient. > > > Also, since the CRC workaround is done before enabling pipe CRC, > > > isn't > > > there a possibility that the idle frame counter times out and > > > activates > > > PSR1 before CRC is enabled? > > > > There still the possibility since syscalls can be preempted too but > > this is a huge improvement over current scenario, now it will give > > at > > least the time of 6 idle frames between > > intel_crtc_crc_setup_workarounds() and the I915_WRITE() to the PIPE > > CRC > > registers and it happens right after > > intel_crtc_crc_setup_workarounds() > > returns. > > Yeah, it should be okay. Please document in the commit message the > idea > is that we rely on the idle frame counter to have not expired before > CRC is enabled and we are only kicking the hardware out of PSR1. > Since > what this patch does is different from what psr_exit() implements, I > think it makes sense to clarify what exit means. > > With the commit message fixed, this looks okay to me but let's see if > the timeouts go away. Done, thanks. > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > > + /* Force a PSR exit when enabling CRC to avoid > > > > CRC > > > > timeouts */ > > > > + if (crtc_state->crc_enabled && psr->enabled) > > > > + psr_force_hw_tracking_exit(dev_priv); > > > > > > The patch fixes a PSR1 issue, why isn't there any reference to > > > PSR1 > > > anywhere? > > > > fair, going to update the commit message. > > > > > > > > > + > > > > goto unlock; > > > > + } > > > > > > > > if (psr->enabled) > > > > intel_psr_disable_locked(intel_dp); > > > > @@ -1146,18 +1166,8 @@ void intel_psr_flush(struct > > > > drm_i915_private > > > > *dev_priv, > > > > dev_priv->psr.busy_frontbuffer_bits &= > > > > ~frontbuffer_bits; > > > > > > > > /* By definition flush = invalidate + flush */ > > > > - if (frontbuffer_bits) { > > > > - /* > > > > - * Display WA #0884: all > > > > - * This documented WA for bxt can be safely > > > > applied > > > > - * broadly so we can force HW tracking to exit > > > > PSR > > > > - * instead of disabling and re-enabling. > > > > - * Workaround tells us to write 0 to > > > > CUR_SURFLIVE_A, > > > > - * but it makes more sense write to the current > > > > active > > > > - * pipe. > > > > - */ > > > > - I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > > > > - } > > > > + if (frontbuffer_bits) > > > > + psr_force_hw_tracking_exit(dev_priv); > > > > > > > > if (!dev_priv->psr.active && !dev_priv- > > > > > psr.busy_frontbuffer_bits) > > > > > > > > schedule_work(&dev_priv->psr.work);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index d3e3996551c6..5d66e7313c75 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -452,6 +452,7 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp) * frames, we'll go with 9 frames for now */ idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1); + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; @@ -851,6 +852,20 @@ void intel_psr_disable(struct intel_dp *intel_dp, cancel_work_sync(&dev_priv->psr.work); } +static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv) +{ + /* + * Display WA #0884: all + * This documented WA for bxt can be safely applied + * broadly so we can force HW tracking to exit PSR + * instead of disabling and re-enabling. + * Workaround tells us to write 0 to CUR_SURFLIVE_A, + * but it makes more sense write to the current active + * pipe. + */ + I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); +} + /** * intel_psr_update - Update PSR state * @intel_dp: Intel DP @@ -875,8 +890,13 @@ void intel_psr_update(struct intel_dp *intel_dp, enable = crtc_state->has_psr && psr_global_enabled(psr->debug); psr2_enable = intel_psr2_enabled(dev_priv, crtc_state); - if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) + if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) { + /* Force a PSR exit when enabling CRC to avoid CRC timeouts */ + if (crtc_state->crc_enabled && psr->enabled) + psr_force_hw_tracking_exit(dev_priv); + goto unlock; + } if (psr->enabled) intel_psr_disable_locked(intel_dp); @@ -1146,18 +1166,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits; /* By definition flush = invalidate + flush */ - if (frontbuffer_bits) { - /* - * Display WA #0884: all - * This documented WA for bxt can be safely applied - * broadly so we can force HW tracking to exit PSR - * instead of disabling and re-enabling. - * Workaround tells us to write 0 to CUR_SURFLIVE_A, - * but it makes more sense write to the current active - * pipe. - */ - I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); - } + if (frontbuffer_bits) + psr_force_hw_tracking_exit(dev_priv); if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) schedule_work(&dev_priv->psr.work);
If PSR is active when pipe CRC is enabled the CRC calculations will be inhibit by the transition to low power states that PSR brings. So lets for a PSR exit and as soon as pipe CRC is enabled it will block PSR activation avoid CRC timeouts when running IGT tests. Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 13 deletions(-)