[PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available
Dave Airlie
airlied at gmail.com
Wed Sep 12 01:01:18 UTC 2018
On Wed, 28 Feb 2018 at 11:21, Daniel Stone <daniels at collabora.com> wrote:
>
> From: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
>
> In order to flip between compressed and uncompressed buffers -
> something drmModePageFlip explicitly bans us from doing - we need
> to port use the atomic modesetting API. It's only 'fake' atomic
> though given we still commit for each CRTC separately and
> CRTC and connector properties are not set with the atomic API.
>
> The helper functions to retrieve DRM properties have been borrowed
> from Weston.
>
> Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
> Reviewed-by: Daniel Stone <daniels at collabora.com>
> ---
> configure.ac | 3 +
> hw/xfree86/drivers/modesetting/driver.c | 6 +
> hw/xfree86/drivers/modesetting/driver.h | 1 +
> hw/xfree86/drivers/modesetting/drmmode_display.c | 374 ++++++++++++++++++++++-
> hw/xfree86/drivers/modesetting/drmmode_display.h | 36 +++
> hw/xfree86/drivers/modesetting/pageflip.c | 22 +-
> include/dix-config.h.in | 3 +
> include/meson.build | 2 +
> 8 files changed, 443 insertions(+), 4 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index cef9503d7..46662867f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2106,6 +2106,9 @@ if test "x$GLAMOR" = xyes; then
> fi
> fi
>
> + PKG_CHECK_EXISTS(libdrm >= 2.4.62,
> + [AC_DEFINE(GLAMOR_HAS_DRM_ATOMIC, 1, [libdrm supports atomic API])],
> + [])
> PKG_CHECK_EXISTS(libdrm >= 2.4.74,
> [AC_DEFINE(GLAMOR_HAS_DRM_NAME_FROM_FD_2, 1, [Have GLAMOR_HAS_DRM_NAME_FROM_FD_2])],
> [])
> diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
> index ec2aa9a27..88a42257c 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -1018,6 +1018,12 @@ PreInit(ScrnInfoPtr pScrn, int flags)
> }
> #endif
>
> +#ifdef GLAMOR_HAS_DRM_ATOMIC
> + ret = drmSetClientCap(ms->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> + ret |= drmSetClientCap(ms->fd, DRM_CLIENT_CAP_ATOMIC, 1);
> + ms->atomic_modeset = (ret == 0);
> +#endif
> +
> if (drmmode_pre_init(pScrn, &ms->drmmode, pScrn->bitsPerPixel / 8) == FALSE) {
> xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "KMS setup failed\n");
> goto fail;
> diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h
> index fe835918b..ed32239db 100644
> --- a/hw/xfree86/drivers/modesetting/driver.h
> +++ b/hw/xfree86/drivers/modesetting/driver.h
> @@ -105,6 +105,7 @@ typedef struct _modesettingRec {
> * Page flipping stuff.
> * @{
> */
> + Bool atomic_modeset;
> /** @} */
>
> DamagePtr damage;
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index bfc8c50db..8674c2c16 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -75,6 +75,233 @@ drmmode_zaphod_string_matches(ScrnInfoPtr scrn, const char *s, char *output_name
> return ret;
> }
>
> +static uint64_t
> +drmmode_prop_get_value(drmmode_prop_info_ptr info,
> + drmModeObjectPropertiesPtr props,
> + uint64_t def)
> +{
> + unsigned int i;
> +
> + if (info->prop_id == 0)
> + return def;
> +
> + for (i = 0; i < props->count_props; i++) {
> + unsigned int j;
> +
> + if (props->props[i] != info->prop_id)
> + continue;
> +
> + /* Simple (non-enum) types can return the value directly */
> + if (info->num_enum_values == 0)
> + return props->prop_values[i];
> +
> + /* Map from raw value to enum value */
> + for (j = 0; j < info->num_enum_values; j++) {
> + if (!info->enum_values[j].valid)
> + continue;
> + if (info->enum_values[j].value != props->prop_values[i])
> + continue;
> +
> + return j;
> + }
> + }
> +
> + return def;
> +}
> +
> +static uint32_t
> +drmmode_prop_info_update(drmmode_ptr drmmode,
> + drmmode_prop_info_ptr info,
> + unsigned int num_infos,
> + drmModeObjectProperties *props)
> +{
> + drmModePropertyRes *prop;
> + uint32_t valid_mask = 0;
> + unsigned i, j;
> +
> + assert(num_infos <= 32 && "update return type");
> +
> + for (i = 0; i < props->count_props; i++) {
> + Bool props_incomplete = FALSE;
> + unsigned int k;
> +
> + for (j = 0; j < num_infos; j++) {
> + if (info[j].prop_id == props->props[i])
> + break;
> + if (!info[j].prop_id)
> + props_incomplete = TRUE;
> + }
> +
> + /* We've already discovered this property. */
> + if (j != num_infos)
> + continue;
> +
> + /* We haven't found this property ID, but as we've already
> + * found all known properties, we don't need to look any
> + * further. */
> + if (!props_incomplete)
> + break;
> +
> + prop = drmModeGetProperty(drmmode->fd, props->props[i]);
> + if (!prop)
> + continue;
> +
> + for (j = 0; j < num_infos; j++) {
> + if (!strcmp(prop->name, info[j].name))
> + break;
> + }
> +
> + /* We don't know/care about this property. */
> + if (j == num_infos) {
> + drmModeFreeProperty(prop);
> + continue;
> + }
> +
> + info[j].prop_id = props->props[i];
> + valid_mask |= 1U << j;
> +
> + if (info[j].num_enum_values == 0) {
> + drmModeFreeProperty(prop);
> + continue;
> + }
> +
> + if (!(prop->flags & DRM_MODE_PROP_ENUM)) {
> + xf86DrvMsg(drmmode->scrn->scrnIndex, X_WARNING,
> + "expected property %s to be an enum,"
> + " but it is not; ignoring\n", prop->name);
> + drmModeFreeProperty(prop);
> + continue;
> + }
> +
> + for (k = 0; k < info[j].num_enum_values; k++) {
> + int l;
> +
> + if (info[j].enum_values[k].valid)
> + continue;
> +
> + for (l = 0; l < prop->count_enums; l++) {
> + if (!strcmp(prop->enums[l].name,
> + info[j].enum_values[k].name))
> + break;
> + }
> +
> + if (l == prop->count_enums)
> + continue;
> +
> + info[j].enum_values[k].valid = TRUE;
> + info[j].enum_values[k].value = prop->enums[l].value;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + return valid_mask;
> +}
> +
> +static Bool
> +drmmode_prop_info_copy(drmmode_prop_info_ptr dst,
> + const drmmode_prop_info_rec *src,
> + unsigned int num_props,
> + Bool copy_prop_id)
> +{
> + unsigned int i;
> +
> + memcpy(dst, src, num_props * sizeof(*dst));
> +
> + for (i = 0; i < num_props; i++) {
> + unsigned int j;
> +
> + if (copy_prop_id)
> + dst[i].prop_id = src[i].prop_id;
> + else
> + dst[i].prop_id = 0;
> +
> + if (src[i].num_enum_values == 0)
> + continue;
> +
> + dst[i].enum_values =
> + malloc(src[i].num_enum_values *
> + sizeof(*dst[i].enum_values));
> + if (!dst[i].enum_values)
> + goto err;
> +
> + memcpy(dst[i].enum_values, src[i].enum_values,
> + src[i].num_enum_values * sizeof(*dst[i].enum_values));
> +
> + for (j = 0; j < dst[i].num_enum_values; j++)
> + dst[i].enum_values[j].valid = FALSE;
> + }
> +
> + return TRUE;
> +
> +err:
> + while (i--)
> + free(dst[i].enum_values);
> + free(dst);
> + return FALSE;
> +}
On failure this frees dst, however...
> +drmmode_crtc_create_planes(xf86CrtcPtr crtc, int num)
> +{
> + drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> + drmmode_ptr drmmode = drmmode_crtc->drmmode;
> + drmModePlaneRes *kplane_res;
> + drmModePlane *kplane;
> + drmModeObjectProperties *props;
> + uint32_t i, type;
> + int current_crtc, best_plane = 0;
> +
> + static drmmode_prop_enum_info_rec plane_type_enums[] = {
> + [DRMMODE_PLANE_TYPE_PRIMARY] = {
> + .name = "Primary",
> + },
> + [DRMMODE_PLANE_TYPE_OVERLAY] = {
> + .name = "Overlay",
> + },
> + [DRMMODE_PLANE_TYPE_CURSOR] = {
> + .name = "Cursor",
> + },
> + };
> + static const drmmode_prop_info_rec plane_props[] = {
> + [DRMMODE_PLANE_TYPE] = {
> + .name = "type",
> + .enum_values = plane_type_enums,
> + .num_enum_values = DRMMODE_PLANE_TYPE__COUNT,
> + },
> + [DRMMODE_PLANE_FB_ID] = { .name = "FB_ID", },
> + [DRMMODE_PLANE_CRTC_ID] = { .name = "CRTC_ID", },
> + [DRMMODE_PLANE_SRC_X] = { .name = "SRC_X", },
> + [DRMMODE_PLANE_SRC_Y] = { .name = "SRC_Y", },
> + };
> + drmmode_prop_info_rec tmp_props[DRMMODE_PLANE__COUNT];
We use a stack allocated tmp_props here.
Not sure how to test any fix I write but seems like it shouldn't be a
major problem.
Dave.
More information about the xorg-devel
mailing list