Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Tue, Mar 25, 2014 at 07:15:42PM -0700, Stéphane Marchesin wrote: > On Tue, Mar 25, 2014 at 12:27 AM, Chris Wilson > wrote: > > On Mon, Mar 24, 2014 at 06:21:22PM -0700, Ben Widawsky wrote: > >> 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... 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 -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Tue, Mar 25, 2014 at 12:27 AM, Chris Wilson wrote: > On Mon, Mar 24, 2014 at 06:21:22PM -0700, Ben Widawsky wrote: >> 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 Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Tue, Mar 25, 2014 at 8:27 AM, Chris Wilson wrote: > On Mon, Mar 24, 2014 at 06:21:22PM -0700, Ben Widawsky wrote: >> 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. 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Mon, Mar 24, 2014 at 06:21:22PM -0700, Ben Widawsky wrote: > 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 -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Mon, Mar 24, 2014 at 6:46 PM, Ben Widawsky wrote:
> On Mon, Mar 24, 2014 at 06:41:47PM -0700, Stéphane Marchesin wrote:
>> On Mon, Mar 24, 2014 at 6:21 PM, Ben Widawsky
>> wrote:
>> > 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.
For what it's worth, it's fine for me up to 2k wide. Things seem to go
bad above that.
Stéphane
>
>>
>> >
>> > 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
>> > Signed-off-by: Ben Widawsky
>> > ---
>> > drivers/gpu/drm/i915/intel_pm.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
>> > b/drivers/gpu/drm/i915/intel_pm.c
>> > index a7da962..a9890df 100644
>> > --- a/drivers/gpu/drm/i915/intel_pm.c
>> > +++ b/drivers/gpu/drm/i915/intel_pm.c
>> > @@ -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
>> >
>> > ___
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Mon, Mar 24, 2014 at 06:41:47PM -0700, Stéphane Marchesin wrote:
> On Mon, Mar 24, 2014 at 6:21 PM, Ben Widawsky
> wrote:
> > 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.
>
> >
> > 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
> > Signed-off-by: Ben Widawsky
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index a7da962..a9890df 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -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
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Mon, Mar 24, 2014 at 6:21 PM, Ben Widawsky
wrote:
> 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
>
> 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
> Signed-off-by: Ben Widawsky
> ---
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a7da962..a9890df 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -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
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] 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.
Cc: Rodrigo Vivi
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/intel_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7da962..a9890df 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
