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

[Esaki Tomohito]

Hello Daniel-san

On 2017/09/27 2:09, Daniel Stone wrote:
> Hello Esaki-san,
> 
> On 1 November 2016 at 19:09, Daniel Stone  wrote:
>>> @@ -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,
>>
>> Here I think there is no benefit to using GBM import: I think it would
>> be more consistent if we always bypassed GBM and just used our own
>> non-GBM drm_fb type. What do you think? If we integrate this patch
>> into the drm_plane_state/atomic tree, the changes for this patch are
>> quite simple: mainly we need to add a BUFFER_DMABUF type to drm_fb,
>> and change the prepare_{overlay_scanout}_view paths to put the new
>> buffer into the plane state directly. I am happy to make this change
>> myself if you think it is OK.
> 
> Unfortunately I have had to pull this patch from my tree. The reason
> is that the handle returned by drmPrimeFDToHandle and closed with
> DRM_IOCTL_GEM_CLOSE is shared but not refcounted.
> 
> GEM drivers have a rule that each buffer _must_ map back to a single
> GEM handle for each DRM connection. So, multiple drmPrimeFDToHandle
> calls from the same client will return the same GEM handle if the
> underlying buffer is the same, even if it is through a different FD.
> The first DRM_IOCTL_GEM_CLOSE call will invalidate that handle
> completely.
> 
> This is a problem for EGL drivers which use dmabuf and GEM: for
> instance, on Mesa, the driver will import the dmabuf and create one
> handle, then we will create another handle from our own dmabuf import
> here, and then later close it from underneath the EGL driver. The EGL
> driver can no longer render anything, since the handle is invalid when
> submitting commands.
> 
> GBM now has a GBM_IMPORT_FD_MODIFIER path, which allows specifying
> multiple planes and format modifiers when importing. So, I have
> rewritten the patch to use this new path instead: as GBM uses the same
> DRM BO cache as EGL does, we no longer have the lifetime issue.
> 

I agree to that.
Thank you for the update.

Cheers,
Esaki
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Esaki Tomohito]

Dear Daniel,

On 2016/11/02 4:09, Daniel Stone wrote:
> This includes the small changes I mentioned to patch 1/3: please let
> me know if this change is OK with you. I think the series which is in
> there is pretty good to review already, but needs a tiny bit more
> testing. This is what I am currently rebuilding atomic on top of.

Yes, good to me.

>> @@ -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,
> 
> Here I think there is no benefit to using GBM import: I think it would
> be more consistent if we always bypassed GBM and just used our own
> non-GBM drm_fb type. What do you think? If we integrate this patch
> into the drm_plane_state/atomic tree, the changes for this patch are
> quite simple: mainly we need to add a BUFFER_DMABUF type to drm_fb,
> and change the prepare_{overlay_scanout}_view paths to put the new
> buffer into the plane state directly. I am happy to make this change
> myself if you think it is OK.

Yes, good to me.

Thanks.

-- 
--
IGEL Co., Ltd.
Tomohito Esaki
e...@igel.co.jp
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Daniel Stone]

Dear Esaki-san,

On 30 September 2016 at 10:28, 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.

Thanks very much for these four patches - these three, plus the other
one to properly account for the viewport. I have merged patch 1/3
already into my local tree (with some very minor changes), and like
2/3, but have one comment below.

My current work is here:
git://git.collabora.com/git/user/daniels/weston#wip/2016-10/drm-plane-state

This includes the small changes I mentioned to patch 1/3: please let
me know if this change is OK with you. I think the series which is in
there is pretty good to review already, but needs a tiny bit more
testing. This is what I am currently rebuilding atomic on top of.

> @@ -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,

Here I think there is no benefit to using GBM import: I think it would
be more consistent if we always bypassed GBM and just used our own
non-GBM drm_fb type. What do you think? If we integrate this patch
into the drm_plane_state/atomic tree, the changes for this patch are
quite simple: mainly we need to add a BUFFER_DMABUF type to drm_fb,
and change the prepare_{overlay_scanout}_view paths to put the new
buffer into the plane state directly. I am happy to make this change
myself if you think it is OK.

My apologies for the slow response; I have been unwell lately, so not
able to put nearly as much time into Weston as I had hoped.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Fabien DESSENNE]

Hi,


Please find below a comment.


Fabien


On 09/30/2016 11: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.
>
> Signed-off-by: Tomohito Esaki 
> ---
>   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_ARGB) {
>   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)
Unless I missed something, you shall call gbm_bo_destroy(bo) before 
returning.

> + 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Pekka Paalanen]

On Mon, 3 Oct 2016 15:53:20 +0100
Daniel Stone  wrote:

> Hi,
> 
> On 3 October 2016 at 15:01, Derek Foreman  wrote:

> > My fingers are crossed - I see this (both atomic mode setting, and the
> > ability to put dmabuf buffers into planes) as incredibly important work 
> > too.  
> 
> Yes - I think the dmabuf patches from Tomohito-san are very useful,
> and we should push those forward.
> 
> > I won't bother posting any patches to remove existing sprite functionality -
> > it seems they wouldn't be well received. :)  
> 
> Honestly, I've been leaning towards ripping them out as well.

> Given that we've had it not just disabled by default, but requiring
> either patches or user intervention _at every run_ to enable, and that
> the users (e.g. ST) here have competent atomic KMS drivers, I'd be
> very much tempted to make my own life easier by removing it as a
> prelude to the atomic patches, and building back up from there.

Hi,

I can totally ack a separate patch series removing the old sprite
support from the DRM-backend. If you want that in ahead of atomic, it's
fine by me.


Thanks,
pq


pgp4ZtrvkkmRc.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Daniel Stone]

Hi,

On 3 October 2016 at 15:01, Derek Foreman  wrote:
> On 03/10/16 06:03 AM, Fabien DESSENNE wrote:
>> 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.
>
> Sprites are broken because without atomic mode setting it's impossible to
> update all active sprites, well, atomically.
>
> There's no way to ensure they all change in the same screen refresh interval
> - we may have tearing, or on some driver stacks I think we end up with
> vblank waits during plane setting (I think this bit us with the cursor plane
> on intel drivers a while ago, and the cursor plane was made a special case
> to help...)
>
> Since tearing is something wayland was intended to do away with entirely,
> having an option to enable to bring it back seems like a bad move in weston.

Yes. The semantics are incredibly underspecified, and the drivers
diverge so much in implementation that we can never really have a
reliable implementation. If you don't have a vblank wait, then you
really need to do a pageflip of the primary plane as well to get the
event to drive things off. Or use the vblank callbacks, which is
really broken and racy. There's no way to ever fix that API properly.

>> 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.
>
> This truly is a shame, I'd hoped to see it by now as well.

Er, yeah.

>> 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)
>
> My fingers are crossed - I see this (both atomic mode setting, and the
> ability to put dmabuf buffers into planes) as incredibly important work too.

Yes - I think the dmabuf patches from Tomohito-san are very useful,
and we should push those forward.

> I won't bother posting any patches to remove existing sprite functionality -
> it seems they wouldn't be well received. :)

Honestly, I've been leaning towards ripping them out as well.

Unfortunately a lot of things have come up this year which have kept
me from spending quality time with the atomic code, but I'm now back
on it and setting myself pretty aggressive targets to get new
revisions out there. It's long past due, and really needs to be
merged.

What held things up last time, is that I finally hit the breaking
point with drm_{output,sprite}::{current,next}. The entire setup is so
fragile that the only proper fix - which aligns nicely with the
kernel's atomic API - is to split things up into three state members:
one each for actually-on-screen-now,
submitted-to-kernel-but-not-there-yet, and in-flight. Doing this was a
lot of churn, and hit a lot of fragile points, especially in the
sprite code. Then when we're finished, we end up with two codepaths
for sprites. compositor-drm becomes pretty huge.

Given that we've had it not just disabled by default, but requiring
either patches or user intervention _at every run_ to enable, and that
the users (e.g. ST) here have competent atomic KMS drivers, I'd be
very much tempted to make my own life easier by removing it as a
prelude to the atomic patches, and building back up from there.

Anyway, I'll review these patches this week, and send out v(n+1) of
atomic next. Thanks!

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Derek Foreman]

On 03/10/16 06:03 AM, Fabien DESSENNE wrote:


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.


Sprites are broken because without atomic mode setting it's impossible 
to update all active sprites, well, atomically.


There's no way to ensure they all change in the same screen refresh 
interval - we may have tearing, or on some driver stacks I think we end 
up with vblank waits during plane setting (I think this bit us with the 
cursor plane on intel drivers a while ago, and the cursor plane was made 
a special case to help...)


Since tearing is something wayland was intended to do away with 
entirely, having an option to enable to bring it back seems like a bad 
move in weston.



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.


Unfortunately there are things (the tearing) that are unimprovable 
without atomic underneath, so I'm worried about the testability of 
sprite changes without atomic mode setting landing first.



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.


This truly is a shame, I'd hoped to see it by now as well.


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)


My fingers are crossed - I see this (both atomic mode setting, and the 
ability to put dmabuf buffers into planes) as incredibly important work too.


I won't bother posting any patches to remove existing sprite 
functionality - it seems they wouldn't be well received. :)


Thanks,
Derek


BR

Fabien, a sprite fan ;)


Signed-off-by: Tomohito Esaki 
---
  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_ARGB) {
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)
   

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

[Fabien DESSENNE]

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 
>> ---
>>   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_ARGB) {
>>  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, 

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

[Derek Foreman]

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


Signed-off-by: Tomohito Esaki 
---
 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_ARGB) {
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Tomohito Esaki]

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.

Signed-off-by: Tomohito Esaki 
---
 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_ARGB) {
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;
}
 
-- 
2.7.4

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel