Re: [RFC 8/9] drm/client: Add drm_client_init_from_id() and drm_client_modeset_set()

[Daniel Vetter <daniel@ffwll.ch>]

On Sun, Feb 16, 2020 at 06:21:16PM +0100, Noralf Trønnes wrote:
> drm_client_init_from_id() provides a way for clients to add a client based
> on the minor. drm_client_modeset_set() provides a way to set the modeset
> for clients that handles connectors and display mode on their own.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/drm_client.c         | 37 ++++++++++++++++++++
>  drivers/gpu/drm/drm_client_modeset.c | 52 ++++++++++++++++++++++++++++
>  include/drm/drm_client.h             |  4 +++
>  3 files changed, 93 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index d9a2e3695525..dbd73fe8d987 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -112,6 +112,43 @@ int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
>  }
>  EXPORT_SYMBOL(drm_client_init);
>  
> +/**
> + * drm_client_init_from_id - Initialise a DRM client
> + * @minor_id: DRM minor id
> + * @client: DRM client
> + * @name: Client name
> + * @funcs: DRM client functions (optional)
> + *
> + * This function looks up the drm_device using the minor id and initializes the client.
> + * It also registeres the client to avoid a possible race with DRM device unregister.

I think another sentence here would be good, explaining that this is for
drivers outside of drm to expose a specific drm driver in some fashion,
and just outright mention your use-case as an example.

I'm also not sure whether lookup-by-minor is the greatest thing for
kernel-internal lookups like this, maybe Greg has some idea?

> + *
> + * See drm_client_init() and drm_client_register().
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_init_from_id(unsigned int minor_id, struct drm_client_dev *client,
> +			    const char *name, const struct drm_client_funcs *funcs)
> +{
> +	struct drm_minor *minor;
> +	int ret;
> +
> +	minor = drm_minor_acquire(minor_id);
> +	if (IS_ERR(minor))
> +		return PTR_ERR(minor);
> +
> +	mutex_lock(&minor->dev->clientlist_mutex);
> +	ret = drm_client_init(minor->dev, client, name, funcs);
> +	if (!ret)
> +		list_add(&client->list, &minor->dev->clientlist);
> +	mutex_unlock(&minor->dev->clientlist_mutex);
> +
> +	drm_minor_release(minor);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_client_init_from_id);
> +
>  /**
>   * drm_client_register - Register client
>   * @client: DRM client
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 895b73f23079..9396267e646c 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -807,6 +807,58 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
>  }
>  EXPORT_SYMBOL(drm_client_modeset_probe);
>  
> +/**
> + * drm_client_modeset_set() - Set modeset
> + * @client: DRM client
> + * @connector: Connector
> + * @mode: Display mode
> + * @fb: Framebuffer
> + *
> + * This function releases any current modeset info and sets the new modeset in
> + * the client's modeset array.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_modeset_set(struct drm_client_dev *client, struct drm_connector *connector,
> +			   struct drm_display_mode *mode, struct drm_framebuffer *fb)

Hm, since you need the functionality would be kinda neat to wire this up
for the fbdev emulation too. Ofc without reallocating the framebuffer
(fbdev can't do that), so would be limited to lower resolutions than we
booted with.
-Daniel

> +{
> +	struct drm_mode_set *modeset;
> +	int ret = -ENOENT;
> +
> +	mutex_lock(&client->modeset_mutex);
> +
> +	drm_client_modeset_release(client);
> +
> +	if (!connector || !mode || !fb) {
> +		ret = 0;
> +		goto unlock;
> +	}
> +
> +	drm_client_for_each_modeset(modeset, client) {
> +		if (!connector_has_possible_crtc(connector, modeset->crtc))
> +			continue;
> +
> +		modeset->mode = drm_mode_duplicate(client->dev, mode);
> +		if (!modeset->mode) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +
> +		drm_connector_get(connector);
> +		modeset->connectors[modeset->num_connectors++] = connector;
> +
> +		modeset->fb = fb;
> +		ret = 0;
> +		break;
> +	}
> +unlock:
> +	mutex_unlock(&client->modeset_mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_client_modeset_set);
> +
>  /**
>   * drm_client_rotation() - Check the initial rotation value
>   * @modeset: DRM modeset
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 5cf2c5dd8b1e..97e4157d07c5 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -104,6 +104,8 @@ struct drm_client_dev {
>  
>  int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
>  		    const char *name, const struct drm_client_funcs *funcs);
> +int drm_client_init_from_id(unsigned int minor_id, struct drm_client_dev *client,
> +			    const char *name, const struct drm_client_funcs *funcs);
>  void drm_client_release(struct drm_client_dev *client);
>  void drm_client_register(struct drm_client_dev *client);
>  
> @@ -155,6 +157,8 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
>  int drm_client_modeset_create(struct drm_client_dev *client);
>  void drm_client_modeset_free(struct drm_client_dev *client);
>  int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width, unsigned int height);
> +int drm_client_modeset_set(struct drm_client_dev *client, struct drm_connector *connector,
> +			   struct drm_display_mode *mode, struct drm_framebuffer *fb);
>  bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
>  int drm_client_modeset_commit_force(struct drm_client_dev *client);
>  int drm_client_modeset_commit(struct drm_client_dev *client);
> -- 
> 2.23.0
> 
> _______________________________________________
> 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