[PATCH v16 13/23] compositor-drm: Add modifiers to GBM dmabuf import

[Daniel Stone]

Hi Pekka,

On Mon, 9 Jul 2018 at 11:22, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Mon, 9 Jul 2018 12:39:30 +0300 Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > On Thu,  5 Jul 2018 18:16:40 +0100 Daniel Stone <daniels at collabora.com> wrote:
> > > +static struct drm_fb *
> > > +drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf,
> > > +                  struct drm_backend *backend, bool is_opaque)
> > > +{
>
> Hi,
>
> one more complication:
>
> Because is_opaque state is now baked in to drm_fb, if
> drm_fb_get_from_bo() returns an existing drm_fb for a bo, we need to
> make sure is_opaque still matches. I think is_opaque might change if
> the application is recycling dmabuf wl_buffers without destroying them
> (as intended), but changes the wl_surface opaque region according to its
> rendering.

Right you are, but on the other hand it always has been, since
drm_check_output_format() would turn ARGB8888 into XRGB8888 if the
surface was opaque.

I filed an issue for this long ago:
https://gitlab.freedesktop.org/wayland/weston/issues/20

I don't know of any good or easy answers. If an alpha <-> opaque
transition occurs whilst the FB is being displayed, calling
drmModeRmFB may forcibly remove it, including disabling the whole CRTC
(!) if we're doing fullscreen direct scanout. The way to properly
support and fix this would be to break the 1:1 buffer -> FB link,
allowing a buffer to have multiple FBs with separate properties
created from it. Given that this isn't a new problem and no-one has
complained yet, I'm inclined to leave this on the backburner.

Cheers,
Daniel