[PATCH] drm/i915/psr: Enable PSR1 on gen-9+ HW

[PATCH] drm/i915/psr: Enable PSR1 on gen-9+ HW

[Dhinakaran Pandiyan]

We have new tests and fixes in place since the feature was last
disabled.

Try again for gen-9+ hardware and enable only PSR1 as a first step.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Jose Roberto de Souza <jose.souza@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
References: commit 2ee7dc497e34 ("drm/i915: disable PSR by default on HSW/BDW")
References: commit dcb2e993f3c0 ("Revert "drm/i915: Enable PSR by default on Valleyview and Cherryview."")
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b6838b525502..fc823f93a4dc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -71,6 +71,10 @@ 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;
+
 	switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
 	case I915_PSR_DEBUG_FORCE_PSR1:
 		return false;
@@ -1051,7 +1055,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
  * intel_psr_init - Init basic PSR work and mutex.
  * @dev_priv: i915 device private
  *
- * This function is  called only once at driver load to initialize basic
+ * This function is called only once at driver load to initialize basic
  * PSR stuff.
  */
 void intel_psr_init(struct drm_i915_private *dev_priv)
@@ -1065,19 +1069,14 @@ 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;
-	}
+	if (i915_modparams.enable_psr == -1)
+		if (INTEL_GEN(dev_priv) < 9 || !dev_priv->vbt.psr.enable)
+			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. */
 		dev_priv->psr.link_standby = false;
 	else
-		/* For new platforms let's respect VBT back again */
 		dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
 
 	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
-- 
2.17.1

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915/psr: Enable PSR1 on gen-9+ HW

[Patchwork]

== Series Details ==

Series: drm/i915/psr: Enable PSR1 on gen-9+ HW
URL   : https://patchwork.freedesktop.org/series/49312/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
0408bf5d3df3 drm/i915/psr: Enable PSR1 on gen-9+ HW
-:19: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#19: 
References: commit 2ee7dc497e34 ("drm/i915: disable PSR by default on HSW/BDW")

-:20: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit dcb2e993f3c0 ("Revert "drm/i915: Enable PSR by default on Valleyview and Cherryview."")'
#20: 
References: commit dcb2e993f3c0 ("Revert "drm/i915: Enable PSR by default on Valleyview and Cherryview."")

total: 1 errors, 1 warnings, 0 checks, 40 lines checked

_______________________________________________
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: Enable PSR1 on gen-9+ HW

[Patchwork]

== Series Details ==

Series: drm/i915/psr: Enable PSR1 on gen-9+ HW
URL   : https://patchwork.freedesktop.org/series/49312/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4782 -> Patchwork_10116 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-cfl-s3:          PASS -> DMESG-WARN (fdo#106350) +12

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    igt@pm_rpm@basic-rte:
      fi-kbl-r:           PASS -> DMESG-WARN (fdo#105602) +2

    
    ==== Possible fixes ====

    igt@amdgpu/amd_cs_nop@sync-fork-gfx0:
      fi-kbl-8809g:       DMESG-WARN (fdo#107762) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-kbl-7560u:       FAIL (fdo#107336) -> PASS

    
    ==== Warnings ====

    igt@amdgpu/amd_prime@amd-to-i915:
      fi-kbl-8809g:       DMESG-FAIL (fdo#107762) -> FAIL (fdo#107341)

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#106350 https://bugs.freedesktop.org/show_bug.cgi?id=106350
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762


== Participating hosts (52 -> 49) ==

  Additional (2): fi-byt-j1900 fi-gdg-551 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4782 -> Patchwork_10116

  CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10116: 0408bf5d3df3e526a55ae9b912d720ae2f144600 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0408bf5d3df3 drm/i915/psr: Enable PSR1 on gen-9+ HW

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915/psr: Enable PSR1 on gen-9+ HW

[Patchwork]

== Series Details ==

Series: drm/i915/psr: Enable PSR1 on gen-9+ HW
URL   : https://patchwork.freedesktop.org/series/49312/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4782_full -> Patchwork_10116_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@perf@blocking:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-snb:          INCOMPLETE (fdo#105411, fdo#106886) -> PASS
      shard-glk:          FAIL (fdo#106886) -> PASS

    igt@gem_eio@in-flight-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106702) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106702 https://bugs.freedesktop.org/show_bug.cgi?id=106702
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4782 -> Patchwork_10116

  CI_DRM_4782: 60edf94611d2374821fbe2a824cebcb425ce7b0d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4632: 94b4e204473a7d9f49e536c8877a4a5636e0d1b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10116: 0408bf5d3df3e526a55ae9b912d720ae2f144600 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915/psr: Enable PSR1 on gen-9+ HW

[Rodrigo Vivi]

On Thu, Sep 06, 2018 at 04:52:02PM -0700, Dhinakaran Pandiyan wrote:
> We have new tests and fixes in place since the feature was last
> disabled.
> 
> Try again for gen-9+ hardware and enable only PSR1 as a first step.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Jose Roberto de Souza <jose.souza@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> References: commit 2ee7dc497e34 ("drm/i915: disable PSR by default on HSW/BDW")
> References: commit dcb2e993f3c0 ("Revert "drm/i915: Enable PSR by default on Valleyview and Cherryview."")
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index b6838b525502..fc823f93a4dc 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -71,6 +71,10 @@ 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;
> +
>  	switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
>  	case I915_PSR_DEBUG_FORCE_PSR1:
>  		return false;
> @@ -1051,7 +1055,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>   * intel_psr_init - Init basic PSR work and mutex.
>   * @dev_priv: i915 device private
>   *
> - * This function is  called only once at driver load to initialize basic
> + * This function is called only once at driver load to initialize basic
>   * PSR stuff.
>   */
>  void intel_psr_init(struct drm_i915_private *dev_priv)
> @@ -1065,19 +1069,14 @@ 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;
> -	}
> +	if (i915_modparams.enable_psr == -1)
> +		if (INTEL_GEN(dev_priv) < 9 || !dev_priv->vbt.psr.enable)
> +			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. */
>  		dev_priv->psr.link_standby = false;
>  	else
> -		/* For new platforms let's respect VBT back again */

bikeshed: Can we please leave the clean-up for a separated patch?
In case we need to revert we don't loose the clean-up part! :$

Also a bikeshed of bikeshed: I think we need to revisit this block entirely
anyways. I can't remember why we stopped respecting the bspec here.
And probably this was only masking some issues that got fixed during
your great journey! ;)

Maybe this block even explain the current gap on hsw and bdw?! ;)

>  		dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
>  
>  	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/psr: Enable PSR1 on gen-9+ HW

[Dhinakaran Pandiyan]

On Thu, 2018-09-06 at 22:06 -0700, Rodrigo Vivi wrote:
> On Thu, Sep 06, 2018 at 04:52:02PM -0700, Dhinakaran Pandiyan wrote:
> > We have new tests and fixes in place since the feature was last
> > disabled.
> > 
> > Try again for gen-9+ hardware and enable only PSR1 as a first step.
> > 
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Jose Roberto de Souza <jose.souza@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > References: commit 2ee7dc497e34 ("drm/i915: disable PSR by default
> > on HSW/BDW")
> > References: commit dcb2e993f3c0 ("Revert "drm/i915: Enable PSR by
> > default on Valleyview and Cherryview."")
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index b6838b525502..fc823f93a4dc 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -71,6 +71,10 @@ 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;
> > +
> >  	switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
> >  	case I915_PSR_DEBUG_FORCE_PSR1:
> >  		return false;
> > @@ -1051,7 +1055,7 @@ void intel_psr_flush(struct drm_i915_private
> > *dev_priv,
> >   * intel_psr_init - Init basic PSR work and mutex.
> >   * @dev_priv: i915 device private
> >   *
> > - * This function is  called only once at driver load to initialize
> > basic
> > + * This function is called only once at driver load to initialize
> > basic
> >   * PSR stuff.
> >   */
> >  void intel_psr_init(struct drm_i915_private *dev_priv)
> > @@ -1065,19 +1069,14 @@ 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;
> > -	}
> > +	if (i915_modparams.enable_psr == -1)
> > +		if (INTEL_GEN(dev_priv) < 9 || !dev_priv-
> > >vbt.psr.enable)
> > +			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. */
> >  		dev_priv->psr.link_standby = false;
> >  	else
> > -		/* For new platforms let's respect VBT back again
> > */
> 
> bikeshed: Can we please leave the clean-up for a separated patch?
> In case we need to revert we don't loose the clean-up part! :$
> 
Okay.

> Also a bikeshed of bikeshed: I think we need to revisit this block
> entirely
> anyways. I can't remember why we stopped respecting the bspec here.
> And probably this was only masking some issues that got fixed during
> your great journey! ;)
> 
> Maybe this block even explain the current gap on hsw and bdw?! ;)
The gap is I haven't had time to investigate :)

> 
> >  		dev_priv->psr.link_standby = dev_priv-
> > >vbt.psr.full_link;
> >  
> >  	INIT_WORK(&dev_priv->psr.work, intel_psr_work);
> > -- 
> > 2.17.1
> > 
> 
> _______________________________________________
> 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

Re: [PATCH] drm/i915/psr: Enable PSR1 on gen-9+ HW

[Ville Syrjälä]

On Thu, Sep 06, 2018 at 10:06:09PM -0700, Rodrigo Vivi wrote:
> On Thu, Sep 06, 2018 at 04:52:02PM -0700, Dhinakaran Pandiyan wrote:
> > We have new tests and fixes in place since the feature was last
> > disabled.
> > 
> > Try again for gen-9+ hardware and enable only PSR1 as a first step.
> > 
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Jose Roberto de Souza <jose.souza@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > References: commit 2ee7dc497e34 ("drm/i915: disable PSR by default on HSW/BDW")
> > References: commit dcb2e993f3c0 ("Revert "drm/i915: Enable PSR by default on Valleyview and Cherryview."")
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index b6838b525502..fc823f93a4dc 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -71,6 +71,10 @@ 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;
> > +
> >  	switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
> >  	case I915_PSR_DEBUG_FORCE_PSR1:
> >  		return false;
> > @@ -1051,7 +1055,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> >   * intel_psr_init - Init basic PSR work and mutex.
> >   * @dev_priv: i915 device private
> >   *
> > - * This function is  called only once at driver load to initialize basic
> > + * This function is called only once at driver load to initialize basic
> >   * PSR stuff.
> >   */
> >  void intel_psr_init(struct drm_i915_private *dev_priv)
> > @@ -1065,19 +1069,14 @@ 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;
> > -	}
> > +	if (i915_modparams.enable_psr == -1)
> > +		if (INTEL_GEN(dev_priv) < 9 || !dev_priv->vbt.psr.enable)
> > +			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. */
> >  		dev_priv->psr.link_standby = false;
> >  	else
> > -		/* For new platforms let's respect VBT back again */
> 
> bikeshed: Can we please leave the clean-up for a separated patch?
> In case we need to revert we don't loose the clean-up part! :$
> 
> Also a bikeshed of bikeshed: I think we need to revisit this block entirely
> anyways. I can't remember why we stopped respecting the bspec here.
> And probably this was only masking some issues that got fixed during
> your great journey! ;)

Another vbt related thing was the aux handshake thing. We tried it here
https://patchwork.freedesktop.org/series/8046/ but IIRC it caused some
problems that no one had time to diagnose so we never merged that stuff.
Not sure if anyone wants to try and figure out what went wrong there.

Actually, after a bit more digging I guess the fails were listed here
https://lists.freedesktop.org/archives/intel-gfx/2016-June/097379.html
Some sink crc issues, but as that was deemed unusable anyway maybe
there was nothing wrong after all?

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

Re: [PATCH] drm/i915/psr: Enable PSR1 on gen-9+ HW

[Dhinakaran Pandiyan]

On Tue, 2018-09-11 at 17:04 +0300, Ville Syrjälä wrote:
> On Thu, Sep 06, 2018 at 10:06:09PM -0700, Rodrigo Vivi wrote:
> > On Thu, Sep 06, 2018 at 04:52:02PM -0700, Dhinakaran Pandiyan
> > wrote:
> > > We have new tests and fixes in place since the feature was last
> > > disabled.
> > > 
> > > Try again for gen-9+ hardware and enable only PSR1 as a first
> > > step.
> > > 
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Jose Roberto de Souza <jose.souza@intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > References: commit 2ee7dc497e34 ("drm/i915: disable PSR by
> > > default on HSW/BDW")
> > > References: commit dcb2e993f3c0 ("Revert "drm/i915: Enable PSR by
> > > default on Valleyview and Cherryview."")
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com
> > > >
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 17 ++++++++---------
> > >  1 file changed, 8 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index b6838b525502..fc823f93a4dc 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -71,6 +71,10 @@ 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;
> > > +
> > >  	switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK)
> > > {
> > >  	case I915_PSR_DEBUG_FORCE_PSR1:
> > >  		return false;
> > > @@ -1051,7 +1055,7 @@ void intel_psr_flush(struct
> > > drm_i915_private *dev_priv,
> > >   * intel_psr_init - Init basic PSR work and mutex.
> > >   * @dev_priv: i915 device private
> > >   *
> > > - * This function is  called only once at driver load to
> > > initialize basic
> > > + * This function is called only once at driver load to
> > > initialize basic
> > >   * PSR stuff.
> > >   */
> > >  void intel_psr_init(struct drm_i915_private *dev_priv)
> > > @@ -1065,19 +1069,14 @@ 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;
> > > -	}
> > > +	if (i915_modparams.enable_psr == -1)
> > > +		if (INTEL_GEN(dev_priv) < 9 || !dev_priv-
> > > >vbt.psr.enable)
> > > +			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. */
> > >  		dev_priv->psr.link_standby = false;
> > >  	else
> > > -		/* For new platforms let's respect VBT back
> > > again */
> > 
> > bikeshed: Can we please leave the clean-up for a separated patch?
> > In case we need to revert we don't loose the clean-up part! :$
> > 
> > Also a bikeshed of bikeshed: I think we need to revisit this block
> > entirely
> > anyways. I can't remember why we stopped respecting the bspec here.
> > And probably this was only masking some issues that got fixed
> > during
> > your great journey! ;)
> 
> Another vbt related thing was the aux handshake thing. We tried it
> here
> https://patchwork.freedesktop.org/series/8046/ but IIRC it caused
> some
> problems that no one had time to diagnose so we never merged that
> stuff.
> Not sure if anyone wants to try and figure out what went wrong there.
> 
I had to check with Art about this; we do need AUX handshake to wake up
the sink like the spec says. The current code is right.

> Actually, after a bit more digging I guess the fails were listed here
> https://lists.freedesktop.org/archives/intel-gfx/2016-June/097379.htm
> l
> Some sink crc issues, but as that was deemed unusable anyway maybe
> there was nothing wrong after all?
Sink crc issues should not been seen anymore.

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