[PATCH weston v2 3/3] compositor-drm: Support linux_dmabuf output for sprite planes without gbm

Mon Oct 3 11:03:12 UTC 2016

On 09/30/2016 08:49 PM, Derek Foreman wrote:
> On 30/09/16 04:28 AM, Tomohito Esaki wrote:
>> Multiplanar formats are supported by using drmModeAddFB2 and bypassing
>> gbm. If drmModeAddFB2 isn't available, the existing gbm bo import path
>> is used and multiplanar formats are unsupported.
> I'm not sure we should be doing anything with the existing sprite code
> at all (except perhaps removing it, though I'm sure that's an unpopular
> point of view :) since it's not really viable without atomic mode
> setting, and is currently disabled until someone uses a debug key to
> enable it.
>
> Pekka - I can't recall, is the atomic mode setting series going to build
> on the current sprite stuff or blow it away and start over?
>
> I'm wondering if we should just tear out the existing sprite code in the
> meantime...
>
> Thanks,
> Derek
Hi Derek,

I do not know all the history of Weston, and I am not aware of all the 
reasons that made the sprites "broken". I have always been told that it 
was not a good idea to enable sprites because it was not working as 
expected unless we have the atomic mode.

Anyway, with my limited background, i'd like to share my point of view:

Firstly, sprite works quite fine (with a bunch of fixes that I will be 
glad to share once sprites are accepted by all) : I can have up to 3 
sprites (inluding gstreamer video playback with linux-dmabuf) + cursor 
working smoothly on two parallel outputs (extended desktop) So, sprites 
are probably a bit broken, but not totally out of order. We just still 
need to improve it.

Secondly, it is a (very) long time now, that I am waiting for atomic. I 
discussed a bit about it recently with Pekka and my understanding is 
that because people are quite busy, atomic is not about to land in main 
in a short time.

So, for theses two reasons, I think that is a good idea to improve 
sprites now : let's keep them marked as broken for the time being.
The improvements proposed by Tomohito will serve as a basis for sprites 
in atomic (I can confirm that the atomic WIP branch does not blow away 
the sprite part, the prepare_overlay_view is still there)

For what it worth, I have recently played with the atomic WIP branch, 
ported it to 1.11, and enabled linux-dmabuf in sprites. Not 100% perfect 
but it works. I know that I am not the only one who did this job 
recently, so I guess that we shall not have so many problem to find 
people to contribute to the atomic revival (with unbroken sprites inside)

BR

Fabien, a sprite fan ;)

>> Signed-off-by: Tomohito Esaki <etom at igel.co.jp>
>> ---
>>   libweston/compositor-drm.c | 53 +++++++++++++++++++++++-----------------------
>>   1 file changed, 26 insertions(+), 27 deletions(-)
>>
>> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
>> index b15fa01..f0e6f7c 100644
>> --- a/libweston/compositor-drm.c
>> +++ b/libweston/compositor-drm.c
>> @@ -1008,11 +1008,9 @@ page_flip_handler(int fd, unsigned int frame,
>>
>>   static uint32_t
>>   drm_output_check_sprite_format(struct drm_sprite *s,
>> -			       struct weston_view *ev, struct gbm_bo *bo)
>> +			       struct weston_view *ev, uint32_t format)
>>   {
>> -	uint32_t i, format;
>> -
>> -	format = gbm_bo_get_format(bo);
>> +	uint32_t i;
>>
>>   	if (format == GBM_FORMAT_ARGB8888) {
>>   		pixman_region32_t r;
>> @@ -1053,15 +1051,12 @@ drm_output_prepare_overlay_view(struct drm_output *output,
>>   	struct drm_sprite *s;
>>   	struct linux_dmabuf_buffer *dmabuf;
>>   	int found = 0;
>> -	struct gbm_bo *bo;
>> +	struct gbm_bo *bo = NULL;
>>   	pixman_region32_t dest_rect, src_rect;
>>   	pixman_box32_t *box, tbox;
>>   	uint32_t format;
>>   	wl_fixed_t sx1, sy1, sx2, sy2;
>>
>> -	if (b->gbm == NULL)
>> -		return NULL;
>> -
>>   	if (viewport->buffer.transform != output->base.transform)
>>   		return NULL;
>>
>> @@ -1101,15 +1096,9 @@ drm_output_prepare_overlay_view(struct drm_output *output,
>>   	if (!found)
>>   		return NULL;
>>
>> -	if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource))) {
>> +	if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource)) &&
>> +	    b->no_addfb2 && b->gbm) {
>>   #ifdef HAVE_GBM_FD_IMPORT
>> -		/* XXX: TODO:
>> -		 *
>> -		 * Use AddFB2 directly, do not go via GBM.
>> -		 * Add support for multiplanar formats.
>> -		 * Both require refactoring in the DRM-backend to
>> -		 * support a mix of gbm_bos and drmfbs.
>> -		 */
>>   		struct gbm_import_fd_data gbm_dmabuf = {
>>   			.fd     = dmabuf->attributes.fd[0],
>>   			.width  = dmabuf->attributes.width,
>> @@ -1126,22 +1115,32 @@ drm_output_prepare_overlay_view(struct drm_output *output,
>>   #else
>>   		return NULL;
>>   #endif
>> -	} else {
>> +	} else if (b->gbm) {
>>   		bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER,
>>   				   buffer_resource, GBM_BO_USE_SCANOUT);
>>   	}
>> -	if (!bo)
>> -		return NULL;
>>
>> -	format = drm_output_check_sprite_format(s, ev, bo);
>> -	if (format == 0) {
>> -		gbm_bo_destroy(bo);
>> -		return NULL;
>> -	}
>> +	if (bo) {
>> +		format = drm_output_check_sprite_format(
>> +			s, ev, gbm_bo_get_format(bo));
>> +		if (format == 0)
>> +			return NULL;
>> +
>> +		s->next = drm_fb_get_from_bo(bo, b, format);
>> +		if (!s->next) {
>> +			gbm_bo_destroy(bo);
>> +			return NULL;
>> +		}
>> +	} else if (dmabuf) {
>> +		format = drm_output_check_sprite_format(
>> +			s, ev, dmabuf->attributes.format);
>> +		if (format == 0)
>> +			return NULL;
>>
>> -	s->next = drm_fb_get_from_bo(bo, b, format);
>> -	if (!s->next) {
>> -		gbm_bo_destroy(bo);
>> +		s->next = drm_fb_create_dmabuf(dmabuf, b, format);
>> +		if (!s->next)
>> +			return NULL;
>> +	} else {
>>   		return NULL;
>>   	}
>>
>>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel