2015-12-09 18:58 GMT+02:00 Daniel Stone <[email protected]>: > 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.
Well, but the plan is indeed to make multiple versions co-installable, and anyway the API/ABI will change in every weston release at least in the core so i don't think there is much point in going the extra mile to make the ABI stable here, just yet. > > 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
