Skip to content
Snippets Groups Projects

tests: Serialise drm-smoke-test against everything else

Merged Daniel Stone requested to merge daniels/weston:drm-test-serialise into main
1 unresolved thread

drm-smoke-test can't run at the same time as anything else which touches DRM devices. This includes any test which would use the GL renderer or GL/GBM on the client side, since they will open DRM devices to probe them at init time.

Signed-off-by: Daniel Stone daniels@collabora.com

cc @mvlad @leandrohrb

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Reviewed-by: Marius Vlad marius.vlad@collabora.com

  • Marius Vlad approved this merge request

    approved this merge request

  • awesome, thanks Marius! going to merge this early so we can unbreak CI

  • merged

    • Could we not have just ensured that we get the Mesa software renderer instead?

      That's what we literally want, so would it not have stopped Mesa from opening DRM devices?

    • If we ever have any test which tries to use KMS apart from drm-smoke, it will need to be serialised against drm-smoke anyway.

      Using MESA_LOADER_DRIVER_OVERRIDE=swrast does give you llvmpipe driver (in its capacity as 'swrast'), but it does still open DRM devices anyway. I just remembered LIBGL_ALWAYS_SOFTWARE=1 and that does appear to avoid the DRM probe completely, but on the other hand that's just an implementation detail.

      So sure, we can force that in all our environments as well, but I don't think the serialisation is at all incorrect.

    • That serialization is already implemented in https://gitlab.freedesktop.org/wayland/weston/-/blob/main/tests/weston-test-fixture-compositor.c#L125 wait_for_lock() so that we do not need to encode test-spefici details in meson build files.

    • Thanks, I didn't know that. Up to you if you just want to revert this and replace it with LIBGL_ALWAYS_SOFTWARE=1, and hope that no Mesa changes introduce any device probing.

    • Right, so the lock is a file lock, checking if there's not other DRM-backend test, running, which clearly there isn't. The lock is taken, but it still fails to bring/force probe the connector which the outcome is clear, we don't have an output.

      --- Fixture 1 ()...
      Waiting for lock on /tmp/tests/weston-test-suite-drm-lock...
      Lock /tmp/tests/weston-test-suite-drm-lock acquired.
      ...
      
      test-client: got seat 0x607000010320 name: 'test-seat'
      test-drm-smoke: ../tests/weston-test-client-helper.c:977: create_client: Assertion `client->output' failed.

      I suppose this lock should instead try (or maybe additional to that file lock) to perform any setMaster operations to make sure it's the only one running or something similar to that? Would that be potential follow-up to consider?

      Edited by Marius Vlad
    • One reason for the lock file is to not prevent other tests running at the same time. We have many tests that do not use GL-renderer at all.

      Yes, naturally the lock file is not taken with headless-backend so it cannot help here. I was just saying that we do not need to deny parallelism completely due to adding more DRM-backend tests in the future.

      Since it works now, that's good enough. We can revisit this once something breaks again.

      I suppose this lock should instead try (or maybe additional to that file lock) to perform any setMaster operations to make sure it's the only one running or something similar to that? Would that be potential follow-up to consider?

      No, that would just cause the same problem. The seatd daemon runs outside of this.

    • Yes, naturally the lock file is not taken with headless-backend so it cannot help here. I was just saying that we do not need to deny parallelism completely due to adding more DRM-backend tests in the future.

      Pff, I misunderstood then, thought you meant amending it, thanks for clarifying.

      Edited by Marius Vlad
    • Please register or sign in to reply
Please register or sign in to reply
Loading