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

[Ville Syrjälä]

On Tue, Sep 15, 2015 at 09:27:27AM -0700, Rafael Antognolli wrote:
> On Tue, Sep 15, 2015 at 10:46:43AM +0300, Ville Syrjälä wrote:
> > On Mon, Sep 14, 2015 at 04:12:30PM -0700, Rafael Antognolli wrote:
> > > This list will be used to get the aux channels registered through the
> > > helpers. Two functions are provided to register/unregister notifier
> > > listeners on the list, and another functiont 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 | 71 +++++++++++++++++++++++++++++++++++++++++
> > >  include/drm/drm_dp_helper.h     |  6 ++++
> > >  2 files changed, 77 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > index 291734e..01a1489 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -710,6 +710,54 @@ 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 BLOCKING_NOTIFIER_HEAD(aux_notifier);
> > > +
> > > +int drm_dp_aux_register_notifier(struct notifier_block *nb)
> > > +{
> > > +	return blocking_notifier_chain_register(&aux_notifier, nb);
> > > +}
> > > +EXPORT_SYMBOL(drm_dp_aux_register_notifier);
> > 
> > Why is this notifier stuff needed? Why not just register/unregister the
> > aux-dev directly?
> > 
> 
> I am not sure it's needed, I was just looking for the best way of
> informing aux-dev that a new aux channel was added.
> 
> By register/unregister the aux-dev directly, do you mean making this
> drm_dp_helper aware of the aux dev, when it's registered, and directly
> calling some callback to inform that new aux channels were added?

That was my thought, yes. It would mean the auxdev module can't be
unloaded like i2c-dev, but I'm not sure that's a use case worth
worrying about.

> 
> > >+
> > > +int drm_dp_aux_unregister_notifier(struct notifier_block *nb)
> > > +{
> > > +	return blocking_notifier_chain_unregister(&aux_notifier, nb);
> > > +}
> > > +EXPORT_SYMBOL(drm_dp_aux_unregister_notifier);
> > > +
> > > +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);
> > > +
> > >  /**
> > >   * drm_dp_aux_register() - initialise and register aux channel
> > >   * @aux: DisplayPort AUX channel
> > > @@ -718,6 +766,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;
> > > @@ -732,6 +781,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);
> > > +	blocking_notifier_call_chain(&aux_notifier, DRM_DP_ADD_AUX, aux);
> > > +
> > >  	return i2c_add_adapter(&aux->ddc);
> > >  }
> > >  EXPORT_SYMBOL(drm_dp_aux_register);
> > > @@ -742,6 +799,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;
> > > +		}
> > > +	}
> > > +	blocking_notifier_call_chain(&aux_notifier, 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 8c52d0ef1..023620c 100644
> > > --- a/include/drm/drm_dp_helper.h
> > > +++ b/include/drm/drm_dp_helper.h
> > > @@ -763,7 +763,13 @@ 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);
> > > +int drm_dp_aux_register_notifier(struct notifier_block *nb);
> > > +int drm_dp_aux_unregister_notifier(struct notifier_block *nb);
> > > +int drm_dp_aux_for_each(void *data, int (*fn)(struct drm_dp_aux *, void *));
> > >  
> > >  #endif /* _DRM_DP_HELPER_H_ */
> > > -- 
> > > 2.4.0
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC

-- 
Ville Syrjälä
Intel OTC