Re: [RFC wayland-protocols] unstable: add protocol to give focus to a foreign surface
On 10 July 2018 6:03:26 pm AEST, David Edmundson wrote: >>Hm. If you wanted to, you could make this explicit by requiring an >event serial in the export_surface request rather than the wl_surface. > >Certainly an option. Though I'm not sure we have a use case of needing >to limit a client releasing its focus? I can't think of one offhand. I just prefer the explicitness of “*this* user action caused us to request focus”. That's just for delegating focus, though. As Markus suggested, this *could* be a special case of a more general interface. We had this handle as MirCookie in Mir, and had a bunch of interfaces which requires a cookie. It turned out that a transferable token containing an unforgeable timestamp of a user interaction was a useful primitive. >>Does this interface need to exist? > >It doesn't /need/ to, but I need to be able to export a handle >multiple times and it's nice in the client to be able to match up >requests with the reply. This is also an advantage of the handle-for-serial style interface - the reply event can contain both the handle and the serial it's for, eliminating this otherwise superfluous interface. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v19 08/10] compositor-drm: Incrementally test plane states in mixed mode
In the plane-only mode, we try to place every view on a hardware plane,
and fail if we can't do this. This requires a full walk of the scene
graph to come up with a complete configuration in order to be able to
test.
In mixed mode, we know at least some visible views will fail to be
promoted to planes and must be composited via the renderer. In order to
still use some planes where possible, we use atomic modesetting's
test-only mode to incrementally test configurations.
We know that the renderer output will always be visible, and because it
is the renderer, that it will be occupying the scanout plane underneath
everything else. The actual renderer buffer doesn't materialise until
after assign_planes, because it cannot know what to render until then.
However, in order to test whether a configuration is valid, we need the
renderer buffer in the scanout plane. For testing, we fake this by
temporarily stealing the old buffer - if it seems sufficiently
compatible - and placing it in the state we construct. This is used to
test whether or not a renderer buffer will work with the addition of
overlay planes.
Doing this incremental testing will allow us to enable plane usage for
atomic by default, since we know ahead of time that our chosen plane
configuration will work.
Signed-off-by: Daniel Stone
---
libweston/compositor-drm.c | 133 -
1 file changed, 102 insertions(+), 31 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 3a627dd34..ab3eea6cf 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1949,14 +1949,17 @@ 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_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;
struct drm_fb *fb;
pixman_box32_t *extents;
+ int ret;
assert(!b->sprites_are_broken);
@@ -1983,11 +1986,20 @@ 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 we already have another client buffer, then we can't use this
+* plane. */
+ 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;
}
@@ -2007,10 +2019,26 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
+ /* In plane-only mode, we don't need to test the state now, as we
+* will only test it once at the end. */
+ 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;
}
@@ -2078,7 +2106,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 &&
+
[PATCH v19 07/10] compositor-drm: Add planes-only mode to state proposal
Add a new mode, which attempts to construct a scene exclusively using
planes. This is a building block for incrementally testing and
constructing state: in the plane-only mode, we test the state exactly
once, when we have constructed a full set of planes and want to know if
it works or not.
When using the renderer, we need to incrementally test views one by one
to see if they will work on planes, falling back to the renderer if not.
This test is different, since the scanout plane will be occupied by the
renderer's buffer. Testing using the renderer or client buffers may have
completely different characteristics, so we need two passes: first,
constructing a state with only planes and testing if that succeeds,
falling back later to a mixed renderer/plane mode which tests
incrementally.
This implements the first mode, and preferentially attempts to use it.
Signed-off-by: Daniel Stone
---
libweston/compositor-drm.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 282d6d67f..3a627dd34 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,6 +1944,7 @@ 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_RENDERER_ONLY, /**< only assign to renderer &
cursor */
+ DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< no renderer use, only planes
*/
};
static struct drm_plane_state *
@@ -3296,6 +3297,7 @@ drm_output_propose_state(struct weston_output
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
+ bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
int ret;
assert(!output->state_last);
@@ -3387,6 +3389,11 @@ drm_output_propose_state(struct weston_output
*output_base,
if (!drm_view_is_opaque(ev))
force_renderer = true;
+ if (force_renderer && !renderer_ok) {
+ pixman_region32_fini(_view);
+ goto err;
+ }
+
if (!force_renderer && !ps)
ps = drm_output_prepare_scanout_view(state, ev);
if (!force_renderer && !ps)
@@ -3409,6 +3416,14 @@ drm_output_propose_state(struct weston_output
*output_base,
continue;
}
+ /* We have been assigned to the primary (renderer) plane:
+* check if this is OK, and add ourselves to the renderer
+* region if so. */
+ if (!renderer_ok) {
+ pixman_region32_fini(_view);
+ goto err;
+ }
+
pixman_region32_union(_region,
_region,
_view);
@@ -3441,16 +3456,21 @@ drm_assign_planes(struct weston_output *output_base,
void *repaint_data)
struct drm_plane_state *plane_state;
struct weston_view *ev;
struct weston_plane *primary = _base->compositor->primary_plane;
- enum drm_output_propose_state_mode mode;
- if (!b->sprites_are_broken)
+ if (!b->sprites_are_broken) {
state = drm_output_propose_state(output_base, pending_state,
-
DRM_OUTPUT_PROPOSE_STATE_MIXED);
+
DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
+ if (!state)
+ state = drm_output_propose_state(output_base,
pending_state,
+
DRM_OUTPUT_PROPOSE_STATE_MIXED);
+ }
if (!state)
state = drm_output_propose_state(output_base, pending_state,
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
+ assert(state);
+
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 v19 10/10] 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
Reviewed-by: Pekka Paalanen
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 a364fb5df..7fe14ae5e 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3826,6 +3826,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",
@@ -6706,17 +6717,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 v19 01/10] compositor-drm: Disallow overlapping overlay planes
The scanout plane strictly stacks under all overlay planes, and the cursor plane above. However, the stacking of overlay planes with respect to each other is undefined. We can control the stacking order of overlay planes with the zpos property, though this significantly complicates plane assignment. In the meantime, simply disallow assigning a view to an overlay, when it overlaps another view which is already on an overlay. This ensures stacking order is irrelevant, since the planes never intersect each other. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen --- libweston/compositor-drm.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 51b2482d9..2305f708f 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3324,6 +3324,16 @@ drm_output_propose_state(struct weston_output *output_base, _view); if (pixman_region32_not_empty(_overlap)) next_plane = primary; + + /* We do not control the stacking order of overlay planes; +* the scanout plane is strictly stacked bottom and the cursor +* plane top, but the ordering of overlay planes with respect +* to each other is undefined. Make sure we do not have two +* planes overlapping each other. */ + pixman_region32_intersect(_overlap, _region, + _view); + if (pixman_region32_not_empty(_overlap)) + next_plane = primary; pixman_region32_fini(_overlap); if (next_plane == NULL) -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v19 06/10] compositor-drm: Never lift solid surfaces to planes
This will never work, so don't even try to do it. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index f7a364837..282d6d67f 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3337,6 +3337,9 @@ drm_output_propose_state(struct weston_output *output_base, if (ev->output_mask != (1u << output->base.id)) force_renderer = true; + if (!ev->surface->buffer_ref.buffer) + force_renderer = true; + /* Ignore views we know to be totally occluded. */ pixman_region32_init(_view); pixman_region32_intersect(_view, -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v19 05/10] 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. Use this when we construct a plane configuration, to
see if it has a chance of ever working. If not, we can fail
assign_planes early.
This will be used in later patches to incrementally build state by
proposing and testing potential configurations one at a time.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 28a580cd2..f7a364837 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 {
@@ -1822,6 +1823,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
@@ -2610,9 +2612,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:
+ flags |= DRM_MODE_ATOMIC_TEST_ONLY;
+ break;
}
ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
+
+ if (mode == DRM_STATE_TEST_ONLY)
+ return ret;
+
if (ret != 0) {
weston_log("atomic: couldn't commit new state: %m\n");
goto out;
@@ -2640,6 +2649,22 @@ out:
*
* Unconditionally takes ownership of pending_state, and clears state_invalid.
*/
+static int
+drm_pending_state_test(struct drm_pending_state *pending_state)
+{
+#ifdef HAVE_DRM_ATOMIC
+ struct drm_backend *b = pending_state->backend;
+
+ if (b->atomic_modeset)
+ return drm_pending_state_apply_atomic(pending_state,
+ DRM_STATE_TEST_ONLY);
+#endif
+
+ /* We have no way to test state before application on the legacy
+* modesetting API, so just claim it succeeded. */
+ return 0;
+}
+
static int
drm_pending_state_apply(struct drm_pending_state *pending_state)
{
@@ -3271,6 +3296,7 @@ drm_output_propose_state(struct weston_output
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
+ int ret;
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3388,7 +3414,18 @@ drm_output_propose_state(struct weston_output
*output_base,
pixman_region32_fini(_region);
pixman_region32_fini(_region);
+ /* Check to see if this state will actually work. */
+ ret = drm_pending_state_test(state->pending_state);
+ if (ret != 0)
+ goto err;
+
return state;
+
+err:
+ pixman_region32_fini(_region);
+ pixman_region32_fini(_region);
+ drm_output_state_free(state);
+ return NULL;
}
static void
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v19 09/10] 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
Reviewed-by: Pekka Paalanen
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ab3eea6cf..a364fb5df 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1980,7 +1980,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;
}
@@ -2010,15 +2010,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;
+
/* In plane-only mode, we don't need to test the state now, as we
* will only test it once at the end. */
if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
@@ -3113,8 +3116,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 v19 02/10] compositor-drm: Use sprites_are_broken for scanout plane
When the sprites_are_broken variable is set, do not attempt to promote
client surfaces to the scanout plane.
We are currently assuming that every client buffer will be compatible
with the scanout plane, but that is not the case, particularly with more
exotic tiled/compressed buffers. Once we promote the client buffer to
scanout, there is no going back: if the repaint fails, we do not mark
this as failed and go back to repaint through composition.
This permanently removes the ability for scanout bypass when using the
non-atomic path. Future patches lift the restriction when using atomic
modesetting, as we can actually test and ensure that the view is
compatible with scanout.
Signed-off-by: Daniel Stone
Reported-by: Pekka Paalanen
---
libweston/compositor-drm.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 2305f708f..d045778aa 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,11 +1944,14 @@ drm_output_prepare_scanout_view(struct drm_output_state
*output_state,
struct weston_view *ev)
{
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_fb *fb;
pixman_box32_t *extents;
+ assert(!b->sprites_are_broken);
+
/* Check the view spans exactly the output size, calculated in the
* logical co-ordinate space. */
extents = pixman_region32_extents(>transform.boundingbox);
@@ -3004,8 +3007,7 @@ drm_output_prepare_overlay_view(struct drm_output_state
*output_state,
struct drm_fb *fb;
unsigned int i;
- if (b->sprites_are_broken)
- return NULL;
+ assert(!b->sprites_are_broken);
fb = drm_fb_get_from_view(output_state, ev);
if (!fb)
@@ -3260,6 +3262,7 @@ drm_output_propose_state(struct weston_output
*output_base,
struct drm_pending_state *pending_state)
{
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;
@@ -3342,6 +3345,9 @@ drm_output_propose_state(struct weston_output
*output_base,
if (next_plane == NULL && !drm_view_is_opaque(ev))
next_plane = primary;
+ if (next_plane == NULL && b->sprites_are_broken)
+ next_plane = primary;
+
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(state, ev);
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v19 10/10] DRM backend planes
Hi, I believe this should resolve all the issues Pekka pointed out in the (too hurried) v18. It's also been split up into a few different patches with much more detailed commit messages, so should be much easier to reason about and review. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v19 03/10] compositor-drm: Add modes to drm_output_propose_state
Add support for multiple modes to drm_output_propose_state. Currently we
intend to operate in three modes: planes-only (no renderer buffer,
client buffers in planes only), mixed-mode (promote client buffers to
planes where possible, falling back to the renderer where not), and
renderer-only (no plane usage at all).
We want to use the first (planes-only) mode where possible: it can avoid
us having to allocate buffers for the renderer, and it also gives us the
best chance of the optimal configuration, with no composition. In this
mode, we walk the scene looking at all views, trying to put them in
planes, and failing as soon as we find a view we cannot place in a
plane.
In the second mode, rather than failing, we assign those views which
cannot be on a plane to the renderer, and allow the renderer to
composite them.
In the third mode, planes are not usable, so everything but the cursor
goes to the renderer. We will use this when we cannot use the planes-only
mode (because some views cannot be placed in planes), but also cannot
use the 'mixed' mode because we have no renderer buffer yet. Since we
walk the scene graph from top to bottom, using atomic modesetting we
will determine if planes can be promoted in mixed mode by placing a
renderer buffer at the bottom of the scene, placing a cursor buffer if
applicable, then testing if we can add overlay planes to this mode.
Without a buffer from the renderer, we cannot do these tests, so we push
everything through the renderer and then switch to mixed mode on the
next repaint.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 69 +-
1 file changed, 45 insertions(+), 24 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index d045778aa..d4898bc46 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1939,6 +1939,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_RENDERER_ONLY, /**< only assign to renderer &
cursor */
+};
+
static struct weston_plane *
drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
@@ -3121,10 +3126,9 @@ drm_output_prepare_cursor_view(struct drm_output_state
*output_state,
struct wl_shm_buffer *shmbuf;
bool needs_update = false;
- if (!plane)
- return NULL;
+ assert(!b->cursors_are_broken);
- if (b->cursors_are_broken)
+ if (!plane)
return NULL;
if (!plane->state_cur->complete)
@@ -3259,7 +3263,8 @@ 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);
@@ -3267,11 +3272,14 @@ drm_output_propose_state(struct weston_output
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
struct weston_plane *primary = _base->compositor->primary_plane;
+ bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
pending_state,
DRM_OUTPUT_STATE_CLEAR_PLANES);
+ if (!planes_ok)
+ return state;
/*
* Find a surface for each sprite in the output using some heuristics:
@@ -3339,13 +3347,16 @@ 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 && b->sprites_are_broken)
+ if (next_plane == NULL && !planes_ok)
next_plane = primary;
if (next_plane == NULL)
@@ -3354,24 +3365,27 @@ drm_output_propose_state(struct weston_output
*output_base,
if (next_plane == NULL)
next_plane =
[PATCH v19 04/10] compositor-drm: Return plane state from plane preparation
Return a pointer to the plane state, rather than returning its
underlying weston_plane.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 53 ++
1 file changed, 25 insertions(+), 28 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index d4898bc46..28a580cd2 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,7 +1944,7 @@ enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer &
cursor */
};
-static struct weston_plane *
+static struct drm_plane_state *
drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
{
@@ -2004,7 +2004,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);
@@ -2170,7 +2170,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];
@@ -2197,7 +2196,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);
@@ -2287,8 +2286,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;
@@ -3000,7 +2999,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)
{
@@ -3071,7 +3070,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);
@@ -3115,7 +3114,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)
{
@@ -3201,7 +3200,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);
@@ -3271,7 +3270,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 planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
assert(!output->state_last);
@@ -3298,7 +3296,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;
@@ -3310,7 +3309,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);
@@ -3334,7 +,7 @@ drm_output_propose_state(struct weston_output
*output_base,
pixman_region32_intersect(_overlap, _region,
_view);
if
[PATCH weston] simple-dmabuf-drm: use GBM generic calls
No need to write libdrm driver specific code for each supported
driver, we can just let GBM call the right one for us now.
Signed-off-by: Emilio Pozuelo Monfort
---
Hi,
This simplifies the code a lot, using gbm_bo as Emil suggested. Some problems
I still see:
- NV12 doesn't work, it seems that
backends/dri/gbm_dri.c:gbm_format_to_dri_format()
doesn't support it.
- We are still linking to libdrm, that's just to get the DRM_FORMAT_*
definitions.
I suppose we could switch those to GBM_FORMAT_* instead.
- Not sure if I need to pass any flags to gbm_bo_create or gbm_bo_map.
This works fine for me with XRGB.
Thoughts? Comments?
Thanks,
Emilio
clients/simple-dmabuf-drm.c | 320 +++-
configure.ac| 2 +-
2 files changed, 27 insertions(+), 295 deletions(-)
diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index fcab30e5..cdee27b3 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -41,18 +41,7 @@
#include
#include
-#include
-
-#ifdef HAVE_LIBDRM_INTEL
-#include
-#include
-#endif
-#ifdef HAVE_LIBDRM_FREEDRENO
-#include
-#endif
-#ifdef HAVE_LIBDRM_ETNAVIV
-#include
-#endif
+#include
#include
#include
@@ -66,14 +55,10 @@
#define DRM_FORMAT_MOD_LINEAR 0
#endif
-struct buffer;
-
/* Possible options that affect the displayed image */
#define OPT_Y_INVERTED 1 /* contents has y axis inverted */
#define OPT_IMMEDIATE 2 /* create wl_buffer immediately */
-#define ALIGN(v, a) ((v + a - 1) & ~(a - 1))
-
struct display {
struct wl_display *display;
struct wl_registry *registry;
@@ -88,46 +73,22 @@ struct display {
int req_dmabuf_modifiers;
};
-struct drm_device {
- int fd;
- char *name;
-
- int (*alloc_bo)(struct buffer *buf);
- void (*free_bo)(struct buffer *buf);
- int (*export_bo_to_prime)(struct buffer *buf);
- int (*map_bo)(struct buffer *buf);
- void (*unmap_bo)(struct buffer *buf);
- void (*device_destroy)(struct buffer *buf);
-};
-
struct buffer {
struct wl_buffer *buffer;
int busy;
- struct drm_device *dev;
int drm_fd;
-
-#ifdef HAVE_LIBDRM_INTEL
- drm_intel_bufmgr *bufmgr;
- drm_intel_bo *intel_bo;
-#endif /* HAVE_LIBDRM_INTEL */
-#if HAVE_LIBDRM_FREEDRENO
- struct fd_device *fd_dev;
- struct fd_bo *fd_bo;
-#endif /* HAVE_LIBDRM_FREEDRENO */
-#if HAVE_LIBDRM_ETNAVIV
- struct etna_device *etna_dev;
- struct etna_bo *etna_bo;
-#endif /* HAVE_LIBDRM_ETNAVIV */
+ struct gbm_device *dev;
+ struct gbm_bo *bo;
uint32_t gem_handle;
int dmabuf_fd;
- uint8_t *mmap;
+ void *mmap;
int width;
int height;
int bpp;
- unsigned long stride;
+ uint32_t stride;
int format;
};
@@ -163,170 +124,6 @@ static const struct wl_buffer_listener buffer_listener = {
buffer_release
};
-
-#ifdef HAVE_LIBDRM_INTEL
-static int
-intel_alloc_bo(struct buffer *my_buf)
-{
- /* XXX: try different tiling modes for testing FB modifiers. */
- uint32_t tiling = I915_TILING_NONE;
-
- assert(my_buf->bufmgr);
-
- my_buf->intel_bo = drm_intel_bo_alloc_tiled(my_buf->bufmgr, "test",
- my_buf->width,
my_buf->height,
- (my_buf->bpp / 8), ,
- _buf->stride, 0);
-
- printf("buffer allocated w %d, h %d, stride %lu, size %lu\n",
- my_buf->width, my_buf->height, my_buf->stride,
my_buf->intel_bo->size);
-
- if (!my_buf->intel_bo)
- return 0;
-
- if (tiling != I915_TILING_NONE)
- return 0;
-
- return 1;
-}
-
-static void
-intel_free_bo(struct buffer *my_buf)
-{
- drm_intel_bo_unreference(my_buf->intel_bo);
-}
-
-static int
-intel_map_bo(struct buffer *my_buf)
-{
- if (drm_intel_gem_bo_map_gtt(my_buf->intel_bo) != 0)
- return 0;
-
- my_buf->mmap = my_buf->intel_bo->virtual;
-
- return 1;
-}
-
-static int
-intel_bo_export_to_prime(struct buffer *buffer)
-{
- return drm_intel_bo_gem_export_to_prime(buffer->intel_bo,
>dmabuf_fd);
-}
-
-static void
-intel_unmap_bo(struct buffer *my_buf)
-{
- drm_intel_gem_bo_unmap_gtt(my_buf->intel_bo);
-}
-
-static void
-intel_device_destroy(struct buffer *my_buf)
-{
- drm_intel_bufmgr_destroy(my_buf->bufmgr);
-}
-
-#endif /* HAVE_LIBDRM_INTEL */
-#ifdef HAVE_LIBDRM_FREEDRENO
-
-static int
-fd_alloc_bo(struct buffer *buf)
-{
- int flags = DRM_FREEDRENO_GEM_CACHE_WCOMBINE;
- int size;
-
- buf->fd_dev = fd_device_new(buf->drm_fd);
- buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
- size = buf->stride * buf->height;
- buf->fd_dev = fd_device_new(buf->drm_fd);
- buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags);
-
- if
Re: [PATCH weston v2] simple-dmabuf-drm: fix build with --disable-egl
On 10/07/18 15:16, Pekka Paalanen wrote:
> On Mon, 9 Jul 2018 17:38:49 +0200
> Emilio Pozuelo Monfort wrote:
>
>> 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);
>
> Hi Emilio,
>
> while this change is good, I found a couple more things that this
> exposes.
>
> The app still gets linked to libEGL and libgbm when it should not.
>
> The code you remove here was hiding a bug where xrgb_format_found
> was not set if the dmabuf interface was negotiated to version 2 or 1.
> The handler dmabuf_format() has no code. Another patch to fix this
> would be nice.
Yes, I have just found some problems when testing this patch for another change.
So let's not apply this for now, I will see what's going on and send an updated
one.
Cheers,
Emilio
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
On 10/07/18 14:52, Pekka Paalanen wrote: > On Tue, 3 Jul 2018 16:46:29 +0100 > Emil Velikov wrote: > >> Hi Emilio, >> >> On 2 July 2018 at 16:22, Emilio Pozuelo Monfort wrote: >>> Signed-off-by: Emilio Pozuelo Monfort >>> --- >>> I tried a build with --disable-egl as I didn't have the headers >>> installed, and it broke here. The EGL usage here seemed optional so I >>> did that, but I didn't run-test the result. If it would make more sense >>> to disable the client if EGL support is disabled I can do that. >>> >>> clients/simple-dmabuf-drm.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >> Fairly orthogonal question, aimed at wayland devs: >> >> Why does this "simple" app has per-device code instead of using >> gbm_bo_map/unmap? >> API has been around for 2 years and every half recent Mesa driver has >> support for it. Only the really old ones do not radeon (r100), r200, >> nouveau_vieux and i915. > > Hi Emil, > > I've had the same question before myself, but now that you ask it > again, I don't remember the answer. > > This client was originally written in 2014, and I think at that time > there were no generic APIs to do what it needed to do. Not sure if > there were other reasons as well. I have a semi-working patch for this. I will send it soon to ask for comments. Emilio ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Upcoming release reminder
On 2018-07-03 04:33 PM, Derek Foreman wrote: > Hi all, > > Just a quick reminder that we're due for alpha shortly, with the > following intended release schedule: I'd like to push this back to Friday, it looks like some atomic patches are just about ready to land, and it seems a shame to block that with a release schedule that was fairly arbitrarily advanced in the first place. Let's just add 3 to every number in the original release plan, and do it all on Fridays this time around. Thanks, Derek > Alpha - July 10th > Beta - July 24th > RC1 - August 7th > First possible release August 14th. > > Thanks, > Derek ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v18 4/4] compositor-drm: Return plane state from plane preparation
On Tue, 10 Jul 2018 14:56:18 +0100
Daniel Stone wrote:
> 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 | 58 --
> 1 file changed, 30 insertions(+), 28 deletions(-)
>
Hi,
this patch looks good and seems to fix all the temporary issues with
the previous patch, but I'm wary of giving an R-b because of the
changes needed in earlier patches and that I'm quite tired by now.
Thanks,
pq
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 81fcd2412..43dc9856d 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1945,7 +1945,7 @@ enum drm_output_propose_state_mode {
> DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer &
> cursor */
> };
>
> -static struct weston_plane *
> +static struct drm_plane_state *
> drm_output_prepare_scanout_view(struct drm_output_state *output_state,
> struct weston_view *ev)
> {
> @@ -2005,7 +2005,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);
> @@ -2171,7 +2171,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];
> @@ -2198,7 +2197,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);
>
> @@ -2288,8 +2287,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;
>
> @@ -3001,7 +3000,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)
> {
> @@ -3072,7 +3071,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);
> @@ -3116,7 +3115,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)
> {
> @@ -3202,7 +3201,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);
> @@ -3272,7 +3271,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 = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
>
> @@ -3298,7 +3296,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;
>
> @@ -3310,7 +3309,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 <<
Re: [PATCH v18 3/4] compositor-drm: Add modes to drm_output_propose_state
On Tue, 10 Jul 2018 14:56:17 +0100
Daniel Stone wrote:
> Add support for multiple modes to drm_output_propose_state. Currently we
> intend to operate in three modes: planes-only (no renderer buffer,
> client buffers in planes only), mixed-mode (promote client buffers to
> planes where possible, falling back to the renderer where not), and
> renderer-only (no plane usage at all).
>
> We want to use the first (planes-only) mode where possible: it can avoid
> us having to allocate buffers for the renderer, and it also gives us the
> best chance of the optimal configuration, with no composition. In this
> mode, we walk the scene looking at all views, trying to put them in
> planes, and failing as soon as we find a view we cannot place in a
> plane.
>
> In the second mode, rather than failing, we assign those views which
> cannot be on a plane to the renderer, and allow the renderer to
> composite them.
>
> In the third mode, planes are not usable, so everything but the cursor
> goes to the renderer. We will use this when we cannot use the planes-only
> mode (because some views cannot be placed in planes), but also cannot
> use the 'mixed' mode because we have no renderer buffer yet. Since we
> walk the scene graph from top to bottom, using atomic modesetting we
> will determine if planes can be promoted in mixed mode by placing a
> renderer buffer at the bottom of the scene, placing a cursor buffer if
> applicable, then testing if we can add overlay planes to this mode.
>
> Without a buffer from the renderer, we cannot do these tests, so we push
> everything through the renderer and then switch to mixed mode on the
> next repaint.
>
> Signed-off-by: Daniel Stone
> Tested-by: Emre Ucan
> ---
> libweston/compositor-drm.c | 86 +++---
> 1 file changed, 61 insertions(+), 25 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 4a48e2d02..81fcd2412 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1939,16 +1939,25 @@ 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 */
> + DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer &
> cursor */
> +};
This is good!
> +
> static struct weston_plane *
> drm_output_prepare_scanout_view(struct drm_output_state *output_state,
> struct weston_view *ev)
> {
> 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_fb *fb;
> pixman_box32_t *extents;
>
> + assert(!b->sprites_are_broken);
> +
> /* Check the view spans exactly the output size, calculated in the
>* logical co-ordinate space. */
> extents = pixman_region32_extents(>transform.boundingbox);
> @@ -3004,8 +3013,7 @@ drm_output_prepare_overlay_view(struct drm_output_state
> *output_state,
> struct drm_fb *fb;
> unsigned int i;
>
> - if (b->sprites_are_broken)
> - return NULL;
> + assert(!b->sprites_are_broken);
>
> fb = drm_fb_get_from_view(output_state, ev);
> if (!fb)
> @@ -3119,10 +3127,9 @@ drm_output_prepare_cursor_view(struct drm_output_state
> *output_state,
> struct wl_shm_buffer *shmbuf;
> bool needs_update = false;
>
> - if (!plane)
> - return NULL;
> + assert(!b->cursors_are_broken);
>
> - if (b->cursors_are_broken)
> + if (!plane)
> return NULL;
>
> if (!plane->state_cur->complete)
> @@ -3257,13 +3264,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 = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
>
> assert(!output->state_last);
> state = drm_output_state_duplicate(output->state_cur,
> @@ -3336,7 +3347,10 @@ drm_output_propose_state(struct weston_output
> *output_base,
> next_plane = primary;
> pixman_region32_fini(_overlap);
>
>
Re: [PATCH v18 2/4] compositor-drm: Use sprites_are_broken for scanout plane
On Tue, 10 Jul 2018 14:56:16 +0100 Daniel Stone wrote: > When the sprites_are_broken variable is set, do not attempt to promote > client surfaces to the scanout plane. > > We are currently assuming that every client buffer will be compatible > with the scanout plane, but that is not the case, particularly with more > exotic tiled/compressed buffers. Once we promote the client buffer to > scanout, there is no going back: if the repaint fails, we do not mark > this as failed and go back to repaint through composition. > > This removes the ability for scanout bypass when using the non-atomic > path, however future patches lift the restriction when using atomic > modesetting, as we can actually test and ensure that the view is > compatible with scanout. > > Signed-off-by: Daniel Stone > Reported-by: Pekka Paalanen > --- > libweston/compositor-drm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c > index 2305f708f..4a48e2d02 100644 > --- a/libweston/compositor-drm.c > +++ b/libweston/compositor-drm.c > @@ -3342,6 +3342,9 @@ drm_output_propose_state(struct weston_output > *output_base, > 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); > Hi, this does not compile at this step of the series, planes_ok is undefined. The commit message reference to sprites_are_broken does not make sense either without the following patch, otherwise the explanation sounds good. Thanks, pq pgp09n_rhYHNM.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v18 1/4] compositor-drm: Disallow overlapping overlay planes
On Tue, 10 Jul 2018 14:56:15 +0100 Daniel Stone wrote: > The scanout plane strictly stacks under all overlay planes, and the > cursor plane above. However, the stacking of overlay planes with respect > to each other is undefined. > > We can control the stacking order of overlay planes with the zpos > property, though this significantly complicates plane assignment. In the > meantime, simply disallow assigning a view to an overlay, when it > overlaps another view which is already on an overlay. This ensures > stacking order is irrelevant, since the planes never intersect each > other. > > Signed-off-by: Daniel Stone > Reported-by: Pekka Paalanen > --- > libweston/compositor-drm.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c > index 51b2482d9..2305f708f 100644 > --- a/libweston/compositor-drm.c > +++ b/libweston/compositor-drm.c > @@ -3324,6 +3324,16 @@ drm_output_propose_state(struct weston_output > *output_base, > _view); > if (pixman_region32_not_empty(_overlap)) > next_plane = primary; > + > + /* We do not control the stacking order of overlay planes; > + * the scanout plane is strictly stacked bottom and the cursor > + * plane top, but the ordering of overlay planes with respect > + * to each other is undefined. Make sure we do not have two > + * planes overlapping each other. */ > + pixman_region32_intersect(_overlap, _region, > + _view); > + if (pixman_region32_not_empty(_overlap)) > + next_plane = primary; > pixman_region32_fini(_overlap); > > if (next_plane == NULL) Reviewed-by: Pekka Paalanen Thanks, pq pgpiXHUKMrV1E.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v18 4/4] 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 | 58 --
1 file changed, 30 insertions(+), 28 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 81fcd2412..43dc9856d 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1945,7 +1945,7 @@ enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer &
cursor */
};
-static struct weston_plane *
+static struct drm_plane_state *
drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
{
@@ -2005,7 +2005,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);
@@ -2171,7 +2171,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];
@@ -2198,7 +2197,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);
@@ -2288,8 +2287,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;
@@ -3001,7 +3000,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)
{
@@ -3072,7 +3071,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);
@@ -3116,7 +3115,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)
{
@@ -3202,7 +3201,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);
@@ -3272,7 +3271,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 = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
@@ -3298,7 +3296,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;
@@ -3310,7 +3309,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);
@@ -3334,7 +,7 @@ drm_output_propose_state(struct weston_output
*output_base,
pixman_region32_intersect(_overlap, _region,
_view);
[PATCH v18 1/4] compositor-drm: Disallow overlapping overlay planes
The scanout plane strictly stacks under all overlay planes, and the cursor plane above. However, the stacking of overlay planes with respect to each other is undefined. We can control the stacking order of overlay planes with the zpos property, though this significantly complicates plane assignment. In the meantime, simply disallow assigning a view to an overlay, when it overlaps another view which is already on an overlay. This ensures stacking order is irrelevant, since the planes never intersect each other. Signed-off-by: Daniel Stone Reported-by: Pekka Paalanen --- libweston/compositor-drm.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 51b2482d9..2305f708f 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3324,6 +3324,16 @@ drm_output_propose_state(struct weston_output *output_base, _view); if (pixman_region32_not_empty(_overlap)) next_plane = primary; + + /* We do not control the stacking order of overlay planes; +* the scanout plane is strictly stacked bottom and the cursor +* plane top, but the ordering of overlay planes with respect +* to each other is undefined. Make sure we do not have two +* planes overlapping each other. */ + pixman_region32_intersect(_overlap, _region, + _view); + if (pixman_region32_not_empty(_overlap)) + next_plane = primary; pixman_region32_fini(_overlap); if (next_plane == NULL) -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v18 0/4] DRM atomic preparation
Hi, A couple of changes after Pekka's review of the second two patches, including two preparatory patches pulled out. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v18 2/4] compositor-drm: Use sprites_are_broken for scanout plane
When the sprites_are_broken variable is set, do not attempt to promote client surfaces to the scanout plane. We are currently assuming that every client buffer will be compatible with the scanout plane, but that is not the case, particularly with more exotic tiled/compressed buffers. Once we promote the client buffer to scanout, there is no going back: if the repaint fails, we do not mark this as failed and go back to repaint through composition. This removes the ability for scanout bypass when using the non-atomic path, however future patches lift the restriction when using atomic modesetting, as we can actually test and ensure that the view is compatible with scanout. Signed-off-by: Daniel Stone Reported-by: Pekka Paalanen --- libweston/compositor-drm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 2305f708f..4a48e2d02 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3342,6 +3342,9 @@ drm_output_propose_state(struct weston_output *output_base, 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); -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v18 3/4] compositor-drm: Add modes to drm_output_propose_state
Add support for multiple modes to drm_output_propose_state. Currently we
intend to operate in three modes: planes-only (no renderer buffer,
client buffers in planes only), mixed-mode (promote client buffers to
planes where possible, falling back to the renderer where not), and
renderer-only (no plane usage at all).
We want to use the first (planes-only) mode where possible: it can avoid
us having to allocate buffers for the renderer, and it also gives us the
best chance of the optimal configuration, with no composition. In this
mode, we walk the scene looking at all views, trying to put them in
planes, and failing as soon as we find a view we cannot place in a
plane.
In the second mode, rather than failing, we assign those views which
cannot be on a plane to the renderer, and allow the renderer to
composite them.
In the third mode, planes are not usable, so everything but the cursor
goes to the renderer. We will use this when we cannot use the planes-only
mode (because some views cannot be placed in planes), but also cannot
use the 'mixed' mode because we have no renderer buffer yet. Since we
walk the scene graph from top to bottom, using atomic modesetting we
will determine if planes can be promoted in mixed mode by placing a
renderer buffer at the bottom of the scene, placing a cursor buffer if
applicable, then testing if we can add overlay planes to this mode.
Without a buffer from the renderer, we cannot do these tests, so we push
everything through the renderer and then switch to mixed mode on the
next repaint.
Signed-off-by: Daniel Stone
Tested-by: Emre Ucan
---
libweston/compositor-drm.c | 86 +++---
1 file changed, 61 insertions(+), 25 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 4a48e2d02..81fcd2412 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1939,16 +1939,25 @@ 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 */
+ DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer &
cursor */
+};
+
static struct weston_plane *
drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
{
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_fb *fb;
pixman_box32_t *extents;
+ assert(!b->sprites_are_broken);
+
/* Check the view spans exactly the output size, calculated in the
* logical co-ordinate space. */
extents = pixman_region32_extents(>transform.boundingbox);
@@ -3004,8 +3013,7 @@ drm_output_prepare_overlay_view(struct drm_output_state
*output_state,
struct drm_fb *fb;
unsigned int i;
- if (b->sprites_are_broken)
- return NULL;
+ assert(!b->sprites_are_broken);
fb = drm_fb_get_from_view(output_state, ev);
if (!fb)
@@ -3119,10 +3127,9 @@ drm_output_prepare_cursor_view(struct drm_output_state
*output_state,
struct wl_shm_buffer *shmbuf;
bool needs_update = false;
- if (!plane)
- return NULL;
+ assert(!b->cursors_are_broken);
- if (b->cursors_are_broken)
+ if (!plane)
return NULL;
if (!plane->state_cur->complete)
@@ -3257,13 +3264,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 = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3336,7 +3347,10 @@ 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
+*
Re: [PATCH weston v2] simple-dmabuf-drm: fix build with --disable-egl
On Mon, 9 Jul 2018 17:38:49 +0200
Emilio Pozuelo Monfort wrote:
> 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);
Hi Emilio,
while this change is good, I found a couple more things that this
exposes.
The app still gets linked to libEGL and libgbm when it should not.
The code you remove here was hiding a bug where xrgb_format_found
was not set if the dmabuf interface was negotiated to version 2 or 1.
The handler dmabuf_format() has no code. Another patch to fix this
would be nice.
Thanks,
pq
pgpmoeG4OZOZF.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] simple-dmabuf-drm: fix build with --disable-egl
On Tue, 3 Jul 2018 16:46:29 +0100 Emil Velikov wrote: > Hi Emilio, > > On 2 July 2018 at 16:22, Emilio Pozuelo Monfort wrote: > > Signed-off-by: Emilio Pozuelo Monfort > > --- > > I tried a build with --disable-egl as I didn't have the headers > > installed, and it broke here. The EGL usage here seemed optional so I > > did that, but I didn't run-test the result. If it would make more sense > > to disable the client if EGL support is disabled I can do that. > > > > clients/simple-dmabuf-drm.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > Fairly orthogonal question, aimed at wayland devs: > > Why does this "simple" app has per-device code instead of using > gbm_bo_map/unmap? > API has been around for 2 years and every half recent Mesa driver has > support for it. Only the really old ones do not radeon (r100), r200, > nouveau_vieux and i915. Hi Emil, I've had the same question before myself, but now that you ask it again, I don't remember the answer. This client was originally written in 2014, and I think at that time there were no generic APIs to do what it needed to do. Not sure if there were other reasons as well. Thanks, pq pgpUcB2fYlAzy.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] libweston: Fix clear timing of output repainted flag
On Tue, 10 Jul 2018 11:47:15 +0900 Tomohito Esaki wrote: > 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(-) Pushed: 8108239c..ddaf95c5 master -> master Thanks, pq > > 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; pgpyBvbeRw2mQ.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 14/14] compositor-drm: Enable planes for atomic
On Mon, 9 Jul 2018 14:23:20 +0100
Daniel Stone wrote:
> 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;
> +
Maybe one day we could replace sprites_are_broken with !atomic_modeset?
Reviewed-by: Pekka Paalanen
Thanks,
pq
> 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;
pgppe4aKqEZJG.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 13/14] compositor-drm: Relax plane restrictions for atomic
On Mon, 9 Jul 2018 14:23:19 +0100
Daniel Stone wrote:
> 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(-)
Reviewed-by: Pekka Paalanen
Thanks,
pq
>
> 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;
> }
pgpmfmWV6T4Dq.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 11/14] compositor-drm: Return plane state from plane preparation
On Mon, 9 Jul 2018 14:23:17 +0100
Daniel Stone wrote:
> 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;
This...
> 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;
...this, and...
> 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;
...this are unrelated but trivial changes.
Btw. this raises a style question. In the past I assumed it was ok to
declare variables at the beginning of a block, it still avoids mixed
code and declarations, but I recall being corrected that variables
should be declared in the beginning of the function only. I don't see
it written down at all, though.
Anyway, beginning of a block is my preferred way, so these bits are ok
by me.
> 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
Re: [PATCH v17 09/14] compositor-drm: Ignore occluded views
Hi, On Tue, 10 Jul 2018 at 10:06, Pekka Paalanen wrote: > On Mon, 9 Jul 2018 14:23:15 +0100 Daniel Stone wrote: > > @@ -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; As you noted here, we only promote fully opaque views. > However, I noticed something else. drm_output_preprare_overlay_view() > may work also for non-opaque views. Therefore we need to handle > overlays as non-opaque or know if they actually are opaque. > > Why do we even toggle opaqueness by the plane assignment? Why not > simply directly add the opaque region to occluded_region regardless of > the assigned plane? > > weston_view::transform.opaque is there for that purpose. > > drm_view_is_opaque() requires the whole view to be opaque, but we don't > actually need that for occluded_region. We should be able to do with > the proper opaque region in global coordinates, so that an opaque > sub-region of a view on a plane would be counted as occlusion. At the moment the whole area must be opaque in order to be promoted to a view. There is another, later, stage where we can begin to promote non-opaque views, however that gets tricky. We need to make sure that DRM's plane blending is in line with what we expect (it might be, but I'm not immediately sure), and we also need to begin to take care of stacking order. At the moment, scanout/primary strictly stacks at the bottom (furthest from eye) and cursor at the top (closest to eye), with all the overlay planes in between. We currently ensure the stacking order is correct by only promoting views which do not overlap each other to overlay planes. If we started loosening that, so planes could overlap each other (opaque or not), we need to be setting the 'zpos' property of planes, or picking the plane in the correct zpos. This got rather difficult rather fast, so I decided it was probably best not to roll it into the series. Fabien and Benji from STM had an old patchset which did implement this, which should be checked and resurrected once this all lands. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v17 09/14] compositor-drm: Ignore occluded views
On Tue, 10 Jul 2018 12:06:00 +0300
Pekka Paalanen wrote:
> On Mon, 9 Jul 2018 14:23:15 +0100
> Daniel Stone wrote:
>
> > 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;
Sorry, it seems I missed this condition. With it:
Reviewed-by: Pekka Paalanen
Even if it does seem to produce less optimal results than my
suggestion, it should be good for now.
Thanks,
pq
> > +
> > 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)
>
> On Mon, 9 Jul 2018 15:46:40 +0100
> Daniel Stone wrote:
>
> > 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,
> > > >
Re: [PATCH v14 35/41] compositor-drm: Add modes to drm_output_propose_state
On Tue, 10 Jul 2018 08:53:54 +0100
Daniel Stone wrote:
> Hi,
> On Mon, 29 Jan 2018 at 10:55, Pekka Paalanen wrote:
> > On Mon, 29 Jan 2018 09:17:49 + Daniel Stone
> > wrote:
> > > On 26 January 2018 at 14:04, Pekka Paalanen wrote:
> > > > On Wed, 20 Dec 2017 12:26:52 +
> > > > Daniel Stone wrote:
> > > >> +enum drm_output_propose_state_mode {
> > > >> + DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
> > > >> + DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes
> > > >> */
> > > >> +};
> > > >
> > > > what is the reason to have a planes-only mode? I saw the patch
> > > > "compositor-drm: Add test-only mode to state application" will first
> > > > attempt a planes-only mode and then fall back to mixed mode. Why does
> > > > the planes-only mode not fall out of mixed mode naturally when it runs
> > > > out of views to show? This would be good to explain in the commit
> > > > message to justify the modes.
> > >
> > > This could again go into a comment or documentation I suppose, but I'm
> > > not sure where I'd put it without adding so much clutter to the code
> > > as to hide it. Ideas?
> >
> > Commit message would be fine by me, you can be as verbose as you want
> > without a worry.
>
> This got missed.
>
> > > We have two options: either punt when we don't have valid scanout
> > > state, or do what we do here. Which is, speculatively build a state
> > > composed purely of planes with no intermediate checks, and test once
> > > at the end. If it works, then we use it. If not, we build a
> > > renderer-only (i.e. renderer + cursor) state and just use that. In the
> > > next repaint, we will use the normal 'mixed' mode.
> > >
> > > (All of the above described is the intention. If these patches differ
> > > from that description, then the patches are wrong.)
> >
> > Ok, this is something that never occurred to me when reading this
> > patch. This is going to a great length to avoid allocating a
> > framebuffer for the renderer unless absolutely necessary.
>
> Not just avoiding allocating, but avoiding putting up one frame where
> everything has gone through the GPU, followed by another where
> everything's on the overlay, for no real reason. That could cause
> flickers when playing media, due to different colourspace conversions
> etc. But yes, we do bend over backwards to use planes where we can.
>
> > A quick check on "compositor-drm: Add test-only mode to state
> > application" revealed code:
> >
> > drm_assign_planes(struct weston_output *output_base, void *repaint_data)
> > {
> > ...
> > state = drm_output_propose_state(output_base, pending_state,
> >
> > DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
> > if (!state)
> > state = drm_output_propose_state(output_base, pending_state,
> >
> > DRM_OUTPUT_PROPOSE_STATE_MIXED);
> >
> > from which I made wrong conclusions on what the two modes do. An
> > important bit that I missed was the hunk added to
> > drm_output_propose_state() that fundamentally changes how 'planes_ok'
> > gets set.
> >
> > Another reason I was confused was that STATE_PLANES_ONLY is attempted
> > on every frame. Should that not be skipped if we have an existing
> > primary fb to test with? Then the renderer-not-needed case might be
> > slightly more obvious to fall out from STATE_MIXED.
>
> It could still be useful. For instance, if the hardware has one
> detiling unit shared between all planes, with the GPU rendering to
> tiled buffers and the media decoder linear. The client has given us
> one GPU buffer placed on top of one fullscreen media buffer. In this
> case, if we test compositor rendering buffer (tiled) + client GPU
> buffer (tiled), we will fail the test and not promote the client GPU
> buffer to an overlay. There's no guarantee that whatever you want to
> put on fullscreen is compatible with the renderer output.
Aha, good to realize.
> > Maybe it would be more clear if the hunk that sets up the real
> > planes_ok logic in "compositor-drm: Add test-only mode to state
> > application" would be moved into this patch?
>
> Sure, I think that could work.
>
> > Or thinking these patches from a higher level...
> >
> > Essentially there are three modes: renderer-only, mixed, and
> > planes-only. In this patch series, the renderer-only mode is a hidden
> > mode under the mixed mode, and it's not obvious why the planes-only
> > mode exists. Mixed mode is a bit special, because it can natually fall
> > into renderer-only or planes-only setups.
> >
> > What would you think of exposing all three modes explicitly, and having
> > the logic to choose one mode and the fallback to renderer-only mode in
> > drm_assing_planes()?
> >
> > You could have a helper like
> > bool drm_output_has_testable_scanout_fb()
> > to make it more self-documenting.
> >
> > Something along the lines of
> >
> >
Re: [PATCH v17 09/14] compositor-drm: Ignore occluded views
On Mon, 9 Jul 2018 14:23:15 +0100
Daniel Stone wrote:
> 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)
On Mon, 9 Jul 2018 15:46:40 +0100
Daniel Stone wrote:
> 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
Re: [RFC wayland-protocols] unstable: add protocol to give focus to a foreign surface
>Hm. If you wanted to, you could make this explicit by requiring an event >serial in the export_surface request rather than the wl_surface. Certainly an option. Though I'm not sure we have a use case of needing to limit a client releasing its focus? >Does this interface need to exist? It doesn't /need/ to, but I need to be able to export a handle multiple times and it's nice in the client to be able to match up requests with the reply. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v14 38/41] compositor-drm: Relax plane restrictions for atomic
Hi, On Mon, 29 Jan 2018 at 13:59, Pekka Paalanen wrote: > On Wed, 20 Dec 2017 12:26:55 + Daniel Stone wrote: > > @@ -1818,12 +1819,10 @@ drm_output_prepare_scanout_view(struct > > drm_output_state *output_state, > > drm_plane_state_coords_for_view(state, ev); > > > > /* 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 || > > - state->dest_w != (unsigned) output->base.current_mode->width || > > - state->dest_h != (unsigned) output->base.current_mode->height) > > + 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)) > > Where did the mode size checks go? These are retained in v17. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v14 35/41] compositor-drm: Add modes to drm_output_propose_state
Hi,
On Mon, 29 Jan 2018 at 10:55, Pekka Paalanen wrote:
> On Mon, 29 Jan 2018 09:17:49 + Daniel Stone wrote:
> > On 26 January 2018 at 14:04, Pekka Paalanen wrote:
> > > On Wed, 20 Dec 2017 12:26:52 +
> > > Daniel Stone wrote:
> > >> +enum drm_output_propose_state_mode {
> > >> + DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
> > >> + DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes */
> > >> +};
> > >
> > > what is the reason to have a planes-only mode? I saw the patch
> > > "compositor-drm: Add test-only mode to state application" will first
> > > attempt a planes-only mode and then fall back to mixed mode. Why does
> > > the planes-only mode not fall out of mixed mode naturally when it runs
> > > out of views to show? This would be good to explain in the commit
> > > message to justify the modes.
> >
> > This could again go into a comment or documentation I suppose, but I'm
> > not sure where I'd put it without adding so much clutter to the code
> > as to hide it. Ideas?
>
> Commit message would be fine by me, you can be as verbose as you want
> without a worry.
This got missed.
> > We have two options: either punt when we don't have valid scanout
> > state, or do what we do here. Which is, speculatively build a state
> > composed purely of planes with no intermediate checks, and test once
> > at the end. If it works, then we use it. If not, we build a
> > renderer-only (i.e. renderer + cursor) state and just use that. In the
> > next repaint, we will use the normal 'mixed' mode.
> >
> > (All of the above described is the intention. If these patches differ
> > from that description, then the patches are wrong.)
>
> Ok, this is something that never occurred to me when reading this
> patch. This is going to a great length to avoid allocating a
> framebuffer for the renderer unless absolutely necessary.
Not just avoiding allocating, but avoiding putting up one frame where
everything has gone through the GPU, followed by another where
everything's on the overlay, for no real reason. That could cause
flickers when playing media, due to different colourspace conversions
etc. But yes, we do bend over backwards to use planes where we can.
> A quick check on "compositor-drm: Add test-only mode to state
> application" revealed code:
>
> drm_assign_planes(struct weston_output *output_base, void *repaint_data)
> {
> ...
> state = drm_output_propose_state(output_base, pending_state,
>
> DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
> if (!state)
> state = drm_output_propose_state(output_base, pending_state,
>
> DRM_OUTPUT_PROPOSE_STATE_MIXED);
>
> from which I made wrong conclusions on what the two modes do. An
> important bit that I missed was the hunk added to
> drm_output_propose_state() that fundamentally changes how 'planes_ok'
> gets set.
>
> Another reason I was confused was that STATE_PLANES_ONLY is attempted
> on every frame. Should that not be skipped if we have an existing
> primary fb to test with? Then the renderer-not-needed case might be
> slightly more obvious to fall out from STATE_MIXED.
It could still be useful. For instance, if the hardware has one
detiling unit shared between all planes, with the GPU rendering to
tiled buffers and the media decoder linear. The client has given us
one GPU buffer placed on top of one fullscreen media buffer. In this
case, if we test compositor rendering buffer (tiled) + client GPU
buffer (tiled), we will fail the test and not promote the client GPU
buffer to an overlay. There's no guarantee that whatever you want to
put on fullscreen is compatible with the renderer output.
> Maybe it would be more clear if the hunk that sets up the real
> planes_ok logic in "compositor-drm: Add test-only mode to state
> application" would be moved into this patch?
Sure, I think that could work.
> Or thinking these patches from a higher level...
>
> Essentially there are three modes: renderer-only, mixed, and
> planes-only. In this patch series, the renderer-only mode is a hidden
> mode under the mixed mode, and it's not obvious why the planes-only
> mode exists. Mixed mode is a bit special, because it can natually fall
> into renderer-only or planes-only setups.
>
> What would you think of exposing all three modes explicitly, and having
> the logic to choose one mode and the fallback to renderer-only mode in
> drm_assing_planes()?
>
> You could have a helper like
> bool drm_output_has_testable_scanout_fb()
> to make it more self-documenting.
>
> Something along the lines of
>
> if (drm_output_has_testable_scanout_fb()) {
> state = drm_output_propose_state(MIXED);
> } else {
> state = drm_output_propose_state(PLANES_ONLY);
> if (!state)
> state = drm_output_propose_state(RENDERER_ONLY);
> }
Hm, I think what we really want is:
/* First try to bypass the
Re: [PATCH v14 40/41] compositor-drm: Support plane IN_FORMATS
On Tue, 10 Jul 2018 08:41:46 +0100 Daniel Stone wrote: > Hi Pekka, > > On Mon, 29 Jan 2018 at 15:01, Pekka Paalanen wrote: > > On Wed, 20 Dec 2017 12:26:57 + Daniel Stone > > wrote: > > > @@ -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])]) > > > > Hi, > > > > I wonder, we are getting more and more of these "libdrm has ..." > > feature checks. How about merging some to produce fewer build types? > > Just a general idea, not specifically for this patch. > > Assuming enough distros support the new bits, I'd be open to having > two levels of libdrm support: ancient or supports-everything. > Annoyingly the gbm check is separate, but I suppose we could connect, > e.g., both of libdrm >= 2.4.83 + gbm >= 17.2 to modifiers, rather than > having modifiers half-supported when only one of them is sufficiently > new. That sounds reasonable to me. Now that we've heard of the weak functions trick, I'd be happy to remove some autoconf checks for library functions and replace them with runtime weak function checks that would also be reported in the log. Maybe that can cut the number of #ifdefs and checks. Let's keep these ideas for follow-up. They would help the Meson migration as well, having less dependency checks to cross-check. > > > + unsigned i, j; > > > + drmModePropertyBlobRes *blob; > > > + struct drm_format_modifier_blob *fmt_mod_blob; > > > + uint32_t *blob_formats; > > > + struct drm_format_modifier *blob_modifiers; > > > + > > > + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id); > > > + if (!blob) > > > + return false; > > > + > > > + fmt_mod_blob = blob->data; > > > > Should we not check that fmt_mod_blob.version == 1? > > No. Future versions can add new bits to the end of the blob, but they > can't break compatibility - doing so would require a new property > name. Usually DRM uses size to do this, but as we have two > variable-length arrays whose size can't be trivially determined by the > client, we use version to indicate any possible future extensions to > the structure, and the offset fields to point to the VLAs. > > The current version is 1; I guess we could check for version 0 and > bail out if we see it, but no kernel has ever had it and it seems a > bit pathological. Future versions will still be parseable as the > original, so those are fine. Understood. Thanks, pq pgp_1lW0E5AnD.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v14 40/41] compositor-drm: Support plane IN_FORMATS
Hi Pekka,
On Mon, 29 Jan 2018 at 15:01, Pekka Paalanen wrote:
> On Wed, 20 Dec 2017 12:26:57 + Daniel Stone wrote:
> > @@ -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])])
>
> Hi,
>
> I wonder, we are getting more and more of these "libdrm has ..."
> feature checks. How about merging some to produce fewer build types?
> Just a general idea, not specifically for this patch.
Assuming enough distros support the new bits, I'd be open to having
two levels of libdrm support: ancient or supports-everything.
Annoyingly the gbm check is separate, but I suppose we could connect,
e.g., both of libdrm >= 2.4.83 + gbm >= 17.2 to modifiers, rather than
having modifiers half-supported when only one of them is sufficiently
new.
> > +#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);
> > +}
>
> These two functions are unlikely to be used anywhere else than
> populate_format_modifiers(), so they might as well be in the same #ifdef
> block. If even functions at all. A long line.
Both done.
> > @@ -3635,6 +3667,61 @@ init_pixman(struct drm_backend *b)
> > return pixman_renderer_init(b->compositor);
> > }
> >
> > +/**
> > + * Populates the formats array, and the modifiers of each format for a
> > drm_plane.
> > + */
> > +#ifdef HAVE_DRM_FORMATS_BLOB
> > +static bool
> > +populate_format_modifiers(struct drm_plane *plane, const drmModePlane
> > *kplane,
> > + uint32_t blob_id)
>
> I think this should be:
>
> static int
> drm_plane_populate_format_modifiers(...
>
> I believe we tend to use int for success/failure returns, where
> negative is a failure. Boolean return values are more used for
> functions that return a truth value, like drm_device_is_kms().
Both done.
> > + unsigned i, j;
> > + drmModePropertyBlobRes *blob;
> > + struct drm_format_modifier_blob *fmt_mod_blob;
> > + uint32_t *blob_formats;
> > + struct drm_format_modifier *blob_modifiers;
> > +
> > + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id);
> > + if (!blob)
> > + return false;
> > +
> > + fmt_mod_blob = blob->data;
>
> Should we not check that fmt_mod_blob.version == 1?
No. Future versions can add new bits to the end of the blob, but they
can't break compatibility - doing so would require a new property
name. Usually DRM uses size to do this, but as we have two
variable-length arrays whose size can't be trivially determined by the
client, we use version to indicate any possible future extensions to
the structure, and the offset fields to point to the VLAs.
The current version is 1; I guess we could check for version 0 and
bail out if we see it, but no kernel has ever had it and it seems a
bit pathological. Future versions will still be parseable as the
original, so those are fine.
> > + blob_formats = formats_ptr(fmt_mod_blob);
> > + blob_modifiers = modifiers_ptr(fmt_mod_blob);
> > +
> > + assert(plane->count_formats == fmt_mod_blob->count_formats);
> > +
> > + for (i = 0; i < fmt_mod_blob->count_formats; i++) {
> > + uint32_t count_modifiers = 0;
> > + uint64_t *modifiers = NULL;
> > +
> > + for (j = 0; j < fmt_mod_blob->count_modifiers; j++) {
> > + struct drm_format_modifier *mod = _modifiers[j];
> > +
> > + if ((i < mod->offset) || (i > mod->offset + 63))
> > + continue;
> > + if (!(mod->formats & (1 << (i - mod->offset
> > + continue;
> > +
> > + modifiers = realloc(modifiers, (count_modifiers + 1)
> > * sizeof(modifiers[0]));
>
> Split line, please.
Done.
> > + if (!modifiers) {
>
> Realloc is a bit inconvenient in that if it fails, the original
> allocation
Implementing right click and middle click drag
I would like to work on implementing right and middle click drag as two/three finger tap followed by a single tap drag, the way it works in synaptics driver. I found that this topic has been discussed previously: https://lists.freedesktop.org/archives/wayland-devel/2017-September/034926.html But this was the single main on that topic in the archives. To Peter Hutterer, where can I find the patch being discussed in that mail? Would it be better to start from scratch since the patch was apparantly flawed? Best Regards Sagar Tewari ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
