Hi, On 9 December 2015 at 16:29, Giulio Camuffo <[email protected]> wrote: > +enum weston_drm_backend_output_mode { > + /** The output is disabled */ > + WESTON_DRM_BACKEND_OUTPUT_OFF, > + /** The output will use the current active mode */ > + WESTON_DRM_BACKEND_OUTPUT_CURRENT, > + /** The output will use the preferred mode. A modeline can be provided > + * by setting weston_backend_output_config::modeline in the form of > + * "WIDTHxHEIGHT" or in the form of an explicit modeline calculated > + * using e.g. the cvt tool. If a valid modeline is supplied it will be > + * used, if invalid or NULL the preferred available mode will be > used. */ > + WESTON_DRM_BACKEND_OUTPUT_PREFERRED, > +}; > + > +struct weston_drm_backend_output_config { > + struct weston_backend_output_config base; > + > + /** The pixel format to be used by the output. Valid values are: > + * - NULL - The format set at backend creation time will be used; > + * - "xrgb8888"; > + * - "rgb565" > + * - "xrgb2101010" > + */ > + char *format; > + /** The seat to be used by the output. Set to NULL to use the > + * default seat. */ > + char *seat; > + /** The modeline to be used by the output. Refer to the documentation > + * of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details. */ > + char *modeline; > +}; > + > +/** The backend configuration struct. > + * > + * weston_drm_backend_config contains the configuration used by a DRM > + * backend. The backend will take ownership of the weston_backend_config > + * object passed to it on initialization and will free it on destruction. */ > +struct weston_drm_backend_config { > + struct weston_backend_config base; > + > + /** The connector id of the output to be initialized. A value of 0 > will > + * enable all available outputs. */ > + int connector; > + /** The tty to be used. Set to 0 to use the current tty. */ > + int tty; > + /** If true the pixman renderer will be used instead of the OpenGL ES > + * renderer. */ > + bool use_pixman; > + /** The seat to be used for input and output. If NULL the default > "seat0" > + * will be used. > + * The backend will take ownership of the seat_id pointer and will > free > + * it on backend destruction. */ > + char *seat_id; > + /** The pixel format of the framebuffer to be used. Valid values are: > + * - NULL - The default format ("xrgb8888") will be used; > + * - "xrgb8888"; > + * - "rgb565" > + * - "xrgb2101010" > + * The backend will take ownership of the format pointer and will free > + * it on backend destruction. */ > + char *format; > + > + /** Callback used to configure the outputs. This function will be > called > + * by the backend when a new DRM output needs to be configured. */ > + enum weston_drm_backend_output_mode > + (*configure_output)(struct weston_compositor *compositor, > + struct weston_drm_backend_config > *backend_config, > + const char *name, > + struct weston_drm_backend_output_config > *output_config); > +};
My main concern here is that encoding this as ABI (every single option for every single backend) seems a bit ambitious, and likely to lead to version churn. To avoid this, you could look at either some kind of generic key/value-store query API, or at least encoding a version number into the struct, so you don't have to incompatibly break ABI every time you add an option. Other than that, looks fine enough to me, at a first pass, whilst being sick. So probably not quite strong enough for a Reviewed-by. ;) Cheers, Daniel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
