On Tue, 21 Feb 2017 15:29:09 +0200 Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Fri, 9 Dec 2016 19:57:30 +0000 > Daniel Stone <dani...@collabora.com> wrote: > > > Rather than magically trying to infer what the buffer is and what we > > should do with it when we go to destroy it, add an explicit type > > instead. > > > > Differential Revision: https://phabricator.freedesktop.org/D1488 > > > > Signed-off-by: Daniel Stone <dani...@collabora.com> > > --- > > libweston/compositor-drm.c | 50 > > +++++++++++++++++++++++++++++----------------- > > 1 file changed, 32 insertions(+), 18 deletions(-) > > weston_buffer_reference(&fb->buffer_ref, buffer); > > } > > > > @@ -452,15 +461,19 @@ drm_output_release_fb(struct drm_output *output, > > struct drm_fb *fb) > > if (!fb) > > return; > > > > - if (fb->map && > > - (fb != output->dumb[0] && fb != output->dumb[1])) { > > - drm_fb_destroy_dumb(fb); > > This piece sent me into a recursive "well, actually..." loop. > > It looked like dead code at first hand, as this gets hit when > output->dumb and fb don't match. When would that be? I would guess when > video mode changed. > > I think changing resolutions would hit this path, when flipping to a > new dumb buffer of a different size than one coming out of scanout > which cannot be destroyed until pageflip completed. > > Except I am wrong in a couple of ways: destroying the buffer is fine, > the kernel will keep it referenced as long as necessary anyway. And, > drm_output_switch_mode() does destroy everything immediately. > > So this bit is ok. Unless there is a third well-actually. And there is it: < MrCooper> pq: IME KMS framebuffers aren't reference-counted in the kernel; destroying an fb which is still being scanned out by any CRTCs turns off those CRTCs Is it then so, that drm_output_switch_mode() is wrong to destroy the FB immediately before even flipping a new one, but as it does so, there is no leak introduced by removing the above drm_fb_destroy_dumb() call? I.e. the patch does not regress anything because of a bug elsewhere? If that is so, my R-b stands, as fixing switch_mode is a whole another matter. Also the refcounting introduced later in this series offer a much better basis for fixing switch_mode. Thanks, pq
pgprcatV6EflH.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
