On Tue, 26 Sep 2017 06:24:56 +0000 "Ray, Ian (GE Healthcare)" <ian....@ge.com> wrote:
> On 22/09/2017, 17.33, "wayland-devel on behalf of Pekka Paalanen" > <wayland-devel-boun...@lists.freedesktop.org on behalf of > ppaala...@gmail.com> wrote: > > > > From: Pekka Paalanen <pekka.paala...@collabora.co.uk> > > > > Rename fbdev_frame_buffer_destroy() to fbdev_frame_buffer_unmap() > > because that is what it does. Adding the destruction of hw_surface in it > > makes it the perfect counterpart to fbdev_frame_buffer_map() which > > simplifies the code. > > > > fbdev_frame_buffer_map() can no longer call that, so just open-code the > > munmap() there. It is an error path, we don't really care about > > failures in an error path. > > > > The error path of fbdev_output_enable() is converted to call > > buffer_unmap() since that is exactly what it did. > > > > fbdev_output_disable() became redundant, being identical to > > fbdev_frame_buffer_unmap(). > > > > Invariant: output->hw_surface cannot be non-NULL without output->fb > > being non-NULL. hw_surface wraps the mmapped memory so cannot exist > > without the mmap. > > > > Signed-off-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> > > --- > > libweston/compositor-fbdev.c | 52 > > ++++++++++++++++++-------------------------- > > 1 file changed, 21 insertions(+), 31 deletions(-) > > > > diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c > > index 402648d0..22100f98 100644 > > --- a/libweston/compositor-fbdev.c > > +++ b/libweston/compositor-fbdev.c > > @@ -40,6 +40,7 @@ > > #include <unistd.h> > > #include <linux/fb.h> > > #include <linux/input.h> > > +#include <assert.h> > > > > #include <libudev.h> > > > > @@ -335,8 +336,6 @@ fbdev_set_screen_info(int fd, struct fbdev_screeninfo > > *info) > > return 1; > > } > > > > -static void fbdev_frame_buffer_destroy(struct fbdev_output *output); > > - > > /* Returns an FD for the frame buffer device. */ > > static int > > fbdev_frame_buffer_open(const char *fb_dev, > > @@ -401,8 +400,10 @@ fbdev_frame_buffer_map(struct fbdev_output *output, > > int fd) > > retval = 0; > > > > out_unmap: > > - if (retval != 0 && output->fb != NULL) > > - fbdev_frame_buffer_destroy(output); > > + if (retval != 0 && output->fb != NULL) { > > + munmap(output->fb, output->fb_info.buffer_length); > > + output->fb = NULL; > > + } > > > > out_close: > > if (fd >= 0) > > @@ -412,9 +413,18 @@ out_close: > > } > > > > static void > > -fbdev_frame_buffer_destroy(struct fbdev_output *output) > > +fbdev_frame_buffer_unmap(struct fbdev_output *output) > > { > > - weston_log("Destroying fbdev frame buffer.\n"); > > + if (!output->fb) { > > + assert(!output->hw_surface); > > + return; > > + } > > + > > + weston_log("Unmapping fbdev frame buffer.\n"); > > + > > + if (output->hw_surface) > > + pixman_image_unref(output->hw_surface); > > + output->hw_surface = NULL; > > nitpick: hw_surface need only be set NULL if it was non-NULL (which > seems to be the pattern elsewhere). But, in any case: Hi Ian, yes, that would be correct as well. I'm just emphasizing that hw_surface is NULL from here on. > Reviewed-by: Ian Ray <ian....@ge.com> Thanks, pq
pgpxSIi5D1qyK.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel