Pandiyan, Dhinakaran schreef op ma 13-02-2017 om 22:48 [+0000]:
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@ffwll.ch Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@int el.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()?
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.
Yeah looks good. I think it makes sense to have such a validation function. There may not be much in it now but that could change when i915 connector properties are made atomic. :)
~Maarten