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

Paulo Zanoni
2015-07-02T13:39:06Z

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?).

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre

--
Paulo Zanoni