On Thu, 2015-11-12 at 13:50 +0000, R, Durgadoss wrote: > Hi Rodrigo, > > > -----Original Message----- > > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On > > Behalf Of Rodrigo Vivi > > Sent: Thursday, November 12, 2015 1:07 AM > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Vivi, Rodrigo > > Subject: [PATCH 1/4] drm/i915: Delay first PSR > > activation. > > > > When debuging the frozen screen caused by HW tracking with low > > power state I noticed that if we keep moving the mouse non stop > > you will miss the screen updates for a while. At least > > until we stop moving the mouse for a small time and move again. > > > > The actual enabling should happen immediately after > > Display Port enabling sequence finished with links trained and > > everything enabled. However we face many issues when enabling PSR > > right after a modeset. > > > > On VLV/CHV we face blank screens on this scenario and on HSW+ > > we face a recoverable frozen screen, at least until next > > exit-activate sequence. > > > > Another workaround for the same issue here would be to increase > > re-enable idle time from 100 to 500 as we did for VLV/CHV. > > However this patch workaround this issue in a better > > way since it doesn't reduce PSR residency and also > > allow us to reduce the delay time between re-enables at least > > on VLV/CHV. > > > > This is also important to make the sysfs toggle working properly. > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_psr.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index 213581c..6b24c24 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -427,6 +427,19 @@ void intel_psr_enable(struct intel_dp > > *intel_dp) > > vlv_psr_enable_source(intel_dp); > > } > > > > + /* > > + * FIXME: Activation should happen immediately since this > > function > > + * is just called after pipe is fully trained and enabled. > > + * However on every platform we face issues when first > > activation > > + * follows a modeset so quickly. > > + * - On VLV/CHV we get bank screen on first activation > > + * - On HSW/BDW we get a recoverable frozen screen > > until next > > + * exit-activate sequence. > > + */ > > + if (INTEL_INFO(dev)->gen < 9) > > + schedule_delayed_work(&dev_priv->psr.work, > > + msecs_to_jiffies(intel_dp > > ->panel_power_cycle_delay * 5)); > > + > > dev_priv->psr.enabled = intel_dp; > > unlock: > > mutex_unlock(&dev_priv->psr.lock); > > @@ -735,8 +748,9 @@ void intel_psr_flush(struct drm_device *dev, > > } > > > > if (!dev_priv->psr.active && !dev_priv > > ->psr.busy_frontbuffer_bits) > > - schedule_delayed_work(&dev_priv->psr.work, > > - msecs_to_jiffies(delay_ms)); > > + if (!work_busy(&dev_priv->psr.work.work)) > > + schedule_delayed_work(&dev_priv->psr.work, > > + > > msecs_to_jiffies(delay_ms)); > > Agree with the theory of the patch as such.. But, Is there any > specific reason for > the !work_busy() check here ? > > I believe when the later work runs, it will anyway bail out in > _activate > function, if it sees PSR_ENABLE bit set already. So, is this check > just to > prevent scheduling one more work item when there is one pending > already ? (or it helps in something else also ?) The !work_busy is to prevent that eventual _activate call reduce the first activation time. for instance: 0s - we enable and schedule first activation to 2.5s 1s - we got a page flip that flushed fb tracking and called psr_activation to 0.1s 1.1s - psr is activated while we want 0s - we enable and schedule first activation to 2.5s 1s - we got a page flip that flushed fb tracking and called psr_activation to 0.1s # just ignore and move ahead since we are going to activate it soon. 2.5s - psr is activated I'm open to hear ideas to make it better or more clear. > > Thanks, > Durga Thank you very much for all the reviews! > > > mutex_unlock(&dev_priv->psr.lock); > > } > > > > -- > > 2.4.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
