On Mon, Apr 30, 2012 at 01:31:29PM +0300, Ander Conselvan de Oliveira wrote:
> Use the new gbm_get/set_user_data() to reuse the kms fbs if the gbm
> surface's bo's are reused.
This doesn't look right. drm_output_render() always allocates and
sets user data, which overwrite previously set user data. That leaks
the KMS FB and we have to create a new one every frame. Also, caching
the KMS FB in user data is useful for gbm bos we get from
drm_output_prepare_scanout_surface() as well. An application may be
flipping between the same two buffers, and in that case we'll want to
reuse the KMS FBs as well.
Instead, I was thinking that we could just move the drmModeAddFB call
into a get_fb_for_bo() helper function, which would look something like this:
data = get_user_data(bo);
if (!data)
return data->id;
data = malloc(sizeof *data);
drmModeAddFB(..., &data->id);
set_user_data(bo, data);
return data->id;
Kristian
> ---
> src/compositor-drm.c | 94 +++++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 75 insertions(+), 19 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index e7433f7..e9b556c 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -199,12 +199,51 @@ drm_output_prepare_scanout_surface(struct drm_output
> *output)
> return 0;
> }
>
> +struct bo_user_data {
> + uint32_t fb_id;
> +};
> +
> +static void
> +bo_set_fb_id(struct gbm_bo *bo, uint32_t fb_id)
> +{
> + struct bo_user_data *user_data= gbm_bo_get_user_data(bo);
> +
> + if (!user_data)
> + return;
> +
> + user_data->fb_id = fb_id;
> +}
> +
> +static uint32_t
> +bo_get_fb_id(struct gbm_bo *bo)
> +{
> + struct bo_user_data *user_data= gbm_bo_get_user_data(bo);
> +
> + if (user_data)
> + return user_data->fb_id;
> + else
> + return 0;
> +}
> +
> +static void
> +bo_free_user_data(struct gbm_bo *bo, void *data)
> +{
> + struct gbm_device *gbm = gbm_bo_get_device(bo);
> + struct bo_user_data *user_data = data;
> +
> + if (user_data->fb_id)
> + drmModeRmFB(gbm_device_get_fd(gbm), user_data->fb_id);
> +
> + free(data);
> +}
> +
> static void
> drm_output_render(struct drm_output *output, pixman_region32_t *damage)
> {
> struct drm_compositor *compositor =
> (struct drm_compositor *) output->base.compositor;
> struct weston_surface *surface;
> + struct bo_user_data *data;
>
> if (!eglMakeCurrent(compositor->base.display, output->egl_surface,
> output->egl_surface, compositor->base.context)) {
> @@ -223,6 +262,15 @@ drm_output_render(struct drm_output *output,
> pixman_region32_t *damage)
> fprintf(stderr, "failed to lock front buffer: %m\n");
> return;
> }
> +
> + data = calloc(1, sizeof *data);
> + if (!data) {
> + gbm_surface_release_buffer(output->surface, output->next_bo);
> + output->next_bo = NULL;
> + return;
> + }
> +
> + gbm_bo_set_user_data(output->next_bo, data, bo_free_user_data);
> }
>
> static void
> @@ -241,6 +289,8 @@ drm_output_repaint(struct weston_output *output_base,
> if (!output->next_bo)
> drm_output_render(output, damage);
>
> + output->next_fb_id = bo_get_fb_id(output->next_bo);
> +
> stride = gbm_bo_get_pitch(output->next_bo);
> handle = gbm_bo_get_handle(output->next_bo).u32;
>
> @@ -252,15 +302,20 @@ drm_output_repaint(struct weston_output *output_base,
> output->next_bo = NULL;
> }
>
> - ret = drmModeAddFB(compositor->drm.fd,
> - output->base.current->width,
> - output->base.current->height,
> - 24, 32, stride, handle, &output->next_fb_id);
> - if (ret) {
> - fprintf(stderr, "failed to create kms fb: %m\n");
> - gbm_surface_release_buffer(output->surface, output->next_bo);
> - output->next_bo = NULL;
> - return;
> + if (!output->next_fb_id) {
> + ret = drmModeAddFB(compositor->drm.fd,
> + output->base.current->width,
> + output->base.current->height,
> + 24, 32, stride, handle, &output->next_fb_id);
> + if (ret) {
> + fprintf(stderr, "failed to create kms fb: %m\n");
> + gbm_surface_release_buffer(output->surface,
> output->next_bo);
> + output->next_bo = NULL;
> + return;
> + }
> +
> + if (output->next_bo)
> + bo_set_fb_id(output->next_bo, output->next_fb_id);
> }
>
> mode = container_of(output->base.current, struct drm_mode, base);
> @@ -355,22 +410,23 @@ page_flip_handler(int fd, unsigned int frame,
> (struct drm_compositor *) output->base.compositor;
> uint32_t msecs;
>
> - if (output->current_fb_id)
> - drmModeRmFB(c->drm.fd, output->current_fb_id);
> - output->current_fb_id = output->next_fb_id;
> - output->next_fb_id = 0;
> -
> - if (output->scanout_buffer) {
> - weston_buffer_post_release(output->scanout_buffer);
> - wl_list_remove(&output->scanout_buffer_destroy_listener.link);
> - output->scanout_buffer = NULL;
> - } else if (output->current_bo) {
> + if (output->current_bo) {
> gbm_surface_release_buffer(output->surface,
> output->current_bo);
> + } else {
> + if (output->scanout_buffer) {
> + weston_buffer_post_release(output->scanout_buffer);
> +
> wl_list_remove(&output->scanout_buffer_destroy_listener.link);
> + output->scanout_buffer = NULL;
> + }
> + if (output->current_fb_id)
> + drmModeRmFB(c->drm.fd, output->current_fb_id);
> }
>
> output->current_bo = output->next_bo;
> output->next_bo = NULL;
> + output->current_fb_id = output->next_fb_id;
> + output->next_fb_id = 0;
>
> if (output->pending_scanout_buffer) {
> output->scanout_buffer = output->pending_scanout_buffer;
> --
> 1.7.4.1
>
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
