Hi,

Le 12/02/2016 18:45, Benoit Gschwind a écrit :
Hello,

Le 09/02/2016 16:14, Quentin Glidic a écrit :
From: Quentin Glidic <[email protected]>

Signed-off-by: Quentin Glidic <[email protected]>
---
  Makefile.am                   | 10 ++---
  {src => lib}/compositor-drm.c | 87
+++++++++++++++++++------------------------
  {src => lib}/libbacklight.c   |  0
  {src => lib}/libbacklight.h   |  0
  lib/libweston.c               | 23 ++++++++++++
  lib/libweston.h               |  1 +
  src/main.c                    |  3 ++
  7 files changed, 71 insertions(+), 53 deletions(-)
  rename {src => lib}/compositor-drm.c (97%)
  rename {src => lib}/libbacklight.c (100%)
  rename {src => lib}/libbacklight.h (100%)

diff --git a/Makefile.am b/Makefile.am
index 36078ba..68997d8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -287,12 +287,12 @@ drm_backend_la_CFLAGS =                \
      $(DRM_COMPOSITOR_CFLAGS)        \
      $(AM_CFLAGS)
  drm_backend_la_SOURCES =            \
-    src/compositor-drm.c            \
+    lib/compositor-drm.c            \
      $(INPUT_BACKEND_SOURCES)        \
      shared/helpers.h            \
      shared/timespec-util.h            \
-    src/libbacklight.c            \
-    src/libbacklight.h
+    lib/libbacklight.c            \
+    lib/libbacklight.h

  if ENABLE_VAAPI_RECORDER
  drm_backend_la_SOURCES += src/vaapi-recorder.c src/vaapi-recorder.h
@@ -1325,8 +1325,8 @@ if BUILD_SETBACKLIGHT
  noinst_PROGRAMS += setbacklight
  setbacklight_SOURCES =                \
      tests/setbacklight.c            \
-    src/libbacklight.c            \
-    src/libbacklight.h
+    lib/libbacklight.c            \
+    lib/libbacklight.h
  setbacklight_CFLAGS = $(AM_CFLAGS) $(SETBACKLIGHT_CFLAGS)
  setbacklight_LDADD = $(SETBACKLIGHT_LIBS)
  endif
diff --git a/src/compositor-drm.c b/lib/compositor-drm.c
similarity index 97%
rename from src/compositor-drm.c
rename to lib/compositor-drm.c
index 04de95e..5d3cd0c 100644
--- a/src/compositor-drm.c
+++ b/lib/compositor-drm.c
@@ -46,7 +46,7 @@
  #include <gbm.h>
  #include <libudev.h>

-#include "libweston.h"
+#include "libweston-internal.h"
  #include "shared/helpers.h"
  #include "shared/timespec-util.h"
  #include "libbacklight.h"
@@ -88,6 +88,7 @@ enum output_config {

  struct drm_backend {
      struct weston_backend base;
+    struct libweston_context *context;
      struct weston_compositor *compositor;

      struct udev *udev;
@@ -2144,15 +2145,15 @@ setup_output_seat_constraint(struct
drm_backend *b,
  }

  static int
-get_gbm_format_from_section(struct weston_config_section *section,
-                uint32_t default_value,
-                uint32_t *format)
+get_gbm_format_from_config(struct libweston_context *context,
+               const char *section,
+               uint32_t default_value,
+               uint32_t *format)
  {
      char *s;
      int ret = 0;

-    weston_config_section_get_string(section,
-                     "gbm-format", &s, NULL);
+    s = context->backend_config.string_getter(section, "gbm-format",
NULL, context->backend_config.user_data);

      if (s == NULL)
          *format = default_value;
@@ -2296,7 +2297,7 @@ create_output_for_connector(struct drm_backend *b,
      struct drm_output *output;
      struct drm_mode *drm_mode, *next, *current;
      struct weston_mode *m;

[REF#1]

-    struct weston_config_section *section;
+    char section[39];
      drmModeModeInfo crtc_mode, modeline;
      int i, width, height, scale;
      char *s;
@@ -2320,9 +2321,9 @@ create_output_for_connector(struct drm_backend *b,
      output->base.serial_number = "unknown";
      wl_list_init(&output->base.mode_list);


Following a comment from Quentin Glidic on IRC, I read wrong the snprintf (I readed printf ... a shame, my apology). Which make the patch valid if output->base.name is not too long.


The _following changes_ seems wrong, in previous version, the function
weston_config_get_section will try to find a section with a 'name' key
and where the value of this 'name' equals to output->base.name. You
replaced it with b->context->backend_config.string_getter, where first
section is undefined see [REF#1]:
  - will probably always fallback to default values when not segfault,
  - even if section were fine, this approach does not handle severals
outputs.

But I keep the following:


This case show that this approach is not very good, In that case the
developer must give a list of unbound outputs setup and key/value is not
well suited for that case. (there is some workaround, like using key
pattern).

This case also leave me to add that section/key/value is not a good
choice and you should stick to key/value.

I still prefer the opaque configuration structure with function to set
the structure content.

Best regards.

-    section = weston_config_get_section(b->compositor->config,
"output", "name",
-                        output->base.name);
-    weston_config_section_get_string(section, "mode", &s, "preferred");
+    snprintf(section, sizeof section, "output %s", output->base.name);
+
+    s = b->context->backend_config.string_getter(section, "mode",
"preferred", b->context->backend_config.user_data);
      if (strcmp(s, "off") == 0)
          config = OUTPUT_CONFIG_OFF;
      else if (strcmp(s, "preferred") == 0)
@@ -2340,20 +2341,21 @@ create_output_for_connector(struct drm_backend
*b,
      }
      free(s);

-    weston_config_section_get_int(section, "scale", &scale, 1);
-    weston_config_section_get_string(section, "transform", &s,
"normal");
+    scale = b->context->backend_config.int_getter(section, "scale",
1, b->context->backend_config.user_data);
+    s = b->context->backend_config.string_getter(section,
"transform", "normal", b->context->backend_config.user_data);
      if (weston_parse_transform(s, &transform) < 0)
          weston_log("Invalid transform \"%s\" for output %s\n",
                 s, output->base.name);

      free(s);

-    if (get_gbm_format_from_section(section,
-                    b->format,
-                    &output->format) == -1)
+    if (get_gbm_format_from_config(b->context,
+                       section,
+                       b->format,
+                       &output->format) == -1)
          output->format = b->format;

-    weston_config_section_get_string(section, "seat", &s, "");
+    s = b->context->backend_config.string_getter(section, "seat", "",
b->context->backend_config.user_data);
      setup_output_seat_constraint(b, &output->base, s);
      free(s);

@@ -3058,16 +3060,16 @@ renderer_switch_binding(struct weston_keyboard
*keyboard, uint32_t time,
  }

  static struct drm_backend *
-drm_backend_create(struct weston_compositor *compositor,
-              struct drm_parameters *param,
-              int *argc, char *argv[],
-              struct weston_config *config)
+drm_backend_create(struct libweston_context *context)
  {
+    struct weston_compositor *compositor = context->compositor;
      struct drm_backend *b;
-    struct weston_config_section *section;
      struct udev_device *drm_device;
      struct wl_event_loop *loop;
      const char *path;
+    int tty;
+    uint32_t connector;
+    const char *seat_id = default_seat;

      weston_log("initializing drm backend\n");

@@ -3075,6 +3077,8 @@ drm_backend_create(struct weston_compositor
*compositor,
      if (b == NULL)
          return NULL;

+    b->context = context;
+
      /*
       * KMS support for hardware planes cannot properly synchronize
       * without nuclear page flip. Without nuclear/atomic, hw plane
@@ -3088,17 +3092,19 @@ drm_backend_create(struct weston_compositor
*compositor,
      b->sprites_are_broken = 1;
      b->compositor = compositor;

-    section = weston_config_get_section(config, "core", NULL, NULL);
-    if (get_gbm_format_from_section(section,
-                    GBM_FORMAT_XRGB8888,
-                    &b->format) == -1)
+    if (get_gbm_format_from_config(context,
+                       "core",
+                       GBM_FORMAT_XRGB8888,
+                       &b->format) == -1)
          goto err_base;

-    b->use_pixman = param->use_pixman;
+    b->use_pixman = context->backend_config.bool_getter(NULL,
"use-pixman", false, context->backend_config.user_data);
+    tty = context->backend_config.int_getter(NULL, "tty", 0,
context->backend_config.user_data);
+    connector = context->backend_config.int_getter(NULL, "connector",
0, context->backend_config.user_data);

      /* Check if we run drm-backend using weston-launch */
-    compositor->launcher = weston_launcher_connect(compositor,
param->tty,
-                               param->seat_id, true);
+    compositor->launcher = weston_launcher_connect(compositor, tty,
+                               seat_id, true);
      if (compositor->launcher == NULL) {
          weston_log("fatal: drm backend should be run "
                 "using weston-launch binary or as root\n");
@@ -3114,7 +3120,7 @@ drm_backend_create(struct weston_compositor
*compositor,
      b->session_listener.notify = session_notify;
      wl_signal_add(&compositor->session_signal, &b->session_listener);

-    drm_device = find_primary_gpu(b, param->seat_id);
+    drm_device = find_primary_gpu(b, seat_id);
      if (drm_device == NULL) {
          weston_log("no drm device found\n");
          goto err_udev;
@@ -3149,12 +3155,12 @@ drm_backend_create(struct weston_compositor
*compositor,
      create_sprites(b);

      if (udev_input_init(&b->input,
-                compositor, b->udev, param->seat_id) < 0) {
+                compositor, b->udev, seat_id) < 0) {
          weston_log("failed to create input devices\n");
          goto err_sprite;
      }

-    if (create_outputs(b, param->connector, drm_device) < 0) {
+    if (create_outputs(b, connector, drm_device) < 0) {
          weston_log("failed to create output for %s\n", path);
          goto err_udev_input;
      }
@@ -3235,26 +3241,11 @@ err_base:
  }

  WL_EXPORT int
-backend_init(struct weston_compositor *compositor, int *argc, char
*argv[],
-         struct weston_config *config,
-         struct weston_backend_config *config_base)
+libweston_backend_init(struct libweston_context *context)
  {
      struct drm_backend *b;
-    struct drm_parameters param = { 0, };
-
-    const struct weston_option drm_options[] = {
-        { WESTON_OPTION_INTEGER, "connector", 0, &param.connector },
-        { WESTON_OPTION_STRING, "seat", 0, &param.seat_id },
-        { WESTON_OPTION_INTEGER, "tty", 0, &param.tty },
-        { WESTON_OPTION_BOOLEAN, "current-mode", 0,
&option_current_mode },
-        { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &param.use_pixman },
-    };
-
-    param.seat_id = default_seat;
-
-    parse_options(drm_options, ARRAY_LENGTH(drm_options), argc, argv);

-    b = drm_backend_create(compositor, &param, argc, argv, config);
+    b = drm_backend_create(context);
      if (b == NULL)
          return -1;
      return 0;
diff --git a/src/libbacklight.c b/lib/libbacklight.c
similarity index 100%
rename from src/libbacklight.c
rename to lib/libbacklight.c
diff --git a/src/libbacklight.h b/lib/libbacklight.h
similarity index 100%
rename from src/libbacklight.h
rename to lib/libbacklight.h
diff --git a/lib/libweston.c b/lib/libweston.c
index da21f8e..dd77f6b 100644
--- a/lib/libweston.c
+++ b/lib/libweston.c
@@ -69,8 +69,31 @@ libweston_load_module(const char *name, const char
*entrypoint)
      return init;
  }

+static enum libweston_backend
+load_backend(struct libweston_context *context, const char *backend)
+{
+    int (*backend_init)(struct libweston_context *context);
+
+    backend_init = libweston_load_module(backend,
"libweston_backend_init");
+    if (!backend_init)
+        return -1;
+
+    return backend_init(context);
+
+}
+
  WL_EXPORT int
  libweston_load_backend(struct libweston_context *context, enum
libweston_backend preffered)
  {
+    if (preffered == LIBWESTON_BACKEND_NONE) {
+        preffered = LIBWESTON_BACKEND_DRM;
+    }
+
+    switch (preffered) {
+    case LIBWESTON_BACKEND_DRM:
+        return load_backend(context, "drm-backend.so");
+    case LIBWESTON_BACKEND_NONE:
+    break;
+    }
      return -1;
  }
diff --git a/lib/libweston.h b/lib/libweston.h
index 7a995b8..3950316 100644
--- a/lib/libweston.h
+++ b/lib/libweston.h
@@ -14,6 +14,7 @@ void *libweston_load_module(const char *name, const
char *entrypoint);

  enum libweston_backend {
      LIBWESTON_BACKEND_NONE = 0,
+    LIBWESTON_BACKEND_DRM,
  };
  int libweston_load_backend(struct libweston_context *context, enum
libweston_backend preffered);

diff --git a/src/main.c b/src/main.c
index 3e52168..c03835a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -660,6 +660,9 @@ load_backend(struct weston_compositor *compositor,
const char *backend,
  {
      enum libweston_backend libweston_backend = LIBWESTON_BACKEND_NONE;

+    if (strstr(backend, "drm"))
+        libweston_backend = LIBWESTON_BACKEND_DRM;
+
      if (libweston_load_backend(compositor->libweston,
libweston_backend) == 0)
          return 0;


_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to