[PATCH 0/1][SRU][D/E] UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/1][SRU][D/E] UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

Sultan Alsawaf
BugLink: https://bugs.launchpad.net/bugs/1849947

In Linux 5.0, PSR (Panel Self Refresh) was enabled by default on capable
hardware using Intel integrated graphics. On the few consumer laptops that do
have a panel with PSR support, enabling PSR causes frequent graphical glitches
that severely impact user experience. Dell laptops are the primary example; it
appears that no Thinkpads come with panels that support PSR, and it's unknown
how many other OEMs ship laptops with PSR, so let's just play it safe and
assume that PSR doesn't work for anyone.

[Impact]
PSR is an esoteric power-saving mechanism that will not be dearly missed when
disabled. The accompanying patch simply toggles PSR off by default in i915; it
can be re-enabled by the end user at the expense of tainting their kernel. The
power savings from enabling PSR are slim enough that I didn't observe a
difference in my laptop's reported power consumption between when I had it
enabled and disabled.

[Fix]
Set the enable_psr parameter in the i915 driver to 0 by default.

[Test Case]
I booted up a patched kernel on a laptop with PSR support and the debugfs node
that reports the status of PSR (/sys/kernel/debug/dri/0/i915_edp_psr_status)
said it was disabled. The graphical glitches were also gone.

[Regression Risk]
Low. This just restores the pre-5.0 behavior of keeping PSR disabled by default
on capable hardware.

Sultan Alsawaf (1):
  UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

 drivers/gpu/drm/i915/i915_params.c | 3 +--
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][SRU][D/E] UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

Sultan Alsawaf
BugLink: https://bugs.launchpad.net/bugs/1849947

On all Dell laptops with screens and chipsets that support PSR, both
PSR1 and PSR2 cause flickering and graphical glitches. Many laptops
don't support PSR so it isn't known if PSR works correctly on any
consumer hardware. PSR was enabled by default in 5.0 for capable
hardware, so this patch just restores the previous functionality of PSR
being disabled by default.

More info is available on the freedesktop bug:
https://gitlab.freedesktop.org/drm/intel/issues/425

Signed-off-by: Sultan Alsawaf <[hidden email]>
---
 drivers/gpu/drm/i915/i915_params.c | 3 +--
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 5b07766a1c26..71953f4e019f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -84,8 +84,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0600,
 
 i915_param_named_unsafe(enable_psr, int, 0600,
  "Enable PSR "
- "(0=disabled, 1=enabled) "
- "Default: -1 (use per-chip default)");
+ "(-1=use per-chip default, 0=disabled [default], 1=enabled) ");
 
 i915_param_named_unsafe(force_probe, charp, 0400,
  "Force probe the driver for specified devices. "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index a4770ce46bd2..d0167c5f47ff 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -50,7 +50,7 @@ struct drm_printer;
  param(int, vbt_sdvo_panel_type, -1) \
  param(int, enable_dc, -1) \
  param(int, enable_fbc, -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) \
--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

CMNT[F needs this patch]: [PATCH 0/1][SRU][D/E] UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

Sultan Alsawaf
In reply to this post by Sultan Alsawaf
On Mon, Feb 10, 2020 at 05:57:05PM -0800, Sultan Alsawaf wrote:

> BugLink: https://bugs.launchpad.net/bugs/1849947
>
> In Linux 5.0, PSR (Panel Self Refresh) was enabled by default on capable
> hardware using Intel integrated graphics. On the few consumer laptops that do
> have a panel with PSR support, enabling PSR causes frequent graphical glitches
> that severely impact user experience. Dell laptops are the primary example; it
> appears that no Thinkpads come with panels that support PSR, and it's unknown
> how many other OEMs ship laptops with PSR, so let's just play it safe and
> assume that PSR doesn't work for anyone.
>
> [Impact]
> PSR is an esoteric power-saving mechanism that will not be dearly missed when
> disabled. The accompanying patch simply toggles PSR off by default in i915; it
> can be re-enabled by the end user at the expense of tainting their kernel. The
> power savings from enabling PSR are slim enough that I didn't observe a
> difference in my laptop's reported power consumption between when I had it
> enabled and disabled.
>
> [Fix]
> Set the enable_psr parameter in the i915 driver to 0 by default.
>
> [Test Case]
> I booted up a patched kernel on a laptop with PSR support and the debugfs node
> that reports the status of PSR (/sys/kernel/debug/dri/0/i915_edp_psr_status)
> said it was disabled. The graphical glitches were also gone.
>
> [Regression Risk]
> Low. This just restores the pre-5.0 behavior of keeping PSR disabled by default
> on capable hardware.
>
> Sultan Alsawaf (1):
>   UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms
>
>  drivers/gpu/drm/i915/i915_params.c | 3 +--
>  drivers/gpu/drm/i915/i915_params.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> --
> 2.20.1
>

Just remembered that this patch applies to Focal as well.

Sultan

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/1][SRU][D/E] UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

Andrea Righi
In reply to this post by Sultan Alsawaf
On Mon, Feb 10, 2020 at 05:57:05PM -0800, Sultan Alsawaf wrote:

> BugLink: https://bugs.launchpad.net/bugs/1849947
>
> In Linux 5.0, PSR (Panel Self Refresh) was enabled by default on capable
> hardware using Intel integrated graphics. On the few consumer laptops that do
> have a panel with PSR support, enabling PSR causes frequent graphical glitches
> that severely impact user experience. Dell laptops are the primary example; it
> appears that no Thinkpads come with panels that support PSR, and it's unknown
> how many other OEMs ship laptops with PSR, so let's just play it safe and
> assume that PSR doesn't work for anyone.
>
> [Impact]
> PSR is an esoteric power-saving mechanism that will not be dearly missed when
> disabled. The accompanying patch simply toggles PSR off by default in i915; it
> can be re-enabled by the end user at the expense of tainting their kernel. The
> power savings from enabling PSR are slim enough that I didn't observe a
> difference in my laptop's reported power consumption between when I had it
> enabled and disabled.
>
> [Fix]
> Set the enable_psr parameter in the i915 driver to 0 by default.
>
> [Test Case]
> I booted up a patched kernel on a laptop with PSR support and the debugfs node
> that reports the status of PSR (/sys/kernel/debug/dri/0/i915_edp_psr_status)
> said it was disabled. The graphical glitches were also gone.
>
> [Regression Risk]
> Low. This just restores the pre-5.0 behavior of keeping PSR disabled by default
> on capable hardware.
>
> Sultan Alsawaf (1):
>   UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms
>
>  drivers/gpu/drm/i915/i915_params.c | 3 +--
>  drivers/gpu/drm/i915/i915_params.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>

Considering that this change has been tested with positive result, the
impact is small and it also restores an old behavior:

Acked-by: Andrea Righi <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/1][SRU][D/E] UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

Kamal Mostafa-2
In reply to this post by Sultan Alsawaf
LGTM.

Acked-by: Kamal Mostafa <[hidden email]>

 -Kamal

On Mon, Feb 10, 2020 at 05:57:05PM -0800, Sultan Alsawaf wrote:

> BugLink: https://bugs.launchpad.net/bugs/1849947
>
> In Linux 5.0, PSR (Panel Self Refresh) was enabled by default on capable
> hardware using Intel integrated graphics. On the few consumer laptops that do
> have a panel with PSR support, enabling PSR causes frequent graphical glitches
> that severely impact user experience. Dell laptops are the primary example; it
> appears that no Thinkpads come with panels that support PSR, and it's unknown
> how many other OEMs ship laptops with PSR, so let's just play it safe and
> assume that PSR doesn't work for anyone.
>
> [Impact]
> PSR is an esoteric power-saving mechanism that will not be dearly missed when
> disabled. The accompanying patch simply toggles PSR off by default in i915; it
> can be re-enabled by the end user at the expense of tainting their kernel. The
> power savings from enabling PSR are slim enough that I didn't observe a
> difference in my laptop's reported power consumption between when I had it
> enabled and disabled.
>
> [Fix]
> Set the enable_psr parameter in the i915 driver to 0 by default.
>
> [Test Case]
> I booted up a patched kernel on a laptop with PSR support and the debugfs node
> that reports the status of PSR (/sys/kernel/debug/dri/0/i915_edp_psr_status)
> said it was disabled. The graphical glitches were also gone.
>
> [Regression Risk]
> Low. This just restores the pre-5.0 behavior of keeping PSR disabled by default
> on capable hardware.
>
> Sultan Alsawaf (1):
>   UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms
>
>  drivers/gpu/drm/i915/i915_params.c | 3 +--
>  drivers/gpu/drm/i915/i915_params.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> --
> 2.20.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED/cmnt: [PATCH 1/1][SRU][D/E] UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

Kleber Souza
In reply to this post by Sultan Alsawaf
On 11.02.20 02:57, Sultan Alsawaf wrote:

> BugLink: https://bugs.launchpad.net/bugs/1849947
>
> On all Dell laptops with screens and chipsets that support PSR, both
> PSR1 and PSR2 cause flickering and graphical glitches. Many laptops
> don't support PSR so it isn't known if PSR works correctly on any
> consumer hardware. PSR was enabled by default in 5.0 for capable
> hardware, so this patch just restores the previous functionality of PSR
> being disabled by default.
>
> More info is available on the freedesktop bug:
> https://gitlab.freedesktop.org/drm/intel/issues/425
>
> Signed-off-by: Sultan Alsawaf <[hidden email]>

Applied to disco/linux (with some fuzzing) and to eoan/linux.
I also fixed the bug nominations.

Thanks,
Kleber



> ---
>  drivers/gpu/drm/i915/i915_params.c | 3 +--
>  drivers/gpu/drm/i915/i915_params.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 5b07766a1c26..71953f4e019f 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -84,8 +84,7 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0600,
>  
>  i915_param_named_unsafe(enable_psr, int, 0600,
>   "Enable PSR "
> - "(0=disabled, 1=enabled) "
> - "Default: -1 (use per-chip default)");
> + "(-1=use per-chip default, 0=disabled [default], 1=enabled) ");
>  
>  i915_param_named_unsafe(force_probe, charp, 0400,
>   "Force probe the driver for specified devices. "
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index a4770ce46bd2..d0167c5f47ff 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -50,7 +50,7 @@ struct drm_printer;
>   param(int, vbt_sdvo_panel_type, -1) \
>   param(int, enable_dc, -1) \
>   param(int, enable_fbc, -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) \
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: Re: [PATCH 1/1][SRU][D/E] UBUNTU: SAUCE: drm/i915: Disable PSR by default on all platforms

Paolo Pisati-5
In reply to this post by Sultan Alsawaf
On Mon, Feb 10, 2020 at 05:57:06PM -0800, Sultan Alsawaf wrote:

> BugLink: https://bugs.launchpad.net/bugs/1849947
>
> On all Dell laptops with screens and chipsets that support PSR, both
> PSR1 and PSR2 cause flickering and graphical glitches. Many laptops
> don't support PSR so it isn't known if PSR works correctly on any
> consumer hardware. PSR was enabled by default in 5.0 for capable
> hardware, so this patch just restores the previous functionality of PSR
> being disabled by default.
>
> More info is available on the freedesktop bug:
> https://gitlab.freedesktop.org/drm/intel/issues/425
>
> Signed-off-by: Sultan Alsawaf <[hidden email]>

Applied to Focal/linux.
--
bye,
p.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team