[PATCH weston v10 17/61] compositor-drm: Refactor sprite create/destroy into helpers

[Daniel Stone]

Hi,

On 7 April 2017 at 15:36, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Tue,  4 Apr 2017 17:54:35 +0100
> Daniel Stone <daniels at collabora.com> wrote:
>> +/**
>> + * Destroy one DRM plane
>> + *
>> + * Destroy a DRM plane, removing it from screen and releasing its retained
>> + * buffers in the process. The counterpart to drm_plane_create.
>> + *
>> + * @param plane Plane to deallocate (will be freed)
>> + */
>> +static void
>> +drm_plane_destroy(struct drm_plane *plane)
>> +{
>> +     drmModeSetPlane(plane->backend->drm.fd, plane->plane_id, 0, 0, 0,
>
> Previously the code was doing a nasty hack to get a valid CRTC id to
> use instead of zero here. Was that just a brainfart, or has something
> changed in the kernel?

It doesn't make a difference. After a plane has an FB ID of 0 set, it
is no longer associated with any CRTC. I avoided the gymnastics
because there didn't seem much point in doing so. Should I make a note
in the commit message, or a comment, or ... ?

>> -     wl_list_for_each_safe(plane, next, &backend->sprite_list, link) {
>> -             drmModeSetPlane(backend->drm.fd,
>> -                             plane->plane_id,
>> -                             output->crtc_id, 0, 0,
>> -                             0, 0, 0, 0, 0, 0, 0, 0);
>> -             assert(!plane->fb_last);
>> -             assert(!plane->fb_pending);
>> -             drm_fb_unref(plane->fb_current);
>> -             weston_plane_release(&plane->base);
>> -             free(plane);
>
> There was no wl_list_remove()... *facepalm*
>
> Nice to fix that, even though it's yet another change that is not
> strictly refactoring.

True. :\

Cheers,
Daniel