Re: [Intel-gfx] [PATCH v7 1/4] drm: Add driver-private objects to atomic state

[Maarten Lankhorst]

On 21-04-17 07:51, Dhinakaran Pandiyan wrote:
> From: "Pandiyan, Dhinakaran" 
>
> 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 
> Cc: Maarten Lankhorst 
> Cc: Archit Taneja 
> Cc: Chris Wilson 
> Cc: Harry Wentland 
>
> Acked-by: Harry Wentland 
> Suggested-by: Daniel Vetter 
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  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(>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);
> +
> 

[Intel-gfx] [PATCH v7 1/4] drm: Add driver-private objects to atomic state

[Dhinakaran Pandiyan]

From: "Pandiyan, Dhinakaran" 

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 
Cc: Maarten Lankhorst 
Cc: Archit Taneja 
Cc: Chris Wilson 
Cc: Harry Wentland 

Acked-by: Harry Wentland 
Suggested-by: Daniel Vetter 
Signed-off-by: Dhinakaran Pandiyan 
---
 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(>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