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:
Reviewed-by: Ian Ray <ian....@ge.com>
<snip>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
