| Message ID | 20200216172117.49832-9-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Regmap over USB for Multifunction USB Device (gpio, display, ...) | expand |
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
Den 17.02.2020 10.38, skrev Daniel Vetter: > 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? > What alternatives do you see? Parent device name? I did a scan to see what others are doing, and most have a consumer name lookup on the struct device (Device Tree or lookup tables): struct reset_control *__reset_control_get(struct device *dev, const char *id, int index, bool shared, bool optional, bool acquired); struct iio_channel *iio_channel_get(struct device *dev, const char *consumer_channel); struct regulator *__must_check regulator_get(struct device *dev, const char *id); struct pwm_device *pwm_get(struct device *dev, const char *con_id) struct gpio_desc *__must_check gpiod_get(struct device *dev, const char *con_id, enum gpiod_flags flags); SPI and I2C use the bus index as lookup: extern struct i2c_adapter *i2c_get_adapter(int nr); extern struct spi_controller *spi_busnum_to_master(u16 busnum); Noralf. >> + * >> + * 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
On Sun, Feb 23, 2020 at 6:43 PM Noralf Trønnes <noralf@tronnes.org> wrote: > Den 17.02.2020 10.38, skrev Daniel Vetter: > > 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? > > > > What alternatives do you see? Parent device name? > > I did a scan to see what others are doing, and most have a consumer name > lookup on the struct device (Device Tree or lookup tables): I think those are all for other purposes. What we want here is that some interface things binds to something else. I was thinking sysfs paths to the underlying struct device (i.e. the one in drm_device->dev) might be neater, since that's more unique. Using minors we get that entire trouble of having duplicates (and in the past triplicatres) due to our uapi-flavors. But in sysfs there's only one underlying device. Anyway if there's no precedence and Greg doesn't have ideas either then I guess we can just go with minor ids. It's about as good as anything else really. -Daniel > > struct reset_control *__reset_control_get(struct device *dev, const char > *id, > int index, bool shared, > bool optional, bool acquired); > > struct iio_channel *iio_channel_get(struct device *dev, > const char *consumer_channel); > > struct regulator *__must_check regulator_get(struct device *dev, > const char *id); > > struct pwm_device *pwm_get(struct device *dev, const char *con_id) > > struct gpio_desc *__must_check gpiod_get(struct device *dev, > const char *con_id, > enum gpiod_flags flags); > > SPI and I2C use the bus index as lookup: > > extern struct i2c_adapter *i2c_get_adapter(int nr); > > extern struct spi_controller *spi_busnum_to_master(u16 busnum); > > > Noralf. > > >> + * > >> + * 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.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. + * + * 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) +{ + 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);
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(+)