On Wed, Nov 16, 2016 at 02:58:04PM +0100, Maarten Lankhorst wrote:
Second approach. Instead of trying to convert all drivers straight away, implement all macros that are required to get state working.
Current situation: Use obj->state, which can refer to old or new state. Use drm_atomic_get_(existing_)obj_state, which can refer to new or old state. Use for_each_obj_in_state, which refers to new or old state.
New situation: When doing some dereferencing outside atomic_state, use drm_atomic_get_current_obj_state which has locking checks, instead of obj->state.
During atomic check:
- Use drm_atomic_get_obj_state to add a object to the atomic state, or get the new state.
- Use drm_atomic_get_(old/new)_obj_state to peek at the new/old state, without adding the object if it's not part of the state. For planes and connectors the relevant crtc_state is added, so this will work to get the crtc_state from foo_state->crtc too, saves some error handling. :)
Hm, this needs to check looking, somehow. Otherwise everyone just randomly peeks at state and all hell breaks loose once more. Or why do you want to avoid adding state for CRTCs?
During atomic commit:
- Do not use drm_atomic_get_obj_state, obj->state or drm_atomic_get_(existing_)obj_state any more, replace with drm_atomic_get_old/new_obj_state calls as required.
Wild idea, can we enforce this? E.g. with a drm_mode_config->in_atomic_check atomic counter that we inc/dec around the atomic_check call, and then a WARN_ON(!dev->mode_config.in_atomic_check); It will have some false positives when concurrent atomic commits happen, but for testing it should catch all offenders.
Of course this won't catch obj->state access, but we can fix that by renaming to obj->_state ...
During both:
- Use for_each_(new,old,oldnew)_obj_in_state to get the old or new state as needed. oldnew will be renamed to for_each_obj_in_state after all callers are converted to the new api.
This will give the correct state regardless of swapping.
Otherwise sounds like a reasonable plan I think. -Daniel
Maarten Lankhorst (4): drm/atomic: Add new iterators over all state, v2. drm/atomic: Add accessor macros for the current state. drm/atomic: Add macros to access existing old/new state drm/atomic: Add checks to ensure get_state is not called after swapping.
drivers/gpu/drm/drm_atomic.c | 24 +++- drivers/gpu/drm/drm_atomic_helper.c | 47 ++++++- drivers/gpu/drm/i915/intel_display.c | 11 +- include/drm/drm_atomic.h | 247 ++++++++++++++++++++++++++++++++++- include/drm/drm_atomic_helper.h | 2 + include/drm/drm_modeset_lock.h | 21 +++ 6 files changed, 338 insertions(+), 14 deletions(-)
-- 2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel