[PATCH 2/2] compositor-drm: don't create and destroy kms fbs on every frame

[Ander Conselvan de Oliveira]

On 04/30/2012 08:06 PM, Kristian Hoegsberg wrote:
> 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.

Yeah, oops. I should have added a check here.

>  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;

That was my first thought too. The problem is that we need to know at 
the page_flip_handler() if the current bo is a client buffer to make a 
decision of calling gbm_bo_destroy() or gbm_surface_release_buffer() and 
we can't rely on output->scanout_buffer because it may have been destroyed.

I could have added a field current_is_client_buffer and 
next_is_client_buffer, but at that point I started thinking it would be 
better to do some refactoring of the code. The intention was to get this 
out of the way first and do the refactoring later.

I've done the refactoring now. Will send shortly.

Regards,
Ander