All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/psr: simplify enable_psr handling
@ 2018-12-21 17:23 Ross Zwisler
  2018-12-21 19:23   ` Dhinakaran Pandiyan
  2019-01-09 19:32 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 6+ messages in thread
From: Ross Zwisler @ 2018-12-21 17:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ross Zwisler, Daniel Vetter, David Airlie, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, dri-devel, intel-gfx, Jon Flatley,
	Dhinakaran Pandiyan

The following commit:

commit 2bdd045e3a30 ("drm/i915/psr: Check if VBT says PSR can be enabled.")

added some code with no usable functionality.  Regardless of how the psr
default is set up in the BDB_DRIVER_FEATURES section, if the enable_psr
module parameter isn't specified it defaults to 0.

Remove this dead code, simplify the way that enable_psr is handled and
update the module parameter string to match the actual functionality.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ross Zwisler <zwisler@google.com>
---
 drivers/gpu/drm/i915/i915_drv.h    | 1 -
 drivers/gpu/drm/i915/i915_params.c | 4 +---
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 drivers/gpu/drm/i915/intel_bios.c  | 1 -
 drivers/gpu/drm/i915/intel_psr.c   | 7 -------
 5 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 872a2e159a5f9..b4c50ba0b22a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1115,7 +1115,6 @@ struct intel_vbt_data {
 	} edp;
 
 	struct {
-		bool enable;
 		bool full_link;
 		bool require_aux_wakeup;
 		int idle_frames;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 295e981e4a398..80ce8758c3c69 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -87,9 +87,7 @@ i915_param_named_unsafe(enable_ppgtt, int, 0400,
 	"(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with extended address space)");
 
 i915_param_named_unsafe(enable_psr, int, 0600,
-	"Enable PSR "
-	"(0=disabled, 1=enabled) "
-	"Default: -1 (use per-chip default)");
+	"Enable PSR (default: false)");
 
 i915_param_named_unsafe(alpha_support, bool, 0400,
 	"Enable alpha quality driver support for latest hardware. "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 6c4d4a21474b5..144572f17a83d 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -42,7 +42,7 @@ struct drm_printer;
 	param(int, enable_dc, -1) \
 	param(int, enable_fbc, -1) \
 	param(int, enable_ppgtt, -1) \
-	param(int, enable_psr, -1) \
+	param(int, enable_psr, 0) \
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 1faa494e2bc91..d676d483d5cf1 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -551,7 +551,6 @@ parse_driver_features(struct drm_i915_private *dev_priv,
 	 */
 	if (!driver->drrs_enabled)
 		dev_priv->vbt.drrs_type = DRRS_NOT_SUPPORTED;
-	dev_priv->vbt.psr.enable = driver->psr_enabled;
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b6838b525502e..26e7eb318cf07 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -1065,13 +1065,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	if (!dev_priv->psr.sink_support)
 		return;
 
-	if (i915_modparams.enable_psr == -1) {
-		i915_modparams.enable_psr = dev_priv->vbt.psr.enable;
-
-		/* Per platform default: all disabled. */
-		i915_modparams.enable_psr = 0;
-	}
-
 	/* Set link_standby x link_off defaults */
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		/* HSW and BDW require workarounds that we don't implement. */
-- 
2.20.1.415.g653613c723-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/psr: simplify enable_psr handling
  2018-12-21 17:23 [PATCH] drm/i915/psr: simplify enable_psr handling Ross Zwisler
@ 2018-12-21 19:23   ` Dhinakaran Pandiyan
  2019-01-09 19:32 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 0 replies; 6+ messages in thread
From: Dhinakaran Pandiyan @ 2018-12-21 19:23 UTC (permalink / raw)
  To: Ross Zwisler, linux-kernel
  Cc: Daniel Vetter, David Airlie, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, dri-devel, intel-gfx, Jon Flatley

On Fri, 2018-12-21 at 10:23 -0700, Ross Zwisler wrote:
> The following commit:
> 
> commit 2bdd045e3a30 ("drm/i915/psr: Check if VBT says PSR can be
> enabled.")
> 
> added some code with no usable functionality.  Regardless of how the
> psr
> default is set up in the BDB_DRIVER_FEATURES section, if the
> enable_psr
> module parameter isn't specified it defaults to 0.
Right, that was intentional and the commit message even makes a note of
it 
" Note: The feature currently remains disabled by default for all
platforms irrespective of what VBT says."


Anyway, we've enabled the feature by default now and the current code
should take into account the VBT flag if the module parameter is left
to a default value. Please check git://anongit.freedesktop.org/drm-tip
drm-tip.

-DK
> 
> Remove this dead code, simplify the way that enable_psr is handled
> and
> update the module parameter string to match the actual functionality.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Ross Zwisler <zwisler@google.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    | 1 -
>  drivers/gpu/drm/i915/i915_params.c | 4 +---
>  drivers/gpu/drm/i915/i915_params.h | 2 +-
>  drivers/gpu/drm/i915/intel_bios.c  | 1 -
>  drivers/gpu/drm/i915/intel_psr.c   | 7 -------
>  5 files changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 872a2e159a5f9..b4c50ba0b22a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1115,7 +1115,6 @@ struct intel_vbt_data {
>  	} edp;
>  
>  	struct {
> -		bool enable;
>  		bool full_link;
>  		bool require_aux_wakeup;
>  		int idle_frames;
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 295e981e4a398..80ce8758c3c69 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -87,9 +87,7 @@ i915_param_named_unsafe(enable_ppgtt, int, 0400,
>  	"(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full
> with extended address space)");
>  
>  i915_param_named_unsafe(enable_psr, int, 0600,
> -	"Enable PSR "
> -	"(0=disabled, 1=enabled) "
> -	"Default: -1 (use per-chip default)");
> +	"Enable PSR (default: false)");
>  
>  i915_param_named_unsafe(alpha_support, bool, 0400,
>  	"Enable alpha quality driver support for latest hardware. "
> diff --git a/drivers/gpu/drm/i915/i915_params.h
> b/drivers/gpu/drm/i915/i915_params.h
> index 6c4d4a21474b5..144572f17a83d 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -42,7 +42,7 @@ struct drm_printer;
>  	param(int, enable_dc, -1) \
>  	param(int, enable_fbc, -1) \
>  	param(int, enable_ppgtt, -1) \
> -	param(int, enable_psr, -1) \
> +	param(int, enable_psr, 0) \
>  	param(int, disable_power_well, -1) \
>  	param(int, enable_ips, 1) \
>  	param(int, invert_brightness, 0) \
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 1faa494e2bc91..d676d483d5cf1 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -551,7 +551,6 @@ parse_driver_features(struct drm_i915_private
> *dev_priv,
>  	 */
>  	if (!driver->drrs_enabled)
>  		dev_priv->vbt.drrs_type = DRRS_NOT_SUPPORTED;
> -	dev_priv->vbt.psr.enable = driver->psr_enabled;
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index b6838b525502e..26e7eb318cf07 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1065,13 +1065,6 @@ void intel_psr_init(struct drm_i915_private
> *dev_priv)
>  	if (!dev_priv->psr.sink_support)
>  		return;
>  
> -	if (i915_modparams.enable_psr == -1) {
> -		i915_modparams.enable_psr = dev_priv->vbt.psr.enable;
> -
> -		/* Per platform default: all disabled. */
> -		i915_modparams.enable_psr = 0;
> -	}
> -
>  	/* Set link_standby x link_off defaults */
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		/* HSW and BDW require workarounds that we don't
> implement. */


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/psr: simplify enable_psr handling
@ 2018-12-21 19:23   ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 6+ messages in thread
From: Dhinakaran Pandiyan @ 2018-12-21 19:23 UTC (permalink / raw)
  To: Ross Zwisler, linux-kernel
  Cc: David Airlie, intel-gfx, dri-devel, Rodrigo Vivi, Jon Flatley

On Fri, 2018-12-21 at 10:23 -0700, Ross Zwisler wrote:
> The following commit:
> 
> commit 2bdd045e3a30 ("drm/i915/psr: Check if VBT says PSR can be
> enabled.")
> 
> added some code with no usable functionality.  Regardless of how the
> psr
> default is set up in the BDB_DRIVER_FEATURES section, if the
> enable_psr
> module parameter isn't specified it defaults to 0.
Right, that was intentional and the commit message even makes a note of
it 
" Note: The feature currently remains disabled by default for all
platforms irrespective of what VBT says."


Anyway, we've enabled the feature by default now and the current code
should take into account the VBT flag if the module parameter is left
to a default value. Please check git://anongit.freedesktop.org/drm-tip
drm-tip.

-DK
> 
> Remove this dead code, simplify the way that enable_psr is handled
> and
> update the module parameter string to match the actual functionality.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Ross Zwisler <zwisler@google.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    | 1 -
>  drivers/gpu/drm/i915/i915_params.c | 4 +---
>  drivers/gpu/drm/i915/i915_params.h | 2 +-
>  drivers/gpu/drm/i915/intel_bios.c  | 1 -
>  drivers/gpu/drm/i915/intel_psr.c   | 7 -------
>  5 files changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 872a2e159a5f9..b4c50ba0b22a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1115,7 +1115,6 @@ struct intel_vbt_data {
>  	} edp;
>  
>  	struct {
> -		bool enable;
>  		bool full_link;
>  		bool require_aux_wakeup;
>  		int idle_frames;
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 295e981e4a398..80ce8758c3c69 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -87,9 +87,7 @@ i915_param_named_unsafe(enable_ppgtt, int, 0400,
>  	"(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full
> with extended address space)");
>  
>  i915_param_named_unsafe(enable_psr, int, 0600,
> -	"Enable PSR "
> -	"(0=disabled, 1=enabled) "
> -	"Default: -1 (use per-chip default)");
> +	"Enable PSR (default: false)");
>  
>  i915_param_named_unsafe(alpha_support, bool, 0400,
>  	"Enable alpha quality driver support for latest hardware. "
> diff --git a/drivers/gpu/drm/i915/i915_params.h
> b/drivers/gpu/drm/i915/i915_params.h
> index 6c4d4a21474b5..144572f17a83d 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -42,7 +42,7 @@ struct drm_printer;
>  	param(int, enable_dc, -1) \
>  	param(int, enable_fbc, -1) \
>  	param(int, enable_ppgtt, -1) \
> -	param(int, enable_psr, -1) \
> +	param(int, enable_psr, 0) \
>  	param(int, disable_power_well, -1) \
>  	param(int, enable_ips, 1) \
>  	param(int, invert_brightness, 0) \
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 1faa494e2bc91..d676d483d5cf1 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -551,7 +551,6 @@ parse_driver_features(struct drm_i915_private
> *dev_priv,
>  	 */
>  	if (!driver->drrs_enabled)
>  		dev_priv->vbt.drrs_type = DRRS_NOT_SUPPORTED;
> -	dev_priv->vbt.psr.enable = driver->psr_enabled;
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index b6838b525502e..26e7eb318cf07 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1065,13 +1065,6 @@ void intel_psr_init(struct drm_i915_private
> *dev_priv)
>  	if (!dev_priv->psr.sink_support)
>  		return;
>  
> -	if (i915_modparams.enable_psr == -1) {
> -		i915_modparams.enable_psr = dev_priv->vbt.psr.enable;
> -
> -		/* Per platform default: all disabled. */
> -		i915_modparams.enable_psr = 0;
> -	}
> -
>  	/* Set link_standby x link_off defaults */
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		/* HSW and BDW require workarounds that we don't
> implement. */

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/psr: simplify enable_psr handling
  2018-12-21 19:23   ` Dhinakaran Pandiyan
  (?)
@ 2018-12-21 19:53   ` Ross Zwisler
  2018-12-24  9:35     ` Daniel Vetter
  -1 siblings, 1 reply; 6+ messages in thread
From: Ross Zwisler @ 2018-12-21 19:53 UTC (permalink / raw)
  To: Dhinakaran Pandiyan
  Cc: linux-kernel, Daniel Vetter, David Airlie, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, dri-devel, intel-gfx, Jon Flatley

On Fri, Dec 21, 2018 at 11:23:07AM -0800, Dhinakaran Pandiyan wrote:
> On Fri, 2018-12-21 at 10:23 -0700, Ross Zwisler wrote:
> > The following commit:
> > 
> > commit 2bdd045e3a30 ("drm/i915/psr: Check if VBT says PSR can be
> > enabled.")
> > 
> > added some code with no usable functionality.  Regardless of how the
> > psr
> > default is set up in the BDB_DRIVER_FEATURES section, if the
> > enable_psr
> > module parameter isn't specified it defaults to 0.
> Right, that was intentional and the commit message even makes a note of
> it 
> " Note: The feature currently remains disabled by default for all
> platforms irrespective of what VBT says."
> 
> 
> Anyway, we've enabled the feature by default now and the current code
> should take into account the VBT flag if the module parameter is left
> to a default value. Please check git://anongit.freedesktop.org/drm-tip
> drm-tip.

Fair enough.  It's a bad pattern to introduce dead code as a placeholder for
some future work, though.  This code has been in the tree for three major
kernel releases (v4.{18,19,20}) without providing any useful functionality.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/psr: simplify enable_psr handling
  2018-12-21 19:53   ` Ross Zwisler
@ 2018-12-24  9:35     ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2018-12-24  9:35 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Dhinakaran Pandiyan, Linux Kernel Mailing List, David Airlie,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, dri-devel, intel-gfx,
	Jon Flatley

On Fri, Dec 21, 2018 at 8:53 PM Ross Zwisler <zwisler@google.com> wrote:
>
> On Fri, Dec 21, 2018 at 11:23:07AM -0800, Dhinakaran Pandiyan wrote:
> > On Fri, 2018-12-21 at 10:23 -0700, Ross Zwisler wrote:
> > > The following commit:
> > >
> > > commit 2bdd045e3a30 ("drm/i915/psr: Check if VBT says PSR can be
> > > enabled.")
> > >
> > > added some code with no usable functionality.  Regardless of how the
> > > psr
> > > default is set up in the BDB_DRIVER_FEATURES section, if the
> > > enable_psr
> > > module parameter isn't specified it defaults to 0.
> > Right, that was intentional and the commit message even makes a note of
> > it
> > " Note: The feature currently remains disabled by default for all
> > platforms irrespective of what VBT says."
> >
> >
> > Anyway, we've enabled the feature by default now and the current code
> > should take into account the VBT flag if the module parameter is left
> > to a default value. Please check git://anongit.freedesktop.org/drm-tip
> > drm-tip.
>
> Fair enough.  It's a bad pattern to introduce dead code as a placeholder for
> some future work, though.  This code has been in the tree for three major
> kernel releases (v4.{18,19,20}) without providing any useful functionality.

Getting PSR enabled by default on at least a few platforms took years.
Insisting that we do not ever merge such code also doesn't work.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/psr: simplify enable_psr handling
  2018-12-21 17:23 [PATCH] drm/i915/psr: simplify enable_psr handling Ross Zwisler
  2018-12-21 19:23   ` Dhinakaran Pandiyan
@ 2019-01-09 19:32 ` Patchwork
  1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2019-01-09 19:32 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr: simplify enable_psr handling
URL   : https://patchwork.freedesktop.org/series/54959/
State : failure

== Summary ==

Applying: drm/i915/psr: simplify enable_psr handling
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_drv.h
M	drivers/gpu/drm/i915/i915_params.c
M	drivers/gpu/drm/i915/i915_params.h
M	drivers/gpu/drm/i915/intel_bios.c
M	drivers/gpu/drm/i915/intel_psr.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_psr.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_psr.c
Auto-merging drivers/gpu/drm/i915/intel_bios.c
Auto-merging drivers/gpu/drm/i915/i915_params.h
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_params.h
Auto-merging drivers/gpu/drm/i915/i915_params.c
Auto-merging drivers/gpu/drm/i915/i915_drv.h
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915/psr: simplify enable_psr handling
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-09 19:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-21 17:23 [PATCH] drm/i915/psr: simplify enable_psr handling Ross Zwisler
2018-12-21 19:23 ` Dhinakaran Pandiyan
2018-12-21 19:23   ` Dhinakaran Pandiyan
2018-12-21 19:53   ` Ross Zwisler
2018-12-24  9:35     ` Daniel Vetter
2019-01-09 19:32 ` ✗ Fi.CI.BAT: failure for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.