Re: [RFC wayland] protocol: Add high-resolution wl_touch timestamp event
Hi, On 24 November 2017 at 11:59, Pekka Paalanen wrote: > On Tue, 21 Nov 2017 15:45:48 +0200 > Alexandros Frantzis wrote: >> 2. Should we introduce similar timestamp events for keyboard and pointer? > > No, unless someone actually has a use for them. That brings the > question, why are you only interested in touch timestamps and not > keyboard and pointer? > > Why are keyboard and pointer latency not interesting to you? I would very much suggest doing so, yeah. Having timestamps for high-frequency/resolution mice seems like a real need. Given the need to correllate the event streams (e.g. 'is someone holding down Ctrl whilst I scroll?'), it makes it easier for clients if we have comparable timestamps between the two. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] gl-renderer: Set pitch correctly for subsampled textures
Hi all, On 29 November 2017 at 18:49, Arnaud Vrac wrote: > this patch is Reviewed-by: Arnaud Vrac I've pushed this with a pile of tags now. Thanks everyone! Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 2/4] tests: Mark tests as used, so they don’t get removed at link-time.
Hi, On 14 April 2017 at 19:48, Emmanuel Gil Peyrot wrote: > Without this attribute, these macros were making Weston’s tests fail to > build with LTO enabled. I've pushed all four of these (i.e. the two for Wayland, two for Weston) upstream now. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] connection: Print the content of arrays in closures
Hi, On 10 July 2017 at 19:28, Emmanuel Gil Peyrot wrote: > The current behaviour when WAYLAND_DEBUG is set is to print “array”, > which is quite unhelpful. > > This patch prints a list of the bytes present in the array. It doesn’t > try to interpret it as uint32_t or anything, leaving that to the reader > because this information isn’t present in the protocol description. To be honest, I'm not really wild about this one. All the array users I know of (key/button, xdg surface states) are uint32_t. I'd prefer to add a pretty-printing hint to the protocol - this could specify how to interpret arrays, and also scratch my long-standing itch of printing uints with %x rather than %u when it makes sense to ... Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] weston-info: Add support for zwp_linux_dmabuf_v1
Hi, On 3 October 2017 at 14:36, Emmanuel Gil Peyrot wrote: > This now prints each (format, modifier) tuple, to show which ones the > compositor sends to its clients. It is only implemented for version 3+, > since I didn’t have any compositor implementing previous versions, and > the old `format` event is deprecated anyway. Useful indeed, thanks! Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Scanner with --no-documentation option (source-code included)
Hi Felipe, On 2 December 2017 at 20:17, ferreiradaselva wrote: > I don't know how useful this feature would be for others (it would be for > me), but I made the wayland-scanner to take a no-documentation option. It > omits the documentation comments (but still keep the copyright notice). Is > this something of interest? > > I added the source on > https://gist.github.com/ferreiradaselva/ad04b8c6302a376f4bff0477ea01d129 Thanks very much for preparing this. It's hard to comment on the code though, without being able to see the differences. The usual manner patches are prepared is: * create a new branch in your Git worktree (say, scanner-no-doc) * make the changes you want to your source code * commit those changes * use git send-email to post the changes to the list That way we can see just the differences between your code and mainline, rather than having to read the whole file and try not to miss anything. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] weston: add --device option for DRM-backend
Hi Pekka, On 28 March 2017 at 16:26, Pekka Paalanen wrote: > Developers with testing rigs having multiple graphics cards plugged in > often want to test things on a specific card. We have ways to choose a > card through seat assignments, but configuring that run by run is > awkward. > > Add a command line option for opening a specific DRM device. > > [...] > > --- a/compositor/main.c > +++ b/compositor/main.c > @@ -565,6 +565,7 @@ usage(int error_code) > " --connector=ID\tBring up only this connector\n" > " --seat=SEAT\t\tThe seat that weston should run on\n" > " --tty=TTY\t\tThe tty to use\n" > + " --device=CARD\t\tThe DRM device to use, e.g. \"card0\".\n" > " --use-pixman\t\tUse the pixman (CPU) renderer\n" > " --current-mode\tPrefer current KMS mode over EDID > preferred mode\n\n"); > #endif > @@ -1225,6 +1226,7 @@ load_drm_backend(struct weston_compositor *c, > { WESTON_OPTION_INTEGER, "connector", 0, &config.connector }, > { WESTON_OPTION_STRING, "seat", 0, &config.seat_id }, > { WESTON_OPTION_INTEGER, "tty", 0, &config.tty }, > + { WESTON_OPTION_STRING, "device", 0, &config.specific_device > }, Please can we call it --drm-device instead? With that, the two 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: [weston v2] linux-dmabuf: align DMABUF exposed formats with EGL supported formats
Hi Vincent, On 20 March 2017 at 09:50, Vincent ABRIOU wrote: > Any feedback on this patch? Sorry for not replying sooner. With the modifiers code having landed, this would need some rework to fit there. When the modifiers query is available, we'd need to drop the RGB format advertisement. For the YUV formats, we'd need to check the base formats that each YUV format decomposes to (e.g. for NV12, check R8 + RG88), take the intersection of available modifiers for those formats, and send out the intersection with the YUV format. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] editor: unify key handling
Hi Weng, Thanks for the patch, and sorry for the horrendous delay in giving you feedback. In general, your patch looks very good and makes a lot of sense. However: On 1 February 2017 at 03:22, Weng Xuetian wrote: > @@ -395,81 +402,8 @@ text_input_keysym(void *data, > uint32_t modifiers) > { > struct text_entry *entry = data; > - [...] > + handle_key(entry, time, key, state, modifiers); > } > > @@ -1381,29 +1313,19 @@ handle_bound_key(struct editor *editor, > } > > static void > -key_handler(struct window *window, > - struct input *input, uint32_t time, > - uint32_t key, uint32_t sym, enum wl_keyboard_key_state state, > - void *data) > -{ > - struct editor *editor = data; > - struct text_entry *entry; > +handle_key(struct text_entry *entry, uint32_t time, > + uint32_t sym, enum wl_keyboard_key_state state, > + uint32_t modifiers) { > + struct input *input = entry->input; > const char *new_char; > char text[16]; > - uint32_t modifiers; > - > - if (!editor->active_entry) > - return; > - > - entry = editor->active_entry; > - > - if (state != WL_KEYBOARD_KEY_STATE_PRESSED) > + if (!input || state != WL_KEYBOARD_KEY_STATE_PRESSED) > return; > > modifiers = input_get_modifiers(input); > if ((modifiers & MOD_CONTROL_MASK) && > (modifiers & MOD_SHIFT_MASK) && This is now incorrect. It overrides the modifiers passed from the keysym event, with those from the general keyboard state, which may be different. Note that the two sets of modifiers are different. The keysym modifiers are 'raw', e.g. see how we look up shift_mask from the XKB keymap and compare against that in the keysym state. In order to use the same codepath, we would need to transform the modifiers from the keysym event the way that input_get_modifiers() does, i.e. into MOD_CONTROL_MASK/MOD_SHIFT_MASK. Could you please submit another revision correcting this? You may want to split this into a couple patches: one preparatory patch which allows clients to do the input_get_modifiers() mapping, another patch which makes the keysym handler use that (deleting shift_mask in the process), and then one last patch which merges the two. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/2] server: add log message when client connection is destroyed due to an error
Hi Mathias, Thanks for your patch! The idea seems fine, but I have a few comments. On 8 June 2017 at 08:39, wrote: > @@ -313,16 +326,30 @@ wl_client_connection_data(int fd, uint32_t mask, void > *data) > uint32_t resource_flags; > int opcode, size, since; > int len; > + int err; > + socklen_t errlen; > > - if (mask & (WL_EVENT_ERROR | WL_EVENT_HANGUP)) { > + if (mask & WL_EVENT_HANGUP) { > wl_client_destroy(client); > return 1; > } > > + if (mask & WL_EVENT_ERROR) { > + // query socket error > + errlen = sizeof(err); > + if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &errlen)) { > + // when getsockopt fails use its error instead > + err = errno; I don't believe that errno is necessarily related in this case, so we can't pass it to the client. > @@ -334,7 +361,8 @@ wl_client_connection_data(int fd, uint32_t mask, void > *data) > if (mask & WL_EVENT_READABLE) { > len = wl_connection_read(connection); > if (len == 0 || (len < 0 && errno != EAGAIN)) { > - wl_client_destroy(client); > + destroy_client_with_error( > + client, "failed to read client connection", > errno); If (len == 0), errno may be irrelevant. The same is true in your first patch: I believe there are a few places where we might leak errno to the client when it is not actually relevant. It would be nice to change the 'if (client->error)' checks in that patch to (client->error != 0) as well, just to be more explicit. Also, your patch uses // C++ comments like this. Please use /* Old-style C comments */ instead, but whilst you're at it, comments like 'query socket error' are obvious enough from reading the surrounding context that they are not really required. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC weston] libweston: Do not include subsurfaces with NULL buffer in bounding box
Hi Philipp, On 28 July 2017 at 15:41, Philipp Kerling wrote: > I was pondering how to remove the window decorations of my application > (which live in subsurfaces) when going full screen without flickering. > > At first I just destroyed the surfaces, but that lead to flicker > (window without decoration will appear for one frame before the full > screen application is displayed) since subsurface destruction is not > specified to be synchronized to the commit of the main surface. > > Then I tried attaching a NULL wl_buffer to the surfaces. This works and > does not flicker, but it has the problem that weston will disconnect my > application when going to full screen for violating the configured > xdg_shell buffer size. The reason seems to be that attaching a NULL > wl_buffer does not reset the surface size and > weston_surface_get_bounding_box still thinks that the surface has the > prior size. > > The patch fixes this by checking explicitly for the buffer and is > confirmed to fix the issue, but I am not sure whether this is the right > solution. Maybe surface->width and surface->height should be set to 0 > anyway? Hm. Yes, it does seem like the width/height should be 0, which is undefined but not disallowed by the subsurface spec. I have no idea if this would work with Mutter in particular, which had problems implementing that for top-levels. In any case, perhaps one way you could do it would be to stack the decoration subsurfaces below your primary surface before committing your fullscreen buffer. It's ugly, but does seem like it should work ... Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] scanner: Add autoconf macro to check for the proper scanner
Jussi, Tomek, Emil, On 18 August 2017 at 10:36, Quentin Glidic wrote: > On 8/18/17 11:30 AM, Quentin Glidic wrote: >> Projects have been using various ways to check for the wayland-scanner, >> mostly based on their developper own use case, and often not allowing >> others use cases to work without a workaround. >> >> Hopefully this macro will support all use cases without needing user >> action. >> >> Signed-off-by: Quentin Glidic >> --- >> >> Everyone should test this macro for their own project and use cases. >> Using the ${WAYLAND_SCANNER} variable should just work (assuming you have >> the proper >> wayland package built in the expected env). In very very rare cases, >> setting the >> WAYLAND_SCANNER variable as a ./configure argument may be needed, but >> nothing else. >> >> Please let me know with enough details if your use case is not working >> with it. Any comments on Quentin's suggestion here? Would they be enough for you? Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2 wayland 04/11] connection: Make wl_closure_destroy() close fds of undispatched closures
Hi Derek, On 13 April 2017 at 17:51, Derek Foreman wrote: > void > -wl_closure_destroy(struct wl_closure *closure) > +wl_closure_destroy(struct wl_closure *closure, bool dispatched) > { > + /* wl_closure_destroy has free() semantics */ > + if (!closure) > + return; > + /* If message is NULL then this closure failed during > +* creation, and any fd cleanup has been done by the > +* caller > +*/ > + if (!dispatched && closure->message) > + wl_closure_close_fds(closure); > free(closure); > } Maybe there's something I'm missing here, but is there any reason to not set closure->message unconditionally? If you moved the closure->message assignment up inside marshal/demarshal, and ran through initialising all the handle-type arg values to -1 (let's call this hypothetical function wl_closure_init - take an 'extra' size param to preserve the single-alloc demarshal semantics as well), then you could just use this everywhere rather than manual fd counting. Also, please use an enum rather than true/false: it was only after I got to like four 'this seems completely backwards?' comments that I realised true meant 'don't do the new behaviour', and false meant 'do extra stuff'. I blame the flu medication, but still ... Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2 wayland 00/11] Stop leaking file descriptors
Hi Derek, On 13 April 2017 at 17:51, Derek Foreman wrote: > Moved the test cases to the end so they're not introduced in a failed > state. > > Reworked the removal of the global zombie singleton patch - we now > create a wl_zombie at proxy creation time and store the number of > fds for each opcode in that. If no signature requires fds we just > use a NULL pointer to avoid useless malloc/free. At client > disconnect we do a map walk to inter any zombie left behind and > make sure we don't leak memory. > > Thanks to Pekka and Jonas for suggestions on how to handle this > without breaking API as I'd previously done. > > To perform the map walk I added a new patch that changes the > wl_map_for_each() function to provide the entry flags - we can't > look up flags from the iterator callback without dereferencing the > pointer, and with my changes we don't know what the data type stored > in that map location is. This only changes internal callers - the > publicly visible map walk function still acts as before. Thanks for this. It looks fairly good to me so far. I've pushed 1, 5 and 9 as trivially correct. As said in comments to 4, I feel like a slightly improved 4 could also replace 2 and 3 at the same time. 6, 8 and 10 all have my R-b, though a comment as to why three is the correct number of roundtrips wouldn't go astray on 10 (it seems right to me, mind). Unfortunately I'm not getting far into 11 without my eyes glazing over, and I haven't given 7 a pass over for actual lifetime handling yet, but that at least gets my A-b. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols 1/3] Add meson build system support
Hi Jonas, On 11 October 2017 at 10:00, Jonas Ådahl wrote: > +wayland_scanner = find_program('wayland-scanner') It would be good to have this follow Quentin's suggestion for how to find wayland-scanner generally. Apart from that, 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-protocols 2/3] tests: Add compile tests
Hi, Meant 'patch' rather than 'series' when offering my R-b to the other patch. Oops. On 11 October 2017 at 10:00, Jonas Ådahl wrote: > + # Check that header can be included by a pedantic C99 compiler > + test_name = 'test-build-pedantic-@0@'.format(protocol.underscorify()) > + test_name_source = '@0@.c'.format(test_name) > + test_source = custom_target(test_name_source, > + input: 'build-pedantic.c.in', > + output: test_name_source, > + command: replace_command) > + pedantic_test_executable = executable(test_name, > +[ test_source, > + client_header, > + server_header, > + code ], > +dependencies: libwayland, > +c_args: [ '-std=c99', > + '-pedantic', > + '-Wall', > + '-Werror' ], > +install: false) > + test(test_name, pedantic_test_executable) > + > + # Check that the header > + if not protocol.contains('xdg-foreign-unstable-v1') The comment ends abruptly, the xdg_foreign exclusion is a massive non-sequitur, and also test_configuration can be deleted. With that, this is also: Reviewed-by: Daniel Stone Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC PATCH xserver] xwayland: Fix non-argb cursor conversion
On 27 September 2017 at 17:01, Olivier Fourdan wrote: > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=103012 > Signed-off-by: Olivier Fourdan Reviewed-by: Daniel Stone ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC wayland-protocols] presentation-time: Add request to subscribe to wl_output presentation timings
Hi Alexandros, On 31 August 2017 at 16:37, Alexandros Frantzis wrote: > thank you for the additional feedback. I have attached an updated > version which addresses your comments. > > I still haven't got any feedback from Chromium concerning the potential > change from wl_output based timings to wl_surface based timings as > discussed in previous emails, so, for the time being, I am keeping this > proposal in RFC status. I think this ended up being fine for Chromium/Exosphere, right? Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] scanner: Add autoconf macro to check for the proper scanner
Hi Quentin, I've added Ross and Fabien, who've been looking at this for Yocto. On 18 August 2017 at 10:30, Quentin Glidic wrote: > --- a/wayland-scanner.m4 > +++ b/wayland-scanner.m4 > @@ -1,3 +1,5 @@ > +#serial 2 I don't quite understand the significance of this. > +# WL_PROG_WAYLAND_SCANNER() > +AC_DEFUN([WL_PROG_WAYLAND_SCANNER], [ > +AC_REQUIRE([AC_CANONICAL_BUILD]) > +AC_REQUIRE([PKG_PROG_PKG_CONFIG]) > +wl_cv_native_pkg_config= > +AS_IF([test x${cross_compiling} = xyes], [ > +wl_cv_native_pkg_config=${build}-pkg-config > +AC_PATH_PROGS([wl_cv_native_pkg_config], [${build}-pkg-config]) Ah, no thanks. I would prefer to have this passed in as $HOST_PKG_CONFIG (following the $HOST_CC precedent), falling back to just plain 'pkg-config'. Guessing triplets is a bit too much like magic, especially when you can be 'cross-compiling' to the same triplet. > +wl_cv_scanner_found=no > +wl_cv_scanner_wanted_version=`${PKG_CONFIG} --modversion wayland-server` > +AC_MSG_CHECKING([that wayland-client and wayland-server versions are the > same]) > +AS_IF([test ${wl_cv_scanner_wanted_version} = `${PKG_CONFIG} > --modversion wayland-client`], [ > +AC_MSG_RESULT([ok]) > +], [ > +AC_MSG_ERROR([mismatch]) > +]) > +AC_MSG_CHECKING([for wayland-scanner ${wl_cv_scanner_wanted_version}]) > +AS_IF([test x${ac_cv_env_WAYLAND_SCANNER_set} = xset], [ > +_WL_PROG_WAYLAND_SCANNER_VERSION_CHECK() > +]) > +AS_IF([test x${cross_compiling} = xyes -a x${wl_cv_native_pkg_config} != > xno], [ > +AS_IF([AC_RUN_LOG([${wl_cv_native_pkg_config} --exists > --print-errors wayland-scanner = ${wl_cv_scanner_wanted_version}])], [ > +WAYLAND_SCANNER=`${wl_cv_native_pkg_config} > --variable=wayland_scanner wayland-scanner = ${wl_cv_scanner_wanted_version}` > +_WL_PROG_WAYLAND_SCANNER_VERSION_CHECK() > +]) > +]) > +AS_IF([test x${wl_cv_scanner_found} = xno], [ > +AS_IF([AC_RUN_LOG([${PKG_CONFIG} --exists --print-errors > wayland-scanner = ${wl_cv_scanner_wanted_version}])], [ > +WAYLAND_SCANNER=`${PKG_CONFIG} --variable=wayland_scanner > wayland-scanner = ${wl_cv_scanner_wanted_version}` > +_WL_PROG_WAYLAND_SCANNER_VERSION_CHECK() > +]) > +]) I found this whole AS_IF forest pretty difficult to read. The last one seems like we could also try to end up running a non-native version if we fall through? It's also quite verbose on checking. Shrug. The rest looks good to me, so assuming we can test it in Yocto and it works fine there, then it should be good to merge. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] configure.ac: use PKG_CHECK_VAR for wayland-protocols
Hi, On 18 July 2017 at 14:31, Pekka Paalanen wrote: > On Fri, 7 Jul 2017 10:51:16 +0200 Olivier Blin > wrote: >> Also note that this requires a relatively new pkg-config for PKG_CHECK_VAR. >> The macro appeared in pkg-config 0.28, from January 2013. >> >> It is for example not available in Ubuntu 14.04 LTS, which only provides >> pkg-config 0.26. > > Hi, > > is anyone actually against requiring pkg-config 0.28? It sounds fine to > me, so I'll give: > Acked-by: Pekka Paalanen I think requiring it is fine. > I think the pkg-config's own version check would need bumping, and I > have a feeling the above M4 could use more quoting just in case. Yes, the second and third arguments should be quoted. > If the MODULE requirement for PKG_CHECK_VAR is not satisfied, does it > fail the whole configure like we would want? The documentation does not > say. It doesn't, but takes action-if-found and action-if-not-found arguments. Olivier, could you please respin to add an AC_MSG_ERROR failure when it's not found? Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] clients: Don't crash when compositor doesn't support drag and drop
Hi Derek, On 20 April 2017 at 20:31, Derek Foreman wrote: > display_create_data_source() can return NULL when there's no data device > manager present. Instead of carrying on blindly, test its return value. Both seem trivially correct, reviewed and pushed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] tests: Fix integer overflows on 32-bit systems
Hi Alexandros, On 1 December 2017 at 17:28, Alexandros Frantzis wrote: > Ensure that the integer type used in expressions involving > multiplication with NSEC_PER_SEC is large enough to avoid overflows on > 32-bit systems. In the expressions fixed by this patch a 64-bit type > (long long) is required. Thanks! Reviewed and pushed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] weston: added missing header time.h
Hi Valery, On 9 August 2017 at 12:27, Valery Kartel wrote: > without it I can't built weston on alpinelinux Thanks for your patch; I've reviewed and pushed it. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] compositor-drm: fix z-order inversion in plane assignment
Hi Matt, On 5 September 2017 at 15:13, Matt Hoosier wrote: > On Tue, Sep 5, 2017 at 9:00 AM, Daniel Stone wrote: >> It can't be correct to raise it to the cursor plane either, since both >> cursor and overlay planes strictly stack above the scanout plane. I >> guess this would read a lot easier with: >> if (picked_scanout) >> next_plane = primary; >> at the top of the loop. > > Okay, sure. I think that in practice, no cursor surface would ever > have the right dimensions to get picked by prepare_overlay_view() or > prepare_scanout_view(), but I agree with you in principle. > > Thanks for the feedback. I see that you've resubmitted the atomic > modesetting series again. Do you feel like it's near enough landing > that there's no use in trying to commit a copy of this fix to master? Might as well just push it by now, which I have in fact done last night. Thanks! Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] gl-renderer: remove unneeded cast
Hi Emil, On 31 July 2017 at 20:45, Emil Velikov wrote: > The variable num is of EGLint type. > > Signed-off-by: Emil Velikov Reviewed and pushed, thanks. > Unrelated: > > The functions gl_renderer_query_dmabuf_* return bool as per the > interface. At the same time: > - the caller does not check for it > - the function happily writes over the output storage on error > > Worth just dropping the return type, and documenting that on error > num_formats shall return 0? Seems reasonable enough, yeah. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] weston-resizor: Don't add new frame callbacks every click
Hi Derek, On 24 March 2017 at 17:39, Derek Foreman wrote: > Sorry, I forgot about this for a while. So did everyone else, it seems. > On 17/02/17 10:31 PM, Jonas Ådahl wrote: >> Instead what we could do is probably to check the result of >> window_lock_pointer(), if it succeeded, continue with setting >> resizor->locked_input. Before trying to lock, check if we already have a >> locked_input, and if so, skip locking and adding the frame callback. >> Would also mean we need to clean up the locked_input on button-release >> and on unlocked (though seems weston-resizer crashes when unlocked by >> the compositor and not by itself). > > I'll gladly review and test such a patch. Should I add a ticket in bugzilla > so this bug isn't forgotten? No idea what's going on here, but I've reviewed and pushed this patch in the meantime. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] config-parser: fix `short_name` type
Hi Eric, On 8 June 2017 at 22:20, Eric Engestrom wrote: > On Wednesday, 2017-05-24 21:23:14 +0100, Eric Engestrom wrote: >> This field is populated with chars, compared to chars and printed as >> a char. It should probably be a char. >> >> Signed-off-by: Eric Engestrom > > Humble ping? > > I don't have commit access either, so you'll have to push this for me :) Sorry for the hideous delay. Reviewed and pushed both now. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2] configure.ac: use AC_HEADER_MAJOR to detect major()/minor()
Hi Sergei, On 31 May 2017 at 22:17, Sergei Trofimovich wrote: > This change slightly updates c4d7f66c12853b9575366dd9f4a7960ec5694934 > which added inclusion. > > Autoconf has AC_HEADER_MAJOR to find out which header defines > reqiured macros: > > https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Particular-Headers.html > > This change should increase portability across other libcs. Thanks for this patch, it's now reviewed and pushed to mainline. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] clients: simple-egl: Restore window size when un-maximized
Hi, On 26 June 2017 at 17:01, Tomohiro Komagata wrote: > The window position was correct but the window size was wrong > when simple-egl returns from maximized window to un-maximized. > Its size should be restored to original size. Thankyou very much for the patch. I have reviewed it and pushed it to git. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] compositor-x11: Implement mode switching
Hi Armin, On 28 October 2016 at 23:26, Armin Krezović wrote: > Signed-off-by: Armin Krezović I couldn't find anything wrong with this one, so have reviewed and pushed it now. Was probably about time. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] input: Do not override keyboard focus on restore
Hi Quentin, On 21 July 2017 at 13:02, Quentin Glidic wrote: > If we start a special (grabbing) client when Weston is unfocused, it > would lose focus when coming back to Weston. Thanks for this; reviewed and pushed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/3] tests: add a create_test_surface function
Hi Emilio, On 3 February 2017 at 15:10, Emilio Pozuelo Monfort wrote: > This doesn't attach a buffer to the surface. This is needed for the > next commit, where we have a test case with a surface that doesn't > have a buffer attached. Thanks, this seems obviously fine so I've pushed this patch. Looks like the other two need more work though. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] desktop-shell: Handle the fullscreen to maximized case safely
Hi Derek, On 6 October 2017 at 20:37, Derek Foreman wrote: > When a client transitions from maximized to fullscreen to maximized (run > weston-terminal, maximize it, hit f11 twice) we're sending size 0,0 for > the unfullscreen configure, which still has maximized set. > > This results in clients correctly picking any size they like, and weston > disconnecting them for it. > > Instead, pass the correct maximized size. Thanks, sounds plausible enough on the face of it. Reviewed and pushed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwm: Handle third data entry in client messages
Hi Iliya, On 24 June 2017 at 18:53, Iliya Bozhinov wrote: > A single client message can be used to modify two properties at once. > That's why when processing such messages we have to check both the second > and the third data entry for states that we must handle. Thanks, correct you are. I've reviewed and pushed this now; sorry for the long delay. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] cursor: add forward declaration for struct wl_buffer
Hi Emil, On 12 October 2017 at 13:39, Emil Velikov wrote: > This makes the header self-contained, since the struct is considered > opaque from waylad-cursor POV. > > As we're here move the wl_shm fwd. declaration alongside the others. > Making it easier to read and track. Thanks for this! Reviewed and pushed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] protocol: Add deprecation note about wl_shell
Hi Jonas, On 2 December 2017 at 02:57, Jonas Ådahl wrote: > Now that xdg_shell is stable and much better defined than wl_shell we > can finally deprecate wl_shell and guide users towards xdg_shell > instead. [its-happening.gif] Reviewed and pushed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] eventloop: clarify post_dispatch_check()
Hi Chris, On 28 August 2017 at 09:03, wrote: > This *technically* changes the semantics of the return value of the source > callbacks. > Previously you could return a negative number from a source callback and it > would prevent > *other* source callbacks from triggering a subsequent recheck. > > Doing that seems like such a bad idea it's not worth supporting. > > v2: Log this case if it is hit, so we don't silently change behaviour. Thanks for this! Reviewed and pushed now. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] Do not create man page links with doxygen
Hi Armin, On 27 July 2017 at 13:08, Armin K wrote: > There is a lot of files created with .so links to non-installed > files, making most of installed pages useless. The files > referenced in .so links are not suitable for installation nor > do they contain any useful information for them to be worth > fixing. Thanks, reviewed and pushed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] protocol: make get_subsurface double-buffered
Hi Pekka, On 9 May 2016 at 12:45, Pekka Paalanen wrote: > The existing specification was not explicitly clear on when > wl_subcompositor.get_subsurface request actually adds the sub-surface to > the parent in the compositor's scenegraph. The implicit assumption was > that this happens immediately, but it was not written anywhere. I think now is as good a time as any to actually land this, with the million review/ack tags it's accumulated since. Which I've now done. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [weston v2 0/8] Implement NET_WM_SYNC_REQUEST basic support
Hi Louis-Francis, On 13 November 2017 at 21:20, Louis-Francis Ratté-Boulianne wrote: > This patchset implements NET_WM_SYNC_REQUEST protocol for throtting > X11 window resizes in Weston's XWM. We wait for the window to be drawn > (by setting an alarm on the sync counter) before configuring the window > again. > > I also did some small fixes the Xwayland window manager as to better > respect size hints from X11 applications and to display the title > more elegantly when there isn't enough space available. I've merged 2, 4, 7 and 8 now. I would certainly like to look at the actual sync support when my brain doesn't feel like it's encased in treacle. I held off on pushing the size-hints part of it after the replies to both of those. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] scanner: Add autoconf macro to check for the proper scanner
Hi, On 5 December 2017 at 14:08, Emil Velikov wrote: > On 4 December 2017 at 21:47, Daniel Stone wrote: >> Any comments on Quentin's suggestion here? Would they be enough for you? > > I fear that the ship has sailed long time ago and there will be > practically zero users of this. > > AFAICT people have been stacking band-aid for years as opposed to > addressing things properly. > With key offender being Yocto and similar tools :-\ The Yocto people are in this thread; they've already dropped a bunch of local hacks they had throughout the tree in favour of proper solutions upstream. Hopefully this will get the last of it out. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 1/3] xwayland: Fix input coordinates of non-decorated windows
Hi Scott, On 24 June 2017 at 08:18, Scott Moreau wrote: > Windows without decorations drawn by the xwayland window manager had > wrong input coordinates because weston_wm_window_get_child_position was > giving wrong x/y offsets. Set the offsets to 0 for windows without > system decorations. I tried to untangle what was going on through xwm here and just tied myself in a knot. It looks like this case would also need a similar change in weston_wm_window_get_frame_size, right? That should be relatively harmless (resulting in us setting larger regions than the actual window size), but still the same thing. This is all a bit of a mess, but it seems correct enough that, with the get_frame_size() change as well, this would be: Acked-by: Daniel Stone Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/3] xwayland: Shape window region to clip shadow area from input
Hi Scott, On 24 June 2017 at 08:18, Scott Moreau wrote: > Decorated windows were getting the size of the window including the > shadow which is correct, but causing some confusion for motion input > events. This was exhibited when one xwayland window overlaps another, > hovering over the window beneath recieves no events if within the 32 > pixel shadow border area of the window above. Seems correct, but can't this just use frame_input_rect() instead of hard-coding the sizes? Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] configure.ac: drop spurious bracket
Hi Guido, On 8 December 2017 at 19:46, Guido Günther wrote: > Otherwise configure output looks like > > checking for library containing pam_open_session... -lpam > ./configure: line 18064: ]: command not found > checking for COLORD... yes Sorry for the noise. Applied and pushed. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] libweston: drop return type from ::query_dmabuf_{formats, modifiers}
Hi Emil, On 5 December 2017 at 14:26, Emil Velikov wrote: > Nobody checks for the bool returned by these functions. At the same > time: a) the functions set the respective num_foo to zero on error and > b) callers honour that variable. > > Just drop the return type - it's useless. Reviewed and pushed, thanks! Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] libweston-desktop/xwayland: Make sure racy surfaces are properly mapped
Hi Quentin, On 8 December 2017 at 10:10, Quentin Glidic wrote: > This fixes a race between Xwayland committing the surface content via > the wl_surface, and the XWM setting the role of the surface. > We now keep track of the (first) content commit on the surface and > forward it to the shell when we finally get the role. > There is no need to track later changes, as the only way for Xwayland to > unmap a surface is to destroy it. Pushed with my ack, thanks. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 4/5] gl-renderer: fix pixel format used in texture uploads when using R/RG textures
Hi Arnaud, On 29 November 2017 at 14:06, Arnaud Vrac wrote: > In glTexImage2D / glTexSubImage2D calls, the only pixel formats allowed > for the GL_R8 and GL_RG internal formats are respectively GL_RED and > GL_RG [1]. > > Make sure we match this requirement, as some drivers will fail with the > current code. I've reviewed and pushed this and 5/5. You can re-send the first 3 with my Reviewed-by after dealing with Emil's suggestions. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Providing shared buffer for applications within Smack environment
Hi Pekka, On 12 December 2017 at 10:44, Pekka Paalanen wrote: > On Tue, 12 Dec 2017 11:00:23 +0100 > José Bollo wrote: >> While working for AGL [1], I want to allow applications to receive the >> buffers allocated by WESTON. The use of the surfaces/buffers >> allocated by Weston is difficult when Smack is activated. > > why do you need to make Weston allocate buffers? Why should those > buffers be accounted to Weston rather than a client that needs them? > > E.g. Weston's screenshooting currently has the client allocate buffers, > pass them to Weston, and Weston writes into them, then sends an event > to say they are done. I assume this is related to the never-upstreamed ivi_share protocol extension. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Providing shared buffer for applications within Smack environment
Hi Jose, On 12 December 2017 at 11:12, José Bollo wrote: > On Tue, 12 Dec 2017 11:00:08 + > Daniel Stone wrote: >> I assume this is related to the never-upstreamed ivi_share protocol >> extension. > > Yes the probability is near 100% > > So it could be a design feature issue. > > I'm not very aware of this part of the system. Can some one explain me > that share protocol, why it is needed and why it never-upstreamed? You'd have to ask the developers - Tanibata-san and the rest of the ADIT-J team. I've personally not seen it used, so I don't know what the motivation is. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] linux-dmabuf: send deprecated format events
Hi Michael and Philipp, On 15 December 2017 at 09:25, Philipp Zabel wrote: > Although the format event is deprecated, some clients, especially the > GStreamer waylandsink, only support zwp_linux_dmabuf_v1 version 1 and > require the deprecated format event. > > Send format events instead of the modifier event, if the client binds on > a protocol version before version 3, skipping formats that only support > non-linear modifiers. Thanks for this! Could I please persuade you to take on one additional change though? Specifically, if has_dmabuf_import_modifiers is not supported, gl-renderer will return 0 for num_formats. In this case, it seems like we should send a conservative list: ARGB, XRGB, and maybe speculatively the YUV formats we have hardcoded conversions for (provided we have R8/RG8 support, cf. the patches from Arnaud). Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] linux-dmabuf: send deprecated format events
Hi Philipp, On 15 December 2017 at 10:46, Philipp Zabel wrote: > On Fri, 2017-12-15 at 09:46 +0000, Daniel Stone wrote: >> On 15 December 2017 at 09:25, Philipp Zabel wrote: >> > Send format events instead of the modifier event, if the client binds on >> > a protocol version before version 3, skipping formats that only support >> > non-linear modifiers. >> >> Thanks for this! Could I please persuade you to take on one additional >> change though? >> >> Specifically, if has_dmabuf_import_modifiers is not supported, >> gl-renderer will return 0 for num_formats. In this case, it seems like >> we should send a conservative list: ARGB, XRGB, and maybe >> speculatively the YUV formats we have hardcoded conversions for >> (provided we have R8/RG8 support, cf. the patches from Arnaud). > > Should we modify gl_renderer_query_dmabuf_formats to return that list of > hardcoded formats if has_dmabuf_import_modifiers is not supported? That was exactly what I had in mind. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] configure: fix sys/sysmacros.h check
Hi, On 18 December 2017 at 08:24, Pekka Paalanen wrote: > This patch is a copy of > https://cgit.freedesktop.org/mesa/drm/commit/?id=7040fea0280bad527ed4b3d5eee7d7bfbf303efc > by Adam Jackson. > > Commit 43c5a65b034a243700cf9c5bfbe6bcefb15f1161 "configure.ac: use > AC_HEADER_MAJOR to detect major()/minor()" started using AC_HEADER_MAJOR > to detect the header where major() is defined. This caused a regression > on systems where glibc is still providing a deprecated definition of > major() through sys/types.h, leading to a bunch of compiler warnings: > > /home/pq/git/weston/libweston/launcher-logind.c: In function > ‘launcher_logind_open’: > /home/pq/git/weston/libweston/launcher-logind.c:182:13: warning: In the GNU C > Library, "major" is defined > by . For historical compatibility, it is > currently defined by as well, but we plan to > remove this soon. To use "major", include > directly. If you did not intend to use a system-defined macro > "major", you should undefine it after including . > fd = launcher_logind_take_device(wl, major(st.st_rdev), > > The issue has been discussed earlier on > https://lists.gnu.org/archive/html/autoconf/2016-09/msg00013.html > > Work around the issue by causing the warning to trigger a build failure > inside AC_HEADER_MAJOR test, so that we get MAJOR_IN_SYSMACROS defined. Painful, but seems like the right thing to do. 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 weston 1/1] xwayland - input size calculation correction
Hi Ian and Nandor, On 20 December 2017 at 10:09, Ian Ray wrote: > The input area for an undecorated non-fullscreen window is based on the > window size itself. > > Fixes a case where mouse events in the `invisible' window margin failed > to reach the window behind. Scott sent some patches to fix similar issues a while ago: https://lists.freedesktop.org/archives/wayland-devel/2017-June/thread.html#34332 I had some questions here: https://lists.freedesktop.org/archives/wayland-devel/2017-December/036116.html https://lists.freedesktop.org/archives/wayland-devel/2017-December/036117.html Would you have any input on these? Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 03/41] compositor-drm: Introduce drm_plane_state structure
Track dynamic plane state (CRTC, FB, position) in separate structures, rather than as part of the plane. This will make it easier to handle state management later, and much more closely tracks what the kernel does with atomic modesets. The fb_last pointer previously used in drm_plane now becomes part of output->state_last, and is not directly visible from the plane itself. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 348 - 1 file changed, 283 insertions(+), 65 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index e123fe503..6ea41ae80 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -268,6 +268,31 @@ struct drm_output_state { struct drm_pending_state *pending_state; struct drm_output *output; struct wl_list link; + struct wl_list plane_list; +}; + +/** + * Plane state holds the dynamic state for a plane: where it is positioned, + * and which buffer it is currently displaying. + * + * The plane state is owned by an output state, except when setting an initial + * state. See drm_output_state for notes on state object lifetime. + */ +struct drm_plane_state { + struct drm_plane *plane; + struct drm_output *output; + struct drm_output_state *output_state; + + struct drm_fb *fb; + + int32_t src_x, src_y; + uint32_t src_w, src_h; + int32_t dest_x, dest_y; + uint32_t dest_w, dest_h; + + bool complete; + + struct wl_list link; /* drm_output_state::plane_list */ }; /** @@ -286,11 +311,8 @@ struct drm_output_state { * are referred to as 'sprites'. */ struct drm_plane { - struct wl_list link; - struct weston_plane base; - struct drm_output *output; struct drm_backend *backend; enum wdrm_plane_type type; @@ -301,19 +323,10 @@ struct drm_plane { struct drm_property_info props[WDRM_PLANE__COUNT]; - /* The last framebuffer submitted to the kernel for this plane. */ - struct drm_fb *fb_current; - /* The previously-submitted framebuffer, where the hardware has not -* yet acknowledged display of fb_current. */ - struct drm_fb *fb_last; - /* Framebuffer we are going to submit to the kernel when the current -* repaint is flushed. */ - struct drm_fb *fb_pending; + /* The last state submitted to the kernel for this plane. */ + struct drm_plane_state *state_cur; - int32_t src_x, src_y; - uint32_t src_w, src_h; - uint32_t dest_x, dest_y; - uint32_t dest_w, dest_h; + struct wl_list link; uint32_t formats[]; }; @@ -944,6 +957,141 @@ drm_fb_unref(struct drm_fb *fb) } } +/** + * Allocate a new, empty, plane state. + */ +static struct drm_plane_state * +drm_plane_state_alloc(struct drm_output_state *state_output, + struct drm_plane *plane) +{ + struct drm_plane_state *state = zalloc(sizeof(*state)); + + assert(state); + state->output_state = state_output; + state->plane = plane; + + /* Here we only add the plane state to the desired link, and not +* set the member. Having an output pointer set means that the +* plane will be displayed on the output; this won't be the case +* when we go to disable a plane. In this case, it must be part of +* the commit (and thus the output state), but the member must be +* NULL, as it will not be on any output when the state takes +* effect. +*/ + if (state_output) + wl_list_insert(&state_output->plane_list, &state->link); + else + wl_list_init(&state->link); + + return state; +} + +/** + * Free an existing plane state. As a special case, the state will not + * normally be freed if it is the current state; see drm_plane_set_state. + */ +static void +drm_plane_state_free(struct drm_plane_state *state, bool force) +{ + if (!state) + return; + + wl_list_remove(&state->link); + wl_list_init(&state->link); + state->output_state = NULL; + + if (force || state != state->plane->state_cur) { + drm_fb_unref(state->fb); + free(state); + } +} + +/** + * Duplicate an existing plane state into a new plane state, storing it within + * the given output state. If the output state already contains a plane state + * for the drm_plane referenced by 'src', that plane state is freed first. + */ +static struct drm_plane_state * +drm_plane_state_duplicate(struct drm_output_state *state_output, + struct drm_plane_state *src) +{ + struct drm_plane_state *dst = malloc(sizeof(*dst)); + struct drm_plane_state *old, *tmp; + + assert(src); + assert(dst); + *dst = *src; +
[PATCH v14 14/41] compositor-drm: Discover atomic properties
From: Pekka Paalanen Set the atomic client cap, where it exists, and use this to discover the plane/CRTC/connector properties we require for atomic modesetting. Signed-off-by: Daniel Stone Co-authored-by: Pekka Paalanen Reviewed-by: Pekka Paalanen --- configure.ac | 3 +++ libweston/compositor-drm.c | 59 +- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 285c2efdf..72b5d6f23 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_ATOMIC, [libdrm >= 2.4.62], + [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])]) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 71e93c1c2..694e307bf 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -88,6 +88,16 @@ */ enum wdrm_plane_property { WDRM_PLANE_TYPE = 0, + WDRM_PLANE_SRC_X, + WDRM_PLANE_SRC_Y, + WDRM_PLANE_SRC_W, + WDRM_PLANE_SRC_H, + WDRM_PLANE_CRTC_X, + WDRM_PLANE_CRTC_Y, + WDRM_PLANE_CRTC_W, + WDRM_PLANE_CRTC_H, + WDRM_PLANE_FB_ID, + WDRM_PLANE_CRTC_ID, WDRM_PLANE__COUNT }; @@ -107,6 +117,7 @@ enum wdrm_plane_type { enum wdrm_connector_property { WDRM_CONNECTOR_EDID = 0, WDRM_CONNECTOR_DPMS, + WDRM_CONNECTOR_CRTC_ID, WDRM_CONNECTOR__COUNT }; @@ -139,6 +150,15 @@ struct drm_property_info { struct drm_property_enum_info *enum_values; /**< array of enum values */ }; +/** + * List of properties attached to DRM CRTCs + */ +enum wdrm_crtc_property { + WDRM_CRTC_MODE_ID = 0, + WDRM_CRTC_ACTIVE, + WDRM_CRTC__COUNT +}; + /** * Mode for drm_output_state_duplicate. */ @@ -195,6 +215,7 @@ struct drm_backend { int cursors_are_broken; bool universal_planes; + bool atomic_modeset; int use_pixman; @@ -347,6 +368,8 @@ struct drm_output { /* Holds the properties for the connector */ struct drm_property_info props_conn[WDRM_CONNECTOR__COUNT]; + /* Holds the properties for the CRTC */ + struct drm_property_info props_crtc[WDRM_CRTC__COUNT]; struct backlight *backlight; @@ -2889,6 +2912,15 @@ init_kms_caps(struct drm_backend *b) weston_log("DRM: %s universal planes\n", b->universal_planes ? "supports" : "does not support"); +#ifdef HAVE_DRM_ATOMIC + if (b->universal_planes && !getenv("WESTON_DISABLE_ATOMIC")) { + ret = drmSetClientCap(b->drm.fd, DRM_CLIENT_CAP_ATOMIC, 1); + b->atomic_modeset = (ret == 0); + } +#endif + weston_log("DRM: %s atomic modesetting\n", + b->atomic_modeset ? "supports" : "does not support"); + return 0; } @@ -3032,6 +3064,16 @@ drm_plane_create(struct drm_backend *b, const drmModePlane *kplane, .enum_values = plane_type_enums, .num_enum_values = WDRM_PLANE_TYPE__COUNT, }, + [WDRM_PLANE_SRC_X] = { .name = "SRC_X", }, + [WDRM_PLANE_SRC_Y] = { .name = "SRC_Y", }, + [WDRM_PLANE_SRC_W] = { .name = "SRC_W", }, + [WDRM_PLANE_SRC_H] = { .name = "SRC_H", }, + [WDRM_PLANE_CRTC_X] = { .name = "CRTC_X", }, + [WDRM_PLANE_CRTC_Y] = { .name = "CRTC_Y", }, + [WDRM_PLANE_CRTC_W] = { .name = "CRTC_W", }, + [WDRM_PLANE_CRTC_H] = { .name = "CRTC_H", }, + [WDRM_PLANE_FB_ID] = { .name = "FB_ID", }, + [WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", }, }; plane = zalloc(sizeof(*plane) + @@ -3225,7 +3267,6 @@ create_sprites(struct drm_backend *b) drmModePlane *kplane; struct drm_plane *drm_plane; uint32_t i; - kplane_res = drmModeGetPlaneResources(b->drm.fd); if (!kplane_res) { weston_log("failed to get plane resources: %s\n", @@ -4284,6 +4325,7 @@ drm_output_destroy(struct weston_output *base) weston_outpu
[PATCH v14 01/41] compositor-drm: Add shutting_down flag
Does what it says on the box: is true when the compositor is shutting down. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 4 1 file changed, 4 insertions(+) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 3eda70f30..6ac9626d9 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -184,6 +184,8 @@ struct drm_backend { int32_t cursor_height; uint32_t pageflip_timeout; + + bool shutting_down; }; struct drm_mode { @@ -3605,6 +3607,8 @@ drm_destroy(struct weston_compositor *ec) wl_event_source_remove(b->udev_drm_source); wl_event_source_remove(b->drm_source); + b->shutting_down = true; + destroy_sprites(b); weston_compositor_shutdown(ec); -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 07/41] compositor-drm: Remove NULL checks in switch_mode
Calling switch_mode with no output or mode never makes any sense. Drop the NULL checks. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 23 +-- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 6ad616ebd..f3c04228f 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -2593,26 +2593,13 @@ drm_output_fini_pixman(struct drm_output *output); static int drm_output_switch_mode(struct weston_output *output_base, struct weston_mode *mode) { - struct drm_output *output; - struct drm_mode *drm_mode; - struct drm_backend *b; - - if (output_base == NULL) { - weston_log("output is NULL.\n"); - return -1; - } - - if (mode == NULL) { - weston_log("mode is NULL.\n"); - return -1; - } - - b = to_drm_backend(output_base->compositor); - output = to_drm_output(output_base); - drm_mode = choose_mode (output, mode); + struct drm_output *output = to_drm_output(output_base); + struct drm_backend *b = to_drm_backend(output_base->compositor); + struct drm_mode *drm_mode = choose_mode(output, mode); if (!drm_mode) { - weston_log("%s, invalid resolution:%dx%d\n", __func__, mode->width, mode->height); + weston_log("%s: invalid resolution %dx%d\n", + output_base->name, mode->width, mode->height); return -1; } -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 02/41] compositor-drm: Introduce drm_output_state structure
Currently this doesn't actually really do anything, but will be used in the future to track the state for both modeset and repaint requests. Completion of the request gives us a single request-completion path for both pageflip and vblank events. This merges the timing paths for scanout and plane-but-but-atomic-plane content. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 357 + 1 file changed, 300 insertions(+), 57 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 6ac9626d9..e123fe503 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -139,6 +139,22 @@ struct drm_property_info { struct drm_property_enum_info *enum_values; /**< array of enum values */ }; +/** + * Mode for drm_output_state_duplicate. + */ +enum drm_output_state_duplicate_mode { + DRM_OUTPUT_STATE_CLEAR_PLANES, /**< reset all planes to off */ + DRM_OUTPUT_STATE_PRESERVE_PLANES, /**< preserve plane state */ +}; + +/** + * Mode for drm_pending_state_apply and co. + */ +enum drm_state_apply_mode { + DRM_STATE_APPLY_SYNC, /**< state fully processed */ + DRM_STATE_APPLY_ASYNC, /**< state pending event delivery */ +}; + struct drm_backend { struct weston_backend base; struct weston_compositor *compositor; @@ -235,6 +251,23 @@ struct drm_edid { */ struct drm_pending_state { struct drm_backend *backend; + struct wl_list output_list; +}; + +/* + * Output state holds the dynamic state for one Weston output, i.e. a KMS CRTC, + * plus >= 1 each of encoder/connector/plane. Since everything but the planes + * is currently statically assigned per-output, we mainly use this to track + * plane state. + * + * pending_state is set when the output state is owned by a pending_state, + * i.e. when it is being constructed and has not yet been applied. When the + * output state has been applied, the owning pending_state is freed. + */ +struct drm_output_state { + struct drm_pending_state *pending_state; + struct drm_output *output; + struct wl_list link; }; /** @@ -328,6 +361,12 @@ struct drm_output { * repaint is flushed. */ struct drm_fb *fb_pending; + /* The last state submitted to the kernel for this CRTC. */ + struct drm_output_state *state_cur; + /* The previously-submitted state, where the hardware has not +* yet acknowledged completion of state_cur. */ + struct drm_output_state *state_last; + struct drm_fb *dumb[2]; pixman_image_t *image[2]; int current_image; @@ -602,6 +641,9 @@ drm_output_set_cursor(struct drm_output *output); static void drm_output_update_msc(struct drm_output *output, unsigned int seq); +static void +drm_output_destroy(struct weston_output *output_base); + static int drm_plane_crtc_supported(struct drm_output *output, struct drm_plane *plane) { @@ -902,11 +944,69 @@ drm_fb_unref(struct drm_fb *fb) } } -static int -drm_view_transform_supported(struct weston_view *ev) +/** + * Allocate a new, empty drm_output_state. This should not generally be used + * in the repaint cycle; see drm_output_state_duplicate. + */ +static struct drm_output_state * +drm_output_state_alloc(struct drm_output *output, + struct drm_pending_state *pending_state) { - return !ev->transform.enabled || - (ev->transform.matrix.type < WESTON_MATRIX_TRANSFORM_ROTATE); + struct drm_output_state *state = zalloc(sizeof(*state)); + + assert(state); + state->output = output; + state->pending_state = pending_state; + if (pending_state) + wl_list_insert(&pending_state->output_list, &state->link); + else + wl_list_init(&state->link); + + return state; +} + +/** + * Duplicate an existing drm_output_state into a new one. This is generally + * used during the repaint cycle, to capture the existing state of an output + * and modify it to create a new state to be used. + * + * The mode determines whether the output will be reset to an a blank state, + * or an exact mirror of the current state. + */ +static struct drm_output_state * +drm_output_state_duplicate(struct drm_output_state *src, + struct drm_pending_state *pending_state, + enum drm_output_state_duplicate_mode plane_mode) +{ + struct drm_output_state *dst = malloc(sizeof(*dst)); + + assert(dst); + + /* Copy the whole structure, then individually modify the +* pending_state, as well as the list link into our pending +* state. */ + *dst = *src; + + dst->pending_state = pending_state; + if (pending_state) + wl_list_insert(&pending_state->output_list, &dst->link); + else + wl_list_init(&dst->link)
[PATCH v14 00/41] Atomic modesetting
Hi, Please find attached another iteration of the atomic series. This round has mostly seen cleanups and correctness fixes through 03-11. The biggest change is around output disables, which necessitated the introduction of patch 1, and now ensures a relatively complicated disable/destroy/shutdown ordering of plane vs. output disabling. Patch 11 has now also grown to include a synchronous state application mode, which is used by output disabling. I didn't want to bulk up patch 11, and spent some time trying to pull various bits of it out so they could be introduced in earlier, smaller, patches. The most promising line was, patch by patch: * split state application out of repaint * add a (mostly unused) DPMS enum to output state * add drm_output_get_disable_state and drm_pending_state_apply_sync and use these from output disable * move all DPMS handling to output state at the same time as moving state application to repaint flush At the end of it, it was still quite fiddly and I felt it made the intermediate patches more difficult to review; it seemed more straightforward to just read patch 11 with the above in mind. But if anyone feels strongly about it, I'm happy to re-try the split along those lines. I did test hotplug and hot-unplug, which managed to tease out the plane_state fixes now seen in patch 06. But beyond that, it seemed entirely stable with whatever I threw at it. Hopefully this means we're actually good to go by now. Hope you all have a great Christmas and New Year. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 21/41] [XXX] compositor-drm: Only disallow scaling for overlay planes
Now we have a more comprehensive overview of the transform we're going to apply, use this to explicitly disallow scaling and rotation. XXX: This does not actually disallow rotation for square surfaces. We would have to build up a full buffer->view->output transform matrix, and then analyse that. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 001eb5278..10bc53ba0 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -2684,7 +2684,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 = &ev->surface->buffer_viewport; struct drm_backend *b = to_drm_backend(ec); struct wl_resource *buffer_resource; struct drm_plane *p; @@ -2710,13 +2709,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; @@ -2740,6 +2732,12 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, if (!state) return NULL; + 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) + goto err; + if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource))) { #ifdef HAVE_GBM_FD_IMPORT /* XXX: TODO: @@ -2769,7 +2767,7 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, if (dmabuf->attributes.n_planes != 1 || dmabuf->attributes.offset[0] != 0 || dmabuf->attributes.flags) - return NULL; + goto err; bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_FD, &gbm_dmabuf, GBM_BO_USE_SCANOUT); @@ -2785,8 +2783,12 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, state->fb = drm_fb_get_from_bo(bo, b, drm_view_is_opaque(ev), BUFFER_CLIENT); - if (!state->fb) + if (!state->fb) { + /* Destroy the BO as we've allocated it, but it won't yet +* be deallocated by the state. */ + gbm_bo_destroy(bo); goto err; + } /* Check whether the format is supported */ for (i = 0; i < p->count_formats; i++) @@ -2797,15 +2799,10 @@ 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 &p->base; err: drm_plane_state_put_back(state); - if (bo) - gbm_bo_destroy(bo); return NULL; } -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 15/41] compositor-drm: Add blob_id member to drm_mode
For atomic modesetting support, the mode is identified by a blob property ID, rather than being passed inline. Add a blob_id member to drm_mode to handle this, including refactoring mode destruction into a helper function. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen --- libweston/compositor-drm.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 694e307bf..2a5bc2d1c 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -232,6 +232,7 @@ struct drm_backend { struct drm_mode { struct weston_mode base; drmModeModeInfo mode_info; + uint32_t blob_id; }; enum drm_fb_type { @@ -3347,6 +3348,7 @@ drm_output_add_mode(struct drm_output *output, const drmModeModeInfo *info) mode->base.refresh = refresh; mode->mode_info = *info; + mode->blob_id = 0; if (info->type & DRM_MODE_TYPE_PREFERRED) mode->base.flags |= WL_OUTPUT_MODE_PREFERRED; @@ -3356,6 +3358,18 @@ drm_output_add_mode(struct drm_output *output, const drmModeModeInfo *info) return mode; } +/** + * Destroys a mode, and removes it from the list. + */ +static void +drm_output_destroy_mode(struct drm_backend *backend, struct drm_mode *mode) +{ + if (mode->blob_id) + drmModeDestroyPropertyBlob(backend->drm.fd, mode->blob_id); + wl_list_remove(&mode->base.link); + free(mode); +} + static int drm_subpixel_to_wayland(int drm_value) { @@ -4314,10 +4328,8 @@ drm_output_destroy(struct weston_output *base) } wl_list_for_each_safe(drm_mode, next, &output->base.mode_list, - base.link) { - wl_list_remove(&drm_mode->base.link); - free(drm_mode); - } + base.link) + drm_output_destroy_mode(b, drm_mode); if (output->pageflip_timer) wl_event_source_remove(output->pageflip_timer); -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 16/41] compositor-drm: Add to_drm_mode helper
Much like we already have to_drm_output and to_drm_backend. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen --- libweston/compositor-drm.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 2a5bc2d1c..cb8f00e95 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -483,6 +483,12 @@ drm_output_pageflip_timer_create(struct drm_output *output) return 0; } +static inline struct drm_mode * +to_drm_mode(struct weston_mode *base) +{ + return container_of(base, struct drm_mode, base); +} + /** * Get the current value of a KMS property * @@ -1829,7 +1835,7 @@ drm_output_apply_state(struct drm_output_state *state) assert(scanout_state->dest_w == scanout_state->src_w >> 16); assert(scanout_state->dest_h == scanout_state->src_h >> 16); - mode = container_of(output->base.current_mode, struct drm_mode, base); + mode = to_drm_mode(output->base.current_mode); if (backend->state_invalid || !scanout_plane->state_cur->fb || scanout_plane->state_cur->fb->stride != scanout_state->fb->stride) { ret = drmModeSetCrtc(backend->drm.fd, output->crtc_id, @@ -2785,7 +2791,7 @@ choose_mode (struct drm_output *output, struct weston_mode *target_mode) output->base.current_mode->height == target_mode->height && (output->base.current_mode->refresh == target_mode->refresh || target_mode->refresh == 0)) - return (struct drm_mode *)output->base.current_mode; + return to_drm_mode(output->base.current_mode); wl_list_for_each(mode, &output->base.mode_list, base.link) { if (mode->mode_info.hdisplay == target_mode->width && -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 18/41] 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. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 111 + 1 file changed, 41 insertions(+), 70 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index d5955c9ca..d61be4f1b 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -946,7 +946,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 }; @@ -969,16 +969,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 || @@ -989,13 +992,14 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, ret = -1; - if (format && !backend->no_addfb2) { + if (!backend->no_addfb2) { handles[0] = fb->handle; pitches[0] = fb->stride; offsets[0] = 0; ret = drmModeAddFB2(backend->drm.fd, fb->width, fb->height, - format, handles, pitches, offsets, + fb->format->format, + handles, pitches, offsets, &fb->fb_id, 0); if (ret) { weston_log("addfb2 failed: %m\n"); @@ -1523,34 +1527,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); + /* 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(&r, 0, 0, + ev->surface->width, + ev->surface->height); + pixman_region32_subtract(&r, &r, &ev->surface->opaque); - 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(&r, 0, 0, - output->base.width, - output->base.height); - pixman_region32_subtract(&r, &r, &es->opaque); + if (!pixman_region32_not_empty(&r)) + ret = true; - if (!pixman_region32_not_empty(&r)) - format = GBM_FORMAT_XRGB; - - pixman_region32_fini(&r); - } + pixman_region32_fini(&r); - if (output->gbm_format == format) - return format; - - return 0; + return ret; } static struct weston_plane * @@ -1564,7 +1560,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 = &ev->surface->buffer_viewport; struct gbm_bo *bo; - uint32_t format; /* Don't import buffers which span multiple outputs. */ if (ev->output_mask !=
[PATCH v14 27/41] 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 --- libweston/compositor-drm.c | 93 -- 1 file changed, 48 insertions(+), 45 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index e6b5efba0..09fa10f5f 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -288,7 +288,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; @@ -821,7 +824,7 @@ drm_fb_destroy_dumb(struct drm_fb *fb) munmap(fb->map, fb->size); memset(&destroy_arg, 0, sizeof(destroy_arg)); - destroy_arg.handle = fb->handle; + destroy_arg.handle = fb->handles[0]; drmIoctl(fb->fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_arg); drm_fb_destroy(fb); @@ -837,6 +840,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->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->fb_id); + return ret; +} + static struct drm_fb * drm_fb_create_dumb(struct drm_backend *b, int width, int height, uint32_t format) @@ -847,12 +876,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); @@ -878,32 +905,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; - ret = -1; - - 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->fb_id, 0); - if (ret) { - ret = drmModeAddFB(b->drm.fd, width, height, - fb->format->depth, fb->format->bpp, - fb->stride, fb->handle, &fb->fb_id); - } - - if (ret) + if (drm_fb_addfb(fb) != 0) { + weston_log("failed to create kms fb: %m\n"); goto err_bo; + } memset(&map_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, &map_arg); if (ret) goto err_add_fb; @@ -938,8 +953,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); @@ -956,10 +969,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(
[PATCH v14 39/41] 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 --- 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 be5aab4e4..cb1c23b03 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3529,6 +3529,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; } @@ -5766,17 +5777,6 @@ drm_backend_create(struct weston_compositor *compositor, wl_array_init(&b->unused_crtcs); wl_array_init(&b->unused_connectors); - /* -* 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.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 05/41] compositor-drm: Use drm_plane for cursor plane
Change the type of cursor_plane from a weston_plane (base tracking structure) to a drm_plane (wrapper containing additional DRM-specific details), and make it a dynamically-allocated pointer. Using the standard drm_plane allows us to reuse code which already deals with drm_planes, e.g. a common cleanup function. This patch introduces a 'special plane' helper, creating a drm_plane either from a real KMS plane when using universal planes, or a fake plane otherwise. Without universal planes, the cursor and primary planes are hidden from us; this helper allows us to pretend otherwise. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 463 +++-- 1 file changed, 358 insertions(+), 105 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 403438398..a600ef40b 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -355,7 +355,7 @@ struct drm_output { int disable_pending; struct drm_fb *gbm_cursor_fb[2]; - struct weston_plane cursor_plane; + struct drm_plane *cursor_plane; struct weston_view *cursor_view; int current_cursor; @@ -649,7 +649,7 @@ drm_property_info_free(struct drm_property_info *info, int num_props) } static void -drm_output_set_cursor(struct drm_output *output); +drm_output_set_cursor(struct drm_output_state *output_state); static void drm_output_update_msc(struct drm_output *output, unsigned int seq); @@ -1091,12 +1091,11 @@ drm_plane_state_put_back(struct drm_plane_state *state) } /** - * Return a plane state from a drm_output_state, either existing or - * freshly allocated. + * Return a plane state from a drm_output_state. */ static struct drm_plane_state * -drm_output_state_get_plane(struct drm_output_state *state_output, - struct drm_plane *plane) +drm_output_state_get_existing_plane(struct drm_output_state *state_output, + struct drm_plane *plane) { struct drm_plane_state *ps; @@ -1105,6 +1104,23 @@ drm_output_state_get_plane(struct drm_output_state *state_output, return ps; } + return NULL; +} + +/** + * Return a plane state from a drm_output_state, either existing or + * freshly allocated. + */ +static struct drm_plane_state * +drm_output_state_get_plane(struct drm_output_state *state_output, + struct drm_plane *plane) +{ + struct drm_plane_state *ps; + + ps = drm_output_state_get_existing_plane(state_output, plane); + if (ps) + return ps; + return drm_plane_state_alloc(state_output, plane); } @@ -1631,8 +1647,8 @@ drm_output_repaint(struct weston_output *output_base, */ if (output->base.disable_planes) { output->cursor_view = NULL; - output->cursor_plane.x = INT32_MIN; - output->cursor_plane.y = INT32_MIN; + output->cursor_plane->base.x = INT32_MIN; + output->cursor_plane->base.y = INT32_MIN; } drm_output_render(state, damage); @@ -1673,7 +1689,7 @@ drm_output_repaint(struct weston_output *output_base, wl_event_source_timer_update(output->pageflip_timer, backend->pageflip_timeout); - drm_output_set_cursor(output); + drm_output_set_cursor(state); /* * Now, update all the sprite surfaces @@ -2185,20 +2201,66 @@ err: return NULL; } +/** + * 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 + */ +static void +cursor_bo_update(struct drm_backend *b, struct gbm_bo *bo, +struct weston_view *ev) +{ + struct weston_buffer *buffer = ev->surface->buffer_ref.buffer; + uint32_t buf[b->cursor_width * b->cursor_height]; + int32_t stride; + uint8_t *s; + int i; + + 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++) + memcpy(buf + i * b->cursor_width, + s + i * stride, + ev->surface->width * 4); + wl_shm_buffer_end_access(buffer->shm_buffer); + + if (gbm_bo_write(bo, buf, sizeof buf) < 0) + weston_log("fai
[PATCH v14 37/41] 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 --- libweston/compositor-drm.c | 176 +++-- 1 file changed, 137 insertions(+), 39 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 8709ecff3..340e27cb5 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -214,6 +214,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 { @@ -1646,6 +1647,7 @@ drm_pending_state_get_output(struct drm_pending_state *pending_state, static int drm_pending_state_apply(struct drm_pending_state *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 @@ -1773,12 +1775,15 @@ 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; + int ret; fb = drm_fb_get_from_view(output_state, ev); if (!fb) @@ -1791,11 +1796,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(&state_old->link); + } else if (state->fb) { drm_fb_unref(fb); return NULL; } @@ -1814,10 +1826,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(&output_state->plane_list, &state_old->link); + state_old->output_state = output_state; + } return NULL; } @@ -1892,7 +1918,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) && @@ -1915,6 +1943,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; @@ -2424,9 +2453,16 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state, case DRM_STATE_APPLY_ASYNC:
[PATCH v14 13/41] compositor-drm: Don't restore original CRTC mode
When leaving Weston, don't attempt to restore the previous CRTC settings. The framebuffer may well have disappeared, and in every likelihood, whoever gets the KMS device afterwards will be repainting anyway. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen --- libweston/compositor-drm.c | 32 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index f4ca6901d..71e93c1c2 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -343,7 +343,6 @@ struct drm_output { uint32_t crtc_id; /* object ID to pass to DRM functions */ int pipe; /* index of CRTC in resource array / bitmasks */ uint32_t connector_id; - drmModeCrtcPtr original_crtc; struct drm_edid edid; /* Holds the properties for the connector */ @@ -1695,8 +1694,6 @@ drm_output_set_gamma(struct weston_output *output_base, /* check */ if (output_base->gamma_size != size) return; - if (!output->original_crtc) - return; rc = drmModeCrtcSetGamma(backend->drm.fd, output->crtc_id, @@ -1820,7 +1817,7 @@ drm_output_apply_state(struct drm_output_state *state) weston_log("set mode failed: %m\n"); goto err; } -} + } if (drmModePageFlip(backend->drm.fd, output->crtc_id, scanout_state->fb->fb_id, @@ -4176,8 +4173,6 @@ drm_output_enable(struct weston_output *base) output->base.assign_planes = drm_assign_planes; output->base.set_dpms = drm_set_dpms; output->base.switch_mode = drm_output_switch_mode; - - output->base.gamma_size = output->original_crtc->gamma_size; output->base.set_gamma = drm_output_set_gamma; if (output->cursor_plane) @@ -4247,9 +4242,8 @@ static void drm_output_destroy(struct weston_output *base) { struct drm_output *output = to_drm_output(base); - struct drm_backend *b = to_drm_backend(base->compositor); + struct drm_backend *b = to_drm_backend(output->base.compositor); struct drm_mode *drm_mode, *next; - drmModeCrtcPtr origcrtc = output->original_crtc; if (output->page_flip_pending || output->vblank_pending) { output->destroy_pending = 1; @@ -4284,14 +4278,6 @@ drm_output_destroy(struct weston_output *base) free(drm_mode); } - if (origcrtc) { - /* Restore original CRTC state */ - drmModeSetCrtc(b->drm.fd, origcrtc->crtc_id, origcrtc->buffer_id, - origcrtc->x, origcrtc->y, - &output->connector_id, 1, &origcrtc->mode); - drmModeFreeCrtc(origcrtc); - } - if (output->pageflip_timer) wl_event_source_remove(output->pageflip_timer); @@ -4405,6 +4391,7 @@ create_output_for_connector(struct drm_backend *b, const char *make = "unknown"; const char *model = "unknown"; const char *serial_number = "unknown"; + drmModeCrtcPtr origcrtc; int i; static const struct drm_property_info connector_props[] = { @@ -4430,8 +4417,6 @@ create_output_for_connector(struct drm_backend *b, output->backlight = backlight_init(drm_device, connector->connector_type); - output->original_crtc = drmModeGetCrtc(b->drm.fd, output->crtc_id); - name = make_connector_name(connector); weston_output_init(&output->base, b->compositor, name); free(name); @@ -4440,6 +4425,13 @@ create_output_for_connector(struct drm_backend *b, output->base.destroy = drm_output_destroy; output->base.disable = drm_output_disable; + origcrtc = drmModeGetCrtc(b->drm.fd, output->crtc_id); + if (origcrtc == NULL) + goto err; + + output->base.gamma_size = origcrtc->gamma_size; + drmModeFreeCrtc(origcrtc); + output->destroy_pending = 0; output->disable_pending = 0; @@ -4458,6 +4450,8 @@ create_output_for_connector(struct drm_backend *b, output->base.serial_number = (char *)serial_number; output->base.subpixel = drm_subpixel_to_wayland(output->connector->subpixel); + drmModeFreeObjectProperties(props); + if (output->connector->connector_type == DRM_MODE_CONNECTOR_LVDS || output->connector->connector_type == DRM_MODE_CONNECTOR_eDP) output->base.connection_internal = true; @@ -4467,8 +4461,6 @@ create_output_for_connector(struct drm_backend *b, output->base.mm_width =
[PATCH v14 08/41] compositor-drm: Don't repaint if no damage
If we don't have any damage for the primary plane, then don't force a repaint; simply reuse the old buffer we already have. Signed-off-by: Daniel Stone Reviewed-by: Pekka Paalanen --- libweston/compositor-drm.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index f3c04228f..04d52f2c6 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1561,6 +1561,7 @@ drm_output_render(struct drm_output_state *state, pixman_region32_t *damage) struct drm_output *output = state->output; struct weston_compositor *c = output->base.compositor; struct drm_plane_state *scanout_state; + struct drm_plane *scanout_plane = output->scanout_plane; struct drm_backend *b = to_drm_backend(c); struct drm_fb *fb; @@ -1571,10 +1572,20 @@ drm_output_render(struct drm_output_state *state, pixman_region32_t *damage) if (scanout_state->fb) return; - if (b->use_pixman) + if (!pixman_region32_not_empty(damage) && + scanout_plane->state_cur->fb && + (scanout_plane->state_cur->fb->type == BUFFER_GBM_SURFACE || +scanout_plane->state_cur->fb->type == BUFFER_PIXMAN_DUMB) && + scanout_plane->state_cur->fb->width == + output->base.current_mode->width && + scanout_plane->state_cur->fb->height == + output->base.current_mode->height) { + fb = drm_fb_ref(scanout_plane->state_cur->fb); + } else if (b->use_pixman) { fb = drm_output_render_pixman(state, damage); - else + } else { fb = drm_output_render_gl(state, damage); + } if (!fb) { drm_plane_state_put_back(scanout_state); -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 25/41] 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 --- libweston/compositor-drm.c | 48 +- 1 file changed, 9 insertions(+), 39 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 195eef725..09318c98c 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1683,24 +1683,19 @@ 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; - - /* Don't import buffers which span multiple outputs. */ - if (ev->output_mask != (1u << output->base.id)) - return NULL; + struct drm_fb *fb; - /* We use GBM to import buffers. */ - if (b->gbm == NULL) + fb = drm_fb_get_from_view(output_state, ev); + if (!fb) return NULL; - if (buffer == NULL) - return NULL; - if (wl_shm_buffer_get(buffer->resource)) + /* 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) { @@ -1708,9 +1703,11 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, * 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; drm_plane_state_coords_for_view(state, ev); @@ -1723,33 +1720,6 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, state->dest_h != (unsigned) output->base.current_mode->height) goto err; - if (ev->alpha != 1.0f) - 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 &scanout_plane->base; err: -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 04/41] compositor-drm: Introduce drm_plane_is_available
Helper for the pattern of checking whether or not a plane can be used on an output during the current repaint cycle. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 6ea41ae80..403438398 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -657,9 +657,25 @@ drm_output_update_msc(struct drm_output *output, unsigned int seq); static void drm_output_destroy(struct weston_output *output_base); -static int -drm_plane_crtc_supported(struct drm_output *output, struct drm_plane *plane) +/** + * Returns true if the plane can be used on the given output for its current + * repaint cycle. + */ +static bool +drm_plane_is_available(struct drm_plane *plane, struct drm_output *output) { + assert(plane->state_cur); + + /* The plane still has a request not yet completed by the kernel. */ + if (!plane->state_cur->complete) + return false; + + /* The plane is still active on another output. */ + if (plane->state_cur->output && plane->state_cur->output != output) + return false; + + /* Check whether the plane can be used with this CRTC; possible_crtcs +* is a bitmask of CRTC indices (pipe), rather than CRTC object ID. */ return !!(plane->possible_crtcs & (1 << output->pipe)); } @@ -2024,12 +2040,7 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, if (p->type != WDRM_PLANE_TYPE_OVERLAY) continue; - if (!drm_plane_crtc_supported(output, p)) - continue; - - if (!p->state_cur->complete) - continue; - if (p->state_cur->output && p->state_cur->output != output) + if (!drm_plane_is_available(p, output)) continue; state = drm_output_state_get_plane(output_state, p); -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 12/41] compositor-drm: Use apply_state for starting repaint
Rather than open-coding it ourselves, use the new apply_state helper in drm_output_start_repaint. Signed-off-by: Daniel Stone Reported-by: Fabien DESSENNE Reviewed-by: Pekka Paalanen --- libweston/compositor-drm.c | 36 ++-- 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 2bb13f3a2..f4ca6901d 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -2026,13 +2026,10 @@ static void drm_output_start_repaint_loop(struct weston_output *output_base) { struct drm_output *output = to_drm_output(output_base); - struct drm_pending_state *pending_state = NULL; - struct drm_output_state *state; - struct drm_plane_state *plane_state; + struct drm_pending_state *pending_state; struct drm_plane *scanout_plane = output->scanout_plane; struct drm_backend *backend = to_drm_backend(output_base->compositor); - uint32_t fb_id; struct timespec ts, tnow; struct timespec vbl2now; int64_t refresh_nsec; @@ -2088,44 +2085,23 @@ drm_output_start_repaint_loop(struct weston_output *output_base) /* Immediate query didn't provide valid timestamp. * Use pageflip fallback. */ - fb_id = scanout_plane->state_cur->fb->fb_id; assert(!output->page_flip_pending); assert(!output->state_last); pending_state = drm_pending_state_alloc(backend); - state = drm_output_state_duplicate(output->state_cur, pending_state, - DRM_OUTPUT_STATE_PRESERVE_PLANES); + drm_output_state_duplicate(output->state_cur, pending_state, + DRM_OUTPUT_STATE_PRESERVE_PLANES); - if (drmModePageFlip(backend->drm.fd, output->crtc_id, fb_id, - DRM_MODE_PAGE_FLIP_EVENT, output) < 0) { - weston_log("queueing pageflip failed: %m\n"); + ret = drm_pending_state_apply(pending_state); + if (ret != 0) { + weston_log("applying repaint-start state failed: %m\n"); goto finish_frame; } - if (output->pageflip_timer) - wl_event_source_timer_update(output->pageflip_timer, -backend->pageflip_timeout); - - wl_list_for_each(plane_state, &state->plane_list, link) { - if (plane_state->plane->type != WDRM_PLANE_TYPE_OVERLAY) - continue; - - vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT; - vbl.request.type |= drm_waitvblank_pipe(output); - vbl.request.sequence = 1; - vbl.request.signal = (unsigned long) plane_state; - drmWaitVBlank(backend->drm.fd, &vbl); - } - - drm_output_assign_state(state, DRM_STATE_APPLY_ASYNC); - drm_pending_state_free(pending_state); - return; finish_frame: - drm_pending_state_free(pending_state); - /* if we cannot page-flip, immediately finish frame */ weston_output_finish_frame(output_base, NULL, WP_PRESENTATION_FEEDBACK_INVALID); -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 41/41] compositor-drm: Support modifiers with GBM
Use the extended GBM allocator interface to support modifiers and multi-planar BOs. Signed-off-by: Daniel Stone --- configure.ac | 3 +++ libweston/compositor-drm.c | 57 ++ 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index bc6fefc1e..9bad4e25a 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 ba2217fc0..65e24cbb2 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -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", @@ -4346,13 +4359,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) { + /* XXX: better error message */ + weston_log("can't use format for output\n"); + 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, + format[0], + plane->formats[i].modifiers, + plane->formats[i].count_modifiers); + } else +#endif + { + output->gbm_surface = + gbm_surface_create(b->gbm, mode->width, mode->height, + format[0], + GBM_BO_USE_RENDERING | GBM_BO_USE_SCANOUT); + } - 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_surfa
[PATCH v14 17/41] compositor-drm: Atomic modesetting support
Add support for using the atomic-modesetting API to apply output state. Unlike previous series, this commit does not unflip sprites_are_broken, until further work has been done with assign_planes to make it reliable. Signed-off-by: Daniel Stone Co-authored-by: Pekka Paalanen Co-authored-by: Louis-Francis Ratté-Boulianne Co-authored-by: Derek Foreman --- configure.ac | 2 +- libweston/compositor-drm.c | 504 ++--- 2 files changed, 431 insertions(+), 75 deletions(-) diff --git a/configure.ac b/configure.ac index 72b5d6f23..ba9247773 100644 --- a/configure.ac +++ b/configure.ac @@ -206,7 +206,7 @@ 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_ATOMIC, [libdrm >= 2.4.62], + 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], diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index cb8f00e95..d5955c9ca 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -83,6 +83,35 @@ #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64 #endif +/** + * Represents the values of an enum-type KMS property + */ +struct drm_property_enum_info { + const char *name; /**< name as string (static, not freed) */ + bool valid; /**< true if value is supported; ignore if false */ + uint64_t value; /**< raw value */ +}; + +/** + * Holds information on a DRM property, including its ID and the enum + * values it holds. + * + * DRM properties are allocated dynamically, and maintained as DRM objects + * within the normal object ID space; they thus do not have a stable ID + * to refer to. This includes enum values, which must be referred to by + * integer values, but these are not stable. + * + * drm_property_info allows a cache to be maintained where Weston can use + * enum values internally to refer to properties, with the mapping to DRM + * ID values being maintained internally. + */ +struct drm_property_info { + const char *name; /**< name as string (static, not freed) */ + uint32_t prop_id; /**< KMS property object ID */ + unsigned int num_enum_values; /**< number of enum values */ + struct drm_property_enum_info *enum_values; /**< array of enum values */ +}; + /** * List of properties attached to DRM planes */ @@ -111,6 +140,36 @@ enum wdrm_plane_type { WDRM_PLANE_TYPE__COUNT }; +static struct drm_property_enum_info plane_type_enums[] = { + [WDRM_PLANE_TYPE_PRIMARY] = { + .name = "Primary", + }, + [WDRM_PLANE_TYPE_OVERLAY] = { + .name = "Overlay", + }, + [WDRM_PLANE_TYPE_CURSOR] = { + .name = "Cursor", + }, +}; + +static const struct drm_property_info plane_props[] = { + [WDRM_PLANE_TYPE] = { + .name = "type", + .enum_values = plane_type_enums, + .num_enum_values = WDRM_PLANE_TYPE__COUNT, + }, + [WDRM_PLANE_SRC_X] = { .name = "SRC_X", }, + [WDRM_PLANE_SRC_Y] = { .name = "SRC_Y", }, + [WDRM_PLANE_SRC_W] = { .name = "SRC_W", }, + [WDRM_PLANE_SRC_H] = { .name = "SRC_H", }, + [WDRM_PLANE_CRTC_X] = { .name = "CRTC_X", }, + [WDRM_PLANE_CRTC_Y] = { .name = "CRTC_Y", }, + [WDRM_PLANE_CRTC_W] = { .name = "CRTC_W", }, + [WDRM_PLANE_CRTC_H] = { .name = "CRTC_H", }, + [WDRM_PLANE_FB_ID] = { .name = "FB_ID", }, + [WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", }, +}; + /** * List of properties attached to a DRM connector */ @@ -121,33 +180,10 @@ enum wdrm_connector_property { WDRM_CONNECTOR__COUNT }; -/** - * Represents the values of an enum-type KMS property - */ -struct drm_property_enum_info { - const char *name; /**< name as string (static, not freed) */ - bool valid; /**< true if value is supported; ignore if false */ - uint64_t value; /**< raw value */ -}; - -/** - * Holds information on a DRM property, including its ID and the enum - * values it holds. - * - * DRM properties are allocated dynamically, and maintained as DRM objects - * within the normal object ID space; they thus do not have a stable ID - * to refer to. This includes enum values, which must be referred to by - * integer values, but these are not stable. - * - * drm_property_info allows a cache to be maintained
[PATCH v14 32/41] 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 --- 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 3ff06c01c..049c80932 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -357,6 +357,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; @@ -1798,6 +1800,7 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, } state->fb = fb; + state->ev = ev; state->output = output; drm_plane_state_coords_for_view(state, ev); @@ -2847,6 +2850,7 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, } state->fb = fb; + state->ev = ev; state->output = output; drm_plane_state_coords_for_view(state, ev); @@ -2983,6 +2987,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]); @@ -3050,17 +3055,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 = &output_base->compositor->primary_plane; assert(!output->state_last); state = drm_output_state_duplicate(output->state_cur, @@ -3081,35 +3084,21 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) * as we do for flipping full screen surfaces. */ pixman_region32_init(&renderer_region); - primary = &output_base->compositor->primary_plane; wl_list_for_each(ev, &output_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(&surface_overlap); pixman_region32_intersect(&surface_overlap, &renderer_region, &ev->transform.boundingbox); - next_plane = NULL; if (pixman_region32_not_empty(&surface_overlap)) next_plane = primary; + pixman_region32_fini(&surface_overlap); + if (next_plane == NULL) next_plane = drm_output_prepare_cursor_view(state, ev); @@ -3122,17 +3111,70 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) if (next_plane == NULL) next_plane = prim
[PATCH v14 36/41] 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 --- libweston/compositor-drm.c | 68 ++ 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index ff70f9c29..8709ecff3 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1771,7 +1771,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) { @@ -1814,7 +1814,7 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, state->dest_h != (unsigned) output->base.current_mode->height) goto err; - return &scanout_plane->base; + return state; err: drm_plane_state_put_back(state); @@ -1988,8 +1988,8 @@ drm_output_apply_state_legacy(struct drm_output_state *state) &output->props_conn[WDRM_CONNECTOR_DPMS]; struct drm_plane_state *scanout_state; struct drm_plane_state *ps; - struct drm_plane *p; struct drm_mode *mode; + struct drm_plane *p; struct timespec now; int ret = 0; @@ -2801,7 +2801,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) { @@ -2859,7 +2859,7 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state, state->src_h != state->dest_h << 16) goto err; - return &p->base; + return state; err: drm_plane_state_put_back(state); @@ -2903,7 +2903,7 @@ cursor_bo_update(struct drm_backend *b, struct gbm_bo *bo, 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) { @@ -2992,7 +2992,7 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state, if (needs_update) cursor_bo_update(b, plane_state->fb->bo, ev); - return &plane->base; + return plane_state; err: drm_plane_state_put_back(plane_state); @@ -3062,7 +3062,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 = &output_base->compositor->primary_plane; bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY); bool planes_ok = !b->sprites_are_broken; @@ -3088,7 +3087,8 @@ drm_output_propose_state(struct weston_output *output_base, pixman_region32_init(&occluded_region); wl_list_for_each(ev, &output_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; @@ -3100,7 +3100,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(&clipped_view); @@ -3124,40 +3124,48 @@ drm_output_propose_state(struct weston_output *output_base, pixman_region32_intersect(&surface_overlap, &renderer_region, &clipped_view); if (pixman_region32_not_empty(&surface_overlap)) - next_plane = primary; + force_renderer = true; pixman_region32_fini(&surface_overlap); + if (force_renderer && !renderer_ok) { + pixman_region32_fini(&clipped_view); + goto err; + } + /* 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 ==
[PATCH v14 09/41] compositor-drm: Track unused connectors and CRTCs
Rather than a more piecemeal approach at backend creation, explicitly track connectors and CRTCs we do not intend to use, so we can ensure they are disabled where appropriate. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 86 ++ 1 file changed, 86 insertions(+) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 04d52f2c6..9c7564383 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -188,6 +188,9 @@ struct drm_backend { void *repaint_data; + struct wl_array unused_connectors; + struct wl_array unused_crtcs; + int cursors_are_broken; bool universal_planes; @@ -386,6 +389,26 @@ static struct gl_renderer_interface *gl_renderer; static const char default_seat[] = "seat0"; +static void +wl_array_remove_uint32(struct wl_array *array, uint32_t elm) +{ + uint32_t *pos, *end; + + end = (uint32_t *) ((char *) array->data + array->size); + + wl_array_for_each(pos, array) { + if (*pos != elm) + continue; + + array->size -= sizeof(*pos); + if (pos + 1 == end) + break; + + memmove(pos, pos + 1, (char *) end - (char *) (pos + 1)); + break; + } +} + static inline struct drm_output * to_drm_output(struct weston_output *base) { @@ -1327,6 +1350,7 @@ drm_output_assign_state(struct drm_output_state *state, enum drm_state_apply_mode mode) { struct drm_output *output = state->output; + struct drm_backend *b = to_drm_backend(output->base.compositor); struct drm_plane_state *plane_state; assert(!output->state_last); @@ -1363,6 +1387,12 @@ drm_output_assign_state(struct drm_output_state *state, else if (plane->type == WDRM_PLANE_TYPE_PRIMARY) output->page_flip_pending = 1; } + + if (output->dpms == WESTON_DPMS_ON) { + wl_array_remove_uint32(&b->unused_connectors, + output->connector_id); + wl_array_remove_uint32(&b->unused_crtcs, output->crtc_id); + } } @@ -3983,6 +4013,7 @@ drm_output_deinit(struct weston_output *base) { struct drm_output *output = to_drm_output(base); struct drm_backend *b = to_drm_backend(base->compositor); + uint32_t *unused; if (b->use_pixman) drm_output_fini_pixman(output); @@ -4004,6 +4035,11 @@ drm_output_deinit(struct weston_output *base) drmModeSetCursor(b->drm.fd, output->crtc_id, 0, 0, 0); } } + + unused = wl_array_add(&b->unused_connectors, sizeof(*unused)); + *unused = output->connector_id; + unused = wl_array_add(&b->unused_crtcs, sizeof(*unused)); + *unused = output->crtc_id; } static void @@ -4099,6 +4135,47 @@ drm_output_disable(struct weston_output *base) return 0; } +/** + * Update the list of unused connectors and CRTCs + * + * This keeps the unused_connectors and unused_crtcs arrays up to date. + * + * @param b Weston backend structure + * @param resources DRM resources for this device + */ +static void +drm_backend_update_unused_outputs(struct drm_backend *b, drmModeRes *resources) +{ + int i; + + wl_array_release(&b->unused_connectors); + wl_array_init(&b->unused_connectors); + + for (i = 0; i < resources->count_connectors; i++) { + uint32_t *connector_id; + + if (drm_output_find_by_connector(b, resources->connectors[i])) + continue; + + connector_id = wl_array_add(&b->unused_connectors, + sizeof(*connector_id)); + *connector_id = resources->connectors[i]; + } + + wl_array_release(&b->unused_crtcs); + wl_array_init(&b->unused_crtcs); + + for (i = 0; i < resources->count_crtcs; i++) { + uint32_t *crtc_id; + + if (drm_output_find_by_crtc(b, resources->crtcs[i])) + continue; + + crtc_id = wl_array_add(&b->unused_crtcs, sizeof(*crtc_id)); + *crtc_id = resources->crtcs[i]; + } +} + /** * Create a Weston output structure * @@ -4259,6 +4336,8 @@ create_outputs(struct drm_backend *b, struct udev_device *drm_device) } } + drm_backend_update_unused_outputs(b, resources); + if (wl_list_empty(&b->compositor->output_list) && wl_list_empty(&b->compositor->pending_output_list)) weston_log("No currently active connector found.\n"); @@ -4350,6 +4429,8 @@ update_outputs(struct
[PATCH v14 06/41] compositor-drm: Use drm_plane for scanout plane
Use a real drm_plane to back the scanout plane, displacing output->fb_{last,cur,pending} to their plane-tracked equivalents. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 222 ++--- 1 file changed, 149 insertions(+), 73 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index a600ef40b..6ad616ebd 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -362,17 +362,8 @@ struct drm_output { struct gbm_surface *gbm_surface; uint32_t gbm_format; - /* Plane for a fullscreen direct scanout view */ - struct weston_plane scanout_plane; - - /* The last framebuffer submitted to the kernel for this CRTC. */ - struct drm_fb *fb_current; - /* The previously-submitted framebuffer, where the hardware has not -* yet acknowledged display of fb_current. */ - struct drm_fb *fb_last; - /* Framebuffer we are going to submit to the kernel when the current -* repaint is flushed. */ - struct drm_fb *fb_pending; + /* Plane being displayed directly on the CRTC */ + struct drm_plane *scanout_plane; /* The last state submitted to the kernel for this CRTC. */ struct drm_output_state *state_cur; @@ -1369,6 +1360,8 @@ drm_output_assign_state(struct drm_output_state *state, if (plane->type == WDRM_PLANE_TYPE_OVERLAY) output->vblank_pending++; + else if (plane->type == WDRM_PLANE_TYPE_PRIMARY) + output->page_flip_pending = 1; } } @@ -1416,6 +1409,8 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, { 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 weston_buffer_viewport *viewport = &ev->surface->buffer_viewport; struct gbm_bo *bo; @@ -1456,6 +1451,15 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, if (ev->alpha != 1.0f) 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. */ + return NULL; + } + bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER, buffer->resource, GBM_BO_USE_SCANOUT); @@ -1465,19 +1469,33 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, format = drm_output_check_scanout_format(output, ev->surface, bo); if (format == 0) { + drm_plane_state_put_back(state); gbm_bo_destroy(bo); return NULL; } - output->fb_pending = drm_fb_get_from_bo(bo, b, format, BUFFER_CLIENT); - if (!output->fb_pending) { + state->fb = drm_fb_get_from_bo(bo, b, format, BUFFER_CLIENT); + if (!state->fb) { + drm_plane_state_put_back(state); gbm_bo_destroy(bo); return NULL; } - drm_fb_set_buffer(output->fb_pending, buffer); + drm_fb_set_buffer(state->fb, buffer); - return &output->scanout_plane; + 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; + state->dest_w = output->base.current_mode->width; + state->dest_h = output->base.current_mode->height; + + return &scanout_plane->base; } static struct drm_fb * @@ -1542,12 +1560,15 @@ drm_output_render(struct drm_output_state *state, pixman_region32_t *damage) { struct drm_output *output = state->output; struct weston_compositor *c = output->base.compositor; + struct drm_plane_state *scanout_state; struct drm_backend *b = to_drm_backend(c); struct drm_fb *fb; /* If we already have a client buffer promoted to scanout, then we don't * want to render. */ - if (output->fb_pending) + scanout_state = drm_output_state_get_plane(state, + output->scanout_plane); + if (scanout_state->fb) return; if (b->use_pixman) @@ -1555,9 +1576,24 @@ drm_output
[PATCH v14 22/41] compositor-drm: Use plane_state_coords_for_view for scanout
Use the shiny new helper we have for working through scanout as well. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 81 ++ 1 file changed, 25 insertions(+), 56 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 10bc53ba0..d97efd761 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1575,13 +1575,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) { @@ -1613,7 +1606,6 @@ 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 = &ev->surface->buffer_viewport; struct gbm_bo *bo; /* Don't import buffers which span multiple outputs. */ @@ -1629,28 +1621,6 @@ 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)) - return NULL; - - if (ev->alpha != 1.0f) - 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, @@ -1660,44 +1630,50 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, return NULL; } + state->output = output; + drm_plane_state_coords_for_view(state, ev); + + /* The legacy API does not let us perform cropping or scaling. */ + if (state->src_x != 0 || state->src_y != 0 || + state->src_w != state->dest_w << 16 || + state->src_h != state->dest_h << 16 || + state->dest_x != 0 || state->dest_y != 0 || + state->dest_w != (unsigned) output->base.current_mode->width || + state->dest_h != (unsigned) output->base.current_mode->height) + goto err; + + if (ev->alpha != 1.0f) + 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; - state->dest_w = output->base.current_mode->width; - state->dest_h = output->base.current_mode->height; - return &scanout_plane->base; + +err: + drm_plane_state_put_back(state); + return NULL; } static struct drm_fb * @@ -3006,7 +2982,6 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) struct weston_view *ev, *n
[PATCH v14 24/41] 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 --- libweston/compositor-drm.c | 221 - 1 file changed, 119 insertions(+), 102 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index dbe53513b..195eef725 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1234,6 +1234,109 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state, state->src_h = wl_fixed_from_double(syf2 - syf1) << 8; } +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(&r, 0, 0, + ev->surface->width, + ev->surface->height); + pixman_region32_subtract(&r, &r, &ev->surface->opaque); + + if (!pixman_region32_not_empty(&r)) + ret = true; + + pixman_region32_fini(&r); + + 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 (!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, &gbm_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. */ @@ -1575,28 +1678,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(&r, 0, 0, - ev->surface->width, - ev->surface->height); - pixman_region32_subtract(&r, &r, &ev->surface->
[PATCH v14 19/41] 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 --- libweston/compositor-drm.c | 157 + 1 file changed, 88 insertions(+), 69 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index d61be4f1b..71bf94edd 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1179,6 +1179,92 @@ 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 = &ev->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(&ev->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(&dest_rect); + pixman_region32_intersect(&dest_rect, &ev->transform.boundingbox, + &output->base.region); + pixman_region32_translate(&dest_rect, -output->base.x, -output->base.y); + box = pixman_region32_extents(&dest_rect); + tbox = weston_transformed_rect(output->base.width, + output->base.height, + output->base.transform, + output->base.current_scale, + *box); + 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(&dest_rect); + + /* Now calculate the source rectangle, by finding the points in the +* view and working backwards to source co-ordinates. */ + pixman_region32_init(&src_rect); + pixman_region32_intersect(&src_rect, &ev->transform.boundingbox, + &output->base.region); + box = pixman_region32_extents(&src_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), + &sx1, &sy1); + weston_view_from_global_fixed(ev, + wl_fixed_from_int(box->x2), + wl_fixed_from_int(box->y2), + &sx2, &sy2); + + /* XXX: How is this possible ... ? */ + 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; + + /* 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); + + 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; + pixman_region32_fini(&src_rect); +} + /** * Return a plane state from a drm_output_state. */ @@ -2629,16 +2715,13 @@ 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 drm_backend *b = to_drm_backend(ec); struct weston_buffer_viewport *viewport =
[PATCH v14 11/41] compositor-drm: Move repaint state application to flush
Split repaint into two stages, as implied by the grouped-repaint interface: drm_output_repaint generates the repaint state only, and drm_repaint_flush applies it. This also moves DPMS into output state. Previously, the usual way to DPMS off was that repaint would be called and apply its state, followed by set_dpms being called afterwards to push the DPMS state separately. As this happens before the repaint_flush hook, with no change to DPMS we would set DPMS off, then immediately re-enable the output by posting the repaint. Not ideal. Moving DPMS application at the same time complicates this patch, but I couldn't find a way to split it; if we keep set_dpms before begin_flush then we break DPMS off, or if we try to move DPMS to output state before using the repaint flush, we get stuck as the repaint hook generates an asynchronous state update, followed immediately by set_dpms generating a synchronous state update. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 366 - 1 file changed, 292 insertions(+), 74 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 87c2603c7..2bb13f3a2 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -272,6 +272,7 @@ struct drm_output_state { struct drm_pending_state *pending_state; struct drm_output *output; struct wl_list link; + enum dpms_enum dpms; struct wl_list plane_list; }; @@ -348,13 +349,13 @@ struct drm_output { /* Holds the properties for the connector */ struct drm_property_info props_conn[WDRM_CONNECTOR__COUNT]; - enum dpms_enum dpms; struct backlight *backlight; int vblank_pending; int page_flip_pending; int destroy_pending; int disable_pending; + int dpms_off_pending; struct drm_fb *gbm_cursor_fb[2]; struct drm_plane *cursor_plane; @@ -1149,6 +1150,7 @@ drm_output_state_alloc(struct drm_output *output, assert(state); state->output = output; + state->dpms = WESTON_DPMS_OFF; state->pending_state = pending_state; if (pending_state) wl_list_insert(&pending_state->output_list, &state->link); @@ -1225,6 +1227,30 @@ drm_output_state_free(struct drm_output_state *state) free(state); } +/** + * Get output state to disable output + * + * Returns a pointer to an output_state object which can be used to disable + * an output (e.g. DPMS off). + * + * @param pending_state The pending state object owning this update + * @param output The output to disable + * @returns A drm_output_state to disable the output + */ +static struct drm_output_state * +drm_output_get_disable_state(struct drm_pending_state *pending_state, +struct drm_output *output) +{ + struct drm_output_state *output_state; + + output_state = drm_output_state_duplicate(output->state_cur, + pending_state, + DRM_OUTPUT_STATE_CLEAR_PLANES); + output_state->dpms = WESTON_DPMS_OFF; + + return output_state; +} + /** * Allocate a new drm_pending_state * @@ -1297,6 +1323,9 @@ drm_pending_state_get_output(struct drm_pending_state *pending_state, return NULL; } +static int drm_pending_state_apply(struct drm_pending_state *state); +static int drm_pending_state_apply_sync(struct drm_pending_state *state); + /** * Mark a drm_output_state (the output's last state) as complete. This handles * any post-completion actions such as updating the repaint timer, disabling the @@ -1306,6 +1335,7 @@ static void drm_output_update_complete(struct drm_output *output, uint32_t flags, unsigned int sec, unsigned int usec) { + struct drm_backend *b = to_drm_backend(output->base.compositor); struct drm_plane_state *ps; struct timespec ts; @@ -1325,6 +1355,21 @@ drm_output_update_complete(struct drm_output *output, uint32_t flags, } else if (output->disable_pending) { weston_output_disable(&output->base); output->disable_pending = 0; + output->dpms_off_pending = 0; + return; + } else if (output->dpms_off_pending) { + struct drm_pending_state *pending = drm_pending_state_alloc(b); + drm_output_get_disable_state(pending, output); + drm_pending_state_apply(pending); + output->dpms_off_pending = 0; + return; + } else if (output->state_cur->dpms == WESTON_DPMS_OFF && + output->base.repaint_status != REPAINT_AWAITING_COMPLETION) { + /* DPMS can happen to us either in the middle of a repaint +* cycle (when we have painted fresh content, only to
[PATCH v14 40/41] 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 Signed-off-by: Daniel Stone --- configure.ac | 3 ++ libweston/compositor-drm.c | 128 + 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 8d0d6582a..bc6fefc1e 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 cb1c23b03..ba2217fc0 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -83,6 +83,20 @@ #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64 #endif +#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 + /** * Represents the values of an enum-type KMS property */ @@ -127,6 +141,7 @@ enum wdrm_plane_property { WDRM_PLANE_CRTC_H, WDRM_PLANE_FB_ID, WDRM_PLANE_CRTC_ID, + WDRM_PLANE_IN_FORMATS, WDRM_PLANE__COUNT }; @@ -168,6 +183,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" }, }; /** @@ -403,7 +419,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_output { @@ -2879,7 +2899,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) @@ -3635,6 +3667,61 @@ init_pixman(struct drm_backend *b) return pixman_renderer_init(b->compositor); } +/** + * Populates the formats array, and the modifiers of each format for a drm_plane. + */ +#ifdef HAVE_DRM_FORMATS_BLOB +static bool +populate_format_modifiers(struct drm_plane *plane, const drmModePlane *kplane, + uint32_t blob_id) +{ + unsigned i, j; + drmModePropertyBlobRes *blob; + struct drm_format_modifier_blob *fmt_mod_blob; + uint32_t *blob_formats; + struct drm_format_modifier *blob_modifiers; + + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id); + if (!blob) + return false; + + fmt_mod_blob = blob->data; + blob_formats = formats_ptr(fmt_mod_blob); + blob_modifiers = modifiers_ptr(fmt_mod_blob); + + assert(plane->count_formats == fmt_mod_blob->count_formats); + + for (i = 0; i < fmt_mod_blob->coun
[PATCH v14 33/41] 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 --- 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 049c80932..845f43e5b 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1389,10 +1389,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; @@ -2925,10 +2921,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; @@ -3088,6 +3080,16 @@ drm_output_propose_state(struct weston_output *output_base, wl_list_for_each(ev, &output_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. */ @@ -3137,6 +3139,11 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) wl_list_for_each(ev, &output_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.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 31/41] compositor-drm: Rename region variable
Make it a bit more clear what the purpose of the variable is. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 19aeb5326..3ff06c01c 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3059,7 +3059,7 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) struct drm_output_state *state; struct drm_plane_state *plane_state; struct weston_view *ev; - pixman_region32_t overlap, surface_overlap; + pixman_region32_t surface_overlap, renderer_region; struct weston_plane *primary, *next_plane; assert(!output->state_last); @@ -3080,7 +3080,7 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) * the client buffer can be used directly for the sprite surface * as we do for flipping full screen surfaces. */ - pixman_region32_init(&overlap); + pixman_region32_init(&renderer_region); primary = &output_base->compositor->primary_plane; wl_list_for_each(ev, &output_base->compositor->view_list, link) { @@ -3104,7 +3104,7 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) es->keep_buffer = false; pixman_region32_init(&surface_overlap); - pixman_region32_intersect(&surface_overlap, &overlap, + pixman_region32_intersect(&surface_overlap, &renderer_region, &ev->transform.boundingbox); next_plane = NULL; @@ -3125,7 +3125,8 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) weston_view_move_to_plane(ev, next_plane); if (next_plane == primary) - pixman_region32_union(&overlap, &overlap, + pixman_region32_union(&renderer_region, + &renderer_region, &ev->transform.boundingbox); if (next_plane == primary || @@ -3142,7 +3143,7 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) pixman_region32_fini(&surface_overlap); } - pixman_region32_fini(&overlap); + pixman_region32_fini(&renderer_region); /* We rely on output->cursor_view being both an accurate reflection of * the cursor plane's state, but also being maintained across repaints -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 29/41] 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 --- configure.ac | 6 +- libweston/compositor-drm.c | 181 + 2 files changed, 137 insertions(+), 50 deletions(-) diff --git a/configure.ac b/configure.ac index 1f3cc28aa..8d0d6582a 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 713bbabdd..b030234e4 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -278,6 +278,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 */ @@ -971,6 +972,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, + &import_mod, + GBM_BO_USE_SCANOUT); + } else { + fb->bo = gbm_bo_import(backend->gbm, GBM_BO_IMPORT_FD, + &import_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_g
[PATCH v14 34/41] 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 --- 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 845f43e5b..71027a5ae 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3054,7 +3054,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 = &output_base->compositor->primary_plane; assert(!output->state_last); @@ -3076,9 +3076,12 @@ drm_output_propose_state(struct weston_output *output_base, * as we do for flipping full screen surfaces. */ pixman_region32_init(&renderer_region); + pixman_region32_init(&occluded_region); wl_list_for_each(ev, &output_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. */ @@ -3090,13 +3093,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(&clipped_view); + pixman_region32_intersect(&clipped_view, + &ev->transform.boundingbox, + &output->base.region); + + pixman_region32_init(&surface_overlap); + pixman_region32_subtract(&surface_overlap, &clipped_view, +&occluded_region); + occluded = !pixman_region32_not_empty(&surface_overlap); + if (occluded) { + pixman_region32_fini(&surface_overlap); + pixman_region32_fini(&clipped_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(&surface_overlap); pixman_region32_intersect(&surface_overlap, &renderer_region, - &ev->transform.boundingbox); - + &clipped_view); if (pixman_region32_not_empty(&surface_overlap)) next_plane = primary; pixman_region32_fini(&surface_overlap); @@ -3104,6 +3121,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); @@ -3116,9 +3136,16 @@ drm_output_propose_state(struct weston_output *output_base, if (next_plane == primary) pixman_region32_union(&renderer_region, &renderer_region, - &ev->transform.boundingbox); + &clipped_view); + else if (output->cursor_plane && +next_plane != &output->cursor_plane->base) + pixman_region32_union(&occluded_region, + &occluded_region, + &clipped_view); + pixman_region32_fini(&clipped_view); } pixman_region32_fini(&renderer_region); + pixman_region32_fini(&occluded_region); return state; } -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 10/41] compositor-drm: Disable unused CRTCs/connectors
If we have an unused CRTC or connector, explicitly disable it during the end of the repaint cycle, or when we get VT-switched back in. This commit moves state_invalid from an output property to a backend property, as the unused CRTCs or connectors are likely not tracked by drm_outputs. This matches the mechanics of later commits, where we move to a global repaint-flush hook, applying the state for all outputs in one go. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 35 +-- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 9c7564383..87c2603c7 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -188,6 +188,7 @@ struct drm_backend { void *repaint_data; + bool state_invalid; struct wl_array unused_connectors; struct wl_array unused_crtcs; @@ -350,8 +351,6 @@ struct drm_output { enum dpms_enum dpms; struct backlight *backlight; - bool state_invalid; - int vblank_pending; int page_flip_pending; int destroy_pending; @@ -1748,7 +1747,7 @@ drm_output_repaint(struct weston_output *output_base, assert(scanout_state->dest_h == scanout_state->src_h >> 16); mode = container_of(output->base.current_mode, struct drm_mode, base); - if (output->state_invalid || !scanout_plane->state_cur->fb || + if (backend->state_invalid || !scanout_plane->state_cur->fb || scanout_plane->state_cur->fb->stride != scanout_state->fb->stride) { ret = drmModeSetCrtc(backend->drm.fd, output->crtc_id, scanout_state->fb->fb_id, @@ -1760,9 +1759,7 @@ drm_output_repaint(struct weston_output *output_base, goto err; } output_base->set_dpms(output_base, WESTON_DPMS_ON); - - output->state_invalid = false; - } +} if (drmModePageFlip(backend->drm.fd, output->crtc_id, scanout_state->fb->fb_id, @@ -1869,7 +1866,7 @@ drm_output_start_repaint_loop(struct weston_output *output_base) /* Need to smash all state in from scratch; current timings might not * be what we want, page flip might not work, etc. */ - if (output->state_invalid) + if (backend->state_invalid) goto finish_frame; assert(scanout_plane->state_cur->output == output); @@ -2033,12 +2030,26 @@ drm_repaint_flush(struct weston_compositor *compositor, void *repaint_data) struct drm_backend *b = to_drm_backend(compositor); struct drm_pending_state *pending_state = repaint_data; struct drm_output_state *output_state, *tmp; + uint32_t *unused; + + if (b->state_invalid) { + /* If we need to reset all our state (e.g. because we've +* just started, or just been VT-switched in), explicitly +* disable all the CRTCs we aren't using. This also disables +* all connectors on these CRTCs, so we don't need to do that +* separately with the pre-atomic API. */ + wl_array_for_each(unused, &b->unused_crtcs) + drmModeSetCrtc(b->drm.fd, *unused, 0, 0, 0, NULL, 0, + NULL); + } wl_list_for_each_safe(output_state, tmp, &pending_state->output_list, link) { drm_output_assign_state(output_state, DRM_STATE_APPLY_ASYNC); } + b->state_invalid = false; + drm_pending_state_free(pending_state); b->repaint_data = NULL; } @@ -2659,7 +2670,7 @@ drm_output_switch_mode(struct weston_output *output_base, struct weston_mode *mo * sledgehammer modeswitch first, and only later showing new * content. */ - output->state_invalid = true; + b->state_invalid = true; if (b->use_pixman) { drm_output_fini_pixman(output); @@ -4000,8 +4011,6 @@ drm_output_enable(struct weston_output *base) output->connector->count_modes == 0 ? ", built-in" : ""); - output->state_invalid = true; - return 0; err: @@ -4516,10 +4525,7 @@ session_notify(struct wl_listener *listener, void *data) weston_log("activating session\n"); weston_compositor_wake(compositor); weston_compositor_damage_all(compositor); - - wl_list_for_each(output, &compositor->output_list, base.link) - output->state_invalid = true; - + b->state_invalid = true;
[PATCH v14 26/41] compositor-drm: Remove no_addfb2 handling
If AddFB2 ever fails for any reason, we fall back to legacy AddFB, which doesn't support the same swathe of formats, or multi-planar formats, or modifiers. This can happen with arbitrary client buffers, condemning us to the fallback forever more. Remove this, at the cost of an unnecessary ioctl for users on old kernels without AddFB2; unfortunately, we cannot detect the complete absence of the ioctl, as the return here is -EINVAL rather than -ENOTTY. Signed-off-by: Daniel Stone --- libweston/compositor-drm.c | 46 -- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 09318c98c..e6b5efba0 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -241,7 +241,6 @@ struct drm_backend { */ int min_width, max_width; int min_height, max_height; - int no_addfb2; struct wl_list plane_list; int sprites_are_broken; @@ -848,6 +847,7 @@ 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) @@ -887,23 +887,12 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int height, ret = -1; - if (!b->no_addfb2) { - uint32_t handles[4] = { 0 }, pitches[4] = { 0 }, offsets[4] = { 0 }; - - 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->fb_id, 0); - if (ret) { - weston_log("addfb2 failed: %m\n"); - b->no_addfb2 = 1; - } - } + 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->fb_id, 0); if (ret) { ret = drmModeAddFB(b->drm.fd, width, height, fb->format->depth, fb->format->bpp, @@ -990,24 +979,13 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, goto err_free; } - ret = -1; - - if (!backend->no_addfb2) { - handles[0] = fb->handle; - pitches[0] = fb->stride; - offsets[0] = 0; - - ret = drmModeAddFB2(backend->drm.fd, fb->width, fb->height, - fb->format->format, - handles, pitches, offsets, - &fb->fb_id, 0); - if (ret) { - weston_log("addfb2 failed: %m\n"); - backend->no_addfb2 = 1; - backend->sprites_are_broken = 1; - } - } + handles[0] = fb->handle; + pitches[0] = fb->stride; + offsets[0] = 0; + ret = drmModeAddFB2(backend->drm.fd, fb->width, fb->height, + fb->format->format, handles, pitches, offsets, + &fb->fb_id, 0); if (ret && fb->format->depth && fb->format->bpp) ret = drmModeAddFB(backend->drm.fd, fb->width, fb->height, fb->format->depth, fb->format->bpp, -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 35/41] 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 --- 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 71027a5ae..ff70f9c29 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1766,6 +1766,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) @@ -3049,13 +3054,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 = &output_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, @@ -3118,36 +3127,49 @@ drm_output_propose_state(struct weston_output *output_base, next_plane = primary; pixman_region32_fini(&surface_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(&renderer_region, &renderer_region, &clipped_view); - else if (output->cursor_plane && -next_plane != &output->cursor_plane->base) + } else if (output->cursor_plane && + next_plane != &output->cursor_plane->base) { pixman_region32_union(&occluded_region, &occluded_region, &clipped_view); + } pixman_region32_fini(&clipped_view); } pixman_region32_fini(&renderer_region); pixman_region32_fini(&occluded_region); return state; + +err: + pixman_region32_fini(&renderer_region); + pixman_region32_fini(&occluded_region); + drm_output_state_free(state); + return NULL; } static void @@ -3161,7 +3183,8 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) struct weston_view *ev; struct weston_plane *primary = &output_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, &output_base->compositor->view_list, link) { struct drm_plane *target_plane = NULL; -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 30/41] compositor-drm: Don't need safe view-list traversal
Nothing in this loop reorders views within the compositor's view_list. 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 b030234e4..19aeb5326 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -3058,7 +3058,7 @@ drm_assign_planes(struct weston_output *output_base, void *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, *next; + struct weston_view *ev; pixman_region32_t overlap, surface_overlap; struct weston_plane *primary, *next_plane; @@ -3083,7 +3083,7 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) pixman_region32_init(&overlap); primary = &output_base->compositor->primary_plane; - wl_list_for_each_safe(ev, next, &output_base->compositor->view_list, link) { + wl_list_for_each(ev, &output_base->compositor->view_list, link) { struct weston_surface *es = ev->surface; /* Test whether this buffer can ever go into a plane: -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 38/41] 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 --- libweston/compositor-drm.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 340e27cb5..be5aab4e4 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1779,6 +1779,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; @@ -1790,7 +1791,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; } @@ -1818,12 +1819,10 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state, drm_plane_state_coords_for_view(state, ev); /* The legacy API does not let us perform cropping or scaling. */ - if (state->src_x != 0 || state->src_y != 0 || - state->src_w != state->dest_w << 16 || - state->src_h != state->dest_h << 16 || - state->dest_x != 0 || state->dest_y != 0 || - state->dest_w != (unsigned) output->base.current_mode->width || - state->dest_h != (unsigned) output->base.current_mode->height) + if (!b->atomic_modeset && + (state->src_x != 0 || state->src_y != 0 || +state->src_w != state->dest_w << 16 || +state->src_h != state->dest_h << 16)) goto err; if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) { @@ -2895,8 +2894,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.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 20/41] 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 --- libweston/compositor-drm.c | 55 ++ 1 file changed, 12 insertions(+), 43 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index 71bf94edd..001eb5278 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -1188,10 +1188,9 @@ 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 = &ev->surface->buffer_viewport; 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(&ev->transform.boundingbox); @@ -1217,52 +1216,22 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state, state->dest_h = tbox.y2 - tbox.y1; pixman_region32_fini(&dest_rect); - /* Now calculate the source rectangle, by finding the points in the -* view and working backwards to source co-ordinates. */ + /* Now calculate the source rectangle, by finding the extents of the +* view, and working backwards to source co-ordinates. */ pixman_region32_init(&src_rect); pixman_region32_intersect(&src_rect, &ev->transform.boundingbox, &output->base.region); box = pixman_region32_extents(&src_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), - &sx1, &sy1); - weston_view_from_global_fixed(ev, - wl_fixed_from_int(box->x2), - wl_fixed_from_int(box->y2), - &sx2, &sy2); - - /* XXX: How is this possible ... ? */ - 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; - - /* 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); - - 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; + weston_view_from_global_float(ev, box->x1, box->y1, &sxf1, &syf1); + weston_surface_to_buffer_float(ev->surface, sxf1, syf1, &sxf1, &syf1); + weston_view_from_global_float(ev, box->x2, box->y2, &sxf2, &syf2); + weston_surface_to_buffer_float(ev->surface, sxf2, syf2, &sxf2, &syf2); pixman_region32_fini(&src_rect); + + 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; } /** -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 28/41] compositor-drm: Support modifiers for drm_fb
Use the new drmModeAddFB2WithModifiers interface to import buffers with modifiers. Signed-off-by: Daniel Stone --- configure.ac | 3 +++ libweston/compositor-drm.c | 26 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ba9247773..1f3cc28aa 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 09fa10f5f..713bbabdd 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -293,6 +293,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; @@ -843,7 +844,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; 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->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->fb_id, @@ -905,6 +927,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; @@ -972,6 +995,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.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v14 23/41] 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. Signed-off-by: Daniel Stone Reported-by: Derek Foreman --- libweston/compositor-drm.c | 48 +++--- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c index d97efd761..dbe53513b 100644 --- a/libweston/compositor-drm.c +++ b/libweston/compositor-drm.c @@ -2827,10 +2827,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 = &ev->surface->buffer_viewport; struct wl_shm_buffer *shmbuf; bool needs_update = false; - float x, y; if (!plane) return NULL; @@ -2860,16 +2858,6 @@ 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; @@ -2880,6 +2868,26 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state, 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; + drm_plane_state_coords_for_view(plane_state, ev); + 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; + + /* The cursor API is somewhat special: in cursor_bo_update(), we upload +* a buffer which is always cursor_width x cursor_height, even if the +* surface we want to promote is actually smaller than this. Manually +* mangle the plane state to deal with this. */ + plane_state->src_w = b->cursor_width << 16; + plane_state->src_h = b->cursor_height << 16; + plane_state->dest_w = b->cursor_width; + plane_state->dest_h = b->cursor_height; + /* 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 @@ -2896,26 +2904,18 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state, } output->cursor_view = ev; - weston_view_to_global_float(ev, 0, 0, &x, &y); - plane->base.x = x; - plane->base.y = y; plane_state->fb = drm_fb_ref(output->gbm_cursor_fb[output->current_cursor]); - plane_state->output = output; - plane_state->src_x = 0; - plane_state->src_y = 0; - plane_state->src_w = b->cursor_width << 16; - plane_state->src_h = b->cursor_height << 16; - plane_state->dest_x = (x - output->base.x) * output->base.current_scale; - plane_state->dest_y = (y - output->base.y) * output->base.current_scale; - plane_state->dest_w = b->cursor_width; - plane_state->dest_h = b->cursor_height; if (needs_update) cursor_bo_update(b, plane_state->fb->bo, ev); return &plane->base; + +err: + drm_plane_state_put_back(plane_state); + return NULL; } static void -- 2.14.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v14 00/41] Atomic modesetting
Hi, On 20 December 2017 at 12:26, Daniel Stone wrote: > Please find attached another iteration of the atomic series. This round > has mostly seen cleanups and correctness fixes through 03-11. In the festive rush, I forgot to mention that it can also be found here: https://gitlab.collabora.com/daniels/weston # wip/2017-12/atomic-v14 Note that this is different from the previous git://git.collabora.com/git/user/daniels/weston repository. It also contains two patches on top which you won't want unless testing: one adds hugely spammy debug (enough to cause noticeable frame drops when the log flushes), and the other makes weston-simple-egl use a viewport (which breaks core repaint in ways I've not got to the bottom of). Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v3 03/10] connection: Refactor out closure allocation
Hi Derek, On 6 December 2017 at 17:22, Derek Foreman wrote: > Moves the common/similar bits into one place. > > This has a minor functional change - count and message are now initialized > immediately, previously they'd only be set if (de)marshal was successful. I've reviewed this does what it says on the box, and the outcome is correct, so I'm going to land it. But as a drive-by, you could get rid of some of the local variables like 'count = closure->count', so it's a little more clear to the casual reader what's going on. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v3 04/10] connection: Clear fds we shouldn't close to -1
Hi Derek, On 6 December 2017 at 17:22, Derek Foreman wrote: > This initializes all the fd arguments in closures to -1 and clears > them back to -1 when they've been dispatched or serialized. > > This means that any valid fd in a closure is currently libwayland's > responsibility to close in the case of an error. As with the previous, I'm totally happy with this and intend to land, but a good follow-on might be to have wl_closure_{invoke,dispatch} take full ownership of the closure, i.e. unconditionally destroy it before return. All its users do this anyway. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v3 03/10] connection: Refactor out closure allocation
Hi, On 27 December 2017 at 14:08, Daniel Stone wrote: > On 6 December 2017 at 17:22, Derek Foreman wrote: >> Moves the common/similar bits into one place. >> >> This has a minor functional change - count and message are now initialized >> immediately, previously they'd only be set if (de)marshal was successful. > > I've reviewed this does what it says on the box, and the outcome is > correct, so I'm going to land it. But as a drive-by, you could get rid > of some of the local variables like 'count = closure->count', so it's > a little more clear to the casual reader what's going on. The other thing I forgot to mention here, is that you could move the wl_connection_consume bits up earlier, rather than doing it immediately before return in both success/failure branches. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v3 06/10] util: Pass flags to map iterators
Hi, On 6 December 2017 at 17:22, Derek Foreman wrote: > static enum wl_iterator_result > -destroy_resource(void *element, void *data) > +_destroy_resource(void *element, void *data, uint32_t flags) > { > struct wl_resource *resource = element; > - struct wl_client *client = resource->client; > - uint32_t flags; > > wl_signal_emit(&resource->deprecated_destroy_signal, resource); > /* Don't emit the new signal for deprecated resources, as that would > @@ -670,7 +668,6 @@ destroy_resource(void *element, void *data) > if (!resource_is_deprecated(resource)) > wl_priv_signal_emit(&resource->destroy_signal, resource); > > - flags = wl_map_lookup_flags(&client->objects, resource->object.id); > if (resource->destroy) > resource->destroy(resource); > > @@ -680,6 +677,18 @@ destroy_resource(void *element, void *data) > return WL_ITERATOR_CONTINUE; > } > > +static enum wl_iterator_result > +destroy_resource(void *element, void *data) > +{ > + struct wl_resource *resource = element; > + struct wl_client *client = resource->client; > + uint32_t flags; > + > + flags = wl_map_lookup_flags(&client->objects, resource->object.id); > + > + return _destroy_resource(element, data, flags); > +} I know I already gave R-b for this, but on closer inspection I spied there was only one destroy_resource() user at all, so I just inlined the flag lookup into it and binned the wrapper. Hope that's OK. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel