| Message ID | 1434653006-5693-2-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 18, 2015 at 8:43 PM, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > By Spec we should just mask memup and hotplug detection > for hardware tracking cases. However we always masked > LPSP that is for low power tracking support because > without it PSR was constantly exiting and never really > getting activated. > > Now with runtime PM being enabled by default Matthew > reported that he was facing missed screen updates. So > let's remove this undesirable mask and let HW tracking > take care of cases like this were power saving features > are also running. > > WARNING: With this patch PSR depends on Audio and GPU > runtime PM to be properly enabled, working on "auto". > If either audio runtime PM or gpu runtime pm are not > properly set PSR will constant Exit and Performance > Counter will be 0. > > But the best thing of this patch is that with one more > HW tracking working the risks of missed blank screen > are minimized at most. > > This affects just core platforms where PSR exit are also > helped by HW tracking: Haswell, Broadwell and Skylake > for now. > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Matthew Garrett <mjg59@srcf.ucam.org via codon.org.uk> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> I guess I don't really understand your description, but it does sound strange ... runtime pm enabling from my patch is only about D3, power well changes are still done. And as long as we have anything enabled (even with PSR) we'll prevent D3. So the only thing I can think of is that somehow D3 wreaks something in the PSR setup and that's causing issues. Unfortunately I have no idea about our hw details around PSR and D3, so no idea. Maybe Art has some? > --- > drivers/gpu/drm/i915/intel_psr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index 5ee0fa5..6549d58 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -400,7 +400,7 @@ void intel_psr_enable(struct intel_dp *intel_dp) > > /* Avoid continuous PSR exit by masking memup and hpd */ Need to adjust the comment. > I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP | > - EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP); > + EDP_PSR_DEBUG_MASK_HPD); > > /* Enable PSR on the panel */ > hsw_psr_enable_sink(intel_dp); > -- > 2.1.0 >
On Fri, Jun 19, 2015 at 1:32 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > On Thu, Jun 18, 2015 at 8:43 PM, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: >> By Spec we should just mask memup and hotplug detection >> for hardware tracking cases. However we always masked >> LPSP that is for low power tracking support because >> without it PSR was constantly exiting and never really >> getting activated. >> >> Now with runtime PM being enabled by default Matthew >> reported that he was facing missed screen updates. So >> let's remove this undesirable mask and let HW tracking >> take care of cases like this were power saving features >> are also running. >> >> WARNING: With this patch PSR depends on Audio and GPU >> runtime PM to be properly enabled, working on "auto". >> If either audio runtime PM or gpu runtime pm are not >> properly set PSR will constant Exit and Performance >> Counter will be 0. >> >> But the best thing of this patch is that with one more >> HW tracking working the risks of missed blank screen >> are minimized at most. >> >> This affects just core platforms where PSR exit are also >> helped by HW tracking: Haswell, Broadwell and Skylake >> for now. >> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Matthew Garrett <mjg59@srcf.ucam.org via codon.org.uk> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > I guess I don't really understand your description, but it does sound > strange ... runtime pm enabling from my patch is only about D3, power > well changes are still done. And as long as we have anything enabled > (even with PSR) we'll prevent D3. > > So the only thing I can think of is that somehow D3 wreaks something > in the PSR setup and that's causing issues. Unfortunately I have no > idea about our hw details around PSR and D3, so no idea. Maybe Art has > some? I don't know this relation as well. When I found this LPSP maks that made PSR working it was totally by forcing all masks and start removing one by one up to the point that this Low Power something did the trick. At that time Artur had told about power well handling enabled, but now after Mathew reported that issue I noticed this Low power flag was also related to runtime PM... > >> --- >> drivers/gpu/drm/i915/intel_psr.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c >> index 5ee0fa5..6549d58 100644 >> --- a/drivers/gpu/drm/i915/intel_psr.c >> +++ b/drivers/gpu/drm/i915/intel_psr.c >> @@ -400,7 +400,7 @@ void intel_psr_enable(struct intel_dp *intel_dp) >> >> /* Avoid continuous PSR exit by masking memup and hpd */ > > Need to adjust the comment. not actually... memup and hpd are still there... comment matched spec but never matched the code... now it matches. > >> I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP | >> - EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP); >> + EDP_PSR_DEBUG_MASK_HPD); >> >> /* Enable PSR on the panel */ >> hsw_psr_enable_sink(intel_dp); >> -- >> 2.1.0 >> > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Daniel >> I guess I don't really understand your description, but it does sound >> strange ... runtime pm enabling from my patch is only about D3, power >> well changes are still done. And as long as we have anything enabled >> (even with PSR) we'll prevent D3. >> >> So the only thing I can think of is that somehow D3 wreaks something >> in the PSR setup and that's causing issues. Unfortunately I have no >> idea about our hw details around PSR and D3, so no idea. Maybe Art has >> some? > -- Rodrigo >I don't know this relation as well. When I found this LPSP maks that >made PSR working it was totally by forcing all masks and start >removing one by one up to the point that this Low Power something did >the trick. At that time Artur had told about power well handling >enabled, but now after Mathew reported that issue I noticed this Low >power flag was also related to runtime PM... > Let me see if I understand what is happening. Runtime PM seemed to cause PSR to miss some screen updates when you had LPSP masked, then you stopped masking LPSP and it fixed the missing updates? My first guess is that you are not in LPSP at this point, so removing the mask is effectively disabling PSR, which prevents PSR from missing screen updates. Are you still getting good PSR residency with LPSP masked in this situation? The LPSP mask is only there fro debug. We don't normally want PSR to enter without LPSP because there can be undesirable things like clocks stopping which could break the display audio controller and codec.
On Mon, Jun 22, 2015 at 3:31 PM Runyan, Arthur J <arthur.j.runyan@intel.com> wrote: > -- Daniel > >> I guess I don't really understand your description, but it does sound > >> strange ... runtime pm enabling from my patch is only about D3, power > >> well changes are still done. And as long as we have anything enabled > >> (even with PSR) we'll prevent D3. > >> > >> So the only thing I can think of is that somehow D3 wreaks something > >> in the PSR setup and that's causing issues. Unfortunately I have no > >> idea about our hw details around PSR and D3, so no idea. Maybe Art has > >> some? > > > -- Rodrigo > >I don't know this relation as well. When I found this LPSP maks that > >made PSR working it was totally by forcing all masks and start > >removing one by one up to the point that this Low Power something did > >the trick. At that time Artur had told about power well handling > >enabled, but now after Mathew reported that issue I noticed this Low > >power flag was also related to runtime PM... > > > > Let me see if I understand what is happening. Runtime PM seemed to cause > PSR to miss some screen updates when you had LPSP masked, then you stopped > masking LPSP and it fixed the missing updates? > Yes. > My first guess is that you are not in LPSP at this point, so removing the > mask is effectively disabling PSR, which prevents PSR from missing screen > updates. Are you still getting good PSR residency with LPSP masked in > this situation? > Without LPSP masked residency is 0 until I enable runtime_pm from i915 and audio. But once they get enabled residency counter increases fast. Also if I have wireless searching for network or trying to connect residency stay 0, but once connection gets stablished or stop trying to connect than PSR residency start increasing again. > > The LPSP mask is only there fro debug. We don't normally want PSR to > enter without LPSP because there can be undesirable things like clocks > stopping which could break the display audio controller and codec. > Yeah, I know. I just masked because I could never get PSR count bigger than 0 without this mask previously...
>From: Rodrigo Vivi [mailto:rodrigo.vivi@gmail.com] >>On Mon, Jun 22, 2015 at 3:31 PM Runyan, Arthur J <arthur.j.runyan@intel.com> wrote: >>-- Daniel >>>> I guess I don't really understand your description, but it does sound >>>> strange ... runtime pm enabling from my patch is only about D3, power >>>> well changes are still done. And as long as we have anything enabled >>>> (even with PSR) we'll prevent D3. >>>> >>>> So the only thing I can think of is that somehow D3 wreaks something >>>> in the PSR setup and that's causing issues. Unfortunately I have no >>>> idea about our hw details around PSR and D3, so no idea. Maybe Art has >>>> some? >>> >>-- Rodrigo >>>I don't know this relation as well. When I found this LPSP maks that >>>made PSR working it was totally by forcing all masks and start >>>removing one by one up to the point that this Low Power something did >>>the trick. At that time Artur had told about power well handling >>>enabled, but now after Mathew reported that issue I noticed this Low >>>power flag was also related to runtime PM... >>Let me see if I understand what is happening. Runtime PM seemed to cause PSR to miss some screen updates when you had LPSP masked, then you stopped masking LPSP and it fixed the missing updates? >Yes. >>My first guess is that you are not in LPSP at this point, so removing the mask is effectively disabling PSR, which prevents PSR from missing screen updates. Are you still getting good PSR residency with LPSP masked in this situation? >Without LPSP masked residency is 0 until I enable runtime_pm from i915 and audio. But once they get enabled residency counter increases fast. It sounds like you aren't really getting LPSP without runtime_pm. Probably because the audio codec has to be in D3 to allow LPSP. So I think this is a general problem with missing screen updates when PSR is entered, and the runtime_pm is modulating when PSR can be entered. Do you have a case where PSR residency is increassing and the screen updates are not missed? Are these flips or are they front buffer modifications? >Also if I have wireless searching for network or trying to connect residency stay 0, but once connection gets stablished or stop trying to connect than PSR residency start increasing again. The wireless searching is causing frequent flips or other screen udpates? Also the residency counter measurement of time is not accurate since it doesn't account for time when clocks get stopped, but you should see it making some progress if PSR is really entered. >>The LPSP mask is only there fro debug. We don't normally want PSR to enter without LPSP because there can be undesirable things like clocks stopping which could break the display audio controller and codec. >Yeah, I know. I just masked because I could never get PSR count bigger than 0 without this mask previously...
On Tue, Jun 23, 2015 at 11:40 AM Runyan, Arthur J <arthur.j.runyan@intel.com> wrote: > >From: Rodrigo Vivi [mailto:rodrigo.vivi@gmail.com] > >>On Mon, Jun 22, 2015 at 3:31 PM Runyan, Arthur J < > arthur.j.runyan@intel.com> wrote: > >>-- Daniel > >>>> I guess I don't really understand your description, but it does sound > >>>> strange ... runtime pm enabling from my patch is only about D3, power > >>>> well changes are still done. And as long as we have anything enabled > >>>> (even with PSR) we'll prevent D3. > >>>> > >>>> So the only thing I can think of is that somehow D3 wreaks something > >>>> in the PSR setup and that's causing issues. Unfortunately I have no > >>>> idea about our hw details around PSR and D3, so no idea. Maybe Art has > >>>> some? > >>> > >>-- Rodrigo > >>>I don't know this relation as well. When I found this LPSP maks that > >>>made PSR working it was totally by forcing all masks and start > >>>removing one by one up to the point that this Low Power something did > >>>the trick. At that time Artur had told about power well handling > >>>enabled, but now after Mathew reported that issue I noticed this Low > >>>power flag was also related to runtime PM... > > >>Let me see if I understand what is happening. Runtime PM seemed to > cause PSR to miss some screen updates when you had LPSP masked, then you > stopped masking LPSP and it fixed the missing updates? > >Yes. > > >>My first guess is that you are not in LPSP at this point, so removing > the mask is effectively disabling PSR, which prevents PSR from missing > screen updates. Are you still getting good PSR residency with LPSP masked > in this situation? > >Without LPSP masked residency is 0 until I enable runtime_pm from i915 > and audio. But once they get enabled residency counter increases fast. > It sounds like you aren't really getting LPSP without runtime_pm. > Probably because the audio codec has to be in D3 to allow LPSP. So I think > this is a general problem with missing screen updates when PSR is entered, > and the runtime_pm is modulating when PSR can be entered. > I believe this address Daniel's doubt. So we are fine and we need this patch if runtime_pms are enabled otherwise PSR wont work. > Do you have a case where PSR residency is increassing and the screen > updates are not missed? > yes, with runtime_pm enabled (auto) on i915 and on audio and no wireless activity PSR residency increases and we don't miss any screen update. > Are these flips or are they front buffer modifications? > The reported issue when LPSP is masked and runtime_pm is working is with frontbuffer modifications. > > >Also if I have wireless searching for network or trying to connect > residency stay 0, but once connection gets stablished or stop trying to > connect than PSR residency start increasing again. > The wireless searching is causing frequent flips or other screen udpates? > (Witout LPSP mask and rutime_pm enabled) The wireless search blocks PSR residency on 0. But screen updates happen normally as expected. When it stop trying connecting residency start increasing and screen updates still work as expected. > Also the residency counter measurement of time is not accurate since it > doesn't account for time when clocks get stopped, but you should see it > making some progress if PSR is really entered. > Yeah, it works as expected. > > >>The LPSP mask is only there fro debug. We don't normally want PSR to > enter without LPSP because there can be undesirable things like clocks > stopping which could break the display audio controller and codec. > >Yeah, I know. I just masked because I could never get PSR count bigger > than 0 without this mask previously... > > Thanks, Rodrigo.
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 5ee0fa5..6549d58 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -400,7 +400,7 @@ void intel_psr_enable(struct intel_dp *intel_dp) /* Avoid continuous PSR exit by masking memup and hpd */ I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP | - EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP); + EDP_PSR_DEBUG_MASK_HPD); /* Enable PSR on the panel */ hsw_psr_enable_sink(intel_dp);
By Spec we should just mask memup and hotplug detection for hardware tracking cases. However we always masked LPSP that is for low power tracking support because without it PSR was constantly exiting and never really getting activated. Now with runtime PM being enabled by default Matthew reported that he was facing missed screen updates. So let's remove this undesirable mask and let HW tracking take care of cases like this were power saving features are also running. WARNING: With this patch PSR depends on Audio and GPU runtime PM to be properly enabled, working on "auto". If either audio runtime PM or gpu runtime pm are not properly set PSR will constant Exit and Performance Counter will be 0. But the best thing of this patch is that with one more HW tracking working the risks of missed blank screen are minimized at most. This affects just core platforms where PSR exit are also helped by HW tracking: Haswell, Broadwell and Skylake for now. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Matthew Garrett <mjg59@srcf.ucam.org via codon.org.uk> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)