Nouveau - Jan 2018 - [PATCH] drm/nouveau/therm/gp100: Do not report temperature when subdev is shadowed

This fixes wrong temperature outputs e.g. 511°C if the card is asleep.

Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
index 9f0dea3f61dc..45d0ec632b5a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
@@ -32,8 +32,10 @@ gp100_temp_get(struct nvkm_therm *therm)
 	u32 inttemp = (tsensor & 0x0001fff8);
 
 	/* device SHADOWed */
-	if (tsensor & 0x40000000)
+	if (tsensor & 0x40000000) {
 		nvkm_trace(subdev, "reading temperature from SHADOWed sensor\n");
+		return -ENODEV;
+	}
 
 	/* device valid */
 	if (tsensor & 0x20000000)
-- 
2.16.1

On 25 January 2018 at 13:16, Tobias Klausmann <
tobias.johannes.klausmann at mni.thm.de> wrote:
> This fixes wrong temperature outputs e.g. 511°C if the card is asleep.
>
> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at
mni.thm.de>
>
LGTM,

Reviewed-by: Rhys Kidd <rhyskidd at gmail.com>

> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
> index 9f0dea3f61dc..45d0ec632b5a 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
> @@ -32,8 +32,10 @@ gp100_temp_get(struct nvkm_therm *therm)
>         u32 inttemp = (tsensor & 0x0001fff8);
>
>         /* device SHADOWed */
> -       if (tsensor & 0x40000000)
> +       if (tsensor & 0x40000000) {
>                 nvkm_trace(subdev, "reading temperature from SHADOWed
> sensor\n");
> +               return -ENODEV;
> +       }
>
>         /* device valid */
>         if (tsensor & 0x20000000)
> --
> 2.16.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20180125/5267984e/attachment.html>

no, we can't do that. We actually have to prevent this from hwom. The
issue here is, that the reg read returns 0xffffffff and parsing that
is the first step in the first place.

On Thu, Jan 25, 2018 at 7:16 PM, Tobias Klausmann
<tobias.johannes.klausmann at mni.thm.de> wrote:
> This fixes wrong temperature outputs e.g. 511°C if the card is asleep.
>
> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at
mni.thm.de>
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
> index 9f0dea3f61dc..45d0ec632b5a 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
> @@ -32,8 +32,10 @@ gp100_temp_get(struct nvkm_therm *therm)
>         u32 inttemp = (tsensor & 0x0001fff8);
>
>         /* device SHADOWed */
> -       if (tsensor & 0x40000000)
> +       if (tsensor & 0x40000000) {
>                 nvkm_trace(subdev, "reading temperature from SHADOWed
sensor\n");
> +               return -ENODEV;
> +       }
>
>         /* device valid */
>         if (tsensor & 0x20000000)
> --
> 2.16.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Not sure if i understand completely what you intend to say here, with 
this we prevent hwmon from reporting utterly wrong temperature values 
returning an error (we could return -EBUSY or somehting instead, 
granted), yet if the device is shadowed, getting a sane temp value out 
of is seems unlikely to me!

Greetings,

Tobias

On 1/26/18 12:40 PM, Karol Herbst wrote:
> no, we can't do that. We actually have to prevent this from hwom. The
> issue here is, that the reg read returns 0xffffffff and parsing that
> is the first step in the first place.
>
> On Thu, Jan 25, 2018 at 7:16 PM, Tobias Klausmann
> <tobias.johannes.klausmann at mni.thm.de> wrote:
>> This fixes wrong temperature outputs e.g. 511°C if the card is asleep.
>>
>> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at
mni.thm.de>
>> ---
>>   drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
>> index 9f0dea3f61dc..45d0ec632b5a 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/gp100.c
>> @@ -32,8 +32,10 @@ gp100_temp_get(struct nvkm_therm *therm)
>>          u32 inttemp = (tsensor & 0x0001fff8);
>>
>>          /* device SHADOWed */
>> -       if (tsensor & 0x40000000)
>> +       if (tsensor & 0x40000000) {
>>                  nvkm_trace(subdev, "reading temperature from
SHADOWed sensor\n");
>> +               return -ENODEV;
>> +       }
>>
>>          /* device valid */
>>          if (tsensor & 0x20000000)
>> --
>> 2.16.1
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau