2014-12-25 8:33 GMT-02:00 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>: > On Tue, Dec 23, 2014 at 10:35:43AM -0200, Paulo Zanoni wrote: >> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> >> >> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
