2016-08-23 16:06 GMT+02:00 Martin Peres <martin.pe...@free.fr>: > On 23/08/16 11:31, Karol Herbst wrote: >> >> maybe it makes sense to expose the SLI LED, too. >> >> Regardless of my comments this patch is reviewed-by me. > > > You reviewed the wrong patch, I should have named the re-send v3. > > I accidentally sent the v1 patch as a v2 :s >
yeah, mailman was stupid today. I got your new version way too late. rby still valid though. >> >> 2016-08-23 1:39 GMT+02:00 Martin Peres <martin.pe...@free.fr>: >>> >>> We received a donation of a Titan which has this useless feature >>> allowing users to control the brightness of the LED behind the >>> logo of NVIDIA. In the true spirit of open source, let's expose >>> that to the users of very expensive cards! >>> >>> This patch hooks up this LED/PWM to the LED subsystem which allows >>> blinking it in sync with cpu/disk/network/whatever activity (heartbeat >>> is quite nice!). Users may also implement some breathing effect or >>> morse code support in the userspace if they feel like it. >>> >>> Signed-off-by: Martin Peres <martin.pe...@free.fr> >>> --- >>> drm/nouveau/Kbuild | 1 + >>> drm/nouveau/include/nvkm/subdev/bios/gpio.h | 1 + >>> drm/nouveau/nouveau_drm.c | 7 ++ >>> drm/nouveau/nouveau_drm.h | 3 + >>> drm/nouveau/nouveau_led.c | 131 >>> ++++++++++++++++++++++++++++ >>> drm/nouveau/nouveau_led.h | 48 ++++++++++ >>> 6 files changed, 191 insertions(+) >>> create mode 100644 drm/nouveau/nouveau_led.c >>> create mode 100644 drm/nouveau/nouveau_led.h >>> >>> diff --git a/drm/nouveau/Kbuild b/drm/nouveau/Kbuild >>> index 2527bf4..312bca9 100644 >>> --- a/drm/nouveau/Kbuild >>> +++ b/drm/nouveau/Kbuild >>> @@ -22,6 +22,7 @@ nouveau-$(CONFIG_DEBUG_FS) += nouveau_debugfs.o >>> nouveau-y += nouveau_drm.o >>> nouveau-y += nouveau_hwmon.o >>> nouveau-$(CONFIG_COMPAT) += nouveau_ioc32.o >>> +nouveau-y += nouveau_led.o >>> nouveau-y += nouveau_nvif.o >>> nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o >>> nouveau-y += nouveau_usif.o # userspace <-> nvif >>> diff --git a/drm/nouveau/include/nvkm/subdev/bios/gpio.h >>> b/drm/nouveau/include/nvkm/subdev/bios/gpio.h >>> index a47d46d..b7a54e6 100644 >>> --- a/drm/nouveau/include/nvkm/subdev/bios/gpio.h >>> +++ b/drm/nouveau/include/nvkm/subdev/bios/gpio.h >>> @@ -6,6 +6,7 @@ enum dcb_gpio_func_name { >>> DCB_GPIO_TVDAC1 = 0x2d, >>> DCB_GPIO_FAN = 0x09, >>> DCB_GPIO_FAN_SENSE = 0x3d, >>> + DCB_GPIO_LOGO_LED_PWM = 0x84, >>> DCB_GPIO_UNUSED = 0xff, >>> DCB_GPIO_VID0 = 0x04, >>> DCB_GPIO_VID1 = 0x05, >>> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c >>> index d06877d..dc97401 100644 >>> --- a/drm/nouveau/nouveau_drm.c >>> +++ b/drm/nouveau/nouveau_drm.c >>> @@ -49,6 +49,7 @@ >>> #include "nouveau_ttm.h" >>> #include "nouveau_gem.h" >>> #include "nouveau_vga.h" >>> +#include "nouveau_led.h" >>> #include "nouveau_hwmon.h" >>> #include "nouveau_acpi.h" >>> #include "nouveau_bios.h" >>> @@ -468,6 +469,7 @@ nouveau_drm_load(struct drm_device *dev, unsigned >>> long flags) >>> nouveau_hwmon_init(dev); >>> nouveau_accel_init(drm); >>> nouveau_fbcon_init(dev); >>> + nouveau_led_init(dev); >>> >>> if (nouveau_runtime_pm != 0) { >>> pm_runtime_use_autosuspend(dev->dev); >>> @@ -499,6 +501,7 @@ nouveau_drm_unload(struct drm_device *dev) >>> struct nouveau_drm *drm = nouveau_drm(dev); >>> >>> pm_runtime_get_sync(dev->dev); >>> + nouveau_led_fini(dev); >>> nouveau_fbcon_fini(dev); >>> nouveau_accel_fini(drm); >>> nouveau_hwmon_fini(dev); >>> @@ -550,6 +553,8 @@ nouveau_do_suspend(struct drm_device *dev, bool >>> runtime) >>> struct nouveau_cli *cli; >>> int ret; >>> >>> + nouveau_led_suspend(dev); >>> + >>> if (dev->mode_config.num_crtc) { >>> NV_INFO(drm, "suspending console...\n"); >>> nouveau_fbcon_set_suspend(dev, 1); >>> @@ -638,6 +643,8 @@ nouveau_do_resume(struct drm_device *dev, bool >>> runtime) >>> nouveau_fbcon_set_suspend(dev, 0); >>> } >>> >>> + nouveau_led_resume(dev); >>> + >>> return 0; >>> } >>> >>> diff --git a/drm/nouveau/nouveau_drm.h b/drm/nouveau/nouveau_drm.h >>> index 5c363ed..b148dcb 100644 >>> --- a/drm/nouveau/nouveau_drm.h >>> +++ b/drm/nouveau/nouveau_drm.h >>> @@ -166,6 +166,9 @@ struct nouveau_drm { >>> struct nouveau_hwmon *hwmon; >>> struct nouveau_debugfs *debugfs; >>> >>> + /* led management */ >>> + struct nouveau_led *led; >>> + >>> /* display power reference */ >>> bool have_disp_power_ref; >>> >>> diff --git a/drm/nouveau/nouveau_led.c b/drm/nouveau/nouveau_led.c >>> new file mode 100644 >>> index 0000000..3f23ff6 >>> --- /dev/null >>> +++ b/drm/nouveau/nouveau_led.c >>> @@ -0,0 +1,131 @@ >>> +/* >>> + * Copyright (C) 2016 Martin Peres >>> + * >>> + * Permission is hereby granted, free of charge, to any person obtaining >>> + * a copy of this software and associated documentation files (the >>> + * "Software"), to deal in the Software without restriction, including >>> + * without limitation the rights to use, copy, modify, merge, publish, >>> + * distribute, sublicense, and/or sell copies of the Software, and to >>> + * permit persons to whom the Software is furnished to do so, subject to >>> + * the following conditions: >>> + * >>> + * The above copyright notice and this permission notice (including the >>> + * next paragraph) shall be included in all copies or substantial >>> + * portions of the Software. >>> + * >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND >>> NONINFRINGEMENT. >>> + * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE >>> + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN >>> ACTION >>> + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION >>> + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. >>> + * >>> + */ >>> + >>> +/* >>> + * Authors: >>> + * Martin Peres <martin.pe...@free.fr> >>> + */ >>> + >>> +#include <linux/leds.h> >>> + >>> +#include "nouveau_drm.h" >>> +#include "nouveau_led.h" >>> +#include <nvkm/subdev/gpio.h> >>> + >>> +static enum led_brightness >>> +nouveau_led_get_brightness(struct led_classdev *led) >>> +{ >>> + struct drm_device *drm_dev = container_of(led, struct >>> nouveau_led, led)->dev; >>> + struct nouveau_drm *drm = nouveau_drm(drm_dev); >>> + struct nvif_object *device = &drm->device.object; >>> + u32 div, duty; >>> + >>> + div = nvif_rd32(device, 0x61c880) & 0x00ffffff; >>> + duty = nvif_rd32(device, 0x61c884) & 0x00ffffff; >>> + >>> + return duty * LED_FULL / div; >>> +} >>> + >>> +static void >>> +nouveau_led_set_brightness(struct led_classdev *led, enum led_brightness >>> value) >>> +{ >>> + struct drm_device *drm_dev = container_of(led, struct >>> nouveau_led, led)->dev; >>> + struct nouveau_drm *drm = nouveau_drm(drm_dev); >>> + struct nvif_object *device = &drm->device.object; >>> + >>> + u32 input_clk = 27e6; /* PDISPLAY.SOR[1].PWM is connected to the >>> crystal */ >>> + u32 freq = 100; /* this is what nvidia uses and it should be >>> good-enough */ >>> + u32 div, duty; >>> + >>> + div = input_clk / freq; >>> + duty = value * div / LED_FULL; >>> + >>> + /* for now, this is safe to directly poke those registers >>> because: >>> + * - A: nvidia never puts the logo led to any other PWM >>> controler >>> + * than PDISPLAY.SOR[1].PWM. >>> + * - B: nouveau does not touch these registers anywhere else >>> + */ >>> + nvif_wr32(device, 0x61c880, div); >>> + nvif_wr32(device, 0x61c884, 0xc0000000 | duty); >> >> I feel like there should be a comment explaining the 0x40000000 >> (connected to crystal). Or maybe we should be in general more strict >> about using constants, I feel like those hardcoded magic numbers are >> hard to understand by users. > > > Well, true, but as long as it is well documented in rnndb, I think it is the > proper way to go. Otherwise, we would need to maintain macros which we tried > and failed at :s > well, not everybody knows where to look for stuff. But this is an issue unrelated to your patch anyway. > In this particular case, the comments at the top of the function should give > all the information. > > We need to make the use of lookup known: > > $ lookup -a E4 0x61c884 0xc0000000 > PDISPLAY.SOR[0x1].PWM_CTL => { DUTY = 0 | CLOCK_SELECT = UNK1 | TRIGGER } > > OK, I forgot to update rnnDB :s upstream or local? The XML looked fine to me, but maybe I looked at the wrong thing. nvm. _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau