Re: [RFC v4 11/25] drm/connector: Add connector array functions

[Daniel Vetter <daniel@ffwll.ch>]

On Sat, Apr 14, 2018 at 01:53:04PM +0200, Noralf Trønnes wrote:
> Add functions to deal with the registred connectors as an array:
> - drm_connector_get_all()
> - drm_connector_put_all()
> 
> And to get the enabled status of those connectors:
> drm_connector_get_enabled_status()
> 
> This is prep work to remove struct drm_fb_helper_connector.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/drm_connector.c | 105 ++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h     |   5 ++
>  2 files changed, 110 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b9eb143d70fc..25c333c05a4e 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1854,3 +1854,108 @@ drm_connector_pick_cmdline_mode(struct drm_connector *connector)
>  	return mode;
>  }
>  EXPORT_SYMBOL(drm_connector_pick_cmdline_mode);
> +
> +/**
> + * drm_connector_get_all - Get all connectors into an array
> + * @dev: DRM device
> + * @connectors: Returned connector array
> + *
> + * This function iterates through all registered connectors and adds them to an
> + * array allocated by this function. A ref is taken on the connectors.
> + *
> + * Use drm_connector_put_all() to drop refs and free the array.
> + *
> + * Returns:
> + * Number of connectors or -ENOMEM on failure.
> + */
> +int drm_connector_get_all(struct drm_device *dev, struct drm_connector ***connectors)

I honestly don't like the fbdev pattern of having connector arrays all
that much. I think in a world of hotplug we should have as much code as
possible iterating over the life connector list using the special
functions.

Given that these functions here set a bit a bad example imo I'd drop them
and just live with the code duplication.
-Daniel

> +{
> +	struct drm_connector *connector, **temp, **conns = NULL;
> +	struct drm_connector_list_iter conn_iter;
> +	int connector_count = 0;
> +
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		temp = krealloc(conns, (connector_count + 1) * sizeof(*conns), GFP_KERNEL);
> +		if (!temp)
> +			goto err_put_free;
> +
> +		conns = temp;
> +		conns[connector_count++] = connector;
> +		drm_connector_get(connector);
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
> +
> +	*connectors = conns;
> +
> +	return connector_count;
> +
> +err_put_free:
> +	drm_connector_list_iter_end(&conn_iter);
> +	drm_connector_put_all(conns, connector_count);
> +
> +	return -ENOMEM;
> +}
> +EXPORT_SYMBOL(drm_connector_get_all);
> +
> +/**
> + * drm_connector_put_all - Put and free connector array
> + * @connectors: Array of connectors
> + * @connector_count: Number of connectors in the array (can be negative or zero)
> + *
> + * This function drops the ref on the connectors an frees the array.
> + */
> +void drm_connector_put_all(struct drm_connector **connectors, int connector_count)
> +{
> +	int i;
> +
> +	if (connector_count < 1)
> +		return;
> +
> +	for (i = 0; i < connector_count; i++)
> +		drm_connector_put(connectors[i]);
> +	kfree(connectors);
> +}
> +EXPORT_SYMBOL(drm_connector_put_all);
> +
> +/**
> + * drm_connector_get_enabled_status - Get enabled status on connectors
> + * @connectors: Array of connectors
> + * @connector_count: Number of connectors in the array
> + *
> + * This loops over the connector array and sets enabled if connector status is
> + * _connected_. If no connectors are connected, a new pass is done and
> + * connectors that are not _disconnected_ are set enabled.
> + *
> + * The caller is responsible for freeing the array using kfree().
> + *
> + * Returns:
> + * A boolean array of connector enabled statuses or NULL on allocation failure.
> + */
> +bool *drm_connector_get_enabled_status(struct drm_connector **connectors,
> +				       unsigned int connector_count)
> +{
> +	bool *enabled, any_enabled = false;
> +	unsigned int i;
> +
> +	enabled = kcalloc(connector_count, sizeof(*enabled), GFP_KERNEL);
> +	if (!enabled)
> +		return NULL;
> +
> +	for (i = 0; i < connector_count; i++) {
> +		if (connectors[i]->status == connector_status_connected) {
> +			enabled[i] = true;
> +			any_enabled = true;
> +		}
> +	}
> +
> +	if (any_enabled)
> +		return enabled;
> +
> +	for (i = 0; i < connector_count; i++)
> +		if (connectors[i]->status != connector_status_disconnected)
> +			enabled[i] = true;
> +
> +	return enabled;
> +}
> +EXPORT_SYMBOL(drm_connector_get_enabled_status);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 9cb4ca42373c..c3556a5f40c9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1169,4 +1169,9 @@ drm_connector_has_preferred_mode(struct drm_connector *connector,
>  struct drm_display_mode *
>  drm_connector_pick_cmdline_mode(struct drm_connector *connector);
>  
> +int drm_connector_get_all(struct drm_device *dev, struct drm_connector ***connectors);
> +void drm_connector_put_all(struct drm_connector **connectors, int connector_count);
> +bool *drm_connector_get_enabled_status(struct drm_connector **connectors,
> +				       unsigned int connector_count);
> +
>  #endif
> -- 
> 2.15.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx