On Mon, 2017-02-13 at 21:26 +0000, Pandiyan, Dhinakaran wrote: > On Mon, 2017-02-13 at 09:05 +0000, Lankhorst, Maarten wrote: > > Pandiyan, Dhinakaran schreef op do 09-02-2017 om 18:55 [+0000]: > > > 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@xxxxxxxx> > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx > > > > > > > > > > > --- > > > > > 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()? > > > > That makes things a bit more nasty. Maybe add a > > conn_funcs->atomic_check that always gets called, something like I did > > below? > > > > I'd love to use it for some atomic connector properties too. > > > Adding and unconditionally calling conn_funcs->atomic_check() should be > doable. It also follows the pattern we have for encoders and CRTCs. But > I'll have to move the connector->state->crtc state checks inside the > function. > > -DK This is what I mean -https://pastebin.ubuntu.com/23991405/ But, I do have one concern with calling this conn_func->atomic_check(). We are not validating the new connector_state like atomic_check() seems to do generally but only cleaning up vcpi resources for compute_config() to later acquire. Let me know if I am wrong in my understanding what atomic_check() is expected to do. -DK > > > > > > > > > > > > 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@xxxxxxxxxxxxxxxxxxxxx > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel