Pierre Moreau
2016-Apr-15 14:57 UTC
[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces
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. Fixes: fdo#86539 Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> --- drm/nouveau/nouveau_backlight.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c index 89eb460..914e2cb 100644 --- a/drm/nouveau/nouveau_backlight.c +++ b/drm/nouveau/nouveau_backlight.c @@ -36,6 +36,10 @@ #include "nouveau_reg.h" #include "nouveau_encoder.h" +static atomic_t bl_interfaces_nb = { 0 }; + +static char* nouveau_get_backlight_name(void); + static int nv40_get_intensity(struct backlight_device *bd) { @@ -74,6 +78,7 @@ nv40_backlight_init(struct drm_connector *connector) struct nvif_object *device = &drm->device.object; struct backlight_properties props; struct backlight_device *bd; + char* backlight_name = NULL; if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK)) return 0; @@ -81,8 +86,14 @@ 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, + backlight_name = nouveau_get_backlight_name(); + bd = backlight_device_register(backlight_name , connector->kdev, drm, &nv40_bl_ops, &props); + + // backlight_device_register() makes a copy + kfree(backlight_name); + backlight_name = NULL; + if (IS_ERR(bd)) return PTR_ERR(bd); drm->backlight = bd; @@ -182,6 +193,7 @@ nv50_backlight_init(struct drm_connector *connector) struct backlight_properties props; struct backlight_device *bd; const struct backlight_ops *ops; + char* backlight_name = NULL; nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS); if (!nv_encoder) { @@ -203,8 +215,14 @@ 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, + backlight_name = nouveau_get_backlight_name(); + bd = backlight_device_register(backlight_name, connector->kdev, nv_encoder, ops, &props); + + // backlight_device_register() makes a copy + kfree(backlight_name); + backlight_name = NULL; + if (IS_ERR(bd)) return PTR_ERR(bd); @@ -252,3 +270,16 @@ nouveau_backlight_exit(struct drm_device *dev) drm->backlight = NULL; } } + +static char* +nouveau_get_backlight_name(void) +{ + // 12 chars for "nv_backlight" + 2 for two digits number + 1 for '\0' + char* backlight_name = (char*)kmalloc(sizeof(char[15]), GFP_KERNEL); + const int nb = atomic_inc_return(&bl_interfaces_nb) - 1; + if (nb > 0 && nb < 100) + sprintf(backlight_name, "nv_backlight%d", nb); + else + sprintf(backlight_name, "nv_backlight"); + return backlight_name; +} -- 2.8.0
Pierre Moreau
2016-Apr-15 14:57 UTC
[Nouveau] [PATCH 2/2] nouveau/bl: Do not register interface if Apple GMUX detected
The Apple GMUX is the one managing the backlight, so there is no need for Nouveau to register its own backlight interface. Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> --- drm/nouveau/nouveau_backlight.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c index 914e2cb..37a1577 100644 --- a/drm/nouveau/nouveau_backlight.c +++ b/drm/nouveau/nouveau_backlight.c @@ -30,6 +30,7 @@ * Register locations derived from NVClock by Roderick Colenbrander */ +#include <linux/apple-gmux.h> #include <linux/backlight.h> #include "nouveau_drm.h" @@ -239,6 +240,12 @@ nouveau_backlight_init(struct drm_device *dev) struct nvif_device *device = &drm->device; struct drm_connector *connector; + if (apple_gmux_present()) { + NV_INFO(drm, "Apple GMUX detected: not registering Nouveau" + " backlight interface"); + return 0; + } + 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) -- 2.8.0
Ilia Mirkin
2016-Apr-15 15:06 UTC
[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces
On Fri, Apr 15, 2016 at 10:57 AM, Pierre Moreau <pierre.morrow at free.fr> wrote:> 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. > > Fixes: fdo#86539 > Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> > --- > drm/nouveau/nouveau_backlight.c | 35 +++++++++++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c > index 89eb460..914e2cb 100644 > --- a/drm/nouveau/nouveau_backlight.c > +++ b/drm/nouveau/nouveau_backlight.c > @@ -36,6 +36,10 @@ > #include "nouveau_reg.h" > #include "nouveau_encoder.h" > > +static atomic_t bl_interfaces_nb = { 0 };static data is initialized to 0, this should be unnecessary. I'd also call it "backlights" or something like that. No need for multiple words...> + > +static char* nouveau_get_backlight_name(void);Please organize the code to avoid forward declarations.> + > static int > nv40_get_intensity(struct backlight_device *bd) > { > @@ -74,6 +78,7 @@ nv40_backlight_init(struct drm_connector *connector) > struct nvif_object *device = &drm->device.object; > struct backlight_properties props; > struct backlight_device *bd; > + char* backlight_name = NULL; > > if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK)) > return 0; > @@ -81,8 +86,14 @@ 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, > + backlight_name = nouveau_get_backlight_name(); > + bd = backlight_device_register(backlight_name , connector->kdev, drm, > &nv40_bl_ops, &props); > + > + // backlight_device_register() makes a copy > + kfree(backlight_name); > + backlight_name = NULL; > + > if (IS_ERR(bd)) > return PTR_ERR(bd); > drm->backlight = bd; > @@ -182,6 +193,7 @@ nv50_backlight_init(struct drm_connector *connector) > struct backlight_properties props; > struct backlight_device *bd; > const struct backlight_ops *ops; > + char* backlight_name = NULL; > > nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS); > if (!nv_encoder) { > @@ -203,8 +215,14 @@ 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, > + backlight_name = nouveau_get_backlight_name(); > + bd = backlight_device_register(backlight_name, connector->kdev, > nv_encoder, ops, &props); > + > + // backlight_device_register() makes a copy > + kfree(backlight_name); > + backlight_name = NULL; > + > if (IS_ERR(bd)) > return PTR_ERR(bd); > > @@ -252,3 +270,16 @@ nouveau_backlight_exit(struct drm_device *dev) > drm->backlight = NULL; > } > } > + > +static char* > +nouveau_get_backlight_name(void) > +{ > + // 12 chars for "nv_backlight" + 2 for two digits number + 1 for '\0' > + char* backlight_name = (char*)kmalloc(sizeof(char[15]), GFP_KERNEL);Making this stack-allocated in the caller would be so much simpler...> + const int nb = atomic_inc_return(&bl_interfaces_nb) - 1;This kinda sucks if you reload nouveau a bunch. How about using an "ida". Have a look in drivers/gpu/drm/drm_crtc.c for how I use that one.> + if (nb > 0 && nb < 100) > + sprintf(backlight_name, "nv_backlight%d", nb); > + else > + sprintf(backlight_name, "nv_backlight"); > + return backlight_name; > +} > -- > 2.8.0 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
Pierre Moreau
2016-Apr-15 15:22 UTC
[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces
On 11:06 AM - Apr 15 2016, Ilia Mirkin wrote:> On Fri, Apr 15, 2016 at 10:57 AM, Pierre Moreau <pierre.morrow at free.fr> wrote: > > 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. > > > > Fixes: fdo#86539 > > Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> > > --- > > drm/nouveau/nouveau_backlight.c | 35 +++++++++++++++++++++++++++++++++-- > > 1 file changed, 33 insertions(+), 2 deletions(-) > > > > diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c > > index 89eb460..914e2cb 100644 > > --- a/drm/nouveau/nouveau_backlight.c > > +++ b/drm/nouveau/nouveau_backlight.c > > @@ -36,6 +36,10 @@ > > #include "nouveau_reg.h" > > #include "nouveau_encoder.h" > > > > +static atomic_t bl_interfaces_nb = { 0 }; > > static data is initialized to 0, this should be unnecessary.I didn’t know that. But on the other hand, I like having it explicit, and it should not add any overhead.> > I'd also call it "backlights" or something like that. No need for > multiple words...I prefer to use a plural noun when talking about a container of those nouns, rathter than a counter of those nouns. But I’ll change it.> > > + > > +static char* nouveau_get_backlight_name(void); > > Please organize the code to avoid forward declarations. > > > + > > static int > > nv40_get_intensity(struct backlight_device *bd) > > { > > @@ -74,6 +78,7 @@ nv40_backlight_init(struct drm_connector *connector) > > struct nvif_object *device = &drm->device.object; > > struct backlight_properties props; > > struct backlight_device *bd; > > + char* backlight_name = NULL; > > > > if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK)) > > return 0; > > @@ -81,8 +86,14 @@ 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, > > + backlight_name = nouveau_get_backlight_name(); > > + bd = backlight_device_register(backlight_name , connector->kdev, drm, > > &nv40_bl_ops, &props); > > + > > + // backlight_device_register() makes a copy > > + kfree(backlight_name); > > + backlight_name = NULL; > > + > > if (IS_ERR(bd)) > > return PTR_ERR(bd); > > drm->backlight = bd; > > @@ -182,6 +193,7 @@ nv50_backlight_init(struct drm_connector *connector) > > struct backlight_properties props; > > struct backlight_device *bd; > > const struct backlight_ops *ops; > > + char* backlight_name = NULL; > > > > nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS); > > if (!nv_encoder) { > > @@ -203,8 +215,14 @@ 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, > > + backlight_name = nouveau_get_backlight_name(); > > + bd = backlight_device_register(backlight_name, connector->kdev, > > nv_encoder, ops, &props); > > + > > + // backlight_device_register() makes a copy > > + kfree(backlight_name); > > + backlight_name = NULL; > > + > > if (IS_ERR(bd)) > > return PTR_ERR(bd); > > > > @@ -252,3 +270,16 @@ nouveau_backlight_exit(struct drm_device *dev) > > drm->backlight = NULL; > > } > > } > > + > > +static char* > > +nouveau_get_backlight_name(void) > > +{ > > + // 12 chars for "nv_backlight" + 2 for two digits number + 1 for '\0' > > + char* backlight_name = (char*)kmalloc(sizeof(char[15]), GFP_KERNEL); > > Making this stack-allocated in the caller would be so much simpler...--" I was planning to use a dynamic size to allow numbers bigger than 99, and lesser than 10, but stopped midway through the process. I’ll revert to a plain stack-allocated.> > > + const int nb = atomic_inc_return(&bl_interfaces_nb) - 1; > > This kinda sucks if you reload nouveau a bunch. How about using an > "ida". Have a look in drivers/gpu/drm/drm_crtc.c for how I use that > one.Yeah, I’m not really fond of that part. I’ll have a look at drm_crtc.c! Pierre> > > + if (nb > 0 && nb < 100) > > + sprintf(backlight_name, "nv_backlight%d", nb); > > + else > > + sprintf(backlight_name, "nv_backlight"); > > + return backlight_name; > > +} > > -- > > 2.8.0 > > > > _______________________________________________ > > Nouveau mailing list > > Nouveau at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20160415/37ae3e4d/attachment.sig>
Pierre Moreau
2016-Apr-17 08:18 UTC
[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces
[…]> > > + const int nb = atomic_inc_return(&bl_interfaces_nb) - 1; > > This kinda sucks if you reload nouveau a bunch. How about using an > "ida". Have a look in drivers/gpu/drm/drm_crtc.c for how I use that > one.I had a quick look at drm_crtc.c. This seems to be exactly what I need, except that I do not see how I can enforce to have one of the active ones being named `nv_backlight`, to not break (possibly) existing userspace applications. (I’m guessing that I would be doing the same as in drm_crtc.c: looping over all connectors and assigning them an ida, regardless of them being active or not, as they might become active later on.) There is another issue that I failed to address with this patch, and just realised about it. Each time `nvXX_backlight_init(connector)` is called on a connector, a new backlight device is created, and is stored in `drm->backlight`, which is unique at a device level, not connector level, meaning the old one just gets overriden (and leaked). Having a list `drm->backlights` should solve this issue. Would that be fine with you Ben? Pierre> > > + if (nb > 0 && nb < 100) > > + sprintf(backlight_name, "nv_backlight%d", nb); > > + else > > + sprintf(backlight_name, "nv_backlight"); > > + return backlight_name; > > +} > > -- > > 2.8.0 > > > > _______________________________________________ > > Nouveau mailing list > > Nouveau at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20160417/872a5f51/attachment.sig>
Pierre Moreau
2016-Apr-17 19:57 UTC
[Nouveau] [PATCH v2 1/2] nouveau/bl: Assign different names to interfaces
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. Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> --- drm/nouveau/nouveau_backlight.c | 64 ++++++++++++++++++++++++++++++++++++++--- drm/nouveau/nouveau_display.h | 10 +++++++ drm/nouveau/nouveau_drm.c | 2 ++ drm/nouveau/nouveau_drm.h | 1 + 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c index 89eb460..41330e4 100644 --- a/drm/nouveau/nouveau_backlight.c +++ b/drm/nouveau/nouveau_backlight.c @@ -31,11 +31,31 @@ */ #include <linux/backlight.h> +#include <linux/idr.h> #include "nouveau_drm.h" #include "nouveau_reg.h" #include "nouveau_encoder.h" +static struct ida bl_ida; + +struct backlight_connector { + struct list_head head; + int id; +}; + +static void +nouveau_get_backlight_name(char backlight_name[15], struct backlight_connector + *connector) +{ + const int nb = ida_simple_get(&bl_ida, 0, 0, GFP_KERNEL); + if (nb > 0 && nb < 100) + sprintf(backlight_name, "nv_backlight%d", nb); + else + sprintf(backlight_name, "nv_backlight"); + connector->id = nb; +} + static int nv40_get_intensity(struct backlight_device *bd) { @@ -74,6 +94,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[15]; // 12 for name + 2 for digits + 1 for '\0' if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK)) return 0; @@ -81,10 +103,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 +210,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[15]; // 12 for name + 2 for digits + 1 for '\0' nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS); if (!nv_encoder) { @@ -203,11 +233,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 +257,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) @@ -246,9 +284,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/drm/nouveau/nouveau_display.h b/drm/nouveau/nouveau_display.h index 5a57d8b..bb07cc8 100644 --- a/drm/nouveau/nouveau_display.h +++ b/drm/nouveau/nouveau_display.h @@ -90,6 +90,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) @@ -100,6 +102,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 #endif diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c index d06877d..0d89d7c 100644 --- a/drm/nouveau/nouveau_drm.c +++ b/drm/nouveau/nouveau_drm.c @@ -1097,6 +1097,7 @@ nouveau_drm_init(void) #endif nouveau_register_dsm_handler(); + nouveau_backlight_ctor(); return drm_pci_init(&driver_pci, &nouveau_drm_pci_driver); } @@ -1107,6 +1108,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/drm/nouveau/nouveau_drm.h b/drm/nouveau/nouveau_drm.h index 5c363ed..593828e 100644 --- a/drm/nouveau/nouveau_drm.h +++ b/drm/nouveau/nouveau_drm.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.8.0
Pierre Moreau
2016-Apr-17 19:57 UTC
[Nouveau] [PATCH REBASED 2/2] nouveau/bl: Do not register interface if Apple GMUX detected
The Apple GMUX is the one managing the backlight, so there is no need for Nouveau to register its own backlight interface. Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> --- drm/nouveau/nouveau_backlight.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c index 41330e4..94ac3cb 100644 --- a/drm/nouveau/nouveau_backlight.c +++ b/drm/nouveau/nouveau_backlight.c @@ -30,6 +30,7 @@ * Register locations derived from NVClock by Roderick Colenbrander */ +#include <linux/apple-gmux.h> #include <linux/backlight.h> #include <linux/idr.h> @@ -257,6 +258,12 @@ nouveau_backlight_init(struct drm_device *dev) struct nvif_device *device = &drm->device; struct drm_connector *connector; + if (apple_gmux_present()) { + NV_INFO(drm, "Apple GMUX detected: not registering Nouveau" + " backlight interface"); + return 0; + } + INIT_LIST_HEAD(&drm->bl_connectors); list_for_each_entry(connector, &dev->mode_config.connector_list, head) { -- 2.8.0
Pierre Moreau
2016-Nov-13 19:57 UTC
[Nouveau] [PATCH v3 1/2] nouveau/bl: Assign different names to interfaces
From: Pierre Moreau <pierre.morrow at 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.morrow at 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; +} + 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
Pierre Moreau
2016-Nov-13 19:57 UTC
[Nouveau] [PATCH REBASED 2/2] Do not register interface if Apple GMUX detected
From: Pierre Moreau <pierre.morrow at free.fr> The Apple GMUX is the one managing the backlight, so there is no need for Nouveau to register its own backlight interface. v2: Do not split information message on two lines as it prevents from grepping it, as pointed out by Lukas Wunner Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> --- drivers/gpu/drm/nouveau/nouveau_backlight.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c index 0e69612..3c91c24 100644 --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c @@ -30,6 +30,7 @@ * Register locations derived from NVClock by Roderick Colenbrander */ +#include <linux/apple-gmux.h> #include <linux/backlight.h> #include <linux/idr.h> @@ -258,6 +259,11 @@ nouveau_backlight_init(struct drm_device *dev) struct nvif_device *device = &drm->device; struct drm_connector *connector; + if (apple_gmux_present()) { + NV_INFO(drm, "Apple GMUX detected: not registering Nouveau backlight interface"); + return 0; + } + INIT_LIST_HEAD(&drm->bl_connectors); list_for_each_entry(connector, &dev->mode_config.connector_list, head) { -- 2.10.2
Lukas Wunner
2016-Nov-14 10:17 UTC
[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 <pierre.morrow at 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.morrow at 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-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Pierre Moreau
2016-Dec-07 23:57 UTC
[Nouveau] [PATCH v4 1/2] nouveau/bl: Assign different names to interfaces
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. v4: * Do not create similarly named interfaces when reaching the maximum amount of unique names, but fail instead, as pointed out by Lukas Wunner Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> --- drm/nouveau/nouveau_backlight.c | 74 ++++++++++++++++++++++++++++++++++++++--- drm/nouveau/nouveau_display.h | 10 ++++++ drm/nouveau/nouveau_drm.c | 2 ++ drm/nouveau/nouveau_drv.h | 1 + 4 files changed, 83 insertions(+), 4 deletions(-) diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c index 5e2c568..a34cd35 100644 --- a/drm/nouveau/nouveau_backlight.c +++ b/drm/nouveau/nouveau_backlight.c @@ -31,11 +31,35 @@ */ #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 bool +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) + return false; + if (nb > 0) + snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb); + else + snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight"); + connector->id = nb; + return true; +} + static int nv40_get_intensity(struct backlight_device *bd) { @@ -74,6 +98,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 +107,19 @@ 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, + if (!nouveau_get_backlight_name(backlight_name, &bl_connector)) { + NV_ERROR(drm, "Failed to retrieve a unique name for the backlight interface\n"); + return 0; + } + 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 +217,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 +240,20 @@ 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, + if (!nouveau_get_backlight_name(backlight_name, &bl_connector)) { + NV_ERROR(drm, "Failed to retrieve a unique name for the backlight interface\n"); + return 0; + } + 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 +267,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 +295,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/drm/nouveau/nouveau_display.h b/drm/nouveau/nouveau_display.h index 330fe0f..4a75df0 100644 --- a/drm/nouveau/nouveau_display.h +++ b/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/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c index 064a925..59348fc 100644 --- a/drm/nouveau/nouveau_drm.c +++ b/drm/nouveau/nouveau_drm.c @@ -1122,6 +1122,7 @@ nouveau_drm_init(void) #endif nouveau_register_dsm_handler(); + nouveau_backlight_ctor(); return drm_pci_init(&driver_pci, &nouveau_drm_pci_driver); } @@ -1132,6 +1133,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/drm/nouveau/nouveau_drv.h b/drm/nouveau/nouveau_drv.h index 9730c0e..8d5ed5b 100644 --- a/drm/nouveau/nouveau_drv.h +++ b/drm/nouveau/nouveau_drv.h @@ -163,6 +163,7 @@ struct nouveau_drm { struct nvbios vbios; struct nouveau_display *display; struct backlight_device *backlight; + struct list_head bl_connectors; struct work_struct hpd_work; #ifdef CONFIG_ACPI struct notifier_block acpi_nb; -- 2.10.2
Pierre Moreau
2016-Dec-07 23:57 UTC
[Nouveau] [PATCH v3 2/2] Do not register interface if Apple GMUX detected
The Apple GMUX is the one managing the backlight, so there is no need for Nouveau to register its own backlight interface. v2: Do not split information message on two lines as it prevents from grepping it, as pointed out by Lukas Wunner v3: Add a missing end-of-line character to the printed message Signed-off-by: Pierre Moreau <pierre.morrow at free.fr> --- drm/nouveau/nouveau_backlight.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c index a34cd35..8b1ca4a 100644 --- a/drm/nouveau/nouveau_backlight.c +++ b/drm/nouveau/nouveau_backlight.c @@ -30,6 +30,7 @@ * Register locations derived from NVClock by Roderick Colenbrander */ +#include <linux/apple-gmux.h> #include <linux/backlight.h> #include <linux/idr.h> @@ -267,6 +268,11 @@ nouveau_backlight_init(struct drm_device *dev) struct nvif_device *device = &drm->device; struct drm_connector *connector; + if (apple_gmux_present()) { + NV_INFO(drm, "Apple GMUX detected: not registering Nouveau backlight interface\n"); + return 0; + } + INIT_LIST_HEAD(&drm->bl_connectors); list_for_each_entry(connector, &dev->mode_config.connector_list, head) { -- 2.10.2
Possibly Parallel Threads
- [PATCH v4 1/2] nouveau/bl: Assign different names to interfaces
- [PATCH v3 1/2] nouveau/bl: Assign different names to interfaces
- [PATCH v3 1/2] nouveau/bl: Assign different names to interfaces
- [PATCH 1/2] nouveau/bl: Assign different names to interfaces
- [PATCH 1/2] nouveau/bl: Assign different names to interfaces