* [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 @ 2022-02-23 19:41 José Roberto de Souza 2022-02-24 10:12 ` Ville Syrjälä 2022-02-24 12:39 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork 0 siblings, 2 replies; 9+ messages in thread From: José Roberto de Souza @ 2022-02-23 19:41 UTC (permalink / raw) To: intel-gfx Some users are suffering with PSR2 issues that are under debug or issues that were root caused to panel firmware, to make life of those users easier here adding a option to disable PSR1 with kernel parameter. Using the same enable_psr that is current used to turn PSR1 and PSR2 off or on and adding a new value to only disable PSR2. The previous valid values did not had their behavior changed. Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4951 Cc: Jouni Högander <jouni.hogander@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++ drivers/gpu/drm/i915/i915_params.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 2e0b092f4b6be..fc6b684bb7bec 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -100,11 +100,15 @@ static bool psr_global_enabled(struct intel_dp *intel_dp) static bool psr2_global_enabled(struct intel_dp *intel_dp) { + struct drm_i915_private *i915 = dp_to_i915(intel_dp); + switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) { case I915_PSR_DEBUG_DISABLE: case I915_PSR_DEBUG_FORCE_PSR1: return false; default: + if (i915->params.enable_psr == 2) + return false; return true; } } diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index eea355c2fc28a..a9b97e6eb3df0 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -94,7 +94,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400, i915_param_named_unsafe(enable_psr, int, 0400, "Enable PSR " - "(0=disabled, 1=enabled) " + "(0=disabled, 1=enable up to PSR2 if supported, 2=enable up to PSR1) " "Default: -1 (use per-chip default)"); i915_param_named(psr_safest_params, bool, 0400, -- 2.35.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 2022-02-23 19:41 [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 José Roberto de Souza @ 2022-02-24 10:12 ` Ville Syrjälä 2022-02-24 13:01 ` Souza, Jose 2022-02-24 12:39 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork 1 sibling, 1 reply; 9+ messages in thread From: Ville Syrjälä @ 2022-02-24 10:12 UTC (permalink / raw) To: José Roberto de Souza; +Cc: intel-gfx On Wed, Feb 23, 2022 at 11:41:03AM -0800, José Roberto de Souza wrote: > Some users are suffering with PSR2 issues that are under debug or > issues that were root caused to panel firmware, to make life of those > users easier here adding a option to disable PSR1 with kernel > parameter. > > Using the same enable_psr that is current used to turn PSR1 and PSR2 > off or on and adding a new value to only disable PSR2. > The previous valid values did not had their behavior changed. > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4951 > Cc: Jouni Högander <jouni.hogander@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++ > drivers/gpu/drm/i915/i915_params.c | 2 +- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index 2e0b092f4b6be..fc6b684bb7bec 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -100,11 +100,15 @@ static bool psr_global_enabled(struct intel_dp *intel_dp) > > static bool psr2_global_enabled(struct intel_dp *intel_dp) > { > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > + > switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) { > case I915_PSR_DEBUG_DISABLE: > case I915_PSR_DEBUG_FORCE_PSR1: > return false; > default: > + if (i915->params.enable_psr == 2) > + return false; > return true; > } > } > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index eea355c2fc28a..a9b97e6eb3df0 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -94,7 +94,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400, > > i915_param_named_unsafe(enable_psr, int, 0400, > "Enable PSR " > - "(0=disabled, 1=enabled) " > + "(0=disabled, 1=enable up to PSR2 if supported, 2=enable up to PSR1) " That seems very unintuitive. I would just make it 1==PSR1 and 2==PSR2. > "Default: -1 (use per-chip default)"); > > i915_param_named(psr_safest_params, bool, 0400, > -- > 2.35.1 -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 2022-02-24 10:12 ` Ville Syrjälä @ 2022-02-24 13:01 ` Souza, Jose 2022-02-24 13:06 ` Ville Syrjälä 0 siblings, 1 reply; 9+ messages in thread From: Souza, Jose @ 2022-02-24 13:01 UTC (permalink / raw) To: ville.syrjala@linux.intel.com; +Cc: intel-gfx@lists.freedesktop.org On Thu, 2022-02-24 at 12:12 +0200, Ville Syrjälä wrote: > On Wed, Feb 23, 2022 at 11:41:03AM -0800, José Roberto de Souza wrote: > > Some users are suffering with PSR2 issues that are under debug or > > issues that were root caused to panel firmware, to make life of those > > users easier here adding a option to disable PSR1 with kernel > > parameter. > > > > Using the same enable_psr that is current used to turn PSR1 and PSR2 > > off or on and adding a new value to only disable PSR2. > > The previous valid values did not had their behavior changed. > > > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4951 > > Cc: Jouni Högander <jouni.hogander@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++ > > drivers/gpu/drm/i915/i915_params.c | 2 +- > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > index 2e0b092f4b6be..fc6b684bb7bec 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -100,11 +100,15 @@ static bool psr_global_enabled(struct intel_dp *intel_dp) > > > > static bool psr2_global_enabled(struct intel_dp *intel_dp) > > { > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > + > > switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) { > > case I915_PSR_DEBUG_DISABLE: > > case I915_PSR_DEBUG_FORCE_PSR1: > > return false; > > default: > > + if (i915->params.enable_psr == 2) > > + return false; > > return true; > > } > > } > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > index eea355c2fc28a..a9b97e6eb3df0 100644 > > --- a/drivers/gpu/drm/i915/i915_params.c > > +++ b/drivers/gpu/drm/i915/i915_params.c > > @@ -94,7 +94,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400, > > > > i915_param_named_unsafe(enable_psr, int, 0400, > > "Enable PSR " > > - "(0=disabled, 1=enabled) " > > + "(0=disabled, 1=enable up to PSR2 if supported, 2=enable up to PSR1) " > > That seems very unintuitive. I would just make it 1==PSR1 and 2==PSR2. This will break current behavior. Other option is to add another parameter... > > > "Default: -1 (use per-chip default)"); > > > > i915_param_named(psr_safest_params, bool, 0400, > > -- > > 2.35.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 2022-02-24 13:01 ` Souza, Jose @ 2022-02-24 13:06 ` Ville Syrjälä 2022-02-24 13:11 ` Ville Syrjälä 0 siblings, 1 reply; 9+ messages in thread From: Ville Syrjälä @ 2022-02-24 13:06 UTC (permalink / raw) To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org On Thu, Feb 24, 2022 at 01:01:24PM +0000, Souza, Jose wrote: > On Thu, 2022-02-24 at 12:12 +0200, Ville Syrjälä wrote: > > On Wed, Feb 23, 2022 at 11:41:03AM -0800, José Roberto de Souza wrote: > > > Some users are suffering with PSR2 issues that are under debug or > > > issues that were root caused to panel firmware, to make life of those > > > users easier here adding a option to disable PSR1 with kernel > > > parameter. > > > > > > Using the same enable_psr that is current used to turn PSR1 and PSR2 > > > off or on and adding a new value to only disable PSR2. > > > The previous valid values did not had their behavior changed. > > > > > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4951 > > > Cc: Jouni Högander <jouni.hogander@intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++ > > > drivers/gpu/drm/i915/i915_params.c | 2 +- > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > > index 2e0b092f4b6be..fc6b684bb7bec 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > @@ -100,11 +100,15 @@ static bool psr_global_enabled(struct intel_dp *intel_dp) > > > > > > static bool psr2_global_enabled(struct intel_dp *intel_dp) > > > { > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > + > > > switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) { > > > case I915_PSR_DEBUG_DISABLE: > > > case I915_PSR_DEBUG_FORCE_PSR1: > > > return false; > > > default: > > > + if (i915->params.enable_psr == 2) > > > + return false; > > > return true; > > > } > > > } > > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > > index eea355c2fc28a..a9b97e6eb3df0 100644 > > > --- a/drivers/gpu/drm/i915/i915_params.c > > > +++ b/drivers/gpu/drm/i915/i915_params.c > > > @@ -94,7 +94,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400, > > > > > > i915_param_named_unsafe(enable_psr, int, 0400, > > > "Enable PSR " > > > - "(0=disabled, 1=enabled) " > > > + "(0=disabled, 1=enable up to PSR2 if supported, 2=enable up to PSR1) " > > > > That seems very unintuitive. I would just make it 1==PSR1 and 2==PSR2. > > This will break current behavior. It's a modparam. We routinely break those since they are not meant to used by normal users as any kind of permanent "make my machine work" knob. -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 2022-02-24 13:06 ` Ville Syrjälä @ 2022-02-24 13:11 ` Ville Syrjälä 2022-02-24 14:15 ` Souza, Jose 0 siblings, 1 reply; 9+ messages in thread From: Ville Syrjälä @ 2022-02-24 13:11 UTC (permalink / raw) To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org On Thu, Feb 24, 2022 at 03:06:30PM +0200, Ville Syrjälä wrote: > On Thu, Feb 24, 2022 at 01:01:24PM +0000, Souza, Jose wrote: > > On Thu, 2022-02-24 at 12:12 +0200, Ville Syrjälä wrote: > > > On Wed, Feb 23, 2022 at 11:41:03AM -0800, José Roberto de Souza wrote: > > > > Some users are suffering with PSR2 issues that are under debug or > > > > issues that were root caused to panel firmware, to make life of those > > > > users easier here adding a option to disable PSR1 with kernel > > > > parameter. > > > > > > > > Using the same enable_psr that is current used to turn PSR1 and PSR2 > > > > off or on and adding a new value to only disable PSR2. > > > > The previous valid values did not had their behavior changed. > > > > > > > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4951 > > > > Cc: Jouni Högander <jouni.hogander@intel.com> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++ > > > > drivers/gpu/drm/i915/i915_params.c | 2 +- > > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > > > index 2e0b092f4b6be..fc6b684bb7bec 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > @@ -100,11 +100,15 @@ static bool psr_global_enabled(struct intel_dp *intel_dp) > > > > > > > > static bool psr2_global_enabled(struct intel_dp *intel_dp) > > > > { > > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > + > > > > switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) { > > > > case I915_PSR_DEBUG_DISABLE: > > > > case I915_PSR_DEBUG_FORCE_PSR1: > > > > return false; > > > > default: > > > > + if (i915->params.enable_psr == 2) > > > > + return false; > > > > return true; > > > > } > > > > } > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > > > index eea355c2fc28a..a9b97e6eb3df0 100644 > > > > --- a/drivers/gpu/drm/i915/i915_params.c > > > > +++ b/drivers/gpu/drm/i915/i915_params.c > > > > @@ -94,7 +94,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400, > > > > > > > > i915_param_named_unsafe(enable_psr, int, 0400, > > > > "Enable PSR " > > > > - "(0=disabled, 1=enabled) " > > > > + "(0=disabled, 1=enable up to PSR2 if supported, 2=enable up to PSR1) " > > > > > > That seems very unintuitive. I would just make it 1==PSR1 and 2==PSR2. > > > > This will break current behavior. > > It's a modparam. We routinely break those since they are not meant > to used by normal users as any kind of permanent "make my machine > work" knob. But I guess if we want to make it a bit less painful your idea of a new modparam might work. + deprecate the old param and remove after one or two kernel releases. -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 2022-02-24 13:11 ` Ville Syrjälä @ 2022-02-24 14:15 ` Souza, Jose 2022-02-24 14:29 ` Rodrigo Vivi 2022-02-24 14:29 ` Ville Syrjälä 0 siblings, 2 replies; 9+ messages in thread From: Souza, Jose @ 2022-02-24 14:15 UTC (permalink / raw) To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo + Rodrigo On Thu, 2022-02-24 at 15:11 +0200, Ville Syrjälä wrote: > On Thu, Feb 24, 2022 at 03:06:30PM +0200, Ville Syrjälä wrote: > > On Thu, Feb 24, 2022 at 01:01:24PM +0000, Souza, Jose wrote: > > > On Thu, 2022-02-24 at 12:12 +0200, Ville Syrjälä wrote: > > > > On Wed, Feb 23, 2022 at 11:41:03AM -0800, José Roberto de Souza wrote: > > > > > Some users are suffering with PSR2 issues that are under debug or > > > > > issues that were root caused to panel firmware, to make life of those > > > > > users easier here adding a option to disable PSR1 with kernel > > > > > parameter. > > > > > > > > > > Using the same enable_psr that is current used to turn PSR1 and PSR2 > > > > > off or on and adding a new value to only disable PSR2. > > > > > The previous valid values did not had their behavior changed. > > > > > > > > > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4951 > > > > > Cc: Jouni Högander <jouni.hogander@intel.com> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++ > > > > > drivers/gpu/drm/i915/i915_params.c | 2 +- > > > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > index 2e0b092f4b6be..fc6b684bb7bec 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > @@ -100,11 +100,15 @@ static bool psr_global_enabled(struct intel_dp *intel_dp) > > > > > > > > > > static bool psr2_global_enabled(struct intel_dp *intel_dp) > > > > > { > > > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > > + > > > > > switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) { > > > > > case I915_PSR_DEBUG_DISABLE: > > > > > case I915_PSR_DEBUG_FORCE_PSR1: > > > > > return false; > > > > > default: > > > > > + if (i915->params.enable_psr == 2) > > > > > + return false; > > > > > return true; > > > > > } > > > > > } > > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > > > > index eea355c2fc28a..a9b97e6eb3df0 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_params.c > > > > > +++ b/drivers/gpu/drm/i915/i915_params.c > > > > > @@ -94,7 +94,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400, > > > > > > > > > > i915_param_named_unsafe(enable_psr, int, 0400, > > > > > "Enable PSR " > > > > > - "(0=disabled, 1=enabled) " > > > > > + "(0=disabled, 1=enable up to PSR2 if supported, 2=enable up to PSR1) " > > > > > > > > That seems very unintuitive. I would just make it 1==PSR1 and 2==PSR2. > > > > > > This will break current behavior. > > > > It's a modparam. We routinely break those since they are not meant > > to used by normal users as any kind of permanent "make my machine > > work" knob. > > But I guess if we want to make it a bit less painful your idea of a new > modparam might work. + deprecate the old param and remove after one or > two kernel releases. Was thinking about a new one to limit the version of PSR: enable_psr_version default = 0(per-chip default), 1 = up to PSR1, 2 = PSR2 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 2022-02-24 14:15 ` Souza, Jose @ 2022-02-24 14:29 ` Rodrigo Vivi 2022-02-24 14:29 ` Ville Syrjälä 1 sibling, 0 replies; 9+ messages in thread From: Rodrigo Vivi @ 2022-02-24 14:29 UTC (permalink / raw) To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org On Thu, Feb 24, 2022 at 02:15:44PM +0000, Souza, Jose wrote: > + Rodrigo > > On Thu, 2022-02-24 at 15:11 +0200, Ville Syrjälä wrote: > > On Thu, Feb 24, 2022 at 03:06:30PM +0200, Ville Syrjälä wrote: > > > On Thu, Feb 24, 2022 at 01:01:24PM +0000, Souza, Jose wrote: > > > > On Thu, 2022-02-24 at 12:12 +0200, Ville Syrjälä wrote: > > > > > On Wed, Feb 23, 2022 at 11:41:03AM -0800, José Roberto de Souza wrote: > > > > > > Some users are suffering with PSR2 issues that are under debug or > > > > > > issues that were root caused to panel firmware, to make life of those > > > > > > users easier here adding a option to disable PSR1 with kernel > > > > > > parameter. > > > > > > > > > > > > Using the same enable_psr that is current used to turn PSR1 and PSR2 > > > > > > off or on and adding a new value to only disable PSR2. > > > > > > The previous valid values did not had their behavior changed. > > > > > > > > > > > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4951 > > > > > > Cc: Jouni Högander <jouni.hogander@intel.com> > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++ > > > > > > drivers/gpu/drm/i915/i915_params.c | 2 +- > > > > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > index 2e0b092f4b6be..fc6b684bb7bec 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > @@ -100,11 +100,15 @@ static bool psr_global_enabled(struct intel_dp *intel_dp) > > > > > > > > > > > > static bool psr2_global_enabled(struct intel_dp *intel_dp) > > > > > > { > > > > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > > > + > > > > > > switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) { > > > > > > case I915_PSR_DEBUG_DISABLE: > > > > > > case I915_PSR_DEBUG_FORCE_PSR1: > > > > > > return false; > > > > > > default: > > > > > > + if (i915->params.enable_psr == 2) > > > > > > + return false; > > > > > > return true; > > > > > > } > > > > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > > > > > index eea355c2fc28a..a9b97e6eb3df0 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_params.c > > > > > > +++ b/drivers/gpu/drm/i915/i915_params.c > > > > > > @@ -94,7 +94,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400, > > > > > > > > > > > > i915_param_named_unsafe(enable_psr, int, 0400, > > > > > > "Enable PSR " > > > > > > - "(0=disabled, 1=enabled) " > > > > > > + "(0=disabled, 1=enable up to PSR2 if supported, 2=enable up to PSR1) " > > > > > > > > > > That seems very unintuitive. I would just make it 1==PSR1 and 2==PSR2. > > > > > > > > This will break current behavior. > > > > > > It's a modparam. We routinely break those since they are not meant > > > to used by normal users as any kind of permanent "make my machine > > > work" knob. > > > > But I guess if we want to make it a bit less painful your idea of a new > > modparam might work. + deprecate the old param and remove after one or > > two kernel releases. > > Was thinking about a new one to limit the version of PSR: > > enable_psr_version > default = 0(per-chip default), 1 = up to PSR1, 2 = PSR2 > > > > My trauma with PSR makes me to ask if we really wants this option. Is PSR1 really more stable at this point? Shouldn't we just disable everything while we fix the PSR2 and avoid touching the parameter? If we are confident that PSR1 is better than simply disabling, then I like the idea of a new parameter that overrides the previous one so we can delete in a couple months and keep only one. i915.psr default=-1 (per-chip default), 0=disabled, 1=psr1, 2=psr2 but I also would be okay breaking a parameter that is marked as unstable and which main use externally is to disable the psr (i915.enable_psr=0). Thanks, Rodrigo. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 2022-02-24 14:15 ` Souza, Jose 2022-02-24 14:29 ` Rodrigo Vivi @ 2022-02-24 14:29 ` Ville Syrjälä 1 sibling, 0 replies; 9+ messages in thread From: Ville Syrjälä @ 2022-02-24 14:29 UTC (permalink / raw) To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo On Thu, Feb 24, 2022 at 02:15:44PM +0000, Souza, Jose wrote: > + Rodrigo > > On Thu, 2022-02-24 at 15:11 +0200, Ville Syrjälä wrote: > > On Thu, Feb 24, 2022 at 03:06:30PM +0200, Ville Syrjälä wrote: > > > On Thu, Feb 24, 2022 at 01:01:24PM +0000, Souza, Jose wrote: > > > > On Thu, 2022-02-24 at 12:12 +0200, Ville Syrjälä wrote: > > > > > On Wed, Feb 23, 2022 at 11:41:03AM -0800, José Roberto de Souza wrote: > > > > > > Some users are suffering with PSR2 issues that are under debug or > > > > > > issues that were root caused to panel firmware, to make life of those > > > > > > users easier here adding a option to disable PSR1 with kernel > > > > > > parameter. > > > > > > > > > > > > Using the same enable_psr that is current used to turn PSR1 and PSR2 > > > > > > off or on and adding a new value to only disable PSR2. > > > > > > The previous valid values did not had their behavior changed. > > > > > > > > > > > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4951 > > > > > > Cc: Jouni Högander <jouni.hogander@intel.com> > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++ > > > > > > drivers/gpu/drm/i915/i915_params.c | 2 +- > > > > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > index 2e0b092f4b6be..fc6b684bb7bec 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > @@ -100,11 +100,15 @@ static bool psr_global_enabled(struct intel_dp *intel_dp) > > > > > > > > > > > > static bool psr2_global_enabled(struct intel_dp *intel_dp) > > > > > > { > > > > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > > > + > > > > > > switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) { > > > > > > case I915_PSR_DEBUG_DISABLE: > > > > > > case I915_PSR_DEBUG_FORCE_PSR1: > > > > > > return false; > > > > > > default: > > > > > > + if (i915->params.enable_psr == 2) > > > > > > + return false; > > > > > > return true; > > > > > > } > > > > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > > > > > index eea355c2fc28a..a9b97e6eb3df0 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_params.c > > > > > > +++ b/drivers/gpu/drm/i915/i915_params.c > > > > > > @@ -94,7 +94,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0400, > > > > > > > > > > > > i915_param_named_unsafe(enable_psr, int, 0400, > > > > > > "Enable PSR " > > > > > > - "(0=disabled, 1=enabled) " > > > > > > + "(0=disabled, 1=enable up to PSR2 if supported, 2=enable up to PSR1) " > > > > > > > > > > That seems very unintuitive. I would just make it 1==PSR1 and 2==PSR2. > > > > > > > > This will break current behavior. > > > > > > It's a modparam. We routinely break those since they are not meant > > > to used by normal users as any kind of permanent "make my machine > > > work" knob. > > > > But I guess if we want to make it a bit less painful your idea of a new > > modparam might work. + deprecate the old param and remove after one or > > two kernel releases. > > Was thinking about a new one to limit the version of PSR: > > enable_psr_version > default = 0(per-chip default), 1 = up to PSR1, 2 = PSR2 I would use the standard -1==default, 0==disable convention here. Then we can drop the old param after a short transition period. -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Allow users to disable PSR2 2022-02-23 19:41 [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 José Roberto de Souza 2022-02-24 10:12 ` Ville Syrjälä @ 2022-02-24 12:39 ` Patchwork 1 sibling, 0 replies; 9+ messages in thread From: Patchwork @ 2022-02-24 12:39 UTC (permalink / raw) To: José Roberto de Souza; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 3628 bytes --] == Series Details == Series: drm/i915/display: Allow users to disable PSR2 URL : https://patchwork.freedesktop.org/series/100658/ State : success == Summary == CI Bug Log - changes from CI_DRM_11277 -> Patchwork_22389 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22389/index.html Participating hosts (45 -> 41) ------------------------------ Missing (4): fi-bsw-cyan fi-icl-u2 fi-bdw-samus fi-pnv-d510 Known issues ------------ Here are the changes found in Patchwork_22389 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@amdgpu/amd_cs_nop@fork-compute0: - fi-ivb-3770: NOTRUN -> [SKIP][1] ([fdo#109271]) +17 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22389/fi-ivb-3770/igt@amdgpu/amd_cs_nop@fork-compute0.html * igt@i915_selftest@live@hangcheck: - fi-snb-2600: [PASS][2] -> [INCOMPLETE][3] ([i915#3921]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11277/fi-snb-2600/igt@i915_selftest@live@hangcheck.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22389/fi-snb-2600/igt@i915_selftest@live@hangcheck.html #### Possible fixes #### * igt@i915_selftest@live@hangcheck: - fi-ivb-3770: [INCOMPLETE][4] ([i915#3303]) -> [PASS][5] [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11277/fi-ivb-3770/igt@i915_selftest@live@hangcheck.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22389/fi-ivb-3770/igt@i915_selftest@live@hangcheck.html #### Warnings #### * igt@kms_psr@primary_page_flip: - fi-skl-6600u: [FAIL][6] ([i915#4547]) -> [INCOMPLETE][7] ([i915#4547] / [i915#4838]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11277/fi-skl-6600u/igt@kms_psr@primary_page_flip.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22389/fi-skl-6600u/igt@kms_psr@primary_page_flip.html * igt@runner@aborted: - fi-skl-6600u: [FAIL][8] ([i915#4312]) -> [FAIL][9] ([i915#2722] / [i915#4312]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11277/fi-skl-6600u/igt@runner@aborted.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22389/fi-skl-6600u/igt@runner@aborted.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722 [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303 [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576 [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547 [i915#4838]: https://gitlab.freedesktop.org/drm/intel/issues/4838 Build changes ------------- * Linux: CI_DRM_11277 -> Patchwork_22389 CI-20190529: 20190529 CI_DRM_11277: a9d1ffee8dbe2c5506cccf9077eab8fe439eea46 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_6355: 83ec34916bd8268bc331105cf77c4d3d3cd352be @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_22389: f980fb64e671359fad50127e8d6468cad25a85ab @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == f980fb64e671 drm/i915/display: Allow users to disable PSR2 == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22389/index.html [-- Attachment #2: Type: text/html, Size: 4414 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-24 14:30 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-02-23 19:41 [Intel-gfx] [PATCH] drm/i915/display: Allow users to disable PSR2 José Roberto de Souza 2022-02-24 10:12 ` Ville Syrjälä 2022-02-24 13:01 ` Souza, Jose 2022-02-24 13:06 ` Ville Syrjälä 2022-02-24 13:11 ` Ville Syrjälä 2022-02-24 14:15 ` Souza, Jose 2022-02-24 14:29 ` Rodrigo Vivi 2022-02-24 14:29 ` Ville Syrjälä 2022-02-24 12:39 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox