[PATCH] drm/i915/psr: Remove few mod parameters option.

[PATCH] drm/i915/psr: Remove few mod parameters option.

[Rodrigo Vivi]

Reduce the module parameter to enable or disable.

The link stand by vs full link off was used only once.

And it was actually masking another bug fixed by commit
'84bb2916a683 ("drm/i915/psr: Check for SET_POWER_CAPABLE
bit at PSR init time.")'

So, let's remove these options for now. End goal is to
fully remove the mod param, moving it to a debugfs
interface in upcoming patches.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Tarun Vyas <tarun.vyas@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c |  2 +-
 drivers/gpu/drm/i915/intel_psr.c   | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 49fcc4679db6..817576701ed7 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -92,7 +92,7 @@ i915_param_named_unsafe(enable_ppgtt, int, 0400,
 
 i915_param_named_unsafe(enable_psr, int, 0600,
 	"Enable PSR "
-	"(0=disabled, 1=enabled - link mode chosen per-platform, 2=force link-standby mode, 3=force link-off mode) "
+	"(0=disabled, 1=enabled) "
 	"Default: -1 (use per-chip default)");
 
 i915_param_named_unsafe(alpha_support, bool, 0400,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 23acc9ac8d4d..ea467e2798e2 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -966,16 +966,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 		/* For new platforms let's respect VBT back again */
 		dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
 
-	/* Override link_standby x link_off defaults */
-	if (i915_modparams.enable_psr == 2 && !dev_priv->psr.link_standby) {
-		DRM_DEBUG_KMS("PSR: Forcing link standby\n");
-		dev_priv->psr.link_standby = true;
-	}
-	if (i915_modparams.enable_psr == 3 && dev_priv->psr.link_standby) {
-		DRM_DEBUG_KMS("PSR: Forcing main link off\n");
-		dev_priv->psr.link_standby = false;
-	}
-
 	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
 	mutex_init(&dev_priv->psr.lock);
 }
-- 
2.17.1

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

✓ Fi.CI.BAT: success for drm/i915/psr: Remove few mod parameters option.

[Patchwork]

== Series Details ==

Series: drm/i915/psr: Remove few mod parameters option.
URL   : https://patchwork.freedesktop.org/series/46350/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4473 -> Patchwork_9620 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46350/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9620 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-skl-6700k2:      FAIL (fdo#103191) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191


== Participating hosts (46 -> 40) ==

  Additional (1): fi-skl-guc 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-elk-e7500 fi-kbl-7560u 


== Build changes ==

    * Linux: CI_DRM_4473 -> Patchwork_9620

  CI_DRM_4473: 17c95b123220c6896ac9cf99c78be4709da704d4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4549: e19fd5549e9cf603251704117fc64f4068be5016 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9620: 5a7072ee8db66529ec01153fc648575295f7d997 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5a7072ee8db6 drm/i915/psr: Remove few mod parameters option.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9620/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/psr: Remove few mod parameters option.

[Dhinakaran Pandiyan]

On Wed, 2018-07-11 at 22:27 -0700, Rodrigo Vivi wrote:
> Reduce the module parameter to enable or disable.
> 
> The link stand by vs full link off was used only once.
> 
> And it was actually masking another bug fixed by commit
> '84bb2916a683 ("drm/i915/psr: Check for SET_POWER_CAPABLE
> bit at PSR init time.")'
> 
> So, let's remove these options for now. End goal is to
> fully remove the mod param, moving it to a debugfs
> interface in upcoming patches.
Sounds like a good idea, but I have not had use for this module
parameter anyway. 
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

What do you think of using enable_psr=1 to force PSR1 and enable_psr=2
to force PSR2 on PSR2 capable panels?

> Cc: Tarun Vyas <tarun.vyas@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_params.c |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c   | 10 ----------
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 49fcc4679db6..817576701ed7 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -92,7 +92,7 @@ i915_param_named_unsafe(enable_ppgtt, int, 0400,
>  
>  i915_param_named_unsafe(enable_psr, int, 0600,
>  	"Enable PSR "
> -	"(0=disabled, 1=enabled - link mode chosen per-platform,
> 2=force link-standby mode, 3=force link-off mode) "
> +	"(0=disabled, 1=enabled) "
>  	"Default: -1 (use per-chip default)");
>  
>  i915_param_named_unsafe(alpha_support, bool, 0400,
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 23acc9ac8d4d..ea467e2798e2 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -966,16 +966,6 @@ void intel_psr_init(struct drm_i915_private
> *dev_priv)
>  		/* For new platforms let's respect VBT back again */
>  		dev_priv->psr.link_standby = dev_priv-
> >vbt.psr.full_link;
>  
> -	/* Override link_standby x link_off defaults */
> -	if (i915_modparams.enable_psr == 2 && !dev_priv-
> >psr.link_standby) {
> -		DRM_DEBUG_KMS("PSR: Forcing link standby\n");
> -		dev_priv->psr.link_standby = true;
> -	}
> -	if (i915_modparams.enable_psr == 3 && dev_priv-
> >psr.link_standby) {
> -		DRM_DEBUG_KMS("PSR: Forcing main link off\n");
> -		dev_priv->psr.link_standby = false;
> -	}
> -
>  	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
>  	mutex_init(&dev_priv->psr.lock);
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/psr: Remove few mod parameters option.

[Rodrigo Vivi]

On Wed, Jul 11, 2018 at 11:08:17PM -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2018-07-11 at 22:27 -0700, Rodrigo Vivi wrote:
> > Reduce the module parameter to enable or disable.
> > 
> > The link stand by vs full link off was used only once.
> > 
> > And it was actually masking another bug fixed by commit
> > '84bb2916a683 ("drm/i915/psr: Check for SET_POWER_CAPABLE
> > bit at PSR init time.")'
> > 
> > So, let's remove these options for now. End goal is to
> > fully remove the mod param, moving it to a debugfs
> > interface in upcoming patches.
> Sounds like a good idea, but I have not had use for this module
> parameter anyway. 
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Thanks

> 
> What do you think of using enable_psr=1 to force PSR1 and enable_psr=2
> to force PSR2 on PSR2 capable panels?

dam... you are messing up with my next patch that I
have here hehehe

my idea here is from the regular path we call intel_psr_enable
that would check platform defaults and vbt and call
_intel_psr_enable

and for debugfs I was creating a toggle function
1 - enable
0 - disable

and change _intel_psr_enable to check crtc active.

If we add psr2 selection to the picture now I have to think
better ideas on how to handle this flow...

------

hmmm

maybe change intel_psr_enable signature to receive the type

enum psr_type
{
DEFAULT = 0,
PSR = 1,
PSR2 = 2,
};

so on regular path we would call
intel_psr_enable(..., DEFAULT);

on debugfs
intel_psr_enable(..., val);

?!

Better and/or cleaner ideas?

Thanks,
Rodrigo.

> 
> > Cc: Tarun Vyas <tarun.vyas@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_params.c |  2 +-
> >  drivers/gpu/drm/i915/intel_psr.c   | 10 ----------
> >  2 files changed, 1 insertion(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_params.c
> > b/drivers/gpu/drm/i915/i915_params.c
> > index 49fcc4679db6..817576701ed7 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -92,7 +92,7 @@ i915_param_named_unsafe(enable_ppgtt, int, 0400,
> >  
> >  i915_param_named_unsafe(enable_psr, int, 0600,
> >  	"Enable PSR "
> > -	"(0=disabled, 1=enabled - link mode chosen per-platform,
> > 2=force link-standby mode, 3=force link-off mode) "
> > +	"(0=disabled, 1=enabled) "
> >  	"Default: -1 (use per-chip default)");
> >  
> >  i915_param_named_unsafe(alpha_support, bool, 0400,
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 23acc9ac8d4d..ea467e2798e2 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -966,16 +966,6 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> >  		/* For new platforms let's respect VBT back again */
> >  		dev_priv->psr.link_standby = dev_priv-
> > >vbt.psr.full_link;
> >  
> > -	/* Override link_standby x link_off defaults */
> > -	if (i915_modparams.enable_psr == 2 && !dev_priv-
> > >psr.link_standby) {
> > -		DRM_DEBUG_KMS("PSR: Forcing link standby\n");
> > -		dev_priv->psr.link_standby = true;
> > -	}
> > -	if (i915_modparams.enable_psr == 3 && dev_priv-
> > >psr.link_standby) {
> > -		DRM_DEBUG_KMS("PSR: Forcing main link off\n");
> > -		dev_priv->psr.link_standby = false;
> > -	}
> > -
> >  	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
> >  	mutex_init(&dev_priv->psr.lock);
> >  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/psr: Remove few mod parameters option.

[Rodrigo Vivi]

On Thu, Jul 12, 2018 at 12:09:01AM -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2018-07-11 at 23:17 -0700, Rodrigo Vivi wrote:
> > On Wed, Jul 11, 2018 at 11:08:17PM -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > On Wed, 2018-07-11 at 22:27 -0700, Rodrigo Vivi wrote:
> > > > 
> > > > Reduce the module parameter to enable or disable.
> > > > 
> > > > The link stand by vs full link off was used only once.
> > > > 
> > > > And it was actually masking another bug fixed by commit
> > > > '84bb2916a683 ("drm/i915/psr: Check for SET_POWER_CAPABLE
> > > > bit at PSR init time.")'
> > > > 
> > > > So, let's remove these options for now. End goal is to
> > > > fully remove the mod param, moving it to a debugfs
> > > > interface in upcoming patches.
> > > Sounds like a good idea, but I have not had use for this module
> > > parameter anyway. 
> > > > 
> > > > 
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Thanks
> > 
> > > 
> > > 
> > > What do you think of using enable_psr=1 to force PSR1 and
> > > enable_psr=2
> > > to force PSR2 on PSR2 capable panels?
> > dam... you are messing up with my next patch that I
> > have here hehehe
> > 
> > my idea here is from the regular path we call intel_psr_enable
> > that would check platform defaults and vbt and call
> > _intel_psr_enable
> > 
> > and for debugfs I was creating a toggle function
> > 1 - enable
> > 0 - disable
> > 
> > and change _intel_psr_enable to check crtc active.
> > 
> > If we add psr2 selection to the picture now I have to think
> > better ideas on how to handle this flow...
> > 
> > ------
> > 
> > hmmm
> > 
> > maybe change intel_psr_enable signature to receive the type
> > 
> > enum psr_type
> > {
> > DEFAULT = 0,
> > PSR = 1,
> > PSR2 = 2,
> > };
> > 
> Yeah, but I prefer the same mapping as module params - 0 for disable.
> 
> Maarten(CC'd) had a patch that did something similar (https://patchwork
> .freedesktop.org/patch/210792/). My concern at that time (and now too)
> was that our tests were broken and we shouldn't be adding more
> complexity to the driver. Having said that, I understand that toggling
> PSR via debugfs is something we need to do.

ops, I had forgotten about that. I'm sorry....

well, I thought what I had in mind would be slight simpler,
but specially because we are now removing these link options.
Or maybe because I forgot something and over simplify it.

So if Maarten is going to send a v2 I just drop my initial stashed changes
here, otherwise I can also put in a pastebin later. But up to Maarten.

And the only change on test cases would be the order, instead
setting up the parameter and doing the modeset, it would do the modeset
and then toggle the debugfs.

We will need it sooner or later.

and it is no more complicated than dropping sink_crc in favor of the mmio
reads ;)

Thanks,
Rodrigo.

> 
> 
> -DK
> 
> 
> > so on regular path we would call
> > intel_psr_enable(..., DEFAULT);
> > 
> > on debugfs
> > intel_psr_enable(..., val);
> > 
> > ?!
> > 
> > Better and/or cleaner ideas?
> > 
> > Thanks,
> > Rodrigo.
> > 
> > > 
> > > 
> > > > 
> > > > Cc: Tarun Vyas <tarun.vyas@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_params.c |  2 +-
> > > >  drivers/gpu/drm/i915/intel_psr.c   | 10 ----------
> > > >  2 files changed, 1 insertion(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_params.c
> > > > b/drivers/gpu/drm/i915/i915_params.c
> > > > index 49fcc4679db6..817576701ed7 100644
> > > > --- a/drivers/gpu/drm/i915/i915_params.c
> > > > +++ b/drivers/gpu/drm/i915/i915_params.c
> > > > @@ -92,7 +92,7 @@ i915_param_named_unsafe(enable_ppgtt, int,
> > > > 0400,
> > > >  
> > > >  i915_param_named_unsafe(enable_psr, int, 0600,
> > > >  	"Enable PSR "
> > > > -	"(0=disabled, 1=enabled - link mode chosen per-platform,
> > > > 2=force link-standby mode, 3=force link-off mode) "
> > > > +	"(0=disabled, 1=enabled) "
> > > >  	"Default: -1 (use per-chip default)");
> > > >  
> > > >  i915_param_named_unsafe(alpha_support, bool, 0400,
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 23acc9ac8d4d..ea467e2798e2 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -966,16 +966,6 @@ void intel_psr_init(struct drm_i915_private
> > > > *dev_priv)
> > > >  		/* For new platforms let's respect VBT back
> > > > again */
> > > >  		dev_priv->psr.link_standby = dev_priv-
> > > > > 
> > > > > vbt.psr.full_link;
> > > >  
> > > > -	/* Override link_standby x link_off defaults */
> > > > -	if (i915_modparams.enable_psr == 2 && !dev_priv-
> > > > > 
> > > > > psr.link_standby) {
> > > > -		DRM_DEBUG_KMS("PSR: Forcing link standby\n");
> > > > -		dev_priv->psr.link_standby = true;
> > > > -	}
> > > > -	if (i915_modparams.enable_psr == 3 && dev_priv-
> > > > > 
> > > > > psr.link_standby) {
> > > > -		DRM_DEBUG_KMS("PSR: Forcing main link off\n");
> > > > -		dev_priv->psr.link_standby = false;
> > > > -	}
> > > > -
> > > >  	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
> > > >  	mutex_init(&dev_priv->psr.lock);
> > > >  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/psr: Remove few mod parameters option.

[Dhinakaran Pandiyan]

On Wed, 2018-07-11 at 23:17 -0700, Rodrigo Vivi wrote:
> On Wed, Jul 11, 2018 at 11:08:17PM -0700, Dhinakaran Pandiyan wrote:
> > 
> > On Wed, 2018-07-11 at 22:27 -0700, Rodrigo Vivi wrote:
> > > 
> > > Reduce the module parameter to enable or disable.
> > > 
> > > The link stand by vs full link off was used only once.
> > > 
> > > And it was actually masking another bug fixed by commit
> > > '84bb2916a683 ("drm/i915/psr: Check for SET_POWER_CAPABLE
> > > bit at PSR init time.")'
> > > 
> > > So, let's remove these options for now. End goal is to
> > > fully remove the mod param, moving it to a debugfs
> > > interface in upcoming patches.
> > Sounds like a good idea, but I have not had use for this module
> > parameter anyway. 
> > > 
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Thanks
> 
> > 
> > 
> > What do you think of using enable_psr=1 to force PSR1 and
> > enable_psr=2
> > to force PSR2 on PSR2 capable panels?
> dam... you are messing up with my next patch that I
> have here hehehe
> 
> my idea here is from the regular path we call intel_psr_enable
> that would check platform defaults and vbt and call
> _intel_psr_enable
> 
> and for debugfs I was creating a toggle function
> 1 - enable
> 0 - disable
> 
> and change _intel_psr_enable to check crtc active.
> 
> If we add psr2 selection to the picture now I have to think
> better ideas on how to handle this flow...
> 
> ------
> 
> hmmm
> 
> maybe change intel_psr_enable signature to receive the type
> 
> enum psr_type
> {
> DEFAULT = 0,
> PSR = 1,
> PSR2 = 2,
> };
> 
Yeah, but I prefer the same mapping as module params - 0 for disable.

Maarten(CC'd) had a patch that did something similar (https://patchwork
.freedesktop.org/patch/210792/). My concern at that time (and now too)
was that our tests were broken and we shouldn't be adding more
complexity to the driver. Having said that, I understand that toggling
PSR via debugfs is something we need to do.


-DK


> so on regular path we would call
> intel_psr_enable(..., DEFAULT);
> 
> on debugfs
> intel_psr_enable(..., val);
> 
> ?!
> 
> Better and/or cleaner ideas?
> 
> Thanks,
> Rodrigo.
> 
> > 
> > 
> > > 
> > > Cc: Tarun Vyas <tarun.vyas@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_params.c |  2 +-
> > >  drivers/gpu/drm/i915/intel_psr.c   | 10 ----------
> > >  2 files changed, 1 insertion(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_params.c
> > > b/drivers/gpu/drm/i915/i915_params.c
> > > index 49fcc4679db6..817576701ed7 100644
> > > --- a/drivers/gpu/drm/i915/i915_params.c
> > > +++ b/drivers/gpu/drm/i915/i915_params.c
> > > @@ -92,7 +92,7 @@ i915_param_named_unsafe(enable_ppgtt, int,
> > > 0400,
> > >  
> > >  i915_param_named_unsafe(enable_psr, int, 0600,
> > >  	"Enable PSR "
> > > -	"(0=disabled, 1=enabled - link mode chosen per-platform,
> > > 2=force link-standby mode, 3=force link-off mode) "
> > > +	"(0=disabled, 1=enabled) "
> > >  	"Default: -1 (use per-chip default)");
> > >  
> > >  i915_param_named_unsafe(alpha_support, bool, 0400,
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 23acc9ac8d4d..ea467e2798e2 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -966,16 +966,6 @@ void intel_psr_init(struct drm_i915_private
> > > *dev_priv)
> > >  		/* For new platforms let's respect VBT back
> > > again */
> > >  		dev_priv->psr.link_standby = dev_priv-
> > > > 
> > > > vbt.psr.full_link;
> > >  
> > > -	/* Override link_standby x link_off defaults */
> > > -	if (i915_modparams.enable_psr == 2 && !dev_priv-
> > > > 
> > > > psr.link_standby) {
> > > -		DRM_DEBUG_KMS("PSR: Forcing link standby\n");
> > > -		dev_priv->psr.link_standby = true;
> > > -	}
> > > -	if (i915_modparams.enable_psr == 3 && dev_priv-
> > > > 
> > > > psr.link_standby) {
> > > -		DRM_DEBUG_KMS("PSR: Forcing main link off\n");
> > > -		dev_priv->psr.link_standby = false;
> > > -	}
> > > -
> > >  	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
> > >  	mutex_init(&dev_priv->psr.lock);
> > >  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/psr: Remove few mod parameters option.

[Rodrigo Vivi]

On Wed, Jul 11, 2018 at 11:08:17PM -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2018-07-11 at 22:27 -0700, Rodrigo Vivi wrote:
> > Reduce the module parameter to enable or disable.
> > 
> > The link stand by vs full link off was used only once.
> > 
> > And it was actually masking another bug fixed by commit
> > '84bb2916a683 ("drm/i915/psr: Check for SET_POWER_CAPABLE
> > bit at PSR init time.")'
> > 
> > So, let's remove these options for now. End goal is to
> > fully remove the mod param, moving it to a debugfs
> > interface in upcoming patches.
> Sounds like a good idea, but I have not had use for this module
> parameter anyway. 
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

pushed, thanks

> 
> What do you think of using enable_psr=1 to force PSR1 and enable_psr=2
> to force PSR2 on PSR2 capable panels?
> 
> > Cc: Tarun Vyas <tarun.vyas@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_params.c |  2 +-
> >  drivers/gpu/drm/i915/intel_psr.c   | 10 ----------
> >  2 files changed, 1 insertion(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_params.c
> > b/drivers/gpu/drm/i915/i915_params.c
> > index 49fcc4679db6..817576701ed7 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -92,7 +92,7 @@ i915_param_named_unsafe(enable_ppgtt, int, 0400,
> >  
> >  i915_param_named_unsafe(enable_psr, int, 0600,
> >  	"Enable PSR "
> > -	"(0=disabled, 1=enabled - link mode chosen per-platform,
> > 2=force link-standby mode, 3=force link-off mode) "
> > +	"(0=disabled, 1=enabled) "
> >  	"Default: -1 (use per-chip default)");
> >  
> >  i915_param_named_unsafe(alpha_support, bool, 0400,
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 23acc9ac8d4d..ea467e2798e2 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -966,16 +966,6 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> >  		/* For new platforms let's respect VBT back again */
> >  		dev_priv->psr.link_standby = dev_priv-
> > >vbt.psr.full_link;
> >  
> > -	/* Override link_standby x link_off defaults */
> > -	if (i915_modparams.enable_psr == 2 && !dev_priv-
> > >psr.link_standby) {
> > -		DRM_DEBUG_KMS("PSR: Forcing link standby\n");
> > -		dev_priv->psr.link_standby = true;
> > -	}
> > -	if (i915_modparams.enable_psr == 3 && dev_priv-
> > >psr.link_standby) {
> > -		DRM_DEBUG_KMS("PSR: Forcing main link off\n");
> > -		dev_priv->psr.link_standby = false;
> > -	}
> > -
> >  	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
> >  	mutex_init(&dev_priv->psr.lock);
> >  }
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx