[PATCH 2/3] compositor-drm: Add switch_mode support.

[David Herrmann]

Hi

On Wed, Mar 7, 2012 at 10:01 AM,  <zhiwen.wu at linux.intel.com> wrote:
> From: Alex Wu <zhiwen.wu at linux.intel.com>
>
> Implement switch_mode hook for drm back end.
> ---
>  src/compositor-drm.c |  143 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 142 insertions(+), 1 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 1b0a9b9..e9583fe 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -120,6 +120,23 @@ struct drm_sprite {
>        uint32_t formats[];
>  };
>
> +static struct drm_mode *
> +choose_mode (struct drm_output *output, int32_t width, int32_t height, uint32_t refresh)
> +{
> +       struct drm_mode *tmp_mode = NULL, *mode, *next;
> +
> +       wl_list_for_each_safe(mode, next, &output->base.mode_list,base.link) {
> +               if (mode->mode_info.hdisplay == width && mode->mode_info.vdisplay == height) {
> +                       if (mode->mode_info.vrefresh == refresh)
> +                               return mode;
> +                       else if (!tmp_mode)
> +                               tmp_mode = mode;
> +               }
> +       }
> +
> +       return tmp_mode;
> +}
> +
>  static int
>  surface_is_primary(struct weston_compositor *ec, struct weston_surface *es)
>  {
> @@ -742,6 +759,130 @@ drm_output_destroy(struct weston_output *output_base)
>  }
>
>  static int
> +drm_output_switch_mode (struct weston_output *output_base, int32_t width, int32_t height, uint32_t refresh)
> +{
> +       struct drm_output *output;
> +       struct drm_mode *drm_mode;
> +       struct weston_mode *old_mode;
> +       int i, ret;
> +       unsigned handle, stride;
> +       struct drm_compositor *ec;
> +
> +       if (output_base == NULL) {
> +               fprintf(stderr, "output is NULL.\n");
> +               return -1;
> +       }
> +
> +       ec = (struct drm_compositor *)output_base->compositor;
> +       output = (struct drm_output *)output_base;
> +       old_mode  = output->base.current;
> +       drm_mode  = choose_mode (output, width, height, refresh);
> +
> +       if (!drm_mode) {
> +               printf("%s, invalid resolution:%dx%d\n", __func__, width, height);
> +               return -1;
> +       }
> +
> +       glFramebufferRenderbuffer(GL_FRAMEBUFFER,
> +                                 GL_COLOR_ATTACHMENT0,
> +                                 GL_RENDERBUFFER, 0);
> +
> +       /* Destroy output buffers */
> +       for (i = 0; i < 2; i++) {
> +               drmModeRmFB(ec->drm.fd, output->fb_id[i]);
> +               ec->base.destroy_image(ec->base.display, output->image[i]);
> +               gbm_bo_destroy(output->bo[i]);
> +       }

output->fb_id[0] = -1;
output->fb_id[1] = -1;
output->image[0] = NULL;
... see below ...

> +
> +       glBindRenderbuffer(GL_RENDERBUFFER, 0);
> +       glDeleteRenderbuffers(2, output->rbo);
> +       weston_output_destroy(&output->base);
> +
> +       output->base.current = &drm_mode->base;
> +       drm_mode->base.flags =
> +               WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED;
> +
> +       /* create new output buffers */
> +       glGenRenderbuffers(2, output->rbo);
> +       for (i = 0; i < 2; i++) {
> +               glBindRenderbuffer(GL_RENDERBUFFER, output->rbo[i]);
> +
> +               output->bo[i] =
> +               gbm_bo_create(ec->gbm,
> +                             output->base.current->width,
> +                             output->base.current->height,
> +                             GBM_BO_FORMAT_XRGB8888,
> +                             GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING);
> +
> +               if (!output->bo[i])
> +                       goto err_bufs;
> +
> +               output->image[i] = ec->base.create_image(ec->base.display,
> +                                                        NULL,
> +                                                        EGL_NATIVE_PIXMAP_KHR,
> +                                                        output->bo[i], NULL);
> +               if (!output->image[i])
> +                       goto err_bufs;
> +
> +               ec->base.image_target_renderbuffer_storage(GL_RENDERBUFFER,
> +                                                          output->image[i]);
> +               stride = gbm_bo_get_pitch(output->bo[i]);
> +               handle = gbm_bo_get_handle(output->bo[i]).u32;
> +
> +               ret = drmModeAddFB(ec->drm.fd,
> +                                  output->base.current->width,
> +                                  output->base.current->height,
> +                                  24, 32, stride, handle, &output->fb_id[i]);
> +
> +               if (ret) {
> +                       fprintf(stderr, "failed to add fb %d: %m\n", i);
> +                       goto err_bufs;
> +               }
> +       }
> +
> +       output->current = 0;
> +       glFramebufferRenderbuffer(GL_FRAMEBUFFER,
> +                                 GL_COLOR_ATTACHMENT0,
> +                                 GL_RENDERBUFFER,
> +                                 output->rbo[output->current]);
> +
> +       ret = drmModeSetCrtc(ec->drm.fd,
> +                            output->crtc_id,
> +                            output->fb_id[output->current ^ 1], 0, 0,
> +                            &output->connector_id, 1, &drm_mode->mode_info);
> +
> +       if (ret) {
> +               fprintf(stderr, "failed to set mode\n");
> +               goto err_fb;
> +       }
> +
> +       output->base.dirty = 1;
> +       weston_output_move(&output->base, output->base.x, output->base.y);
> +
> +       return 0;
> +
> +err_fb:
> +       glFramebufferRenderbuffer(GL_FRAMEBUFFER,
> +                                 GL_COLOR_ATTACHMENT0,
> +                                 GL_RENDERBUFFER, 0);
> +err_bufs:
> +       for (i = 0; i < 2; i++) {
> +               if (output->fb_id[i] != -1)
> +                       drmModeRmFB(ec->drm.fd, output->fb_id[i]);

This doesn't make sense if you do not reset them to -1 before:
See "create_output_for_connector".

> +               if (output->image[i])
> +                       ec->base.destroy_image(ec->base.display,
> +                                              output->image[i]);

Same here.

> +               if (output->bo[i])
> +                       gbm_bo_destroy(output->bo[i]);

And here.
Even if setting this failed and output->bo[0] is NULL, this doesn't
mean that output->bo[1] is NULL, too so reset them properly first.

> +       }
> +       glBindRenderbuffer(GL_RENDERBUFFER, 0);
> +       glDeleteRenderbuffers(2, output->rbo);
> +
> +       output->base.current = old_mode;
> +       return -1;

What happens to this output if this function fails? As I see it it
keeps it in the list but should rather remove it entirely from the
output-list so the next update_outputs() call will reenable this one
and not leave it disabled.

> +}
> +
> +static int
>  on_drm_input(int fd, uint32_t mask, void *data)
>  {
>        drmEventContext evctx;
> @@ -1140,7 +1281,7 @@ create_output_for_connector(struct drm_compositor *ec,
>        output->base.destroy = drm_output_destroy;
>        output->base.assign_planes = drm_assign_planes;
>        output->base.set_dpms = drm_set_dpms;
> -       output->base.switch_mode = NULL;
> +       output->base.switch_mode = drm_output_switch_mode;
>
>        return 0;
>
> --

Regards
David