On Mon, May 21, 2012 at 07:13:54AM +0100, Emil Velikov wrote: > On Mon, 21 May 2012 07:30:32 +0100, Ben Skeggs <skeg...@gmail.com> 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. > Point taken, I believe the whole therm subdev will need some love after > the connection with the i2c devices have been finalised > > > > >> > >>Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com> > >>--- > >> 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? > It boils to the point of - what is the reasonable approach to get > out of the > situation - call nouveau_subdev_fini()? How about cards that may not > have on > die sensor but have one via i2c ? My "vision" for this, not being overly experienced with the whole subsystem, is that nouveau_thermal_create() will detect what sensors are needed, instantiate the modules to control them and hook them up with the relevant cooling devices etc.
If this isn't necessary for a given board, nouveau_subdev_create() should never be called (or at least, destroyed again before returing from nouveau_therm_create()), and return 0 (so init doesn't completely fail). Ben. > > Regards > Emil Velikov _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau