On Sun, Nov 13, 2016 at 08:57:06PM +0100, Pierre Moreau wrote:
> From: Pierre Moreau <pierre.mor...@free.fr>
>
> Currently, every backlight interface created by Nouveau uses the same name,
> nv_backlight. This leads to a sysfs warning as it tries to create an already
> existing folder. This patch adds a incremented number to the name, but keeps
> the initial name as nv_backlight, to avoid possibly breaking userspace; the
> second interface will be named nv_backlight1, and so on.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86539
>
> v2:
> * Switch to using ida for generating unique IDs, as suggested by Ilia Mirkin;
> * Allocate backlight name on the stack, as suggested by Ilia Mirkin;
> * Move `nouveau_get_backlight_name()` to avoid forward declaration, as
> suggested by Ilia Mirkin;
> * Fix reference to bug report formatting, as reported by Nick Tenney.
>
> v3:
> * Define a macro for the size of the backlight name, to avoid defining
> it multiple times;
> * Use snprintf in place of sprintf.
>
> Signed-off-by: Pierre Moreau <pierre.mor...@free.fr>
> ---
> drivers/gpu/drm/nouveau/nouveau_backlight.c | 65
> +++++++++++++++++++++++++++--
> drivers/gpu/drm/nouveau/nouveau_display.h | 10 +++++
> drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +
> drivers/gpu/drm/nouveau/nouveau_drv.h | 1 +
> 4 files changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index 5e2c568..0e69612 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -31,11 +31,32 @@
> */
>
> #include <linux/backlight.h>
> +#include <linux/idr.h>
>
> #include "nouveau_drv.h"
> #include "nouveau_reg.h"
> #include "nouveau_encoder.h"
>
> +static struct ida bl_ida;
> +#define BL_NAME_SIZE 15 // 12 for name + 2 for digits + 1 for '\0'
> +
> +struct backlight_connector {
> + struct list_head head;
> + int id;
> +};
> +
> +static void
> +nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE], struct
> + backlight_connector *connector)
> +{
> + const int nb = ida_simple_get(&bl_ida, 0, 0, GFP_KERNEL);
> + if (nb > 0 && nb < 100)
> + snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
> + else
> + snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight");
> + connector->id = nb;
> +}
> +
Just a minor issue, in the >= 100 case, backlight creation should probably
fail rather than reverting to "nv_backlight". Other than that LGTM.
Thanks,
Lukas
> static int
> nv40_get_intensity(struct backlight_device *bd)
> {
> @@ -74,6 +95,8 @@ nv40_backlight_init(struct drm_connector *connector)
> struct nvif_object *device = &drm->device.object;
> struct backlight_properties props;
> struct backlight_device *bd;
> + struct backlight_connector bl_connector;
> + char backlight_name[BL_NAME_SIZE];
>
> if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK))
> return 0;
> @@ -81,10 +104,16 @@ nv40_backlight_init(struct drm_connector *connector)
> memset(&props, 0, sizeof(struct backlight_properties));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = 31;
> - bd = backlight_device_register("nv_backlight", connector->kdev, drm,
> + nouveau_get_backlight_name(backlight_name, &bl_connector);
> + bd = backlight_device_register(backlight_name , connector->kdev, drm,
> &nv40_bl_ops, &props);
> - if (IS_ERR(bd))
> +
> + if (IS_ERR(bd)) {
> + if (bl_connector.id > 0)
> + ida_simple_remove(&bl_ida, bl_connector.id);
> return PTR_ERR(bd);
> + }
> + list_add(&bl_connector.head, &drm->bl_connectors);
> drm->backlight = bd;
> bd->props.brightness = nv40_get_intensity(bd);
> backlight_update_status(bd);
> @@ -182,6 +211,8 @@ nv50_backlight_init(struct drm_connector *connector)
> struct backlight_properties props;
> struct backlight_device *bd;
> const struct backlight_ops *ops;
> + struct backlight_connector bl_connector;
> + char backlight_name[BL_NAME_SIZE];
>
> nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
> if (!nv_encoder) {
> @@ -203,11 +234,17 @@ nv50_backlight_init(struct drm_connector *connector)
> memset(&props, 0, sizeof(struct backlight_properties));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = 100;
> - bd = backlight_device_register("nv_backlight", connector->kdev,
> + nouveau_get_backlight_name(backlight_name, &bl_connector);
> + bd = backlight_device_register(backlight_name , connector->kdev,
> nv_encoder, ops, &props);
> - if (IS_ERR(bd))
> +
> + if (IS_ERR(bd)) {
> + if (bl_connector.id > 0)
> + ida_simple_remove(&bl_ida, bl_connector.id);
> return PTR_ERR(bd);
> + }
>
> + list_add(&bl_connector.head, &drm->bl_connectors);
> drm->backlight = bd;
> bd->props.brightness = bd->ops->get_brightness(bd);
> backlight_update_status(bd);
> @@ -221,6 +258,8 @@ nouveau_backlight_init(struct drm_device *dev)
> struct nvif_device *device = &drm->device;
> struct drm_connector *connector;
>
> + INIT_LIST_HEAD(&drm->bl_connectors);
> +
> list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
> connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> @@ -247,9 +286,27 @@ void
> nouveau_backlight_exit(struct drm_device *dev)
> {
> struct nouveau_drm *drm = nouveau_drm(dev);
> + struct backlight_connector *connector;
> +
> + list_for_each_entry(connector, &drm->bl_connectors, head) {
> + if (connector->id >= 0)
> + ida_simple_remove(&bl_ida, connector->id);
> + }
>
> if (drm->backlight) {
> backlight_device_unregister(drm->backlight);
> drm->backlight = NULL;
> }
> }
> +
> +void
> +nouveau_backlight_ctor(void)
> +{
> + ida_init(&bl_ida);
> +}
> +
> +void
> +nouveau_backlight_dtor(void)
> +{
> + ida_destroy(&bl_ida);
> +}
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h
> b/drivers/gpu/drm/nouveau/nouveau_display.h
> index 330fe0f..4a75df0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -91,6 +91,8 @@ int nouveau_crtc_set_config(struct drm_mode_set *set);
> #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
> extern int nouveau_backlight_init(struct drm_device *);
> extern void nouveau_backlight_exit(struct drm_device *);
> +extern void nouveau_backlight_ctor(void);
> +extern void nouveau_backlight_dtor(void);
> #else
> static inline int
> nouveau_backlight_init(struct drm_device *dev)
> @@ -101,6 +103,14 @@ nouveau_backlight_init(struct drm_device *dev)
> static inline void
> nouveau_backlight_exit(struct drm_device *dev) {
> }
> +
> +static inline void
> +nouveau_backlight_ctor(void) {
> +}
> +
> +static inline void
> +nouveau_backlight_dtor(void) {
> +}
> #endif
>
> struct drm_framebuffer *
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 9876e6f..2b93b55 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1113,6 +1113,7 @@ nouveau_drm_init(void)
> #endif
>
> nouveau_register_dsm_handler();
> + nouveau_backlight_ctor();
> return drm_pci_init(&driver_pci, &nouveau_drm_pci_driver);
> }
>
> @@ -1123,6 +1124,7 @@ nouveau_drm_exit(void)
> return;
>
> drm_pci_exit(&driver_pci, &nouveau_drm_pci_driver);
> + nouveau_backlight_dtor();
> nouveau_unregister_dsm_handler();
>
> #ifdef CONFIG_NOUVEAU_PLATFORM_DRIVER
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
> b/drivers/gpu/drm/nouveau/nouveau_drv.h
> index 4cd47ba..923dbce 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drv.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
> @@ -161,6 +161,7 @@ struct nouveau_drm {
> struct nvbios vbios;
> struct nouveau_display *display;
> struct backlight_device *backlight;
> + struct list_head bl_connectors;
>
> /* power management */
> struct nouveau_hwmon *hwmon;
> --
> 2.10.2
>
> _______________________________________________
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
