modetest: allocate and commit atomic request around set_property()
authorMarijn Suijten <marijn.suijten@somainline.org>
Tue, 13 Jun 2023 19:53:22 +0000 (21:53 +0200)
committerDmitry Baryshkov <dbaryshkov@gmail.com>
Sun, 10 Sep 2023 12:14:06 +0000 (12:14 +0000)
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 <marijn.suijten@somainline.org>
tests/modetest/modetest.c

index 4b45994..a7f69a7 100644 (file)
@@ -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,14 +2386,14 @@ 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");
+               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);
-               }
+               if (count && plane_count)
+                       atomic_clear_FB(&dev, plane_args, plane_count);
 
                drmModeAtomicFree(dev.req);
        } else {