| Message ID | 20180227212913.14083-2-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > 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)
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@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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)
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)
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@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 64 +++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 27 deletions(-)