On Tue, 28 Feb 2012 13:52:21 +0200
Tiago Vignatti <tiago.vigna...@intel.com> wrote:
> @@ -907,11 +912,76 @@ sprite_handle_pending_buffer_destroy(struct wl_listener 
> *listener,
>       sprite->pending_surface = NULL;
>  }
>  
> +static void
> +drm_set_backlight(struct weston_output *output_base, uint32_t up_down)
> +{
> +     struct drm_output *output = (struct drm_output *) output_base;
> +     long brightness;
> +
> +     if (!output->backlight)
> +             return;
> +
> +     /* I know, very very rudimentary logic for turning brighter/darker.
> +      * Maybe we should normalize brightness value to have a scale say from
> +      * 1 to 10 or something. */

So if it's just up & down, why not make an enum to make the code
clearer?  Then you don't need the last else clause too.


> +     brightness = backlight_get_brightness(output->backlight);
> +     if (up_down == 1)
> +             brightness++;
> +     else if (up_down == -1)
> +             brightness--;
> +     else
> +             return;
> +
> +     if (brightness > backlight_get_max_brightness(output->backlight) ||
> +         brightness < 0)
> +             return;
> +
> +     backlight_set_brightness(output->backlight, brightness);
> +     fprintf(stderr, "setting brightness to: %ld\n", brightness);

Debug output, can be dropped.

> +}
> +
> +static void
> +drm_set_dpms(struct weston_output *output_base, uint32_t state)
> +{
> +     struct drm_output *output = (struct drm_output *) output_base;
> +     struct weston_compositor *ec = output_base->compositor;
> +     struct drm_compositor *c = (struct drm_compositor *) ec;
> +     drmModeConnectorPtr connector;
> +     int i;
> +
> +     connector = drmModeGetConnector(c->drm.fd, output->connector_id);
> +     if (!connector)
> +             return;
> +
> +     if (connector->connection == DRM_MODE_DISCONNECTED)
> +             return;

Will this ever be the case?  Or will we have torn down the output
already?


> +
> +     for (i = 0; i < connector->count_props; i++) {
> +             drmModePropertyPtr props;
> +             props = drmModeGetProperty(c->drm.fd, connector->props[i]);
> +             if (!props)
> +                     continue;
> +
> +             if (!strcmp(props->name, "DPMS")) {
> +                     drmModeConnectorSetProperty(c->drm.fd,
> +                                     connector->connector_id,
> +                                     props->prop_id,
> +                                     state);
> +                     fprintf(stderr, "setting dpms state to: %d\n", state);
> +                     drmModeFreeProperty(props);
> +                     drmModeFreeConnector(connector);
> +                     return;
> +             }
> +             drmModeFreeProperty(props);
> +     }

Not sure if we modify properties elsewhere, but it might be good to
separate this out into a helper that finds a specific property and
returns it.

> +     output->backlight = backlight_init(drm_device,
> +                                        connector->connector_type);
> +     if (!output->backlight) {
> +             fprintf(stderr, "failed to initiate backlight control\n");
> +     }
> +

This will be a normal case for desktop configs, so it shouldn't be
treated like an error.

> +static void
> +weston_compositor_dpms_on(struct weston_compositor *compositor)
> +{
> +        struct weston_output *output;
> +
> +     /* TODO: we'd need a state machine to test whether dpms is on/off
> +      * before just going and setting it */
> +        wl_list_for_each(output, &compositor->output_list, link)
> +             if (output->set_dpms)
> +                     output->set_dpms(output, 0);
> +}
> +
>  WL_EXPORT void
>  weston_compositor_activity(struct weston_compositor *compositor)
>  {
>       if (compositor->state == WESTON_COMPOSITOR_ACTIVE) {
>               weston_compositor_wake(compositor);
>       } else {
> +             weston_compositor_dpms_on(compositor);
>               compositor->shell->unlock(compositor->shell);
>       }
>  }
> @@ -2482,6 +2495,7 @@ int main(int argc, char *argv[])
>               exit(EXIT_FAILURE);
>       }
>  
> +     weston_compositor_dpms_on(ec);


Yeah just some simple timer arm & reset calls here would do the trick I
think.

>       weston_compositor_wake(ec);
>       if (setjmp(segv_jmp_buf) == 0)
>               wl_display_run(display);
> diff --git a/src/compositor.h b/src/compositor.h
> index 881f53c..bb45bd1 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -86,6 +86,8 @@ struct weston_output {
>       void (*repaint)(struct weston_output *output);
>       void (*destroy)(struct weston_output *output);
>       void (*assign_planes)(struct weston_output *output);
> +     void (*set_backlight)(struct weston_output *output, uint32_t up_down);
> +     void (*set_dpms)(struct weston_output *output, uint32_t state);
>  };
>  
>  struct weston_input_device {
> diff --git a/src/shell.c b/src/shell.c
> index d949d0c..1510035 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -1311,10 +1311,16 @@ lock(struct weston_shell *base)
>       struct weston_surface *tmp;
>       struct weston_input_device *device;
>       struct shell_surface *shsurf;
> +     struct weston_output *output;
>       uint32_t time;
>  
> -     if (shell->locked)
> +     if (shell->locked) {
> +             /* set DPMS off for all outputs */
> +             wl_list_for_each(output, &shell->compositor->output_list, link)
> +                     if (output->set_dpms)
> +                             output->set_dpms(output, 1);
>               return;
> +     }

May as well use an enum for set_dpms too just to make it a little more
readable.

>  
>       shell->locked = true;
>  
> @@ -1833,6 +1839,29 @@ switcher_binding(struct wl_input_device *device, 
> uint32_t time,
>  }
>  
>  static void
> +backlight_binding(struct wl_input_device *device, uint32_t time,
> +               uint32_t key, uint32_t button, uint32_t state, void *data)
> +{
> +     struct weston_compositor *compositor = data;
> +     struct weston_output *output;
> +
> +     /* TODO: find a better way to get the desired output */
> +     output = get_default_output(compositor);
> +     if (!output)
> +             return;

Limiting ourselves to laptop/tablet/phone backlights is probably fine
here (i.e. assuming just one on the primary display).  We can extend
things later if we ever get support for setting backlights on random
desktop LCD panels.

> +
> +     if (!output->set_backlight)
> +             return;
> +
> +     /* darker */
> +     if (key == KEY_F9)
> +             output->set_backlight(output, -1);
> +     /* brighter */
> +     else if (key == KEY_F10)
> +             output->set_backlight(output, 1);
> +}
> +
> +static void
>  shell_destroy(struct weston_shell *base)
>  {
>       struct wl_shell *shell = container_of(base, struct wl_shell, shell);
> @@ -1904,9 +1933,12 @@ shell_init(struct weston_compositor *ec)
>       weston_compositor_add_binding(ec, 0, BTN_LEFT,
>                                     MODIFIER_SUPER | MODIFIER_ALT,
>                                     rotate_binding, NULL);
> -
>       weston_compositor_add_binding(ec, KEY_TAB, 0, MODIFIER_SUPER,
>                                     switcher_binding, ec);
> +     weston_compositor_add_binding(ec, KEY_F9, 0, MODIFIER_CTRL,
> +                                   backlight_binding, ec);
> +     weston_compositor_add_binding(ec, KEY_F10, 0, MODIFIER_CTRL,
> +                                   backlight_binding, ec);
>  
>       ec->shell = &shell->shell;

Isn't there an explicit brightness up/down code we can use in many
cases?  And fall back to f9/f10 otherwise?

-- 
Jesse Barnes, Intel Open Source Technology Center

Attachment: signature.asc
Description: PGP signature

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to