On Thu, Jul 02, 2015 at 10:39:05AM -0300, Paulo Zanoni wrote: > 2015-07-01 17:44 GMT-03:00 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>: > > 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx