Re: [Nouveau] [RFC PATCH] drm/nouveau/therm: initial implementation of new gp1xx temperature sensor

On Wed, Aug 30, 2017 at 10:55 PM, Rhys Kidd rhyskidd@gmail.com wrote:

Signed-off-by: Rhys Kidd rhyskidd@gmail.com

.../gpu/drm/nouveau/include/nvkm/subdev/therm.h | 1 + drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 6 +++ drivers/gpu/drm/nouveau/nvkm/subdev/therm/Kbuild | 1 + drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c | 3 +- drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c | 57 ++++++++++++++++++++++ 5 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h index b268b96faece..adf14731ea15 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h @@ -96,4 +96,5 @@ int g84_therm_new(struct nvkm_device *, int, struct nvkm_therm **); int gt215_therm_new(struct nvkm_device *, int, struct nvkm_therm **); int gf119_therm_new(struct nvkm_device *, int, struct nvkm_therm **); int gm107_therm_new(struct nvkm_device *, int, struct nvkm_therm **); +int gp100_therm_new(struct nvkm_device *, int, struct nvkm_therm **); #endif diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c index 7bdc7a5ae723..a6221b0996bf 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c @@ -2166,6 +2166,7 @@ nv130_chipset = { .ltc = gp100_ltc_new, .mc = gp100_mc_new, .mmu = gf100_mmu_new,

•   .therm = gp100_therm_new,
    .secboot = gm200_secboot_new,
    .pci = gp100_pci_new,
    .pmu = gp100_pmu_new,
  

@@ -2200,6 +2201,7 @@ nv132_chipset = { .ltc = gp100_ltc_new, .mc = gp100_mc_new, .mmu = gf100_mmu_new,

•   .therm = gp100_therm_new,
    .secboot = gp102_secboot_new,
    .pci = gp100_pci_new,
    .pmu = gp102_pmu_new,
  

@@ -2234,6 +2236,7 @@ nv134_chipset = { .ltc = gp100_ltc_new, .mc = gp100_mc_new, .mmu = gf100_mmu_new,

•   .therm = gp100_therm_new,
    .secboot = gp102_secboot_new,
    .pci = gp100_pci_new,
    .pmu = gp102_pmu_new,
  

@@ -2268,6 +2271,7 @@ nv136_chipset = { .ltc = gp100_ltc_new, .mc = gp100_mc_new, .mmu = gf100_mmu_new,

•   .therm = gp100_therm_new,
    .secboot = gp102_secboot_new,
    .pci = gp100_pci_new,
    .pmu = gp102_pmu_new,
  

@@ -2302,6 +2306,7 @@ nv137_chipset = { .ltc = gp100_ltc_new, .mc = gp100_mc_new, .mmu = gf100_mmu_new,

•   .therm = gp100_therm_new,
    .secboot = gp102_secboot_new,
    .pci = gp100_pci_new,
    .pmu = gp102_pmu_new,
  

Why not add it to nv138_chipset?

@@ -2332,6 +2337,7 @@ nv13b_chipset = { .ltc = gp100_ltc_new, .mc = gp10b_mc_new, .mmu = gf100_mmu_new,

•   .therm = gp100_therm_new,
  

GK20A and GM20B both don't have therm. I'm guessing this shouldn't either.

    .secboot = gp10b_secboot_new,
    .pmu = gm20b_pmu_new,
    .timer = gk20a_timer_new,

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/Kbuild b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/Kbuild index 135758ba3e28..3ef1b396a946 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/Kbuild +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/Kbuild @@ -11,3 +11,4 @@ nvkm-y += nvkm/subdev/therm/g84.o nvkm-y += nvkm/subdev/therm/gt215.o nvkm-y += nvkm/subdev/therm/gf119.o nvkm-y += nvkm/subdev/therm/gm107.o +nvkm-y += nvkm/subdev/therm/gp100.o diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c index 952a7cb0a59a..f27fc6d0d4c6 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c @@ -341,7 +341,8 @@ nvkm_therm_init(struct nvkm_subdev *subdev) { struct nvkm_therm *therm = nvkm_therm(subdev);

•   therm->func->init(therm);
  

•   if (therm->func->init)
  

•           therm->func->init(therm);
  
    if (therm->suspend >= 0) {
            /* restore the pwm value only when on manual or auto mode */
  

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c new file mode 100644 index 000000000000..b83d4af8c8bb --- /dev/null +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c @@ -0,0 +1,57 @@ +/*

• • Copyright 2017 Rhys Kidd
• • 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 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 HOLDER(S) OR AUTHOR(S) 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: Rhys Kidd
• */

+#include "priv.h"

+static int +gp100_temp_get(struct nvkm_therm *therm) +{

•   struct nvkm_device *device = therm->subdev.device;
  

•   struct nvkm_subdev *subdev = &therm->subdev;
  

•   u32 tsensor = nvkm_rd32(device, 0x20460);
  

0x020460 (registers are 24-bit values... this isn't done uniformly, but it's good practice)

•   u32 status  = (tsensor & 0x60000000) >> 29;
  

•   u32 t_f24_8 = (tsensor & 0x0001fff8);
  

•   /* device SHADOWed */
  

•   if (status & 0x00000002)
  

if (tsensor & 0x40000000)

•           nvkm_error(subdev, "Reading temperature from SHADOWed sensor\n");
  

Is this really worth doing for *every* read? Maybe nvkm_info or nvkm_debug? (What's the significance of this?)

•   /* device valid */
  

•   if (status & 0x00000001)
  

if (tsensor & 0x20000000)

•           return t_f24_8 / 256;
  

aka >> 8

•   else
  

•           return -ENODEV;
  

+}

+static const struct nvkm_therm_func +gp100_therm = {

•   .temp_get = gp100_temp_get,
  

•   .program_alarms = nvkm_therm_program_alarms_polling,
  

+};

+int +gp100_therm_new(struct nvkm_device *device, int index,

•           struct nvkm_therm **ptherm)
  

+{

•   return nvkm_therm_new_(&gp100_therm, device, index, ptherm);
  

+} \ No newline at end of file

Please fix this.

-- 2.11.0

---

Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau