On Mon, 17 Apr 2017 20:54:21 +0100 Daniel Stone <[email protected]> wrote:
> Hi, > > On 7 April 2017 at 15:36, Pekka Paalanen <[email protected]> wrote: > > On Tue, 4 Apr 2017 17:54:35 +0100 > > Daniel Stone <[email protected]> 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 ... ? Hi Daniel, yes please, explain this in the commit message. It's very nice to know the gymnastics are not necessary. Makes me wonder why it was there in the first place. I've seen versions of weston that cannot shut down properly because they have the gymnastics and may fail to have a weston_output at all to get any CRTC id from, at which state it just abandons all stuff. > >> - 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. :\ Equally fixed by mentioning this in the commit message, and maybe not using "refactor" in the title. With these two, and the other trivial comments addressed: Reviewed-by: Pekka Paalanen <[email protected]> Thanks, pq
pgpyr5FYIAP6J.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
