[PATCH weston] libweston: Fix clear timing of output repainted flag
Since the repaint status of the flushed output may be reset if a output repaint is failed, it is necessary to clear the repainted flag immediately after output repaint flush/cancel. Signed-off-by: Tomohito Esaki --- libweston/compositor.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libweston/compositor.c b/libweston/compositor.c index 516be96..9deb781 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -2450,8 +2450,6 @@ weston_output_maybe_repaint(struct weston_output *output, struct timespec *now, int ret = 0; int64_t msec_to_repaint; - output->repainted = false; - /* We're not ready yet; come back to make a decision later. */ if (output->repaint_status != REPAINT_SCHEDULED) return ret; @@ -2563,6 +2561,9 @@ output_repaint_timer_handler(void *data) repaint_data); } + wl_list_for_each(output, >output_list, link) + output->repainted = false; + output_repaint_timer_arm(compositor); return 0; -- 2.7.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] simple-dmabuf-drm: fix build with --disable-egl
This code calls into EGL to see if the dmabuf import modifiers
extension is available, and if not it assumes XRGB is supported.
Rather than disabling this client doesn't get build if one disables
EGL, we can just remove this and stop lying to ourselves.
Signed-off-by: Emilio Pozuelo Monfort
---
Alright, this removes this check entirely, which means the client will
fail if the extension is not available. This is just a test client, so
it's not the end of the world if that combination is tested.
clients/simple-dmabuf-drm.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index fcab30e5..2a21fe51 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -57,7 +57,6 @@
#include
#include "shared/zalloc.h"
-#include "shared/platform.h"
#include "xdg-shell-unstable-v6-client-protocol.h"
#include "fullscreen-shell-unstable-v1-client-protocol.h"
#include "linux-dmabuf-unstable-v1-client-protocol.h"
@@ -850,7 +849,6 @@ static struct display *
create_display(int opts, int format)
{
struct display *display;
- const char *extensions;
display = malloc(sizeof *display);
if (display == NULL) {
@@ -863,15 +861,6 @@ create_display(int opts, int format)
display->req_dmabuf_immediate = opts & OPT_IMMEDIATE;
display->req_dmabuf_modifiers = (format == DRM_FORMAT_NV12);
- /*
-* hard code format if the platform egl doesn't support format
-* querying / advertising.
-*/
- extensions = eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS);
- if (extensions && !weston_check_egl_extension(extensions,
- "EGL_EXT_image_dma_buf_import_modifiers"))
- display->xrgb_format_found = 1;
-
display->registry = wl_display_get_registry(display->display);
wl_registry_add_listener(display->registry,
_listener, display);
--
2.18.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/2] shot: add test for sub-surface with unmapped parent
This reproduces https://bugs.freedesktop.org/show_bug.cgi?id=94735.
Signed-off-by: Emilio Pozuelo Monfort
---
tests/reference/subsurface_mapped-00.png | Bin 0 -> 799 bytes
tests/reference/subsurface_mapped-01.png | Bin 0 -> 841 bytes
tests/subsurface-shot-test.c | 76 +++
3 files changed, 76 insertions(+)
create mode 100644 tests/reference/subsurface_mapped-00.png
create mode 100644 tests/reference/subsurface_mapped-01.png
diff --git a/tests/reference/subsurface_mapped-00.png
b/tests/reference/subsurface_mapped-00.png
new file mode 100644
index
..32cf32a0ed11d38b9028eda29109e06c99dce000
GIT binary patch
literal 799
zcmeAS@N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P@Hb9Ck$=lt9;Xep2*t>i(0|V0)
zPZ!6KiaBpDJMtb-U^sBV(DcSZzE>X)w43YGIc&-r>L$&*16m=d#Wzp$P!YDfm(V
literal 0
HcmV?d1
diff --git a/tests/reference/subsurface_mapped-01.png
b/tests/reference/subsurface_mapped-01.png
new file mode 100644
index
..4e7196a2229a2e63adbf49d32a8c47db043f1c1c
GIT binary patch
literal 841
zcmeAS@N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P@Hb9Ck$=lt9;Xep2*t>i(0|V1P
zPZ!6KiaBquZsa}Wz`){YUu2kf{Oyc6Wg6{8K0QxPoXkGz!#aIUUe52048Hw0nHzqy
z@FX&|88AA}Xi)SyAfT4OA#BjXDRF32cth#8hDqmtn^qrc-8g5ndd!V)&);y?A7max
zA$7y5+Tmoxxtqcd+SvzD9B$b7_V^p#1HN)}PDa0(vIRL#J}^{z1zopr
E02fd6egFUf
literal 0
HcmV?d1
diff --git a/tests/subsurface-shot-test.c b/tests/subsurface-shot-test.c
index e8bab676..53c54578 100644
--- a/tests/subsurface-shot-test.c
+++ b/tests/subsurface-shot-test.c
@@ -261,3 +261,79 @@ TEST(subsurface_z_order)
if (bufs[i])
buffer_destroy(bufs[i]);
}
+
+TEST(subsurface_mapped)
+{
+ const char *test_name = get_test_name();
+ struct client *client;
+ struct wl_surface *surface;
+ struct rectangle clip;
+ int fail = 0;
+ struct wl_subcompositor *subco;
+ struct wl_surface *child;
+ struct wl_subsurface *sub;
+ struct buffer *buf, *child_buf;
+ pixman_color_t red, blue;
+
+ color(, 255, 0, 0);
+ color(, 0, 0, 255);
+
+ /* Create the client */
+ client = create_client();
+ assert(client);
+ client->surface = create_test_surface(client);
+ surface = client->surface->wl_surface;
+
+ /* move the pointer clearly away from our screenshooting area */
+ weston_test_move_pointer(client->test->weston_test, 0, 1, 0, 2, 30);
+
+ client->surface->x = 100;
+ client->surface->y = 100;
+ weston_test_move_surface(client->test->weston_test,
client->surface->wl_surface,
+client->surface->x, client->surface->y);
+
+ wl_surface_set_buffer_scale(surface, 1);
+ wl_surface_set_buffer_transform(surface, WL_OUTPUT_TRANSFORM_NORMAL);
+
+ wl_surface_commit(surface);
+
+ clip.x = 100;
+ clip.y = 100;
+ clip.width = 100;
+ clip.height = 100;
+ printf("Clip: %d,%d %d x %d\n", clip.x, clip.y, clip.width,
clip.height);
+
+ /* create a wl_surface */
+ subco = get_subcompositor(client);
+ assert(subco);
+
+ child = wl_compositor_create_surface(client->wl_compositor);
+ assert(child);
+
+ /* make it a sub-surface */
+ sub = wl_subcompositor_get_subsurface(subco, child, surface);
+ assert(sub);
+
+ wl_subsurface_set_desync(sub);
+
+ /* give it content */
+ child_buf = surface_commit_color(client, child, , 50, 50);
+
+ /* verify that the sub-surface is not mapped */
+ fail += check_screen(client, test_name, 0, , 0);
+
+ /* give a buffer to the parent surface and make sure both the
+* parent and the child get mapped */
+ buf = surface_commit_color(client, surface, , 100, 100);
+
+ fail += check_screen(client, test_name, 1, , 1);
+
+ printf("Test complete\n");
+ assert(fail == 0);
+
+ wl_subsurface_destroy(sub);
+ wl_surface_destroy(child);
+ buffer_destroy(child_buf);
+ wl_surface_destroy(surface);
+ buffer_destroy(buf);
+}
--
2.18.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 2/2] weston-test: don't map surfaces that have no content
If a surface has no content (e.g. no buffer), then it shouldn't
be mapped, so that its subsurfaces don't get mapped either.
This works fine in the desktop-shell, but is currently broken
on the weston-test module.
https://bugs.freedesktop.org/show_bug.cgi?id
Signed-off-by: Emilio Pozuelo Monfort
---
tests/weston-test.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tests/weston-test.c b/tests/weston-test.c
index 412eb243..b67bd9fd 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -157,12 +157,21 @@ notify_pointer_position(struct weston_test *test, struct
wl_resource *resource)
weston_test_send_pointer_position(resource, pointer->x, pointer->y);
}
+static bool
+surface_has_content(struct weston_surface *surface)
+{
+ return surface->width > 0 && surface->height > 0;
+}
+
static void
test_surface_committed(struct weston_surface *surface, int32_t sx, int32_t sy)
{
struct weston_test_surface *test_surface = surface->committed_private;
struct weston_test *test = test_surface->test;
+ if (!surface_has_content(surface))
+ return;
+
if (wl_list_empty(_surface->view->layer_link.link))
weston_layer_entry_insert(>layer.view_list,
_surface->view->layer_link);
--
2.18.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 0/2 v3] sub-surface with unmapped parent
I forgot to git add the test ref images. That's the only change here. Emilio Pozuelo Monfort (2): shot: add test for sub-surface with unmapped parent weston-test: don't map surfaces that have no content tests/reference/subsurface_mapped-00.png | Bin 0 -> 799 bytes tests/reference/subsurface_mapped-01.png | Bin 0 -> 841 bytes tests/subsurface-shot-test.c | 76 +++ tests/weston-test.c | 9 +++ 4 files changed, 85 insertions(+) create mode 100644 tests/reference/subsurface_mapped-00.png create mode 100644 tests/reference/subsurface_mapped-01.png -- 2.18.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v14 34/41] compositor-drm: Ignore occluded views
Hi Pekka, On Fri, 26 Jan 2018 at 12:45, Pekka Paalanen wrote: > On Wed, 20 Dec 2017 12:26:51 + Daniel Stone wrote: > > @@ -3116,9 +3136,16 @@ drm_output_propose_state(struct weston_output > > *output_base, > > if (next_plane == primary) > > pixman_region32_union(_region, > > _region, > > - >transform.boundingbox); > > + _view); > > + else if (output->cursor_plane && > > + next_plane != >cursor_plane->base) > > Should this not be: > > if (!output->cursor_plane || next_plane != > >cursor_plane->base) > > so that lack of a cursor plane goes straight to occluded? Very right. I've force-pushed this fix (and the resultant changes when rebasing 'Add modes to drm_output_propose_state' as well as 'Return plane state from plane preparation') to the v17 branch on Collabora GitLab, but happy to re-send if that makes life easier. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v17.1 01/14] helpers: Move static_assert definition to shared
On Mon, 9 Jul 2018 15:27:46 +0100
Daniel Stone wrote:
> Collect the fallback definitions of static_assert() from desktop-shell
> and the test shell, and move them to helpers.h. This allows code
> throughout the tree to use static_assert() for build-time assertions,
> where it is supported by the compiler.
>
> As GCC goes out of its way to only add static_assert() when C11 has been
> explicitly requested - which we don't do - make sure to use the more
> widely available _Static_assert() if that is provided.
>
> This will be used in future patches to ensure two array lengths don't go
> out of sync.
>
> Signed-off-by: Daniel Stone
> ---
> desktop-shell/shell.c | 4
> shared/helpers.h | 33 +++
> tests/weston-test-desktop-shell.c | 4
> 3 files changed, 33 insertions(+), 8 deletions(-)
>
> On Mon, 9 Jul 2018 at 14:54, Pekka Paalanen wrote:
> > I'm fairly paranoid of copying GPL code. The implementation sure is
> > trivial, but what exactly did you take from the kernel?
> > The documentation bit looks already copyrightable.
>
> They probably have better documentation. :) I wrote that documentation
> myself, so it's just the macro body which was taken from the kernel. My
> understanding is that this isn't copyrightable.
>
> > Did you know of the existing uses of static_assert in Weston?
> > Oh, but there are no uses left, all we have left are the fallback empty
> > definitions... two of them. >_<
>
> But that's a good point. How about the following patch instead, which
> uses static_assert() where possible? It turns out static_assert() was
> actually mostly not defined, but I tried this one with:
> static_assert(1 != 0, "must not trigger");
> static_assert(1 == 0, "must trigger");
>
> ... and that worked.
>
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 8b7a23ade..ea3c45354 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -47,10 +47,6 @@
> #define DEFAULT_NUM_WORKSPACES 1
> #define DEFAULT_WORKSPACE_CHANGE_ANIMATION_LENGTH 200
>
> -#ifndef static_assert
> -#define static_assert(cond, msg)
> -#endif
> -
> struct focus_state {
> struct desktop_shell *shell;
> struct weston_seat *seat;
> diff --git a/shared/helpers.h b/shared/helpers.h
> index 46f745d1b..9e1de947e 100644
> --- a/shared/helpers.h
> +++ b/shared/helpers.h
> @@ -100,6 +100,39 @@ extern "C" {
> (type *)( (char *)__mptr - offsetof(type,member) );})
> #endif
>
> +/**
> + * Build-time static assertion support
> + *
> + * A build-time equivalent to assert(), will generate a compilation error
> + * if the supplied condition does not evaluate true.
> + *
> + * The following example demonstrates use of static_assert to ensure that
> + * arrays which are supposed to mirror each other have a consistent
> + * size.
> + *
> + * This is only a fallback definition; support must be provided by the
> + * compiler itself.
> + *
> + * @code
> + * int small[4];
> + * long expanded[4];
> + *
> + * static_assert(ARRAY_LENGTH(small) != ARRAY_LENGTH(expanded));
I think this condition is inverted.
> + * for (i = 0; i < ARRAY_LENGTH(small); i++)
> + * expanded[i] = small[4];
> + * @endcode
> + *
> + * @param condition Expression to check for truth
> + * @param msg Message to print on failure
> + */
> +#ifndef static_assert
> +# ifdef _Static_assert
> +# define static_assert(cond, msg) _Static_assert(cond, msg)
> +# else
> +# define static_assert(cond, msg)
> +# endif
> +#endif
> +
If one wanted to be really fancy, the empty fallback definition could
be the -1 elements array trick. But this is enough for me already.
The doc fixed:
Reviewed-by: Pekka Paalanen
Thanks,
pq
> #ifdef __cplusplus
> }
> #endif
> diff --git a/tests/weston-test-desktop-shell.c
> b/tests/weston-test-desktop-shell.c
> index de8442512..c780316d9 100644
> --- a/tests/weston-test-desktop-shell.c
> +++ b/tests/weston-test-desktop-shell.c
> @@ -41,10 +41,6 @@
> #include "shared/helpers.h"
> #include "libweston-desktop/libweston-desktop.h"
>
> -#ifndef static_assert
> -#define static_assert(cond, msg)
> -#endif
> -
> struct desktest_shell {
> struct wl_listener compositor_destroy_listener;
> struct weston_desktop *desktop;
pgpyGrHRx16bN.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17.1 01/14] helpers: Move static_assert definition to shared
Collect the fallback definitions of static_assert() from desktop-shell
and the test shell, and move them to helpers.h. This allows code
throughout the tree to use static_assert() for build-time assertions,
where it is supported by the compiler.
As GCC goes out of its way to only add static_assert() when C11 has been
explicitly requested - which we don't do - make sure to use the more
widely available _Static_assert() if that is provided.
This will be used in future patches to ensure two array lengths don't go
out of sync.
Signed-off-by: Daniel Stone
---
desktop-shell/shell.c | 4
shared/helpers.h | 33 +++
tests/weston-test-desktop-shell.c | 4
3 files changed, 33 insertions(+), 8 deletions(-)
On Mon, 9 Jul 2018 at 14:54, Pekka Paalanen wrote:
> I'm fairly paranoid of copying GPL code. The implementation sure is
> trivial, but what exactly did you take from the kernel?
> The documentation bit looks already copyrightable.
They probably have better documentation. :) I wrote that documentation
myself, so it's just the macro body which was taken from the kernel. My
understanding is that this isn't copyrightable.
> Did you know of the existing uses of static_assert in Weston?
> Oh, but there are no uses left, all we have left are the fallback empty
> definitions... two of them. >_<
But that's a good point. How about the following patch instead, which
uses static_assert() where possible? It turns out static_assert() was
actually mostly not defined, but I tried this one with:
static_assert(1 != 0, "must not trigger");
static_assert(1 == 0, "must trigger");
... and that worked.
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 8b7a23ade..ea3c45354 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -47,10 +47,6 @@
#define DEFAULT_NUM_WORKSPACES 1
#define DEFAULT_WORKSPACE_CHANGE_ANIMATION_LENGTH 200
-#ifndef static_assert
-#define static_assert(cond, msg)
-#endif
-
struct focus_state {
struct desktop_shell *shell;
struct weston_seat *seat;
diff --git a/shared/helpers.h b/shared/helpers.h
index 46f745d1b..9e1de947e 100644
--- a/shared/helpers.h
+++ b/shared/helpers.h
@@ -100,6 +100,39 @@ extern "C" {
(type *)( (char *)__mptr - offsetof(type,member) );})
#endif
+/**
+ * Build-time static assertion support
+ *
+ * A build-time equivalent to assert(), will generate a compilation error
+ * if the supplied condition does not evaluate true.
+ *
+ * The following example demonstrates use of static_assert to ensure that
+ * arrays which are supposed to mirror each other have a consistent
+ * size.
+ *
+ * This is only a fallback definition; support must be provided by the
+ * compiler itself.
+ *
+ * @code
+ * int small[4];
+ * long expanded[4];
+ *
+ * static_assert(ARRAY_LENGTH(small) != ARRAY_LENGTH(expanded));
+ * for (i = 0; i < ARRAY_LENGTH(small); i++)
+ * expanded[i] = small[4];
+ * @endcode
+ *
+ * @param condition Expression to check for truth
+ * @param msg Message to print on failure
+ */
+#ifndef static_assert
+# ifdef _Static_assert
+# define static_assert(cond, msg) _Static_assert(cond, msg)
+# else
+# define static_assert(cond, msg)
+# endif
+#endif
+
#ifdef __cplusplus
}
#endif
diff --git a/tests/weston-test-desktop-shell.c
b/tests/weston-test-desktop-shell.c
index de8442512..c780316d9 100644
--- a/tests/weston-test-desktop-shell.c
+++ b/tests/weston-test-desktop-shell.c
@@ -41,10 +41,6 @@
#include "shared/helpers.h"
#include "libweston-desktop/libweston-desktop.h"
-#ifndef static_assert
-#define static_assert(cond, msg)
-#endif
-
struct desktest_shell {
struct wl_listener compositor_destroy_listener;
struct weston_desktop *desktop;
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols] xdg-output: add a transform example for the logical size
Hi, On Mon, 9 Jul 2018 at 11:09, Simon Ser wrote: > > Bump: this is the source of bugs in xwayland [1]. > > [1]: https://lists.x.org/archives/xorg-devel/2018-July/057285.html > > On May 18, 2018 9:40 PM, Simon Ser wrote: > > Signed-off-by: Simon Ser > > --- > > > > I believe it's easy to understand that transformations are not applied to > > the logical size. > > > > unstable/xdg-output/xdg-output-unstable-v1.xml | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml > > b/unstable/xdg-output/xdg-output-unstable-v1.xml > > index bd9a8a8..320c0a0 100644 > > --- a/unstable/xdg-output/xdg-output-unstable-v1.xml > > +++ b/unstable/xdg-output/xdg-output-unstable-v1.xml > > @@ -135,6 +135,9 @@ > > - A compositor using a fractional scale of 1.5 will advertise a > > logical size to 2560×1620. > > > > + For example, for a wl_output mode 1920×1080 and a 90 degree rotation, > > the > > + compositor will advertise a logical size of 1080x1920. > > + > > The logical_size event is sent after creating an xdg_output > > (see xdg_output_manager.get_xdg_output) and whenever the logical > > size of the output changes, either as a result of a change in the > > -- > > 2.17.0 Reviewed-by: Olivier Fourdan That r-b doesn't apply to the Xwayland patch though, to which I 'll reply in the relevant thread. Cheers, Olivier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v17 06/14] compositor-drm: Use GBM modifier API
On Mon, 9 Jul 2018 14:23:12 +0100
Daniel Stone wrote:
> Now that we collect information about which modifiers are supported for
> KMS display, and are able to create KMS framebuffers with modifiers,
> begin using the modifier-aware GBM API.
>
> Client buffers from dmabuf already store multi-plane and modifier
> information into drm_fb. Extend this to drm_fb_get_from_bo(), used for
> wl_buffer, cursor, and gbm_surface buffers. wl_buffer buffers should by
> convention not require modifiers. Cursor buffers must not require
> modifiers, as they should be linear. Prior to this patch, GBM buffers
> must have been single-planar, and able to used without explicitly naming
> modifiers.
>
> Using gbm_surface_create_with_modifiers allows us to pass the list of
> modifiers acceptable to KMS for scanout to GBM, so it can allocate
> multi-planar buffers or those which are otherwise only addressible with
> modifiers. On platforms supporting and preferring modifiers for scanout,
> this means that the gbm_bos we get from our scanout surface need to use
> the extended API to query multiple planes, offsets, modifiers, etc.
>
> Signed-off-by: Daniel Stone
> Tested-by: Emre Ucan
> ---
> configure.ac | 3 ++
> libweston/compositor-drm.c | 57 --
> 2 files changed, 51 insertions(+), 9 deletions(-)
>
Reviewed-by: Pekka Paalanen
Thanks,
pq
> diff --git a/configure.ac b/configure.ac
> index c550198ae..357b6471e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -215,6 +215,9 @@ if test x$enable_drm_compositor = xyes; then
>PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83],
> [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports
> modifier advertisement])],
> [AC_MSG_WARN([libdrm does not support modifier
> advertisement])])
> + PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM_MODIFIERS, [gbm >= 17.1],
> + [AC_DEFINE([HAVE_GBM_MODIFIERS], 1, [GBM supports
> modifiers])],
> + [AC_MSG_WARN([GBM does not support modifiers])])
>PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
> [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import
> with modifiers])],
> [AC_MSG_WARN([GBM does not support dmabuf import, will omit
> that capability])])
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index bc4022201..acaf2639e 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1189,6 +1189,9 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct
> drm_backend *backend,
> bool is_opaque, enum drm_fb_type type)
> {
> struct drm_fb *fb = gbm_bo_get_user_data(bo);
> +#ifdef HAVE_GBM_MODIFIERS
> + int i;
> +#endif
>
> if (fb) {
> assert(fb->type == type);
> @@ -1202,15 +1205,25 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct
> drm_backend *backend,
> fb->type = type;
> fb->refcnt = 1;
> fb->bo = bo;
> + fb->fd = backend->drm.fd;
>
> fb->width = gbm_bo_get_width(bo);
> fb->height = gbm_bo_get_height(bo);
> + fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
> + fb->size = 0;
> +
> +#ifdef HAVE_GBM_MODIFIERS
> + fb->modifier = gbm_bo_get_modifier(bo);
> + for (i = 0; i < gbm_bo_get_plane_count(bo); i++) {
> + fb->strides[i] = gbm_bo_get_stride_for_plane(bo, i);
> + fb->handles[i] = gbm_bo_get_handle_for_plane(bo, i).u32;
> + fb->offsets[i] = gbm_bo_get_offset(bo, i);
> + }
> +#else
> fb->strides[0] = gbm_bo_get_stride(bo);
> fb->handles[0] = gbm_bo_get_handle(bo).u32;
> - fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
> fb->modifier = DRM_FORMAT_MOD_INVALID;
> - fb->size = 0;
> - fb->fd = backend->drm.fd;
> +#endif
>
> if (!fb->format) {
> weston_log("couldn't look up format 0x%lx\n",
> @@ -4355,13 +4368,39 @@ drm_output_init_egl(struct drm_output *output, struct
> drm_backend *b)
> fallback_format_for(output->gbm_format),
> };
> int n_formats = 1;
> + struct weston_mode *mode = output->base.current_mode;
> + struct drm_plane *plane = output->scanout_plane;
> + unsigned int i;
> +
> + for (i = 0; i < plane->count_formats; i++) {
> + if (plane->formats[i].format == output->gbm_format)
> + break;
> + }
> +
> + if (i == plane->count_formats) {
> + weston_log("format 0x%x not supported by output %s\n",
> +output->gbm_format, output->base.name);
> + return -1;
> + }
> +
> +#ifdef HAVE_GBM_MODIFIERS
> + if (plane->formats[i].count_modifiers > 0) {
> + output->gbm_surface =
> + gbm_surface_create_with_modifiers(b->gbm,
> + mode->width,
> +
Re: [PATCH v17 05/14] compositor-drm: Support plane IN_FORMATS
On Mon, 9 Jul 2018 14:23:11 +0100
Daniel Stone wrote:
> From: Sergi Granell
>
> The per-plane IN_FORMATS KMS property describes the format/modifier
> combinations supported for display on this plane. Read and parse this
> format, storing the data in each plane, so we can know which
> combinations might work, and which combinations definitely will not
> work.
>
> Similarly to f11ec02cad40 ("compositor-drm: Extract overlay FB import to
> helper"), we now use this when considering promoting a view to overlay
> planes. If the framebuffer's modifier is definitely not supported by the
> plane, we do not attempt to use that plane for that view.
>
> This will also be used in a follow-patch, passing the list of modifiers
> to GBM surface allocation to allow it to allocate more optimal buffers.
>
> Signed-off-by: Sergi Granell
> Reviewed-by: Daniel Stone
> Tested-by: Emre Ucan
> ---
> configure.ac | 3 +
> libweston/compositor-drm.c | 134 ++---
> 2 files changed, 128 insertions(+), 9 deletions(-)
> +#ifdef HAVE_DRM_FORMATS_BLOB
> +static inline uint32_t *
> +formats_ptr(struct drm_format_modifier_blob *blob)
> +{
> + return (uint32_t *)(((char *)blob) + blob->formats_offset);
> +}
> +
> +static inline struct drm_format_modifier *
> +modifiers_ptr(struct drm_format_modifier_blob *blob)
> +{
> + return (struct drm_format_modifier *)
> + (((char *)blob) + blob->modifiers_offset);
> +}
> +#endif
> +
> +/**
> + * Populates the plane's formats array, using either the IN_FORMATS blob
> + * property (if available), or the plane's format list if not.
> + */
> +static int
> +drm_plane_populate_formats(struct drm_plane *plane, const drmModePlane
> *kplane,
> +const drmModeObjectProperties *props)
> +{
> + unsigned i;
> +#ifdef HAVE_DRM_FORMATS_BLOB
> + drmModePropertyBlobRes *blob;
> + struct drm_format_modifier_blob *fmt_mod_blob;
> + struct drm_format_modifier *blob_modifiers;
> + uint32_t *blob_formats;
> + uint32_t blob_id;
> +
> + blob_id = drm_property_get_value(>props[WDRM_PLANE_IN_FORMATS],
> + props,
> + 0);
> + if (blob_id == 0)
> + goto fallback;
> +
> + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id);
> + if (!blob)
> + goto fallback;
> +
> + fmt_mod_blob = blob->data;
> + blob_formats = formats_ptr(fmt_mod_blob);
> + blob_modifiers = modifiers_ptr(fmt_mod_blob);
> +
> + if (plane->count_formats != fmt_mod_blob->count_formats) {
> + weston_log("DRM backend: format count differs between "
> +"plane (%d) and IN_FORMATS (%d)\n",
> +plane->count_formats,
> +fmt_mod_blob->count_formats);
> + weston_log("This represents a kernel bug; Weston is "
> +"unable to continue.\n");
> + abort();
> + }
Awesome.
Reviewed-by: Pekka Paalanen
Thanks,
pq
pgpL0LnFmqeIO.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v17 04/14] compositor-drm: Add modifiers to GBM dmabuf import
On Mon, 9 Jul 2018 14:23:10 +0100
Daniel Stone wrote:
> Add support for the GBM_BO_IMPORT_FD_MODIFIER path, which allows us to
> import multi-plane dmabufs, as well as format modifiers.
>
> Signed-off-by: Daniel Stone
> Tested-by: Emre Ucan
> ---
> configure.ac | 6 +-
> libweston/compositor-drm.c | 201 +
> 2 files changed, 160 insertions(+), 47 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index adb998dda..ef55ace36 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -212,9 +212,9 @@ if test x$enable_drm_compositor = xyes; then
>PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78],
> [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic
> API])],
> [AC_MSG_WARN([libdrm does not support atomic modesetting,
> will omit that capability])])
> - PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 10.2],
> - [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports dmabuf
> import])],
> - [AC_MSG_WARN([gbm does not support dmabuf import, will omit
> that capability])])
> + PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
> + [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import
> with modifiers])],
> + [AC_MSG_WARN([GBM does not support dmabuf import, will omit
> that capability])])
> fi
>
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 29fa98da6..ae4a4a542 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -326,6 +326,7 @@ struct drm_mode {
> enum drm_fb_type {
> BUFFER_INVALID = 0, /**< never used */
> BUFFER_CLIENT, /**< directly sourced from client */
> + BUFFER_DMABUF, /**< imported from linux_dmabuf client */
> BUFFER_PIXMAN_DUMB, /**< internal Pixman rendering */
> BUFFER_GBM_SURFACE, /**< internal EGL rendering */
> BUFFER_CURSOR, /**< internal cursor buffer */
> @@ -1038,6 +1039,145 @@ drm_fb_ref(struct drm_fb *fb)
> return fb;
> }
>
> +static void
> +drm_fb_destroy_dmabuf(struct drm_fb *fb)
> +{
> + /* We deliberately do not close the GEM handles here; GBM manages
> + * their lifetime through the BO. */
> + if (fb->bo)
> + gbm_bo_destroy(fb->bo);
> + drm_fb_destroy(fb);
> +}
> +
> +static struct drm_fb *
> +drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf,
> +struct drm_backend *backend, bool is_opaque)
> +{
> +#ifdef HAVE_GBM_FD_IMPORT
> + struct drm_fb *fb;
> + struct gbm_import_fd_data import_legacy = {
> + .width = dmabuf->attributes.width,
> + .height = dmabuf->attributes.height,
> + .format = dmabuf->attributes.format,
> + .stride = dmabuf->attributes.stride[0],
> + .fd = dmabuf->attributes.fd[0],
> + };
> + struct gbm_import_fd_modifier_data import_mod = {
> + .width = dmabuf->attributes.width,
> + .height = dmabuf->attributes.height,
> + .format = dmabuf->attributes.format,
> + .num_fds = dmabuf->attributes.n_planes,
> + .modifier = dmabuf->attributes.modifier[0],
> + };
> + int i;
> +
> + /* XXX: TODO:
> + *
> + * Currently the buffer is rejected if any dmabuf attribute
> + * flag is set. This keeps us from passing an inverted /
> + * interlaced / bottom-first buffer (or any other type that may
> + * be added in the future) through to an overlay. Ultimately,
> + * these types of buffers should be handled through buffer
> + * transforms and not as spot-checks requiring specific
> + * knowledge. */
> + if (dmabuf->attributes.flags)
> + return NULL;
> +
> + fb = zalloc(sizeof *fb);
> + if (fb == NULL)
> + return NULL;
> +
> + fb->refcnt = 1;
> + fb->type = BUFFER_DMABUF;
> +
> + BUILD_BUG_ON(ARRAY_LENGTH(import_mod.fds) !=
> + ARRAY_LENGTH(dmabuf->attributes.fd));
> + BUILD_BUG_ON(sizeof(import_mod.fds) != sizeof(dmabuf->attributes.fd));
> + memcpy(import_mod.fds, dmabuf->attributes.fd, sizeof(import_mod.fds));
Do we need the ARRAY_LENGTH check as well? I'd thought sizeof check
would suffice to avoid any damage.
> +
> + BUILD_BUG_ON(ARRAY_LENGTH(import_mod.strides) !=
> + ARRAY_LENGTH(dmabuf->attributes.stride));
> + BUILD_BUG_ON(sizeof(import_mod.strides) !=
> + sizeof(dmabuf->attributes.stride));
> + memcpy(import_mod.strides, dmabuf->attributes.stride,
> +sizeof(import_mod.strides));
> +
> + BUILD_BUG_ON(ARRAY_LENGTH(import_mod.offsets) !=
> + ARRAY_LENGTH(dmabuf->attributes.offset));
> + BUILD_BUG_ON(sizeof(import_mod.offsets) !=
> + sizeof(dmabuf->attributes.offset));
> + memcpy(import_mod.offsets, dmabuf->attributes.offset,
> +sizeof(import_mod.offsets));
Re: [PATCH v17 03/14] compositor-drm: Don't set fb->size for non-dumb buffers
On Mon, 9 Jul 2018 14:23:09 +0100
Daniel Stone wrote:
> When creating a drm_fb from client (wl_buffer/dmabuf), gbm_surface, or
> client buffers, set fb->size to 0. The size member is only used for dumb
> buffers, where we mmap the whole buffer, and need the size recorded to
> later pass to munmap.
>
> Determining the full size of multi-planar buffers is difficult, as
> auxiliary planes are not guaranteed to have a (height*stride)
> allocation, e.g. if they are subsampled or if they do not contain pixel
> data at all but, e.g., compression information. Single-plane tiled
> buffers also often pad the buffer allocation to a multiple of tile
> height, making our existing calculation incorrect.
>
> Though it does no harm to record incorrect information, it also does
> no good as we never use it; remove it in order to avoid any confusion.
>
> Signed-off-by: Daniel Stone
> ---
> libweston/compositor-drm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 7753dfba6..29fa98da6 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1063,7 +1063,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct
> drm_backend *backend,
> fb->handles[0] = gbm_bo_get_handle(bo).u32;
> fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
> fb->modifier = DRM_FORMAT_MOD_INVALID;
> - fb->size = fb->strides[0] * fb->height;
> + fb->size = 0;
> fb->fd = backend->drm.fd;
>
> if (!fb->format) {
Reviewed-by: Pekka Paalanen
Thanks,
pq
pgpicrplFPoYO.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v17 02/14] compositor-drm: Avoid cast by using unsigned loop index
On Mon, 9 Jul 2018 14:23:08 +0100
Daniel Stone wrote:
> ARRAY_LENGTH returns a size_t; rather than casting its result to
> int so we can compare to our signed index variable, just declare the
> index as a compatible type in the first place.
>
> Signed-off-by: Daniel Stone
> ---
> libweston/compositor-drm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index e09719ce4..7753dfba6 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -914,7 +914,7 @@ drm_fb_addfb(struct drm_fb *fb)
> int ret = -EINVAL;
> #ifdef HAVE_DRM_ADDFB2_MODIFIERS
> uint64_t mods[4] = { };
> - int i;
> + size_t i;
> #endif
>
> /* If we have a modifier set, we must only use the WithModifiers
> @@ -923,7 +923,7 @@ drm_fb_addfb(struct drm_fb *fb)
> /* KMS demands that if a modifier is set, it must be the same
>* for all planes. */
> #ifdef HAVE_DRM_ADDFB2_MODIFIERS
> - for (i = 0; i < (int) ARRAY_LENGTH(mods) && fb->handles[i]; i++)
> + for (i = 0; i < ARRAY_LENGTH(mods) && fb->handles[i]; i++)
> mods[i] = fb->modifier;
> ret = drmModeAddFB2WithModifiers(fb->fd, fb->width, fb->height,
>fb->format->format,
Reviewed-by: Pekka Paalanen
Thanks,
pq
pgp2QxoFeVGxY.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v17 01/14] helpers: Add BUILD_BUG_ON from Linux kernel
On Mon, 9 Jul 2018 14:23:07 +0100
Daniel Stone wrote:
> Import the trivial definition of BUILD_BUG_ON() from the Linux kernel.
> This evaluates an expression such that it will call sizeof() on a
> negative-length array if the expression is false, generating a compiler
> error.
>
> This will be used in future patches to ensure two array lengths don't go
> out of sync.
>
> Signed-off-by: Daniel Stone
> ---
> shared/helpers.h | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/shared/helpers.h b/shared/helpers.h
> index 46f745d1b..9ecc3c4e9 100644
> --- a/shared/helpers.h
> +++ b/shared/helpers.h
> @@ -100,6 +100,31 @@ extern "C" {
> (type *)( (char *)__mptr - offsetof(type,member) );})
> #endif
>
> +/**
> + * Build-time static assertion support
> + *
> + * A build-time equivalent to assert(), will generate a compilation error
> + * if the supplied condition does not evaluate true.
> + *
> + * The following example demonstrates use of BUILD_BUG_ON to ensure that
> + * arrays which are supposed to mirror each other have a consistent
> + * size.
> + *
> + * @code
> + * int small[4];
> + * long expanded[4];
> + *
> + * BUILD_BUG_ON(ARRAY_LENGTH(small) != ARRAY_LENGTH(expanded));
> + * for (i = 0; i < ARRAY_LENGTH(small); i++)
> + * expanded[i] = small[4];
> + * @endcode
> + *
> + * @param condition Expression to check for truth
> + */
> +#ifndef BUILD_BUG_ON
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#endif
> +
> #ifdef __cplusplus
> }
> #endif
Hi,
I'm fairly paranoid of copying GPL code. The implementation sure is
trivial, but what exactly did you take from the kernel?
The documentation bit looks already copyrightable.
Did you know of the existing uses of static_assert in Weston?
Oh, but there are no uses left, all we have left are the fallback empty
definitions... two of them. >_<
Thanks,
pq
pgpanqgNPS993.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 07/14] compositor-drm: Split drm_assign_planes in two
Move drm_assign_planes into two functions: one which proposes a plane
configuration, and another which applies that state to the Weston
internal structures. This will be used to try multiple configurations
and see which is supported.
Signed-off-by: Daniel Stone
Reviewed-by: Pekka Paalanen
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 155 +++--
1 file changed, 97 insertions(+), 58 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index acaf2639e..2eb3d4073 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -407,6 +407,8 @@ struct drm_plane_state {
struct drm_fb *fb;
+ struct weston_view *ev; /**< maintained for drm_assign_planes only */
+
int32_t src_x, src_y;
uint32_t src_w, src_h;
int32_t dest_x, dest_y;
@@ -1974,6 +1976,7 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
}
state->fb = fb;
+ state->ev = ev;
state->output = output;
if (!drm_plane_state_coords_for_view(state, ev))
goto err;
@@ -3045,6 +3048,7 @@ drm_output_prepare_overlay_view(struct drm_output_state
*output_state,
}
state->fb = fb;
+ state->ev = ev;
state->output = output;
if (!drm_plane_state_coords_for_view(state, ev))
@@ -3172,6 +3176,7 @@ drm_output_prepare_cursor_view(struct drm_output_state
*output_state,
}
output->cursor_view = ev;
+ plane_state->ev = ev;
plane_state->fb =
drm_fb_ref(output->gbm_cursor_fb[output->current_cursor]);
@@ -3248,40 +3253,15 @@ err:
drmModeSetCursor(b->drm.fd, output->crtc_id, 0, 0, 0);
}
-/*
- * Get the aspect-ratio from drmModeModeInfo mode flags.
- *
- * @param drm_mode_flags- flags from drmModeModeInfo structure.
- * @returns aspect-ratio as encoded in enum 'weston_mode_aspect_ratio'.
- */
-static enum weston_mode_aspect_ratio
-drm_to_weston_mode_aspect_ratio(uint32_t drm_mode_flags)
-{
- return (drm_mode_flags & DRM_MODE_FLAG_PIC_AR_MASK) >>
- DRM_MODE_FLAG_PIC_AR_BITS_POS;
-}
-
-static const char *
-aspect_ratio_to_string(enum weston_mode_aspect_ratio ratio)
-{
- if (ratio < 0 || ratio >= ARRAY_LENGTH(aspect_ratio_as_string) ||
- !aspect_ratio_as_string[ratio])
- return " (unknown aspect ratio)";
-
- return aspect_ratio_as_string[ratio];
-}
-
-static void
-drm_assign_planes(struct weston_output *output_base, void *repaint_data)
+static struct drm_output_state *
+drm_output_propose_state(struct weston_output *output_base,
+struct drm_pending_state *pending_state)
{
- struct drm_backend *b = to_drm_backend(output_base->compositor);
- struct drm_pending_state *pending_state = repaint_data;
struct drm_output *output = to_drm_output(output_base);
struct drm_output_state *state;
- struct drm_plane_state *plane_state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region;
- struct weston_plane *primary, *next_plane;
+ struct weston_plane *primary = _base->compositor->primary_plane;
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3302,35 +3282,21 @@ drm_assign_planes(struct weston_output *output_base,
void *repaint_data)
* as we do for flipping full screen surfaces.
*/
pixman_region32_init(_region);
- primary = _base->compositor->primary_plane;
wl_list_for_each(ev, _base->compositor->view_list, link) {
- 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, keep a reference when using the pixman renderer.
-* That makes it possible to do a seamless switch to the GL
-* renderer and since the pixman renderer keeps a reference
-* to the buffer anyway, there is no side effects.
-*/
- if (b->use_pixman ||
- (es->buffer_ref.buffer &&
- (!wl_shm_buffer_get(es->buffer_ref.buffer->resource) ||
-(ev->surface->width <= b->cursor_width &&
- ev->surface->height <= b->cursor_height
- es->keep_buffer = true;
- else
- es->keep_buffer = false;
+ struct weston_plane *next_plane = NULL;
+ /* Since we process views from top to bottom, we know that if
+* the view intersects the calculated renderer region, it must
+* be part of, or occluded by, it, and cannot go on a plane. */
pixman_region32_init(_overlap);
pixman_region32_intersect(_overlap, _region,
[PATCH v17 09/14] compositor-drm: Ignore occluded views
When trying to assign planes, keep track of the areas which are
already occluded, and ignore views which are completely occluded. This
allows us to build a state using planes only, when there are occluded
views which cannot go into a plane behind views which can.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 37 -
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ca885f96e..b4a65209a 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3252,7 +3252,7 @@ drm_output_propose_state(struct weston_output
*output_base,
struct drm_output *output = to_drm_output(output_base);
struct drm_output_state *state;
struct weston_view *ev;
- pixman_region32_t surface_overlap, renderer_region;
+ pixman_region32_t surface_overlap, renderer_region, occluded_region;
struct weston_plane *primary = _base->compositor->primary_plane;
assert(!output->state_last);
@@ -3274,9 +3274,12 @@ drm_output_propose_state(struct weston_output
*output_base,
* as we do for flipping full screen surfaces.
*/
pixman_region32_init(_region);
+ pixman_region32_init(_region);
wl_list_for_each(ev, _base->compositor->view_list, link) {
struct weston_plane *next_plane = NULL;
+ pixman_region32_t clipped_view;
+ bool occluded = false;
/* If this view doesn't touch our output at all, there's no
* reason to do anything with it. */
@@ -3288,13 +3291,27 @@ drm_output_propose_state(struct weston_output
*output_base,
if (ev->output_mask != (1u << output->base.id))
next_plane = primary;
+ /* Ignore views we know to be totally occluded. */
+ pixman_region32_init(_view);
+ pixman_region32_intersect(_view,
+ >transform.boundingbox,
+ >base.region);
+
+ pixman_region32_init(_overlap);
+ pixman_region32_subtract(_overlap, _view,
+_region);
+ occluded = !pixman_region32_not_empty(_overlap);
+ if (occluded) {
+ pixman_region32_fini(_overlap);
+ pixman_region32_fini(_view);
+ continue;
+ }
+
/* Since we process views from top to bottom, we know that if
* the view intersects the calculated renderer region, it must
* be part of, or occluded by, it, and cannot go on a plane. */
- pixman_region32_init(_overlap);
pixman_region32_intersect(_overlap, _region,
- >transform.boundingbox);
-
+ _view);
if (pixman_region32_not_empty(_overlap))
next_plane = primary;
pixman_region32_fini(_overlap);
@@ -3302,6 +3319,9 @@ drm_output_propose_state(struct weston_output
*output_base,
if (next_plane == NULL)
next_plane = drm_output_prepare_cursor_view(state, ev);
+ if (next_plane == NULL && !drm_view_is_opaque(ev))
+ next_plane = primary;
+
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(state, ev);
@@ -3314,9 +3334,16 @@ drm_output_propose_state(struct weston_output
*output_base,
if (next_plane == primary)
pixman_region32_union(_region,
_region,
- >transform.boundingbox);
+ _view);
+ else if (output->cursor_plane &&
+next_plane != >cursor_plane->base)
+ pixman_region32_union(_region,
+ _region,
+ _view);
+ pixman_region32_fini(_view);
}
pixman_region32_fini(_region);
+ pixman_region32_fini(_region);
return state;
}
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 12/14] compositor-drm: Add test-only mode to state application
The atomic API can allow us to test state before we apply it, to see if
it will be valid. Add support for this, which we will later use in
assign_planes to ensure our plane configuration is valid.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 176 -
1 file changed, 136 insertions(+), 40 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ec0f9e7eb..0a3f524f5 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -260,6 +260,7 @@ enum drm_output_state_duplicate_mode {
enum drm_state_apply_mode {
DRM_STATE_APPLY_SYNC, /**< state fully processed */
DRM_STATE_APPLY_ASYNC, /**< state pending event delivery */
+ DRM_STATE_TEST_ONLY, /**< test if the state can be applied */
};
struct drm_backend {
@@ -1812,6 +1813,7 @@ drm_pending_state_get_output(struct drm_pending_state
*pending_state,
}
static int drm_pending_state_apply_sync(struct drm_pending_state *state);
+static int drm_pending_state_test(struct drm_pending_state *state);
/**
* Mark a drm_output_state (the output's last state) as complete. This handles
@@ -1936,13 +1938,16 @@ enum drm_output_propose_state_mode {
static struct drm_plane_state *
drm_output_prepare_scanout_view(struct drm_output_state *output_state,
- struct weston_view *ev)
+ struct weston_view *ev,
+ enum drm_output_propose_state_mode mode)
{
struct drm_output *output = output_state->output;
struct drm_plane *scanout_plane = output->scanout_plane;
struct drm_plane_state *state;
+ struct drm_plane_state *state_old = NULL;
struct drm_fb *fb;
pixman_box32_t *extents;
+ int ret;
/* Check the view spans exactly the output size, calculated in the
* logical co-ordinate space. */
@@ -1967,11 +1972,18 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
}
state = drm_output_state_get_plane(output_state, scanout_plane);
- if (state->fb) {
- /* If there is already a framebuffer on the scanout plane,
-* a client view has already been placed on the scanout
-* view. In that case, do not free or put back the state,
-* but just leave it in place and quietly exit. */
+
+ /* Check if we've already placed a buffer on this plane; if there's a
+* buffer there but it comes from GBM, then it's the result of
+* drm_output_propose_state placing it here for testing purposes. */
+ if (state->fb &&
+ (state->fb->type == BUFFER_GBM_SURFACE ||
+state->fb->type == BUFFER_PIXMAN_DUMB)) {
+ state_old = calloc(1, sizeof(*state_old));
+ memcpy(state_old, state, sizeof(*state_old));
+ state_old->output_state = NULL;
+ wl_list_init(_old->link);
+ } else if (state->fb) {
drm_fb_unref(fb);
return NULL;
}
@@ -1991,10 +2003,24 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
+ if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
+ drm_plane_state_free(state_old, false);
+ return state;
+ }
+
+ ret = drm_pending_state_test(output_state->pending_state);
+ if (ret != 0)
+ goto err;
+
+ drm_plane_state_free(state_old, false);
return state;
err:
- drm_plane_state_put_back(state);
+ drm_plane_state_free(state, false);
+ if (state_old) {
+ wl_list_insert(_state->plane_list, _old->link);
+ state_old->output_state = output_state;
+ }
return NULL;
}
@@ -2062,7 +2088,9 @@ drm_output_render(struct drm_output_state *state,
pixman_region32_t *damage)
* want to render. */
scanout_state = drm_output_state_get_plane(state,
output->scanout_plane);
- if (scanout_state->fb)
+ if (scanout_state->fb &&
+ scanout_state->fb->type != BUFFER_GBM_SURFACE &&
+ scanout_state->fb->type != BUFFER_PIXMAN_DUMB)
return;
if (!pixman_region32_not_empty(damage) &&
@@ -2085,6 +2113,7 @@ drm_output_render(struct drm_output_state *state,
pixman_region32_t *damage)
return;
}
+ drm_fb_unref(scanout_state->fb);
scanout_state->fb = fb;
scanout_state->output = output;
@@ -2597,9 +2626,16 @@ drm_pending_state_apply_atomic(struct drm_pending_state
*pending_state,
case DRM_STATE_APPLY_ASYNC:
flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
break;
+ case DRM_STATE_TEST_ONLY:
+
[PATCH v17 14/14] compositor-drm: Enable planes for atomic
Now that we can sensibly test proposed plane configurations with atomic,
sprites are not broken.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index b97872aa1..f0f40385d 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3772,6 +3772,17 @@ init_kms_caps(struct drm_backend *b)
weston_log("DRM: %s atomic modesetting\n",
b->atomic_modeset ? "supports" : "does not support");
+ /*
+* KMS support for hardware planes cannot properly synchronize
+* without nuclear page flip. Without nuclear/atomic, hw plane
+* and cursor plane updates would either tear or cause extra
+* waits for vblanks which means dropping the compositor framerate
+* to a fraction. For cursors, it's not so bad, so they are
+* enabled.
+*/
+ if (!b->atomic_modeset)
+ b->sprites_are_broken = 1;
+
ret = drmSetClientCap(b->drm.fd, DRM_CLIENT_CAP_ASPECT_RATIO, 1);
b->aspect_ratio_supported = (ret == 0);
weston_log("DRM: %s picture aspect ratio\n",
@@ -6652,17 +6663,6 @@ drm_backend_create(struct weston_compositor *compositor,
b->drm.fd = -1;
wl_array_init(>unused_crtcs);
- /*
-* KMS support for hardware planes cannot properly synchronize
-* without nuclear page flip. Without nuclear/atomic, hw plane
-* and cursor plane updates would either tear or cause extra
-* waits for vblanks which means dropping the compositor framerate
-* to a fraction. For cursors, it's not so bad, so they are
-* enabled.
-*
-* These can be enabled again when nuclear/atomic support lands.
-*/
- b->sprites_are_broken = 1;
b->compositor = compositor;
b->use_pixman = config->use_pixman;
b->pageflip_timeout = config->pageflip_timeout;
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 11/14] compositor-drm: Return plane state from plane preparation
Return a pointer to the plane state, rather than indirecting via a
weston_plane.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 71 +-
1 file changed, 39 insertions(+), 32 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index c91428d96..ec0f9e7eb 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1934,7 +1934,7 @@ enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes */
};
-static struct weston_plane *
+static struct drm_plane_state *
drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
{
@@ -1991,7 +1991,7 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
- return _plane->base;
+ return state;
err:
drm_plane_state_put_back(state);
@@ -2157,7 +2157,6 @@ drm_output_apply_state_legacy(struct drm_output_state
*state)
struct drm_property_info *dpms_prop;
struct drm_plane_state *scanout_state;
struct drm_plane_state *ps;
- struct drm_plane *p;
struct drm_mode *mode;
struct drm_head *head;
uint32_t connectors[MAX_CLONED_CONNECTORS];
@@ -2184,7 +2183,7 @@ drm_output_apply_state_legacy(struct drm_output_state
*state)
if (state->dpms != WESTON_DPMS_ON) {
wl_list_for_each(ps, >plane_list, link) {
- p = ps->plane;
+ struct drm_plane *p = ps->plane;
assert(ps->fb == NULL);
assert(ps->output == NULL);
@@ -2274,8 +2273,8 @@ drm_output_apply_state_legacy(struct drm_output_state
*state)
.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT,
.request.sequence = 1,
};
+ struct drm_plane *p = ps->plane;
- p = ps->plane;
if (p->type != WDRM_PLANE_TYPE_OVERLAY)
continue;
@@ -2987,7 +2986,7 @@ atomic_flip_handler(int fd, unsigned int frame, unsigned
int sec,
}
#endif
-static struct weston_plane *
+static struct drm_plane_state *
drm_output_prepare_overlay_view(struct drm_output_state *output_state,
struct weston_view *ev)
{
@@ -3059,7 +3058,7 @@ drm_output_prepare_overlay_view(struct drm_output_state
*output_state,
state->src_h != state->dest_h << 16)
goto err;
- return >base;
+ return state;
err:
drm_plane_state_put_back(state);
@@ -3103,7 +3102,7 @@ cursor_bo_update(struct drm_plane_state *plane_state,
struct weston_view *ev)
weston_log("failed update cursor: %m\n");
}
-static struct weston_plane *
+static struct drm_plane_state *
drm_output_prepare_cursor_view(struct drm_output_state *output_state,
struct weston_view *ev)
{
@@ -3190,7 +3189,7 @@ drm_output_prepare_cursor_view(struct drm_output_state
*output_state,
plane_state->dest_w = b->cursor_width;
plane_state->dest_h = b->cursor_height;
- return >base;
+ return plane_state;
err:
drm_plane_state_put_back(plane_state);
@@ -3260,7 +3259,6 @@ drm_output_propose_state(struct weston_output
*output_base,
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
- struct weston_plane *primary = _base->compositor->primary_plane;
bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
bool planes_ok = !b->sprites_are_broken;
@@ -3286,7 +3284,8 @@ drm_output_propose_state(struct weston_output
*output_base,
pixman_region32_init(_region);
wl_list_for_each(ev, _base->compositor->view_list, link) {
- struct weston_plane *next_plane = NULL;
+ struct drm_plane_state *ps = NULL;
+ bool force_renderer = false;
pixman_region32_t clipped_view;
bool occluded = false;
@@ -3298,7 +3297,7 @@ drm_output_propose_state(struct weston_output
*output_base,
/* We only assign planes to views which are exclusively present
* on our output. */
if (ev->output_mask != (1u << output->base.id))
- next_plane = primary;
+ force_renderer = true;
/* Ignore views we know to be totally occluded. */
pixman_region32_init(_view);
@@ -3322,40 +3321,48 @@ drm_output_propose_state(struct weston_output
*output_base,
pixman_region32_intersect(_overlap, _region,
_view);
if
[PATCH v17 10/14] compositor-drm: Add modes to drm_output_propose_state
Add support for multiple modes: toggling whether or not the renderer
and/or planes are acceptable. This will be used to implement a smarter
plane-placement heuristic when we have support for testing output
states.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 39 ++
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index b4a65209a..c91428d96 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1929,6 +1929,11 @@ drm_output_assign_state(struct drm_output_state *state,
}
}
+enum drm_output_propose_state_mode {
+ DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
+ DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes */
+};
+
static struct weston_plane *
drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
@@ -3247,13 +3252,17 @@ err:
static struct drm_output_state *
drm_output_propose_state(struct weston_output *output_base,
-struct drm_pending_state *pending_state)
+struct drm_pending_state *pending_state,
+enum drm_output_propose_state_mode mode)
{
struct drm_output *output = to_drm_output(output_base);
+ struct drm_backend *b = to_drm_backend(output_base->compositor);
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
struct weston_plane *primary = _base->compositor->primary_plane;
+ bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
+ bool planes_ok = !b->sprites_are_broken;
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3316,36 +3325,49 @@ drm_output_propose_state(struct weston_output
*output_base,
next_plane = primary;
pixman_region32_fini(_overlap);
- if (next_plane == NULL)
+ /* The cursor plane is 'special' in the sense that we can still
+* place it in the legacy API, and we gate that with a separate
+* cursors_are_broken flag. */
+ if (next_plane == NULL && !b->cursors_are_broken)
next_plane = drm_output_prepare_cursor_view(state, ev);
if (next_plane == NULL && !drm_view_is_opaque(ev))
next_plane = primary;
+ if (next_plane == NULL && !planes_ok)
+ next_plane = primary;
+
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(state, ev);
if (next_plane == NULL)
next_plane = drm_output_prepare_overlay_view(state, ev);
- if (next_plane == NULL)
- next_plane = primary;
+ if (!next_plane || next_plane == primary) {
+ if (!renderer_ok)
+ goto err;
- if (next_plane == primary)
pixman_region32_union(_region,
_region,
_view);
- else if (output->cursor_plane &&
-next_plane != >cursor_plane->base)
+ } else if (output->cursor_plane &&
+ next_plane != >cursor_plane->base) {
pixman_region32_union(_region,
_region,
_view);
+ }
pixman_region32_fini(_view);
}
pixman_region32_fini(_region);
pixman_region32_fini(_region);
return state;
+
+err:
+ pixman_region32_fini(_region);
+ pixman_region32_fini(_region);
+ drm_output_state_free(state);
+ return NULL;
}
static void
@@ -3359,7 +3381,8 @@ drm_assign_planes(struct weston_output *output_base, void
*repaint_data)
struct weston_view *ev;
struct weston_plane *primary = _base->compositor->primary_plane;
- state = drm_output_propose_state(output_base, pending_state);
+ state = drm_output_propose_state(output_base, pending_state,
+DRM_OUTPUT_PROPOSE_STATE_MIXED);
wl_list_for_each(ev, _base->compositor->view_list, link) {
struct drm_plane *target_plane = NULL;
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 03/14] compositor-drm: Don't set fb->size for non-dumb buffers
When creating a drm_fb from client (wl_buffer/dmabuf), gbm_surface, or
client buffers, set fb->size to 0. The size member is only used for dumb
buffers, where we mmap the whole buffer, and need the size recorded to
later pass to munmap.
Determining the full size of multi-planar buffers is difficult, as
auxiliary planes are not guaranteed to have a (height*stride)
allocation, e.g. if they are subsampled or if they do not contain pixel
data at all but, e.g., compression information. Single-plane tiled
buffers also often pad the buffer allocation to a multiple of tile
height, making our existing calculation incorrect.
Though it does no harm to record incorrect information, it also does
no good as we never use it; remove it in order to avoid any confusion.
Signed-off-by: Daniel Stone
---
libweston/compositor-drm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 7753dfba6..29fa98da6 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1063,7 +1063,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend
*backend,
fb->handles[0] = gbm_bo_get_handle(bo).u32;
fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
fb->modifier = DRM_FORMAT_MOD_INVALID;
- fb->size = fb->strides[0] * fb->height;
+ fb->size = 0;
fb->fd = backend->drm.fd;
if (!fb->format) {
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 04/14] compositor-drm: Add modifiers to GBM dmabuf import
Add support for the GBM_BO_IMPORT_FD_MODIFIER path, which allows us to
import multi-plane dmabufs, as well as format modifiers.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
configure.ac | 6 +-
libweston/compositor-drm.c | 201 +
2 files changed, 160 insertions(+), 47 deletions(-)
diff --git a/configure.ac b/configure.ac
index adb998dda..ef55ace36 100644
--- a/configure.ac
+++ b/configure.ac
@@ -212,9 +212,9 @@ if test x$enable_drm_compositor = xyes; then
PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78],
[AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic
API])],
[AC_MSG_WARN([libdrm does not support atomic modesetting,
will omit that capability])])
- PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 10.2],
- [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports dmabuf
import])],
- [AC_MSG_WARN([gbm does not support dmabuf import, will omit
that capability])])
+ PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
+ [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import
with modifiers])],
+ [AC_MSG_WARN([GBM does not support dmabuf import, will omit
that capability])])
fi
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 29fa98da6..ae4a4a542 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -326,6 +326,7 @@ struct drm_mode {
enum drm_fb_type {
BUFFER_INVALID = 0, /**< never used */
BUFFER_CLIENT, /**< directly sourced from client */
+ BUFFER_DMABUF, /**< imported from linux_dmabuf client */
BUFFER_PIXMAN_DUMB, /**< internal Pixman rendering */
BUFFER_GBM_SURFACE, /**< internal EGL rendering */
BUFFER_CURSOR, /**< internal cursor buffer */
@@ -1038,6 +1039,145 @@ drm_fb_ref(struct drm_fb *fb)
return fb;
}
+static void
+drm_fb_destroy_dmabuf(struct drm_fb *fb)
+{
+ /* We deliberately do not close the GEM handles here; GBM manages
+* their lifetime through the BO. */
+ if (fb->bo)
+ gbm_bo_destroy(fb->bo);
+ drm_fb_destroy(fb);
+}
+
+static struct drm_fb *
+drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf,
+ struct drm_backend *backend, bool is_opaque)
+{
+#ifdef HAVE_GBM_FD_IMPORT
+ struct drm_fb *fb;
+ struct gbm_import_fd_data import_legacy = {
+ .width = dmabuf->attributes.width,
+ .height = dmabuf->attributes.height,
+ .format = dmabuf->attributes.format,
+ .stride = dmabuf->attributes.stride[0],
+ .fd = dmabuf->attributes.fd[0],
+ };
+ struct gbm_import_fd_modifier_data import_mod = {
+ .width = dmabuf->attributes.width,
+ .height = dmabuf->attributes.height,
+ .format = dmabuf->attributes.format,
+ .num_fds = dmabuf->attributes.n_planes,
+ .modifier = dmabuf->attributes.modifier[0],
+ };
+ int i;
+
+ /* XXX: TODO:
+*
+* Currently the buffer is rejected if any dmabuf attribute
+* flag is set. This keeps us from passing an inverted /
+* interlaced / bottom-first buffer (or any other type that may
+* be added in the future) through to an overlay. Ultimately,
+* these types of buffers should be handled through buffer
+* transforms and not as spot-checks requiring specific
+* knowledge. */
+ if (dmabuf->attributes.flags)
+ return NULL;
+
+ fb = zalloc(sizeof *fb);
+ if (fb == NULL)
+ return NULL;
+
+ fb->refcnt = 1;
+ fb->type = BUFFER_DMABUF;
+
+ BUILD_BUG_ON(ARRAY_LENGTH(import_mod.fds) !=
+ARRAY_LENGTH(dmabuf->attributes.fd));
+ BUILD_BUG_ON(sizeof(import_mod.fds) != sizeof(dmabuf->attributes.fd));
+ memcpy(import_mod.fds, dmabuf->attributes.fd, sizeof(import_mod.fds));
+
+ BUILD_BUG_ON(ARRAY_LENGTH(import_mod.strides) !=
+ARRAY_LENGTH(dmabuf->attributes.stride));
+ BUILD_BUG_ON(sizeof(import_mod.strides) !=
+sizeof(dmabuf->attributes.stride));
+ memcpy(import_mod.strides, dmabuf->attributes.stride,
+ sizeof(import_mod.strides));
+
+ BUILD_BUG_ON(ARRAY_LENGTH(import_mod.offsets) !=
+ARRAY_LENGTH(dmabuf->attributes.offset));
+ BUILD_BUG_ON(sizeof(import_mod.offsets) !=
+sizeof(dmabuf->attributes.offset));
+ memcpy(import_mod.offsets, dmabuf->attributes.offset,
+ sizeof(import_mod.offsets));
+
+ /* The legacy FD-import path does not allow us to supply modifiers,
+* multiple planes, or buffer offsets. */
+ if (dmabuf->attributes.modifier[0] != DRM_FORMAT_MOD_INVALID ||
+ import_mod.num_fds > 1 ||
+
[PATCH v17 06/14] compositor-drm: Use GBM modifier API
Now that we collect information about which modifiers are supported for
KMS display, and are able to create KMS framebuffers with modifiers,
begin using the modifier-aware GBM API.
Client buffers from dmabuf already store multi-plane and modifier
information into drm_fb. Extend this to drm_fb_get_from_bo(), used for
wl_buffer, cursor, and gbm_surface buffers. wl_buffer buffers should by
convention not require modifiers. Cursor buffers must not require
modifiers, as they should be linear. Prior to this patch, GBM buffers
must have been single-planar, and able to used without explicitly naming
modifiers.
Using gbm_surface_create_with_modifiers allows us to pass the list of
modifiers acceptable to KMS for scanout to GBM, so it can allocate
multi-planar buffers or those which are otherwise only addressible with
modifiers. On platforms supporting and preferring modifiers for scanout,
this means that the gbm_bos we get from our scanout surface need to use
the extended API to query multiple planes, offsets, modifiers, etc.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
configure.ac | 3 ++
libweston/compositor-drm.c | 57 --
2 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index c550198ae..357b6471e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -215,6 +215,9 @@ if test x$enable_drm_compositor = xyes; then
PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83],
[AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports
modifier advertisement])],
[AC_MSG_WARN([libdrm does not support modifier
advertisement])])
+ PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM_MODIFIERS, [gbm >= 17.1],
+ [AC_DEFINE([HAVE_GBM_MODIFIERS], 1, [GBM supports
modifiers])],
+ [AC_MSG_WARN([GBM does not support modifiers])])
PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
[AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import
with modifiers])],
[AC_MSG_WARN([GBM does not support dmabuf import, will omit
that capability])])
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index bc4022201..acaf2639e 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1189,6 +1189,9 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend
*backend,
bool is_opaque, enum drm_fb_type type)
{
struct drm_fb *fb = gbm_bo_get_user_data(bo);
+#ifdef HAVE_GBM_MODIFIERS
+ int i;
+#endif
if (fb) {
assert(fb->type == type);
@@ -1202,15 +1205,25 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct
drm_backend *backend,
fb->type = type;
fb->refcnt = 1;
fb->bo = bo;
+ fb->fd = backend->drm.fd;
fb->width = gbm_bo_get_width(bo);
fb->height = gbm_bo_get_height(bo);
+ fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
+ fb->size = 0;
+
+#ifdef HAVE_GBM_MODIFIERS
+ fb->modifier = gbm_bo_get_modifier(bo);
+ for (i = 0; i < gbm_bo_get_plane_count(bo); i++) {
+ fb->strides[i] = gbm_bo_get_stride_for_plane(bo, i);
+ fb->handles[i] = gbm_bo_get_handle_for_plane(bo, i).u32;
+ fb->offsets[i] = gbm_bo_get_offset(bo, i);
+ }
+#else
fb->strides[0] = gbm_bo_get_stride(bo);
fb->handles[0] = gbm_bo_get_handle(bo).u32;
- fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
fb->modifier = DRM_FORMAT_MOD_INVALID;
- fb->size = 0;
- fb->fd = backend->drm.fd;
+#endif
if (!fb->format) {
weston_log("couldn't look up format 0x%lx\n",
@@ -4355,13 +4368,39 @@ drm_output_init_egl(struct drm_output *output, struct
drm_backend *b)
fallback_format_for(output->gbm_format),
};
int n_formats = 1;
+ struct weston_mode *mode = output->base.current_mode;
+ struct drm_plane *plane = output->scanout_plane;
+ unsigned int i;
+
+ for (i = 0; i < plane->count_formats; i++) {
+ if (plane->formats[i].format == output->gbm_format)
+ break;
+ }
+
+ if (i == plane->count_formats) {
+ weston_log("format 0x%x not supported by output %s\n",
+ output->gbm_format, output->base.name);
+ return -1;
+ }
+
+#ifdef HAVE_GBM_MODIFIERS
+ if (plane->formats[i].count_modifiers > 0) {
+ output->gbm_surface =
+ gbm_surface_create_with_modifiers(b->gbm,
+ mode->width,
+ mode->height,
+ output->gbm_format,
+
plane->formats[i].modifiers,
+
[PATCH v17 13/14] compositor-drm: Relax plane restrictions for atomic
Since we now incrementally test atomic state as we build it, we can
loosen restrictions on what we can do with planes, and let the kernel
tell us whether or not it's OK.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 0a3f524f5..b97872aa1 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1942,6 +1942,7 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
enum drm_output_propose_state_mode mode)
{
struct drm_output *output = output_state->output;
+ struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_plane *scanout_plane = output->scanout_plane;
struct drm_plane_state *state;
struct drm_plane_state *state_old = NULL;
@@ -1966,7 +1967,7 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
return NULL;
/* Can't change formats with just a pageflip */
- if (fb->format->format != output->gbm_format) {
+ if (!b->atomic_modeset && fb->format->format != output->gbm_format) {
drm_fb_unref(fb);
return NULL;
}
@@ -1994,15 +1995,18 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
if (!drm_plane_state_coords_for_view(state, ev))
goto err;
- /* The legacy API does not let us perform cropping or scaling. */
- if (state->src_x != 0 || state->src_y != 0 ||
- state->src_w != state->dest_w << 16 ||
- state->src_h != state->dest_h << 16 ||
- state->dest_x != 0 || state->dest_y != 0 ||
+ if (state->dest_x != 0 || state->dest_y != 0 ||
state->dest_w != (unsigned) output->base.current_mode->width ||
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
+ /* The legacy API does not let us perform cropping or scaling. */
+ if (!b->atomic_modeset &&
+ (state->src_x != 0 || state->src_y != 0 ||
+state->src_w != state->dest_w << 16 ||
+state->src_h != state->dest_h << 16))
+ goto err;
+
if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
drm_plane_state_free(state_old, false);
return state;
@@ -3092,8 +3096,9 @@ drm_output_prepare_overlay_view(struct drm_output_state
*output_state,
state->ev = ev;
state->output = output;
drm_plane_state_coords_for_view(state, ev);
- if (state->src_w != state->dest_w << 16 ||
- state->src_h != state->dest_h << 16) {
+ if (!b->atomic_modeset &&
+ (state->src_w != state->dest_w << 16 ||
+state->src_h != state->dest_h << 16)) {
drm_plane_state_put_back(state);
continue;
}
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 05/14] compositor-drm: Support plane IN_FORMATS
From: Sergi Granell
The per-plane IN_FORMATS KMS property describes the format/modifier
combinations supported for display on this plane. Read and parse this
format, storing the data in each plane, so we can know which
combinations might work, and which combinations definitely will not
work.
Similarly to f11ec02cad40 ("compositor-drm: Extract overlay FB import to
helper"), we now use this when considering promoting a view to overlay
planes. If the framebuffer's modifier is definitely not supported by the
plane, we do not attempt to use that plane for that view.
This will also be used in a follow-patch, passing the list of modifiers
to GBM surface allocation to allow it to allocate more optimal buffers.
Signed-off-by: Sergi Granell
Reviewed-by: Daniel Stone
Tested-by: Emre Ucan
---
configure.ac | 3 +
libweston/compositor-drm.c | 134 ++---
2 files changed, 128 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index ef55ace36..c550198ae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -212,6 +212,9 @@ if test x$enable_drm_compositor = xyes; then
PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78],
[AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic
API])],
[AC_MSG_WARN([libdrm does not support atomic modesetting,
will omit that capability])])
+ PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83],
+ [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports
modifier advertisement])],
+ [AC_MSG_WARN([libdrm does not support modifier
advertisement])])
PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
[AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import
with modifiers])],
[AC_MSG_WARN([GBM does not support dmabuf import, will omit
that capability])])
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ae4a4a542..bc4022201 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -144,6 +144,7 @@ enum wdrm_plane_property {
WDRM_PLANE_CRTC_H,
WDRM_PLANE_FB_ID,
WDRM_PLANE_CRTC_ID,
+ WDRM_PLANE_IN_FORMATS,
WDRM_PLANE__COUNT
};
@@ -185,6 +186,7 @@ static const struct drm_property_info plane_props[] = {
[WDRM_PLANE_CRTC_H] = { .name = "CRTC_H", },
[WDRM_PLANE_FB_ID] = { .name = "FB_ID", },
[WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", },
+ [WDRM_PLANE_IN_FORMATS] = { .name = "IN_FORMATS" },
};
/**
@@ -448,7 +450,11 @@ struct drm_plane {
struct wl_list link;
- uint32_t formats[];
+ struct {
+ uint32_t format;
+ uint32_t count_modifiers;
+ uint64_t *modifiers;
+ } formats[];
};
struct drm_head {
@@ -2992,7 +2998,19 @@ drm_output_prepare_overlay_view(struct drm_output_state
*output_state,
/* Check whether the format is supported */
for (i = 0; i < p->count_formats; i++) {
- if (p->formats[i] == fb->format->format)
+ unsigned int j;
+
+ if (p->formats[i].format != fb->format->format)
+ continue;
+
+ if (fb->modifier == DRM_FORMAT_MOD_INVALID)
+ break;
+
+ for (j = 0; j < p->formats[i].count_modifiers; j++) {
+ if (p->formats[i].modifiers[j] == fb->modifier)
+ break;
+ }
+ if (j != p->formats[i].count_modifiers)
break;
}
if (i == p->count_formats)
@@ -3637,6 +3655,100 @@ init_pixman(struct drm_backend *b)
return pixman_renderer_init(b->compositor);
}
+#ifdef HAVE_DRM_FORMATS_BLOB
+static inline uint32_t *
+formats_ptr(struct drm_format_modifier_blob *blob)
+{
+ return (uint32_t *)(((char *)blob) + blob->formats_offset);
+}
+
+static inline struct drm_format_modifier *
+modifiers_ptr(struct drm_format_modifier_blob *blob)
+{
+ return (struct drm_format_modifier *)
+ (((char *)blob) + blob->modifiers_offset);
+}
+#endif
+
+/**
+ * Populates the plane's formats array, using either the IN_FORMATS blob
+ * property (if available), or the plane's format list if not.
+ */
+static int
+drm_plane_populate_formats(struct drm_plane *plane, const drmModePlane *kplane,
+ const drmModeObjectProperties *props)
+{
+ unsigned i;
+#ifdef HAVE_DRM_FORMATS_BLOB
+ drmModePropertyBlobRes *blob;
+ struct drm_format_modifier_blob *fmt_mod_blob;
+ struct drm_format_modifier *blob_modifiers;
+ uint32_t *blob_formats;
+ uint32_t blob_id;
+
+ blob_id = drm_property_get_value(>props[WDRM_PLANE_IN_FORMATS],
+props,
[PATCH v17 08/14] compositor-drm: Ignore views on other outputs
When we come to assign_planes, try very hard to ignore views which are
only visible on other outputs, rather than forcibly moving them to the
primary plane, which causes damage all round and unnecessary repaints.
Signed-off-by: Daniel Stone
Reviewed-by: Pekka Paalanen
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 2eb3d4073..ca885f96e 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1552,10 +1552,6 @@ drm_fb_get_from_view(struct drm_output_state *state,
struct weston_view *ev)
struct linux_dmabuf_buffer *dmabuf;
struct drm_fb *fb;
- /* Don't import buffers which span multiple outputs. */
- if (ev->output_mask != (1u << output->base.id))
- return NULL;
-
if (ev->alpha != 1.0f)
return NULL;
@@ -3125,10 +3121,6 @@ drm_output_prepare_cursor_view(struct drm_output_state
*output_state,
if (plane->state_cur->output && plane->state_cur->output != output)
return NULL;
- /* Don't import buffers which span multiple outputs. */
- if (ev->output_mask != (1u << output->base.id))
- return NULL;
-
/* We use GBM to import SHM buffers. */
if (b->gbm == NULL)
return NULL;
@@ -3286,6 +3278,16 @@ drm_output_propose_state(struct weston_output
*output_base,
wl_list_for_each(ev, _base->compositor->view_list, link) {
struct weston_plane *next_plane = NULL;
+ /* If this view doesn't touch our output at all, there's no
+* reason to do anything with it. */
+ if (!(ev->output_mask & (1u << output->base.id)))
+ continue;
+
+ /* We only assign planes to views which are exclusively present
+* on our output. */
+ if (ev->output_mask != (1u << output->base.id))
+ next_plane = primary;
+
/* Since we process views from top to bottom, we know that if
* the view intersects the calculated renderer region, it must
* be part of, or occluded by, it, and cannot go on a plane. */
@@ -3335,6 +3337,11 @@ drm_assign_planes(struct weston_output *output_base,
void *repaint_data)
wl_list_for_each(ev, _base->compositor->view_list, link) {
struct drm_plane *target_plane = NULL;
+ /* If this view doesn't touch our output at all, there's no
+* reason to do anything with it. */
+ if (!(ev->output_mask & (1u << output->base.id)))
+ continue;
+
/* Test whether this buffer can ever go into a plane:
* non-shm, or small enough to be a cursor.
*
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 00/14] DRM backend planes and modifiers
Hi, Another re-send after Pekka's comments on the modifiers patches. Patches 01-12 from the last series were merged after the last round. Patch 1 is new; patches 2 and 3 were extracted from patches 5 and 6. Patches 4-6 have otherwise seen changes after Pekka's review; all the comments were addressed. Patches 7-14 are unchanged from the previous submission, except where a rebase may have modified them. This iteration of the series is available from: https://gitlab.collabora.com/daniels/weston/commits/wip/2018-07/atomic-v17 The tree on GitLab includes a 'drm debug' patch at the top, which adds a lot of debug spew to the KMS backend. It is very useful to cherry-pick this patch and include any debug information when reporting bugs on this series, however the log volume is great enough that it can visibly slow the compositor. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 02/14] compositor-drm: Avoid cast by using unsigned loop index
ARRAY_LENGTH returns a size_t; rather than casting its result to
int so we can compare to our signed index variable, just declare the
index as a compatible type in the first place.
Signed-off-by: Daniel Stone
---
libweston/compositor-drm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index e09719ce4..7753dfba6 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -914,7 +914,7 @@ drm_fb_addfb(struct drm_fb *fb)
int ret = -EINVAL;
#ifdef HAVE_DRM_ADDFB2_MODIFIERS
uint64_t mods[4] = { };
- int i;
+ size_t i;
#endif
/* If we have a modifier set, we must only use the WithModifiers
@@ -923,7 +923,7 @@ drm_fb_addfb(struct drm_fb *fb)
/* KMS demands that if a modifier is set, it must be the same
* for all planes. */
#ifdef HAVE_DRM_ADDFB2_MODIFIERS
- for (i = 0; i < (int) ARRAY_LENGTH(mods) && fb->handles[i]; i++)
+ for (i = 0; i < ARRAY_LENGTH(mods) && fb->handles[i]; i++)
mods[i] = fb->modifier;
ret = drmModeAddFB2WithModifiers(fb->fd, fb->width, fb->height,
fb->format->format,
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 01/14] helpers: Add BUILD_BUG_ON from Linux kernel
Import the trivial definition of BUILD_BUG_ON() from the Linux kernel.
This evaluates an expression such that it will call sizeof() on a
negative-length array if the expression is false, generating a compiler
error.
This will be used in future patches to ensure two array lengths don't go
out of sync.
Signed-off-by: Daniel Stone
---
shared/helpers.h | 25 +
1 file changed, 25 insertions(+)
diff --git a/shared/helpers.h b/shared/helpers.h
index 46f745d1b..9ecc3c4e9 100644
--- a/shared/helpers.h
+++ b/shared/helpers.h
@@ -100,6 +100,31 @@ extern "C" {
(type *)( (char *)__mptr - offsetof(type,member) );})
#endif
+/**
+ * Build-time static assertion support
+ *
+ * A build-time equivalent to assert(), will generate a compilation error
+ * if the supplied condition does not evaluate true.
+ *
+ * The following example demonstrates use of BUILD_BUG_ON to ensure that
+ * arrays which are supposed to mirror each other have a consistent
+ * size.
+ *
+ * @code
+ * int small[4];
+ * long expanded[4];
+ *
+ * BUILD_BUG_ON(ARRAY_LENGTH(small) != ARRAY_LENGTH(expanded));
+ * for (i = 0; i < ARRAY_LENGTH(small); i++)
+ * expanded[i] = small[4];
+ * @endcode
+ *
+ * @param condition Expression to check for truth
+ */
+#ifndef BUILD_BUG_ON
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#endif
+
#ifdef __cplusplus
}
#endif
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 14/23] compositor-drm: Support plane IN_FORMATS
On Mon, 9 Jul 2018 13:42:45 +0100 Daniel Stone wrote: > Hi, > On Mon, 9 Jul 2018 at 11:08, Pekka Paalanen wrote: > > On Thu, 5 Jul 2018 18:16:41 +0100 Daniel Stone > > wrote: > > > The per-plane IN_FORMATS property adds information about format > > > modifiers; we can use these to both feed GBM with the set of modifiers > > > we want to use for rendering, and also as an early-out test when we're > > > trying to see if a FB will go on a particular plane. > > > > I can see the early-out test, but where does this patch affect feeding > > GBM? > > I've clarified the commit message. > > > > + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id); > > > + if (!blob) > > > + goto fallback; > > > + > > > + fmt_mod_blob = blob->data; > > > + blob_formats = formats_ptr(fmt_mod_blob); > > > + blob_modifiers = modifiers_ptr(fmt_mod_blob); > > > + > > > + assert(plane->count_formats == fmt_mod_blob->count_formats); > > > > I don't think this should be an assert. We are comparing to something > > the kernel just told us, so if they don't match then it is either a > > wrong assumption or a kernel bug. Which one is it? > > Almost: we're comparing the number of formats inside the format-blob > container, to the number of formats from drmModeGetPlane. Any mismatch > between them is a kernel bug, as it's an invariant of the API that the > two must be equal. Nothing we can do about it if so, but given the > result would be overwriting random bits from the heap, and that quite > a few driver developers use Weston as a test, leaving the assert in > seemed wise. > > I'm not hugely attached to it though, so even though I've left it in > for v17, do feel free to remove it when pushing. I mean it should be turned into a weston_log() and maybe a abort(). Asserts can be disabled, this should not be. Thanks, pq > > > + /* No IN_MODIFIERS blob available, so just use the old. */ > > > + for (i = 0; i < kplane->count_formats; i++) > > > + plane->formats[i].format = kplane->formats[i]; > > > + > > > + return 0; > > > +} > > > > Otherwise the patch looks fine. > > I've fixed up the references to 'IN_MODIFIERS' (sic: IN_FORMAT) whilst here. > > Cheers, > Daniel pgp2TC6sFOJzY.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 13/23] compositor-drm: Add modifiers to GBM dmabuf import
On Mon, 9 Jul 2018 13:25:50 +0100
Daniel Stone wrote:
> Hi Pekka,
>
> On Mon, 9 Jul 2018 at 11:22, Pekka Paalanen wrote:
> > On Mon, 9 Jul 2018 12:39:30 +0300 Pekka Paalanen
> > wrote:
> > > On Thu, 5 Jul 2018 18:16:40 +0100 Daniel Stone
> > > 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 ARGB into XRGB 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.
Hi,
I did not mean that a client changes the opaqueness *while* the FB is
up. I agree that that would be nonsense.
I meant a sequence more like this from a client:
- create buffer A
- draw buffer A
- set opaque region and commit buffer A
- create buffer B
- draw buffer B
- commit buffer B
- get release for buffer A
- draw buffer A with something transparent
- unset opaque region and commit buffer A
In the last step, if the compositor re-uses the FB it originally
created for buffer A, it will remain opaque contrary to what the client
intended.
A good client will wait for the release, so doesn't that prevent the
possibility of destroying the FB while on screen? If the compositor
checked the FB is not on screen, it could honour the legit opaqueness
change by destroying and re-creating the FB, while nonsensical
opaqueness changes (i.e. without content update or drawing into a
reserved buffer) would remain equally broken as they used to.
Anyway, it's good that the bug report exists, and I think that's
sufficient. I wanted to make sure this issue is known.
Thanks,
pq
pgpsBvKsfoOqI.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 14/23] compositor-drm: Support plane IN_FORMATS
Hi, On Mon, 9 Jul 2018 at 11:08, Pekka Paalanen wrote: > On Thu, 5 Jul 2018 18:16:41 +0100 Daniel Stone wrote: > > The per-plane IN_FORMATS property adds information about format > > modifiers; we can use these to both feed GBM with the set of modifiers > > we want to use for rendering, and also as an early-out test when we're > > trying to see if a FB will go on a particular plane. > > I can see the early-out test, but where does this patch affect feeding > GBM? I've clarified the commit message. > > + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id); > > + if (!blob) > > + goto fallback; > > + > > + fmt_mod_blob = blob->data; > > + blob_formats = formats_ptr(fmt_mod_blob); > > + blob_modifiers = modifiers_ptr(fmt_mod_blob); > > + > > + assert(plane->count_formats == fmt_mod_blob->count_formats); > > I don't think this should be an assert. We are comparing to something > the kernel just told us, so if they don't match then it is either a > wrong assumption or a kernel bug. Which one is it? Almost: we're comparing the number of formats inside the format-blob container, to the number of formats from drmModeGetPlane. Any mismatch between them is a kernel bug, as it's an invariant of the API that the two must be equal. Nothing we can do about it if so, but given the result would be overwriting random bits from the heap, and that quite a few driver developers use Weston as a test, leaving the assert in seemed wise. I'm not hugely attached to it though, so even though I've left it in for v17, do feel free to remove it when pushing. > > + /* No IN_MODIFIERS blob available, so just use the old. */ > > + for (i = 0; i < kplane->count_formats; i++) > > + plane->formats[i].format = kplane->formats[i]; > > + > > + return 0; > > +} > > Otherwise the patch looks fine. I've fixed up the references to 'IN_MODIFIERS' (sic: IN_FORMAT) whilst here. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] compositor-drm: Define DPMS property as an enum
On Fri, 6 Jul 2018 11:36:49 +0100
Daniel Stone wrote:
> The DPMS connector property is an enum property in KMS, which made our
> property handling complain at startup as we weren't defining its enums.
> Fix our definition so we parse the enum values.
>
> The only user of the property is the legacy path, which can continue
> using fixed values as those values are part of the KMS ABI. The atomic
> path does not need any changes, since atomic uses routing and CRTC
> active to determine the connector's power state, rather than a property.
>
> Signed-off-by: Daniel Stone
> Fixes: https://gitlab.freedesktop.org/wayland/weston/issues/125
> ---
> libweston/compositor-drm.c | 41 +-
> 1 file changed, 32 insertions(+), 9 deletions(-)
Nice! Pushed:
79e95cb9..7625577a master -> master
Thanks,
pq
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 7dcc634c6..0de6d6ba0 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -186,9 +186,36 @@ enum wdrm_connector_property {
> WDRM_CONNECTOR__COUNT
> };
>
> +enum wdrm_dpms_state {
> + WDRM_DPMS_STATE_OFF = 0,
> + WDRM_DPMS_STATE_ON,
> + WDRM_DPMS_STATE_STANDBY, /* unused */
> + WDRM_DPMS_STATE_SUSPEND, /* unused */
> + WDRM_DPMS_STATE__COUNT
> +};
> +
> +static struct drm_property_enum_info dpms_state_enums[] = {
> + [WDRM_DPMS_STATE_OFF] = {
> + .name = "Off",
> + },
> + [WDRM_DPMS_STATE_ON] = {
> + .name = "On",
> + },
> + [WDRM_DPMS_STATE_STANDBY] = {
> + .name = "Standby",
> + },
> + [WDRM_DPMS_STATE_SUSPEND] = {
> + .name = "Suspend",
> + },
> +};
> +
> static const struct drm_property_info connector_props[] = {
> [WDRM_CONNECTOR_EDID] = { .name = "EDID" },
> - [WDRM_CONNECTOR_DPMS] = { .name = "DPMS" },
> + [WDRM_CONNECTOR_DPMS] = {
> + .name = "DPMS",
> + .enum_values = dpms_state_enums,
> + .num_enum_values = WDRM_DPMS_STATE__COUNT,
> + },
> [WDRM_CONNECTOR_CRTC_ID] = { .name = "CRTC_ID", },
> };
>
> @@ -2372,6 +2399,8 @@ drm_output_apply_state_atomic(struct drm_output_state
> *state,
>current_mode->blob_id);
> ret |= crtc_add_prop(req, output, WDRM_CRTC_ACTIVE, 1);
>
> + /* No need for the DPMS property, since it is implicit in
> + * routing and CRTC activity. */
> wl_list_for_each(head, >base.head_list,
> base.output_link) {
> ret |= connector_add_prop(req, head,
> WDRM_CONNECTOR_CRTC_ID,
> output->crtc_id);
> @@ -2380,6 +2409,8 @@ drm_output_apply_state_atomic(struct drm_output_state
> *state,
> ret |= crtc_add_prop(req, output, WDRM_CRTC_MODE_ID, 0);
> ret |= crtc_add_prop(req, output, WDRM_CRTC_ACTIVE, 0);
>
> + /* No need for the DPMS property, since it is implicit in
> + * routing and CRTC activity. */
> wl_list_for_each(head, >base.head_list,
> base.output_link)
> ret |= connector_add_prop(req, head,
> WDRM_CONNECTOR_CRTC_ID, 0);
> }
> @@ -2463,14 +2494,6 @@ drm_pending_state_apply_atomic(struct
> drm_pending_state *pending_state,
> info->prop_id, 0);
> if (err <= 0)
> ret = -1;
> -
> - info = >props_conn[WDRM_CONNECTOR_DPMS];
> - if (info->prop_id > 0)
> - err = drmModeAtomicAddProperty(req,
> head->connector_id,
> -info->prop_id,
> -
> DRM_MODE_DPMS_OFF);
> - if (err <= 0)
> - ret = -1;
> }
>
> wl_array_for_each(unused, >unused_crtcs) {
pgpklgCI2ASxa.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 13/23] compositor-drm: Add modifiers to GBM dmabuf import
Hi Pekka, On Mon, 9 Jul 2018 at 10:39, Pekka Paalanen wrote: > On Thu, 5 Jul 2018 18:16:40 +0100 > Daniel Stone wrote: > > +/* XXX: TODO: > > + * > > + * Currently the buffer is rejected if any dmabuf attribute > > + * flag is set. This keeps us from passing an inverted / > > + * interlaced / bottom-first buffer (or any other type that may > > + * be added in the future) through to an overlay. Ultimately, > > + * these types of buffers should be handled through buffer > > + * transforms and not as spot-checks requiring specific > > + * knowledge. */ > > Bad whitespace in the above indentation. > > I'm also not sure I agree with how things should be done instead. It's > possible the dmabuf wl_buffer is created by a library (libEGL, media, > etc.) which may not communicate these details outside, may not give a > possibility to add state to the wl_surface.commit, or may not actually > drive wl_surface itself. There could be many client-side designs where > this detail is not combinable with wl_surface state in the client. > > Also, only y-inverted would be possible to deal with buffer transform. > The other flags probably do require specific knowledge if they were to > be implemented properly. > > Hmm, it's an interesting question how buffer_damage should be > interpreted in case of e.g. y-inverted dmabuf. Micah wrote the above comment, which I'm just transporting from one place to another. My interpretation of what he wrote is _not_ that the client should use protocol calls to add a transformation, but that setting the flag should result in a transformation in Weston core, rather than being checked and accounted for directly in the end users of that buffer. I probably read it like that because I agree with it. ;) Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] man: Remove description of DRM specific mode-options from weston.ini.man
On Fri, 6 Jul 2018 16:18:39 +0530 "Nautiyal, Ankit K" wrote: > From: Ankit Nautiyal > > The weston.ini.man describes the mode-formats that a user can specify > for selecting a video mode. The DRM specific examples are already > provided in weston-drm.man, so this inofrmation is redundant and can > be removed. > > This patch removes the DRM specific mode option details from the > description of mode configuration in weston.ini.man. > A pointer to weston-drm.man is given, which has complete information > about the mode-format options supported by DRM backend. > > Signed-off-by: Ankit Nautiyal > --- > man/weston.ini.man | 30 +- > 1 file changed, 5 insertions(+), 25 deletions(-) R-b me and pushed: 65a4dbcc..79e95cb9 master -> master Thanks, pq > > diff --git a/man/weston.ini.man b/man/weston.ini.man > index 02c9b03..199b465 100644 > --- a/man/weston.ini.man > +++ b/man/weston.ini.man > @@ -406,31 +406,11 @@ for more details. > sets the output mode (string). The mode parameter is handled differently > depending on the backend. On the X11 backend, it just sets the WIDTHxHEIGHT > of > the weston window. > -The DRM backend accepts different modes: > -.PP > -.RS 10 > -.nf > -.BR "WIDTHxHEIGHT" "Resolution size width and height in pixels" > -.BR "preferred " "Uses the preferred mode" > -.BR "current " "Uses the current crt controller mode" > -.BR "off " "Disables the output" > -.fi > -.RE > -.RS > -.PP > -Optionally, a user may specify a modeline, such as: > -.PP > -.nf > -.in +4n > -.nf > -173.00 1920 2048 2248 2576 1080 1083 1088 1120 -hsync +vsync > -.fi > -.in > -.PP > -It consists of the refresh rate in Hz, horizontal and vertical resolution, > -options for horizontal and vertical synchronisation. The program > -.B "cvt(1)" > -can provide suitable modeline string. > +The DRM backend accepts different modes, along with an option of a modeline > string. > + > +See > +.B "weston-drm(7)" > +for examples of modes-formats supported by DRM backend. > .RE > .TP 7 > .BI "transform=" normal pgpzOzApuT2EE.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 15/23] compositor-drm: Support modifiers with GBM
On Thu, 5 Jul 2018 18:16:42 +0100
Daniel Stone wrote:
> Use the extended GBM allocator interface to support modifiers and
> multi-planar BOs.
>
> Signed-off-by: Daniel Stone
> Tested-by: Emre Ucan
> ---
> configure.ac | 3 ++
> libweston/compositor-drm.c | 61 +++---
> 2 files changed, 53 insertions(+), 11 deletions(-)
>
Hi,
I feel there are two parts in this patch which are not clearly
discussed in the commit message. The changes to drm_fb_get_from_bo()
affect all three of BUFFER_CLIENT, BUFFER_GBM_SURFACE and BUFFER_CURSOR
cases. The changes to drm_output_init_egl() affect only the
BUFFER_GBM_SURFACE case.
If I understand right, this mainly affects GBM import from wl_buffer
and GBM surface use cases.
Is this the final patch that makes etnaviv work? That would be nice to
mention.
> diff --git a/configure.ac b/configure.ac
> index c550198ae..357b6471e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -215,6 +215,9 @@ if test x$enable_drm_compositor = xyes; then
>PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83],
> [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports
> modifier advertisement])],
> [AC_MSG_WARN([libdrm does not support modifier
> advertisement])])
> + PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM_MODIFIERS, [gbm >= 17.1],
> + [AC_DEFINE([HAVE_GBM_MODIFIERS], 1, [GBM supports
> modifiers])],
> + [AC_MSG_WARN([GBM does not support modifiers])])
>PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
> [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import
> with modifiers])],
> [AC_MSG_WARN([GBM does not support dmabuf import, will omit
> that capability])])
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 3f8e77554..3471d9b0f 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -871,7 +871,7 @@ drm_fb_addfb(struct drm_fb *fb)
> int ret = -EINVAL;
> #ifdef HAVE_DRM_ADDFB2_MODIFIERS
> uint64_t mods[4] = { };
> - int i;
> + size_t i;
> #endif
>
> /* If we have a modifier set, we must only use the WithModifiers
> @@ -880,7 +880,7 @@ drm_fb_addfb(struct drm_fb *fb)
> /* KMS demands that if a modifier is set, it must be the same
>* for all planes. */
> #ifdef HAVE_DRM_ADDFB2_MODIFIERS
> - for (i = 0; i < (int) ARRAY_LENGTH(mods) && fb->handles[i]; i++)
> + for (i = 0; i < ARRAY_LENGTH(mods) && fb->handles[i]; i++)
This is an unrelated change and would ideally be in a separate patch so
it could land ahead of time.
> mods[i] = fb->modifier;
> ret = drmModeAddFB2WithModifiers(fb->fd, fb->width, fb->height,
>fb->format->format,
> @@ -1114,6 +1114,9 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct
> drm_backend *backend,
> bool is_opaque, enum drm_fb_type type)
> {
> struct drm_fb *fb = gbm_bo_get_user_data(bo);
> +#ifdef HAVE_GBM_MODIFIERS
> + int i;
> +#endif
>
> if (fb) {
> assert(fb->type == type);
> @@ -1127,15 +1130,25 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct
> drm_backend *backend,
> fb->type = type;
> fb->refcnt = 1;
> fb->bo = bo;
> + fb->fd = backend->drm.fd;
>
> fb->width = gbm_bo_get_width(bo);
> fb->height = gbm_bo_get_height(bo);
> + fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
> + fb->size = 0;
Making the size zero should have a rationale in the commit message.
> +
> +#ifdef HAVE_GBM_MODIFIERS
> + fb->modifier = gbm_bo_get_modifier(bo);
> + for (i = 0; i < gbm_bo_get_plane_count(bo); i++) {
> + fb->strides[i] = gbm_bo_get_stride_for_plane(bo, i);
> + fb->handles[i] = gbm_bo_get_handle_for_plane(bo, i).u32;
> + fb->offsets[i] = gbm_bo_get_offset(bo, i);
> + }
> +#else
> fb->strides[0] = gbm_bo_get_stride(bo);
> fb->handles[0] = gbm_bo_get_handle(bo).u32;
> - fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
> fb->modifier = DRM_FORMAT_MOD_INVALID;
> - fb->size = fb->strides[0] * fb->height;
> - fb->fd = backend->drm.fd;
> +#endif
>
> if (!fb->format) {
> weston_log("couldn't look up format 0x%lx\n",
> @@ -4212,13 +4225,39 @@ drm_output_init_egl(struct drm_output *output, struct
> drm_backend *b)
> fallback_format_for(output->gbm_format),
> };
> int n_formats = 1;
> + struct weston_mode *mode = output->base.current_mode;
> + struct drm_plane *plane = output->scanout_plane;
> + unsigned int i;
> +
> + for (i = 0; i < plane->count_formats; i++) {
> + if (plane->formats[i].format == output->gbm_format)
> + break;
> + }
> +
> + if (i == plane->count_formats) {
> +
Re: [PATCH v16 13/23] compositor-drm: Add modifiers to GBM dmabuf import
On Mon, 9 Jul 2018 12:39:30 +0300
Pekka Paalanen wrote:
> On Thu, 5 Jul 2018 18:16:40 +0100
> Daniel Stone wrote:
>
> > Add support for the GBM_BO_IMPORT_FD_MODIFIER path, which allows us to
> > import multi-plane dmabufs, as well as format modifiers.
> >
> > Signed-off-by: Daniel Stone
> > Tested-by: Emre Ucan
> > ---
> > configure.ac | 6 +-
> > libweston/compositor-drm.c | 181 +++--
> > 2 files changed, 137 insertions(+), 50 deletions(-)
> > +
> > +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.
Thanks,
pq
pgpnmFuL6LiNC.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 14/23] compositor-drm: Support plane IN_FORMATS
On Thu, 5 Jul 2018 18:16:41 +0100
Daniel Stone wrote:
> From: Sergi Granell
>
> The per-plane IN_FORMATS property adds information about format
> modifiers; we can use these to both feed GBM with the set of modifiers
> we want to use for rendering, and also as an early-out test when we're
> trying to see if a FB will go on a particular plane.
Hi,
I can see the early-out test, but where does this patch affect feeding
GBM?
>
> Signed-off-by: Sergi Granell
> Reviewed-by: Daniel Stone
> Tested-by: Emre Ucan
> ---
> configure.ac | 3 +
> libweston/compositor-drm.c | 125 ++---
> 2 files changed, 119 insertions(+), 9 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index ef55ace36..c550198ae 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -212,6 +212,9 @@ if test x$enable_drm_compositor = xyes; then
>PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78],
> [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic
> API])],
> [AC_MSG_WARN([libdrm does not support atomic modesetting,
> will omit that capability])])
> + PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83],
> + [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports
> modifier advertisement])],
> + [AC_MSG_WARN([libdrm does not support modifier
> advertisement])])
>PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
> [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import
> with modifiers])],
> [AC_MSG_WARN([GBM does not support dmabuf import, will omit
> that capability])])
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 98c8ed584..3f8e77554 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -131,6 +131,7 @@ enum wdrm_plane_property {
> WDRM_PLANE_CRTC_H,
> WDRM_PLANE_FB_ID,
> WDRM_PLANE_CRTC_ID,
> + WDRM_PLANE_IN_FORMATS,
> WDRM_PLANE__COUNT
> };
>
> @@ -172,6 +173,7 @@ static const struct drm_property_info plane_props[] = {
> [WDRM_PLANE_CRTC_H] = { .name = "CRTC_H", },
> [WDRM_PLANE_FB_ID] = { .name = "FB_ID", },
> [WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", },
> + [WDRM_PLANE_IN_FORMATS] = { .name = "IN_FORMATS" },
> };
>
> /**
> @@ -406,7 +408,11 @@ struct drm_plane {
>
> struct wl_list link;
>
> - uint32_t formats[];
> + struct {
> + uint32_t format;
> + uint32_t count_modifiers;
> + uint64_t *modifiers;
> + } formats[];
> };
>
> struct drm_head {
> @@ -2908,7 +2914,19 @@ drm_output_prepare_overlay_view(struct
> drm_output_state *output_state,
>
> /* Check whether the format is supported */
> for (i = 0; i < p->count_formats; i++) {
> - if (p->formats[i] == fb->format->format)
> + unsigned int j;
> +
> + if (p->formats[i].format != fb->format->format)
> + continue;
> +
> + if (fb->modifier == DRM_FORMAT_MOD_INVALID)
> + break;
> +
> + for (j = 0; j < p->formats[i].count_modifiers; j++) {
> + if (p->formats[i].modifiers[j] == fb->modifier)
> + break;
> + }
> + if (j != p->formats[i].count_modifiers)
> break;
> }
> if (i == p->count_formats)
> @@ -3505,6 +3523,91 @@ init_pixman(struct drm_backend *b)
> return pixman_renderer_init(b->compositor);
> }
>
> +#ifdef HAVE_DRM_FORMATS_BLOB
> +static inline uint32_t *
> +formats_ptr(struct drm_format_modifier_blob *blob)
> +{
> + return (uint32_t *)(((char *)blob) + blob->formats_offset);
> +}
> +
> +static inline struct drm_format_modifier *
> +modifiers_ptr(struct drm_format_modifier_blob *blob)
> +{
> + return (struct drm_format_modifier *)
> + (((char *)blob) + blob->modifiers_offset);
> +}
> +#endif
> +
> +/**
> + * Populates the plane's formats array, using either the IN_MODIFIERS blob
> + * property (if available), or the plane's format list if not.
> + */
> +static int
> +drm_plane_populate_formats(struct drm_plane *plane, const drmModePlane
> *kplane,
> +const drmModeObjectProperties *props)
> +{
> + unsigned i;
> +#ifdef HAVE_DRM_FORMATS_BLOB
> + drmModePropertyBlobRes *blob;
> + struct drm_format_modifier_blob *fmt_mod_blob;
> + struct drm_format_modifier *blob_modifiers;
> + uint32_t *blob_formats;
> + uint32_t blob_id;
> +
> + blob_id = drm_property_get_value(>props[WDRM_PLANE_IN_FORMATS],
> + props,
> + 0);
> + if (blob_id == 0)
> + goto fallback;
> +
> + blob =
Re: [PATCH v16 13/23] compositor-drm: Add modifiers to GBM dmabuf import
On Thu, 5 Jul 2018 18:16:40 +0100
Daniel Stone wrote:
> Add support for the GBM_BO_IMPORT_FD_MODIFIER path, which allows us to
> import multi-plane dmabufs, as well as format modifiers.
>
> Signed-off-by: Daniel Stone
> Tested-by: Emre Ucan
> ---
> configure.ac | 6 +-
> libweston/compositor-drm.c | 181 +++--
> 2 files changed, 137 insertions(+), 50 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index adb998dda..ef55ace36 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -212,9 +212,9 @@ if test x$enable_drm_compositor = xyes; then
>PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78],
> [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic
> API])],
> [AC_MSG_WARN([libdrm does not support atomic modesetting,
> will omit that capability])])
> - PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 10.2],
> - [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports dmabuf
> import])],
> - [AC_MSG_WARN([gbm does not support dmabuf import, will omit
> that capability])])
> + PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
> + [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import
> with modifiers])],
> + [AC_MSG_WARN([GBM does not support dmabuf import, will omit
> that capability])])
> fi
>
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 0aaaf79e8..98c8ed584 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -284,6 +284,7 @@ struct drm_mode {
> enum drm_fb_type {
> BUFFER_INVALID = 0, /**< never used */
> BUFFER_CLIENT, /**< directly sourced from client */
> + BUFFER_DMABUF, /**< imported from linux_dmabuf client */
> BUFFER_PIXMAN_DUMB, /**< internal Pixman rendering */
> BUFFER_GBM_SURFACE, /**< internal EGL rendering */
> BUFFER_CURSOR, /**< internal cursor buffer */
> @@ -988,6 +989,120 @@ drm_fb_ref(struct drm_fb *fb)
> return fb;
> }
>
> +static void
> +drm_fb_destroy_dmabuf(struct drm_fb *fb)
> +{
> + /* We deliberately do not close the GEM handles here; GBM manages
> + * their lifetime through the BO. */
> + gbm_bo_destroy(fb->bo);
Hi,
gbm_bo_destroy() may be called with NULL. Is that ok?
> + drm_fb_destroy(fb);
> +}
> +
> +static struct drm_fb *
> +drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf,
> +struct drm_backend *backend, bool is_opaque)
> +{
> +#ifdef HAVE_GBM_FD_IMPORT
> + struct drm_fb *fb;
> + struct gbm_import_fd_data import_legacy = {
> + .width = dmabuf->attributes.width,
> + .height = dmabuf->attributes.height,
> + .format = dmabuf->attributes.format,
> + .stride = dmabuf->attributes.stride[0],
> + .fd = dmabuf->attributes.fd[0],
> + };
> + struct gbm_import_fd_modifier_data import_mod = {
> + .width = dmabuf->attributes.width,
> + .height = dmabuf->attributes.height,
> + .format = dmabuf->attributes.format,
> + .num_fds = dmabuf->attributes.n_planes,
> + .modifier = dmabuf->attributes.modifier[0],
> + };
> + int i;
> +
> +/* XXX: TODO:
> + *
> + * Currently the buffer is rejected if any dmabuf attribute
> + * flag is set. This keeps us from passing an inverted /
> + * interlaced / bottom-first buffer (or any other type that may
> + * be added in the future) through to an overlay. Ultimately,
> + * these types of buffers should be handled through buffer
> + * transforms and not as spot-checks requiring specific
> + * knowledge. */
Bad whitespace in the above indentation.
I'm also not sure I agree with how things should be done instead. It's
possible the dmabuf wl_buffer is created by a library (libEGL, media,
etc.) which may not communicate these details outside, may not give a
possibility to add state to the wl_surface.commit, or may not actually
drive wl_surface itself. There could be many client-side designs where
this detail is not combinable with wl_surface state in the client.
Also, only y-inverted would be possible to deal with buffer transform.
The other flags probably do require specific knowledge if they were to
be implemented properly.
Hmm, it's an interesting question how buffer_damage should be
interpreted in case of e.g. y-inverted dmabuf.
> + if (dmabuf->attributes.flags)
> + return NULL;
> +
> + fb = zalloc(sizeof *fb);
> + if (fb == NULL)
> + return NULL;
> +
> + fb->refcnt = 1;
> + fb->type = BUFFER_DMABUF;
> +
> + memcpy(import_mod.fds, dmabuf->attributes.fd, sizeof(import_mod.fds));
> + memcpy(import_mod.strides, dmabuf->attributes.stride,
> +sizeof(import_mod.fds));
> + memcpy(import_mod.offsets, dmabuf->attributes.offset,
> +
Re: [PATCH wayland-protocols] xdg-output: add a transform example for the logical size
Bump: this is the source of bugs in xwayland [1]. [1]: https://lists.x.org/archives/xorg-devel/2018-July/057285.html On May 18, 2018 9:40 PM, Simon Ser wrote: > Signed-off-by: Simon Ser > --- > > I believe it's easy to understand that transformations are not applied to > the logical size. > > unstable/xdg-output/xdg-output-unstable-v1.xml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml > b/unstable/xdg-output/xdg-output-unstable-v1.xml > index bd9a8a8..320c0a0 100644 > --- a/unstable/xdg-output/xdg-output-unstable-v1.xml > +++ b/unstable/xdg-output/xdg-output-unstable-v1.xml > @@ -135,6 +135,9 @@ > - A compositor using a fractional scale of 1.5 will advertise a > logical size to 2560×1620. > > + For example, for a wl_output mode 1920×1080 and a 90 degree rotation, > the > + compositor will advertise a logical size of 1080x1920. > + > The logical_size event is sent after creating an xdg_output > (see xdg_output_manager.get_xdg_output) and whenever the logical > size of the output changes, either as a result of a change in the > -- > 2.17.0 > > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
