diff mbox

drm/i915/fbc: enable FBC by default on HSW and BDW

Message ID 1455655643-2535-1-git-send-email-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Feb. 16, 2016, 8:47 p.m. UTC
These platforms should be fine now.

FBC can allow very significant power savings for screen-on idle
systems, but it is worth mentioning that a lot of people won't get
significant power savings by enabling this feature because they may
have something else preventing the system from getting into the
deepest sleep states. Examples may include a hungry wifi device or a
max_performance SATA link power management policy. You can check your
PC state residencies on the powertop "Idle stats" tab. I recommend
trying to run "sudo powertop --auto-tune" and then seeing if the
residencies improve.

Oh, and in case you - the person reading this commit message - found
this commit through git bisect, please do the following:
 - Check your dmesg and see if there are error messages mentioning
   underruns around the time your problem started happening.
 - Download intel-gpu-tools, compile it, and run:
   $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | tee fbc.txt
   Then send us the fbc.txt file, especially if you get a failure.
   This will really maximize your chances of getting the bug fixed
   quickly.
 - Try to find a reliable way to reproduce the problem, and tell us.
 - Boot with drm.debug=0xe, reproduce the problem, then send us the
   dmesg file.

v2: Don't enable by default on SKL.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Feb. 17, 2016, 4:23 p.m. UTC | #1
On Tue, Feb 16, 2016 at 06:47:21PM -0200, Paulo Zanoni wrote:
> These platforms should be fine now.
> 
> FBC can allow very significant power savings for screen-on idle
> systems, but it is worth mentioning that a lot of people won't get
> significant power savings by enabling this feature because they may
> have something else preventing the system from getting into the
> deepest sleep states. Examples may include a hungry wifi device or a
> max_performance SATA link power management policy. You can check your
> PC state residencies on the powertop "Idle stats" tab. I recommend
> trying to run "sudo powertop --auto-tune" and then seeing if the
> residencies improve.
> 
> Oh, and in case you - the person reading this commit message - found
> this commit through git bisect, please do the following:
>  - Check your dmesg and see if there are error messages mentioning
>    underruns around the time your problem started happening.
>  - Download intel-gpu-tools, compile it, and run:
>    $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | tee fbc.txt
>    Then send us the fbc.txt file, especially if you get a failure.
>    This will really maximize your chances of getting the bug fixed
>    quickly.
>  - Try to find a reliable way to reproduce the problem, and tell us.
>  - Boot with drm.debug=0xe, reproduce the problem, then send us the
>    dmesg file.
> 
> v2: Don't enable by default on SKL.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 3614a95..0f0492f 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -823,13 +823,15 @@ static bool intel_fbc_can_choose(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>  	struct intel_fbc *fbc = &dev_priv->fbc;
> +	bool enable_by_default = IS_HASWELL(dev_priv) ||
> +				 IS_BROADWELL(dev_priv);
>  
>  	if (intel_vgpu_active(dev_priv->dev)) {
>  		fbc->no_fbc_reason = "VGPU is active";
>  		return false;
>  	}
>  
> -	if (i915.enable_fbc < 0) {
> +	if (i915.enable_fbc < 0 && !enable_by_default) {
>  		fbc->no_fbc_reason = "disabled per chip default";
>  		return false;
>  	}
> -- 
> 2.7.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Zanoni, Paulo R Feb. 19, 2016, 8:12 p.m. UTC | #2
Em Qua, 2016-02-17 às 08:40 +0000, Patchwork escreveu:
> == Summary ==

> 

> Series 3500v1 drm/i915/fbc: enable FBC by default on HSW and BDW

> http://patchwork.freedesktop.org/api/1.0/series/3500/revisions/1/mbox

> /

> 

> Test pm_rpm:

>         Subgroup basic-pci-d3-state:

>                 pass       -> DMESG-WARN (bsw-nuc-2)


https://bugs.freedesktop.org/show_bug.cgi?id=94163

I asked Daniel if this needed additional acks/reviews due to potential
controversy, but he said I can merge the patch.

Patch merged. Thanks for all the reviews leading to this :)

>         Subgroup basic-rte:

>                 dmesg-warn -> PASS       (bsw-nuc-2)

> 

> bsw-nuc-

> 2        total:165  pass:134  dwarn:2   dfail:0   fail:0   skip:29 

> byt-

> nuc          total:165  pass:140  dwarn:1   dfail:0   fail:0   skip:2

> 4 

> ilk-

> hp8440p      total:165  pass:116  dwarn:0   dfail:0   fail:1   skip:4

> 8 

> 

> Results at /archive/results/CI_IGT_test/Patchwork_1419/

> 

> bd0b1a9aa8b7fdb2e06a5cbf1756ef93de2fa3fd drm-intel-nightly: 2016y-

> 02m-16d-17h-53m-05s UTC integration manifest

> d6b61af1225d72afe8e72e8faf1140435266f8e7 drm/i915/fbc: enable FBC by

> default on HSW and BDW

> 

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 3614a95..0f0492f 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -823,13 +823,15 @@  static bool intel_fbc_can_choose(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 	struct intel_fbc *fbc = &dev_priv->fbc;
+	bool enable_by_default = IS_HASWELL(dev_priv) ||
+				 IS_BROADWELL(dev_priv);
 
 	if (intel_vgpu_active(dev_priv->dev)) {
 		fbc->no_fbc_reason = "VGPU is active";
 		return false;
 	}
 
-	if (i915.enable_fbc < 0) {
+	if (i915.enable_fbc < 0 && !enable_by_default) {
 		fbc->no_fbc_reason = "disabled per chip default";
 		return false;
 	}