[PATCH] drm/i915: disable PSR by default on HSW/BDW

[PATCH] drm/i915: disable PSR by default on HSW/BDW

[Paulo Zanoni]

We've been ignoring the poor bugzilla reporters that say PSR causes
system lockups and all other sorts of problems. The earliest bug
report is from April, so I think we can use the "revert the offending
commit if no fixes are presented within 8 months" rule here.

Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell and Broadwell.")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
Cc: <stable@vger.kernel.org> # v4.6+
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index d5f8d03..6aca8ff 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
 		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
 
-	/* Per platform default */
-	if (i915.enable_psr == -1) {
-		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-			i915.enable_psr = 1;
-		else
-			i915.enable_psr = 0;
-	}
+	/* Per platform default: all disabled. */
+	if (i915.enable_psr == -1)
+		i915.enable_psr = 0;
 
 	/* Set link_standby x link_off defaults */
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-- 
2.7.4

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

Re: [PATCH] drm/i915: disable PSR by default on HSW/BDW

[Vivi, Rodrigo]

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



On Tue, 2016-12-13 at 18:57 -0200, Paulo Zanoni wrote:
> We've been ignoring the poor bugzilla reporters that say PSR causes
> system lockups and all other sorts of problems. The earliest bug
> report is from April, so I think we can use the "revert the offending
> commit if no fixes are presented within 8 months" rule here.
> 
> Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell and Broadwell.")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
> Cc: <stable@vger.kernel.org> # v4.6+
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d5f8d03..6aca8ff 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
>  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
>  
> -	/* Per platform default */
> -	if (i915.enable_psr == -1) {
> -		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -			i915.enable_psr = 1;
> -		else
> -			i915.enable_psr = 0;
> -	}
> +	/* Per platform default: all disabled. */
> +	if (i915.enable_psr == -1)
> +		i915.enable_psr = 0;
>  
>  	/* Set link_standby x link_off defaults */
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))

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

✗ Fi.CI.BAT: failure for drm/i915: disable PSR by default on HSW/BDW

[Patchwork]

== Series Details ==

Series: drm/i915: disable PSR by default on HSW/BDW
URL   : https://patchwork.freedesktop.org/series/16759/
State : failure

== Summary ==

Series 16759v1 drm/i915: disable PSR by default on HSW/BDW
https://patchwork.freedesktop.org/api/1.0/series/16759/revisions/1/mbox/

Test drv_module_reload:
        Subgroup basic-reload-final:
                pass       -> FAIL       (fi-ilk-650)
        Subgroup basic-reload-inject:
                pass       -> DMESG-FAIL (fi-ilk-650)
Test gem_basic:
        Subgroup bad-close:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup create-close:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup create-fd-close:
                pass       -> SKIP       (fi-ilk-650)
Test gem_busy:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-hang-default:
                pass       -> SKIP       (fi-ilk-650)
Test gem_close_race:
        Subgroup basic-process:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-threads:
                pass       -> SKIP       (fi-ilk-650)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> SKIP       (fi-ilk-650)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> SKIP       (fi-ilk-650)
Test gem_exec_basic:
        Subgroup basic-bsd:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-render:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup gtt-bsd:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup gtt-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup gtt-render:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup readonly-bsd:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup readonly-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup readonly-render:
                pass       -> SKIP       (fi-ilk-650)
Test gem_exec_create:
        Subgroup basic:
                pass       -> SKIP       (fi-ilk-650)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-batch-kernel-default-wb:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-uc-pro-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-uc-prw-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-uc-ro-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-uc-rw-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-uc-set-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-wb-pro-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-wb-prw-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-wb-ro-before-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-wb-ro-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-wb-rw-before-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-wb-rw-default:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-wb-set-default:
                pass       -> SKIP       (fi-ilk-650)
Test gem_exec_gttfill:
        Subgroup basic:
                pass       -> SKIP       (fi-ilk-650)
Test gem_exec_nop:
        Subgroup basic-parallel:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-series:
                pass       -> SKIP       (fi-ilk-650)
Test gem_exec_parallel:
        Subgroup basic:
                pass       -> SKIP       (fi-ilk-650)
Test gem_exec_reloc:
        Subgroup basic-cpu:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-gtt:
                pass       -> SKIP       (fi-ilk-650)
        Subgroup basic-noreloc:
                pass       -> SKIP       (fi-ilk-650)
WARNING: Long output truncated

6b4321f148f37077cd8f81984c54f06e51141ebf drm-tip: 2016y-12m-13d-21h-47m-37s UTC integration manifest
151070c drm/i915: disable PSR by default on HSW/BDW

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3283/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable PSR by default on HSW/BDW

[Argotti, Yann]

> 
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> 
> On Tue, 2016-12-13 at 18:57 -0200, Paulo Zanoni wrote:
> > We've been ignoring the poor bugzilla reporters that say PSR causes
> > system lockups and all other sorts of problems. The earliest bug
> > report is from April, so I think we can use the "revert the offending
> > commit if no fixes are presented within 8 months" rule here.
> >
Would it means that, since we cannot fix and disable it by default then PSR is going to be deprecated ? I recall that on reported bugs we pay attention whether or not default parameter values is used or not to proceed on fix.
-Yann

> > Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell and
> > Broadwell.")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
> > Cc: <stable@vger.kernel.org> # v4.6+
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Jim Bride <jim.bride@linux.intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index d5f8d03..6aca8ff 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private
> *dev_priv)
> >  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
> >  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
> >
> > -	/* Per platform default */
> > -	if (i915.enable_psr == -1) {
> > -		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > -			i915.enable_psr = 1;
> > -		else
> > -			i915.enable_psr = 0;
> > -	}
> > +	/* Per platform default: all disabled. */
> > +	if (i915.enable_psr == -1)
> > +		i915.enable_psr = 0;
> >
> >  	/* Set link_standby x link_off defaults */
> >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> 
> _______________________________________________
> 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: disable PSR by default on HSW/BDW

[Paulo Zanoni]

Em Qua, 2016-12-14 às 08:00 +0000, Argotti, Yann escreveu:
> > 
> > 
> > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > 
> > 
> > On Tue, 2016-12-13 at 18:57 -0200, Paulo Zanoni wrote:
> > > 
> > > We've been ignoring the poor bugzilla reporters that say PSR
> > > causes
> > > system lockups and all other sorts of problems. The earliest bug
> > > report is from April, so I think we can use the "revert the
> > > offending
> > > commit if no fixes are presented within 8 months" rule here.
> > > 
> Would it means that, since we cannot fix and disable it by default
> then PSR is going to be deprecated ? I recall that on reported bugs
> we pay attention whether or not default parameter values is used or
> not to proceed on fix.

The plan should be to re-enable it at some point, but I'm not working
on that. What I did was just fix the regression so that users have
usable machines while we try to fix the feature in the appropriate way.
I'll let Rodrigo and Jim provide better details on their current plans
regarding PSR.

> -Yann
> 
> > 
> > > 
> > > Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell
> > > and
> > > Broadwell.")
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
> > > Cc: <stable@vger.kernel.org> # v4.6+
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Jim Bride <jim.bride@linux.intel.com>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
> > >  1 file changed, 3 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index d5f8d03..6aca8ff 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> > > 
> > >  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
> > >  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
> > > 
> > > -	/* Per platform default */
> > > -	if (i915.enable_psr == -1) {
> > > -		if (IS_HASWELL(dev_priv) ||
> > > IS_BROADWELL(dev_priv))
> > > -			i915.enable_psr = 1;
> > > -		else
> > > -			i915.enable_psr = 0;
> > > -	}
> > > +	/* Per platform default: all disabled. */
> > > +	if (i915.enable_psr == -1)
> > > +		i915.enable_psr = 0;
> > > 
> > >  	/* Set link_standby x link_off defaults */
> > >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > 
> > _______________________________________________
> > 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: [Intel-gfx] [PATCH] drm/i915: disable PSR by default on HSW/BDW

[Jani Nikula]

On Tue, 13 Dec 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> We've been ignoring the poor bugzilla reporters that say PSR causes
> system lockups and all other sorts of problems. The earliest bug
> report is from April, so I think we can use the "revert the offending
> commit if no fixes are presented within 8 months" rule here.

Ugh. Should be more like 2 weeks or so. We suck. :(

Acked-by: Jani Nikula <jani.nikula@intel.com>

PS. You're using a version of git that screws up # comments at the end
of Cc: lines, so I presume this didn't make it to stable list. It's
enough to have the Cc: in the commit when it gets applied though.

>
> Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell and Broadwell.")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
> Cc: <stable@vger.kernel.org> # v4.6+
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d5f8d03..6aca8ff 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
>  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
>  
> -	/* Per platform default */
> -	if (i915.enable_psr == -1) {
> -		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -			i915.enable_psr = 1;
> -		else
> -			i915.enable_psr = 0;
> -	}
> +	/* Per platform default: all disabled. */
> +	if (i915.enable_psr == -1)
> +		i915.enable_psr = 0;
>  
>  	/* Set link_standby x link_off defaults */
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915: disable PSR by default on HSW/BDW

[Jani Nikula]

On Tue, 13 Dec 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> We've been ignoring the poor bugzilla reporters that say PSR causes
> system lockups and all other sorts of problems. The earliest bug
> report is from April, so I think we can use the "revert the offending
> commit if no fixes are presented within 8 months" rule here.

Ugh. Should be more like 2 weeks or so. We suck. :(

Acked-by: Jani Nikula <jani.nikula@intel.com>

PS. You're using a version of git that screws up # comments at the end
of Cc: lines, so I presume this didn't make it to stable list. It's
enough to have the Cc: in the commit when it gets applied though.

>
> Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell and Broadwell.")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
> Cc: <stable@vger.kernel.org> # v4.6+
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d5f8d03..6aca8ff 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
>  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
>  
> -	/* Per platform default */
> -	if (i915.enable_psr == -1) {
> -		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -			i915.enable_psr = 1;
> -		else
> -			i915.enable_psr = 0;
> -	}
> +	/* Per platform default: all disabled. */
> +	if (i915.enable_psr == -1)
> +		i915.enable_psr = 0;
>  
>  	/* Set link_standby x link_off defaults */
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable PSR by default on HSW/BDW

[Vivi, Rodrigo]

On Wed, 2016-12-14 at 08:00 +0000, Argotti, Yann wrote:
> > 
> > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > 
> > 
> > On Tue, 2016-12-13 at 18:57 -0200, Paulo Zanoni wrote:
> > > We've been ignoring the poor bugzilla reporters that say PSR causes
> > > system lockups and all other sorts of problems. The earliest bug
> > > report is from April, so I think we can use the "revert the offending
> > > commit if no fixes are presented within 8 months" rule here.
> > >
> Would it means that, since we cannot fix and disable it by default then PSR is going to be deprecated ?

No, it is not getting deprecated.

But with bug list increasing let's do what is more safe for end users
while we continue working to get this in a enough stable level again.

>  I recall that on reported bugs we pay attention whether or not default parameter values is used or not to proceed on fix.

This is definitely a good approach to define the priorities of the bugs,
but not to close or ignore them completely. Here what happens is that
we end up ignoring the bugs even with parameter enabled so it is better
do disable it for now.

Thanks,
Rodrigo.

> -Yann
> 
> > > Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell and
> > > Broadwell.")
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
> > > Cc: <stable@vger.kernel.org> # v4.6+
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Jim Bride <jim.bride@linux.intel.com>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
> > >  1 file changed, 3 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index d5f8d03..6aca8ff 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> > >  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
> > >  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
> > >
> > > -	/* Per platform default */
> > > -	if (i915.enable_psr == -1) {
> > > -		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > > -			i915.enable_psr = 1;
> > > -		else
> > > -			i915.enable_psr = 0;
> > > -	}
> > > +	/* Per platform default: all disabled. */
> > > +	if (i915.enable_psr == -1)
> > > +		i915.enable_psr = 0;
> > >
> > >  	/* Set link_standby x link_off defaults */
> > >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > 
> > _______________________________________________
> > 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: disable PSR by default on HSW/BDW

[Argotti, Yann]

> 
> On Wed, 2016-12-14 at 08:00 +0000, Argotti, Yann wrote:
> > >
> > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > >
> > >
> > >
> > > On Tue, 2016-12-13 at 18:57 -0200, Paulo Zanoni wrote:
> > > > We've been ignoring the poor bugzilla reporters that say PSR
> > > > causes system lockups and all other sorts of problems. The
> > > > earliest bug report is from April, so I think we can use the
> > > > "revert the offending commit if no fixes are presented within 8
> months" rule here.
> > > >
> > Would it means that, since we cannot fix and disable it by default then
> PSR is going to be deprecated ?
> 
> No, it is not getting deprecated.
> 
> But with bug list increasing let's do what is more safe for end users
> while we continue working to get this in a enough stable level again.
> 
> >  I recall that on reported bugs we pay attention whether or not default
> parameter values is used or not to proceed on fix.
> 
> This is definitely a good approach to define the priorities of the bugs,
> but not to close or ignore them completely. Here what happens is that we
> end up ignoring the bugs even with parameter enabled so it is better do
> disable it for now.
> 
> Thanks,
> Rodrigo.
Thanks for the confirmation. I think this is fair approach. Let me know whenever some patches are available to re-test PSR
> 
> > -Yann
> >
> > > > Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell
> > > > and
> > > > Broadwell.")
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
> > > > Cc: <stable@vger.kernel.org> # v4.6+
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Jim Bride <jim.bride@linux.intel.com>
> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
> > > >  1 file changed, 3 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index d5f8d03..6aca8ff 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private
> > > *dev_priv)
> > > >  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
> > > >  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
> > > >
> > > > -	/* Per platform default */
> > > > -	if (i915.enable_psr == -1) {
> > > > -		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > > > -			i915.enable_psr = 1;
> > > > -		else
> > > > -			i915.enable_psr = 0;
> > > > -	}
> > > > +	/* Per platform default: all disabled. */
> > > > +	if (i915.enable_psr == -1)
> > > > +		i915.enable_psr = 0;
> > > >
> > > >  	/* Set link_standby x link_off defaults */
> > > >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > >
> > > _______________________________________________
> > > 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: disable PSR by default on HSW/BDW

[Paulo Zanoni]

Em Ter, 2016-12-13 às 21:22 +0000, Vivi, Rodrigo escreveu:
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 

Patch pushed. Thanks everybody.

I'll just go ahead and close the bugzilla tickets. Even though we want
to fix PSR for real and those bugs could be useful as a documentation
of what's wrong, I think it's better to just close them since they're
actually fixed now. We can still go back to them later since they're
documented in the git commit message.

If anybody disagrees, please feel free to reopen them.

> 
> On Tue, 2016-12-13 at 18:57 -0200, Paulo Zanoni wrote:
> > 
> > We've been ignoring the poor bugzilla reporters that say PSR causes
> > system lockups and all other sorts of problems. The earliest bug
> > report is from April, so I think we can use the "revert the
> > offending
> > commit if no fixes are presented within 8 months" rule here.
> > 
> > Fixes: 9b58e352b463 ("drm/i915: Enable PSR by default on Haswell
> > and Broadwell.")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97602
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97515
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96736
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96704
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96569
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94985
> > Cc: <stable@vger.kernel.org> # v4.6+
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Jim Bride <jim.bride@linux.intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index d5f8d03..6aca8ff 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -823,13 +823,9 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> >  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
> >  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
> >  
> > -	/* Per platform default */
> > -	if (i915.enable_psr == -1) {
> > -		if (IS_HASWELL(dev_priv) ||
> > IS_BROADWELL(dev_priv))
> > -			i915.enable_psr = 1;
> > -		else
> > -			i915.enable_psr = 0;
> > -	}
> > +	/* Per platform default: all disabled. */
> > +	if (i915.enable_psr == -1)
> > +		i915.enable_psr = 0;
> >  
> >  	/* Set link_standby x link_off defaults */
> >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx