drm-lease: sharing planes between DRM lessees

[wxiaowen-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org]

There is a use case to share h/w pipe resources between lessee masters. 
Two planes are created and allocated to two masters, but internally they 
share the same h/w pipe:

masterA          masterB
planeA           planeB
   |                |
   \--- same H/W ---/

A seamless handoff between the shared plane is also required, that in 
above diagram if planeB is staged and committed, planeA should be 
unstaged at the same time to handoff h/w pipe to planeB. The idea is to 
maximize the use of h/w pipe, so that if plane is not used in one 
master, it can be reused in another master. This idea is similar to 
universal plane used between crtcs, but this time we need to move it to 
crtc in another master.

### Option 1 ###:
Based on the requirement, we planned to add a shared plane check in 
drm_atomic_helper_check_modeset(), to add affected shared planes from 
other lessees and unstage them in the same commit:

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index f7eccab..d0eb578 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -516,6 +516,40 @@ static enum drm_mode_status mode_valid_path(struct 
drm_connector *connector,
         return 0;
}

+static int
+update_shared_planes(struct drm_device *dev, struct drm_atomic_state 
*state)
+{
+       struct drm_plane *plane;
+       struct drm_plane_state *plane_state;
+       uint32_t plane_mask = 0;
+       int i, ret;
+
+       for_each_plane_in_state(state, plane, plane_state, i) {
+               if (plane->type == DRM_PLANE_TYPE_SHARED && 
plane_state->crtc)
+                       plane_mask |= (1 << drm_plane_index(plane));
+       }
+
+       drm_for_each_sibling_plane_mask(plane, dev, plane_mask) {
+               /*
+                * sibling planes should not appear in plane_mask, 
otherwise
+                * we have multiple sibling planes staged in one commit
+                */
+               if (plane_mask & (1 << drm_plane_index(plane)))
+                       return -EINVAL;
+
+               plane_state = drm_atomic_get_plane_state(state, plane);
+               if (IS_ERR(plane_state))
+                       return PTR_ERR(plane_state);
+
+               drm_atomic_set_fb_for_plane(plane_state, NULL);
+               ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
+               if (ret)
+                       return ret;
+       }
+
+       return 0;
+}
+
/**
   * drm_atomic_helper_check_modeset - validate state object for modeset 
changes
   * @dev: DRM device
@@ -570,6 +604,10 @@ static enum drm_mode_status mode_valid_path(struct 
drm_connector *connector,
         int i, ret;
         unsigned connectors_mask = 0;

+       ret = update_shared_planes(dev, state)
+       if (ret)
+               return ret;
+
         for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
                 bool has_connectors =
                         !!new_crtc_state->connector_mask;

We have a question for above update_shared_planes function, that can 
kernel modify affected plane/crtc states implicitly for shared planes 
from another lessee? This created an inter-access-lease-master behavior 
and we're not sure if it's allowed. Please note that if this method of 
accessing is acceptable, we can maintain the change at the vendor level 
subdriver and above drm_atomic_helper_check_modeset changes can be 
avoided.


### Option2 ###:
Another approach is to hide hardware ownership inside vendor's 
subdriver. In this approach, there are no DRM state changes during 
handoff, only the validation in vendor subdriver will fail, if two 
commits from different masters tried to stage the planes with the same 
h/w pipe.

This approach doesn't use DRM framework to update the plane states. 
Detaching old plane and attaching new plane will be in two different 
atomic commits from two masters so no inter-access-lease-master problem 
here, but this also resulting DRM state not reflecting the actual h/w 
pipe status during handoff commit, and vendor subdriver needs to 
check/update more internal states than the atomic state in the commit.


Thanks,
Xiaowen Wu
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno