Re: [PATCH 2/7] drm/i915: move FBC code out of i915_gem_stolen.c

From: Daniel Vetter <daniel@ffwll.ch>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 2/7] drm/i915: move FBC code out of i915_gem_stolen.c
Date: Mon, 6 Jul 2015 10:44:11 +0200	[thread overview]
Message-ID: <20150706084411.GJ2156@phenom.ffwll.local> (raw)
In-Reply-To: <CA+gsUGRZszK1O4Or6KogJm+jgOFq976FAcLrnzh-hYke6xRRXQ@mail.gmail.com>

On Thu, Jul 02, 2015 at 10:39:05AM -0300, Paulo Zanoni wrote:
> 2015-07-01 17:44 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> > On Wed, Jul 01, 2015 at 05:15:21PM -0300, Paulo Zanoni wrote:
> >
> > Looks much cleaner with the split.
> >
> >> +void intel_fbc_cleanup_cfb(struct drm_device *dev)
> >> +{
> >> +     struct drm_i915_private *dev_priv = dev->dev_private;
> >> +
> >> +     if (dev_priv->fbc.uncompressed_size == 0)
> >> +             return;
> >> +
> >> +     i915_gem_stolen_remove_node(&dev_priv->fbc.compressed_fb);
> >> +
> >> +     if (dev_priv->fbc.compressed_llb) {
> >> +             i915_gem_stolen_remove_node(dev_priv->fbc.compressed_llb);
> >> +             kfree(dev_priv->fbc.compressed_llb);
> >> +     }
> >
> > Any reason why one node is embedded and the other allocated? Just feels
> > a little inconsistent, so lacks an explanation. Just that one is always
> > used, and the other on rare gen would probably suffice.
> 
> I really didn't stop to pay attention to the ancient FBC pieces. IMHO
> reasoning/explanation/change about this should be on a separate patch,
> since this one is just moving the code around.
> 
> I only think about the gen2-4 FBC code when I remember it has the
> "disable FBC when more than one pipe is visible" restriction which I
> can't even find on the documentation I have. I wish we could either
> remove it or just remove the whole gen2-4 FBC support (will we ever
> have the courage to enable it by default?).

Yeah I think killing gen2-4 fbc would be ok. Same for g4x fbc (hw too
broken) and ilk fbc (same really according to Art). Then we'd only need to
deal with fbc on gen6+, which is reasonably sane and consistent.

But we can rip the code out whenever you want, no hurry.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx