[Intel-gfx] [PATCH 7/9] drm/i915: don't alloc/free fbc_work at every update

[Paulo Zanoni]

2014-12-25 8:33 GMT-02:00 Chris Wilson <chris at chris-wilson.co.uk>:
> On Tue, Dec 23, 2014 at 10:35:43AM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> Because there's no need for it. Just use a static structure with a
>> bool field to tell if it's in use or not. The big advantage here is
>> not saving kzalloc/kfree calls, it's cutting the ugly "failed to
>> allocate FBC work structure" code path: in this path we call
>> enable_fbc() directly but we don't update fbc.crtc, fbc.fb_id and
>> fbc.y - they are updated in intel_fbc_work_fn(), which we're not
>> calling. And since testing out-of-memory cases like this is really
>> hard, getting rid of the code path is a major relief.
>
> No, it is not that hard to test.

Yeah, I know we have tons of checks for out-of-memory stuff, but it
takes time to test and I think we just don't have the FBC-specific
test.

>
> The complaint here should be addressed by a function to call
> dev_priv->display.enable_fbc, which would do the common task of setting
> dev_priv->fbc.crtc, .fb_id, .y and *.enabled*.

Ok, I'll do that. I thought that by keeping a single code path we'd be
able to avoid it :)

>
> That would remove duplicated code first. And then you can argue about
> the merits of replacing the kmalloc by growing our global state.

And what is your opinion on this? Do you think it's an improvement to
the codebase?

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



-- 
Paulo Zanoni