| Message ID | 20181204230032.6352-1-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] drm/i915/psr: Allow PSR2 to be enabled when debugfs asks | expand |
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. > > /* Cannot enable DSC and PSR2 simultaneously */
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 */
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; /* Cannot enable DSC and PSR2 simultaneously */
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(-)