Hi Daniel, Thanks for your review On Fri, Nov 13, 2020 at 10:02:40PM +0100, Daniel Vetter wrote: > > + * is not used by the atomic helpers. > > + * > > + * This function is called at the end of > > + * drm_atomic_helper_setup_commit(), so once the commit has been > > + * properly setup across the generic DRM object states. It allows > > + * drivers to do some additional commit tracking that isn't related to a > > + * CRTC, plane or connector, typically a private object. > > + * > > + * This hook is optional. > > + */ > > + int (*atomic_commit_setup)(struct drm_atomic_state *state); > > I think the kerneldoc for drm_private_obj should also explain when it is > necessary to use this, and when not: > > - when the private state is a tied to an existing drm object (drm_crtc, > drm_plane or drm_crtc) and never moves, then sufficient synchronization > is already guaranteed by that superior object. This could even hold > when the private object can be e.g. reassigned between planes, but > always stays on the same crtc. > > - if the private state object can be globally reassigned, then > drm_crtc_commit synchronization points need to be set up in > ->atomic_commit_setup and waited on as the first step in > ->atomic_commit_tail Does the comment added on patch 2 sufficient for you, or would you extend it / make it clearer? Maxime
Attachment:
signature.asc
Description: PGP signature
