drm/i915: Enable FBC on GEN7 by default
Subsystems (2)
PATCH 1/1 drm/i915: Enable FBC on GEN7 by default
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
--- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/i915/intel_pm.c +1 -1
@@ -512,7 +512,7 @@ void intel_update_fbc(struct drm_device *dev) adjusted_mode = &intel_crtc->config.adjusted_mode;if (i915.enable_fbc <0 && -INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev)) { +INTEL_INFO (dev)->gen <=6 ) {if (set_no_fbc_reason (dev_priv,FBC_CHIP_DEFAULT ))DRM_DEBUG_KMS ("disabled per chip default\n" ); goto out_disable;
Signature · 1 line
1.9.1
Quoted 4 lines · Ben Widawsky
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
Did you try with a high resolution panel? I could never get IVB FBC to be completely stable with a 2560x1700 panel...
Stéphane
Quoted 26 lines
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
Cc Rodrigo Vivi <[email protected]>Signed-off-by Ben Widawsky <[email protected]>--- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/i915/intel_pm.c +1
-2@@ -512,7 +512,7 @@ void intel_update_fbc(struct drm_device *dev) adjusted_mode = &intel_crtc->config.adjusted_mode;if (i915.enable_fbc <0 && -INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev)) { +INTEL_INFO (dev)->gen <=6 ) {if (set_no_fbc_reason (dev_priv,FBC_CHIP_DEFAULT ))DRM_DEBUG_KMS ("disabled per chip default\n" ); goto out_disable; --1.9.1
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 11 lines · Stéphane Marchesin
Quoted 4 lines · Ben Widawsky
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
Did you try with a high resolution panel? I could never get IVB FBC to be completely stable with a 2560x1700 panel...
Stéphane
1600x900 is working fine. At my current laziness, that's the best I can test. I can try on a higher resolution tomorrow. Perhaps the solution would be to disable if the resolution goes above a certain point.
Honestly, I assumed there was a reason it was disabled, I just couldn't find it.
Quoted 35 lines
Quoted 27 lines
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
Cc Rodrigo Vivi <[email protected]>Signed-off-by Ben Widawsky <[email protected]>--- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/i915/intel_pm.c +1
-2@@ -512,7 +512,7 @@ void intel_update_fbc(struct drm_device *dev) adjusted_mode = &intel_crtc->config.adjusted_mode;if (i915.enable_fbc <0 && -INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev)) { +INTEL_INFO (dev)->gen <=6 ) {if (set_no_fbc_reason (dev_priv,FBC_CHIP_DEFAULT ))DRM_DEBUG_KMS ("disabled per chip default\n" ); goto out_disable; --1.9.1
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 5 lines
Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 11 lines · Ben Widawsky
Quoted 10 lines · Stéphane Marchesin
Quoted 4 lines · Ben Widawsky
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
Did you try with a high resolution panel? I could never get IVB FBC to be completely stable with a 2560x1700 panel...
Quoted 8 lines
Quoted 1 line
Stéphane
1600x900 is working fine. At my current laziness, that's the best I can test. I can try on a higher resolution tomorrow. Perhaps the solution would be to disable if the resolution goes above a certain point.
Honestly, I assumed there was a reason it was disabled, I just couldn't find it.
For what it's worth, it's fine for me up to 2k wide. Things seem to go bad above that.
Stéphane
Quoted 37 lines
Quoted 28 lines
Quoted 28 lines
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
Cc Rodrigo Vivi <[email protected]>Signed-off-by Ben Widawsky <[email protected]>--- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/i915/intel_pm.c +1
-2@@ -512,7 +512,7 @@ void intel_update_fbc(struct drm_device *dev) adjusted_mode = &intel_crtc->config.adjusted_mode;if (i915.enable_fbc <0 && -INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev)) { +INTEL_INFO (dev)->gen <=6 ) {if (set_no_fbc_reason (dev_priv,FBC_CHIP_DEFAULT ))DRM_DEBUG_KMS ("disabled per chip default\n" ); goto out_disable; --1.9.1
Quoted 8 lines
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 2 lines
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v2 1/1 drm/i915: Enable FBC on GEN7 by default
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
v2: Disable FBC if width is greater than 2k, and user doesn't override. This is due to issues seen by Stéphane, and possibly others. I wasn't sure what to do with max_height. v2 was only compile tested.
--- drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +9 -2
@@ -512,7 +512,7 @@ void intel_update_fbc(struct drm_device *dev) adjusted_mode = &intel_crtc->config.adjusted_mode;if (i915.enable_fbc <0 && -INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev)) { +INTEL_INFO (dev)->gen <=6 ) {if (set_no_fbc_reason (dev_priv,FBC_CHIP_DEFAULT ))DRM_DEBUG_KMS ("disabled per chip default\n" ); goto out_disable;
@@ -530,7 +530,14 @@ void intel_update_fbc(struct drm_device *dev) goto out_disable; } -if (IS_G4X (dev) ||INTEL_INFO (dev)->gen >=5 ) { +/* If the user hasn't overridden the defaults, try to get them working +* FBC on GEN7. If they have overridden the defaults, then assume they +* know what they're doing and allow foot shooting. +*/ +if (i915.enable_fbc <0 &&IS_GEN7 (dev) && !IS_HASWELL (dev)) { + max_width =2048 ; + max_height =-1 ; + }else if (IS_G4X (dev) ||INTEL_INFO (dev)->gen >=5 ) { max_width =4096 ; max_height =2048 ; }else {
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 7 lines · Ben Widawsky
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
Other than the major performance degredation due to our implementation, and that there is a known deadlock (when unplugging/plugging in external displays) due to the broken locking... It should not have been enabled. -Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Quoted 6 lines · Chris Wilson
Quoted 5 lines · Ben Widawsky
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
Quoted 6 lines
Quoted 2 lines
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
Other than the major performance degredation due to our implementation, and that there is a known deadlock (when unplugging/plugging in external displays) due to the broken locking... It should not have been enabled.
Also, have you run the full kms_fbc_crc testsuite to make sure it's actually functionally correct? Iirc we even fail at that stage still on some platforms ...
-Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Quoted 10 lines · Chris Wilson
Quoted 7 lines · Ben Widawsky
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
Other than the major performance degredation due to our implementation,
That sounds interesting, can you elaborate on the performance degradation? I haven't noticed any, but of course I don't know how it's supposed to behave...
Stéphane
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 7 lines · Stéphane Marchesin
Quoted 6 lines · Chris Wilson
Quoted 5 lines · Ben Widawsky
I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness.
Other than the major performance degredation due to our implementation,
That sounds interesting, can you elaborate on the performance degradation? I haven't noticed any, but of course I don't know how it's supposed to behave...
The way we setup the FBC is that it causes it to be invalidated after every operation (not just batch, or upon flushing the framebuffer). The impact of this is that lightweight 3D rendering operations such as firefox are about 60% slower, game impact though is less than 10% (more often in the noise), and all but the most GPU bound of BLT operations are orders of magnitude slower. All of this is mitigable by disabling FBC invalidations except when we write to the scanout.
https://bugs.freedesktop.org/show_bug.cgi?id=72023
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre