[PATCH weston v4 2/3] compositor: Implement idle inhibition
Derek Foreman
derekf at osg.samsung.com
Tue Aug 16 14:57:31 UTC 2016
On 15/08/16 09:15 PM, Bryce Harrington wrote:
> On Mon, Aug 15, 2016 at 07:01:24PM -0500, Derek Foreman wrote:
>> On 10/08/16 07:25 PM, Bryce Harrington wrote:
>>> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
>>> ---
>>> Makefile.am | 4 ++-
>>> configure.ac | 2 ++
>>> libweston/compositor.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 93 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index 32627f5..951f9ed 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -139,7 +139,9 @@ nodist_libweston_ at LIBWESTON_MAJOR@_la_SOURCES = \
>>> protocol/relative-pointer-unstable-v1-protocol.c \
>>> protocol/relative-pointer-unstable-v1-server-protocol.h \
>>> protocol/pointer-constraints-unstable-v1-protocol.c \
>>> - protocol/pointer-constraints-unstable-v1-server-protocol.h
>>> + protocol/pointer-constraints-unstable-v1-server-protocol.h \
>>> + protocol/idle-inhibit-unstable-v1-protocol.c \
>>> + protocol/idle-inhibit-unstable-v1-server-protocol.h
>>>
>>> BUILT_SOURCES += $(nodist_libweston_ at LIBWESTON_MAJOR@_la_SOURCES)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 74f931d..de26599 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -7,6 +7,8 @@ m4_define([libweston_major_version], [0])
>>> m4_define([libweston_minor_version], [0])
>>> m4_define([libweston_patch_version], [0])
>>>
>>> +# Note: Inhibition patchset requires inhibition protocol in wayland-protocol
>>> +
>>
>> That's landed in wayland-protocol now - can we just make sure we require
>> the right version of it instead of having a comment?
>
> Yeah it now requires 1.7 so no need for this comment.
>
>>> AC_PREREQ([2.64])
>>> AC_INIT([weston],
>>> [weston_version],
>>> diff --git a/libweston/compositor.c b/libweston/compositor.c
>>> index aa14504..e4ce273 100644
>>> --- a/libweston/compositor.c
>>> +++ b/libweston/compositor.c
>>> @@ -51,6 +51,8 @@
>>> #include <time.h>
>>> #include <errno.h>
>>>
>>> +#include <idle-inhibit-unstable-v1-server-protocol.h>
>>> +
>>> #include "timeline.h"
>>>
>>> #include "compositor.h"
>>> @@ -4665,6 +4667,88 @@ compositor_bind(struct wl_client *client,
>>> compositor, NULL);
>>> }
>>>
>>> +struct weston_idle_inhibitor {
>>> + struct weston_surface *surface;
>>> +};
>>> +
>>> +static void
>>> +destroy_idle_inhibitor(struct wl_resource *resource)
>>> +{
>>> + struct weston_idle_inhibitor *inhibitor = wl_resource_get_user_data(resource);
>>> +
>>> + inhibitor->surface = NULL;
>>> + free(inhibitor);
>>> +}
>>> +
>>> +static void
>>> +idle_inhibitor_destroy(struct wl_client *client, struct wl_resource *resource)
>>> +{
>>> + struct weston_idle_inhibitor *inhibitor = wl_resource_get_user_data(resource);
>>> +
>>> + assert(inhibitor);
>>> +
>>> + inhibitor->surface->inhibit_idling = false;
>>
>> Is there some mechanism in place that causes a re-assessment of screen
>> blanker state when this happens?
>>
>> ie) if I destroy my inhibition will the monitor the video player was on
>> immediately join the other monitors in sleep?
>
> Yep, that's right.
Ok, I saw the code path for going to sleep when the surface is
destroyed, but can't figure out how it gets put to sleep when the
inhibit is destroyed. Is that done when it's next considered for redraw
(what if it's not updating...)
Is there a test client? I realize this is a bit hard to test in make
check, so I'm not asking for that, but IMHO we can't land the series
without a weston-idle-inhibit client or something that can be trivially
used to check this all works manually.
>>> +}
>>> +
>>> +static const struct zwp_idle_inhibitor_v1_interface idle_inhibitor_interface = {
>>> + idle_inhibitor_destroy
>>> +};
>>> +
>>> +static void
>>> +idle_inhibit_manager_destroy(struct wl_client *client, struct wl_resource *resource)
>>> +{
>>> +}
>>> +
>>> +static void
>>> +idle_inhibit_manager_create_inhibitor(struct wl_client *client, struct wl_resource *resource,
>>> + uint32_t id, struct wl_resource *surface_resource)
>>> +{
>>> + struct weston_surface *surface = wl_resource_get_user_data(surface_resource);
>>> + struct weston_idle_inhibitor *inhibitor;
>>> + struct wl_resource *cr;
>>> +
>>> + cr = wl_resource_create(client, &zwp_idle_inhibitor_v1_interface,
>>> + wl_resource_get_version(resource), id);
>>> + if (cr == NULL) {
>>> + wl_client_post_no_memory(client);
>>> + return;
>>> + }
>>> +
>>> + inhibitor = zalloc(sizeof *inhibitor);
>>> + if (inhibitor == NULL) {
>>> + wl_client_post_no_memory(client);
>>> + return;
>>> + }
>>> +
>>> + inhibitor->surface = surface;
>>> + inhibitor->surface->inhibit_idling = true;
>>
>> Will there be an immediate wake up of the monitor if it was already off now?
>
> No, in that case the user would need to re-wake the screen and let it
> re-idle; the inhibition would take effect at that point.
I don't like that - I think it should wake the display if something
inhibits the screen blanker, even if it's already blank. (I'm thinking
some kind of critical notification or alarm could wake the display this
way...)
But I could probably be convinced either way, would love to hear what
others have to say about it...
Thanks,
Derek
> Bryce
>
>> Thanks,
>> Derek
>>
>>> +
>>> + wl_resource_set_implementation(cr, &idle_inhibitor_interface,
>>> + inhibitor, destroy_idle_inhibitor);
>>> +}
>>> +
>>> +static const struct zwp_idle_inhibit_manager_v1_interface idle_inhibit_manager_interface = {
>>> + idle_inhibit_manager_destroy,
>>> + idle_inhibit_manager_create_inhibitor
>>> +};
>>> +
>>> +static void
>>> +bind_idle_inhibit_manager(struct wl_client *client,
>>> + void *data, uint32_t version, uint32_t id)
>>> +{
>>> + struct wl_resource *resource;
>>> +
>>> + resource = wl_resource_create(client, &zwp_idle_inhibit_manager_v1_interface,
>>> + version, id);
>>> + if (resource == NULL) {
>>> + wl_client_post_no_memory(client);
>>> + return;
>>> + }
>>> +
>>> + wl_resource_set_implementation(resource, &idle_inhibit_manager_interface,
>>> + NULL, NULL);
>>> +}
>>> +
>>> WL_EXPORT int
>>> weston_environment_get_fd(const char *env)
>>> {
>>> @@ -4760,6 +4844,10 @@ weston_compositor_create(struct wl_display *display, void *user_data)
>>> if (weston_input_init(ec) != 0)
>>> goto fail;
>>>
>>> + if (!wl_global_create(ec->wl_display, &zwp_idle_inhibit_manager_v1_interface, 1,
>>> + ec, bind_idle_inhibit_manager))
>>> + goto fail;
>>> +
>>> wl_list_init(&ec->view_list);
>>> wl_list_init(&ec->plane_list);
>>> wl_list_init(&ec->layer_list);
>>>
More information about the wayland-devel
mailing list