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

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@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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel