Hi Kieran,
On Mon, Mar 22, 2021 at 04:35:34PM +0000, Kieran Bingham wrote:
Create rcar_du_group_atomic_check() and rcar_du_group_atomic_setup() functions to track and apply group state through the DRM atomic state. The use_count field is moved from the rcar_du_group structure to an enabled field in the rcar_du_group_state structure.
This allows separating group setup from the configuration of the CRTCs that are part of the group, simplifying the CRTC code and improving overall readability. The existing rcar_du_group_{get,put}() functions are now redundant and removed.
The groups share clocking with the CRTCs within the group, and so are accessible only when one of its CRTCs has been powered through rcar_du_crtc_atomic_exit_standby().
Reviewed-by: Ulrich Hecht uli+renesas@fpond.eu Signed-off-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
It's a bit weird to have both your and Laurent's SoB without a Co-Developped-By or an authorship from him.
However, using a drm_private_obj shared between CRTC has a gotcha: you don't have any ordering guarantee between commits if they affect different CRTCs (and they are non-blocking).
Let's assume we have two subsequent commits, commit1 and commit2, both non-blocking, and affecting different CRTC, plane and connectors. In this case, commit1 old private state will be commit0 new private state, and commit 2 old private state will be commit1 new private state.
If commit2 is executed before commit1, then it will free its old state when done with it (so commit1 new private state), and will thus result in an use-after-free when commit1 is ran.
In order to fix this, you should store (and get a reference to) the drm_crtc_commit in your private state in atomic_commit_setup, and call drm_crtc_commit_wait on that commit as the first part of your commit_tail to serialize those commits.
Maxime