| Message ID | 1492753893-3748-2-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21-04-17 07:51, Dhinakaran Pandiyan wrote: > From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> > > It is necessary to track states for objects other than connector, crtc > and plane for atomic modesets. But adding objects like DP MST link > bandwidth to drm_atomic_state would mean that a non-core object will be > modified by the core helper functions for swapping and clearing > it's state. So, lets add void * objects and helper functions that operate > on void * types to keep these objects and states private to the core. > Drivers can then implement specific functions to swap and clear states. > The other advantage having just void * for these objects in > drm_atomic_state is that objects of different types can be managed in the > same state array. > > v6: More kernel-doc to keep 0-day happy > v5: Remove more NULL checks (Maarten) > v4: Avoid redundant NULL checks when private_objs array is empty (Maarten) > v3: Macro alignment (Chris) > v2: Added docs and new iterator to filter private objects (Daniel) > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Archit Taneja <architt@codeaurora.org> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Harry Wentland <Harry.wentland@amd.com> > > Acked-by: Harry Wentland <harry.wentland@amd.com> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/drm_atomic.c | 65 +++++++++++++++++++++++ > drivers/gpu/drm/drm_atomic_helper.c | 5 ++ > include/drm/drm_atomic.h | 101 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 171 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 30229ab..8e5f3eb 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -57,6 +57,7 @@ void drm_atomic_state_default_release(struct drm_atomic_state *state) > kfree(state->connectors); > kfree(state->crtcs); > kfree(state->planes); > + kfree(state->private_objs); > } > EXPORT_SYMBOL(drm_atomic_state_default_release); > > @@ -184,6 +185,17 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) > state->planes[i].ptr = NULL; > state->planes[i].state = NULL; > } > + > + for (i = 0; i < state->num_private_objs; i++) { > + void *obj_state = state->private_objs[i].obj_state; > + > + state->private_objs[i].funcs->destroy_state(obj_state); > + state->private_objs[i].obj = NULL; > + state->private_objs[i].obj_state = NULL; > + state->private_objs[i].funcs = NULL; > + } > + state->num_private_objs = 0; > + > } > EXPORT_SYMBOL(drm_atomic_state_default_clear); > > @@ -978,6 +990,59 @@ static void drm_atomic_plane_print_state(struct drm_printer *p, > } > > /** > + * drm_atomic_get_private_obj_state - get private object state > + * @state: global atomic state > + * @obj: private object to get the state for > + * @funcs: pointer to the struct of function pointers that identify the object > + * type > + * > + * This function returns the private object state for the given private object, > + * allocating the state if needed. It does not grab any locks as the caller is > + * expected to care of any required locking. > + * > + * RETURNS: > + * > + * Either the allocated state or the error code encoded into a pointer. > + */ > +void * > +drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj, > + const struct drm_private_state_funcs *funcs) > +{ > + int index, num_objs, i; > + size_t size; > + struct __drm_private_objs_state *arr; > + > + for (i = 0; i < state->num_private_objs; i++) > + if (obj == state->private_objs[i].obj && > + state->private_objs[i].obj_state) > + return state->private_objs[i].obj_state; > + > + num_objs = state->num_private_objs + 1; > + size = sizeof(*state->private_objs) * num_objs; > + arr = krealloc(state->private_objs, size, GFP_KERNEL); > + if (!arr) > + return ERR_PTR(-ENOMEM); > + > + state->private_objs = arr; > + index = state->num_private_objs; > + memset(&state->private_objs[index], 0, sizeof(*state->private_objs)); > + > + state->private_objs[index].obj_state = funcs->duplicate_state(state, obj); > + if (!state->private_objs[index].obj_state) > + return ERR_PTR(-ENOMEM); > + > + state->private_objs[index].obj = obj; > + state->private_objs[index].funcs = funcs; > + state->num_private_objs = num_objs; > + > + DRM_DEBUG_ATOMIC("Added new private object state %p to %p\n", > + state->private_objs[index].obj_state, state); > + > + return state->private_objs[index].obj_state; > +} > +EXPORT_SYMBOL(drm_atomic_get_private_obj_state); > + > +/** > * drm_atomic_get_connector_state - get connector state > * @state: global atomic state object > * @connector: connector to get state object for > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index cfeda5f..cce05fb 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2032,6 +2032,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, > struct drm_plane *plane; > struct drm_plane_state *old_plane_state, *new_plane_state; > struct drm_crtc_commit *commit; > + void *obj, *obj_state; > + const struct drm_private_state_funcs *funcs; > > if (stall) { > for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { > @@ -2092,6 +2094,9 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, > state->planes[i].state = old_plane_state; > plane->state = new_plane_state; > } > + > + __for_each_private_obj(state, obj, obj_state, i, funcs) > + funcs->swap_state(obj, &state->private_objs[i].obj_state); > } > EXPORT_SYMBOL(drm_atomic_helper_swap_state); > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 788daf7..285b3a3 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -155,6 +155,53 @@ struct __drm_connnectors_state { > }; > > /** > + * struct drm_private_state_funcs - atomic state functions for private objects > + * > + * These hooks are used by atomic helpers to create, swap and destroy states of > + * private objects. The structure itself is used as a vtable to identify the > + * associated private object type. Each private object type that needs to be > + * added to the atomic states is expected to have an implementation of these > + * hooks and pass a pointer to it's drm_private_state_funcs struct to > + * drm_atomic_get_private_obj_state(). > + */ > +struct drm_private_state_funcs { > + /** > + * @duplicate_state: > + * > + * Duplicate the current state of the private object and return it. It > + * is an error to call this before obj->state has been initialized. > + * > + * RETURNS: > + * > + * Duplicated atomic state or NULL when obj->state is not > + * initialized or allocation failed. > + */ > + void *(*duplicate_state)(struct drm_atomic_state *state, void *obj); > + > + /** > + * @swap_state: > + * > + * This function swaps the existing state of a private object @obj with > + * it's newly created state, the pointer to which is passed as > + * @obj_state_ptr. > + */ > + void (*swap_state)(void *obj, void **obj_state_ptr); > + > + /** > + * @destroy_state: > + * > + * Frees the private object state created with @duplicate_state. > + */ > + void (*destroy_state)(void *obj_state); > +}; > + > +struct __drm_private_objs_state { > + void *obj; > + void *obj_state; > + const struct drm_private_state_funcs *funcs; > +}; > + > +/** > * struct drm_atomic_state - the global state object for atomic updates > * @ref: count of all references to this state (will not be freed until zero) > * @dev: parent DRM device > @@ -164,6 +211,8 @@ struct __drm_connnectors_state { > * @crtcs: pointer to array of CRTC pointers > * @num_connector: size of the @connectors and @connector_states arrays > * @connectors: pointer to array of structures with per-connector data > + * @num_private_objs: size of the @private_objs array > + * @private_objs: pointer to array of private object pointers > * @acquire_ctx: acquire context for this atomic modeset state update > */ > struct drm_atomic_state { > @@ -176,6 +225,8 @@ struct drm_atomic_state { > struct __drm_crtcs_state *crtcs; > int num_connector; > struct __drm_connnectors_state *connectors; > + int num_private_objs; > + struct __drm_private_objs_state *private_objs; > > struct drm_modeset_acquire_ctx *acquire_ctx; > > @@ -268,6 +319,11 @@ int drm_atomic_connector_set_property(struct drm_connector *connector, > struct drm_connector_state *state, struct drm_property *property, > uint64_t val); > > +void * __must_check > +drm_atomic_get_private_obj_state(struct drm_atomic_state *state, > + void *obj, > + const struct drm_private_state_funcs *funcs); > + > /** > * drm_atomic_get_existing_crtc_state - get crtc state, if it exists > * @state: global atomic state object > @@ -753,6 +809,51 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > for_each_if (plane) > > /** > + * __for_each_private_obj - iterate over all private objects > + * @__state: &struct drm_atomic_state pointer > + * @obj: private object iteration cursor > + * @obj_state: private object state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * @__funcs: &struct drm_private_state_funcs iteration cursor > + * > + * This macro iterates over the array containing private object data in atomic > + * state > + */ > +#define __for_each_private_obj(__state, obj, obj_state, __i, __funcs) \ > + for ((__i) = 0; \ > + (__i) < (__state)->num_private_objs && \ > + ((obj) = (__state)->private_objs[__i].obj, \ > + (__funcs) = (__state)->private_objs[__i].funcs, \ > + (obj_state) = (__state)->private_objs[__i].obj_state, \ > + 1); \ > + (__i)++) \ > + > +/** > + * for_each_private_obj - iterate over a specify type of private object > + * @__state: &struct drm_atomic_state pointer > + * @obj_funcs: &struct drm_private_state_funcs function table to filter > + * private objects > + * @obj: private object iteration cursor > + * @obj_state: private object state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * @__funcs: &struct drm_private_state_funcs iteration cursor > + * > + * This macro iterates over the private objects state array while filtering the > + * objects based on the vfunc table that is passed as @obj_funcs. New macros > + * can be created by passing in the vfunc table associated with a specific > + * private object. > + */ > +#define for_each_private_obj(__state, obj_funcs, obj, obj_state, __i, __funcs) \ > + for ((__i) = 0; \ > + (__i) < (__state)->num_private_objs && \ > + ((obj) = (__state)->private_objs[__i].obj, \ > + (__funcs) = (__state)->private_objs[__i].funcs, \ > + (obj_state) = (__state)->private_objs[__i].obj_state, \ > + 1); \ > + (__i)++) \ > + for_each_if (__funcs == obj_funcs) > + Maybe use __for_each_private_obj in this macro for clarity? #define for_each_private_obj(...) \ __for_each_private_obj(...) \ for_each_if (__funcs == obj_funcs) ? Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 30229ab..8e5f3eb 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -57,6 +57,7 @@ void drm_atomic_state_default_release(struct drm_atomic_state *state) kfree(state->connectors); kfree(state->crtcs); kfree(state->planes); + kfree(state->private_objs); } EXPORT_SYMBOL(drm_atomic_state_default_release); @@ -184,6 +185,17 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) state->planes[i].ptr = NULL; state->planes[i].state = NULL; } + + for (i = 0; i < state->num_private_objs; i++) { + void *obj_state = state->private_objs[i].obj_state; + + state->private_objs[i].funcs->destroy_state(obj_state); + state->private_objs[i].obj = NULL; + state->private_objs[i].obj_state = NULL; + state->private_objs[i].funcs = NULL; + } + state->num_private_objs = 0; + } EXPORT_SYMBOL(drm_atomic_state_default_clear); @@ -978,6 +990,59 @@ static void drm_atomic_plane_print_state(struct drm_printer *p, } /** + * drm_atomic_get_private_obj_state - get private object state + * @state: global atomic state + * @obj: private object to get the state for + * @funcs: pointer to the struct of function pointers that identify the object + * type + * + * This function returns the private object state for the given private object, + * allocating the state if needed. It does not grab any locks as the caller is + * expected to care of any required locking. + * + * RETURNS: + * + * Either the allocated state or the error code encoded into a pointer. + */ +void * +drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj, + const struct drm_private_state_funcs *funcs) +{ + int index, num_objs, i; + size_t size; + struct __drm_private_objs_state *arr; + + for (i = 0; i < state->num_private_objs; i++) + if (obj == state->private_objs[i].obj && + state->private_objs[i].obj_state) + return state->private_objs[i].obj_state; + + num_objs = state->num_private_objs + 1; + size = sizeof(*state->private_objs) * num_objs; + arr = krealloc(state->private_objs, size, GFP_KERNEL); + if (!arr) + return ERR_PTR(-ENOMEM); + + state->private_objs = arr; + index = state->num_private_objs; + memset(&state->private_objs[index], 0, sizeof(*state->private_objs)); + + state->private_objs[index].obj_state = funcs->duplicate_state(state, obj); + if (!state->private_objs[index].obj_state) + return ERR_PTR(-ENOMEM); + + state->private_objs[index].obj = obj; + state->private_objs[index].funcs = funcs; + state->num_private_objs = num_objs; + + DRM_DEBUG_ATOMIC("Added new private object state %p to %p\n", + state->private_objs[index].obj_state, state); + + return state->private_objs[index].obj_state; +} +EXPORT_SYMBOL(drm_atomic_get_private_obj_state); + +/** * drm_atomic_get_connector_state - get connector state * @state: global atomic state object * @connector: connector to get state object for diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index cfeda5f..cce05fb 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2032,6 +2032,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; struct drm_crtc_commit *commit; + void *obj, *obj_state; + const struct drm_private_state_funcs *funcs; if (stall) { for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { @@ -2092,6 +2094,9 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, state->planes[i].state = old_plane_state; plane->state = new_plane_state; } + + __for_each_private_obj(state, obj, obj_state, i, funcs) + funcs->swap_state(obj, &state->private_objs[i].obj_state); } EXPORT_SYMBOL(drm_atomic_helper_swap_state); diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 788daf7..285b3a3 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -155,6 +155,53 @@ struct __drm_connnectors_state { }; /** + * struct drm_private_state_funcs - atomic state functions for private objects + * + * These hooks are used by atomic helpers to create, swap and destroy states of + * private objects. The structure itself is used as a vtable to identify the + * associated private object type. Each private object type that needs to be + * added to the atomic states is expected to have an implementation of these + * hooks and pass a pointer to it's drm_private_state_funcs struct to + * drm_atomic_get_private_obj_state(). + */ +struct drm_private_state_funcs { + /** + * @duplicate_state: + * + * Duplicate the current state of the private object and return it. It + * is an error to call this before obj->state has been initialized. + * + * RETURNS: + * + * Duplicated atomic state or NULL when obj->state is not + * initialized or allocation failed. + */ + void *(*duplicate_state)(struct drm_atomic_state *state, void *obj); + + /** + * @swap_state: + * + * This function swaps the existing state of a private object @obj with + * it's newly created state, the pointer to which is passed as + * @obj_state_ptr. + */ + void (*swap_state)(void *obj, void **obj_state_ptr); + + /** + * @destroy_state: + * + * Frees the private object state created with @duplicate_state. + */ + void (*destroy_state)(void *obj_state); +}; + +struct __drm_private_objs_state { + void *obj; + void *obj_state; + const struct drm_private_state_funcs *funcs; +}; + +/** * struct drm_atomic_state - the global state object for atomic updates * @ref: count of all references to this state (will not be freed until zero) * @dev: parent DRM device @@ -164,6 +211,8 @@ struct __drm_connnectors_state { * @crtcs: pointer to array of CRTC pointers * @num_connector: size of the @connectors and @connector_states arrays * @connectors: pointer to array of structures with per-connector data + * @num_private_objs: size of the @private_objs array + * @private_objs: pointer to array of private object pointers * @acquire_ctx: acquire context for this atomic modeset state update */ struct drm_atomic_state { @@ -176,6 +225,8 @@ struct drm_atomic_state { struct __drm_crtcs_state *crtcs; int num_connector; struct __drm_connnectors_state *connectors; + int num_private_objs; + struct __drm_private_objs_state *private_objs; struct drm_modeset_acquire_ctx *acquire_ctx; @@ -268,6 +319,11 @@ int drm_atomic_connector_set_property(struct drm_connector *connector, struct drm_connector_state *state, struct drm_property *property, uint64_t val); +void * __must_check +drm_atomic_get_private_obj_state(struct drm_atomic_state *state, + void *obj, + const struct drm_private_state_funcs *funcs); + /** * drm_atomic_get_existing_crtc_state - get crtc state, if it exists * @state: global atomic state object @@ -753,6 +809,51 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); for_each_if (plane) /** + * __for_each_private_obj - iterate over all private objects + * @__state: &struct drm_atomic_state pointer + * @obj: private object iteration cursor + * @obj_state: private object state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * @__funcs: &struct drm_private_state_funcs iteration cursor + * + * This macro iterates over the array containing private object data in atomic + * state + */ +#define __for_each_private_obj(__state, obj, obj_state, __i, __funcs) \ + for ((__i) = 0; \ + (__i) < (__state)->num_private_objs && \ + ((obj) = (__state)->private_objs[__i].obj, \ + (__funcs) = (__state)->private_objs[__i].funcs, \ + (obj_state) = (__state)->private_objs[__i].obj_state, \ + 1); \ + (__i)++) \ + +/** + * for_each_private_obj - iterate over a specify type of private object + * @__state: &struct drm_atomic_state pointer + * @obj_funcs: &struct drm_private_state_funcs function table to filter + * private objects + * @obj: private object iteration cursor + * @obj_state: private object state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * @__funcs: &struct drm_private_state_funcs iteration cursor + * + * This macro iterates over the private objects state array while filtering the + * objects based on the vfunc table that is passed as @obj_funcs. New macros + * can be created by passing in the vfunc table associated with a specific + * private object. + */ +#define for_each_private_obj(__state, obj_funcs, obj, obj_state, __i, __funcs) \ + for ((__i) = 0; \ + (__i) < (__state)->num_private_objs && \ + ((obj) = (__state)->private_objs[__i].obj, \ + (__funcs) = (__state)->private_objs[__i].funcs, \ + (obj_state) = (__state)->private_objs[__i].obj_state, \ + 1); \ + (__i)++) \ + for_each_if (__funcs == obj_funcs) + +/** * drm_atomic_crtc_needs_modeset - compute combined modeset need * @state: &drm_crtc_state for the CRTC *