[Intel-gfx] Atomic commit on planes with more than one possible CRTC's

[Leo]

Hello,


I'm looking for ideas and suggestions on how to enable IGT to work on
planes with multiple possible_crtcs during atomic commit. Amdgpu
currently has features that rely on this, and planes with multiple
crtc's are not being properly committed when tests run an atomic commit.


To explain, DRM has a field on planes called "possible_crtcs" that
allows a plane to be valid for multiple CRTC's. In igt_display_init(),
it is used to determine whether a igt_plane should be attached to a
pipe. Therefore, when a plane specifies a possible_crtcs of say, 0xFF,
igt_display_init() will create *copies* of this plane and attach it to
all available pipes.

The problem arises when committing changes via igt_atomic_commit (which
eventually calls drmModeAtomicCommit). Say we have two pipes (A and
B) and a single plane with possible_crtcs=0xFF. igt_display_init() will
create copies of this plane and store it within both A and B, as 0xFF
makes it a valid plane for both pipes. Keyword here is "copies", since a
change to A.plane will not be reflected in B.plane.

For example, say we call igt_plane_set_fb() using A.plane, changing its
fb_id. After this call, A.plane.fb_id will be updated, and B.plane.fb_id
will remain stale. We then call igt_atomic_commit() to commit the fb
changes. When preparing the set of properties to commit, the updated
fb_id from pipe A will first be set, followed by the stale fb_id from
pipe B. DRM then removes duplicates from the property set before calling
the atomic commit ioctl. Since the algorithm keeps the duplicate item
with the largest index, the update to A.plane gets overridden by
B.plane's stale fb_id property, causing unexpected behavior.


One possible solution is to have igt_pipe_t use an array of pointers to
igt_plane_t, and have the set of unique igt_plane_t's stored elsewhere
(perhaps under igt_display_t). Changes to A.plane will then be properly
reflected on B.plane since it is by reference. However,
igt_display_init() will have to be modified, and igt_plane_t's pipe and
index fields will have a different meaning.

Another possibility is to introduce a field within igt_plane_t to
reference the "one true copy". Any read or write to the plane's fields
will then have to first check if it is a copy, and reference the one
true copy in the event that it is. This can get quite messy.


The modifications seem to be quite wide, and I'd like some input on this
before I jump in. Any ideas and/or suggestions will be very much
appreciated!


Thanks,
Leo