[Nouveau] [PATCH 14/14] drm/nouveau/therm: Rework nouveau_therm_create()

[Emil Velikov]

On Mon, 21 May 2012 07:30:32 +0100, Ben Skeggs <skeggsb at 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.velikov at 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 ?

Regards
Emil Velikov