[Nouveau] [PATCH v3 1/2] nouveau/bl: Assign different names to interfaces
From: Pierre Moreau
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
---
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
+#include
#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;
+}
+
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_
Re: [Nouveau] [PATCH v3 1/2] nouveau/bl: Assign different names to interfaces
On 11:17 am - Nov 14 2016, Lukas Wunner wrote:
> On Sun, Nov 13, 2016 at 08:57:06PM +0100, Pierre Moreau wrote:
> > From: Pierre Moreau
> >
> > 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
> > ---
> > 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
> > +#include
> >
> > #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.
I don’t see how you could get to more than 100 backlight interfaces, but you
are absolutely right, it should fail rather than reverting to "nv_backlight".
I’ll add a return code to `nouveau_get_backlight_name()` and check for that in
the `nvX0_backlight_init()` functions. If it is non-zero, I’m not sure whether
I should return an error code (and if so, which one) or just a plain 0.
Thanks,
Pierre
PS: I have no idea what went wrong with the e-mail address, and why it decided
to send it from my other e-mail address rather than the usual one… I’ll have to
fix that before sending the updated version.
>
> 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_NA
Re: [Nouveau] [PATCH v3 1/2] nouveau/bl: Assign different names to interfaces
On Sun, Nov 13, 2016 at 08:57:06PM +0100, Pierre Moreau wrote:
> From: Pierre Moreau
>
> 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
> ---
> 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
> +#include
>
> #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);
> + }
>
