On Mon, 2018-12-10 at 19:52 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-04 at 15:00 -0800, José Roberto de Souza wrote:
> > For now PSR2 is still disabled by default for all platforms but is
> > our intention to let debugfs to enable it for debug and tests
> > proporses, so intel_psr2_enabled() that is also used by debugfs to
> > decide if PSR2 is going to be enabled needs to take in
> > consideration
> > the debug field.
> > 
> > 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 | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 4c4dd1c310ce..15a2121aa64f 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -71,8 +71,11 @@ static bool psr_global_enabled(u32 debug)
> >  static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
> >  			       const struct intel_crtc_state
> > *crtc_state)
> >  {
> > +	const u32 debug_mode = dev_priv->psr.debug &
> > I915_PSR_DEBUG_MODE_MASK;
> > +
> >  	/* Disable PSR2 by default for all platforms */
> > -	if (i915_modparams.enable_psr == -1)
> > +	if (i915_modparams.enable_psr == -1 &&
> > +	    debug_mode != I915_PSR_DEBUG_ENABLE)
> >  		return false;
> 
> @@ -71,17 +71,17 @@ static bool psr_global_enabled(u32 debug)
>  static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
>                                const struct intel_crtc_state
> *crtc_state)
>  {
> -       /* Disable PSR2 by default for all platforms */
> -       if (i915_modparams.enable_psr == -1)
> -               return false;
> -
>         /* Cannot enable DSC and PSR2 simultaneously */
>         WARN_ON(crtc_state->dsc_params.compression_enable &&
>                 crtc_state->has_psr2);
>  
>         switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
> +       case I915_PSR_DEBUG_DISABLE:
>         case I915_PSR_DEBUG_FORCE_PSR1:
>                 return false;
> +       case I915_PSR_DEBUG_DEFAULT:
> +               if (i915_modparams.enable_psr <= 0)
> +                       return false;
>         default:
>                 return crtc_state->has_psr2;
>         }
> 
> Does this read any better? Keeping the condition checks together and
> also having a consistent priority between debugfs and module
> parameter
> options is easier to follow IMHO.

Yes, looks better. Changing to this.
Thanks

> 
> >  
> >  	/* Cannot enable DSC and PSR2 simultaneously */