Re: [PATCH 1/2] drm/i915: enable FBC on gen9+ too

Daniel Vetter
2016-12-27T15:08:06Z

On Fri, Dec 23, 2016 at 10:23:58AM -0200, Paulo Zanoni wrote:
> Gen9+ platforms have been seeing a lot of screen flickerings and
> underruns, so I never felt comfortable in enabling FBC on these
> platforms since I didn't want to throw yet another feature on top of
> the already complex problem. We now have code that automatically
> disables FBC if we ever get an underrun, and the screen flickerings
> seem to be mostly gone, so it may be a good time to try to finally
> enable FBC by default on the newer platforms.
>
> Besides, BDW FBC has been working fine over the year, which gives me a
> little more confidence now.
>
> For a little more information, please refer to commit a98ee79317b4
> ("drm/i915/fbc: enable FBC by default on HSW and BDW").
>
> v2: Enable not only on SKL, but for everything new (Daniel).
> v3: Rebase after the intel_sanitize_fbc_option() change.
> v4: New rebase after 8 months, drop expired R-B tags.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Daniel gave me a R-B for this patch maaaaany months ago, but I think
> we can consider it as expired now.
>
> QA just went through a round of testing and confirmed 100% pass rate
> for SKL/KBL/BXT (VIZ-7181).

For gen9+ still think we can move ahead:

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

> Besides, we do have at least one user with FBC enabled on SKL and
> reporting possible issues, as we saw on #98213, which is already
> fixed.
>
> The problem that I've been discussing with Chris and Daniel is not a
> user visible bug since it just keeps FBC deactivated if the client
> doesn't issue the dirtyfb calls, and from what I can see, current user
> space is fine.
>
> I have a pending patch for an improvement on the CRTC-choosing code,
> but that's not a bug fix and doesn't change how HSW+ behaves. It's
> patch 2 of this series.
>
> I think we can also consider reenabling FBC on HSW due to the
> confirmation I got some time ago that the HSW regression is gone (both
> bugs from c7f7e2feffb0 have confirmed that some patches that are
> alredy merged fixed the problem without reverting HSW support). But
> I won't propose this until we get to understand what's going on with
> bug #99169.

Hm, this is indeed wtf, but I guess first QA needs to deliver that bisect.
-Daniel

>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 9aec63b..26a81a9 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -1317,7 +1317,7 @@ static int intel_sanitize_fbc_option(struct drm_i915_private *dev_priv)
> if (!HAS_FBC(dev_priv))
> return 0;
>
> - if (IS_BROADWELL(dev_priv))
> + if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
> return 1;
>
> return 0;
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation