| Message ID | 1486622291-3524-8-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dhinakaran Pandiyan schreef op wo 08-02-2017 om 22:38 [-0800]: > Having a ->atomic_release callback is useful to release shared > resources > that get allocated in compute_config(). This function is expected to > be > called in the atomic_check() phase before new resources are acquired. > > v2: Moved the caller hunk to this patch (Daniel) > > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++ > include/drm/drm_modeset_helper_vtables.h | 13 +++++++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 8795088..92bd741 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -576,6 +576,25 @@ drm_atomic_helper_check_modeset(struct > drm_device *dev, > } > } > > + for_each_connector_in_state(state, connector, > connector_state, i) { > + const struct drm_connector_helper_funcs *conn_funcs; > + struct drm_crtc_state *crtc_state; > + > + conn_funcs = connector->helper_private; > + if (!conn_funcs->atomic_release) > + continue; > + > + if (!connector->state->crtc) > + continue; > + > + crtc_state = > drm_atomic_get_existing_crtc_state(state, connector->state->crtc); > + > + if (crtc_state->connectors_changed || > + crtc_state->mode_changed || > + (crtc_state->active_changed && !crtc_state- > >active)) > + conn_funcs->atomic_release(connector, > connector_state); > + } Could we deal with the VCPI state separately in intel_modeset_checks, like we do with dpll? Maybe implementing the relevant VCPI state could be done as an atomic helper function too, so other atomic drivers can just plug it in. Not sure how doable this is, but if it's not too hard, then it's probably cleaner :)
On Thu, 2017-02-09 at 09:01 +0000, Lankhorst, Maarten wrote: > Dhinakaran Pandiyan schreef op wo 08-02-2017 om 22:38 [-0800]: > > Having a ->atomic_release callback is useful to release shared > > resources > > that get allocated in compute_config(). This function is expected to > > be > > called in the atomic_check() phase before new resources are acquired. > > > > v2: Moved the caller hunk to this patch (Daniel) > > > > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++ > > include/drm/drm_modeset_helper_vtables.h | 13 +++++++++++++ > > 2 files changed, 32 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index 8795088..92bd741 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -576,6 +576,25 @@ drm_atomic_helper_check_modeset(struct > > drm_device *dev, > > } > > } > > > > + for_each_connector_in_state(state, connector, > > connector_state, i) { > > + const struct drm_connector_helper_funcs *conn_funcs; > > + struct drm_crtc_state *crtc_state; > > + > > + conn_funcs = connector->helper_private; > > + if (!conn_funcs->atomic_release) > > + continue; > > + > > + if (!connector->state->crtc) > > + continue; > > + > > + crtc_state = > > drm_atomic_get_existing_crtc_state(state, connector->state->crtc); > > + > > + if (crtc_state->connectors_changed || > > + crtc_state->mode_changed || > > + (crtc_state->active_changed && !crtc_state- > > >active)) > > + conn_funcs->atomic_release(connector, > > connector_state); > > + } > > Could we deal with the VCPI state separately in intel_modeset_checks, > like we do with dpll? We'd want to release the VCPI slots before they are acquired in ->compute_config(). intel_modeset_checks() will be too late to release them. Are you suggesting both acquiring and releasing slots should be done in intel_modeset_checks()? > > Maybe implementing the relevant VCPI state could be done as an atomic > helper function too, so other atomic drivers can just plug it in. > The idea was to reduce boilerplate in the drivers and use the private_obj state for different object types. > Not sure how doable this is, but if it's not too hard, then it's > probably cleaner :) > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 8795088..92bd741 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -576,6 +576,25 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, } } + for_each_connector_in_state(state, connector, connector_state, i) { + const struct drm_connector_helper_funcs *conn_funcs; + struct drm_crtc_state *crtc_state; + + conn_funcs = connector->helper_private; + if (!conn_funcs->atomic_release) + continue; + + if (!connector->state->crtc) + continue; + + crtc_state = drm_atomic_get_existing_crtc_state(state, connector->state->crtc); + + if (crtc_state->connectors_changed || + crtc_state->mode_changed || + (crtc_state->active_changed && !crtc_state->active)) + conn_funcs->atomic_release(connector, connector_state); + } + return mode_fixup(state); } EXPORT_SYMBOL(drm_atomic_helper_check_modeset); diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 091c422..394ec0c 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -836,6 +836,19 @@ struct drm_connector_helper_funcs { */ struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector, struct drm_connector_state *connector_state); + + /** + * @atomic_release: + * + * This function is used to release shared resources that were + * previously acquired. + * + * NOTE: + * + * This function is called in the check phase of an atomic update. + */ + void (*atomic_release)(struct drm_connector *connector, + struct drm_connector_state *connector_state); }; /**
Having a ->atomic_release callback is useful to release shared resources that get allocated in compute_config(). This function is expected to be called in the atomic_check() phase before new resources are acquired. v2: Moved the caller hunk to this patch (Daniel) Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++ include/drm/drm_modeset_helper_vtables.h | 13 +++++++++++++ 2 files changed, 32 insertions(+)