[PATCH v17 04/14] compositor-drm: Add modifiers to GBM dmabuf import
Add support for the GBM_BO_IMPORT_FD_MODIFIER path, which allows us to import multi-plane dmabufs, as well as format modifiers. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- configure.ac | 6 +- libweston/compositor-drm.c | 201 + 2 files changed, 160 insertions(+), 47 deletions(-) diff --git a/configure.ac b/configure.ac index adb998dda..ef55ace36 100644 --- a/configure.ac +++ b/configure.ac @@ -212,9 +212,9 @@ if test x$enable_drm_compositor = xyes; then PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78], [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic API])], [AC_MSG_WARN([libdrm does not support atomic modesetting, will omit that capability])]) - PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 10.2], - [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports dmabuf import])], - [AC_MSG_WARN([gbm does not support dmabuf import, will omit that capability])]) + PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2], + [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import with modifiers])], + [AC_MSG_WARN([GBM does not support dmabuf import, will omit that capability])]) fi diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 29fa98da6..ae4a4a542 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -326,6 +326,7 @@ struct drm_mode { enum drm_fb_type { BUFFER_INVALID = 0, /**< never used */ BUFFER_CLIENT, /**< directly sourced from client */ + BUFFER_DMABUF, /**< imported from linux_dmabuf client */ BUFFER_PIXMAN_DUMB, /**< internal Pixman rendering */ BUFFER_GBM_SURFACE, /**< internal EGL rendering */ BUFFER_CURSOR, /**< internal cursor buffer */ @@ -1038,6 +1039,145 @@ drm_fb_ref(struct drm_fb *fb) return fb; } +static void +drm_fb_destroy_dmabuf(struct drm_fb *fb) +{ + /* We deliberately do not close the GEM handles here; GBM manages +* their lifetime through the BO. */ + if (fb->bo) + gbm_bo_destroy(fb->bo); + drm_fb_destroy(fb); +} + +static struct drm_fb * +drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf, + struct drm_backend *backend, bool is_opaque) +{ +#ifdef HAVE_GBM_FD_IMPORT + struct drm_fb *fb; + struct gbm_import_fd_data import_legacy = { + .width = dmabuf->attributes.width, + .height = dmabuf->attributes.height, + .format = dmabuf->attributes.format, + .stride = dmabuf->attributes.stride[0], + .fd = dmabuf->attributes.fd[0], + }; + struct gbm_import_fd_modifier_data import_mod = { + .width = dmabuf->attributes.width, + .height = dmabuf->attributes.height, + .format = dmabuf->attributes.format, + .num_fds = dmabuf->attributes.n_planes, + .modifier = dmabuf->attributes.modifier[0], + }; + int i; + + /* XXX: TODO: +* +* Currently the buffer is rejected if any dmabuf attribute +* flag is set. This keeps us from passing an inverted / +* interlaced / bottom-first buffer (or any other type that may +* be added in the future) through to an overlay. Ultimately, +* these types of buffers should be handled through buffer +* transforms and not as spot-checks requiring specific +* knowledge. */ + if (dmabuf->attributes.flags) + return NULL; + + fb = zalloc(sizeof *fb); + if (fb == NULL) + return NULL; + + fb->refcnt = 1; + fb->type = BUFFER_DMABUF; + + BUILD_BUG_ON(ARRAY_LENGTH(import_mod.fds) != +ARRAY_LENGTH(dmabuf->attributes.fd)); + BUILD_BUG_ON(sizeof(import_mod.fds) != sizeof(dmabuf->attributes.fd)); + memcpy(import_mod.fds, dmabuf->attributes.fd, sizeof(import_mod.fds)); + + BUILD_BUG_ON(ARRAY_LENGTH(import_mod.strides) != +ARRAY_LENGTH(dmabuf->attributes.stride)); + BUILD_BUG_ON(sizeof(import_mod.strides) != +sizeof(dmabuf->attributes.stride)); + memcpy(import_mod.strides, dmabuf->attributes.stride, + sizeof(import_mod.strides)); + + BUILD_BUG_ON(ARRAY_LENGTH(import_mod.offsets) != +ARRAY_LENGTH(dmabuf->attributes.offset)); + BUILD_BUG_ON(sizeof(import_mod.offsets) != +sizeof(dmabuf->attributes.offset)); + memcpy(import_mod.offsets, dmabuf->attributes.offset, + sizeof(import_mod.offsets)); + + /* The legacy FD-import path does not allow us to supply modifiers, +* multiple planes, or buffer offsets. */ +
[PATCH v17 06/14] compositor-drm: Use GBM modifier API
Now that we collect information about which modifiers are supported for KMS display, and are able to create KMS framebuffers with modifiers, begin using the modifier-aware GBM API. Client buffers from dmabuf already store multi-plane and modifier information into drm_fb. Extend this to drm_fb_get_from_bo(), used for wl_buffer, cursor, and gbm_surface buffers. wl_buffer buffers should by convention not require modifiers. Cursor buffers must not require modifiers, as they should be linear. Prior to this patch, GBM buffers must have been single-planar, and able to used without explicitly naming modifiers. Using gbm_surface_create_with_modifiers allows us to pass the list of modifiers acceptable to KMS for scanout to GBM, so it can allocate multi-planar buffers or those which are otherwise only addressible with modifiers. On platforms supporting and preferring modifiers for scanout, this means that the gbm_bos we get from our scanout surface need to use the extended API to query multiple planes, offsets, modifiers, etc. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- configure.ac | 3 ++ libweston/compositor-drm.c | 57 -- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index c550198ae..357b6471e 100644 --- a/configure.ac +++ b/configure.ac @@ -215,6 +215,9 @@ if test x$enable_drm_compositor = xyes; then PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83], [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports modifier advertisement])], [AC_MSG_WARN([libdrm does not support modifier advertisement])]) + PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM_MODIFIERS, [gbm >= 17.1], + [AC_DEFINE([HAVE_GBM_MODIFIERS], 1, [GBM supports modifiers])], + [AC_MSG_WARN([GBM does not support modifiers])]) PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2], [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import with modifiers])], [AC_MSG_WARN([GBM does not support dmabuf import, will omit that capability])]) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index bc4022201..acaf2639e 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1189,6 +1189,9 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, bool is_opaque, enum drm_fb_type type) { struct drm_fb *fb = gbm_bo_get_user_data(bo); +#ifdef HAVE_GBM_MODIFIERS + int i; +#endif if (fb) { assert(fb->type == type); @@ -1202,15 +1205,25 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, fb->type = type; fb->refcnt = 1; fb->bo = bo; + fb->fd = backend->drm.fd; fb->width = gbm_bo_get_width(bo); fb->height = gbm_bo_get_height(bo); + fb->format = pixel_format_get_info(gbm_bo_get_format(bo)); + fb->size = 0; + +#ifdef HAVE_GBM_MODIFIERS + fb->modifier = gbm_bo_get_modifier(bo); + for (i = 0; i < gbm_bo_get_plane_count(bo); i++) { + fb->strides[i] = gbm_bo_get_stride_for_plane(bo, i); + fb->handles[i] = gbm_bo_get_handle_for_plane(bo, i).u32; + fb->offsets[i] = gbm_bo_get_offset(bo, i); + } +#else fb->strides[0] = gbm_bo_get_stride(bo); fb->handles[0] = gbm_bo_get_handle(bo).u32; - fb->format = pixel_format_get_info(gbm_bo_get_format(bo)); fb->modifier = DRM_FORMAT_MOD_INVALID; - fb->size = 0; - fb->fd = backend->drm.fd; +#endif if (!fb->format) { weston_log("couldn't look up format 0x%lx\n", @@ -4355,13 +4368,39 @@ drm_output_init_egl(struct drm_output *output, struct drm_backend *b) fallback_format_for(output->gbm_format), }; int n_formats = 1; + struct weston_mode *mode = output->base.current_mode; + struct drm_plane *plane = output->scanout_plane; + unsigned int i; + + for (i = 0; i < plane->count_formats; i++) { + if (plane->formats[i].format == output->gbm_format) + break; + } + + if (i == plane->count_formats) { + weston_log("format 0x%x not supported by output %s\n", + output->gbm_format, output->base.name); + return -1; + } + +#ifdef HAVE_GBM_MODIFIERS + if (plane->formats[i].count_modifiers > 0) { + output->gbm_surface = + gbm_surface_create_with_modifiers(b->gbm, + mode->width, + mode->height, +
[PATCH v17 13/14] compositor-drm: Relax plane restrictions for atomic
Since we now incrementally test atomic state as we build it, we can loosen restrictions on what we can do with planes, and let the kernel tell us whether or not it's OK. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 0a3f524f5..b97872aa1 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1942,6 +1942,7 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, enum drm_output_propose_state_mode mode) { struct drm_output *output = output_state->output; + struct drm_backend *b = to_drm_backend(output->base.compositor); struct drm_plane *scanout_plane = output->scanout_plane; struct drm_plane_state *state; struct drm_plane_state *state_old = NULL; @@ -1966,7 +1967,7 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, return NULL; /* Can't change formats with just a pageflip */ - if (fb->format->format != output->gbm_format) { + if (!b->atomic_modeset && fb->format->format != output->gbm_format) { drm_fb_unref(fb); return NULL; } @@ -1994,15 +1995,18 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, if (!drm_plane_state_coords_for_view(state, ev)) goto err; - /* The legacy API does not let us perform cropping or scaling. */ - if (state->src_x != 0 || state->src_y != 0 || - state->src_w != state->dest_w << 16 || - state->src_h != state->dest_h << 16 || - state->dest_x != 0 || state->dest_y != 0 || + if (state->dest_x != 0 || state->dest_y != 0 || state->dest_w != (unsigned) output->base.current_mode->width || state->dest_h != (unsigned) output->base.current_mode->height) goto err; + /* The legacy API does not let us perform cropping or scaling. */ + if (!b->atomic_modeset && + (state->src_x != 0 || state->src_y != 0 || +state->src_w != state->dest_w << 16 || +state->src_h != state->dest_h << 16)) + goto err; + if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) { drm_plane_state_free(state_old, false); return state; @@ -3092,8 +3096,9 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, state->ev = ev; state->output = output; drm_plane_state_coords_for_view(state, ev); - if (state->src_w != state->dest_w << 16 || - state->src_h != state->dest_h << 16) { + if (!b->atomic_modeset && + (state->src_w != state->dest_w << 16 || +state->src_h != state->dest_h << 16)) { drm_plane_state_put_back(state); continue; } -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 05/14] compositor-drm: Support plane IN_FORMATS
From: Sergi Granell The per-plane IN_FORMATS KMS property describes the format/modifier combinations supported for display on this plane. Read and parse this format, storing the data in each plane, so we can know which combinations might work, and which combinations definitely will not work. Similarly to f11ec02cad40 ("compositor-drm: Extract overlay FB import to helper"), we now use this when considering promoting a view to overlay planes. If the framebuffer's modifier is definitely not supported by the plane, we do not attempt to use that plane for that view. This will also be used in a follow-patch, passing the list of modifiers to GBM surface allocation to allow it to allocate more optimal buffers. Signed-off-by: Sergi Granell Reviewed-by: Daniel Stone Tested-by: Emre Ucan --- configure.ac | 3 + libweston/compositor-drm.c | 134 ++--- 2 files changed, 128 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index ef55ace36..c550198ae 100644 --- a/configure.ac +++ b/configure.ac @@ -212,6 +212,9 @@ if test x$enable_drm_compositor = xyes; then PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78], [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic API])], [AC_MSG_WARN([libdrm does not support atomic modesetting, will omit that capability])]) + PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83], + [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports modifier advertisement])], + [AC_MSG_WARN([libdrm does not support modifier advertisement])]) PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2], [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import with modifiers])], [AC_MSG_WARN([GBM does not support dmabuf import, will omit that capability])]) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index ae4a4a542..bc4022201 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -144,6 +144,7 @@ enum wdrm_plane_property { WDRM_PLANE_CRTC_H, WDRM_PLANE_FB_ID, WDRM_PLANE_CRTC_ID, + WDRM_PLANE_IN_FORMATS, WDRM_PLANE__COUNT }; @@ -185,6 +186,7 @@ static const struct drm_property_info plane_props[] = { [WDRM_PLANE_CRTC_H] = { .name = "CRTC_H", }, [WDRM_PLANE_FB_ID] = { .name = "FB_ID", }, [WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", }, + [WDRM_PLANE_IN_FORMATS] = { .name = "IN_FORMATS" }, }; /** @@ -448,7 +450,11 @@ struct drm_plane { struct wl_list link; - uint32_t formats[]; + struct { + uint32_t format; + uint32_t count_modifiers; + uint64_t *modifiers; + } formats[]; }; struct drm_head { @@ -2992,7 +2998,19 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, /* Check whether the format is supported */ for (i = 0; i < p->count_formats; i++) { - if (p->formats[i] == fb->format->format) + unsigned int j; + + if (p->formats[i].format != fb->format->format) + continue; + + if (fb->modifier == DRM_FORMAT_MOD_INVALID) + break; + + for (j = 0; j < p->formats[i].count_modifiers; j++) { + if (p->formats[i].modifiers[j] == fb->modifier) + break; + } + if (j != p->formats[i].count_modifiers) break; } if (i == p->count_formats) @@ -3637,6 +3655,100 @@ init_pixman(struct drm_backend *b) return pixman_renderer_init(b->compositor); } +#ifdef HAVE_DRM_FORMATS_BLOB +static inline uint32_t * +formats_ptr(struct drm_format_modifier_blob *blob) +{ + return (uint32_t *)(((char *)blob) + blob->formats_offset); +} + +static inline struct drm_format_modifier * +modifiers_ptr(struct drm_format_modifier_blob *blob) +{ + return (struct drm_format_modifier *) + (((char *)blob) + blob->modifiers_offset); +} +#endif + +/** + * Populates the plane's formats array, using either the IN_FORMATS blob + * property (if available), or the plane's format list if not. + */ +static int +drm_plane_populate_formats(struct drm_plane *plane, const drmModePlane *kplane, + const drmModeObjectProperties *props) +{ + unsigned i; +#ifdef HAVE_DRM_FORMATS_BLOB + drmModePropertyBlobRes *blob; + struct drm_format_modifier_blob *fmt_mod_blob; + struct drm_format_modifier *blob_modifiers; + uint32_t *blob_formats; + uint32_t blob_id; + +
[PATCH v17 08/14] compositor-drm: Ignore views on other outputs
When we come to assign_planes, try very hard to ignore views which are only visible on other outputs, rather than forcibly moving them to the primary plane, which causes damage all round and unnecessary repaints. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen Tested-by: Emre Ucan --- libweston/compositor-drm.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 2eb3d4073..ca885f96e 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1552,10 +1552,6 @@ drm_fb_get_from_view(struct drm_output_state *state, struct weston_view *ev) struct linux_dmabuf_buffer *dmabuf; struct drm_fb *fb; - /* Don't import buffers which span multiple outputs. */ - if (ev->output_mask != (1u << output->base.id)) - return NULL; - if (ev->alpha != 1.0f) return NULL; @@ -3125,10 +3121,6 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state, if (plane->state_cur->output && plane->state_cur->output != output) return NULL; - /* Don't import buffers which span multiple outputs. */ - if (ev->output_mask != (1u << output->base.id)) - return NULL; - /* We use GBM to import SHM buffers. */ if (b->gbm == NULL) return NULL; @@ -3286,6 +3278,16 @@ drm_output_propose_state(struct weston_output *output_base, wl_list_for_each(ev, _base->compositor->view_list, link) { struct weston_plane *next_plane = NULL; + /* If this view doesn't touch our output at all, there's no +* reason to do anything with it. */ + if (!(ev->output_mask & (1u << output->base.id))) + continue; + + /* We only assign planes to views which are exclusively present +* on our output. */ + if (ev->output_mask != (1u << output->base.id)) + next_plane = primary; + /* Since we process views from top to bottom, we know that if * the view intersects the calculated renderer region, it must * be part of, or occluded by, it, and cannot go on a plane. */ @@ -3335,6 +3337,11 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) wl_list_for_each(ev, _base->compositor->view_list, link) { struct drm_plane *target_plane = NULL; + /* If this view doesn't touch our output at all, there's no +* reason to do anything with it. */ + if (!(ev->output_mask & (1u << output->base.id))) + continue; + /* Test whether this buffer can ever go into a plane: * non-shm, or small enough to be a cursor. * -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 00/14] DRM backend planes and modifiers
Hi, Another re-send after Pekka's comments on the modifiers patches. Patches 01-12 from the last series were merged after the last round. Patch 1 is new; patches 2 and 3 were extracted from patches 5 and 6. Patches 4-6 have otherwise seen changes after Pekka's review; all the comments were addressed. Patches 7-14 are unchanged from the previous submission, except where a rebase may have modified them. This iteration of the series is available from: https://gitlab.collabora.com/daniels/weston/commits/wip/2018-07/atomic-v17 The tree on GitLab includes a 'drm debug' patch at the top, which adds a lot of debug spew to the KMS backend. It is very useful to cherry-pick this patch and include any debug information when reporting bugs on this series, however the log volume is great enough that it can visibly slow the compositor. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 02/14] compositor-drm: Avoid cast by using unsigned loop index
ARRAY_LENGTH returns a size_t; rather than casting its result to int so we can compare to our signed index variable, just declare the index as a compatible type in the first place. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index e09719ce4..7753dfba6 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -914,7 +914,7 @@ drm_fb_addfb(struct drm_fb *fb) int ret = -EINVAL; #ifdef HAVE_DRM_ADDFB2_MODIFIERS uint64_t mods[4] = { }; - int i; + size_t i; #endif /* If we have a modifier set, we must only use the WithModifiers @@ -923,7 +923,7 @@ drm_fb_addfb(struct drm_fb *fb) /* KMS demands that if a modifier is set, it must be the same * for all planes. */ #ifdef HAVE_DRM_ADDFB2_MODIFIERS - for (i = 0; i < (int) ARRAY_LENGTH(mods) && fb->handles[i]; i++) + for (i = 0; i < ARRAY_LENGTH(mods) && fb->handles[i]; i++) mods[i] = fb->modifier; ret = drmModeAddFB2WithModifiers(fb->fd, fb->width, fb->height, fb->format->format, -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v17 01/14] helpers: Add BUILD_BUG_ON from Linux kernel
Import the trivial definition of BUILD_BUG_ON() from the Linux kernel. This evaluates an expression such that it will call sizeof() on a negative-length array if the expression is false, generating a compiler error. This will be used in future patches to ensure two array lengths don't go out of sync. Signed-off-by: Daniel Stone --- shared/helpers.h | 25 + 1 file changed, 25 insertions(+) diff --git a/shared/helpers.h b/shared/helpers.h index 46f745d1b..9ecc3c4e9 100644 --- a/shared/helpers.h +++ b/shared/helpers.h @@ -100,6 +100,31 @@ extern "C" { (type *)( (char *)__mptr - offsetof(type,member) );}) #endif +/** + * Build-time static assertion support + * + * A build-time equivalent to assert(), will generate a compilation error + * if the supplied condition does not evaluate true. + * + * The following example demonstrates use of BUILD_BUG_ON to ensure that + * arrays which are supposed to mirror each other have a consistent + * size. + * + * @code + * int small[4]; + * long expanded[4]; + * + * BUILD_BUG_ON(ARRAY_LENGTH(small) != ARRAY_LENGTH(expanded)); + * for (i = 0; i < ARRAY_LENGTH(small); i++) + * expanded[i] = small[4]; + * @endcode + * + * @param condition Expression to check for truth + */ +#ifndef BUILD_BUG_ON +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)])) +#endif + #ifdef __cplusplus } #endif -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 14/23] compositor-drm: Support plane IN_FORMATS
Hi, On Mon, 9 Jul 2018 at 11:08, Pekka Paalanen wrote: > On Thu, 5 Jul 2018 18:16:41 +0100 Daniel Stone wrote: > > The per-plane IN_FORMATS property adds information about format > > modifiers; we can use these to both feed GBM with the set of modifiers > > we want to use for rendering, and also as an early-out test when we're > > trying to see if a FB will go on a particular plane. > > I can see the early-out test, but where does this patch affect feeding > GBM? I've clarified the commit message. > > + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id); > > + if (!blob) > > + goto fallback; > > + > > + fmt_mod_blob = blob->data; > > + blob_formats = formats_ptr(fmt_mod_blob); > > + blob_modifiers = modifiers_ptr(fmt_mod_blob); > > + > > + assert(plane->count_formats == fmt_mod_blob->count_formats); > > I don't think this should be an assert. We are comparing to something > the kernel just told us, so if they don't match then it is either a > wrong assumption or a kernel bug. Which one is it? Almost: we're comparing the number of formats inside the format-blob container, to the number of formats from drmModeGetPlane. Any mismatch between them is a kernel bug, as it's an invariant of the API that the two must be equal. Nothing we can do about it if so, but given the result would be overwriting random bits from the heap, and that quite a few driver developers use Weston as a test, leaving the assert in seemed wise. I'm not hugely attached to it though, so even though I've left it in for v17, do feel free to remove it when pushing. > > + /* No IN_MODIFIERS blob available, so just use the old. */ > > + for (i = 0; i < kplane->count_formats; i++) > > + plane->formats[i].format = kplane->formats[i]; > > + > > + return 0; > > +} > > Otherwise the patch looks fine. I've fixed up the references to 'IN_MODIFIERS' (sic: IN_FORMAT) whilst here. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 13/23] compositor-drm: Add modifiers to GBM dmabuf import
Hi Pekka, On Mon, 9 Jul 2018 at 10:39, Pekka Paalanen wrote: > On Thu, 5 Jul 2018 18:16:40 +0100 > Daniel Stone wrote: > > +/* XXX: TODO: > > + * > > + * Currently the buffer is rejected if any dmabuf attribute > > + * flag is set. This keeps us from passing an inverted / > > + * interlaced / bottom-first buffer (or any other type that may > > + * be added in the future) through to an overlay. Ultimately, > > + * these types of buffers should be handled through buffer > > + * transforms and not as spot-checks requiring specific > > + * knowledge. */ > > Bad whitespace in the above indentation. > > I'm also not sure I agree with how things should be done instead. It's > possible the dmabuf wl_buffer is created by a library (libEGL, media, > etc.) which may not communicate these details outside, may not give a > possibility to add state to the wl_surface.commit, or may not actually > drive wl_surface itself. There could be many client-side designs where > this detail is not combinable with wl_surface state in the client. > > Also, only y-inverted would be possible to deal with buffer transform. > The other flags probably do require specific knowledge if they were to > be implemented properly. > > Hmm, it's an interesting question how buffer_damage should be > interpreted in case of e.g. y-inverted dmabuf. Micah wrote the above comment, which I'm just transporting from one place to another. My interpretation of what he wrote is _not_ that the client should use protocol calls to add a transformation, but that setting the flag should result in a transformation in Weston core, rather than being checked and accounted for directly in the end users of that buffer. I probably read it like that because I agree with it. ;) Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] MAINTAINERS: update drm tree
Hi, On Mon, 9 Jul 2018 at 08:38, Daniel Vetter wrote: > On Fri, Jul 06, 2018 at 04:43:09PM +0100, Mike Lothian wrote: > > Any change of this moving to https or the gitlab instance where its on as > > default? > > Moving all the drm repos over to gitlab is somewhere on the plans, but we > need to figure out a transition strategy for the scripts and all the > committers. So not something that can happen instantly unfortunately. > > Also fd.o admins asked we don't use the kerne repos as the first ones to > switch, since they're the biggest (the drm move here started about the > same time as gitlab.fd.o went productive). Yeah, right now GitLab is holding up quite well for other projects, but kernel repos are a fairly unique stress test - probably only Chromium/WebKit/etc are worse. There are a couple of infrastructure changes coming up within GitLab and how we've got it deployed on fd.o which I want to see land before we switch the kernel repos over. They decouple quite a few things and allow us to scale the servers better to deal with load and storage, as well as allow us to more quickly upgrade the services and make config changes with less downtime. I'm expecting to see that happen around XDC time. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 08/23] compositor-drm: Use plane_coords_for_view for cursor
Hi Pekka, On Fri, 6 Jul 2018 at 14:14, Pekka Paalanen wrote: > On Thu, 5 Jul 2018 18:16:35 +0100 Daniel Stone wrote: > > Use the new helper to populate the cursor state as well, with some > > special-case handling to account for how we always upload a full-size > > BO. > > > > As this now fully takes care of buffer transformations, HiDPI client > > cursors work, and we also clip the cursor plane completely to CRTC > > bounds. > > > > Signed-off-by: Daniel Stone > > Reported-by: Derek Foreman > > Tested-by: Emre Ucan > > Fixes: https://gitlab.freedesktop.org/wayland/weston/issues/118 > > --- > > libweston/compositor-drm.c | 68 ++ > > 1 file changed, 32 insertions(+), 36 deletions(-) > > The subject should say plane_state_coords_for_view. Er ... quite. > > @@ -2857,18 +2857,16 @@ cursor_bo_update(struct drm_backend *b, struct > > gbm_bo *bo, > > > > assert(buffer && buffer->shm_buffer); > > assert(buffer->shm_buffer == wl_shm_buffer_get(buffer->resource)); > > - assert(ev->surface->width <= b->cursor_width); > > - assert(ev->surface->height <= b->cursor_height); > > Rather than removing these asserts, it would be better to fix them to > use buffer size. It would catch overflowing buf. > > An assert to ensure bytes-per-pixel is the hardcoded four below would > be a nice addition. Thanks, I've included these (apart from the bpp assertion, which I'll send as a follow-on), as well as all your other suggested fixups (as discussed on IRC), and pushed up to 'compositor-drm: Support modifiers for drm_fb' as those four patches already had R-b. I've force-pushed the remainder of the series, rebased on top of what I've just pushed, to the wip/2018-07/atomic-v16 branch. Thanks a lot for the very speedy review! Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 15/23] compositor-drm: Support modifiers with GBM
Hey Emil, On Fri, 6 Jul 2018 at 11:02, Emil Velikov wrote: > On 5 July 2018 at 18:16, Daniel Stone wrote: > > Use the extended GBM allocator interface to support modifiers and > > multi-planar BOs. > > Instead of such lovely checks and multiple #ifdef blocks through in > the code, one can use weak symbols. > See kmscube code has some examples. > > That said, it's something that could be handled at a later stage. > There's no point in delaying the series over that detail. Yeah, all the options are lovely in their own way. I'd happily check and review a patch to make it use weak functions to cover all the new GBM API though. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v16 07/23] compositor-drm: Use plane_state_coords_for_view for scanout
Hi Pekka, On Fri, 6 Jul 2018 at 13:48, Pekka Paalanen wrote: > > On Thu, 5 Jul 2018 18:16:34 +0100 > Daniel Stone wrote: > > + /* Check the view spans exactly the output size, calculated in the > > + * logical co-ordinate space. */ > > + extents = pixman_region32_extents(>transform.boundingbox); > > + if (extents->x1 != output->base.x || > > + extents->y1 != output->base.y || > > + extents->x2 != output->base.x + output->base.width || > > + extents->y2 != output->base.y + output->base.height) > > return NULL; > > Isn't this check a sub-set of the dest and src rectangles check below? > Is this needed for something? Just an early exit? Yep, it allows us to skip trying to import a FB or set up plane state if we know the view can never be compatible. As you say, the below check is far more comprehensive, but this is enough to skip obviously pointless work. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] compositor-drm: Define DPMS property as an enum
The DPMS connector property is an enum property in KMS, which made our property handling complain at startup as we weren't defining its enums. Fix our definition so we parse the enum values. The only user of the property is the legacy path, which can continue using fixed values as those values are part of the KMS ABI. The atomic path does not need any changes, since atomic uses routing and CRTC active to determine the connector's power state, rather than a property. Signed-off-by: Daniel Stone Fixes: https://gitlab.freedesktop.org/wayland/weston/issues/125 --- libweston/compositor-drm.c | 41 +- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 7dcc634c6..0de6d6ba0 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -186,9 +186,36 @@ enum wdrm_connector_property { WDRM_CONNECTOR__COUNT }; +enum wdrm_dpms_state { + WDRM_DPMS_STATE_OFF = 0, + WDRM_DPMS_STATE_ON, + WDRM_DPMS_STATE_STANDBY, /* unused */ + WDRM_DPMS_STATE_SUSPEND, /* unused */ + WDRM_DPMS_STATE__COUNT +}; + +static struct drm_property_enum_info dpms_state_enums[] = { + [WDRM_DPMS_STATE_OFF] = { + .name = "Off", + }, + [WDRM_DPMS_STATE_ON] = { + .name = "On", + }, + [WDRM_DPMS_STATE_STANDBY] = { + .name = "Standby", + }, + [WDRM_DPMS_STATE_SUSPEND] = { + .name = "Suspend", + }, +}; + static const struct drm_property_info connector_props[] = { [WDRM_CONNECTOR_EDID] = { .name = "EDID" }, - [WDRM_CONNECTOR_DPMS] = { .name = "DPMS" }, + [WDRM_CONNECTOR_DPMS] = { + .name = "DPMS", + .enum_values = dpms_state_enums, + .num_enum_values = WDRM_DPMS_STATE__COUNT, + }, [WDRM_CONNECTOR_CRTC_ID] = { .name = "CRTC_ID", }, }; @@ -2372,6 +2399,8 @@ drm_output_apply_state_atomic(struct drm_output_state *state, current_mode->blob_id); ret |= crtc_add_prop(req, output, WDRM_CRTC_ACTIVE, 1); + /* No need for the DPMS property, since it is implicit in +* routing and CRTC activity. */ wl_list_for_each(head, >base.head_list, base.output_link) { ret |= connector_add_prop(req, head, WDRM_CONNECTOR_CRTC_ID, output->crtc_id); @@ -2380,6 +2409,8 @@ drm_output_apply_state_atomic(struct drm_output_state *state, ret |= crtc_add_prop(req, output, WDRM_CRTC_MODE_ID, 0); ret |= crtc_add_prop(req, output, WDRM_CRTC_ACTIVE, 0); + /* No need for the DPMS property, since it is implicit in +* routing and CRTC activity. */ wl_list_for_each(head, >base.head_list, base.output_link) ret |= connector_add_prop(req, head, WDRM_CONNECTOR_CRTC_ID, 0); } @@ -2463,14 +2494,6 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state, info->prop_id, 0); if (err <= 0) ret = -1; - - info = >props_conn[WDRM_CONNECTOR_DPMS]; - if (info->prop_id > 0) - err = drmModeAtomicAddProperty(req, head->connector_id, - info->prop_id, - DRM_MODE_DPMS_OFF); - if (err <= 0) - ret = -1; } wl_array_for_each(unused, >unused_crtcs) { -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Intel-gfx] Shared atomic state causing Weston repaint failure
Hey Jakob, On Thu, 5 Jul 2018 at 14:32, Jakob Bornecrantz wrote: > So from a VR compositor getting blocked like this is a no-go as the > user would quickly throw EPUKE. The situation is compounded by the > fact that the VR compositor has no idea what the display compositor is > doing with regards to setting modes so can not do any mitigating on > its side (like displaying a black screen). Yeah, definitely. > Some solutions that springs to mind (some I admit are probably not possible). > > - Make sure we don't get into this situation by locking the resources > of the VR crtc group or allocating enough bandwidth for the display > compositor crtcs up front. > > - Add priority and preemption to atomic so that VR compositor can > never be blocked. > > - Add X/Wayland protocol for the compositor to tell the VR compositor > that a modeset might effect it, so it can display a black-screen > during that time. > > - Make it possible for the VR compositor to tell the kernel what it > should do this case, like show black if I happen to get block before I > can queue a new pageflip. The specific issue on Intel is that they have a shared FIFO setup, and that enabling/reconfiguring a new CRTC requires FIFO reconfiguration, which in turn requires a global stall, even if you're not touching any leased resources directly. So there's no way to avoid the resources, or get around it with priorities. Ultimately I think we're going to need a general protocol to deal with this though. I can think of other situations - say, DDR reclocking, where the reclocking isn't voluntary but forced by platform thermals - where you might need to sit out a frame or two due to external factors. Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: Shared atomic state causing Weston repaint failure
Hey Jakob, On Thu, 5 Jul 2018 at 14:32, Jakob Bornecrantz wrote: > So from a VR compositor getting blocked like this is a no-go as the > user would quickly throw EPUKE. The situation is compounded by the > fact that the VR compositor has no idea what the display compositor is > doing with regards to setting modes so can not do any mitigating on > its side (like displaying a black screen). Yeah, definitely. > Some solutions that springs to mind (some I admit are probably not possible). > > - Make sure we don't get into this situation by locking the resources > of the VR crtc group or allocating enough bandwidth for the display > compositor crtcs up front. > > - Add priority and preemption to atomic so that VR compositor can > never be blocked. > > - Add X/Wayland protocol for the compositor to tell the VR compositor > that a modeset might effect it, so it can display a black-screen > during that time. > > - Make it possible for the VR compositor to tell the kernel what it > should do this case, like show black if I happen to get block before I > can queue a new pageflip. The specific issue on Intel is that they have a shared FIFO setup, and that enabling/reconfiguring a new CRTC requires FIFO reconfiguration, which in turn requires a global stall, even if you're not touching any leased resources directly. So there's no way to avoid the resources, or get around it with priorities. Ultimately I think we're going to need a general protocol to deal with this though. I can think of other situations - say, DDR reclocking, where the reclocking isn't voluntary but forced by platform thermals - where you might need to sit out a frame or two due to external factors. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v16 22/23] compositor-drm: Relax plane restrictions for atomic
Since we now incrementally test atomic state as we build it, we can loosen restrictions on what we can do with planes, and let the kernel tell us whether or not it's OK. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 9e062a7b3..f23583a9e 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1854,6 +1854,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; @@ -1878,7 +1879,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; } @@ -1906,15 +1907,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; @@ -3008,8 +3012,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 v16 07/23] compositor-drm: Use plane_state_coords_for_view for scanout
Now that we have a helper to fill the plane state co-ordinates from a view, use this for the scanout plane. We now explicitly check that the view fills exactly the fullscreen area and nothing else. We then use the new helper to fill out the plane state values, and do further checks against the filled-in co-ordinates, i.e. that we're not trying to show an offset into the buffer, or to scale the image. An audit of the error paths found some places where we would leave a plane state hanging; this makes them all consistent. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 64 +- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index c8eb7ef59..2927a0ebf 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1635,8 +1635,8 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, struct drm_plane *scanout_plane = output->scanout_plane; struct drm_plane_state *state; struct weston_buffer *buffer = ev->surface->buffer_ref.buffer; - struct weston_buffer_viewport *viewport = >surface->buffer_viewport; struct gbm_bo *bo; + pixman_box32_t *extents; /* Don't import buffers which span multiple outputs. */ if (ev->output_mask != (1u << output->base.id)) @@ -1651,23 +1651,13 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, if (wl_shm_buffer_get(buffer->resource)) return NULL; - /* Make sure our view is exactly compatible with the output. */ - if (ev->geometry.x != output->base.x || - ev->geometry.y != output->base.y) - return NULL; - if (buffer->width != output->base.current_mode->width || - buffer->height != output->base.current_mode->height) - return NULL; - - if (ev->transform.enabled) - return NULL; - if (ev->geometry.scissor_enabled) - return NULL; - if (viewport->buffer.transform != output->base.transform) - return NULL; - if (viewport->buffer.scale != output->base.current_scale) - return NULL; - if (!drm_view_transform_supported(ev, >base)) + /* Check the view spans exactly the output size, calculated in the +* logical co-ordinate space. */ + extents = pixman_region32_extents(>transform.boundingbox); + if (extents->x1 != output->base.x || + extents->y1 != output->base.y || + extents->x2 != output->base.x + output->base.width || + extents->y2 != output->base.y + output->base.height) return NULL; if (ev->alpha != 1.0f) @@ -1682,44 +1672,48 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, return NULL; } + state->output = output; + 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 || + state->dest_w != (unsigned) output->base.current_mode->width || + state->dest_h != (unsigned) output->base.current_mode->height) + goto err; + bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER, buffer->resource, GBM_BO_USE_SCANOUT); /* Unable to use the buffer for scanout */ if (!bo) - return NULL; + goto err; state->fb = drm_fb_get_from_bo(bo, b, drm_view_is_opaque(ev), BUFFER_CLIENT); if (!state->fb) { - drm_plane_state_put_back(state); + /* We need to explicitly destroy the BO. */ gbm_bo_destroy(bo); - return NULL; + goto err; } /* Can't change formats with just a pageflip */ if (state->fb->format->format != output->gbm_format) { /* No need to destroy the GBM BO here, as it's now owned * by the FB. */ - drm_plane_state_put_back(state); - return NULL; + goto err; } drm_fb_set_buffer(state->fb, buffer); - state->output = output; - - state->src_x = 0; - state->src_y = 0; - state->src_w = state->fb->width << 16; - state->src_h = state->fb->height << 16; - - state->dest_x = 0; - state->dest_y = 0
[PATCH v16 11/23] compositor-drm: Extract drm_fb_addfb into a helper
We currently do the same thing in two places, and will soon have a third. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen Tested-by: Emre Ucan --- libweston/compositor-drm.c | 91 -- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 51e5b4262..658e91a96 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -294,7 +294,10 @@ struct drm_fb { int refcnt; - uint32_t fb_id, stride, handle, size; + uint32_t fb_id, size; + uint32_t handles[4]; + uint32_t strides[4]; + uint32_t offsets[4]; const struct pixel_format_info *format; int width, height; int fd; @@ -838,7 +841,7 @@ drm_fb_destroy_dumb(struct drm_fb *fb) munmap(fb->map, fb->size); memset(_arg, 0, sizeof(destroy_arg)); - destroy_arg.handle = fb->handle; + destroy_arg.handle = fb->handles[0]; drmIoctl(fb->fd, DRM_IOCTL_MODE_DESTROY_DUMB, _arg); drm_fb_destroy(fb); @@ -854,6 +857,32 @@ drm_fb_destroy_gbm(struct gbm_bo *bo, void *data) drm_fb_destroy(fb); } +static int +drm_fb_addfb(struct drm_fb *fb) +{ + int ret; + + ret = drmModeAddFB2(fb->fd, fb->width, fb->height, fb->format->format, + fb->handles, fb->strides, fb->offsets, >fb_id, + 0); + if (ret == 0) + return 0; + + /* Legacy AddFB can't always infer the format from depth/bpp alone, so +* check if our format is one of the lucky ones. */ + if (!fb->format->depth || !fb->format->bpp) + return ret; + + /* Cannot fall back to AddFB for multi-planar formats either. */ + if (fb->handles[1] || fb->handles[2] || fb->handles[3]) + return ret; + + ret = drmModeAddFB(fb->fd, fb->width, fb->height, + fb->format->depth, fb->format->bpp, + fb->strides[0], fb->handles[0], >fb_id); + return ret; +} + static struct drm_fb * drm_fb_create_dumb(struct drm_backend *b, int width, int height, uint32_t format) @@ -864,12 +893,10 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int height, struct drm_mode_create_dumb create_arg; struct drm_mode_destroy_dumb destroy_arg; struct drm_mode_map_dumb map_arg; - uint32_t handles[4] = { 0 }, pitches[4] = { 0 }, offsets[4] = { 0 }; fb = zalloc(sizeof *fb); if (!fb) return NULL; - fb->refcnt = 1; fb->format = pixel_format_get_info(format); @@ -895,30 +922,20 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int height, goto err_fb; fb->type = BUFFER_PIXMAN_DUMB; - fb->handle = create_arg.handle; - fb->stride = create_arg.pitch; + fb->handles[0] = create_arg.handle; + fb->strides[0] = create_arg.pitch; fb->size = create_arg.size; fb->width = width; fb->height = height; fb->fd = b->drm.fd; - handles[0] = fb->handle; - pitches[0] = fb->stride; - offsets[0] = 0; - - ret = drmModeAddFB2(b->drm.fd, width, height, fb->format->format, - handles, pitches, offsets, >fb_id, 0); - if (ret) { - ret = drmModeAddFB(b->drm.fd, width, height, - fb->format->depth, fb->format->bpp, - fb->stride, fb->handle, >fb_id); - } - - if (ret) + if (drm_fb_addfb(fb) != 0) { + weston_log("failed to create kms fb: %m\n"); goto err_bo; + } memset(_arg, 0, sizeof map_arg); - map_arg.handle = fb->handle; + map_arg.handle = fb->handles[0]; ret = drmIoctl(fb->fd, DRM_IOCTL_MODE_MAP_DUMB, _arg); if (ret) goto err_add_fb; @@ -953,8 +970,6 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, bool is_opaque, enum drm_fb_type type) { struct drm_fb *fb = gbm_bo_get_user_data(bo); - uint32_t handles[4] = { 0 }, pitches[4] = { 0 }, offsets[4] = { 0 }; - int ret; if (fb) { assert(fb->type == type); @@ -971,10 +986,10 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, fb->width = gbm_bo_get_width(bo); fb->height = gbm_bo_get_height(bo); - fb->stride = gbm_bo_get_stride(bo); - fb->handle = gbm_bo_get_handle(bo).u32; + fb->strides[0] = gbm_bo_get_stride(bo); + fb->handles[0] = gbm_bo_get_handle(bo).u32; fb->format = pixel_format_get_info(gbm_bo_get_format(b
[PATCH v16 23/23] compositor-drm: Enable planes for atomic
Now that we can sensibly test proposed plane configurations with atomic, sprites are not broken. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index f23583a9e..7dcc634c6 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3645,6 +3645,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; + return 0; } @@ -6480,17 +6491,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 v16 14/23] compositor-drm: Support plane IN_FORMATS
From: Sergi Granell The per-plane IN_FORMATS property adds information about format modifiers; we can use these to both feed GBM with the set of modifiers we want to use for rendering, and also as an early-out test when we're trying to see if a FB will go on a particular plane. Signed-off-by: Sergi Granell Reviewed-by: Daniel Stone Tested-by: Emre Ucan --- configure.ac | 3 + libweston/compositor-drm.c | 125 ++--- 2 files changed, 119 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index ef55ace36..c550198ae 100644 --- a/configure.ac +++ b/configure.ac @@ -212,6 +212,9 @@ if test x$enable_drm_compositor = xyes; then PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78], [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic API])], [AC_MSG_WARN([libdrm does not support atomic modesetting, will omit that capability])]) + PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83], + [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports modifier advertisement])], + [AC_MSG_WARN([libdrm does not support modifier advertisement])]) PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2], [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import with modifiers])], [AC_MSG_WARN([GBM does not support dmabuf import, will omit that capability])]) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 98c8ed584..3f8e77554 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -131,6 +131,7 @@ enum wdrm_plane_property { WDRM_PLANE_CRTC_H, WDRM_PLANE_FB_ID, WDRM_PLANE_CRTC_ID, + WDRM_PLANE_IN_FORMATS, WDRM_PLANE__COUNT }; @@ -172,6 +173,7 @@ static const struct drm_property_info plane_props[] = { [WDRM_PLANE_CRTC_H] = { .name = "CRTC_H", }, [WDRM_PLANE_FB_ID] = { .name = "FB_ID", }, [WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", }, + [WDRM_PLANE_IN_FORMATS] = { .name = "IN_FORMATS" }, }; /** @@ -406,7 +408,11 @@ struct drm_plane { struct wl_list link; - uint32_t formats[]; + struct { + uint32_t format; + uint32_t count_modifiers; + uint64_t *modifiers; + } formats[]; }; struct drm_head { @@ -2908,7 +2914,19 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, /* Check whether the format is supported */ for (i = 0; i < p->count_formats; i++) { - if (p->formats[i] == fb->format->format) + unsigned int j; + + if (p->formats[i].format != fb->format->format) + continue; + + if (fb->modifier == DRM_FORMAT_MOD_INVALID) + break; + + for (j = 0; j < p->formats[i].count_modifiers; j++) { + if (p->formats[i].modifiers[j] == fb->modifier) + break; + } + if (j != p->formats[i].count_modifiers) break; } if (i == p->count_formats) @@ -3505,6 +3523,91 @@ init_pixman(struct drm_backend *b) return pixman_renderer_init(b->compositor); } +#ifdef HAVE_DRM_FORMATS_BLOB +static inline uint32_t * +formats_ptr(struct drm_format_modifier_blob *blob) +{ + return (uint32_t *)(((char *)blob) + blob->formats_offset); +} + +static inline struct drm_format_modifier * +modifiers_ptr(struct drm_format_modifier_blob *blob) +{ + return (struct drm_format_modifier *) + (((char *)blob) + blob->modifiers_offset); +} +#endif + +/** + * Populates the plane's formats array, using either the IN_MODIFIERS blob + * property (if available), or the plane's format list if not. + */ +static int +drm_plane_populate_formats(struct drm_plane *plane, const drmModePlane *kplane, + const drmModeObjectProperties *props) +{ + unsigned i; +#ifdef HAVE_DRM_FORMATS_BLOB + drmModePropertyBlobRes *blob; + struct drm_format_modifier_blob *fmt_mod_blob; + struct drm_format_modifier *blob_modifiers; + uint32_t *blob_formats; + uint32_t blob_id; + + blob_id = drm_property_get_value(>props[WDRM_PLANE_IN_FORMATS], +props, +0); + if (blob_id == 0) + goto fallback; + + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id); + if (!blob) + goto fallback; + + fmt_mod_blob = blob->data; + blob_formats = formats_ptr(fmt_mod
[PATCH v16 10/23] compositor-drm: Use plane FB-import helper for scanout
Use the same codepath, which has the added advantage of being able to import dmabufs. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen Tested-by: Emre Ucan --- libweston/compositor-drm.c | 53 ++ 1 file changed, 13 insertions(+), 40 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index fb48a4938..51e5b4262 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1715,26 +1715,11 @@ 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 weston_buffer *buffer = ev->surface->buffer_ref.buffer; - struct gbm_bo *bo; + struct drm_fb *fb; pixman_box32_t *extents; - /* Don't import buffers which span multiple outputs. */ - if (ev->output_mask != (1u << output->base.id)) - return NULL; - - /* We use GBM to import buffers. */ - if (b->gbm == NULL) - return NULL; - - if (buffer == NULL) - return NULL; - if (wl_shm_buffer_get(buffer->resource)) - return NULL; - /* Check the view spans exactly the output size, calculated in the * logical co-ordinate space. */ extents = pixman_region32_extents(>transform.boundingbox); @@ -1747,15 +1732,27 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, if (ev->alpha != 1.0f) return NULL; + fb = drm_fb_get_from_view(output_state, ev); + if (!fb) + return NULL; + + /* Can't change formats with just a pageflip */ + if (fb->format->format != output->gbm_format) { + drm_fb_unref(fb); + return NULL; + } + 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. */ + drm_fb_unref(fb); return NULL; } + state->fb = fb; state->output = output; if (!drm_plane_state_coords_for_view(state, ev)) goto err; @@ -1769,30 +1766,6 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, state->dest_h != (unsigned) output->base.current_mode->height) goto err; - bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER, - buffer->resource, GBM_BO_USE_SCANOUT); - - /* Unable to use the buffer for scanout */ - if (!bo) - goto err; - - state->fb = drm_fb_get_from_bo(bo, b, drm_view_is_opaque(ev), - BUFFER_CLIENT); - if (!state->fb) { - /* We need to explicitly destroy the BO. */ - gbm_bo_destroy(bo); - goto err; - } - - /* Can't change formats with just a pageflip */ - if (state->fb->format->format != output->gbm_format) { - /* No need to destroy the GBM BO here, as it's now owned -* by the FB. */ - goto err; - } - - drm_fb_set_buffer(state->fb, buffer); - return _plane->base; err: -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 19/23] compositor-drm: Add modes to drm_output_propose_state
Add support for multiple modes: toggling whether or not the renderer and/or planes are acceptable. This will be used to implement a smarter plane-placement heuristic when we have support for testing output states. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 39 ++ 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index c546aec83..377c63b7e 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1841,6 +1841,11 @@ drm_output_assign_state(struct drm_output_state *state, } } +enum drm_output_propose_state_mode { + DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */ + DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes */ +}; + static struct weston_plane * drm_output_prepare_scanout_view(struct drm_output_state *output_state, struct weston_view *ev) @@ -3161,13 +3166,17 @@ err: static struct drm_output_state * drm_output_propose_state(struct weston_output *output_base, -struct drm_pending_state *pending_state) +struct drm_pending_state *pending_state, +enum drm_output_propose_state_mode mode) { struct drm_output *output = to_drm_output(output_base); + struct drm_backend *b = to_drm_backend(output_base->compositor); struct drm_output_state *state; struct weston_view *ev; pixman_region32_t surface_overlap, renderer_region, occluded_region; struct weston_plane *primary = _base->compositor->primary_plane; + bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY); + bool planes_ok = !b->sprites_are_broken; assert(!output->state_last); state = drm_output_state_duplicate(output->state_cur, @@ -3230,36 +3239,49 @@ drm_output_propose_state(struct weston_output *output_base, next_plane = primary; pixman_region32_fini(_overlap); - if (next_plane == NULL) + /* The cursor plane is 'special' in the sense that we can still +* place it in the legacy API, and we gate that with a separate +* cursors_are_broken flag. */ + if (next_plane == NULL && !b->cursors_are_broken) next_plane = drm_output_prepare_cursor_view(state, ev); if (next_plane == NULL && !drm_view_is_opaque(ev)) next_plane = primary; + if (next_plane == NULL && !planes_ok) + next_plane = primary; + if (next_plane == NULL) next_plane = drm_output_prepare_scanout_view(state, ev); if (next_plane == NULL) next_plane = drm_output_prepare_overlay_view(state, ev); - if (next_plane == NULL) - next_plane = primary; + if (!next_plane || next_plane == primary) { + if (!renderer_ok) + goto err; - if (next_plane == primary) pixman_region32_union(_region, _region, _view); - else if (output->cursor_plane && -next_plane != >cursor_plane->base) + } else if (output->cursor_plane && + next_plane != >cursor_plane->base) { pixman_region32_union(_region, _region, _view); + } pixman_region32_fini(_view); } pixman_region32_fini(_region); pixman_region32_fini(_region); return state; + +err: + pixman_region32_fini(_region); + pixman_region32_fini(_region); + drm_output_state_free(state); + return NULL; } static void @@ -3273,7 +3295,8 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) struct weston_view *ev; struct weston_plane *primary = _base->compositor->primary_plane; - state = drm_output_propose_state(output_base, pending_state); + state = drm_output_propose_state(output_base, pending_state, +DRM_OUTPUT_PROPOSE_STATE_MIXED); wl_list_for_each(ev, _base->compositor->view_list, link) { struct drm_plane *target_plane = NULL; -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 05/23] compositor-drm: Fully account for buffer transformation
In our new and improved helper to determine the src/dest values for a buffer on a given plane, make sure we account for all buffer transformations, including viewport clipping. Rather than badly open-coding it ourselves, just use the helper which does exactly this. Signed-off-by: Daniel Stone Reported-by: Tiago Gomes Tested-by: Emre Ucan --- libweston/compositor-drm.c | 60 +- 1 file changed, 20 insertions(+), 40 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 03ff031d9..59e5e4ec6 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1181,10 +1181,10 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state, struct weston_view *ev) { struct drm_output *output = state->output; - struct weston_buffer_viewport *viewport = >surface->buffer_viewport; + struct weston_buffer *buffer = ev->surface->buffer_ref.buffer; pixman_region32_t dest_rect, src_rect; pixman_box32_t *box, tbox; - wl_fixed_t sx1, sy1, sx2, sy2; + float sxf1, syf1, sxf2, syf2; /* Update the base weston_plane co-ordinates. */ box = pixman_region32_extents(>transform.boundingbox); @@ -1216,51 +1216,31 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state, pixman_region32_intersect(_rect, >transform.boundingbox, >base.region); box = pixman_region32_extents(_rect); + weston_view_from_global_float(ev, box->x1, box->y1, , ); + weston_surface_to_buffer_float(ev->surface, sxf1, syf1, , ); + weston_view_from_global_float(ev, box->x2, box->y2, , ); + weston_surface_to_buffer_float(ev->surface, sxf2, syf2, , ); + pixman_region32_fini(_rect); - /* Accounting for any transformations made to this particular surface -* view, find the source rectangle to use. */ - weston_view_from_global_fixed(ev, - wl_fixed_from_int(box->x1), - wl_fixed_from_int(box->y1), - , ); - weston_view_from_global_fixed(ev, - wl_fixed_from_int(box->x2), - wl_fixed_from_int(box->y2), - , ); + /* Shift from S23.8 wl_fixed to U16.16 KMS fixed-point encoding. */ + state->src_x = wl_fixed_from_double(sxf1) << 8; + state->src_y = wl_fixed_from_double(syf1) << 8; + state->src_w = wl_fixed_from_double(sxf2 - sxf1) << 8; + state->src_h = wl_fixed_from_double(syf2 - syf1) << 8; /* Clamp our source co-ordinates to surface bounds; it's possible * for intermediate translations to give us slightly incorrect * co-ordinates if we have, for example, multiple zooming * transformations. View bounding boxes are also explicitly rounded * greedily. */ - 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; - pixman_region32_fini(_rect); - - /* Apply viewport transforms in reverse, to get the source co-ordinates -* in buffer space. */ - 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); - - /* Shift from S23.8 wl_fixed to U16.16 KMS fixed-point encoding. */ - state->src_x = tbox.x1 << 8; - state->src_y = tbox.y1 << 8; - state->src_w = (tbox.x2 - tbox.x1) << 8; - state->src_h = (tbox.y2 - tbox.y1) << 8; + if (state->src_x < 0) + state->src_x = 0; + if (state->src_y < 0) + state->src_y = 0; + if (state->src_w > (uint32_t) ((buffer->width << 16) - state->src_x)) + state->src_w = (buffer->width << 16) - state->src_x; + if (state->src_h > (uint32_t) ((buffer->height << 16) - state->src_y)) + state->src_h = (buffer->height << 16) - state->src_y; } /** -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 17/23] compositor-drm: Ignore views on other outputs
When we come to assign_planes, try very hard to ignore views which are only visible on other outputs, rather than forcibly moving them to the primary plane, which causes damage all round and unnecessary repaints. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen Tested-by: Emre Ucan --- libweston/compositor-drm.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 4a4bcca18..0a49bac5a 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1465,10 +1465,6 @@ drm_fb_get_from_view(struct drm_output_state *state, struct weston_view *ev) struct linux_dmabuf_buffer *dmabuf; struct drm_fb *fb; - /* Don't import buffers which span multiple outputs. */ - if (ev->output_mask != (1u << output->base.id)) - return NULL; - if (ev->alpha != 1.0f) return NULL; @@ -3039,10 +3035,6 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state, if (plane->state_cur->output && plane->state_cur->output != output) return NULL; - /* Don't import buffers which span multiple outputs. */ - if (ev->output_mask != (1u << output->base.id)) - return NULL; - /* We use GBM to import SHM buffers. */ if (b->gbm == NULL) return NULL; @@ -3200,6 +3192,16 @@ drm_output_propose_state(struct weston_output *output_base, wl_list_for_each(ev, _base->compositor->view_list, link) { struct weston_plane *next_plane = NULL; + /* If this view doesn't touch our output at all, there's no +* reason to do anything with it. */ + if (!(ev->output_mask & (1u << output->base.id))) + continue; + + /* We only assign planes to views which are exclusively present +* on our output. */ + if (ev->output_mask != (1u << output->base.id)) + next_plane = primary; + /* Since we process views from top to bottom, we know that if * the view intersects the calculated renderer region, it must * be part of, or occluded by, it, and cannot go on a plane. */ @@ -3249,6 +3251,11 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) wl_list_for_each(ev, _base->compositor->view_list, link) { struct drm_plane *target_plane = NULL; + /* If this view doesn't touch our output at all, there's no +* reason to do anything with it. */ + if (!(ev->output_mask & (1u << output->base.id))) + continue; + /* Test whether this buffer can ever go into a plane: * non-shm, or small enough to be a cursor. * -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 20/23] compositor-drm: Return plane state from plane preparation
Return a pointer to the plane state, rather than indirecting via a weston_plane. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 71 +- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 377c63b7e..3e22140a8 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1846,7 +1846,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) { @@ -1903,7 +1903,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); @@ -2069,7 +2069,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]; @@ -2096,7 +2095,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); @@ -2186,8 +2185,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; @@ -2903,7 +2902,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) { @@ -2975,7 +2974,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); @@ -3017,7 +3016,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) { @@ -3104,7 +3103,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); @@ -3174,7 +3173,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; @@ -3200,7 +3198,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; @@ -3212,7 +3211,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); @@ -3236,40 +3235,48 @@ drm_output_propose_state(struct weston_output *output_base, pixman_region32_intersect(_over
[PATCH v16 21/23] compositor-drm: Add test-only mode to state application
The atomic API can allow us to test state before we apply it, to see if it will be valid. Add support for this, which we will later use in assign_planes to ensure our plane configuration is valid. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 176 - 1 file changed, 136 insertions(+), 40 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 3e22140a8..9e062a7b3 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -220,6 +220,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 { @@ -1724,6 +1725,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 @@ -1848,13 +1850,16 @@ enum drm_output_propose_state_mode { static struct drm_plane_state * drm_output_prepare_scanout_view(struct drm_output_state *output_state, - struct weston_view *ev) + struct weston_view *ev, + enum drm_output_propose_state_mode mode) { struct drm_output *output = output_state->output; struct drm_plane *scanout_plane = output->scanout_plane; struct drm_plane_state *state; + struct drm_plane_state *state_old = NULL; struct drm_fb *fb; pixman_box32_t *extents; + int ret; /* Check the view spans exactly the output size, calculated in the * logical co-ordinate space. */ @@ -1879,11 +1884,18 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, } state = drm_output_state_get_plane(output_state, scanout_plane); - if (state->fb) { - /* If there is already a framebuffer on the scanout plane, -* a client view has already been placed on the scanout -* view. In that case, do not free or put back the state, -* but just leave it in place and quietly exit. */ + + /* Check if we've already placed a buffer on this plane; if there's a +* buffer there but it comes from GBM, then it's the result of +* drm_output_propose_state placing it here for testing purposes. */ + if (state->fb && + (state->fb->type == BUFFER_GBM_SURFACE || +state->fb->type == BUFFER_PIXMAN_DUMB)) { + state_old = calloc(1, sizeof(*state_old)); + memcpy(state_old, state, sizeof(*state_old)); + state_old->output_state = NULL; + wl_list_init(_old->link); + } else if (state->fb) { drm_fb_unref(fb); return NULL; } @@ -1903,10 +1915,24 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, state->dest_h != (unsigned) output->base.current_mode->height) goto err; + if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) { + drm_plane_state_free(state_old, false); + return state; + } + + ret = drm_pending_state_test(output_state->pending_state); + if (ret != 0) + goto err; + + drm_plane_state_free(state_old, false); return state; err: - drm_plane_state_put_back(state); + drm_plane_state_free(state, false); + if (state_old) { + wl_list_insert(_state->plane_list, _old->link); + state_old->output_state = output_state; + } return NULL; } @@ -1974,7 +2000,9 @@ drm_output_render(struct drm_output_state *state, pixman_region32_t *damage) * want to render. */ scanout_state = drm_output_state_get_plane(state, output->scanout_plane); - if (scanout_state->fb) + if (scanout_state->fb && + scanout_state->fb->type != BUFFER_GBM_SURFACE && + scanout_state->fb->type != BUFFER_PIXMAN_DUMB) return; if (!pixman_region32_not_empty(damage) && @@ -1997,6 +2025,7 @@ drm_output_render(struct drm_output_state *state, pixman_region32_t *damage) return; } + drm_fb_unref(scanout_state->fb); scanout_state->fb = fb; scanout_state->output = output; @@ -2513,9 +2542,16 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state, case DRM_STATE_APPLY_ASYNC:
[PATCH v16 18/23] compositor-drm: Ignore occluded views
When trying to assign planes, keep track of the areas which are already occluded, and ignore views which are completely occluded. This allows us to build a state using planes only, when there are occluded views which cannot go into a plane behind views which can. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 37 - 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 0a49bac5a..c546aec83 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3166,7 +3166,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); @@ -3188,9 +3188,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. */ @@ -3202,13 +3205,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); @@ -3216,6 +3233,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); @@ -3228,9 +3248,16 @@ drm_output_propose_state(struct weston_output *output_base, if (next_plane == primary) pixman_region32_union(_region, _region, - >transform.boundingbox); + _view); + else if (output->cursor_plane && +next_plane != >cursor_plane->base) + pixman_region32_union(_region, + _region, + _view); + pixman_region32_fini(_view); } pixman_region32_fini(_region); + pixman_region32_fini(_region); return state; } -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 16/23] compositor-drm: Split drm_assign_planes in two
Move drm_assign_planes into two functions: one which proposes a plane configuration, and another which applies that state to the Weston internal structures. This will be used to try multiple configurations and see which is supported. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen Tested-by: Emre Ucan --- libweston/compositor-drm.c | 109 + 1 file changed, 74 insertions(+), 35 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 3471d9b0f..4a4bcca18 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -365,6 +365,8 @@ struct drm_plane_state { struct drm_fb *fb; + struct weston_view *ev; /**< maintained for drm_assign_planes only */ + int32_t src_x, src_y; uint32_t src_w, src_h; int32_t dest_x, dest_y; @@ -1886,6 +1888,7 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, } state->fb = fb; + state->ev = ev; state->output = output; if (!drm_plane_state_coords_for_view(state, ev)) goto err; @@ -2961,6 +2964,7 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, } state->fb = fb; + state->ev = ev; state->output = output; if (!drm_plane_state_coords_for_view(state, ev)) @@ -3086,6 +3090,7 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state, } output->cursor_view = ev; + plane_state->ev = ev; plane_state->fb = drm_fb_ref(output->gbm_cursor_fb[output->current_cursor]); @@ -3162,17 +3167,15 @@ err: drmModeSetCursor(b->drm.fd, output->crtc_id, 0, 0, 0); } -static void -drm_assign_planes(struct weston_output *output_base, void *repaint_data) +static struct drm_output_state * +drm_output_propose_state(struct weston_output *output_base, +struct drm_pending_state *pending_state) { - struct drm_backend *b = to_drm_backend(output_base->compositor); - struct drm_pending_state *pending_state = repaint_data; struct drm_output *output = to_drm_output(output_base); struct drm_output_state *state; - struct drm_plane_state *plane_state; struct weston_view *ev; pixman_region32_t surface_overlap, renderer_region; - struct weston_plane *primary, *next_plane; + struct weston_plane *primary = _base->compositor->primary_plane; assert(!output->state_last); state = drm_output_state_duplicate(output->state_cur, @@ -3193,35 +3196,21 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) * as we do for flipping full screen surfaces. */ pixman_region32_init(_region); - primary = _base->compositor->primary_plane; wl_list_for_each(ev, _base->compositor->view_list, link) { - struct weston_surface *es = ev->surface; - - /* Test whether this buffer can ever go into a plane: -* non-shm, or small enough to be a cursor. -* -* Also, keep a reference when using the pixman renderer. -* That makes it possible to do a seamless switch to the GL -* renderer and since the pixman renderer keeps a reference -* to the buffer anyway, there is no side effects. -*/ - if (b->use_pixman || - (es->buffer_ref.buffer && - (!wl_shm_buffer_get(es->buffer_ref.buffer->resource) || -(ev->surface->width <= b->cursor_width && - ev->surface->height <= b->cursor_height - es->keep_buffer = true; - else - es->keep_buffer = false; + struct weston_plane *next_plane = NULL; + /* Since we process views from top to bottom, we know that if +* the view intersects the calculated renderer region, it must +* be part of, or occluded by, it, and cannot go on a plane. */ pixman_region32_init(_overlap); pixman_region32_intersect(_overlap, _region, >transform.boundingbox); - next_plane = NULL; if (pixman_region32_not_empty(_overlap)) next_plane = primary; + pixman_region32_fini(_overlap); + if (next_plane == NULL) next_plane = drm_output_prepare_cursor_view(state, ev); @@ -3234,17 +3223,70 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) if (next_plane == NULL) next_plane = primary; - weston_v
[PATCH v16 15/23] compositor-drm: Support modifiers with GBM
Use the extended GBM allocator interface to support modifiers and multi-planar BOs. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- configure.ac | 3 ++ libweston/compositor-drm.c | 61 +++--- 2 files changed, 53 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index c550198ae..357b6471e 100644 --- a/configure.ac +++ b/configure.ac @@ -215,6 +215,9 @@ if test x$enable_drm_compositor = xyes; then PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83], [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports modifier advertisement])], [AC_MSG_WARN([libdrm does not support modifier advertisement])]) + PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM_MODIFIERS, [gbm >= 17.1], + [AC_DEFINE([HAVE_GBM_MODIFIERS], 1, [GBM supports modifiers])], + [AC_MSG_WARN([GBM does not support modifiers])]) PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2], [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import with modifiers])], [AC_MSG_WARN([GBM does not support dmabuf import, will omit that capability])]) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 3f8e77554..3471d9b0f 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -871,7 +871,7 @@ drm_fb_addfb(struct drm_fb *fb) int ret = -EINVAL; #ifdef HAVE_DRM_ADDFB2_MODIFIERS uint64_t mods[4] = { }; - int i; + size_t i; #endif /* If we have a modifier set, we must only use the WithModifiers @@ -880,7 +880,7 @@ drm_fb_addfb(struct drm_fb *fb) /* KMS demands that if a modifier is set, it must be the same * for all planes. */ #ifdef HAVE_DRM_ADDFB2_MODIFIERS - for (i = 0; i < (int) ARRAY_LENGTH(mods) && fb->handles[i]; i++) + for (i = 0; i < ARRAY_LENGTH(mods) && fb->handles[i]; i++) mods[i] = fb->modifier; ret = drmModeAddFB2WithModifiers(fb->fd, fb->width, fb->height, fb->format->format, @@ -1114,6 +1114,9 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, bool is_opaque, enum drm_fb_type type) { struct drm_fb *fb = gbm_bo_get_user_data(bo); +#ifdef HAVE_GBM_MODIFIERS + int i; +#endif if (fb) { assert(fb->type == type); @@ -1127,15 +1130,25 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, fb->type = type; fb->refcnt = 1; fb->bo = bo; + fb->fd = backend->drm.fd; fb->width = gbm_bo_get_width(bo); fb->height = gbm_bo_get_height(bo); + fb->format = pixel_format_get_info(gbm_bo_get_format(bo)); + fb->size = 0; + +#ifdef HAVE_GBM_MODIFIERS + fb->modifier = gbm_bo_get_modifier(bo); + for (i = 0; i < gbm_bo_get_plane_count(bo); i++) { + fb->strides[i] = gbm_bo_get_stride_for_plane(bo, i); + fb->handles[i] = gbm_bo_get_handle_for_plane(bo, i).u32; + fb->offsets[i] = gbm_bo_get_offset(bo, i); + } +#else fb->strides[0] = gbm_bo_get_stride(bo); fb->handles[0] = gbm_bo_get_handle(bo).u32; - fb->format = pixel_format_get_info(gbm_bo_get_format(bo)); fb->modifier = DRM_FORMAT_MOD_INVALID; - fb->size = fb->strides[0] * fb->height; - fb->fd = backend->drm.fd; +#endif if (!fb->format) { weston_log("couldn't look up format 0x%lx\n", @@ -4212,13 +4225,39 @@ drm_output_init_egl(struct drm_output *output, struct drm_backend *b) fallback_format_for(output->gbm_format), }; int n_formats = 1; + struct weston_mode *mode = output->base.current_mode; + struct drm_plane *plane = output->scanout_plane; + unsigned int i; + + for (i = 0; i < plane->count_formats; i++) { + if (plane->formats[i].format == output->gbm_format) + break; + } + + if (i == plane->count_formats) { + weston_log("format 0x%x not supported by output %s\n", + output->gbm_format, output->base.name); + return -1; + } + +#ifdef HAVE_GBM_MODIFIERS + if (plane->formats[i].count_modifiers > 0) { + output->gbm_surface = + gbm_surface_create_with_modifiers(b->gbm, + mode->width, + mode->height, + output->gbm_format, +
[PATCH v16 09/23] compositor-drm: Extract overlay FB import to helper
... in order to be able to use it from scanout as well. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen Tested-by: Emre Ucan --- libweston/compositor-drm.c | 225 - 1 file changed, 122 insertions(+), 103 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 9aab6e523..fb48a4938 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1267,6 +1267,112 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state, return true; } +static bool +drm_view_is_opaque(struct weston_view *ev) +{ + pixman_region32_t r; + bool ret = false; + + /* We can scanout an ARGB buffer if the surface's +* opaque region covers the whole output, but we have +* to use XRGB as the KMS format code. */ + pixman_region32_init_rect(, 0, 0, + ev->surface->width, + ev->surface->height); + pixman_region32_subtract(, , >surface->opaque); + + if (!pixman_region32_not_empty()) + ret = true; + + pixman_region32_fini(); + + return ret; +} + +static struct drm_fb * +drm_fb_get_from_view(struct drm_output_state *state, struct weston_view *ev) +{ + struct drm_output *output = state->output; + struct drm_backend *b = to_drm_backend(output->base.compositor); + struct weston_buffer *buffer = ev->surface->buffer_ref.buffer; + struct linux_dmabuf_buffer *dmabuf; + struct drm_fb *fb; + struct gbm_bo *bo; + + /* Don't import buffers which span multiple outputs. */ + if (ev->output_mask != (1u << output->base.id)) + return NULL; + + if (ev->alpha != 1.0f) + return NULL; + + if (!drm_view_transform_supported(ev, >base)) + return NULL; + + if (!buffer) + return NULL; + + if (wl_shm_buffer_get(buffer->resource)) + return NULL; + + if (!b->gbm) + return NULL; + + dmabuf = linux_dmabuf_buffer_get(buffer->resource); + if (dmabuf) { +#ifdef HAVE_GBM_FD_IMPORT + /* XXX: TODO: +* +* Use AddFB2 directly, do not go via GBM. +* Add support for multiplanar formats. +* Both require refactoring in the DRM-backend to +* support a mix of gbm_bos and drmfbs. +*/ +struct gbm_import_fd_data gbm_dmabuf = { +.fd = dmabuf->attributes.fd[0], +.width = dmabuf->attributes.width, +.height = dmabuf->attributes.height, +.stride = dmabuf->attributes.stride[0], +.format = dmabuf->attributes.format +}; + +/* XXX: TODO: + * + * Currently the buffer is rejected if any dmabuf attribute + * flag is set. This keeps us from passing an inverted / + * interlaced / bottom-first buffer (or any other type that may + * be added in the future) through to an overlay. Ultimately, + * these types of buffers should be handled through buffer + * transforms and not as spot-checks requiring specific + * knowledge. */ + if (dmabuf->attributes.n_planes != 1 || +dmabuf->attributes.offset[0] != 0 || + dmabuf->attributes.flags) + return NULL; + + bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_FD, _dmabuf, + GBM_BO_USE_SCANOUT); +#else + return NULL; +#endif + } else { + bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER, + buffer->resource, GBM_BO_USE_SCANOUT); + } + + if (!bo) + return NULL; + + fb = drm_fb_get_from_bo(bo, b, drm_view_is_opaque(ev), BUFFER_CLIENT); + if (!fb) { + gbm_bo_destroy(bo); + return NULL; + } + + drm_fb_set_buffer(fb, buffer); + return fb; +} + /** * Return a plane state from a drm_output_state. */ @@ -1604,28 +1710,6 @@ drm_output_assign_state(struct drm_output_state *state, } } -static bool -drm_view_is_opaque(struct weston_view *ev) -{ - pixman_region32_t r; - bool ret = false; - - /* We can scanout an ARGB buffer if the surface's -* opaque region covers the whole output, but we have -* to use XRGB as the KMS format code. */ - pixman_region32_init_rect(, 0, 0, - ev->surface->width, - ev->surface->height); - pixman_region32_subtract(, , >surface->opa
[PATCH v16 06/23] compositor-drm: Centralise more transform checks
Put some more transformation checks in drm_view_transform_supported. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 54 -- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 59e5e4ec6..c8eb7ef59 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1172,11 +1172,30 @@ drm_plane_state_put_back(struct drm_plane_state *state) (void) drm_plane_state_alloc(state_output, plane); } +static int +drm_view_transform_supported(struct weston_view *ev, struct weston_output *output) +{ + struct weston_buffer_viewport *viewport = >surface->buffer_viewport; + + /* This will incorrectly disallow cases where the combination of +* buffer and view transformations match the output transform. +* Fixing this requires a full analysis of the transformation +* chain. */ + if (ev->transform.enabled && + ev->transform.matrix.type >= WESTON_MATRIX_TRANSFORM_ROTATE) + return false; + + if (viewport->buffer.transform != output->transform) + return false; + + return true; +} + /** * Given a weston_view, fill the drm_plane_state's co-ordinates to display on * a given plane. */ -static void +static bool drm_plane_state_coords_for_view(struct drm_plane_state *state, struct weston_view *ev) { @@ -1186,6 +1205,9 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state, pixman_box32_t *box, tbox; float sxf1, syf1, sxf2, syf2; + if (!drm_view_transform_supported(ev, >base)) + return false; + /* Update the base weston_plane co-ordinates. */ box = pixman_region32_extents(>transform.boundingbox); state->plane->base.x = box->x1; @@ -1241,6 +1263,8 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state, state->src_w = (buffer->width << 16) - state->src_x; if (state->src_h > (uint32_t) ((buffer->height << 16) - state->src_y)) state->src_h = (buffer->height << 16) - state->src_y; + + return true; } /** @@ -1580,13 +1604,6 @@ drm_output_assign_state(struct drm_output_state *state, } } -static int -drm_view_transform_supported(struct weston_view *ev) -{ - return !ev->transform.enabled || - (ev->transform.matrix.type < WESTON_MATRIX_TRANSFORM_ROTATE); -} - static bool drm_view_is_opaque(struct weston_view *ev) { @@ -1650,7 +1667,7 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, return NULL; if (viewport->buffer.scale != output->base.current_scale) return NULL; - if (!drm_view_transform_supported(ev)) + if (!drm_view_transform_supported(ev, >base)) return NULL; if (ev->alpha != 1.0f) @@ -2702,7 +2719,6 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, { struct drm_output *output = output_state->output; struct weston_compositor *ec = output->base.compositor; - struct weston_buffer_viewport *viewport = >surface->buffer_viewport; struct drm_backend *b = to_drm_backend(ec); struct wl_resource *buffer_resource; struct drm_plane *p; @@ -2728,13 +2744,6 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, if (wl_shm_buffer_get(buffer_resource)) return NULL; - if (viewport->buffer.transform != output->base.transform) - return NULL; - if (viewport->buffer.scale != output->base.current_scale) - return NULL; - if (!drm_view_transform_supported(ev)) - return NULL; - if (ev->alpha != 1.0f) return NULL; @@ -2758,6 +2767,14 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, if (!state) return NULL; + state->output = output; + if (!drm_plane_state_coords_for_view(state, ev)) + goto err; + + if (state->src_w != state->dest_w << 16 || + state->src_h != state->dest_h << 16) + goto err; + if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource))) { #ifdef HAVE_GBM_FD_IMPORT /* XXX: TODO: @@ -2816,9 +2833,6 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, drm_fb_set_buffer(state->fb, ev->surface->buffer_ref.buffer); - state->output = output; - drm_plane_state_coords_for_view(state, ev); - return >base; err: -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 00/23] DRM backend planes and modifiers
Hi, This is v16 (this time for sure) of the atomic series. Patches 1 and 2 are new, falling out of previous review. Patch 5 is a bit less trusting, and now properly clips to CRTC bounds if required. Similarly, I've gone through patch 8 and am more confident that it actually does the right thing in all cases, which should fix the HiDPI cursor bugs people have been seeing. Patch 6 is somewhat different to how it previously was, and I think the result is cleaner as we centralise all the checks. I've moved support for modifiers early in the series, since it's required to run on i.MX systems using the open-source etnaviv/imx-drm drivers; I would really like to see this land in 5.0.0 so we can help wean people off that particular vendor driver. The patches beyond have mostly seen relatively light changes, except that the ownership model for test-only commits has been rewritten, and should be much more solid. I hope so, anyway. The complete series is available at: https://gitlab.collabora.com/daniels/weston/tree/wip/2018-07/atomic-v16 That tree includes a patch to enable a _lot_ of debugging: please include this if you run into any issues or want to file bugs! Thanks to Zodiac Inflight Innovations for sponsoring a recent burst of work on the open i.MX6 stack. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 08/23] compositor-drm: Use plane_coords_for_view for cursor
Use the new helper to populate the cursor state as well, with some special-case handling to account for how we always upload a full-size BO. As this now fully takes care of buffer transformations, HiDPI client cursors work, and we also clip the cursor plane completely to CRTC bounds. Signed-off-by: Daniel Stone Reported-by: Derek Foreman Tested-by: Emre Ucan Fixes: https://gitlab.freedesktop.org/wayland/weston/issues/118 --- libweston/compositor-drm.c | 68 ++ 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 2927a0ebf..9aab6e523 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -2841,14 +2841,14 @@ err: /** * Update the image for the current cursor surface * - * @param b DRM backend structure - * @param bo GBM buffer object to write into - * @param ev View to use for cursor image + * @param plane_state DRM cursor plane state + * @param ev Source view for cursor */ static void -cursor_bo_update(struct drm_backend *b, struct gbm_bo *bo, -struct weston_view *ev) +cursor_bo_update(struct drm_plane_state *plane_state, struct weston_view *ev) { + struct drm_backend *b = plane_state->plane->backend; + struct gbm_bo *bo = plane_state->fb->bo; struct weston_buffer *buffer = ev->surface->buffer_ref.buffer; uint32_t buf[b->cursor_width * b->cursor_height]; int32_t stride; @@ -2857,18 +2857,16 @@ cursor_bo_update(struct drm_backend *b, struct gbm_bo *bo, assert(buffer && buffer->shm_buffer); assert(buffer->shm_buffer == wl_shm_buffer_get(buffer->resource)); - assert(ev->surface->width <= b->cursor_width); - assert(ev->surface->height <= b->cursor_height); memset(buf, 0, sizeof buf); stride = wl_shm_buffer_get_stride(buffer->shm_buffer); s = wl_shm_buffer_get_data(buffer->shm_buffer); wl_shm_buffer_begin_access(buffer->shm_buffer); - for (i = 0; i < ev->surface->height; i++) + for (i = 0; i < buffer->height; i++) memcpy(buf + i * b->cursor_width, s + i * stride, - ev->surface->width * 4); + buffer->width * 4); wl_shm_buffer_end_access(buffer->shm_buffer); if (gbm_bo_write(bo, buf, sizeof buf) < 0) @@ -2883,10 +2881,8 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state, struct drm_backend *b = to_drm_backend(output->base.compositor); struct drm_plane *plane = output->cursor_plane; struct drm_plane_state *plane_state; - struct weston_buffer_viewport *viewport = >surface->buffer_viewport; struct wl_shm_buffer *shmbuf; bool needs_update = false; - float x, y; if (!plane) return NULL; @@ -2916,26 +2912,25 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state, if (wl_shm_buffer_get_format(shmbuf) != WL_SHM_FORMAT_ARGB) return NULL; - if (output->base.transform != WL_OUTPUT_TRANSFORM_NORMAL) - return NULL; - if (ev->transform.enabled && - (ev->transform.matrix.type > WESTON_MATRIX_TRANSFORM_TRANSLATE)) - return NULL; - if (viewport->buffer.scale != output->base.current_scale) - return NULL; - if (ev->geometry.scissor_enabled) - return NULL; - - if (ev->surface->width > b->cursor_width || - ev->surface->height > b->cursor_height) - return NULL; - plane_state = drm_output_state_get_plane(output_state, output->cursor_plane); if (plane_state && plane_state->fb) return NULL; + /* We can't scale with the legacy API, and we don't try to account for +* simple cropping/translation in cursor_bo_update. */ + plane_state->output = output; + if (!drm_plane_state_coords_for_view(plane_state, ev)) + goto err; + + if (plane_state->src_x != 0 || plane_state->src_y != 0 || + plane_state->src_w > (unsigned) b->cursor_width << 16 || + plane_state->src_h > (unsigned) b->cursor_height << 16 || + plane_state->src_w != plane_state->dest_w << 16 || + plane_state->src_h != plane_state->dest_h << 16) + goto err; + /* Since we're setting plane state up front, we need to work out * whether or not we need to upload a new cursor. We can't use the * plane damage, since the planes haven't actually been calculated @@ -2952,26 +2947,27 @@ drm_output_prepare_cursor_view(struct drm_output_state
[PATCH v16 13/23] compositor-drm: Add modifiers to GBM dmabuf import
Add support for the GBM_BO_IMPORT_FD_MODIFIER path, which allows us to import multi-plane dmabufs, as well as format modifiers. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- configure.ac | 6 +- libweston/compositor-drm.c | 181 +++-- 2 files changed, 137 insertions(+), 50 deletions(-) diff --git a/configure.ac b/configure.ac index adb998dda..ef55ace36 100644 --- a/configure.ac +++ b/configure.ac @@ -212,9 +212,9 @@ if test x$enable_drm_compositor = xyes; then PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78], [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic API])], [AC_MSG_WARN([libdrm does not support atomic modesetting, will omit that capability])]) - PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 10.2], - [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports dmabuf import])], - [AC_MSG_WARN([gbm does not support dmabuf import, will omit that capability])]) + PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2], + [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import with modifiers])], + [AC_MSG_WARN([GBM does not support dmabuf import, will omit that capability])]) fi diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 0aaaf79e8..98c8ed584 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -284,6 +284,7 @@ struct drm_mode { enum drm_fb_type { BUFFER_INVALID = 0, /**< never used */ BUFFER_CLIENT, /**< directly sourced from client */ + BUFFER_DMABUF, /**< imported from linux_dmabuf client */ BUFFER_PIXMAN_DUMB, /**< internal Pixman rendering */ BUFFER_GBM_SURFACE, /**< internal EGL rendering */ BUFFER_CURSOR, /**< internal cursor buffer */ @@ -988,6 +989,120 @@ drm_fb_ref(struct drm_fb *fb) return fb; } +static void +drm_fb_destroy_dmabuf(struct drm_fb *fb) +{ + /* We deliberately do not close the GEM handles here; GBM manages +* their lifetime through the BO. */ + gbm_bo_destroy(fb->bo); + drm_fb_destroy(fb); +} + +static struct drm_fb * +drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf, + struct drm_backend *backend, bool is_opaque) +{ +#ifdef HAVE_GBM_FD_IMPORT + struct drm_fb *fb; + struct gbm_import_fd_data import_legacy = { + .width = dmabuf->attributes.width, + .height = dmabuf->attributes.height, + .format = dmabuf->attributes.format, + .stride = dmabuf->attributes.stride[0], + .fd = dmabuf->attributes.fd[0], + }; + struct gbm_import_fd_modifier_data import_mod = { + .width = dmabuf->attributes.width, + .height = dmabuf->attributes.height, + .format = dmabuf->attributes.format, + .num_fds = dmabuf->attributes.n_planes, + .modifier = dmabuf->attributes.modifier[0], + }; + int i; + +/* XXX: TODO: + * + * Currently the buffer is rejected if any dmabuf attribute + * flag is set. This keeps us from passing an inverted / + * interlaced / bottom-first buffer (or any other type that may + * be added in the future) through to an overlay. Ultimately, + * these types of buffers should be handled through buffer + * transforms and not as spot-checks requiring specific + * knowledge. */ + if (dmabuf->attributes.flags) + return NULL; + + fb = zalloc(sizeof *fb); + if (fb == NULL) + return NULL; + + fb->refcnt = 1; + fb->type = BUFFER_DMABUF; + + memcpy(import_mod.fds, dmabuf->attributes.fd, sizeof(import_mod.fds)); + memcpy(import_mod.strides, dmabuf->attributes.stride, + sizeof(import_mod.fds)); + memcpy(import_mod.offsets, dmabuf->attributes.offset, + sizeof(import_mod.fds)); + + if (dmabuf->attributes.modifier[0] != DRM_FORMAT_MOD_INVALID) { + fb->bo = gbm_bo_import(backend->gbm, GBM_BO_IMPORT_FD_MODIFIER, + _mod, + GBM_BO_USE_SCANOUT); + } else { + fb->bo = gbm_bo_import(backend->gbm, GBM_BO_IMPORT_FD, + _legacy, + GBM_BO_USE_SCANOUT); + } + + if (!fb->bo) + goto err_free; + + fb->width = dmabuf->attributes.width; + fb->height = dmabuf->attributes.height; + memcpy(fb->strides, dmabuf->attributes.stride, sizeof(fb->strides)); + memcpy(fb->offsets, dmabuf->attributes.offset, sizeof(fb->offsets)); + fb->format = pixel_format_get_info(
[PATCH v16 01/23] compositor-drm: Property accessor can be const
Since it doesn't write to the parameter, we can make it const. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index b96e29671..c6b82ac44 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -568,7 +568,7 @@ to_drm_mode(struct weston_mode *base) */ static uint64_t drm_property_get_value(struct drm_property_info *info, - drmModeObjectPropertiesPtr props, + const drmModeObjectProperties *props, uint64_t def) { unsigned int i; -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 02/23] compositor-drm: Remove unnecessary picked_scanout variable
e2e80136334f fixed the same issue as df573031d0ba in a different way. The latter commit (applied earlier in the upstream tree) adds a variable to assign_planes to keep track of when we successfully assign a view to the scanout plane, and doesn't call prepare_scanout_view if we have. The former commit adds this checking inside prepare_scanout_view: if the pending output state already has a framebuffer assigned to the scanout plane, we drop out of prepare_scanout_view early. The picked_scanout variable inside assign_planes can thus be removed. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index c6b82ac44..cfd7d268f 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3053,7 +3053,6 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) struct weston_view *ev; pixman_region32_t surface_overlap, renderer_region; struct weston_plane *primary, *next_plane; - bool picked_scanout = false; assert(!output->state_last); state = drm_output_state_duplicate(output->state_cur, @@ -3101,19 +3100,13 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) >transform.boundingbox); next_plane = NULL; - if (pixman_region32_not_empty(_overlap) || picked_scanout) + if (pixman_region32_not_empty(_overlap)) next_plane = primary; if (next_plane == NULL) next_plane = drm_output_prepare_cursor_view(state, ev); - /* If a higher-stacked view already got assigned to scanout, it's incorrect to -* assign a subsequent (lower-stacked) view to scanout. -*/ - if (next_plane == NULL) { + if (next_plane == NULL) next_plane = drm_output_prepare_scanout_view(state, ev); - if (next_plane) - picked_scanout = true; - } if (next_plane == NULL) next_plane = drm_output_prepare_overlay_view(state, ev); -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 12/23] compositor-drm: Support modifiers for drm_fb
Use the new drmModeAddFB2WithModifiers interface to import buffers with modifiers. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen Tested-by: Emre Ucan --- configure.ac | 3 +++ libweston/compositor-drm.c | 26 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 39168e205..adb998dda 100644 --- a/configure.ac +++ b/configure.ac @@ -206,6 +206,9 @@ AM_CONDITIONAL(ENABLE_DRM_COMPOSITOR, test x$enable_drm_compositor = xyes) if test x$enable_drm_compositor = xyes; then AC_DEFINE([BUILD_DRM_COMPOSITOR], [1], [Build the DRM compositor]) PKG_CHECK_MODULES(DRM_COMPOSITOR, [libudev >= 136 libdrm >= 2.4.30 gbm]) + PKG_CHECK_MODULES(DRM_COMPOSITOR_MODIFIERS, [libdrm >= 2.4.71], + [AC_DEFINE([HAVE_DRM_ADDFB2_MODIFIERS], 1, [libdrm supports modifiers])], + [AC_MSG_WARN([libdrm does not support AddFB2 with modifiers])]) 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])]) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 658e91a96..0aaaf79e8 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -299,6 +299,7 @@ struct drm_fb { uint32_t strides[4]; uint32_t offsets[4]; const struct pixel_format_info *format; + uint64_t modifier; int width, height; int fd; struct weston_buffer_reference buffer_ref; @@ -860,7 +861,28 @@ drm_fb_destroy_gbm(struct gbm_bo *bo, void *data) static int drm_fb_addfb(struct drm_fb *fb) { - int ret; + int ret = -EINVAL; +#ifdef HAVE_DRM_ADDFB2_MODIFIERS + uint64_t mods[4] = { }; + int i; +#endif + + /* If we have a modifier set, we must only use the WithModifiers +* entrypoint; we cannot import it through legacy ioctls. */ + if (fb->modifier != DRM_FORMAT_MOD_INVALID) { + /* KMS demands that if a modifier is set, it must be the same +* for all planes. */ +#ifdef HAVE_DRM_ADDFB2_MODIFIERS + for (i = 0; i < (int) ARRAY_LENGTH(mods) && fb->handles[i]; i++) + mods[i] = fb->modifier; + ret = drmModeAddFB2WithModifiers(fb->fd, fb->width, fb->height, +fb->format->format, +fb->handles, fb->strides, +fb->offsets, mods, >fb_id, +DRM_MODE_FB_MODIFIERS); +#endif + return ret; + } ret = drmModeAddFB2(fb->fd, fb->width, fb->height, fb->format->format, fb->handles, fb->strides, fb->offsets, >fb_id, @@ -922,6 +944,7 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int height, goto err_fb; fb->type = BUFFER_PIXMAN_DUMB; + fb->modifier = DRM_FORMAT_MOD_INVALID; fb->handles[0] = create_arg.handle; fb->strides[0] = create_arg.pitch; fb->size = create_arg.size; @@ -989,6 +1012,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, fb->strides[0] = gbm_bo_get_stride(bo); fb->handles[0] = gbm_bo_get_handle(bo).u32; fb->format = pixel_format_get_info(gbm_bo_get_format(bo)); + fb->modifier = DRM_FORMAT_MOD_INVALID; fb->size = fb->strides[0] * fb->height; fb->fd = backend->drm.fd; -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v16 04/23] compositor-drm: Extract buffer->plane co-ord translation
Pull this into a helper function, so we can use it everywhere. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen Tested-by: Emre Ucan --- libweston/compositor-drm.c | 162 + 1 file changed, 93 insertions(+), 69 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 93a0fb59f..03ff031d9 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1172,6 +1172,97 @@ drm_plane_state_put_back(struct drm_plane_state *state) (void) drm_plane_state_alloc(state_output, plane); } +/** + * Given a weston_view, fill the drm_plane_state's co-ordinates to display on + * a given plane. + */ +static void +drm_plane_state_coords_for_view(struct drm_plane_state *state, + struct weston_view *ev) +{ + struct drm_output *output = state->output; + struct weston_buffer_viewport *viewport = >surface->buffer_viewport; + pixman_region32_t dest_rect, src_rect; + pixman_box32_t *box, tbox; + wl_fixed_t sx1, sy1, sx2, sy2; + + /* Update the base weston_plane co-ordinates. */ + box = pixman_region32_extents(>transform.boundingbox); + state->plane->base.x = box->x1; + state->plane->base.y = box->y1; + + /* First calculate the destination co-ordinates by taking the +* area of the view which is visible on this output, performing any +* transforms to account for output rotation and scale as necessary. */ + pixman_region32_init(_rect); + pixman_region32_intersect(_rect, >transform.boundingbox, + >base.region); + pixman_region32_translate(_rect, -output->base.x, -output->base.y); + box = pixman_region32_extents(_rect); + tbox = weston_transformed_rect(output->base.width, + output->base.height, + output->base.transform, + output->base.current_scale, + *box); + state->dest_x = tbox.x1; + state->dest_y = tbox.y1; + state->dest_w = tbox.x2 - tbox.x1; + state->dest_h = tbox.y2 - tbox.y1; + pixman_region32_fini(_rect); + + /* Now calculate the source rectangle, by finding the extents of the +* view, and working backwards to source co-ordinates. */ + pixman_region32_init(_rect); + pixman_region32_intersect(_rect, >transform.boundingbox, + >base.region); + box = pixman_region32_extents(_rect); + + /* Accounting for any transformations made to this particular surface +* view, find the source rectangle to use. */ + weston_view_from_global_fixed(ev, + wl_fixed_from_int(box->x1), + wl_fixed_from_int(box->y1), + , ); + weston_view_from_global_fixed(ev, + wl_fixed_from_int(box->x2), + wl_fixed_from_int(box->y2), + , ); + + /* Clamp our source co-ordinates to surface bounds; it's possible +* for intermediate translations to give us slightly incorrect +* co-ordinates if we have, for example, multiple zooming +* transformations. View bounding boxes are also explicitly rounded +* greedily. */ + 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; + pixman_region32_fini(_rect); + + /* Apply viewport transforms in reverse, to get the source co-ordinates +* in buffer space. */ + 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); + + /* Shift from S23.8 wl_fixed to U16.16 KMS fixed-point encoding. */ + state->src_x = tbox.x1 << 8; + state->src_y = tbox.y1 << 8; + state->src_w = (tbox.x2 - tbox.x1) << 8; + state->src_h = (tbox.y2 - tbox.y1) << 8; +} + /** * Return a plane state from a drm_output_state. */ @@ -2631,16 +2722,13 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, { struct
[PATCH v16 03/23] compositor-drm: Make alpha-to-opaque handling common
Rather than a hardcoded ARGB -> XRGB translation inside a GBM-specific helper, just determine whether or not the view is opaque, and use the generic helpers to implement the format translation. As a consequence of reordering the calls in drm_output_prepare_overlay_view(), we move the GBM BO dereference into a different failure path, before it gets captured by the plane state. Signed-off-by: Daniel Stone Tested-by: Emre Ucan --- libweston/compositor-drm.c | 118 ++--- 1 file changed, 45 insertions(+), 73 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index cfd7d268f..93a0fb59f 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -950,7 +950,7 @@ drm_fb_ref(struct drm_fb *fb) static struct drm_fb * drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, - uint32_t format, enum drm_fb_type type) + bool is_opaque, enum drm_fb_type type) { struct drm_fb *fb = gbm_bo_get_user_data(bo); uint32_t handles[4] = { 0 }, pitches[4] = { 0 }, offsets[4] = { 0 }; @@ -961,8 +961,6 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, return drm_fb_ref(fb); } - assert(format != 0); - fb = zalloc(sizeof *fb); if (fb == NULL) return NULL; @@ -975,16 +973,19 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, fb->height = gbm_bo_get_height(bo); fb->stride = gbm_bo_get_stride(bo); fb->handle = gbm_bo_get_handle(bo).u32; - fb->format = pixel_format_get_info(format); + fb->format = pixel_format_get_info(gbm_bo_get_format(bo)); fb->size = fb->stride * fb->height; fb->fd = backend->drm.fd; if (!fb->format) { weston_log("couldn't look up format 0x%lx\n", - (unsigned long) format); + (unsigned long) gbm_bo_get_format(bo)); goto err_free; } + if (is_opaque) + fb->format = pixel_format_get_opaque_substitute(fb->format); + if (backend->min_width > fb->width || fb->width > backend->max_width || backend->min_height > fb->height || @@ -1515,34 +1516,26 @@ drm_view_transform_supported(struct weston_view *ev) (ev->transform.matrix.type < WESTON_MATRIX_TRANSFORM_ROTATE); } -static uint32_t -drm_output_check_scanout_format(struct drm_output *output, - struct weston_surface *es, struct gbm_bo *bo) +static bool +drm_view_is_opaque(struct weston_view *ev) { - uint32_t format; pixman_region32_t r; + bool ret = false; - format = gbm_bo_get_format(bo); - - if (format == GBM_FORMAT_ARGB) { - /* We can scanout an ARGB buffer if the surface's -* opaque region covers the whole output, but we have -* to use XRGB as the KMS format code. */ - pixman_region32_init_rect(, 0, 0, - output->base.width, - output->base.height); - pixman_region32_subtract(, , >opaque); + /* We can scanout an ARGB buffer if the surface's +* opaque region covers the whole output, but we have +* to use XRGB as the KMS format code. */ + pixman_region32_init_rect(, 0, 0, + ev->surface->width, + ev->surface->height); + pixman_region32_subtract(, , >surface->opaque); - if (!pixman_region32_not_empty()) - format = GBM_FORMAT_XRGB; + if (!pixman_region32_not_empty()) + ret = true; - pixman_region32_fini(); - } + pixman_region32_fini(); - if (output->gbm_format == format) - return format; - - return 0; + return ret; } static struct weston_plane * @@ -1556,7 +1549,6 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, struct weston_buffer *buffer = ev->surface->buffer_ref.buffer; struct weston_buffer_viewport *viewport = >surface->buffer_viewport; struct gbm_bo *bo; - uint32_t format; /* Don't import buffers which span multiple outputs. */ if (ev->output_mask != (1u << output->base.id)) @@ -1609,17 +1601,19 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, if (!bo) return NULL; - format = drm_output_check_scanout_format(output, ev->surface, bo); - if (format == 0) { + state->fb = drm_fb_get_from_bo(bo, b, drm_view_is_opaque(ev), + BUFFER_CL
Re: [PATCH v14 41/41] compositor-drm: Support modifiers with GBM
Hi Pekka, On Tue, 30 Jan 2018 at 09:58, Pekka Paalanen wrote: > On Wed, 20 Dec 2017 12:26:58 + > Daniel Stone wrote: > > + for (i = 0; i < plane->count_formats; i++) { > > + if (plane->formats[i].format == output->gbm_format) > > Here it is output->gbm_format... > > > + break; > > + } > > + > > + if (i == plane->count_formats) { > > + /* XXX: better error message */ > > + weston_log("can't use format for output\n"); > > XXX :-) > > Maybe I could add name strings to the pixel format list and a helper > that would be safe for arbitrary and even invalid format codes for > turning a format into pretty string. > > Should this fall back to the fallback format as well? We shouldn't fall back to the fallback format. For XRGB, the format is ARGB, which will enable alpha blending. Maybe we could try doing so in a separate patch, if we know we always set solid alpha channels, but ... > > - output->gbm_surface = gbm_surface_create(b->gbm, > > - output->base.current_mode->width, > > - > > output->base.current_mode->height, > > - format[0], > > - GBM_BO_USE_SCANOUT | > > - GBM_BO_USE_RENDERING); > > if (!output->gbm_surface) { > > weston_log("failed to create gbm surface\n"); > > return -1; > > This is not new in this patch, but the GBM surface is created with > gbm_format, but the EGLConfig may be chosen by gbm_format or its > fallback format. If it picks with the fallback format, do we have a > problem? It's not a problem that I know of, since the formats are chosen to be completely interchangeable. Mesa at least will happily render to the other format, and I believe this is also true of other implementations. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] [PATCH rfc 0/3] Be able to disable EGL/GLX_EXT_buffer_age
Hi Qiang, On Thu, 5 Jul 2018 at 03:32, Qiang Yu wrote: > For GPU like ARM mali Utgard EGL/GLX_EXT_buffer_age will make > performace worse. But mesa has no way to disable it. > > This patch series make driver be able to disable it and add a > gallium pipe cap for gallium driver usage. Due to currently > only out of tree lima driver need it, and not sure if this is > the right way to disable it, so I send this RFC before lima be > able to upstream. Series is: Acked-by: Daniel Stone Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Intel-gfx] Shared atomic state causing Weston repaint failure
Hi, The atomic API being super-explicit about how userspace sequences its calls is great and all, but having shared global state implicitly dragged in is kind of ruining my day. Currently on Intel, Weston sometimes fails on hotplug, because a commit which only enables CRTC B (not touching CRTC A or any other CRTC!), causes all commits to CRTC A to fail until CRTC B's modeset commit has fully retired: https://gitlab.freedesktop.org/wayland/weston/issues/24 The reason is that committing CRTC B resizes the DDB allocation for CRTC A as well, pulling CRTC A's CRTC state into the commit. This makes sense, but on the other hand it's totally opaque to userspace, and impossible for us to reason about when making commits. I suggested some options in that GitLab commit, none of which I like: * if any other CRTCs are pulled into a commit state, always execute a blocking commit in the kernel * if we're passing ALLOW_MODESET in userspace, only ever do blocking commits * whenever we get -EBUSY in userspace, assume we've been screwed by the kernel and defer until other outputs have completed * whenever we want to reconfigure any output in userspace, wait until all outputs are completely quiescent and do a single atomic commit covering all outputs The first one seems completely non-obvious from the kernel, but on the other hand the current -EBUSY failing behaviour is also non-obvious. The second is maybe the most reasonable, but on the other hand just working around a painful leaky abstraction: we also can't know upfront from userspace if this is actually going to be required, or if we're just killing responsiveness blocking for no reason. The third is the thing I least want to do, because it might well paper over legitimate bugs in userspace, and complicates our state tracking for no reason. The fourth is probably the most legitimate, but, well ... someone has to type up all the code to make our output-configuration API completely asynchronous. I suspect we're the first ones to be hitting this, because Weston has a truly independent per-CRTC repaint loop, we're one of the few atomic users, and also because Pekka did some seriously brutal hotplug testing whilst reworking Weston's output configuration API. Also because our approach to failed output repaints is to just freeze the output until it next cycles off and on, which is much more apparent than just silently dropping a frame here and there. ;) Any bright ideas on what could practically be done here? Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shared atomic state causing Weston repaint failure
Hi, The atomic API being super-explicit about how userspace sequences its calls is great and all, but having shared global state implicitly dragged in is kind of ruining my day. Currently on Intel, Weston sometimes fails on hotplug, because a commit which only enables CRTC B (not touching CRTC A or any other CRTC!), causes all commits to CRTC A to fail until CRTC B's modeset commit has fully retired: https://gitlab.freedesktop.org/wayland/weston/issues/24 The reason is that committing CRTC B resizes the DDB allocation for CRTC A as well, pulling CRTC A's CRTC state into the commit. This makes sense, but on the other hand it's totally opaque to userspace, and impossible for us to reason about when making commits. I suggested some options in that GitLab commit, none of which I like: * if any other CRTCs are pulled into a commit state, always execute a blocking commit in the kernel * if we're passing ALLOW_MODESET in userspace, only ever do blocking commits * whenever we get -EBUSY in userspace, assume we've been screwed by the kernel and defer until other outputs have completed * whenever we want to reconfigure any output in userspace, wait until all outputs are completely quiescent and do a single atomic commit covering all outputs The first one seems completely non-obvious from the kernel, but on the other hand the current -EBUSY failing behaviour is also non-obvious. The second is maybe the most reasonable, but on the other hand just working around a painful leaky abstraction: we also can't know upfront from userspace if this is actually going to be required, or if we're just killing responsiveness blocking for no reason. The third is the thing I least want to do, because it might well paper over legitimate bugs in userspace, and complicates our state tracking for no reason. The fourth is probably the most legitimate, but, well ... someone has to type up all the code to make our output-configuration API completely asynchronous. I suspect we're the first ones to be hitting this, because Weston has a truly independent per-CRTC repaint loop, we're one of the few atomic users, and also because Pekka did some seriously brutal hotplug testing whilst reworking Weston's output configuration API. Also because our approach to failed output repaints is to just freeze the output until it next cycles off and on, which is much more apparent than just silently dropping a frame here and there. ;) Any bright ideas on what could practically be done here? Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 19/24] drm/armada: Move GEM BO to drm_framebuffer
Hi Russell, On Tue, 26 Jun 2018 at 15:49, Russell King - ARM Linux wrote: > On Thu, May 17, 2018 at 04:41:35PM +0100, Daniel Stone wrote: > > Thanks Russell. I did do a build test locally as well which had no > > complaints. I'll merge this through drm-misc. > > I've not seen this go in during the last merge window, so I assume > either it missed the window or it's been forgotten. Mind if I pick > it up instead - I finally have armada on the way to atomic modeset > conversion. Thanks for chasing this up! AFAICT it has been merged though though: it went into drm-misc-next last month, which Dave merged into drm-next with f4366e44efeb895c. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH xserver] xf86-video-modesetting: Lease planes as well if using atomic
Hi, On Wed, 27 Jun 2018 at 00:35, Keith Packard wrote: > @@ -3251,6 +3251,9 @@ drmmode_create_lease(RRLeasePtr lease, int *fd) > > nobjects = ncrtc + noutput; > > +if (ms->atomic_modeset) > +nobjects += ncrtc; This seems like it definitely wants a comment as to why we add ncrtc twice (for the primary plane). But, with that (and no need to pass it through the list again): Reviewed-by: Daniel Stone Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] xf86-video-modesetting: Don't enable UNIVERSAL_PLANES separately
On Tue, 26 Jun 2018 at 22:06, Keith Packard wrote: > We don't want universal_planes unless we're using atomic APIs for > modesetting, and the kernel already enables universal_planes > automatically when atomic is enabled. > > If we enable universal_planes when we're not using atomic, then we > won't have selected a plane for each crtc, and this will break lease > creation which requires planes for each output when universal_planes > is enabled. Reviewed-by: Daniel Stone ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2 1/2] glamor: Unbreak glamor_fd_from_pixmap()
Hey Lyude, On Thu, 21 Jun 2018 at 00:13, Lyude Paul wrote: > -/* Pixmaps with multi-planes/modifier are not supported in this > interface */ > -if (ret != 1 || offsets[0] != 0) { > -while (ret > 0) > -close(fds[--ret]); > +ret = _glamor_fds_from_pixmap(screen, pixmap, , , NULL, size, > + NULL); > +if (ret != 1) > return -1; This needs the removed code just above it, where it closes excess FDs. I think the rest looks good though; I don't have a PRIME system so wasn't able to test at the time. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [Mesa-dev] [ANNOUNCE] Mesa 18.1.2 release candidate
Hi Dylan, On Tue, 19 Jun 2018 at 16:42, Dylan Baker wrote: > Historical artifact. When I created the proposed branch we didn't have a > policy > of putting it on master instead of a personal repo, and we were discussing a > transition to gitlab. I also seem to remember that the gitlab doesn't allow > force pushes (unless you're a repo owner?), which are a requirement for a > staging/proposed branch. I'd rather not move it at this point since that would > require changing all of our CI automation to match that new repo, but if we > can > force-push I'd be happy to put future staging branches in the main mesa repo. Force-pushes are a repo-by-repo policy thing. GitLab currently disallows force-pushes on master and release (e.g. '18.0') branches, but allows them on everything else. So as long as it's tucked away under another namespace (e.g. 'testing/18.1.2-candidate' or whatever) then you should be good. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [PATCH] dim: remove all mentions of drm-upstream
On Tue, 19 Jun 2018 at 13:08, Daniel Vetter wrote: > The one case in dim_status is entirely unused. > > The usage in dim_setup seems still in use, but this is code for > backwards compat hacks predating the original creating of the > maintainer-tools branch - i.e. no maintainer or committer except me > ever needed this. > > There's also no need to make sure drm-tip has all the right remotes > nowadays, because that's all encoded in nightly.conf, and will be > checked (if the remotes are missing) in dim_rebuild_tip. Thanks Daniel! Acked-by: Daniel Stone ___ dim-tools mailing list dim-tools@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dim-tools
[PATCH] dim: Change drm-upstream remote to drm
Unbreak 'dim setup' and 'dim status' by noting the new repo name, as configured in rerere config. Signed-off-by: Daniel Stone Cc: Daniel Vetter --- dim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dim b/dim index 1d562aa2360c..7092cd97b284 100755 --- a/dim +++ b/dim @@ -1999,7 +1999,7 @@ function dim_status fetch_all - drm_remote=$(repo_to_remote drm-upstream) + drm_remote=$(repo_to_remote drm) for branch in $dim_branches ; do repo=$(branch_to_repo $branch) @@ -2087,8 +2087,8 @@ function dim_setup setup_aux_checkout drm-tip $drm_tip_ssh drm-tip cd drm-tip - if git remote | grep -q drm-upstream ; then - git config remote.drm-upstream.url $drm_upstream_git + if git remote | grep -q '^drm$'; then + git config remote.drm.url $drm_upstream_git else remote=$(url_to_remote $drm_tip_ssh) remote=$(url_to_remote $drm_upstream_git) -- 2.17.1 ___ dim-tools mailing list dim-tools@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dim-tools
Re: [PATCH weston 0/4] Preparing for Meson
Hi Pekka, On Mon, 18 Jun 2018 at 15:41, Pekka Paalanen wrote: > these changes are intended to make using Meson easier. Daniel's patches > are v2, mine are v1. > > Daniel Stone (2): > tests: Don't rely on build directory layout > tests: Reshuffle IVI layout tests > > Pekka Paalanen (2): > shared: remove weston_config_get_libexec_dir() > tests: remove WESTON_BUILD_DIR from env Thanks a lot for picking these up; series is: Reviewed-by: Daniel Stone Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 1/2] contributing: add review guidelines
Hi Pekka, On Mon, 18 Jun 2018 at 14:43, Pekka Paalanen wrote: > This sets up the standards for patch review, and defines when a patch > can be merged. I believe these are the practises we have been using > already for a long time, now they are just written down explicitly. > > It's not an exhaustive list of criteria and likely cannot ever be, but > it should give a good idea of what level of review we want to have. > > It has been written in general terms, so that we can easily apply the > same text not just to Wayland, but also Weston and other projects as > necessary. > > This addition is not redundant with > https://wayland.freedesktop.org/reviewing.html . > > The web page is a friendly introduction and encouragement for people to > get involved. The guidelines here are more specific and aimed for people > who seek commit rights or maintainership. Thanks a lot for tackling this! Having it written down is fantastic, and I like how you've put it. I have two minor suggestions below: > +- The code is correct and does not ignore corner-cases. In general this is what we aim for, but the presence of 'XXX'/'FIXME' comments in the code shows we don't always live up to that ideal. ;) Maybe this could instead be something like: The code is correct and does not introduce new failures for existing users, nor add new corner-case bugs. When fixing bugs, 'root cause' fixes are preferred rather than hiding symptoms: identify why the issue has occurred, and fix it as close as possible to the source. If it is not practical to completely fix the issue, partial fixes should be accompanied by a comment in proximate code, as well as a new issue opened in GitLab clearly noting known corner cases with a suggestion on how to fix them. > +- The code does what it says in the commit message and changes nothing else. Which leads naturally to something like: If the commit message starts getting too long and addresses multiple points, this may be a sign that the commit should be broken into smaller individual commits. I'm happy to take alternate wording, since what you've written here is far more concise than mine. Anyway, both patches are: Reviewed-by: Daniel Stone Could this later be used as the basis for similar Weston patches? Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 1/3] doc: move Contributing
Hi Pekka, On Wed, 13 Jun 2018 at 14:33, Pekka Paalanen wrote: > Gitlab expects a CONTRIBUTING.md in the root directory, so move our > guide there. > > Conversion to proper markup is a follow-up patch. Thanks for these; both series are: Reviewed-by: Daniel Stone Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: gitlab migration
Hi Felix, On Wed, 13 Jun 2018 at 10:24, Felix Miata wrote: > Daniel Stone composed on 2018-06-13 09:24 (UTC+0100): > > Felix Miata wrote: > >> James Cloos composed on 2018-06-12 17:38 (UTC-0400): > >> > BZ is superior to GL (or GH or the like). > > >> Strongly agree, especially for returning useful search results!!! > > > What kind of searches have you tried which returned better search > > results on Bugzilla than GitLab? > > I gave up trying too long ago to remember. Best of my recollection is all of > them, generally getting too many to sift through, or zarro. I'm speaking of > gitlab/github (which is it anyway?) generally, not any specific project's. BZ > I > got used to over 17 years go, when learning new things had not yet become > problematic. GitLab and GitHub are two completely different services. They offer different features in different ways through different user interfaces, and have both been evolving pretty quickly. Saying emphatically!!! that GitLab's search results are useless, based on vague recollections of having used a _different service_ which was so long ago you can't even remember, isn't really a great contribution to a discussion. > Bad as yesteryear's web was, the current one is worse. Firefox ESR 60 download > is 648% the size of Firefox 1.0. All the benefit of broadband has been eaten > up > by bottlenecks, bloated browsers and gargantuan web pages. Googling 'gitlab > xorg' or 'github xorg' don't provide anything resembling an actual home page > URI > like https://bugs.freedesktop.org/. https://www.x.org/wiki/Development/ > doesn't > either. Maybe these are a good sign, one to indicate Ajax's migration hasn't > actually started. Yes. This thread is to discuss migrating to GitLab at some point in the future. Since it has not happened yet, there is no entrypoint to speak of. I can't solve your problems with web browsers, bottlenecks and people using JavaScript, but again, both Bugzilla and GitLab are web-based tools, and GitLab has a complete API with CLI-based tools you can use instead of a browser. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: gitlab migration
Hi Felix, On Wed, 13 Jun 2018 at 04:17, Felix Miata wrote: > James Cloos composed on 2018-06-12 17:38 (UTC-0400): > > Two comments: > > > BZ is superior to GL (or GH or the like). > > Strongly agree, especially for returning useful search results!!! What kind of searches have you tried which returned better search results on Bugzilla than GitLab? Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: gitlab migration
On Tue, 12 Jun 2018 at 22:36, James Cloos wrote: > >>>>> "DS" == Daniel Stone writes: > DS> No need to test; it's guaranteed to fail since we require Recaptcha > DS> for login due to massive spam issues. > > Which is of course grossly unethical and malicious and should never be > used by any site, under any circumstances. > > If some sort of captcha is ever desired, it always must be something > which works with non-ecmacsipt, TUI browsers. > > A simple math question with a simple html text box for the answer is OK. > Or a technical question specific to the given project. > > But never goog's malicious crap. I assume that you're unaware of the levels of spam we've been dealing with, including the number of times we've been blacklisted by major mail providers, the extent to which search engines used to distrust us, and the amount of admin time spent dealing with it. Now you've got whatever it was out of your system, maybe you could come back with some kind of actionable feedback, or a constructive plan to address the genuine problems that we've been discussing both here and at various conferences for the last two or three years which have led to GitLab. Maybe you could venture a real suggestion for dealing with spam which also works with noscript lynx or whatever; ideally one which was better than our previous version which did genuinely cause people issues. Or maybe you could go look at the CLI client for GitLab I've already pointed to, and read the API documentation. Or just continue the useless flames trying to enforce your opinion as universal fact, every single line of which should carry '[citation needed]'. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: gitlab migration
Hi Alexander, On 12 June 2018 at 14:53, Alexander E. Patrakov wrote: > Daniel Stone : >> > That said, I could not even create an account with a noscript/xhtml basic >> > browser (if you want to test that, install the famous noscript module with >> > an >> > empty "white list" in firefox or chromium, or use lynx or links or w3m). >> >> No need to test; it's guaranteed to fail since we require Recaptcha >> for login due to massive spam issues. > > Have you tested whether Chinese or Russian users can login or sign up? > Asking because Recaptcha was blocked for quite a long time by Russian > authorities. We do have active users from both countries currently. Cheers, Daniel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: gitlab migration
Hi Alexander, On 12 June 2018 at 14:53, Alexander E. Patrakov wrote: > Daniel Stone : >> > That said, I could not even create an account with a noscript/xhtml basic >> > browser (if you want to test that, install the famous noscript module with >> > an >> > empty "white list" in firefox or chromium, or use lynx or links or w3m). >> >> No need to test; it's guaranteed to fail since we require Recaptcha >> for login due to massive spam issues. > > Have you tested whether Chinese or Russian users can login or sign up? > Asking because Recaptcha was blocked for quite a long time by Russian > authorities. We do have active users from both countries currently. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: gitlab migration
Hi Sylvain, On 12 June 2018 at 13:38, wrote: > On Tue, Jun 12, 2018 at 12:34:31PM +0100, Daniel Stone wrote: >> GitLab has a pretty comprehensive and well-documented API which might >> help if you don't want to use a web browser. There are also clients >> like 'lab': https://gitlab.com/zaquestion/lab which provide a good CLI >> interface. > > Those "web APIs" usually require the use of an heavy javascript browser for > authentification or "app authorization". > > That said, I could not even create an account with a noscript/xhtml basic > browser (if you want to test that, install the famous noscript module with an > empty "white list" in firefox or chromium, or use lynx or links or w3m). No need to test; it's guaranteed to fail since we require Recaptcha for login due to massive spam issues. Cheers, Daniel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: gitlab migration
Hi Sylvain, On 12 June 2018 at 13:38, wrote: > On Tue, Jun 12, 2018 at 12:34:31PM +0100, Daniel Stone wrote: >> GitLab has a pretty comprehensive and well-documented API which might >> help if you don't want to use a web browser. There are also clients >> like 'lab': https://gitlab.com/zaquestion/lab which provide a good CLI >> interface. > > Those "web APIs" usually require the use of an heavy javascript browser for > authentification or "app authorization". > > That said, I could not even create an account with a noscript/xhtml basic > browser (if you want to test that, install the famous noscript module with an > empty "white list" in firefox or chromium, or use lynx or links or w3m). No need to test; it's guaranteed to fail since we require Recaptcha for login due to massive spam issues. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: gitlab migration
Hi Michel, On 11 June 2018 at 11:33, Michel Dänzer wrote: > On 2018-06-08 08:08 PM, Adam Jackson wrote: >> I'd like us to start moving repos and bug tracking into gitlab. >> Hopefully everyone's aware that gitlab exists and why fdo projects are >> migrating to it. If not, the thread about Mesa's migration provides >> some useful background: >> >> https://lists.x.org/archives/mesa-dev/2018-May/195634.html >> >> This should be mostly mechanical, except for moving some of the older >> junk into the archive and deciding which drivers _not_ to move yet (I >> imagine intel has some of its processes hooked up to bz, for example). > > As the maintainer of xf86-video-amdgpu and -ati, I'm fine with migrating > these to GitLab for Git and patch review. > > However, I'm not sure what to do about bugs/issues. My first thought was > to allow creating new issues in GitLab and disable creating new reports > in Bugzilla, but not to migrate existing reports from Bugzilla. However, > it still happens fairly often that a report is initially filed against > the Xorg drivers, even though it's actually an issue in the X server, > Mesa or the kernel (old habits die hard...). Therefore, I'm inclined to > stick to Bugzilla until at least the X server and Mesa have moved to > GitLab for issue tracking, to hopefully allow moving such misfiled issues. One thing some Mesa people said during that discussion is that they like the ability to move issues between Mesa and the kernel, which won't be possible if they're on split systems. So I probably wouldn't hold my breath for that to be honest. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: gitlab migration
Hi Sylvain, It looks like Mutt is mangling email addresses - I've fixed Michel's. On 11 June 2018 at 14:30, wrote: > On Mon, Jun 11, 2018 at 12:33:52PM +0200, Michel Dänzer wrote: >> Adding the amd-gfx list, in cases somebody there has concerns or other >> feedback. > > For feedback: > I attempted a migration on gitlab of my repos but I was blocked because it's > not noscript/xhtml basic browser friendly. > > I was successfull with launchpad.net (which has now git support). > > I did not test if the issue system was noscript/xhtml basic friendly though. GitLab has a pretty comprehensive and well-documented API which might help if you don't want to use a web browser. There are also clients like 'lab': https://gitlab.com/zaquestion/lab which provide a good CLI interface. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: gitlab migration
Hi Michel, On 11 June 2018 at 11:33, Michel Dänzer wrote: > On 2018-06-08 08:08 PM, Adam Jackson wrote: >> I'd like us to start moving repos and bug tracking into gitlab. >> Hopefully everyone's aware that gitlab exists and why fdo projects are >> migrating to it. If not, the thread about Mesa's migration provides >> some useful background: >> >> https://lists.x.org/archives/mesa-dev/2018-May/195634.html >> >> This should be mostly mechanical, except for moving some of the older >> junk into the archive and deciding which drivers _not_ to move yet (I >> imagine intel has some of its processes hooked up to bz, for example). > > As the maintainer of xf86-video-amdgpu and -ati, I'm fine with migrating > these to GitLab for Git and patch review. > > However, I'm not sure what to do about bugs/issues. My first thought was > to allow creating new issues in GitLab and disable creating new reports > in Bugzilla, but not to migrate existing reports from Bugzilla. However, > it still happens fairly often that a report is initially filed against > the Xorg drivers, even though it's actually an issue in the X server, > Mesa or the kernel (old habits die hard...). Therefore, I'm inclined to > stick to Bugzilla until at least the X server and Mesa have moved to > GitLab for issue tracking, to hopefully allow moving such misfiled issues. One thing some Mesa people said during that discussion is that they like the ability to move issues between Mesa and the kernel, which won't be possible if they're on split systems. So I probably wouldn't hold my breath for that to be honest. Cheers, Daniel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: gitlab migration
Hi Sylvain, It looks like Mutt is mangling email addresses - I've fixed Michel's. On 11 June 2018 at 14:30, wrote: > On Mon, Jun 11, 2018 at 12:33:52PM +0200, Michel Dänzer wrote: >> Adding the amd-gfx list, in cases somebody there has concerns or other >> feedback. > > For feedback: > I attempted a migration on gitlab of my repos but I was blocked because it's > not noscript/xhtml basic browser friendly. > > I was successfull with launchpad.net (which has now git support). > > I did not test if the issue system was noscript/xhtml basic friendly though. GitLab has a pretty comprehensive and well-documented API which might help if you don't want to use a web browser. There are also clients like 'lab': https://gitlab.com/zaquestion/lab which provide a good CLI interface. Cheers, Daniel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: gitlab migration
On 9 June 2018 at 00:11, Eric Anholt wrote: > Adam Jackson writes: >> I'd like us to start moving repos and bug tracking into gitlab. >> Hopefully everyone's aware that gitlab exists and why fdo projects are >> migrating to it. If not, the thread about Mesa's migration provides >> some useful background: >> >> https://lists.x.org/archives/mesa-dev/2018-May/195634.html >> >> This should be mostly mechanical, except for moving some of the older >> junk into the archive and deciding which drivers _not_ to move yet (I >> imagine intel has some of its processes hooked up to bz, for example). Migrating bugs is pretty easy, but there are currently over 2800 bugs in the 'xorg' product, of which 921 are in the server + xf86-video-modesetting + xf86-input-{evdev,libinput} components. The import does preserve components as tags, and it does add a 'bugzilla' tag to make it fairly easy to see what has and hasn't been triaged, but I would seriously recommend doing a massive clean-up pass before doing the migration. Easier said than done, of course. >> As far as contribution model, I'd personally prefer to stop using >> mailing lists, and for most of the X components I expect that's >> probably an improvement, because most components do not have especially >> active maintenance and it's currently very easy for patches to get lost >> in the mailing list history. Conversely for the server it can be >> difficult to keep track of a patch series' approval state. Again, not >> solely my decision to make, so I'd like to hear some rough consensus on >> how to proceed. Anyone with strong opinions, please do speak up. > > I, for one, would love to see xserver use a MR-based contribution > process. Every once in a while I go to review some old patches I had > personally marked as still to be reviewed, and find they're already > merged. I'm sure the reverse is happening, too. > > For our libraries with less active maintainership, MRs that stay open > until they're actually handled should be an even bigger win. > > I'm also *really* interested in a merge process that runs through the > server's automated tests before the code hits master. I know that won't > be day 1, but gitlab is progress toward that. Totally AOL from me. It looks like the repo currently runs exactly the same tests on GitLab CI as it did through Travis, for Linux at least. For Windows we can set up a GitLab -> Appveyor hook to check and just reuse that directly. That leaves us with OS X: we don't have an OS X CI worker set up for fd.o, and Travis have flatly said they don't plan to build any integration with GitLab. GStreamer do have OS X workers for Jenkins, and they're actively looking into migrating to GitLab; I believe their OS X worker more or less works. Maybe once they've moved we could reuse that and cover all three platforms that way. I'd be interested to see the relative speed of our worker compared to Travis. If it is a bit slower, then we're actively working on sourcing more CI workers, both x86 and ARM; hopefully within the month or so. If it's already faster, great. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH weston] configure.ac: bump libdrm requirement to 2.4.68
Hi, On 11 June 2018 at 10:25, Pekka Paalanen wrote: > On Mon, 11 Jun 2018 09:29:49 +1000 > Peter Hutterer wrote: >> +PKG_CHECK_MODULES(LIBDRM, [libdrm >= 2.4.68], [], [AC_MSG_ERROR([ >> libdrm is a hard build-time dependency for libweston core, >> but a sufficient version was not found. However, libdrm >> is not a runtime dependency unless you have features > > this patch is correct, but I would like to hear more opinions if we > want to bump the hard build-time dependency from release of Jan 2012 to > Apr 2016. > > If we do this bump, we could remove the fallback definitions for > DRM_FORMAT_R8 and DRM_FORMAT_GR88. > > In case we want to consider bumping even further, DRM_FORMAT_MOD_* were > introduced in 2.4.83 (Aug 2017), and atomic in 2.4.78 (Apr 2017). I'd be in favour of bumping all the way to .83. It's available in Debian testing and unstable, as well as for stable (stretch) via the backports.debian.org repository. It's available for Ubuntu 16.04 LTS via the xenial-updates repository, and releases since. It's available in Yocto Rocko (Oct 2017), but unsurprisingly not in Pyro (May 2017); I believe there are a few BSPs based on Pyro and earlier still, but they would be pretty trivial bumps to include if vendors want to upgrade Weston. Either patch is: Acked-by: Daniel Stone Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Migrating Wayland & Weston to GitLab
Hi, On 4 June 2018 at 09:05, Daniel Stone wrote: > On 29 May 2018 at 10:59, Daniel Stone wrote: >> I would like to get the issues migrated as well. In order to do that >> though, we need some more fixes to the 'bztogl' migration tool we've >> been using to push issues from Bugzilla to GitLab, as well as do >> migrations for some other projects which requested to migrate their >> issues a while ago. It also needs a fair few changes in order to fix >> support for Phabricator task import as well. >> >> But even once those are done, we need to clean up the bugs we're >> importing. The plan is to only import open bugs: closed bugs will stay >> in Bugzilla/Phabricator forever as a read-only archive, and GitLab >> will only have new active issues. I think a sensible transition plan >> would be for us to aim to do the import at the end of June, which >> means sweeping through all our open bugs before then, closing them if >> they're no longer useful or just cleaning up titles/etc to be helpful >> in future. > > I've started cleaning through the bugs as well, but would appreciate > all the help I can get. With the cleanup and the Phabricator script fixing having gone far more quickly than thought, I've migrated the issues now: https://gitlab.freedesktop.org/wayland/wayland/issues/ https://gitlab.freedesktop.org/wayland/weston/issues/ If you want to be notified of all new issues, you can 'watch' either the Wayland group or the Wayland/Weston projects. You can also subscribe to individual issue labels (e.g. 'DRM/KMS backend') if you want to keep a narrow interest. Currently the issues are a bit of a hodge-podge, but I intend to spend some time going through applying relevant labels and fixing up any stray formatting etc. Of course, any help with the triage would be excellent: simply add the labels you think are relevant and drop the 'bugzilla' label so we know it's been reasonably triaged. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] doc: Update for GitLab migration
Update issue report and build instruction URLs for moving to GitLab, and for everything having been HTTPS-only for quite some time. Signed-off-by: Daniel Stone --- README | 4 ++-- configure.ac | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README b/README index 84c30a4ca..a0a078c46 100644 --- a/README +++ b/README @@ -12,11 +12,11 @@ shell. Finally, weston also provides integration with the Xorg server and can pull X clients into the Wayland desktop and act as an X window manager. -Refer to http://wayland.freedesktop.org/building.html for building +Refer to https://wayland.freedesktop.org/building.html for building weston and its dependencies. The test suite can be invoked via `make check`; see -http://wayland.freedesktop.org/testing.html for additional details. +https://wayland.freedesktop.org/testing.html for additional details. Developer documentation can be built via `make doc`. Output will be in the build root under diff --git a/configure.ac b/configure.ac index ba11b6c8b..2a62b62ae 100644 --- a/configure.ac +++ b/configure.ac @@ -10,9 +10,9 @@ m4_define([libweston_patch_version], [0]) AC_PREREQ([2.64]) AC_INIT([weston], [weston_version], - [https://bugs.freedesktop.org/enter_bug.cgi?product=Wayland=weston=weston_version], +[https://gitlab.freedesktop.org/wayland/weston/issues/], [weston], -[http://wayland.freedesktop.org]) +[https://wayland.freedesktop.org]) WAYLAND_PREREQ_VERSION="1.12.0" -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland] doc: Update URLs for GitLab transition
Update bug and Git URLs for GitLab; the site has also been served over HTTPS for quite some time. Signed-off-by: Daniel Stone --- README | 4 ++-- configure.ac | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README b/README index 63ffa31d..3ae9f0fc 100644 --- a/README +++ b/README @@ -24,12 +24,12 @@ clients. Building the wayland libraries is fairly simple, aside from libffi, they don't have many dependencies: -$ git clone git://anongit.freedesktop.org/wayland/wayland +$ git clone https://gitlab.freedesktop.org/wayland/wayland $ cd wayland $ ./autogen.sh --prefix=PREFIX $ make $ make install where PREFIX is where you want to install the libraries. See -http://wayland.freedesktop.org for more complete build instructions +https://wayland.freedesktop.org for more complete build instructions for wayland, weston, xwayland and various toolkits. diff --git a/configure.ac b/configure.ac index c74ee97b..d0a10e17 100644 --- a/configure.ac +++ b/configure.ac @@ -8,9 +8,9 @@ m4_define([wayland_version], AC_INIT([wayland], [wayland_version], - [https://bugs.freedesktop.org/enter_bug.cgi?product=Wayland=wayland=wayland_version], + [https://gitlab.freedesktop.org/wayland/wayland/issues/], [wayland], -[http://wayland.freedesktop.org/]) +[https://wayland.freedesktop.org/]) AC_SUBST([WAYLAND_VERSION_MAJOR], [wayland_major_version]) AC_SUBST([WAYLAND_VERSION_MINOR], [wayland_minor_version]) -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[Ubuntu-x-swat] [Bug 36812]
(In reply to Daniel Stone from comment #191) > Another, probably better, way to do it would be to define a new flag like > XkbSA_HasGroupFlags inside the XkbModAction flags field when group_flags and > group_XXX are valid rather than potentially garbage. That would avoid the > whole version-negotiation nightmare, as nothing appears to be too picky > about extra flags being defined. > > Five years later, it would also be good to have support inside libxkbcommon > (which has a pretty decent test suite) and xcb-proto for the flags. This comment lays out the best way forward for anyone interested to fix this bug. It shouldn't be too difficult, but personally I haven't worked on X11 for quite some time. -- You received this bug notification because you are a member of Ubuntu-X, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions ___ Mailing list: https://launchpad.net/~ubuntu-x-swat Post to : ubuntu-x-swat@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-x-swat More help : https://help.launchpad.net/ListHelp
[Ubuntu-x-swat] [Bug 36812]
(In reply to Andreas Wettstein from comment #190) > No news since. Apart from the formal proposal, there are some old patches > for its implementation: > https://lists.x.org/archives/xorg-devel/2012-November/034427.html > https://lists.x.org/archives/xorg-devel/2012-November/034430.html > https://lists.x.org/archives/xorg-devel/2012-November/034429.html > https://lists.x.org/archives/xorg-devel/2012-November/034431.html > https://lists.x.org/archives/xorg-devel/2012-November/034428.html Here's what I think we would need to do in order to not break old clients: https://lists.freedesktop.org/archives/xorg-devel/2013-January/035049.html Another, probably better, way to do it would be to define a new flag like XkbSA_HasGroupFlags inside the XkbModAction flags field when group_flags and group_XXX are valid rather than potentially garbage. That would avoid the whole version-negotiation nightmare, as nothing appears to be too picky about extra flags being defined. Five years later, it would also be good to have support inside libxkbcommon (which has a pretty decent test suite) and xcb-proto for the flags. -- You received this bug notification because you are a member of Ubuntu-X, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions ___ Mailing list: https://launchpad.net/~ubuntu-x-swat Post to : ubuntu-x-swat@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-x-swat More help : https://help.launchpad.net/ListHelp
[Ubuntu-x-swat] [Bug 36812]
(In reply to Kovács Viktor from comment #181) > Sorry, on newer Linux you can set up hot key combination for that problem as > graphical UI settings, older Linux versions will not be updated. May I close > It? Please do not close this bug. If you do not want to receive any further updates on it, you can unsubscribe by removing yourself from the Cc list. -- You received this bug notification because you are a member of Ubuntu-X, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions ___ Mailing list: https://launchpad.net/~ubuntu-x-swat Post to : ubuntu-x-swat@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-x-swat More help : https://help.launchpad.net/ListHelp
[Desktop-packages] [Bug 36812]
(In reply to Daniel Stone from comment #191) > Another, probably better, way to do it would be to define a new flag like > XkbSA_HasGroupFlags inside the XkbModAction flags field when group_flags and > group_XXX are valid rather than potentially garbage. That would avoid the > whole version-negotiation nightmare, as nothing appears to be too picky > about extra flags being defined. > > Five years later, it would also be good to have support inside libxkbcommon > (which has a pretty decent test suite) and xcb-proto for the flags. This comment lays out the best way forward for anyone interested to fix this bug. It shouldn't be too difficult, but personally I haven't worked on X11 for quite some time. -- You received this bug notification because you are a member of Desktop Packages, which is subscribed to xorg-server in Ubuntu. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts Status in gnome-control-center: Unknown Status in X.Org X server: Confirmed Status in gnome-control-center package in Ubuntu: Invalid Status in xorg-server package in Ubuntu: Fix Released Bug description: This is a bug about shortcuts mapped to combinations which include each other. For example, if we have Ctrl+Shift (for keyboard layout) and Ctrl+Shift+N (to open a new terminal), then we are practically unable to use the second shortcut; this is what happens: Ctrl press (nothing happens) Shift press (keyboard layout change) N (a simple N appears, since a shortcut has already fired) The expected behavior is to fire shortcuts on the release (not on press) of the special keys (ctrl,shift,alt, etc) which is also how Windows behave. This is a serious problem for bilingual layouts, typically using Alt+Shift or Ctrl+Shift for keyboard layout change. For users being affected by this problem, the easiest solution for now is to add this PPA in your repositories: https://launchpad.net/~oded-geek/+archive/xorg-patches Practical summary of this bug for ubuntu developers (since reading 120 comments is impractical for most): This problem is a really old (since 2004) issue of the xkb part of xorg; the main discussion was made upstream in freedesktop-bugs #865. There has been a patch from Ilya Murav'jov for upstream (#55), and attached here (#61). Upstream xorg has refused to apply the patch, mainly because it "explicitly contradicts the (xkb) spec" (#84, #91). This patch has been reported to work for many people without any problems, and there is also a PPA by Oded Arbel (#95) where he maintains a patched version of the ubuntu xorg. The proper resolution of this bug would be to apply this patch to the upstream xorg, or at minimum to the official ubuntu xorg package. To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- Mailing list: https://launchpad.net/~desktop-packages Post to : desktop-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~desktop-packages More help : https://help.launchpad.net/ListHelp
[Desktop-packages] [Bug 36812]
(In reply to Andreas Wettstein from comment #190) > No news since. Apart from the formal proposal, there are some old patches > for its implementation: > https://lists.x.org/archives/xorg-devel/2012-November/034427.html > https://lists.x.org/archives/xorg-devel/2012-November/034430.html > https://lists.x.org/archives/xorg-devel/2012-November/034429.html > https://lists.x.org/archives/xorg-devel/2012-November/034431.html > https://lists.x.org/archives/xorg-devel/2012-November/034428.html Here's what I think we would need to do in order to not break old clients: https://lists.freedesktop.org/archives/xorg-devel/2013-January/035049.html Another, probably better, way to do it would be to define a new flag like XkbSA_HasGroupFlags inside the XkbModAction flags field when group_flags and group_XXX are valid rather than potentially garbage. That would avoid the whole version-negotiation nightmare, as nothing appears to be too picky about extra flags being defined. Five years later, it would also be good to have support inside libxkbcommon (which has a pretty decent test suite) and xcb-proto for the flags. -- You received this bug notification because you are a member of Desktop Packages, which is subscribed to xorg-server in Ubuntu. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts Status in gnome-control-center: Unknown Status in X.Org X server: Confirmed Status in gnome-control-center package in Ubuntu: Invalid Status in xorg-server package in Ubuntu: Fix Released Bug description: This is a bug about shortcuts mapped to combinations which include each other. For example, if we have Ctrl+Shift (for keyboard layout) and Ctrl+Shift+N (to open a new terminal), then we are practically unable to use the second shortcut; this is what happens: Ctrl press (nothing happens) Shift press (keyboard layout change) N (a simple N appears, since a shortcut has already fired) The expected behavior is to fire shortcuts on the release (not on press) of the special keys (ctrl,shift,alt, etc) which is also how Windows behave. This is a serious problem for bilingual layouts, typically using Alt+Shift or Ctrl+Shift for keyboard layout change. For users being affected by this problem, the easiest solution for now is to add this PPA in your repositories: https://launchpad.net/~oded-geek/+archive/xorg-patches Practical summary of this bug for ubuntu developers (since reading 120 comments is impractical for most): This problem is a really old (since 2004) issue of the xkb part of xorg; the main discussion was made upstream in freedesktop-bugs #865. There has been a patch from Ilya Murav'jov for upstream (#55), and attached here (#61). Upstream xorg has refused to apply the patch, mainly because it "explicitly contradicts the (xkb) spec" (#84, #91). This patch has been reported to work for many people without any problems, and there is also a PPA by Oded Arbel (#95) where he maintains a patched version of the ubuntu xorg. The proper resolution of this bug would be to apply this patch to the upstream xorg, or at minimum to the official ubuntu xorg package. To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- Mailing list: https://launchpad.net/~desktop-packages Post to : desktop-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~desktop-packages More help : https://help.launchpad.net/ListHelp
[Desktop-packages] [Bug 36812]
(In reply to Kovács Viktor from comment #181) > Sorry, on newer Linux you can set up hot key combination for that problem as > graphical UI settings, older Linux versions will not be updated. May I close > It? Please do not close this bug. If you do not want to receive any further updates on it, you can unsubscribe by removing yourself from the Cc list. -- You received this bug notification because you are a member of Desktop Packages, which is subscribed to xorg-server in Ubuntu. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts Status in gnome-control-center: Unknown Status in X.Org X server: Confirmed Status in gnome-control-center package in Ubuntu: Invalid Status in xorg-server package in Ubuntu: Fix Released Bug description: This is a bug about shortcuts mapped to combinations which include each other. For example, if we have Ctrl+Shift (for keyboard layout) and Ctrl+Shift+N (to open a new terminal), then we are practically unable to use the second shortcut; this is what happens: Ctrl press (nothing happens) Shift press (keyboard layout change) N (a simple N appears, since a shortcut has already fired) The expected behavior is to fire shortcuts on the release (not on press) of the special keys (ctrl,shift,alt, etc) which is also how Windows behave. This is a serious problem for bilingual layouts, typically using Alt+Shift or Ctrl+Shift for keyboard layout change. For users being affected by this problem, the easiest solution for now is to add this PPA in your repositories: https://launchpad.net/~oded-geek/+archive/xorg-patches Practical summary of this bug for ubuntu developers (since reading 120 comments is impractical for most): This problem is a really old (since 2004) issue of the xkb part of xorg; the main discussion was made upstream in freedesktop-bugs #865. There has been a patch from Ilya Murav'jov for upstream (#55), and attached here (#61). Upstream xorg has refused to apply the patch, mainly because it "explicitly contradicts the (xkb) spec" (#84, #91). This patch has been reported to work for many people without any problems, and there is also a PPA by Oded Arbel (#95) where he maintains a patched version of the ubuntu xorg. The proper resolution of this bug would be to apply this patch to the upstream xorg, or at minimum to the official ubuntu xorg package. To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- Mailing list: https://launchpad.net/~desktop-packages Post to : desktop-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~desktop-packages More help : https://help.launchpad.net/ListHelp
[Bug 36812]
(In reply to Daniel Stone from comment #191) > Another, probably better, way to do it would be to define a new flag like > XkbSA_HasGroupFlags inside the XkbModAction flags field when group_flags and > group_XXX are valid rather than potentially garbage. That would avoid the > whole version-negotiation nightmare, as nothing appears to be too picky > about extra flags being defined. > > Five years later, it would also be good to have support inside libxkbcommon > (which has a pretty decent test suite) and xcb-proto for the flags. This comment lays out the best way forward for anyone interested to fix this bug. It shouldn't be too difficult, but personally I haven't worked on X11 for quite some time. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 36812]
(In reply to Andreas Wettstein from comment #190) > No news since. Apart from the formal proposal, there are some old patches > for its implementation: > https://lists.x.org/archives/xorg-devel/2012-November/034427.html > https://lists.x.org/archives/xorg-devel/2012-November/034430.html > https://lists.x.org/archives/xorg-devel/2012-November/034429.html > https://lists.x.org/archives/xorg-devel/2012-November/034431.html > https://lists.x.org/archives/xorg-devel/2012-November/034428.html Here's what I think we would need to do in order to not break old clients: https://lists.freedesktop.org/archives/xorg-devel/2013-January/035049.html Another, probably better, way to do it would be to define a new flag like XkbSA_HasGroupFlags inside the XkbModAction flags field when group_flags and group_XXX are valid rather than potentially garbage. That would avoid the whole version-negotiation nightmare, as nothing appears to be too picky about extra flags being defined. Five years later, it would also be good to have support inside libxkbcommon (which has a pretty decent test suite) and xcb-proto for the flags. -- You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 36812]
(In reply to Kovács Viktor from comment #181) > Sorry, on newer Linux you can set up hot key combination for that problem as > graphical UI settings, older Linux versions will not be updated. May I close > It? Please do not close this bug. If you do not want to receive any further updates on it, you can unsubscribe by removing yourself from the Cc list. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 36812]
(In reply to Daniel Stone from comment #191) > Another, probably better, way to do it would be to define a new flag like > XkbSA_HasGroupFlags inside the XkbModAction flags field when group_flags and > group_XXX are valid rather than potentially garbage. That would avoid the > whole version-negotiation nightmare, as nothing appears to be too picky > about extra flags being defined. > > Five years later, it would also be good to have support inside libxkbcommon > (which has a pretty decent test suite) and xcb-proto for the flags. This comment lays out the best way forward for anyone interested to fix this bug. It shouldn't be too difficult, but personally I haven't worked on X11 for quite some time. -- You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 36812]
(In reply to Kovács Viktor from comment #181) > Sorry, on newer Linux you can set up hot key combination for that problem as > graphical UI settings, older Linux versions will not be updated. May I close > It? Please do not close this bug. If you do not want to receive any further updates on it, you can unsubscribe by removing yourself from the Cc list. -- You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 36812]
(In reply to Andreas Wettstein from comment #190) > No news since. Apart from the formal proposal, there are some old patches > for its implementation: > https://lists.x.org/archives/xorg-devel/2012-November/034427.html > https://lists.x.org/archives/xorg-devel/2012-November/034430.html > https://lists.x.org/archives/xorg-devel/2012-November/034429.html > https://lists.x.org/archives/xorg-devel/2012-November/034431.html > https://lists.x.org/archives/xorg-devel/2012-November/034428.html Here's what I think we would need to do in order to not break old clients: https://lists.freedesktop.org/archives/xorg-devel/2013-January/035049.html Another, probably better, way to do it would be to define a new flag like XkbSA_HasGroupFlags inside the XkbModAction flags field when group_flags and group_XXX are valid rather than potentially garbage. That would avoid the whole version-negotiation nightmare, as nothing appears to be too picky about extra flags being defined. Five years later, it would also be good to have support inside libxkbcommon (which has a pretty decent test suite) and xcb-proto for the flags. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to the bug report. https://bugs.launchpad.net/bugs/36812 Title: Keyboard layout change on hotkeys press instead of release and do not work well with shortcuts To manage notifications about this bug go to: https://bugs.launchpad.net/gnome-control-center/+bug/36812/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [PATCH weston 2/2] Add .gitlab-ci.yml
Hi Emil, I replied to the rest further down-thread, but: On 6 June 2018 at 15:33, Emil Velikov wrote: > On 5 June 2018 at 23:06, Daniel Stone wrote: >> + - git clone --depth=1 >> git://anongit.freedesktop.org/git/wayland/wayland-protocols >> + - export WAYLAND_PROTOCOLS_DIR="$(pwd)/prefix-wayland-protocols" >> + - export >> PKG_CONFIG_PATH="$WAYLAND_PROTOCOLS_DIR/share/pkgconfig:$PKG_CONFIG_PATH" >> + - cd wayland-protocols >> + - git show -s HEAD > Is this needed? It's not entirely necessary, but we do get quite verbose output from the package manager as to which versions etc it's installing; I figured being more verbose in the logs was better here, so people could figure out what a particular job was built against. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] Add .gitlab-ci.yml
Hi, On 6 June 2018 at 16:41, Emil Velikov wrote: > On 6 June 2018 at 15:47, Simon McVittie wrote: >> On Wed, 06 Jun 2018 at 15:33:13 +0100, Emil Velikov wrote: >>> On 5 June 2018 at 23:06, Daniel Stone wrote: >>> > + - apt-get -y --no-install-recommends install build-essential automake >>> > autoconf libtool pkg-config libexpat1-dev libffi-dev libxml2-dev >>> > libpixman-1-dev libpng-dev libjpeg-dev libcolord-dev mesa-common-dev >>> > libglu1-mesa-dev libegl1-mesa-dev libgles2-mesa-dev libwayland-dev >>> > libxcb1-dev libxcb-composite0-dev libxcb-xfixes0-dev libxcb-xkb-dev >>> > libx11-xcb-dev libx11-dev libudev-dev libgbm-dev libxkbcommon-dev >>> > libcairo2-dev libpango1.0-dev libgdk-pixbuf2.0-dev libxcursor-dev >>> > libmtdev-dev libpam0g-dev libvpx-dev libsystemd-dev libinput-dev >>> > libwebp-dev libjpeg-dev libva-dev liblcms2-dev git >>> >>> I think this can be simplify the explicit list to: >>> apt-get -y --no-install-recommends build-dep wayland >> >> Only if the latest version of wayland has the same build-dependencies >> as the (likely much older) version visible in the Sources index >> for the container's apt configuration. If the container is >> Debian 9 'stretch' (released about a year ago) then apt-get build-dep >> will get the build-dependencies of wayland 1.12.0, according to >> <https://tracker.debian.org/pkg/wayland>. >> > Sure, there can be differences. Adding the odd extra piece tends to be > shorter and easier (imho) than trying to keep the huge list in sync. > Distribution maintainers already that laborious job, why not make use of it. There are a couple of reasons I typed out the full dependency list. One of them is what Simon said, and over time as we get further and further away from Stretch, the list of extra stuff we have to add grows. Even if we move on to newer Debian, there's also the risk that we build things they don't, or some kind of transient dependency drops out, so we end up with some breakage. So I prefer the explicit set for that reason: it means we can be sure about what we're doing, rather than effectively CI'ing the Debian packaging. Secondly, Stretch was just the easiest starting point: I knew how to get it up quickly, in a way where the runtime was also fast (unlike with e.g. yum/dnf, where I don't know the necessary runes to speed them up). But it's entirely likely we'll want to be testing on other distros, or even a very much from-scratch build if we are able to reuse the work done on GitLab CI for Mesa here: https://gitlab.com/igalia/graphics/mesa/pipelines/22874845 Either way, having the list of dependencies explictly typed out is helpful for whoever's doing the conversion, and it's easy to make sure the lists stay in sync if we do have multiple lists. >>> > + - make -j4 >>> > + - make check >>> > + - make install >>> > + - make distcheck >>> I was under the impression that "make -j4 distcheck" is enough >> >> install and check both imply (depend on) a normal build, so that one is >> redundant. >> >> distcheck runs the tests like check does, but differently-configured, >> so for full coverage you might want to do both. >> >> Similarly, distcheck runs "make install", but into a temporary directory, >> not the requested $PREFIX. > > Right - did not spot that "prefix-*" is also collected as artefacts. Not just prefix, but also as far as I can see there's no way to collect test suite logs from a distcheck run? We've also had inexplicable bugs in the past where 'make check' failed but 'make distcheck' succeeded (or was it vice-versa?), and given the test runtime is still quite short, seemed reasonable enough to explicitly test out all of those. >>> Alternatively it's worth setting MAKEFLAGS="-jX" once so it's used across >>> all. >> >> Using -j would definitely help for "make distcheck". >> >> If parallel install works, it would be good to test it so that it stays >> that way: quite a lot of projects can build correctly in parallel, but >> don't work reliably for "make -jX install". > > There has been issues in the past and everything ran fine last time > I've checked. > That's why I'm suggesting adding the toggle ;-) > > I'm slightly confused if the reply is a) for, b) against or c) simply > provides more info. > In either way, the background info is always appreciated. Right, we have certainly had bugs with 'install' in the past. Hopefully it's improved now; let's find out, I suppose. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] Add .gitlab-ci.yml
Hi Pekka, On 7 June 2018 at 08:46, Pekka Paalanen wrote: > On Wed, 6 Jun 2018 15:37:01 +0100 > Emil Velikov wrote: >> On 6 June 2018 at 09:56, Pekka Paalanen wrote: >> > On Wed, 6 Jun 2018 09:22:59 +0100 >> > Daniel Stone wrote: >> >> On 6 June 2018 at 09:12, Daniel Stone wrote: >> >> > On 6 June 2018 at 09:03, Pekka Paalanen wrote: >> >> >> Distcheck does not --disable-xwayland-test, does it? So should we match >> >> >> it with the autogen.sh arguments? >> >> > >> distcheck uses AM_DISTCHECK_CONFIGURE_FLAGS (if set in the >> Makefile.am) to override the defaults. >> Plus the user can further tweak things via the non AM_ version. For example >> >> DISTCHECK_CONFIGURE_FLAGS="--disable-xwayland-test" make distcheck > > Yeah, that's my point. > > DISTCHECK_CONFIGURE_FLAGS is not set and Makefile.am has > > AM_DISTCHECK_CONFIGURE_FLAGS = --disable-setuid-install > > There is no benefit in using --disable-xwayland-test for autogen.sh, > because distcheck will use it anyway and it works already it seems. Right, I just removed it for the meantime. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] GitLab migration update
Hi, On 2 June 2018 at 16:45, Jason Ekstrand wrote: > When the migration happens, there will be a very brief (less than 30 > minutes?) period during which no one will be able to push to either repo. > I'll fire off an e-mail as well as messages in #dri-devel and #intel-3d > shortly before we actually take the repo offline so that everyone has a bit > of warning. Once the transition is complete, you'll have to change the git > remote you use for pushing to point to gitlab. That should be the ONLY > change to your current workflow and otherwise things should continue to work > smoothly. If someone accidentally tries to push to the remote on > git.freedesktop.org, the push will fail and they will receive a message > reminding them to push to gitlab instead. > > When the mesa repo gets imported into gitlab, I will be disabling all of the > "fancy" features such as merge requests, wikis, issue tracking, etc. They > are there for us to use if we want, but the first change to make is simply > going to be a repo hosting change. All of the above has been done, and the following repos now have their primary (and only push) source under https://gitlab.freedesktop.org/mesa/: crucible demos drm glu glut kmscube mesa piglit shader-db vkpipeline-db The following empty/junk repos were removed: llvm mesa-test piglit-test The following dormant repos were _not_ migrated, but can be migrated (perhaps to an archive/ section?) if they are still useful: clover glw libwsbm linux-agp-compat r600_demo rbug-gui The following repo I don't really know what to do with, but will chase up with Jason: tasks The following repo I'll separately chase up with VMware people about: vmwgfx Please ping me if you have any problems. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Piglit] GitLab migration of Piglit
Hi Emil, On 5 June 2018 at 18:21, Emil Velikov wrote: > On 5 June 2018 at 18:02, Daniel Stone wrote: >>>> drm-gralloc.git > Empty - please nuke, alongside bugzilla & other infra. Done. >>>> drm.git > Out of curiosity - this and others (say igt) projects are accessible > as mesa/$foo and drm/$foo. > I'd image the same approach will be adopted in gitlab? Unfortunately not, it provides a single canonical URL rather than aliasing. But both old anongit URLs will continue to work. >>>> llvm.git > Empty - nuke? Done. >>>> mesa-test.git > Plain copy/paste just like xserver-test. There were no extra > branches/patches wrt the usual repos. > Nuke? Done. >>>> libwsbm.git >>>> linux-agp-compat.git >>>> vmwgfx.git > > Might want to check with the VMware people about these. > I'm suspecting that the developers are not keeping a close eye on mesa-dev@ I'm going to follow up with them separately about vmwgfx.git. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [PATCH xserver] glamor: Enable modifier support for xfree86 too
On 6 June 2018 at 20:56, Adam Jackson wrote: > This was left disabled in 1.20.0, it's time to start being sure it > works. > > Signed-off-by: Adam Jackson Acked-by: Daniel Stone ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xkbcomp] Ignore xkb_keycodes.maximum of > 255
On 7 June 2018 at 05:13, Peter Hutterer wrote: > Continuation from 7fdfabd75 "keycodes: Ignore high keycodes" > > A keymap with a key > 255 will have a xkb_keycodes.maximum of that keycode. > Let's not throw a fatal error on that, just crop it back to the maximum of > 255. This doesn't set the "high_keycode_warned" on purpose so we get this for > the first key that actually matters. Reviewed-by: Daniel Stone ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [Mesa-dev] [PATCH 5/7] i965/screen: Don't advertise support for RG formats
We definitely do, but I assume it's not been tested recently ... (Sorry about mangled formatting) On Wed, 6 Jun 2018, 8:42 pm Jason Ekstrand, wrote: > On June 6, 2018 12:37:58 Daniel Stone wrote: > >> Right, it's a feature we use, because we do all import them as separate >> EGLImages ... and we won't if it's not advertised. > > > I'm a bit skeptical given that it doesn't actually work today because the > DRI format to Mesa format conversation function doesn't handle R it RG > formats today. Maybe it goes through some other path? > > In any case, I'm happy to drop this patch in favor of Lionel's patch to > make the DRI format to Mesa format conversation function actually work for > these formats. > > --Jason > > > >> On Wed, 6 Jun 2018, 7:05 pm Jason Ekstrand, wrote: >> >>> On Wed, Jun 6, 2018 at 11:03 AM, Jason Ekstrand >>> wrote: >>> >>>> On Wed, Jun 6, 2018 at 11:00 AM, Daniel Stone >>>> wrote: >>>> >>>>> Sorry, but as written this will regress ability to import NV12 images >>>>> as separately-addressed planes with shader conversion to RGB; Kodi, Mutter >>>>> and Weston all use this. >>>>> >>>> >>>> I don't believe it will. It only makes it so that we don't advertise R >>>> and RG formats through eglQueryDmaBufFormatsEXT. This means that you can't >>>> import the planes each as separate images but you can still import a planar >>>> image. >>>> >>> >>> Arguably, though, importing the planes as separate images does sound >>> like a feature... >>> >>> >>> >>>> --Jason >>>> >>>> >>>> >>>>> On Wed, 6 Jun 2018, 6:48 pm Jason Ekstrand, >>>>> wrote: >>>>> >>>>>> Cc: mesa-sta...@lists.freedesktop.org >>>>>> --- >>>>>> src/mesa/drivers/dri/i965/intel_screen.c | 12 >>>>>> 1 file changed, 12 insertions(+) >>>>>> >>>>>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c >>>>>> b/src/mesa/drivers/dri/i965/intel_screen.c >>>>>> index 5f0eeb41779..f681b221e7b 100644 >>>>>> --- a/src/mesa/drivers/dri/i965/intel_screen.c >>>>>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c >>>>>> @@ -1269,6 +1269,18 @@ intel_image_format_is_supported(const struct >>>>>> intel_image_format *fmt) >>>>>> fmt->fourcc == __DRI_IMAGE_FOURCC_SABGR) >>>>>>return false; >>>>>> >>>>>> + /* The dri_interface.h file says: >>>>>> +* >>>>>> +*"R8, GR88 and NONE should not be used with >>>>>> createImageFromName or >>>>>> +*createImage, and are returned by query from sub images >>>>>> created with >>>>>> +*createImageFromNames (NONE, see above) and fromPlane (R8 & >>>>>> GR88)." >>>>>> +* >>>>>> +* Let's not advertise support for R or RG formats. >>>>>> +*/ >>>>>> + if (fmt->components == __DRI_IMAGE_COMPONENTS_R || >>>>>> + fmt->components == __DRI_IMAGE_COMPONENTS_RG) >>>>>> + return false; >>>>>> + >>>>>> return true; >>>>>> } >>>>>> >>>>>> -- >>>>>> 2.17.1 >>>>>> >>>>>> ___ >>>>>> mesa-dev mailing list >>>>>> mesa-dev@lists.freedesktop.org >>>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >>>>>> >>>>> >>>> > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/7] i965/screen: Don't advertise support for RG formats
Right, it's a feature we use, because we do all import them as separate EGLImages ... and we won't if it's not advertised. On Wed, 6 Jun 2018, 7:05 pm Jason Ekstrand, wrote: > On Wed, Jun 6, 2018 at 11:03 AM, Jason Ekstrand > wrote: > >> On Wed, Jun 6, 2018 at 11:00 AM, Daniel Stone >> wrote: >> >>> Sorry, but as written this will regress ability to import NV12 images as >>> separately-addressed planes with shader conversion to RGB; Kodi, Mutter and >>> Weston all use this. >>> >> >> I don't believe it will. It only makes it so that we don't advertise R >> and RG formats through eglQueryDmaBufFormatsEXT. This means that you can't >> import the planes each as separate images but you can still import a planar >> image. >> > > Arguably, though, importing the planes as separate images does sound like > a feature... > > > >> --Jason >> >> >> >>> On Wed, 6 Jun 2018, 6:48 pm Jason Ekstrand, >>> wrote: >>> >>>> Cc: mesa-sta...@lists.freedesktop.org >>>> --- >>>> src/mesa/drivers/dri/i965/intel_screen.c | 12 >>>> 1 file changed, 12 insertions(+) >>>> >>>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c >>>> b/src/mesa/drivers/dri/i965/intel_screen.c >>>> index 5f0eeb41779..f681b221e7b 100644 >>>> --- a/src/mesa/drivers/dri/i965/intel_screen.c >>>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c >>>> @@ -1269,6 +1269,18 @@ intel_image_format_is_supported(const struct >>>> intel_image_format *fmt) >>>> fmt->fourcc == __DRI_IMAGE_FOURCC_SABGR) >>>>return false; >>>> >>>> + /* The dri_interface.h file says: >>>> +* >>>> +*"R8, GR88 and NONE should not be used with >>>> createImageFromName or >>>> +*createImage, and are returned by query from sub images >>>> created with >>>> +*createImageFromNames (NONE, see above) and fromPlane (R8 & >>>> GR88)." >>>> +* >>>> +* Let's not advertise support for R or RG formats. >>>> +*/ >>>> + if (fmt->components == __DRI_IMAGE_COMPONENTS_R || >>>> + fmt->components == __DRI_IMAGE_COMPONENTS_RG) >>>> + return false; >>>> + >>>> return true; >>>> } >>>> >>>> -- >>>> 2.17.1 >>>> >>>> ___ >>>> mesa-dev mailing list >>>> mesa-dev@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >>>> >>> >> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/7] i965/screen: Don't advertise support for RG formats
Sorry, but as written this will regress ability to import NV12 images as separately-addressed planes with shader conversion to RGB; Kodi, Mutter and Weston all use this. On Wed, 6 Jun 2018, 6:48 pm Jason Ekstrand, wrote: > Cc: mesa-sta...@lists.freedesktop.org > --- > src/mesa/drivers/dri/i965/intel_screen.c | 12 > 1 file changed, 12 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c > b/src/mesa/drivers/dri/i965/intel_screen.c > index 5f0eeb41779..f681b221e7b 100644 > --- a/src/mesa/drivers/dri/i965/intel_screen.c > +++ b/src/mesa/drivers/dri/i965/intel_screen.c > @@ -1269,6 +1269,18 @@ intel_image_format_is_supported(const struct > intel_image_format *fmt) > fmt->fourcc == __DRI_IMAGE_FOURCC_SABGR) >return false; > > + /* The dri_interface.h file says: > +* > +*"R8, GR88 and NONE should not be used with createImageFromName or > +*createImage, and are returned by query from sub images created > with > +*createImageFromNames (NONE, see above) and fromPlane (R8 & > GR88)." > +* > +* Let's not advertise support for R or RG formats. > +*/ > + if (fmt->components == __DRI_IMAGE_COMPONENTS_R || > + fmt->components == __DRI_IMAGE_COMPONENTS_RG) > + return false; > + > return true; > } > > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [git pull] drm for v4.18-rc1
Hi, On 6 June 2018 at 04:50, Dave Airlie wrote: > First up I've moved the drm tree to a new location on freedesktop.org. The > main > reason was to explore using Daniel's maintainer tools (dim-tools) to manage > pull requests and possibly open the drm to having co-maintainers at the top > level in the future. If there are any issues in the pull formatting or with > the > hopefully correctly signed tag, let me know. > > This location also might change again as there is an fd.o migration to > using gitlab > based hosting for git trees, need to talk to fd.o admins about what might > happen > there. anongit isn't going away for the foreseeable future (probably ever, given the number of Yocto BSPs which will be around forever), so that repo will continue to stay. When that repo gets migrated, the option to clone via HTTPS direct to gitlab will be added, but anongit will remain as a pushed mirror. For most of our projects this is a win, but given you use signed tags it won't make a difference really. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH weston 2/2] Add .gitlab-ci.yml
Hi, On 6 June 2018 at 09:03, Pekka Paalanen wrote: > On Tue, 5 Jun 2018 23:06:59 +0100 > Daniel Stone wrote: >> + - export XDG_RUNTIME_DIR="$(mktemp -p $(pwd) -d xdg-runtime-XX)" >> + - export BUILD_ID="weston-$CI_JOB_NAME_$CI_COMMIT_SHA-$CI_JOB_ID" >> + - export PREFIX="$(pwd)/prefix-$BUILD_ID" >> + - export BUILDDIR="$(pwd)/build-$BUILD_ID" >> + - mkdir "$BUILDDIR" "$PREFIX" >> + - cd "$BUILDDIR" >> + - ../autogen.sh --prefix="$PREFIX" --disable-setuid-install >> --enable-xwayland --disable-xwayland-test --enable-x11-compositor >> --enable-drm-compositor --enable-wayland-compositor >> --enable-headless-compositor --enable-fbdev-compositor >> --enable-screen-sharing --enable-vaapi-recorder --enable-simple-clients >> --enable-simple-egl-clients --enable-simple-dmabuf-drm-client >> --enable-simple-dmabuf-v4l-client --enable-clients >> --enable-resize-optimization --enable-weston-launch >> --enable-fullscreen-shell --enable-colord --enable-dbus >> --enable-systemd-login --enable-junit-xml --enable-ivi-shell >> --enable-wcap-tools --disable-libunwind --enable-demo-clients-install >> --enable-lcms --with-cairo=image > > There is no option --disable-libunwind anymore. Right you are. > Should there be an explicit --disable-rdp-compositor? > > Alternatively, would stretch have any freerdp version Weston builds > with? This can be done later, too. Yeah, there should be an explicit disable for now; freerdp2 isn't in stretch at all. Later we could try to expand this to building on different distros, some of which would provide a new enough RDP, and re-enable it. >> + - make -j4 >> + - make check >> + - make install >> + - make distcheck > > Distcheck does not --disable-xwayland-test, does it? So should we match > it with the autogen.sh arguments? Interesting; it does disable it, but I guess we implicitly pulled in Xwayland from Debian dependencies, as the run did succeed: https://gitlab.freedesktop.org/daniels/weston/-/jobs/1172 Maybe I could just add the Xwayland server to the apt arguments to be explictly installed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Obtaining Xorg DDX commit privilege
Hi Kevin, On 5 June 2018 at 23:37, Kevin Brace wrote: > I did ask freedesktop.org to grant me commit privilege to xf86-video-* DDX > repositories, but I have yet to hear from them. > > https://bugs.freedesktop.org/show_bug.cgi?id=106605 For accounts, freedesktop.org are just a very low-value proxy to the actual project owners. The reason your bug hasn't been acted on is that it needs to have someone from X.Org say 'yes, this is fine and Kevin should have commit access to all our repositories'. Often the process is to start sending out patches, e.g. to fix these compilation warnings for the other DDXes, and after a few have been merged and both sides have built up some confidence then you apply for direct commit access. Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH weston 2/2] Add .gitlab-ci.yml
Signed-off-by: Daniel Stone --- .gitlab-ci.yml | 48 1 file changed, 48 insertions(+) create mode 100644 .gitlab-ci.yml diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml new file mode 100644 index 0..202fa5494 --- /dev/null +++ b/.gitlab-ci.yml @@ -0,0 +1,48 @@ +image: debian:stretch + +stages: + - build + +before_script: + - echo 'path-exclude=/usr/share/doc/*' > /etc/dpkg/dpkg.cfg.d/99-exclude-cruft + - echo 'path-exclude=/usr/share/man/*' >> /etc/dpkg/dpkg.cfg.d/99-exclude-cruft + - echo '#!/bin/sh' > /usr/sbin/policy-rc.d + - echo 'exit 101' >> /usr/sbin/policy-rc.d + - chmod +x /usr/sbin/policy-rc.d + - apt-get update + - apt-get -y --no-install-recommends install build-essential automake autoconf libtool pkg-config libexpat1-dev libffi-dev libxml2-dev libpixman-1-dev libpng-dev libjpeg-dev libcolord-dev mesa-common-dev libglu1-mesa-dev libegl1-mesa-dev libgles2-mesa-dev libwayland-dev libxcb1-dev libxcb-composite0-dev libxcb-xfixes0-dev libxcb-xkb-dev libx11-xcb-dev libx11-dev libudev-dev libgbm-dev libxkbcommon-dev libcairo2-dev libpango1.0-dev libgdk-pixbuf2.0-dev libxcursor-dev libmtdev-dev libpam0g-dev libvpx-dev libsystemd-dev libinput-dev libwebp-dev libjpeg-dev libva-dev liblcms2-dev git + - mkdir -p /tmp/.X11-unix + - chmod 777 /tmp/.X11-unix + +build-native: + stage: build + script: + - git clone --depth=1 git://anongit.freedesktop.org/git/wayland/wayland-protocols + - export WAYLAND_PROTOCOLS_DIR="$(pwd)/prefix-wayland-protocols" + - export PKG_CONFIG_PATH="$WAYLAND_PROTOCOLS_DIR/share/pkgconfig:$PKG_CONFIG_PATH" + - cd wayland-protocols + - git show -s HEAD + - mkdir build + - cd build + - ../autogen.sh --prefix="$WAYLAND_PROTOCOLS_DIR" + - make install + - cd ../../ + - export XDG_RUNTIME_DIR="$(mktemp -p $(pwd) -d xdg-runtime-XX)" + - export BUILD_ID="weston-$CI_JOB_NAME_$CI_COMMIT_SHA-$CI_JOB_ID" + - export PREFIX="$(pwd)/prefix-$BUILD_ID" + - export BUILDDIR="$(pwd)/build-$BUILD_ID" + - mkdir "$BUILDDIR" "$PREFIX" + - cd "$BUILDDIR" + - ../autogen.sh --prefix="$PREFIX" --disable-setuid-install --enable-xwayland --disable-xwayland-test --enable-x11-compositor --enable-drm-compositor --enable-wayland-compositor --enable-headless-compositor --enable-fbdev-compositor --enable-screen-sharing --enable-vaapi-recorder --enable-simple-clients --enable-simple-egl-clients --enable-simple-dmabuf-drm-client --enable-simple-dmabuf-v4l-client --enable-clients --enable-resize-optimization --enable-weston-launch --enable-fullscreen-shell --enable-colord --enable-dbus --enable-systemd-login --enable-junit-xml --enable-ivi-shell --enable-wcap-tools --disable-libunwind --enable-demo-clients-install --enable-lcms --with-cairo=image + - make -j4 + - make check + - make install + - make distcheck + artifacts: +name: weston-$CI_COMMIT_SHA-$CI_JOB_ID +when: always +paths: +- build-*/weston-*.tar.xz +- build-*/*.log +- build-*/logs +- prefix-* -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/2] simple-dmabuf-drm: Fallback DRM_FORMAT_MOD_LINEAR definition
Just in case we're running on something quite old. Signed-off-by: Daniel Stone --- clients/simple-dmabuf-drm.c | 4 1 file changed, 4 insertions(+) diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c index df96758c9..fcab30e5c 100644 --- a/clients/simple-dmabuf-drm.c +++ b/clients/simple-dmabuf-drm.c @@ -62,6 +62,10 @@ #include "fullscreen-shell-unstable-v1-client-protocol.h" #include "linux-dmabuf-unstable-v1-client-protocol.h" +#ifndef DRM_FORMAT_MOD_LINEAR +#define DRM_FORMAT_MOD_LINEAR 0 +#endif + struct buffer; /* Possible options that affect the displayed image */ -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland] Add .gitlab-ci.yml
Signed-off-by: Daniel Stone --- .gitlab-ci.yml | 34 ++ 1 file changed, 34 insertions(+) create mode 100644 .gitlab-ci.yml diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml new file mode 100644 index ..ce23cf3e --- /dev/null +++ b/.gitlab-ci.yml @@ -0,0 +1,34 @@ +image: debian:stretch + +stages: + - build + +before_script: + - echo 'path-exclude=/usr/share/doc/*' > /etc/dpkg/dpkg.cfg.d/99-exclude-cruft + - echo 'path-exclude=/usr/share/man/*' >> /etc/dpkg/dpkg.cfg.d/99-exclude-cruft + - echo '#!/bin/sh' > /usr/sbin/policy-rc.d + - echo 'exit 101' >> /usr/sbin/policy-rc.d + - chmod +x /usr/sbin/policy-rc.d + - apt-get update + - apt-get -y --no-install-recommends install build-essential automake autoconf libtool pkg-config libexpat1-dev libffi-dev libxml2-dev doxygen graphviz xmlto xsltproc docbook-xsl + +build-native: + stage: build + script: + - export BUILD_ID="wayland-$CI_JOB_NAME_$CI_COMMIT_SHA-$CI_JOB_ID" + - export PREFIX="$(pwd)/prefix-$BUILD_ID" + - export BUILDDIR="$(pwd)/build-$BUILD_ID" + - mkdir "$BUILDDIR" "$PREFIX" + - cd "$BUILDDIR" + - ../autogen.sh --prefix="$PREFIX" --with-icondir=/usr/share/X11/icons + - make -j4 + - make check + - make install + - make distcheck + artifacts: +name: wayland-$CI_COMMIT_SHA-$CI_JOB_ID +when: always +paths: +- build-*/wayland-*.tar.xz +- build-*/*.log +- prefix-* -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] [Piglit] GitLab migration of Piglit
On 5 June 2018 at 17:55, Eric Engestrom wrote: > On Tuesday, 2018-06-05 17:52:17 +0100, Daniel Stone wrote: >> > I assume that's now mesa, libdrm, piglit, shader-db, crucible, kmscube, >> > mesa-demos; what about igt? Anything else? >> >> We currently have under /git/mesa: >> clover.git >> crucible.git >> demos.git >> drm-gralloc.git >> drm.git >> glu.git >> glut.git >> glw.git >> kmscube.git >> libwsbm.git >> linux-agp-compat.git >> llvm.git >> mesa-test.git >> mesa.git >> r600_demo.git >> rbug-gui >> shader-db.git >> tasks.git >> vkpipeline-db.git >> vmwgfx.git > > This might just be my outsider impression, but aren't most of those > dead/no longer maintained? Some of them very certainly are, but keeping everything in one place still seems like it might be a reasonable ide. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev