This patch removes the trailing white spaces.
Signed-off-by: Hyungwon Hwang human.hwang@samsung.com --- xf86drmMode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xf86drmMode.c b/xf86drmMode.c index fc19504..d4ed5c1 100644 --- a/xf86drmMode.c +++ b/xf86drmMode.c @@ -879,7 +879,7 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx) int len, i; struct drm_event *e; struct drm_event_vblank *vblank; - + /* The DRM read semantics guarantees that we always get only * complete events. */
@@ -899,7 +899,7 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx) break; vblank = (struct drm_event_vblank *) e; evctx->vblank_handler(fd, - vblank->sequence, + vblank->sequence, vblank->tv_sec, vblank->tv_usec, U642VOID (vblank->user_data));
This patch seprates the code, which sorts proprty sets and eliminates duplicate properties, from drmModeAtomicCommit(). Now drmModeAtomicCleanup() has to do the job before calling drmModeAtomicCommit(), and drmModeAtomicCommit() just converts the cleaned request to IOCTL argument.
Signed-off-by: Hyungwon Hwang human.hwang@samsung.com --- xf86drmMode.c | 72 +++++++++++++++++++++++++++++++++-------------------------- xf86drmMode.h | 1 + 2 files changed, 41 insertions(+), 32 deletions(-)
diff --git a/xf86drmMode.c b/xf86drmMode.c index d4ed5c1..82c4c91 100644 --- a/xf86drmMode.c +++ b/xf86drmMode.c @@ -1303,10 +1303,39 @@ static int sort_req_list(const void *misc, const void *other) return second->property_id - first->property_id; }
+void drmModeAtomicCleanup(drmModeAtomicReqPtr req) +{ + uint32_t last_obj_id = 0; + uint32_t i; + + if (req->cursor == 0) + return; + + /* Sort the list by object ID, then by property ID. */ + qsort(req->items, req->cursor, sizeof(*req->items), + sort_req_list); + + /* Eliminate duplicate property sets. */ + for (i = 0; i < req->cursor; i++) { + if (req->items[i].object_id != last_obj_id) + last_obj_id = req->items[i].object_id; + + if (i == req->cursor - 1) + continue; + + if (req->items[i].object_id != req->items[i + 1].object_id || + req->items[i].property_id != req->items[i + 1].property_id) + continue; + + memmove(&req->items[i], &req->items[i + 1], + (req->cursor - i - 1) * sizeof(*req->items)); + req->cursor--; + } +} + int drmModeAtomicCommit(int fd, drmModeAtomicReqPtr req, uint32_t flags, void *user_data) { - drmModeAtomicReqPtr sorted; struct drm_mode_atomic atomic; uint32_t *objs_ptr = NULL; uint32_t *count_props_ptr = NULL; @@ -1320,33 +1349,13 @@ int drmModeAtomicCommit(int fd, drmModeAtomicReqPtr req, uint32_t flags, if (req->cursor == 0) return 0;
- sorted = drmModeAtomicDuplicate(req); - if (sorted == NULL) - return -ENOMEM; - memclear(atomic);
- /* Sort the list by object ID, then by property ID. */ - qsort(sorted->items, sorted->cursor, sizeof(*sorted->items), - sort_req_list); - - /* Now the list is sorted, eliminate duplicate property sets. */ - for (i = 0; i < sorted->cursor; i++) { - if (sorted->items[i].object_id != last_obj_id) { + for (i = 0; i < req->cursor; i++) { + if (req->items[i].object_id != last_obj_id) { atomic.count_objs++; - last_obj_id = sorted->items[i].object_id; + last_obj_id = req->items[i].object_id; } - - if (i == sorted->cursor - 1) - continue; - - if (sorted->items[i].object_id != sorted->items[i + 1].object_id || - sorted->items[i].property_id != sorted->items[i + 1].property_id) - continue; - - memmove(&sorted->items[i], &sorted->items[i + 1], - (sorted->cursor - i - 1) * sizeof(*sorted->items)); - sorted->cursor--; }
objs_ptr = drmMalloc(atomic.count_objs * sizeof objs_ptr[0]); @@ -1361,28 +1370,28 @@ int drmModeAtomicCommit(int fd, drmModeAtomicReqPtr req, uint32_t flags, goto out; }
- props_ptr = drmMalloc(sorted->cursor * sizeof props_ptr[0]); + props_ptr = drmMalloc(req->cursor * sizeof props_ptr[0]); if (!props_ptr) { errno = ENOMEM; goto out; }
- prop_values_ptr = drmMalloc(sorted->cursor * sizeof prop_values_ptr[0]); + prop_values_ptr = drmMalloc(req->cursor * sizeof prop_values_ptr[0]); if (!prop_values_ptr) { errno = ENOMEM; goto out; }
- for (i = 0, last_obj_id = 0; i < sorted->cursor; i++) { - if (sorted->items[i].object_id != last_obj_id) { + for (i = 0, last_obj_id = 0; i < req->cursor; i++) { + if (req->items[i].object_id != last_obj_id) { obj_idx++; - objs_ptr[obj_idx] = sorted->items[i].object_id; + objs_ptr[obj_idx] = req->items[i].object_id; last_obj_id = objs_ptr[obj_idx]; }
count_props_ptr[obj_idx]++; - props_ptr[i] = sorted->items[i].property_id; - prop_values_ptr[i] = sorted->items[i].value; + props_ptr[i] = req->items[i].property_id; + prop_values_ptr[i] = req->items[i].value;
}
@@ -1400,7 +1409,6 @@ out: drmFree(count_props_ptr); drmFree(props_ptr); drmFree(prop_values_ptr); - drmModeAtomicFree(sorted);
return ret; } diff --git a/xf86drmMode.h b/xf86drmMode.h index 4de7bbb..fe14078 100644 --- a/xf86drmMode.h +++ b/xf86drmMode.h @@ -498,6 +498,7 @@ extern int drmModeAtomicAddProperty(drmModeAtomicReqPtr req, uint32_t object_id, uint32_t property_id, uint64_t value); +extern void drmModeAtomicCleanup(drmModeAtomicReqPtr req); extern int drmModeAtomicCommit(int fd, drmModeAtomicReqPtr req, uint32_t flags,
Hi Hyungwon,
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch seprates the code, which sorts proprty sets and eliminates duplicate properties, from drmModeAtomicCommit(). Now drmModeAtomicCleanup() has to do the job before calling drmModeAtomicCommit(), and drmModeAtomicCommit() just converts the cleaned request to IOCTL argument.
Afaict the commit message should say why we want this, rather than rewording what the patch does.
I'm not sure about the atomic status for wayland and others but this commit might cause issues there. Additionally, with this patch we'll send a lot of useless information to the kernel if one omits drmModeAtomicCleanup(). The kernel will likely discard it but still this doesn't seem like a good idea imho.
-Emil
Hi Emil,
On Thu, 20 Aug 2015 17:17:27 +0100 Emil Velikov emil.l.velikov@gmail.com wrote:
Hi Hyungwon,
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch seprates the code, which sorts proprty sets and eliminates duplicate properties, from drmModeAtomicCommit(). Now drmModeAtomicCleanup() has to do the job before calling drmModeAtomicCommit(), and drmModeAtomicCommit() just converts the cleaned request to IOCTL argument.
Afaict the commit message should say why we want this, rather than rewording what the patch does.
I'm not sure about the atomic status for wayland and others but this commit might cause issues there. Additionally, with this patch we'll send a lot of useless information to the kernel if one omits drmModeAtomicCleanup(). The kernel will likely discard it but still this doesn't seem like a good idea imho.
Yes. I agree that this change burdens the userspace application to use API correctly. In my case, for modetest, the function of cleaning up the request is needed, so I thought that this separation would be needed. Overall, I agree with you. So I will drop this patch, and find another way which is specific for modetest.
Thanks for your review.
Best regards, Hyungwon Hwang
-Emil
On Fri, 21 Aug 2015 13:54:49 +0900 Hyungwon Hwang human.hwang@samsung.com wrote:
Hi Emil,
On Thu, 20 Aug 2015 17:17:27 +0100 Emil Velikov emil.l.velikov@gmail.com wrote:
Hi Hyungwon,
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch seprates the code, which sorts proprty sets and eliminates duplicate properties, from drmModeAtomicCommit(). Now drmModeAtomicCleanup() has to do the job before calling drmModeAtomicCommit(), and drmModeAtomicCommit() just converts the cleaned request to IOCTL argument.
Afaict the commit message should say why we want this, rather than rewording what the patch does.
I'm not sure about the atomic status for wayland and others but this commit might cause issues there. Additionally, with this patch we'll send a lot of useless information to the kernel if one omits drmModeAtomicCleanup(). The kernel will likely discard it but still this doesn't seem like a good idea imho.
Yes. I agree that this change burdens the userspace application to use API correctly. In my case, for modetest, the function of cleaning up the request is needed, so I thought that this separation would be needed. Overall, I agree with you. So I will drop this patch, and find another way which is specific for modetest.
Hi,
why do you need that, exactly?
Thanks, pq
Hi Pekka,
On Fri, 21 Aug 2015 09:42:26 +0300 Pekka Paalanen ppaalanen@gmail.com wrote:
On Fri, 21 Aug 2015 13:54:49 +0900 Hyungwon Hwang human.hwang@samsung.com wrote:
Hi Emil,
On Thu, 20 Aug 2015 17:17:27 +0100 Emil Velikov emil.l.velikov@gmail.com wrote:
Hi Hyungwon,
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch seprates the code, which sorts proprty sets and eliminates duplicate properties, from drmModeAtomicCommit(). Now drmModeAtomicCleanup() has to do the job before calling drmModeAtomicCommit(), and drmModeAtomicCommit() just converts the cleaned request to IOCTL argument.
Afaict the commit message should say why we want this, rather than rewording what the patch does.
I'm not sure about the atomic status for wayland and others but this commit might cause issues there. Additionally, with this patch we'll send a lot of useless information to the kernel if one omits drmModeAtomicCleanup(). The kernel will likely discard it but still this doesn't seem like a good idea imho.
Yes. I agree that this change burdens the userspace application to use API correctly. In my case, for modetest, the function of cleaning up the request is needed, so I thought that this separation would be needed. Overall, I agree with you. So I will drop this patch, and find another way which is specific for modetest.
Hi,
why do you need that, exactly?
To make the buffer for plane, I needed to figure out the width and the height which the user set which are in the request, but not applied to the kernel yet. To get the value from the request, I thought cleaning the request before I try to getting the value from the request was needed because the user could set the different values for the same property.
Thanks, pq
On Fri, 21 Aug 2015 16:18:59 +0900 Hyungwon Hwang human.hwang@samsung.com wrote:
Hi Pekka,
On Fri, 21 Aug 2015 09:42:26 +0300 Pekka Paalanen ppaalanen@gmail.com wrote:
On Fri, 21 Aug 2015 13:54:49 +0900 Hyungwon Hwang human.hwang@samsung.com wrote:
Hi Emil,
On Thu, 20 Aug 2015 17:17:27 +0100 Emil Velikov emil.l.velikov@gmail.com wrote:
Hi Hyungwon,
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch seprates the code, which sorts proprty sets and eliminates duplicate properties, from drmModeAtomicCommit(). Now drmModeAtomicCleanup() has to do the job before calling drmModeAtomicCommit(), and drmModeAtomicCommit() just converts the cleaned request to IOCTL argument.
Afaict the commit message should say why we want this, rather than rewording what the patch does.
I'm not sure about the atomic status for wayland and others but this commit might cause issues there. Additionally, with this patch we'll send a lot of useless information to the kernel if one omits drmModeAtomicCleanup(). The kernel will likely discard it but still this doesn't seem like a good idea imho.
Yes. I agree that this change burdens the userspace application to use API correctly. In my case, for modetest, the function of cleaning up the request is needed, so I thought that this separation would be needed. Overall, I agree with you. So I will drop this patch, and find another way which is specific for modetest.
Hi,
why do you need that, exactly?
To make the buffer for plane, I needed to figure out the width and the height which the user set which are in the request, but not applied to the kernel yet. To get the value from the request, I thought cleaning the request before I try to getting the value from the request was needed because the user could set the different values for the same property.
Hi,
I would say that that is completely out of the scope of the libdrm API. The caller of that API set those values, so it knows them - why should libdrm offer an API to query back the values you just put in the req?
The caller is storing those values somewhere else too in any case, exactly because it wants to allocate buffers of the same size. So very likely those values were already stored *before* the atomic request was even created.
IMHO, it is the responsibility of the application to track what it itself is doing. If you have to write some data structures to do that, then that's what you should do. Libdrm is not a replacement for that.
Thanks, pq
This patch makes 'struct _drmModeAtomicReqItem' and 'struct _drmModeAtomicReq' visible from outside. This is needed for userspace applications to use those structures when calling drmModeAtomicCommit().
Signed-off-by: Hyungwon Hwang human.hwang@samsung.com --- xf86drmMode.c | 14 -------------- xf86drmMode.h | 12 ++++++++++++ 2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/xf86drmMode.c b/xf86drmMode.c index 82c4c91..cf3fa21 100644 --- a/xf86drmMode.c +++ b/xf86drmMode.c @@ -1164,20 +1164,6 @@ int drmModeObjectSetProperty(int fd, uint32_t object_id, uint32_t object_type, return DRM_IOCTL(fd, DRM_IOCTL_MODE_OBJ_SETPROPERTY, &prop); }
-typedef struct _drmModeAtomicReqItem drmModeAtomicReqItem, *drmModeAtomicReqItemPtr; - -struct _drmModeAtomicReqItem { - uint32_t object_id; - uint32_t property_id; - uint64_t value; -}; - -struct _drmModeAtomicReq { - uint32_t cursor; - uint32_t size_items; - drmModeAtomicReqItemPtr items; -}; - drmModeAtomicReqPtr drmModeAtomicAlloc(void) { drmModeAtomicReqPtr req; diff --git a/xf86drmMode.h b/xf86drmMode.h index fe14078..ec05ed8 100644 --- a/xf86drmMode.h +++ b/xf86drmMode.h @@ -485,6 +485,18 @@ extern int drmModeObjectSetProperty(int fd, uint32_t object_id, uint64_t value);
+struct _drmModeAtomicReqItem { + uint32_t object_id; + uint32_t property_id; + uint64_t value; +}; +typedef struct _drmModeAtomicReqItem drmModeAtomicReqItem, *drmModeAtomicReqItemPtr; + +struct _drmModeAtomicReq { + uint32_t cursor; + uint32_t size_items; + drmModeAtomicReqItemPtr items; +}; typedef struct _drmModeAtomicReq drmModeAtomicReq, *drmModeAtomicReqPtr;
extern drmModeAtomicReqPtr drmModeAtomicAlloc(void);
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch makes 'struct _drmModeAtomicReqItem' and 'struct _drmModeAtomicReq' visible from outside. This is needed for userspace applications to use those structures when calling drmModeAtomicCommit().
Hmmm what is missing in the current API, that one needs direct access to the structs ? If we expose these to the user we'll be putting a (ABI) hedgehog down our pants (i.e. it might be ok, but will likely result in a very painful experience).
Thanks Emil
Hi Emil,
On Thu, 20 Aug 2015 17:23:09 +0100 Emil Velikov emil.l.velikov@gmail.com wrote:
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch makes 'struct _drmModeAtomicReqItem' and 'struct _drmModeAtomicReq' visible from outside. This is needed for userspace applications to use those structures when calling drmModeAtomicCommit().
Hmmm what is missing in the current API, that one needs direct access to the structs ? If we expose these to the user we'll be putting a (ABI) hedgehog down our pants (i.e. it might be ok, but will likely result in a very painful experience).
I also agree with you. I think I should drop this patch, and find another way for modetest.
Thanks.
Best regards, Hyungwon Hwang
Thanks Emil
On Fri, 21 Aug 2015 15:06:58 +0900 Hyungwon Hwang human.hwang@samsung.com wrote:
Hi Emil,
On Thu, 20 Aug 2015 17:23:09 +0100 Emil Velikov emil.l.velikov@gmail.com wrote:
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch makes 'struct _drmModeAtomicReqItem' and 'struct _drmModeAtomicReq' visible from outside. This is needed for userspace applications to use those structures when calling drmModeAtomicCommit().
Yeah, this sounds like a very bad idea.
Thanks, pq
Hmmm what is missing in the current API, that one needs direct access to the structs ? If we expose these to the user we'll be putting a (ABI) hedgehog down our pants (i.e. it might be ok, but will likely result in a very painful experience).
I also agree with you. I think I should drop this patch, and find another way for modetest.
Dear,
On Fri, 21 Aug 2015 09:44:42 +0300 Pekka Paalanen ppaalanen@gmail.com wrote:
On Fri, 21 Aug 2015 15:06:58 +0900 Hyungwon Hwang human.hwang@samsung.com wrote:
Hi Emil,
On Thu, 20 Aug 2015 17:23:09 +0100 Emil Velikov emil.l.velikov@gmail.com wrote:
On 19 August 2015 at 01:58, Hyungwon Hwang human.hwang@samsung.com wrote:
This patch makes 'struct _drmModeAtomicReqItem' and 'struct _drmModeAtomicReq' visible from outside. This is needed for userspace applications to use those structures when calling drmModeAtomicCommit().
Yeah, this sounds like a very bad idea.
Yes. Making it visible was not good. But then I think that new API for getting the value of a request item in the request which are not applied to the kernel. Because for preparing the buffer, the userspace program needs width and height which are in the request.
The program can get the value before making it as a request. In that case, the program such as modetest, which does not understand what the object id or propery id means, have to be modified to understand them for extracting the needed value.
How do you think about it? Is modetest just a special program for testing, and is this support not needed for another real program?
Thanks, pq
Hmmm what is missing in the current API, that one needs direct access to the structs ? If we expose these to the user we'll be putting a (ABI) hedgehog down our pants (i.e. it might be ok, but will likely result in a very painful experience).
I also agree with you. I think I should drop this patch, and find another way for modetest.
This patch removes the trailing white spaces.
Signed-off-by: Hyungwon Hwang human.hwang@samsung.com --- tests/modetest/modetest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 4eb9eac..43bd06f 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -1251,7 +1251,7 @@ static void test_page_flip(struct device *dev, struct pipe_arg *pipes, unsigned evctx.version = DRM_EVENT_CONTEXT_VERSION; evctx.vblank_handler = NULL; evctx.page_flip_handler = page_flip_handler; - + while (1) { #if 0 struct pollfd pfd[2]; @@ -1565,7 +1565,7 @@ int main(int argc, char **argv) if (parse_connector(&pipe_args[count], optarg) < 0) usage(argv[0]);
- count++; + count++; break; case 'C': test_cursor = 1;
This patch adds support for atomic modeset. Using -a option, user can make modeset to use DRM_IOCTL_MODE_ATOMIC instead of legacy IOCTLs. Also, by using -W option, user can set the value of each object's property.
Signed-off-by: Hyungwon Hwang human.hwang@samsung.com --- tests/modetest/modetest.c | 237 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 204 insertions(+), 33 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 43bd06f..bf9222d 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -1433,15 +1433,21 @@ static int parse_property(struct property_arg *p, const char *arg)
static void usage(char *name) { - fprintf(stderr, "usage: %s [-cDdefMPpsCvw]\n", name); + fprintf(stderr, "usage: %s [-acDdefMPpsCvwW]\n", name); + fprintf(stderr, "\tA: supported in atomic modeset\n"); + fprintf(stderr, "\tL: supported in legacy modeset\n");
- fprintf(stderr, "\n Query options:\n\n"); + fprintf(stderr, "\n Query options: [AL]\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); fprintf(stderr, "\t-e\tlist encoders\n"); fprintf(stderr, "\t-f\tlist framebuffers\n"); fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
- fprintf(stderr, "\n Test options:\n\n"); + fprintf(stderr, "\n Atomic Test options: [A]\n\n"); + fprintf(stderr, "\t-a\tuse atomic modeset\n"); + fprintf(stderr, "\t-W <obj_id>:<prop_name>:<value>\tset property\n"); + + fprintf(stderr, "\n Legacy test options: [L]\n\n"); fprintf(stderr, "\t-P <crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n"); fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:<mode>[-<vrefresh>][@<format>]\tset a mode\n"); fprintf(stderr, "\t-C\ttest hw cursor\n"); @@ -1449,9 +1455,9 @@ static void usage(char *name) fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
fprintf(stderr, "\n Generic options:\n\n"); - fprintf(stderr, "\t-d\tdrop master after mode set\n"); - fprintf(stderr, "\t-M module\tuse the given driver\n"); - fprintf(stderr, "\t-D device\tuse the given device\n"); + fprintf(stderr, "\t-d\tdrop master after mode set [L]\n"); + fprintf(stderr, "\t-M module\tuse the given driver [AL]\n"); + fprintf(stderr, "\t-D device\tuse the given device [AL]\n");
fprintf(stderr, "\n\tDefault is to dump all info.\n"); exit(0); @@ -1484,7 +1490,154 @@ static int cursor_supported(void) return 1; }
-static char optstr[] = "cdD:efM:P:ps:Cvw:"; +static int parse_atomic_args(drmModeAtomicReqPtr req, const char *p) +{ + uint32_t object_id, property_id; + uint64_t value; + + if (sscanf(p, "%u:%u:%llu", &object_id, &property_id, &value) != 3) + return -1; + + drmModeAtomicAddProperty(req, object_id, property_id, value); + + return 0; +} + +static uint32_t get_atomic_plane_prop_id(struct resources *res, uint32_t obj_id, + const char *name) +{ + drmModePropertyRes *props_info; + struct plane *plane; + unsigned int i, j; + + for (i = 0; i < res->plane_res->count_planes; i++) { + plane = &res->planes[i]; + if (plane->plane->plane_id != obj_id) + continue; + + for (j = 0; j < plane->props->count_props; j++) { + props_info = plane->props_info[j]; + if (!strcmp(props_info->name, name)) + return props_info->prop_id; + } + } + + return 0; +} + +static uint64_t get_atomic_plane_prop_value_in_request(drmModeAtomicReqPtr req, + struct resources *res, uint32_t obj_id, const char *name) +{ + uint32_t prop_id, i; + drmModeAtomicReqItemPtr item; + + prop_id = get_atomic_plane_prop_id(res, obj_id, name); + + for (i = 0; i < req->cursor; i++) { + item = &req->items[i]; + + if (item->object_id != obj_id) + continue; + + if (item->property_id != prop_id) + continue; + + return item->value; + } + + return -1; +} + +static bool is_atomic_obj_in_request(drmModeAtomicReqPtr req, + uint32_t obj_id) +{ + unsigned int i; + + for (i = 0; i < req->cursor; i++) + if (req->items[i].object_id == obj_id) + return true; + + return false; +} + +static int allocate_fb(struct device *dev, drmModeAtomicReqPtr req, struct resources *res, + uint32_t width, uint32_t height, uint32_t pixel_format, + int pattern, struct bo **bo, uint32_t *fb_id) +{ + uint32_t handles[4] = {0}, pitches[4] = {0}, offsets[4] = {0}; + int ret; + + *bo = bo_create(dev->fd, pixel_format, width, height, + handles, pitches, offsets, pattern); + if (*bo == NULL) { + fprintf(stderr, "failed to create bo (%ux%u): %s\n", + width, height, strerror(errno)); + return -1; + } + + ret = drmModeAddFB2(dev->fd, width, height, pixel_format, + handles, pitches, offsets, fb_id, 0); + if (ret) { + fprintf(stderr, "failed to add fb (%ux%u): %s\n", + width, height, strerror(errno)); + bo_destroy(*bo); + return ret; + } + + return 0; +} + +static int allocate_fbs(struct device *dev, drmModeAtomicReqPtr req, struct resources *res) +{ + uint32_t plane_id, fb_obj_id, fb_id, i, width, height, pixel_format; + struct bo *bo; + int ret; + + for (i = 0; i < res->plane_res->count_planes; i++) { + plane_id = res->planes[i].plane->plane_id; + + if (!is_atomic_obj_in_request(req, plane_id)) + continue; + + fb_obj_id = get_atomic_plane_prop_id(res, plane_id, "FB_ID"); + if (!fb_obj_id) + return -1; + + width = get_atomic_plane_prop_value_in_request(req, res, + plane_id, "SRC_W"); + height = get_atomic_plane_prop_value_in_request(req, res, + plane_id, "SRC_H"); + pixel_format = DRM_FORMAT_XRGB8888; + + ret = allocate_fb(dev, req, res, width, height, pixel_format, + PATTERN_SMPTE, &bo, &fb_id); + if (ret < 0) + return ret; + + dev->mode.bo = bo; + dev->mode.fb_id = fb_id; + + ret = drmModeAtomicAddProperty(req, plane_id, fb_obj_id, fb_id); + if (ret < 0) + return ret; + } + + return 0; +} + +static void atomic_modeset(struct device *dev, drmModeAtomicReqPtr req, + struct resources *res) +{ + uint32_t flags = 0; + + drmModeAtomicCleanup(req); + + allocate_fbs(dev, req, res); + + drmModeAtomicCommit(dev->fd, req, flags, NULL); +} + +static char optstr[] = "acdD:efM:P:ps:Cvw:W:";
int main(int argc, char **argv) { @@ -1504,6 +1657,7 @@ int main(int argc, char **argv) struct pipe_arg *pipe_args = NULL; struct plane_arg *plane_args = NULL; struct property_arg *prop_args = NULL; + drmModeAtomicReqPtr req = NULL; unsigned int args = 0; int ret;
@@ -1514,6 +1668,11 @@ int main(int argc, char **argv) args++;
switch (c) { + case 'a': + if (!req) + req = drmModeAtomicAlloc(); + args--; + break; case 'c': connectors = 1; break; @@ -1587,6 +1746,10 @@ int main(int argc, char **argv)
prop_count++; break; + case 'W': + if (!req || parse_atomic_args(req, optarg) < 0) + usage(argv[0]); + break; default: usage(argv[0]); break; @@ -1635,6 +1798,9 @@ int main(int argc, char **argv) return -1; }
+ if (req) + drmSetClientCap(dev.fd, DRM_CLIENT_CAP_ATOMIC, 1); + dev.resources = get_resources(&dev); if (!dev.resources) { drmClose(dev.fd); @@ -1649,46 +1815,51 @@ int main(int argc, char **argv) dump_resource(&dev, planes); dump_resource(&dev, framebuffers);
- for (i = 0; i < prop_count; ++i) - set_property(&dev, &prop_args[i]); + if (req) { + atomic_modeset(&dev, req, dev.resources);
- if (count || plane_count) { - uint64_t cap = 0; + getchar(); + } else { + for (i = 0; i < prop_count; ++i) + set_property(&dev, &prop_args[i]);
- ret = drmGetCap(dev.fd, DRM_CAP_DUMB_BUFFER, &cap); - if (ret || cap == 0) { - fprintf(stderr, "driver doesn't support the dumb buffer API\n"); - return 1; - } + if (count || plane_count) { + uint64_t cap = 0;
- if (count) - set_mode(&dev, pipe_args, count); + ret = drmGetCap(dev.fd, DRM_CAP_DUMB_BUFFER, &cap); + if (ret || cap == 0) { + fprintf(stderr, "driver doesn't support the dumb buffer API\n"); + return 1; + }
- if (plane_count) - set_planes(&dev, plane_args, plane_count); + if (count) + set_mode(&dev, pipe_args, count);
- if (test_cursor) - set_cursors(&dev, pipe_args, count); + if (plane_count) + set_planes(&dev, plane_args, plane_count);
- if (test_vsync) - test_page_flip(&dev, pipe_args, count); + if (test_cursor) + set_cursors(&dev, pipe_args, count);
- if (drop_master) - drmDropMaster(dev.fd); + if (test_vsync) + test_page_flip(&dev, pipe_args, count);
- getchar(); + if (drop_master) + drmDropMaster(dev.fd);
- if (test_cursor) - clear_cursors(&dev); + getchar();
- if (plane_count) - clear_planes(&dev, plane_args, plane_count); + if (test_cursor) + clear_cursors(&dev);
- if (count) - clear_mode(&dev); + if (plane_count) + clear_planes(&dev, plane_args, plane_count); + } }
+ clear_mode(&dev); free_resources(dev.resources); + drmFree(req);
return 0; }
This patch adds support for atomic page flip. User can specify -W option with the plane id for testing atomic page flipping.
Signed-off-by: Hyungwon Hwang human.hwang@samsung.com --- tests/modetest/modetest.c | 150 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 147 insertions(+), 3 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index bf9222d..d1b7dc4 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -719,6 +719,10 @@ struct pipe_arg { struct timeval start;
int swap_count; + + /* for atomic modeset */ + uint32_t plane_id; + uint32_t fb_obj_id; };
struct plane_arg { @@ -1446,6 +1450,7 @@ static void usage(char *name) fprintf(stderr, "\n Atomic Test options: [A]\n\n"); fprintf(stderr, "\t-a\tuse atomic modeset\n"); fprintf(stderr, "\t-W <obj_id>:<prop_name>:<value>\tset property\n"); + fprintf(stderr, "\t-V <flipping_plane_id>\ttest vsynced page flipping\n");
fprintf(stderr, "\n Legacy test options: [L]\n\n"); fprintf(stderr, "\t-P <crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n"); @@ -1637,7 +1642,137 @@ static void atomic_modeset(struct device *dev, drmModeAtomicReqPtr req, drmModeAtomicCommit(dev->fd, req, flags, NULL); }
-static char optstr[] = "acdD:efM:P:ps:Cvw:W:"; +static void +atomic_page_flip_handler(int fd, unsigned int frame, + unsigned int sec, unsigned int usec, void *data) +{ + static drmModeAtomicReqPtr req = NULL; + unsigned int new_fb_id; + struct timeval end; + struct pipe_arg *pipe; + double t; + uint32_t flags = 0; + + pipe = (struct pipe_arg *)(unsigned long)data; + + if (pipe->current_fb_id == pipe->fb_id[0]) + new_fb_id = pipe->fb_id[1]; + else + new_fb_id = pipe->fb_id[0]; + + pipe->current_fb_id = new_fb_id; + pipe->swap_count++; + + req = drmModeAtomicAlloc(); + + drmModeAtomicAddProperty(req, pipe->plane_id, pipe->fb_obj_id, new_fb_id); + + flags = DRM_MODE_PAGE_FLIP_EVENT; + drmModeAtomicCommit(fd, req, flags, pipe); + + if (pipe->swap_count == 60) { + gettimeofday(&end, NULL); + t = end.tv_sec + end.tv_usec * 1e-6 - + (pipe->start.tv_sec + pipe->start.tv_usec * 1e-6); + fprintf(stderr, "freq: %.02fHz\n", pipe->swap_count / t); + pipe->swap_count = 0; + pipe->start = end; + } + + drmModeAtomicFree(req); +} + +static int atomic_test_page_flip(struct device *dev, drmModeAtomicReqPtr req, + struct resources *res, unsigned int plane_id) +{ + struct bo *other_bo; + unsigned int other_fb_id; + struct pipe_arg *pipe = NULL; + drmEventContext evctx; + uint32_t flags = 0, fb_obj_id = 0, pixel_format; + uint64_t width, height; + int ret; + + if (!is_atomic_obj_in_request(req, plane_id)) + return -1; + + fb_obj_id = get_atomic_plane_prop_id(res, plane_id, "FB_ID"); + if (!fb_obj_id) + return -1; + + width = get_atomic_plane_prop_value_in_request(req, res, + plane_id, "SRC_W"); + height = get_atomic_plane_prop_value_in_request(req, res, + plane_id, "SRC_H"); + pixel_format = DRM_FORMAT_XRGB8888; + + ret = allocate_fb(dev, req, res, width, height, pixel_format, + PATTERN_TILES, &other_bo, &other_fb_id); + if (ret < 0) + return ret; + + ret = drmModeAtomicAddProperty(req, plane_id, fb_obj_id, + other_fb_id); + if (ret < 0) + goto err; + + if (!fb_obj_id) { + fprintf(stderr, "page flipping requires at least one plane setting.\n"); + return -1; + } + + pipe = drmMalloc(sizeof *pipe); + + gettimeofday(&pipe->start, NULL); + pipe->swap_count = 0; + pipe->plane_id = plane_id; + pipe->fb_obj_id = fb_obj_id; + pipe->fb_id[0] = dev->mode.fb_id; + pipe->fb_id[1] = other_fb_id; + pipe->current_fb_id = other_fb_id; + + flags = DRM_MODE_PAGE_FLIP_EVENT; + + ret = drmModeAtomicCommit(dev->fd, req, flags, pipe); + if (ret < 0) + goto err_rmfb; + + memset(&evctx, 0, sizeof evctx); + evctx.version = DRM_EVENT_CONTEXT_VERSION; + evctx.vblank_handler = NULL; + evctx.page_flip_handler = atomic_page_flip_handler; + + while (1) { + struct timeval timeout = { .tv_sec = 3, .tv_usec = 0 }; + fd_set fds; + int ret; + + FD_ZERO(&fds); + FD_SET(0, &fds); + FD_SET(dev->fd, &fds); + ret = select(dev->fd + 1, &fds, NULL, NULL, &timeout); + + if (ret <= 0) { + fprintf(stderr, "select timed out or error (ret %d)\n", + ret); + continue; + } else if (FD_ISSET(0, &fds)) { + break; + } + + drmHandleEvent(dev->fd, &evctx); + } + +err_rmfb: + drmFree(pipe); + drmModeRmFB(dev->fd, other_fb_id); +err: + bo_destroy(other_bo); + + return 0; +} + +static char optstr[] = "acdD:efM:P:ps:CvV:w:W:";
int main(int argc, char **argv) { @@ -1651,7 +1786,7 @@ int main(int argc, char **argv) const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos", "tilcdc", "msm", "sti", "tegra", "imx-drm", "rockchip", "atmel-hlcdc" }; char *device = NULL; char *module = NULL; - unsigned int i; + unsigned int i, flip_plane_id = 0; int count = 0, plane_count = 0; unsigned int prop_count = 0; struct pipe_arg *pipe_args = NULL; @@ -1732,6 +1867,11 @@ int main(int argc, char **argv) case 'v': test_vsync = 1; break; + case 'V': + if (sscanf(optarg, "%u", &flip_plane_id) != 1) + usage(argv[0]); + test_vsync = 1; + break; case 'w': prop_args = realloc(prop_args, (prop_count + 1) * sizeof *prop_args); @@ -1788,7 +1928,7 @@ int main(int argc, char **argv) return -1; }
- if (test_vsync && !count) { + if (test_vsync && (!count && !req)) { fprintf(stderr, "page flipping requires at least one -s option.\n"); return -1; } @@ -1818,6 +1958,10 @@ int main(int argc, char **argv) if (req) { atomic_modeset(&dev, req, dev.resources);
+ if (test_vsync) + atomic_test_page_flip(&dev, req, dev.resources, + flip_plane_id); + getchar(); } else { for (i = 0; i < prop_count; ++i)
dri-devel@lists.freedesktop.org
-
Emil Velikov -
Hyungwon Hwang -
Pekka Paalanen