[PATCH RFC v2 1/3] drm/dp: Keep a list of drm_dp_aux helper.

[Daniel Vetter]

On Tue, Sep 15, 2015 at 04:55:02PM -0700, Rafael Antognolli wrote:
> This list will be used to get the aux channels registered through the
> helpers. One function is provided to set a callback for added/removed
> aux channels, and another function to iterate over the list of aux
> channels.
> 
> Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 73 +++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_dp_helper.h     |  5 +++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 9535c5b..41bdd93 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -746,6 +746,56 @@ static const struct i2c_algorithm drm_dp_i2c_algo = {
>  	.master_xfer = drm_dp_i2c_xfer,
>  };
>  
> +struct drm_dp_aux_node {
> +	struct klist_node list;
> +	struct drm_dp_aux *aux;
> +};
> +
> +static DEFINE_KLIST(drm_dp_aux_list, NULL, NULL);
> +
> +static int (*aux_dev_cb)(int action, struct drm_dp_aux *aux) = NULL;
> +
> +void drm_dp_aux_set_aux_dev(int (*fn)(int, struct drm_dp_aux *))
> +{
> +	aux_dev_cb = fn;
> +}
> +EXPORT_SYMBOL(drm_dp_aux_set_aux_dev);
> +
> +static struct drm_dp_aux *next_aux(struct klist_iter *i)
> +{
> +	struct klist_node *n = klist_next(i);
> +	struct drm_dp_aux *aux = NULL;
> +	struct drm_dp_aux_node *aux_node;
> +
> +	if (n) {
> +		aux_node = container_of(n, struct drm_dp_aux_node, list);
> +		aux = aux_node->aux;
> +	}
> +	return aux;
> +}
> +
> +int drm_dp_aux_for_each(void *data, int (*fn)(struct drm_dp_aux *, void *))
> +{
> +	struct klist_iter i;
> +	struct drm_dp_aux *aux;
> +	int error = 0;
> +
> +	klist_iter_init(&drm_dp_aux_list, &i);
> +	while ((aux = next_aux(&i)) && !error)
> +		error = fn(aux, data);
> +	klist_iter_exit(&i);
> +	return error;
> +}
> +EXPORT_SYMBOL(drm_dp_aux_for_each);
> +
> +static int drm_dp_aux_dev_inform(int action, struct drm_dp_aux *aux)
> +{
> +	if (aux_dev_cb) {
> +		return aux_dev_cb(action, aux);
> +	}
> +	return 0;
> +}
> +
>  /**
>   * drm_dp_aux_register() - initialise and register aux channel
>   * @aux: DisplayPort AUX channel
> @@ -754,6 +804,7 @@ static const struct i2c_algorithm drm_dp_i2c_algo = {
>   */
>  int drm_dp_aux_register(struct drm_dp_aux *aux)
>  {
> +	struct drm_dp_aux_node *aux_node;
>  	mutex_init(&aux->hw_mutex);
>  
>  	aux->ddc.algo = &drm_dp_i2c_algo;
> @@ -768,6 +819,14 @@ int drm_dp_aux_register(struct drm_dp_aux *aux)
>  	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
>  		sizeof(aux->ddc.name));
>  
> +	/* add aux to list and notify listeners */
> +	aux_node = kzalloc(sizeof(*aux_node), GFP_KERNEL);
> +	if (!aux_node)
> +		return -ENOMEM;
> +	aux_node->aux = aux;
> +	klist_add_tail(&aux_node->list, &drm_dp_aux_list);
> +	drm_dp_aux_dev_inform(DRM_DP_ADD_AUX, aux);

If you go with the direct function call approach there's no need for all
this indirection, just do something like

	drm_dp_aux_register_devnode(aux);

And drop all the list book-keeping (since there will never be a time when
the aux stuff isn't loaded yet). Essentially you're reinventing notifier
chains here (but without the required locking).

With that this patch just adds 2 lines, probably best to squash it in with
the last one.
-Daniel

> +
>  	return i2c_add_adapter(&aux->ddc);
>  }
>  EXPORT_SYMBOL(drm_dp_aux_register);
> @@ -778,6 +837,20 @@ EXPORT_SYMBOL(drm_dp_aux_register);
>   */
>  void drm_dp_aux_unregister(struct drm_dp_aux *aux)
>  {
> +	struct klist_iter i;
> +	struct klist_node *n;
> +
> +	klist_iter_init(&drm_dp_aux_list, &i);
> +	while ((n = klist_next(&i))) {
> +		struct drm_dp_aux_node *aux_node =
> +			container_of(n, struct drm_dp_aux_node, list);
> +		if (aux_node->aux == aux) {
> +			klist_del(n);
> +			kfree(aux_node);
> +			break;
> +		}
> +	}
> +	drm_dp_aux_dev_inform(DRM_DP_DEL_AUX, aux);
>  	i2c_del_adapter(&aux->ddc);
>  }
>  EXPORT_SYMBOL(drm_dp_aux_unregister);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 9ec4716..f92de26 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -763,7 +763,12 @@ int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  
> +#define DRM_DP_ADD_AUX 0x01
> +#define DRM_DP_DEL_AUX 0x02
> +
>  int drm_dp_aux_register(struct drm_dp_aux *aux);
>  void drm_dp_aux_unregister(struct drm_dp_aux *aux);
> +void drm_dp_aux_set_aux_dev(int (*fn)(int, struct drm_dp_aux *));
> +int drm_dp_aux_for_each(void *data, int (*fn)(struct drm_dp_aux *, void *));
>  
>  #endif /* _DRM_DP_HELPER_H_ */
> -- 
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch