Re: [PATCH v4 4/5] drm/i915/psr: Avoid PSR exit max time timeout

From: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
To: "Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"José Roberto de Souza" <jose.souza@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v4 4/5] drm/i915/psr: Avoid PSR exit max time timeout
Date: Thu, 14 Jun 2018 15:02:29 -0700	[thread overview]
Message-ID: <1529013749.7432.83.camel@intel.com> (raw)
In-Reply-To: <1529013038.7432.75.camel@intel.com>

On Thu, 2018-06-14 at 14:50 -0700, Dhinakaran Pandiyan wrote:
> On Thu, 2018-06-14 at 14:13 -0700, Rodrigo Vivi wrote:
> > 
> > On Thu, Jun 14, 2018 at 01:34:32PM -0700, José Roberto de Souza
> > wrote:
> > > 
> > > 
> > > Specification requires that max time should be masked from bdw
> > > and
> > > forward but it can be also safely enabled to hsw.
> > > This will make PSR exits more deterministic and only when really
> > > needed. If this was used to fix a issue in some panel than can
> > > only self-refresh for a few seconds, that panel will interrupt
> > > and assert one of the PSR errors handled in:
> > > 'drm/i915/psr: Handle PSR RFB storage error' and
> > > 'drm/i915/psr: Begin to handle PSR/PSR2 errors set by sink'
> > > 
> > > Also fixing style here.
> > I don't believe that is a style error. So I believe only
> > the MAX_SLEEP one should be added following same style.
> > 
> > > 
> > > 
> > > 
> > > Spec: 21664
> > > 
> > > v4:
> > > patch moved to before 'drm/i915/psr/bdw+: Enable CRC check in the
> > > static frame on the sink side' to avoid touch in 2 patches
> > > EDP_PSR_DEBUG.
> > I thought this change here depended on having CRC check enabled
> > so it should come first and only bdw+...
> > Isn't this the case?
> The dependency is the other way around, sending CRC's requires
> timeout
> to be disabled. Since disabling timeouts results in a predictable
> behavior, we can do it for all platforms.
> 
> 
> > 
> > 
> > > 
> > > 
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index fd240e45f341..177cd57b1029 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -629,10 +629,11 @@ static void hsw_psr_enable_source(struct
> > > intel_dp *intel_dp,
> > >  		 * on frontbuffer tracking.
> > >  		 */
> > >  		I915_WRITE(EDP_PSR_DEBUG,
> > > -			   EDP_PSR_DEBUG_MASK_MEMUP |
> > > -			   EDP_PSR_DEBUG_MASK_HPD |
> > > -			   EDP_PSR_DEBUG_MASK_LPSP |
> > > -			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> > > +			  EDP_PSR_DEBUG_MASK_MEMUP |
> > > +			  EDP_PSR_DEBUG_MASK_HPD |
> > > +			  EDP_PSR_DEBUG_MASK_LPSP |
> > > +			  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
> > > +			  EDP_PSR_DEBUG_MASK_MAX_SLEEP);

With indentation reverted to how it was earlier,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > >  	}
> > >  }
> > >  
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx