[PATCH weston v9 06/62] compositor-drm: Refactor destroy drm_fb function

[Daniel Stone]

Hey Emil,

On 9 March 2017 at 23:51, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 3 March 2017 at 23:05, Daniel Stone <daniels at collabora.com> wrote:
>> +static void
>> +drm_fb_destroy_dumb(struct drm_fb *fb)
>> +{
>> +       struct drm_mode_destroy_dumb destroy_arg;
>> +
>> +       assert(fb->type == BUFFER_PIXMAN_DUMB);
>> +
>> +       if (fb->map && fb->size > 0)
>> +               munmap(fb->map, fb->size);
>> +
>> +       memset(&destroy_arg, 0, sizeof(destroy_arg));
>> +       destroy_arg.handle = fb->handle;
>> +       drmIoctl(fb->fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_arg);
>> +
>> +       drm_fb_destroy(fb);
>
> Things looks wrong here. Afaict we should unmap, rmfb ('detach' for
> dumb) and then dumb_destroy.
> See the drm_fb_create_dumb() error path - perhaps we can drop that in
> favour of the updated drm_fb_destroy_dumb() ?

Honestly, I don't think it makes any practical difference. The dumb
buffer itself has a reference held by the fb, so won't actually get
destroyed until both the buffer has been destroyed and the FB has been
as well. When we unmap also makes no difference, since we're not
touching the memory anymore. Probably a fair point about reusing this
from the error unwind.

> Not sure where exactly weston_buffer_reference() should be though :-\

It doesn't make a difference; that's only relevant to clients really.
So, anywhere. :)

Cheers,
Daniel