[PATCH v2 weston 00/16] Atomic modesetting support
Pekka Paalanen
ppaalanen at gmail.com
Tue Jun 23 03:26:21 PDT 2015
On Mon, 22 Jun 2015 17:25:05 +0100
Daniel Stone <daniels at collabora.com> wrote:
> Hi,
> Thanks to everyone who reviewed the previous series. This new series
> cleans up the previous patches, introduces a few fixes (e.g. not relying
> on a repaint to pull us out of DPMS), and crucially adds support for the
> libdrm TEST_ONLY interface (allowing us to check before we commit, e.g.
> that a particular plane combination is workable), using the new cursor
> API from libdrm.
>
> This still relies on support not yet mainlined. Currently I am using:
> git://git.collabora.co.uk/git/user/daniels/linux.git#wip/4.2.x/atomic-i915
> git://git.collabora.co.uk/git/user/daniels/libdrm.git#atomic
Hi Daniel,
you asked me to take a look at the assing_planes code wrt. TEST_ONLY
and backtracking.
The selected code quotations below are from after the complete series.
static void
drm_plane_update_begin(struct drm_plane *plane)
{
plane->props.value_pend_mask = 0;
}
/**
* Try to use the primary DRM plane to directly display a full-screen view
*
* If a surface covers an entire output and is unoccluded, attempt to directly
* pageflip the primary DRM plane (not to be confused with the primary Weston
* plane) to the buffer. In legacy DRM usage, this will use drmModePageFlip;
* in atomic, this is just another plane.
*
* @param output Output to target
* @param ev View to prospectively use the primary plane
*/
static struct weston_plane *
drm_output_prepare_scanout_view(struct drm_output *output,
struct weston_view *ev)
{
struct drm_compositor *c =
(struct drm_compositor *) output->base.compositor;
struct weston_buffer *buffer = ev->surface->buffer_ref.buffer;
struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport;
struct gbm_bo *bo;
uint32_t format;
if (ev->geometry.x != output->base.x ||
ev->geometry.y != output->base.y ||
buffer == NULL || c->gbm == NULL ||
buffer->width != output->base.current_mode->width ||
buffer->height != output->base.current_mode->height ||
output->base.transform != viewport->buffer.transform ||
ev->transform.enabled)
return NULL;
if (ev->geometry.scissor_enabled)
return NULL;
bo = gbm_bo_import(c->gbm, GBM_BO_IMPORT_WL_BUFFER,
buffer->resource, GBM_BO_USE_SCANOUT);
/* Unable to use the buffer for scanout */
if (!bo)
return NULL;
format = drm_output_check_scanout_format(output, ev->surface, bo);
if (format == 0) {
gbm_bo_destroy(bo);
return NULL;
}
output->primary_plane->next = drm_fb_get_from_bo(bo, c, format);
if (!output->primary_plane->next) {
gbm_bo_destroy(bo);
return NULL;
}
drm_fb_set_buffer(output->primary_plane->next, buffer);
return &output->fb_plane;
}
static struct weston_plane *
drm_output_prepare_overlay_view(struct drm_output *output,
struct weston_view *ev)
{
struct weston_compositor *ec = output->base.compositor;
struct drm_compositor *c = (struct drm_compositor *)ec;
struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport;
struct drm_plane *p;
struct drm_plane *cur = NULL;
int found = 0;
struct gbm_bo *bo;
pixman_region32_t dest_rect, src_rect;
pixman_box32_t *box, tbox;
uint32_t format;
wl_fixed_t sx1, sy1, sx2, sy2;
if (c->gbm == NULL)
return NULL;
if (viewport->buffer.transform != output->base.transform)
return NULL;
if (viewport->buffer.scale != output->base.current_scale)
return NULL;
if (c->sprites_are_broken)
return NULL;
if (ev->output_mask != (1u << output->base.id))
return NULL;
if (ev->surface->buffer_ref.buffer == NULL)
return NULL;
if (ev->alpha != 1.0f)
return NULL;
if (wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource))
return NULL;
if (!drm_view_transform_supported(ev))
return NULL;
/* Find the current view this plane is assigned to, if any. */
wl_list_for_each(p, &c->plane_list, link) {
if (p->view != ev || p->output != output)
continue;
cur = p;
break;
}
bo = gbm_bo_import(c->gbm, GBM_BO_IMPORT_WL_BUFFER,
ev->surface->buffer_ref.buffer->resource,
GBM_BO_USE_SCANOUT);
wl_list_for_each(p, &c->plane_list, link) {
if (!bo)
continue;
if (!drm_plane_crtc_supported(output, p->possible_crtcs))
continue;
if (p->type != WDRM_PLANE_TYPE_OVERLAY)
continue;
format = drm_output_check_plane_format(p, ev, bo);
if (format == 0)
continue;
/* XXX: At this point, we need two runs through assign_planes;
* one to prepare any necessary views, and see if there
* are any currently-unused planes. This process may
* actually free up some planes for other views to use;
* if any planes have been freed up, we should do another
* pass to see if any planeless views can use any planes
* which have just been freed. But we want to cache what
* we can, so we're not, e.g., calling gbm_bo_import
* twice. */
if (!p->current && !p->next && !p->view) {
found = 1;
assert(p->output == NULL);
break;
}
/* XXX: Factor out all the above to see if we can just use the
* current plane still. */
if (p == cur) {
found = 1;
break;
}
}
/* If this buffer (view) was previously on a plane, but is being moved
* off, then get rid of it. */
if (cur && (!found || cur != p)) {
assert(cur->next == NULL);
cur->output = NULL;
wl_list_remove(&cur->view_destroy.link);
cur->view = NULL;
cur = NULL;
}
/* No planes available. */
if (!found) {
if (bo)
gbm_bo_destroy(bo);
return NULL;
}
if (cur && cur->current &&
cur->current->buffer_ref.buffer == ev->surface->buffer_ref.buffer) {
p->next = p->current;
} else {
p->next = drm_fb_get_from_bo(bo, c, format);
if (!p->next) {
gbm_bo_destroy(bo);
return NULL;
}
drm_fb_set_buffer(p->next, ev->surface->buffer_ref.buffer);
}
p->output = output;
p->view = ev;
wl_signal_add(&ev->destroy_signal, &p->view_destroy);
box = pixman_region32_extents(&ev->transform.boundingbox);
p->base.x = box->x1;
p->base.y = box->y1;
/*
* Calculate the source & dest rects properly based on actual
* position (note the caller has called weston_view_update_transform()
* for us already).
*/
pixman_region32_init(&dest_rect);
pixman_region32_intersect(&dest_rect, &ev->transform.boundingbox,
&output->base.region);
pixman_region32_translate(&dest_rect, -output->base.x, -output->base.y);
box = pixman_region32_extents(&dest_rect);
tbox = weston_transformed_rect(output->base.width,
output->base.height,
output->base.transform,
output->base.current_scale,
*box);
p->dest_x = tbox.x1;
p->dest_y = tbox.y1;
p->dest_w = tbox.x2 - tbox.x1;
p->dest_h = tbox.y2 - tbox.y1;
pixman_region32_fini(&dest_rect);
pixman_region32_init(&src_rect);
pixman_region32_intersect(&src_rect, &ev->transform.boundingbox,
&output->base.region);
box = pixman_region32_extents(&src_rect);
weston_view_from_global_fixed(ev,
wl_fixed_from_int(box->x1),
wl_fixed_from_int(box->y1),
&sx1, &sy1);
weston_view_from_global_fixed(ev,
wl_fixed_from_int(box->x2),
wl_fixed_from_int(box->y2),
&sx2, &sy2);
if (sx1 < 0)
sx1 = 0;
if (sy1 < 0)
sy1 = 0;
if (sx2 > wl_fixed_from_int(ev->surface->width))
sx2 = wl_fixed_from_int(ev->surface->width);
if (sy2 > wl_fixed_from_int(ev->surface->height))
sy2 = wl_fixed_from_int(ev->surface->height);
tbox.x1 = sx1;
tbox.y1 = sy1;
tbox.x2 = sx2;
tbox.y2 = sy2;
tbox = weston_transformed_rect(wl_fixed_from_int(ev->surface->width),
wl_fixed_from_int(ev->surface->height),
viewport->buffer.transform,
viewport->buffer.scale,
tbox);
p->src_x = tbox.x1 << 8;
p->src_y = tbox.y1 << 8;
p->src_w = (tbox.x2 - tbox.x1) << 8;
p->src_h = (tbox.y2 - tbox.y1) << 8;
pixman_region32_fini(&src_rect);
return &p->base;
}
static struct weston_plane *
drm_output_prepare_cursor_view(struct drm_output *output,
struct weston_view *ev)
{
struct drm_compositor *c =
(struct drm_compositor *)output->base.compositor;
struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport;
if (c->gbm == NULL)
return NULL;
if (output->cursor_plane == NULL)
return NULL;
if (output->base.transform != WL_OUTPUT_TRANSFORM_NORMAL)
return NULL;
if (viewport->buffer.scale != output->base.current_scale)
return NULL;
if (output->cursor_view)
return NULL;
if (ev->output_mask != (1u << output->base.id))
return NULL;
if (c->cursors_are_broken)
return NULL;
if (ev->geometry.scissor_enabled)
return NULL;
if (ev->surface->buffer_ref.buffer == NULL ||
!wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource) ||
ev->surface->width > c->cursor_width ||
ev->surface->height > c->cursor_height)
return NULL;
output->cursor_view = ev;
return &output->cursor_plane->base;
}
static int
atomic_plane_add(drmModeAtomicReq *req, struct drm_plane *plane,
enum wdrm_plane_property prop, uint64_t value)
{
struct property_item *item = &plane->props.item[prop];
uint32_t mask = 1U << prop;
if (!item->id)
return -1;
if ((plane->props.value_valid_mask |
plane->props.value_pend_mask) & mask &&
item->value == value)
return 0;
plane_property_debug(plane, prop, value);
if (drmModeAtomicAddProperty(req, plane->plane_id, item->id, value) < 0)
return -1;
plane->props.value_valid_mask &= ~mask;
item->value = value;
plane->props.value_pend_mask |= mask;
return 0;
}
/**
* Populates an existing atomic request with the properties required to change
* the visible framebuffer.
*
* XXX: bool properties are lame
*
* @param output Output for this plane to be displayed on
* @param p Plane to configure
* @param req Atomic request to populate with changes
* @param force If true, will populate request regardless of current state
* @ret -1 on error, 1 if any properties were set, or 0 if no change
*/
static int
drm_output_populate_atomic_plane(struct drm_output *output, struct drm_plane *p,
drmModeAtomicReq *req, bool force)
{
struct drm_compositor *compositor =
(struct drm_compositor *) output->base.compositor;
uint32_t fb_id = 0;
int ret = 0;
if (force)
p->props.value_valid_mask = 0;
assert(p->output == output);
if (p->next && !compositor->sprites_hidden)
fb_id = p->next->fb_id;
if (fb_id == 0) {
output = NULL;
p->src_x = 0;
p->src_y = 0;
p->src_w = 0;
p->src_h = 0;
p->dest_x = 0;
p->dest_y = 0;
p->dest_w = 0;
p->dest_h = 0;
}
drm_plane_update_begin(p);
ret |= atomic_plane_add(req, p, WDRM_PLANE_CRTC_ID,
output ? output->crtc_id : 0);
ret |= atomic_plane_add(req, p, WDRM_PLANE_FB_ID, fb_id);
ret |= atomic_plane_add(req, p, WDRM_PLANE_SRC_X, p->src_x);
ret |= atomic_plane_add(req, p, WDRM_PLANE_SRC_Y, p->src_y);
ret |= atomic_plane_add(req, p, WDRM_PLANE_SRC_W, p->src_w);
ret |= atomic_plane_add(req, p, WDRM_PLANE_SRC_H, p->src_h);
ret |= atomic_plane_add(req, p, WDRM_PLANE_CRTC_X, p->dest_x);
ret |= atomic_plane_add(req, p, WDRM_PLANE_CRTC_Y, p->dest_y);
ret |= atomic_plane_add(req, p, WDRM_PLANE_CRTC_W, p->dest_w);
ret |= atomic_plane_add(req, p, WDRM_PLANE_CRTC_H, p->dest_h);
if (ret)
return -1;
if (!p->props.value_pend_mask)
return 0;
return 1;
}
static void
drm_assign_planes(struct weston_output *output_base)
{
struct drm_compositor *c =
(struct drm_compositor *)output_base->compositor;
struct drm_output *output = (struct drm_output *)output_base;
struct weston_view *ev, *next;
pixman_region32_t overlap, surface_overlap;
struct weston_plane *primary, *next_plane;
drmModeAtomicReqPtr req = NULL;
int ret;
/*
* Find a surface for each sprite in the output using some heuristics:
* 1) size
* 2) frequency of update
* 3) opacity (though some hw might support alpha blending)
* 4) clipping (this can be fixed with color keys)
*
* The idea is to save on blitting since this should save power.
* If we can get a large video surface on the sprite for example,
* the main display surface may not need to update at all, and
* the client buffer can be used directly for the sprite surface
* as we do for flipping full screen surfaces.
*/
pixman_region32_init(&overlap);
primary = &c->base.primary_plane;
if (c->atomic_modeset) {
req = drmModeAtomicAlloc();
if (!req)
return;
}
wl_list_for_each_safe(ev, next, &c->base.view_list, link) {
struct weston_surface *es = ev->surface;
/* Test whether this buffer can ever go into a plane:
* non-shm, or small enough to be a cursor.
*
* Also, keep a reference when using the pixman renderer.
* That makes it possible to do a seamless switch to the GL
* renderer and since the pixman renderer keeps a reference
* to the buffer anyway, there is no side effects.
*/
if (c->use_pixman ||
(es->buffer_ref.buffer &&
(!wl_shm_buffer_get(es->buffer_ref.buffer->resource) ||
(ev->surface->width <= 64 && ev->surface->height <= 64))))
es->keep_buffer = true;
else
es->keep_buffer = false;
pixman_region32_init(&surface_overlap);
pixman_region32_intersect(&surface_overlap, &overlap,
&ev->transform.boundingbox);
next_plane = NULL;
if (pixman_region32_not_empty(&surface_overlap))
next_plane = primary;
if (next_plane == NULL)
next_plane = drm_output_prepare_cursor_view(output, ev);
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(output, ev);
if (next_plane == NULL)
next_plane = drm_output_prepare_overlay_view(output, ev);
/* Check whether or not the kernel likes this import. */
if (c->atomic_modeset && next_plane && next_plane != primary) {
struct drm_plane *plane =
container_of(next_plane, struct drm_plane, base);
int saved_cursor = drmModeAtomicGetCursor(req);
/* This is not matched with an update_success, as we
* never actually commit it, just check that it could
* potentially be committed at some stage. */
drm_plane_update_begin(plane);
ret = drm_output_populate_atomic_plane(output, plane,
req, false);
if (ret > 0)
ret = drmModeAtomicCommit(c->drm.fd, req,
DRM_MODE_ATOMIC_TEST_ONLY,
output);
if (ret != 0) {
drmModeAtomicSetCursor(req, saved_cursor);
if (plane->next) {
drm_output_release_fb(output,
plane->next);
plane->next = NULL;
}
plane->output = NULL;
plane->view = NULL;
wl_list_remove(&plane->view_destroy.link);
next_plane = NULL;
}
}
if (next_plane == NULL)
next_plane = primary;
weston_view_move_to_plane(ev, next_plane);
if (next_plane == primary)
pixman_region32_union(&overlap, &overlap,
&ev->transform.boundingbox);
if (next_plane == primary ||
(output->cursor_plane &&
next_plane == &output->cursor_plane->base)) {
/* cursor plane involves a copy */
ev->psf_flags = 0;
} else {
/* All other planes are a direct scanout of a
* single client buffer.
*/
ev->psf_flags = PRESENTATION_FEEDBACK_KIND_ZERO_COPY;
}
pixman_region32_fini(&surface_overlap);
}
pixman_region32_fini(&overlap);
if (c->atomic_modeset)
drmModeAtomicFree(req);
}
Yeah, the logic for populating the atomic req seems mostly ok, but is
it missing the primary weston_plane assignment to the primary DRM plane?
Or if we are going to do a modeset, shouldn't that be here somehow too?
I mean for cases where the primary DRM plane will actually change. If
it doesn't change, then I'd assume DRM maintains the state that is not
touched.
So, the DRM planes we have not assigned yet but were enabled, shouldn't
they be disabled in the TEST_ONLY commit? Otherwise they will contain
old state and we validate against some new DRM plane states with some
previous DRM plane states? That might lead to unnecessary failures from
TEST_ONLY commit if we would eventually end up disabling or updating
the already enabled DRM planes.
The TEST_ONLY call should contain everything the final atomic commit
call will, right? So once assign_planes is done, we should already have
the complete atomic req, and in output_repaint what's left to do is to
actually render the leftovers and commit.
I suppose this means we need to prepare the next buffer for a primary
weston_plane already at assign_planes start and use it in the TEST_ONLY
commit, before we even render to it. I hope it won't screw up any
fencing in DRM or EGL. But can we get it out of EGL/GBM *before* we
render it? We need an equivalent buffer that the primary DRM plane's
buffer is going to be for TEST_ONLY commits, so that we can check what
can go to overlays, so we know what we will need to render.
The atomic req in the process of being built would look like this:
- primary plane setup
- other plane setup 1 (succeeded)
- other plane setup 2 (succeeded)
<- cursor A
- other plane setup 3 (to be tested)
<- cursor B
- other plane setup 4 (disable)
...
- other plane setup N (disable)
If the TEST_ONLY commit succeeds, rewind to cursor B and try the next
plane. If it fails, rewind to cursor A, fill in the rest of disables,
and be done with it.
Does this make sense?
This is assuming we cannot do without the primary plane. If it was
possible to drive truely universal planes, we would not know if we need
a primary plane until we know if there is anything on it. You'd have to
first assume the primary plane is needed, test the additional plane
setups, and then if we don't need the primary plane, test once more
without it.
Thanks,
pq
More information about the wayland-devel
mailing list