From dfd00c6250e51e7327a1af5311eacb374b9bc8fb Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Tue, 13 Jun 2023 21:53:22 +0200 Subject: [PATCH] modetest: allocate and commit atomic request around set_property() Currently the atomic request is only assigned after `set_property()` is called, leaving `dev.req` in its uninitialized state causing `drmModeAtomicAddProperty()` to return an error code, which is printed as `"Success"` because `errno` is not set by `libdrm` (but it would have been when non-atomic `drmModeObjectSetProperty()` called an IOCTL immediately): sony-akatsuki-row ~ $ modetest -M msm -a -w 81:ACTIVE:0 failed to set CRTC 81 property ACTIVE to 0: Success Solve this by assigning a new atomic request object before calling `set_property()`, when there are properties to set. Likewise, commit these properties after `set_property()` even if there is no other operation (setting modes or planes) specified. Furthermore `drmModeObjectSetProperty()` is implemented in terms of `DRM_IOCTL()` which already returns `-errno` when `ioctl()` returns `-1`, so we should instead pass `ret` to `strerror()` and get an accurate error string out of `drmModeAtomicAddProperty()` too. Fixes: 93220283 ("tests/modetest: Add atomic support") Signed-off-by: Marijn Suijten --- tests/modetest/modetest.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 4b45994e..a7f69a70 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -1073,7 +1073,7 @@ static bool set_property(struct device *dev, struct property_arg *p) if (ret < 0) fprintf(stderr, "failed to set %s %i property %s to %" PRIu64 ": %s\n", - obj_type, p->obj_id, p->name, p->value, strerror(errno)); + obj_type, p->obj_id, p->name, p->value, strerror(-ret)); return true; } @@ -2320,12 +2320,13 @@ int main(int argc, char **argv) dump_resource(&dev, planes); dump_resource(&dev, framebuffers); + if (dev.use_atomic) + dev.req = drmModeAtomicAlloc(); + for (i = 0; i < prop_count; ++i) set_property(&dev, &prop_args[i]); if (dev.use_atomic) { - dev.req = drmModeAtomicAlloc(); - if (set_preferred || (count && plane_count)) { uint64_t cap = 0; @@ -2385,15 +2386,15 @@ int main(int argc, char **argv) if (count) atomic_clear_mode(&dev, pipe_args, count); - - ret = drmModeAtomicCommit(dev.fd, dev.req, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL); - if (ret) - fprintf(stderr, "Atomic Commit failed\n"); - - if (plane_count) - atomic_clear_FB(&dev, plane_args, plane_count); } + ret = drmModeAtomicCommit(dev.fd, dev.req, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL); + if (ret) + fprintf(stderr, "Atomic Commit failed\n"); + + if (count && plane_count) + atomic_clear_FB(&dev, plane_args, plane_count); + drmModeAtomicFree(dev.req); } else { if (dump_path) {