[Intel-gfx] [PATCH v2 2/2] drm/i915/psr: Don't avoid PSR when PSR2 conditions are not met.

[Rodrigo Vivi]

On Tue, Feb 27, 2018 at 02:47:26PM -0800, Pandiyan, Dhinakaran wrote:
> 
> On Tue, 2018-02-27 at 13:29 -0800, Rodrigo Vivi wrote:
> > We can still use PSR1 when PSR2 conditions are not met.
> > 
> > So, let's split the check in a way that we make sure has_psr
> > gets set independently of PSR2 criteria.
> > 
> > v2: Duh! Handle proper return to avoid breaking PSR2.
> > v3: (DK):
> > 	- better name for psr2 conditions check function
> > 	- Don't remove FIXME block and psr2.support check.
> > 	- Add a debug message to show us what PSR or PSR2 is
> > 	  getting enabled now we have ways to enabled PSR on
> > 	  PSR2 panels.
> > 	- s/PSR2 disabled/PSR2 not enabled
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>

merged, thanks.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 64 +++++++++++++++++++++++-----------------
> >  1 file changed, 37 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 2f685beac21b..05770790a4e9 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -446,6 +446,41 @@ static void hsw_psr_activate(struct intel_dp *intel_dp)
> >  		hsw_activate_psr1(intel_dp);
> >  }
> >  
> > +static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> > +				    struct intel_crtc_state *crtc_state)
> > +{
> > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > +	const struct drm_display_mode *adjusted_mode =
> > +		&crtc_state->base.adjusted_mode;
> > +
> > +	/*
> > +	 * FIXME psr2_support is messed up. It's both computed
> > +	 * dynamically during PSR enable, and extracted from sink
> > +	 * caps during eDP detection.
> > +	 */
> > +	if (!dev_priv->psr.psr2_support)
> > +		return false;
> > +
> > +	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
> > +	if (adjusted_mode->crtc_hdisplay > 3640 ||
> > +	    adjusted_mode->crtc_vdisplay > 2304) {
> > +		DRM_DEBUG_KMS("PSR2 not enabled, panel resolution too big\n");
> > +		return false;
> > +	}
> > +
> > +	/*
> > +	 * FIXME:enable psr2 only for y-cordinate psr2 panels
> > +	 * After gtc implementation , remove this restriction.
> > +	 */
> > +	if (!dev_priv->psr.y_cord_support) {
> > +		DRM_DEBUG_KMS("PSR2 not enabled, panel does not support Y coordinate\n");
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> >  void intel_psr_compute_config(struct intel_dp *intel_dp,
> >  			      struct intel_crtc_state *crtc_state)
> >  {
> > @@ -513,34 +548,9 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> >  		return;
> >  	}
> >  
> > -	/*
> > -	 * FIXME psr2_support is messed up. It's both computed
> > -	 * dynamically during PSR enable, and extracted from sink
> > -	 * caps during eDP detection.
> > -	 */
> > -	if (!dev_priv->psr.psr2_support) {
> > -		crtc_state->has_psr = true;
> > -		return;
> > -	}
> > -
> > -	/* PSR2 is restricted to work with panel resolutions up to 3640x2304 */
> > -	if (adjusted_mode->crtc_hdisplay > 3640 ||
> > -	    adjusted_mode->crtc_vdisplay > 2304) {
> > -		DRM_DEBUG_KMS("PSR2 disabled, panel resolution too big\n");
> > -		return;
> > -	}
> > -
> > -	/*
> > -	 * FIXME:enable psr2 only for y-cordinate psr2 panels
> > -	 * After gtc implementation , remove this restriction.
> > -	 */
> > -	if (!dev_priv->psr.y_cord_support) {
> > -		DRM_DEBUG_KMS("PSR2 disabled, panel does not support Y coordinate\n");
> > -		return;
> > -	}
> > -
> >  	crtc_state->has_psr = true;
> > -	crtc_state->has_psr2 = true;
> > +	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
> > +	DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
> >  }
> >  
> >  static void intel_psr_activate(struct intel_dp *intel_dp)