On Mon, May 21, 2012 at 8:30 AM, Ben Skeggs <[email protected]> wrote: > On Mon, May 21, 2012 at 12:15:03AM +0100, Emil Velikov wrote: >> It contains a few changes mainly targeting the following >> * Therm table is present in BIT vbios >> * Parse the vbios table before hooking temp_get(), as it contains the therm >> sensor calibration data >> * Add dummy_therm_temp_get() function to prevent multiple null dereff's > > I didn't take this patch at all yet. I'll let Martin put his input into > this instead. I didn't really touch the thermal stuff aside from matching > APIs because he's got an overhaul pending anyway. > > Comments on specific pieces inline as they may be useful. > >> >> Signed-off-by: Emil Velikov <[email protected]> >> --- >> drivers/gpu/drm/nouveau/nouveau_pm.c | 2 +- >> drivers/gpu/drm/nouveau/nouveau_therm.c | 63 >> ++++++++++++++++++++++++------- >> 2 files changed, 50 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/nouveau/nouveau_pm.c >> b/drivers/gpu/drm/nouveau/nouveau_pm.c >> index 9dd34fe..1b4422b 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_pm.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_pm.c >> @@ -693,7 +693,7 @@ nouveau_hwmon_init(struct nouveau_device *ndev) >> } >> >> /* if the card can read the fan rpm */ >> - if (nouveau_gpio_func_valid(ndev, DCB_GPIO_FAN_SENSE)) { >> + if (pfan && pfan->sense(pfan) >= 0) { >> ret = sysfs_create_group(&dev->pdev->dev.kobj, >> &hwmon_fan_rpm_attrgroup); >> if (ret) >> diff --git a/drivers/gpu/drm/nouveau/nouveau_therm.c >> b/drivers/gpu/drm/nouveau/nouveau_therm.c >> index acf81a9..91095be 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_therm.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_therm.c >> @@ -30,6 +30,12 @@ >> #include "nouveau_pm.h" >> #include "nouveau_therm.h" >> >> +static inline int >> +dummy_therm_temp_get(struct nouveau_therm *ptherm) >> +{ >> + return 0; >> +} >> + > I don't really like this, if we can't expose any thermal data I think we > just shouldn't create a thermal subdev? > >> static int >> nv40_sensor_setup(struct nouveau_therm *ptherm) >> { >> @@ -181,7 +187,7 @@ nouveau_therm_vbios_parse(struct nouveau_therm *ptherm, >> u8 *temp) >> temp = temp + headerlen; >> >> /* Read the entries from the table */ >> - for (i = 0; i < entries; i++) { >> + for (i = 0; i < entries; i++, temp += recordlen) { >> s16 value = ROM16(temp[1]); >> >> switch (temp[0]) { >> @@ -228,7 +234,6 @@ nouveau_therm_vbios_parse(struct nouveau_therm *ptherm, >> u8 *temp) >> ptherm->fan.pwm_freq = value; >> break; >> } >> - temp += recordlen; >> } >> >> nouveau_therm_safety_checks(ptherm); >> @@ -299,6 +304,12 @@ nouveau_therm_probe_i2c(struct nouveau_device *ndev) >> probe_monitoring_device, NV_I2C_DEFAULT(0)); >> } >> >> +static void >> +nouveau_ptherm_destroy(struct nouveau_device *ndev, int subdev) >> +{ >> +// XXX: Undo probe_monitoring_device >> +} >> + >> int >> nouveau_therm_create(struct nouveau_device *ndev, int subdev) >> { >> @@ -307,29 +318,53 @@ nouveau_therm_create(struct nouveau_device *ndev, int >> subdev) >> u8 *temp = NULL; >> int ret; >> >> - ret = nouveau_subdev_create(ndev, subdev, "THERM", "thermal", &ptherm); >> - if (ret) >> - return ret; >> + if (bios->type == NVBIOS_BIT) { >> + if (bit_table(ndev, 'P', &P)) >> + return 0; > The BIT check isn't necessary, bit_table() will fail if it's not a BIT BIOS. > >> >> - if (ndev->chipset >= 0x40 && ndev->chipset < 0x84) >> - ptherm->temp_get = nv40_therm_temp_get; >> - else >> - if (ndev->chipset <= 0xd9) >> - ptherm->temp_get = nv84_therm_temp_get; >> - >> - if (bit_table(ndev, 'P', &P) == 0) { >> if (P.version == 1) >> temp = ROMPTR(ndev, P.data[12]); >> else >> if (P.version == 2) >> temp = ROMPTR(ndev, P.data[16]); >> - else >> + else { >> NV_WARN(ndev, "unknown temp for BIT P %d\n", >> P.version); >> + } >> + } else { >> + return 0; >> + } >> >> - nouveau_therm_vbios_parse(ptherm, temp); >> + if (!temp) { >> + NV_DEBUG(ndev, "temp table pointer invalid\n"); >> + return 0; >> } >> >> + ret = nouveau_subdev_create(ndev, subdev, "THERM", "thermal", &ptherm); >> + if (ret) >> + return ret; >> + >> + nouveau_therm_vbios_parse(ptherm, temp); >> nouveau_therm_probe_i2c(ndev); >> >> + ptherm->base.destroy = nouveau_ptherm_destroy; >> + switch (ndev->card_type) { >> + case NV_40: >> + case NV_50: >> + case NV_C0: >> + case NV_D0: >> + case NV_E0: >> + if (ndev->chipset < 0x84) { >> + ptherm->temp_get = nv40_therm_temp_get; >> + break; >> + } else >> + if (ndev->chipset <= 0xd9) { >> + ptherm->temp_get = nv84_therm_temp_get; >> + break; >> + } >> + default: >> + ptherm->temp_get = dummy_therm_temp_get; >> + break; >> + } >> + >> return nouveau_subdev_init(ndev, subdev, ret); >> } >> -- >> 1.7.10.2 >> >> _______________________________________________ >> Nouveau mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/nouveau > _______________________________________________ > Nouveau mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/nouveau
Does chipset 0x50 really use the NV40 function? -- Far away from the primal instinct, the song seems to fade away, the river get wider between your thoughts and the things we do and say. _______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
