Discussion:
[PATCH weston 1/3] compositor-drm: Read FB2_MODIFIERS capability
Deepak Rawat
2018-09-06 00:18:15 UTC
Permalink
Not all drivers support fb2 modifiers so read the capability before
using drmModeAddFB2WithModifiers.

Signed-off-by: Deepak Rawat <***@vmware.com>
---
libweston/compositor-drm.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 38911763..6a87a296 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -302,6 +302,8 @@ struct drm_backend {
bool shutting_down;

bool aspect_ratio_supported;
+
+ bool fb_modifiers;
};

struct drm_mode {
@@ -903,7 +905,7 @@ drm_fb_destroy_gbm(struct gbm_bo *bo, void *data)
}

static int
-drm_fb_addfb(struct drm_fb *fb)
+drm_fb_addfb(struct drm_fb *fb, bool fb_modifiers)
{
int ret = -EINVAL;
#ifdef HAVE_DRM_ADDFB2_MODIFIERS
@@ -913,7 +915,7 @@ drm_fb_addfb(struct drm_fb *fb)

/* If we have a modifier set, we must only use the WithModifiers
* entrypoint; we cannot import it through legacy ioctls. */
- if (fb->modifier != DRM_FORMAT_MOD_INVALID) {
+ if (fb_modifiers && fb->modifier != DRM_FORMAT_MOD_INVALID) {
/* KMS demands that if a modifier is set, it must be the same
* for all planes. */
#ifdef HAVE_DRM_ADDFB2_MODIFIERS
@@ -996,7 +998,7 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int height,
fb->height = height;
fb->fd = b->drm.fd;

- if (drm_fb_addfb(fb) != 0) {
+ if (drm_fb_addfb(fb, b->fb_modifiers) != 0) {
weston_log("failed to create kms fb: %m\n");
goto err_bo;
}
@@ -1168,7 +1170,7 @@ drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf,
goto err_free;
}

- if (drm_fb_addfb(fb) != 0)
+ if (drm_fb_addfb(fb, backend->fb_modifiers) != 0)
goto err_free;

return fb;
@@ -1239,7 +1241,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend,
goto err_free;
}

- if (drm_fb_addfb(fb) != 0) {
+ if (drm_fb_addfb(fb, backend->fb_modifiers) != 0) {
if (type == BUFFER_GBM_SURFACE)
weston_log("failed to create kms fb: %m\n");
goto err_free;
@@ -3822,6 +3824,14 @@ init_kms_caps(struct drm_backend *b)
weston_log("DRM: %s atomic modesetting\n",
b->atomic_modeset ? "supports" : "does not support");

+#ifdef HAVE_DRM_ADDFB2_MODIFIERS
+ ret = drmGetCap(b->drm.fd, DRM_CAP_ADDFB2_MODIFIERS, &cap);
+ if (ret == 0)
+ b->fb_modifiers = cap;
+ else
+#endif
+ b->fb_modifiers = 0;
+
/*
* KMS support for hardware planes cannot properly synchronize
* without nuclear page flip. Without nuclear/atomic, hw plane
--
2.17.1
Deepak Rawat
2018-09-06 00:18:16 UTC
Permalink
The plane property FB_DAMAGE_CLIPS provides a way mark damaged regions
on the plane in framebuffer coordinates of the framebuffer attached to
the plane.

This patch adds a new member "damage" to compositor version of
drm_plane_state and set FB_DAMAGE_CLIPS property whenever damage is
available.

Signed-off-by: Deepak Rawat <***@vmware.com>
Cc: dri-***@lists.freedesktop.org
---
libweston/compositor-drm.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 6a87a296..30f6fce9 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -129,6 +129,7 @@ enum wdrm_plane_property {
WDRM_PLANE_FB_ID,
WDRM_PLANE_CRTC_ID,
WDRM_PLANE_IN_FORMATS,
+ WDRM_PLANE_FB_DAMAGE_CLIPS,
WDRM_PLANE__COUNT
};

@@ -171,6 +172,7 @@ static const struct drm_property_info plane_props[] = {
[WDRM_PLANE_FB_ID] = { .name = "FB_ID", },
[WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", },
[WDRM_PLANE_IN_FORMATS] = { .name = "IN_FORMATS" },
+ [WDRM_PLANE_FB_DAMAGE_CLIPS] = { .name = "FB_DAMAGE_CLIPS" },
};

/**
@@ -403,6 +405,8 @@ struct drm_plane_state {

bool complete;

+ pixman_region32_t damage; /* damage sent to kernel */
+
struct wl_list link; /* drm_output_state::plane_list */
};

@@ -1306,6 +1310,7 @@ drm_plane_state_alloc(struct drm_output_state *state_output,
assert(state);
state->output_state = state_output;
state->plane = plane;
+ pixman_region32_init(&state->damage);

/* Here we only add the plane state to the desired link, and not
* set the member. Having an output pointer set means that the
@@ -1336,6 +1341,7 @@ drm_plane_state_free(struct drm_plane_state *state, bool force)
wl_list_remove(&state->link);
wl_list_init(&state->link);
state->output_state = NULL;
+ pixman_region32_fini(&state->damage);

if (force || state != state->plane->state_cur) {
drm_fb_unref(state->fb);
@@ -1373,6 +1379,7 @@ drm_plane_state_duplicate(struct drm_output_state *state_output,
if (src->fb)
dst->fb = drm_fb_ref(src->fb);
dst->output_state = state_output;
+ pixman_region32_init(&dst->damage);
dst->complete = false;

return dst;
@@ -2409,6 +2416,33 @@ drm_mode_ensure_blob(struct drm_backend *backend, struct drm_mode *mode)
return ret;
}

+static int
+plane_add_damage(drmModeAtomicReq *req, struct drm_backend *backend,
+ struct drm_plane_state *plane_state)
+{
+ struct drm_plane *plane = plane_state->plane;
+ pixman_box32_t *rects;
+ uint32_t blob_id;
+ int n_rects;
+ int ret;
+
+ if (!pixman_region32_not_empty(&plane_state->damage))
+ return 0;
+
+ rects = pixman_region32_rectangles(&plane_state->damage, &n_rects);
+
+ ret = drmModeCreatePropertyBlob(backend->drm.fd, rects,
+ sizeof(*rects) * n_rects, &blob_id);
+ if (ret != 0)
+ return ret;
+
+ ret = plane_add_prop(req, plane, WDRM_PLANE_FB_DAMAGE_CLIPS, blob_id);
+ if (ret != 0)
+ return ret;
+
+ return 0;
+}
+
static int
drm_output_apply_state_atomic(struct drm_output_state *state,
drmModeAtomicReq *req,
@@ -2477,6 +2511,7 @@ drm_output_apply_state_atomic(struct drm_output_state *state,
plane_state->dest_w);
ret |= plane_add_prop(req, plane, WDRM_PLANE_CRTC_H,
plane_state->dest_h);
+ ret |= plane_add_damage(req, backend, plane_state);

if (ret != 0) {
weston_log("couldn't set plane state\n");
--
2.17.1
Daniel Vetter
2018-09-06 07:25:02 UTC
Permalink
Post by Deepak Rawat
The plane property FB_DAMAGE_CLIPS provides a way mark damaged regions
on the plane in framebuffer coordinates of the framebuffer attached to
the plane.
This patch adds a new member "damage" to compositor version of
drm_plane_state and set FB_DAMAGE_CLIPS property whenever damage is
available.
Note that weston has switched over to gitlab based merge requests:

https://gitlab.freedesktop.org/wayland/weston/blob/master/CONTRIBUTING.md

Cheers, Daniel
Post by Deepak Rawat
---
libweston/compositor-drm.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 6a87a296..30f6fce9 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -129,6 +129,7 @@ enum wdrm_plane_property {
WDRM_PLANE_FB_ID,
WDRM_PLANE_CRTC_ID,
WDRM_PLANE_IN_FORMATS,
+ WDRM_PLANE_FB_DAMAGE_CLIPS,
WDRM_PLANE__COUNT
};
@@ -171,6 +172,7 @@ static const struct drm_property_info plane_props[] = {
[WDRM_PLANE_FB_ID] = { .name = "FB_ID", },
[WDRM_PLANE_CRTC_ID] = { .name = "CRTC_ID", },
[WDRM_PLANE_IN_FORMATS] = { .name = "IN_FORMATS" },
+ [WDRM_PLANE_FB_DAMAGE_CLIPS] = { .name = "FB_DAMAGE_CLIPS" },
};
/**
@@ -403,6 +405,8 @@ struct drm_plane_state {
bool complete;
+ pixman_region32_t damage; /* damage sent to kernel */
+
struct wl_list link; /* drm_output_state::plane_list */
};
@@ -1306,6 +1310,7 @@ drm_plane_state_alloc(struct drm_output_state *state_output,
assert(state);
state->output_state = state_output;
state->plane = plane;
+ pixman_region32_init(&state->damage);
/* Here we only add the plane state to the desired link, and not
* set the member. Having an output pointer set means that the
@@ -1336,6 +1341,7 @@ drm_plane_state_free(struct drm_plane_state *state, bool force)
wl_list_remove(&state->link);
wl_list_init(&state->link);
state->output_state = NULL;
+ pixman_region32_fini(&state->damage);
if (force || state != state->plane->state_cur) {
drm_fb_unref(state->fb);
@@ -1373,6 +1379,7 @@ drm_plane_state_duplicate(struct drm_output_state *state_output,
if (src->fb)
dst->fb = drm_fb_ref(src->fb);
dst->output_state = state_output;
+ pixman_region32_init(&dst->damage);
dst->complete = false;
return dst;
@@ -2409,6 +2416,33 @@ drm_mode_ensure_blob(struct drm_backend *backend, struct drm_mode *mode)
return ret;
}
+static int
+plane_add_damage(drmModeAtomicReq *req, struct drm_backend *backend,
+ struct drm_plane_state *plane_state)
+{
+ struct drm_plane *plane = plane_state->plane;
+ pixman_box32_t *rects;
+ uint32_t blob_id;
+ int n_rects;
+ int ret;
+
+ if (!pixman_region32_not_empty(&plane_state->damage))
+ return 0;
+
+ rects = pixman_region32_rectangles(&plane_state->damage, &n_rects);
+
+ ret = drmModeCreatePropertyBlob(backend->drm.fd, rects,
+ sizeof(*rects) * n_rects, &blob_id);
+ if (ret != 0)
+ return ret;
+
+ ret = plane_add_prop(req, plane, WDRM_PLANE_FB_DAMAGE_CLIPS, blob_id);
+ if (ret != 0)
+ return ret;
+
+ return 0;
+}
+
static int
drm_output_apply_state_atomic(struct drm_output_state *state,
drmModeAtomicReq *req,
@@ -2477,6 +2511,7 @@ drm_output_apply_state_atomic(struct drm_output_state *state,
plane_state->dest_w);
ret |= plane_add_prop(req, plane, WDRM_PLANE_CRTC_H,
plane_state->dest_h);
+ ret |= plane_add_damage(req, backend, plane_state);
if (ret != 0) {
weston_log("couldn't set plane state\n");
--
2.17.1
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Deepak Rawat
2018-09-06 00:18:17 UTC
Permalink
Copy the damage region to scanout drm_plane_state which will be sent to
kernel during atomic state update.

Signed-off-by: Deepak Rawat <***@vmware.com>
---
libweston/compositor-drm.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 30f6fce9..e2ee35eb 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -2111,6 +2111,21 @@ drm_output_render(struct drm_output_state *state, pixman_region32_t *damage)
scanout_state->dest_w = scanout_state->src_w >> 16;
scanout_state->dest_h = scanout_state->src_h >> 16;

+ pixman_region32_copy(&scanout_state->damage, damage);
+ if (output->base.zoom.active) {
+ weston_matrix_transform_region(&scanout_state->damage,
+ &output->base.matrix,
+ &scanout_state->damage);
+ } else {
+ pixman_region32_translate(&scanout_state->damage,
+ -output->base.x, -output->base.y);
+ weston_transformed_region(output->base.width,
+ output->base.height,
+ output->base.transform,
+ output->base.current_scale,
+ &scanout_state->damage,
+ &scanout_state->damage);
+ }

pixman_region32_subtract(&c->primary_plane.damage,
&c->primary_plane.damage, damage);
--
2.17.1
Sinclair Yeh
2018-09-17 17:34:02 UTC
Permalink
Post by Deepak Rawat
Not all drivers support fb2 modifiers so read the capability before
using drmModeAddFB2WithModifiers.
---
libweston/compositor-drm.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 38911763..6a87a296 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -302,6 +302,8 @@ struct drm_backend {
bool shutting_down;
bool aspect_ratio_supported;
+
+ bool fb_modifiers;
};
struct drm_mode {
@@ -903,7 +905,7 @@ drm_fb_destroy_gbm(struct gbm_bo *bo, void *data)
}
static int
-drm_fb_addfb(struct drm_fb *fb)
+drm_fb_addfb(struct drm_fb *fb, bool fb_modifiers)
{
int ret = -EINVAL;
#ifdef HAVE_DRM_ADDFB2_MODIFIERS
@@ -913,7 +915,7 @@ drm_fb_addfb(struct drm_fb *fb)
/* If we have a modifier set, we must only use the WithModifiers
* entrypoint; we cannot import it through legacy ioctls. */
- if (fb->modifier != DRM_FORMAT_MOD_INVALID) {
+ if (fb_modifiers && fb->modifier != DRM_FORMAT_MOD_INVALID) {
/* KMS demands that if a modifier is set, it must be the same
* for all planes. */
#ifdef HAVE_DRM_ADDFB2_MODIFIERS
@@ -996,7 +998,7 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int height,
fb->height = height;
fb->fd = b->drm.fd;
- if (drm_fb_addfb(fb) != 0) {
+ if (drm_fb_addfb(fb, b->fb_modifiers) != 0) {
weston_log("failed to create kms fb: %m\n");
goto err_bo;
}
@@ -1168,7 +1170,7 @@ drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf,
goto err_free;
}
- if (drm_fb_addfb(fb) != 0)
+ if (drm_fb_addfb(fb, backend->fb_modifiers) != 0)
goto err_free;
return fb;
@@ -1239,7 +1241,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend,
goto err_free;
}
- if (drm_fb_addfb(fb) != 0) {
+ if (drm_fb_addfb(fb, backend->fb_modifiers) != 0) {
if (type == BUFFER_GBM_SURFACE)
weston_log("failed to create kms fb: %m\n");
goto err_free;
@@ -3822,6 +3824,14 @@ init_kms_caps(struct drm_backend *b)
weston_log("DRM: %s atomic modesetting\n",
b->atomic_modeset ? "supports" : "does not support");
+#ifdef HAVE_DRM_ADDFB2_MODIFIERS
+ ret = drmGetCap(b->drm.fd, DRM_CAP_ADDFB2_MODIFIERS, &cap);
+ if (ret == 0)
+ b->fb_modifiers = cap;
+ else
+#endif
+ b->fb_modifiers = 0;
+
/*
* KMS support for hardware planes cannot properly synchronize
* without nuclear page flip. Without nuclear/atomic, hw plane
--
2.17.1
Loading...