On 21-04-17 07:51, Dhinakaran Pandiyan wrote:
> From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@xxxxxxxxx>
>
> 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@xxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Archit Taneja <architt@xxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Harry Wentland <Harry.wentland@xxxxxxx>
>
> Acked-by: Harry Wentland <harry.wentland@xxxxxxx>
> Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
