[PATCH 2/3] compositor-drm: Add option to disable early buffer release

[Ander Conselvan de Oliveira]

On 11/19/2013 01:05 PM, Pekka Paalanen wrote:
> On Tue, 19 Nov 2013 11:30:11 +0200
> Ander Conselvan de Oliveira <conselvan2 at gmail.com> wrote:
>
>> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
>>
>> This together with a follow up patch should make it possible to do a
>> runtime renderer switch without causing artifact on the screen.
>> ---
>>   man/weston-drm.man   |    6 ++++++
>>   src/compositor-drm.c |   27 +++++++++++++++++----------
>>   src/compositor.c     |    1 +
>>   3 files changed, 24 insertions(+), 10 deletions(-)
>>
>> diff --git a/man/weston-drm.man b/man/weston-drm.man
>> index 35d62ae..d3d0b70 100644
>> --- a/man/weston-drm.man
>> +++ b/man/weston-drm.man
>> @@ -106,6 +106,12 @@ instead of the default seat
>>   Launch Weston on tty
>>   .I x
>>   instead of using the current tty.
>> +.TP
>> +.B \-\-disable\-early\-buffer\-release
>> +Don't release buffers early and instead keep references to them as
>> +long as any surface uses them, even if the compositor has an
>> +internal copy of the buffer contents. This causes some clients to
>> +allocate more memory than necessary.
>
> Hi Ander,
>
> I wonder, would it make more sense to call that --allow-renderer-switch
> instead?
>
> That way, if you define switching will work only once, then after the
> switch to the GL renderer you could enable the early release and get
> rid of the penalties from disabling it.

Yeah, that's what I've done in the previous series. But it just occurred 
to me that the pixman renderer keeps the buffers around anyway (it 
doesn't copy it) so I could just replace c->disable_buffer_release (or 
c->allow_renderer_switch) with c->use_pixman and get the same effect 
without any extra options. How about that?

Ander

>
> IIRC the use case was to get a compositor up fast with pixman, and then
> switch to GL - permanently?
>
>
> Thanks,
> pq
>
>>   .
>>   .\" ***************************************************************
>>   .SH ENVIRONMENT
>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>> index 5cb0fab..a3c769d 100644
>> --- a/src/compositor-drm.c
>> +++ b/src/compositor-drm.c
>> @@ -103,6 +103,7 @@ struct drm_compositor {
>>   	int cursors_are_broken;
>>
>>   	int use_pixman;
>> +	int disable_early_release;
>>
>>   	uint32_t prev_state;
>>
>> @@ -199,6 +200,7 @@ struct drm_parameters {
>>   	int connector;
>>   	int tty;
>>   	int use_pixman;
>> +	int disable_early_release;
>>   	const char *seat_id;
>>   };
>>
>> @@ -1056,18 +1058,20 @@ drm_assign_planes(struct weston_output *output)
>>   	pixman_region32_init(&overlap);
>>   	primary = &c->base.primary_plane;
>>
>> -	/* Flag all visible surfaces as keep_buffer = 1 */
>> -	wl_list_for_each(ev, &c->base.view_list, link)
>> -		ev->surface->keep_buffer = 1;
>> -
>>   	wl_list_for_each_safe(ev, next, &c->base.view_list, link) {
>> -		/* test whether this buffer can ever go into a plane:
>> -		 * non-shm, or small enough to be a cursor
>> +		struct weston_surface *es = ev->surface;
>> +
>> +		/* Test whether this buffer can ever go into a plane:
>> +		 * non-shm, or small enough to be a cursor. Also honor
>> +		 * the --disable-early-buffer-release option.
>>   		 */
>> -		if (!ev->surface->buffer_ref.buffer ||
>> -		    (wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource) &&
>> -		    (ev->geometry.width > 64 || ev->geometry.height > 64)))
>> -			ev->surface->keep_buffer = 0;
>> +		if (c->disable_early_release ||
>> +		    (es->buffer_ref.buffer &&
>> +		    (!wl_shm_buffer_get(es->buffer_ref.buffer->resource) ||
>> +		     (ev->geometry.width <= 64 && ev->geometry.height <= 64))))
>> +			es->keep_buffer = 1;
>> +		else
>> +			es->keep_buffer = 0;
>>
>>   		pixman_region32_init(&surface_overlap);
>>   		pixman_region32_intersect(&surface_overlap, &overlap,
>> @@ -2691,6 +2695,7 @@ drm_compositor_create(struct wl_display *display,
>>   	free(s);
>>
>>   	ec->use_pixman = param->use_pixman;
>> +	ec->disable_early_release = param->disable_early_release;
>>
>>   	if (weston_compositor_init(&ec->base, display, argc, argv,
>>   				   config) < 0) {
>> @@ -2839,6 +2844,8 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>>   		{ WESTON_OPTION_INTEGER, "tty", 0, &param.tty },
>>   		{ WESTON_OPTION_BOOLEAN, "current-mode", 0, &option_current_mode },
>>   		{ WESTON_OPTION_BOOLEAN, "use-pixman", 0, &param.use_pixman },
>> +		{ WESTON_OPTION_BOOLEAN, "disable-early-buffer-release", 0,
>> +		  &param.disable_early_release }
>>   	};
>>
>>   	param.seat_id = default_seat;
>> diff --git a/src/compositor.c b/src/compositor.c
>> index b8e0c6e..a43e3aa 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -3692,6 +3692,7 @@ usage(int error_code)
>>   		"  --seat=SEAT\t\tThe seat that weston should run on\n"
>>   		"  --tty=TTY\t\tThe tty to use\n"
>>   		"  --use-pixman\t\tUse the pixman (CPU) renderer\n"
>> +		"  --disable-early-bufffer-release\tDisable early buffer release\n"
>>   		"  --current-mode\tPrefer current KMS mode over EDID preferred mode\n\n");
>>
>>   	fprintf(stderr,
>