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