Hi Algea, Thank you for the patch. On Wed, Aug 12, 2020 at 04:34:07PM +0800, Algea Cao wrote: > In some situations, connector should get some work done > when plane is updating. Such as when change output color > format, hdmi should send AVMUTE to make screen black before > crtc updating color format, or screen may flash. After color > updating, hdmi should clear AVMUTE bring screen back to normal. > > The process is as follows: > AVMUTE -> Update CRTC -> Update HDMI -> Clear AVMUTE > > So we introduce connector atomic_begin/atomic_flush. Implementing this through .atomic_begin() and .atomic_flush() seems like a pretty big hack to me. Furthermore, I think this should be implemented as bridge operations, not at the connector level, as the HDMI encoder may not be the component that implements the drm_connector. > Signed-off-by: Algea Cao <algea.cao@xxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 46 ++++++++++++++++++++++++ > include/drm/drm_modeset_helper_vtables.h | 19 ++++++++++ > 2 files changed, 65 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index f68c69a45752..f4abd700d2c4 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2471,6 +2471,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, > struct drm_atomic_state *old_state, > uint32_t flags) > { > + struct drm_connector *connector; > + struct drm_connector_state *old_connector_state, *new_connector_state; > struct drm_crtc *crtc; > struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct drm_plane *plane; > @@ -2479,6 +2481,28 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, > bool active_only = flags & DRM_PLANE_COMMIT_ACTIVE_ONLY; > bool no_disable = flags & DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET; > > + for_each_oldnew_connector_in_state(old_state, connector, > + old_connector_state, > + new_connector_state, i) { > + const struct drm_connector_helper_funcs *funcs; > + > + if (!connector->state->crtc) > + continue; > + > + if (!connector->state->crtc->state->active) > + continue; > + > + funcs = connector->helper_private; > + > + if (!funcs || !funcs->atomic_begin) > + continue; > + > + DRM_DEBUG_ATOMIC("flush beginning [CONNECTOR:%d:%s]\n", > + connector->base.id, connector->name); > + > + funcs->atomic_begin(connector, old_connector_state); > + } > + > for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { > const struct drm_crtc_helper_funcs *funcs; > > @@ -2550,6 +2574,28 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, > > funcs->atomic_flush(crtc, old_crtc_state); > } > + > + for_each_oldnew_connector_in_state(old_state, connector, > + old_connector_state, > + new_connector_state, i) { > + const struct drm_connector_helper_funcs *funcs; > + > + if (!connector->state->crtc) > + continue; > + > + if (!connector->state->crtc->state->active) > + continue; > + > + funcs = connector->helper_private; > + > + if (!funcs || !funcs->atomic_flush) > + continue; > + > + DRM_DEBUG_ATOMIC("flushing [CONNECTOR:%d:%s]\n", > + connector->base.id, connector->name); > + > + funcs->atomic_flush(connector, old_connector_state); > + } > } > EXPORT_SYMBOL(drm_atomic_helper_commit_planes); > > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index 421a30f08463..10f3f2e2fe28 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -1075,6 +1075,25 @@ struct drm_connector_helper_funcs { > void (*atomic_commit)(struct drm_connector *connector, > struct drm_connector_state *state); > > + /** > + * @atomic_begin: > + * > + * flush atomic update > + * > + * This callback is used by the atomic modeset helpers but it is optional. > + */ > + void (*atomic_begin)(struct drm_connector *connector, > + struct drm_connector_state *state); > + > + /** > + * @atomic_begin: > + * > + * begin atomic update > + * > + * This callback is used by the atomic modeset helpers but it is optional. > + */ > + void (*atomic_flush)(struct drm_connector *connector, > + struct drm_connector_state *state); > /** > * @prepare_writeback_job: > * -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel