Nouveau - Aug 2013 - [PATCH 0/5] Thermal management fixes

From: Martin Peres <martin.peres at labri.fr>

This patchset is mostly about fixing fdo bug #66177, reported by Dash Four.
This bug is about fan/temp management not working after a suspend/resume cycle.

Fan/therm management relies on ptimer's alarm feature to call periodically
multiple callbacks that poll the temperature and update the fan speed if needed.
The problem is that there is no easy way to make this work across suspend or
hibernate cycles because it would require making sure the IRQs aren't
serviced
until all nouveau engines and subdevs are initialized after resume. This means
the alarm feature should be the last thing re-activated again. 

The proposed solution is a bit simpler. Ptimer.alarm's clients should cancel
their scheduled tasks before suspending and re-add them when resuming. The code
is thus very simple.

Patch 3 should also fix fan speed reading while suspending.

Emil Velikov (1):
  drm/nouveau/therm: Set the correct pwm_mode upon resume

Martin Peres (4):
  drm/nouveau/fan: restore the PWM value on resume when using the manual
    or auto mode
  drm/nouveau/timer: restore the time on resume
  drm/nouveau/timer: add a way to cancel alarms
  drm/nouveau/therm: survive to suspend/resume cycles

 .../gpu/drm/nouveau/core/include/subdev/timer.h    |  2 ++
 drivers/gpu/drm/nouveau/core/subdev/therm/base.c   | 19 ++++++++++--
 drivers/gpu/drm/nouveau/core/subdev/therm/fan.c    | 20 +++++++++++++
 drivers/gpu/drm/nouveau/core/subdev/therm/priv.h   |  4 +++
 drivers/gpu/drm/nouveau/core/subdev/therm/temp.c   | 21 +++++++++++++
 drivers/gpu/drm/nouveau/core/subdev/timer/base.c   |  7 +++++
 drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c   | 35 +++++++++++++++++++++-
 7 files changed, 104 insertions(+), 4 deletions(-)

-- 
1.8.3.4

From: Emil Velikov <emil.l.velikov at gmail.com>

Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
Signed-off-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Dash Four <mr.dash.four at googlemail.com>
---
 drivers/gpu/drm/nouveau/core/subdev/therm/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
b/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
index a00a5a7..3e9d941 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
@@ -268,7 +268,7 @@ _nouveau_therm_init(struct nouveau_object *object)
 		return ret;
 
 	if (priv->suspend >= 0)
-		nouveau_therm_fan_mode(therm, priv->mode);
+		nouveau_therm_fan_mode(therm, priv->suspend);
 	priv->sensor.program_alarms(therm);
 	return 0;
 }
-- 
1.8.3.4

From: Martin Peres <martin.peres at labri.fr>

If the fan was in manual or auto mode, we should restore the fan speed that was
previously set when resuming.

The initial pwm value is saved when loading the module.

Signed-off-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Dash Four <mr.dash.four at googlemail.com>
---
 drivers/gpu/drm/nouveau/core/subdev/therm/base.c | 7 ++++++-
 drivers/gpu/drm/nouveau/core/subdev/therm/fan.c  | 3 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
b/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
index 3e9d941..2ada3d7 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
@@ -267,8 +267,13 @@ _nouveau_therm_init(struct nouveau_object *object)
 	if (ret)
 		return ret;
 
-	if (priv->suspend >= 0)
+	if (priv->suspend >= 0) {
+		/* restore the pwm value only when on manual or auto mode */
+		if (priv->suspend > 0)
+			nouveau_therm_fan_set(therm, true, priv->fan->percent);
+
 		nouveau_therm_fan_mode(therm, priv->suspend);
+	}
 	priv->sensor.program_alarms(therm);
 	return 0;
 }
diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/fan.c
b/drivers/gpu/drm/nouveau/core/subdev/therm/fan.c
index c728380..6d411e1 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/fan.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/fan.c
@@ -233,6 +233,9 @@ nouveau_therm_fan_ctor(struct nouveau_therm *therm)
 	}
 
 	nv_info(therm, "FAN control: %s\n", priv->fan->type);
+	
+	/* read the current speed, it is useful when resuming */
+	priv->fan->percent = nouveau_therm_fan_get(therm);
 
 	/* attempt to detect a tachometer connection */
 	ret = gpio->find(gpio, 0, DCB_GPIO_FAN_SENSE, 0xff,
&priv->fan->tach);
-- 
1.8.3.4

From: Martin Peres <martin.peres at labri.fr>

This can be useful if some parts of Nouveau try to calculate the time between
two events. Without this patch, the time difference would be negative in the
case where the computer is suspended/resumed between two events.
This patch should fix fan speed probing when done while suspending/resuming.

Solve this by saving the current time before suspending and by restoring it
on resume.

Signed-off-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Martin Peres <martin.peres at labri.fr>
---
 drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c
b/drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c
index 9469b82..8c72da6 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c
@@ -36,6 +36,7 @@ struct nv04_timer_priv {
 	struct nouveau_timer base;
 	struct list_head alarms;
 	spinlock_t lock;
+	u64 suspend_time;
 };
 
 static u64
@@ -146,6 +147,7 @@ nv04_timer_ctor(struct nouveau_object *parent, struct
nouveau_object *engine,
 	priv->base.base.intr = nv04_timer_intr;
 	priv->base.read = nv04_timer_read;
 	priv->base.alarm = nv04_timer_alarm;
+	priv->suspend_time = 0;
 
 	INIT_LIST_HEAD(&priv->alarms);
 	spin_lock_init(&priv->lock);
@@ -164,7 +166,7 @@ nv04_timer_init(struct nouveau_object *object)
 {
 	struct nouveau_device *device = nv_device(object);
 	struct nv04_timer_priv *priv = (void *)object;
-	u32 m = 1, f, n, d;
+	u32 m = 1, f, n, d, lo, hi;
 	int ret;
 
 	ret = nouveau_timer_init(&priv->base);
@@ -220,17 +222,26 @@ nv04_timer_init(struct nouveau_object *object)
 		n >>= 1;
 		d >>= 1;
 	}
+	
+	/* restore the time before suspend */
+	lo = priv->suspend_time;
+	hi = (priv->suspend_time >> 32);
 
 	nv_debug(priv, "input frequency : %dHz\n", f);
 	nv_debug(priv, "input multiplier: %d\n", m);
 	nv_debug(priv, "numerator       : 0x%08x\n", n);
 	nv_debug(priv, "denominator     : 0x%08x\n", d);
 	nv_debug(priv, "timer frequency : %dHz\n", (f * m) * d / n);
+	nv_debug(priv, "time low        : 0x%08x\n", lo);
+	nv_debug(priv, "time high       : 0x%08x\n", hi);
 
 	nv_wr32(priv, NV04_PTIMER_NUMERATOR, n);
 	nv_wr32(priv, NV04_PTIMER_DENOMINATOR, d);
 	nv_wr32(priv, NV04_PTIMER_INTR_0, 0xffffffff);
 	nv_wr32(priv, NV04_PTIMER_INTR_EN_0, 0x00000000);
+	nv_wr32(priv, NV04_PTIMER_TIME_1, hi);
+	nv_wr32(priv, NV04_PTIMER_TIME_0, lo);
+
 	return 0;
 }
 
@@ -238,6 +249,8 @@ static int
 nv04_timer_fini(struct nouveau_object *object, bool suspend)
 {
 	struct nv04_timer_priv *priv = (void *)object;
+	if (suspend)
+		priv->suspend_time = nv04_timer_read(&priv->base);
 	nv_wr32(priv, NV04_PTIMER_INTR_EN_0, 0x00000000);
 	return nouveau_timer_fini(&priv->base, suspend);
 }
-- 
1.8.3.4

From: Martin Peres <martin.peres at labri.fr>

Since alarms don't play well with suspend, it is important every alarm user
cancels his tasks before suspending. The task should be rescheduled on resume.

Signed-off-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Dash Four <mr.dash.four at googlemail.com>
---
 drivers/gpu/drm/nouveau/core/include/subdev/timer.h |  2 ++
 drivers/gpu/drm/nouveau/core/subdev/timer/base.c    |  7 +++++++
 drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c    | 20 ++++++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/timer.h
b/drivers/gpu/drm/nouveau/core/include/subdev/timer.h
index e465d15..9ab70df 100644
--- a/drivers/gpu/drm/nouveau/core/include/subdev/timer.h
+++ b/drivers/gpu/drm/nouveau/core/include/subdev/timer.h
@@ -22,6 +22,7 @@ bool nouveau_timer_wait_eq(void *, u64 nsec, u32 addr, u32
mask, u32 data);
 bool nouveau_timer_wait_ne(void *, u64 nsec, u32 addr, u32 mask, u32 data);
 bool nouveau_timer_wait_cb(void *, u64 nsec, bool (*func)(void *), void *data);
 void nouveau_timer_alarm(void *, u32 nsec, struct nouveau_alarm *);
+void nouveau_timer_alarm_cancel(void *, struct nouveau_alarm *);
 
 #define NV_WAIT_DEFAULT 2000000000ULL
 #define nv_wait(o,a,m,v)                                                      
\
@@ -35,6 +36,7 @@ struct nouveau_timer {
 	struct nouveau_subdev base;
 	u64  (*read)(struct nouveau_timer *);
 	void (*alarm)(struct nouveau_timer *, u64 time, struct nouveau_alarm *);
+	void (*alarm_cancel)(struct nouveau_timer *, struct nouveau_alarm *);
 };
 
 static inline struct nouveau_timer *
diff --git a/drivers/gpu/drm/nouveau/core/subdev/timer/base.c
b/drivers/gpu/drm/nouveau/core/subdev/timer/base.c
index 5d417cc..cf8a0e0 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/timer/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/timer/base.c
@@ -85,3 +85,10 @@ nouveau_timer_alarm(void *obj, u32 nsec, struct nouveau_alarm
*alarm)
 	struct nouveau_timer *ptimer = nouveau_timer(obj);
 	ptimer->alarm(ptimer, nsec, alarm);
 }
+
+void
+nouveau_timer_alarm_cancel(void *obj, struct nouveau_alarm *alarm)
+{
+	struct nouveau_timer *ptimer = nouveau_timer(obj);
+	ptimer->alarm_cancel(ptimer, alarm);
+}
diff --git a/drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c
b/drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c
index 8c72da6..e0d4fd1 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/timer/nv04.c
@@ -114,6 +114,25 @@ nv04_timer_alarm(struct nouveau_timer *ptimer, u64 time,
 }
 
 static void
+nv04_timer_alarm_cancel(struct nouveau_timer *ptimer,
+			struct nouveau_alarm *alarm)
+{
+	struct nv04_timer_priv *priv = (void *)ptimer;
+	unsigned long flags;
+
+	/* avoid deleting an entry while the alarm intr is running */
+	spin_lock_irqsave(&priv->lock, flags);
+
+	/* delete the alarm from the list */
+	list_del(&alarm->head);
+
+	/* reset the head so as list_empty returns 1 */
+	INIT_LIST_HEAD(&alarm->head);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static void
 nv04_timer_intr(struct nouveau_subdev *subdev)
 {
 	struct nv04_timer_priv *priv = (void *)subdev;
@@ -147,6 +166,7 @@ nv04_timer_ctor(struct nouveau_object *parent, struct
nouveau_object *engine,
 	priv->base.base.intr = nv04_timer_intr;
 	priv->base.read = nv04_timer_read;
 	priv->base.alarm = nv04_timer_alarm;
+	priv->base.alarm_cancel = nv04_timer_alarm_cancel;
 	priv->suspend_time = 0;
 
 	INIT_LIST_HEAD(&priv->alarms);
-- 
1.8.3.4

From: Martin Peres <martin.peres at labri.fr>

Therm uses 3 ptimer alarms. Two to drive the fan and one for polling the
temperature. When suspending/resuming, alarms will never be fired.
As we are checking if there isn't an alarm pending before rescheduling
another one, we end up never checking temperature or updating the
fan speed.

This commit also adds debug messages to be able to spot more easily
if this case happens again in the future. Sorry for the spam if you
activate the debug level though.

Tested-by: Dash Four <mr.dash.four at googlemail.com>

v2:
- fix temperature polling too

Signed-off-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Martin Peres <martin.peres at labri.fr>
---
 drivers/gpu/drm/nouveau/core/subdev/therm/base.c | 10 +++++++++-
 drivers/gpu/drm/nouveau/core/subdev/therm/fan.c  | 19 ++++++++++++++++++-
 drivers/gpu/drm/nouveau/core/subdev/therm/priv.h |  4 ++++
 drivers/gpu/drm/nouveau/core/subdev/therm/temp.c | 21 +++++++++++++++++++++
 4 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
b/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
index 2ada3d7..f1de7a9 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/base.c
@@ -95,12 +95,14 @@ nouveau_therm_update(struct nouveau_therm *therm, int mode)
 	int duty;
 
 	spin_lock_irqsave(&priv->lock, flags);
+	nv_debug(therm, "FAN speed check\n");
 	if (mode < 0)
 		mode = priv->mode;
 	priv->mode = mode;
 
 	switch (mode) {
 	case NOUVEAU_THERM_CTRL_MANUAL:
+		ptimer->alarm_cancel(ptimer, &priv->alarm);
 		duty = nouveau_therm_fan_get(therm);
 		if (duty < 0)
 			duty = 100;
@@ -113,6 +115,7 @@ nouveau_therm_update(struct nouveau_therm *therm, int mode)
 		break;
 	case NOUVEAU_THERM_CTRL_NONE:
 	default:
+		ptimer->alarm_cancel(ptimer, &priv->alarm);
 		goto done;
 	}
 
@@ -122,6 +125,8 @@ nouveau_therm_update(struct nouveau_therm *therm, int mode)
 done:
 	if (list_empty(&priv->alarm.head) && (mode ==
NOUVEAU_THERM_CTRL_AUTO))
 		ptimer->alarm(ptimer, 1000000000ULL, &priv->alarm);
+	else if (!list_empty(&priv->alarm.head))
+		nv_debug(therm, "therm fan alarm list is not empty\n");
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
@@ -274,7 +279,8 @@ _nouveau_therm_init(struct nouveau_object *object)
 
 		nouveau_therm_fan_mode(therm, priv->suspend);
 	}
-	priv->sensor.program_alarms(therm);
+	nouveau_therm_sensor_init(therm);
+	nouveau_therm_fan_init(therm);
 	return 0;
 }
 
@@ -284,6 +290,8 @@ _nouveau_therm_fini(struct nouveau_object *object, bool
suspend)
 	struct nouveau_therm *therm = (void *)object;
 	struct nouveau_therm_priv *priv = (void *)therm;
 
+	nouveau_therm_fan_fini(therm, suspend);
+	nouveau_therm_sensor_fini(therm, suspend);
 	if (suspend) {
 		priv->suspend = priv->mode;
 		priv->mode = NOUVEAU_THERM_CTRL_NONE;
diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/fan.c
b/drivers/gpu/drm/nouveau/core/subdev/therm/fan.c
index 6d411e1..39f47b9 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/fan.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/fan.c
@@ -204,6 +204,23 @@ nouveau_therm_fan_safety_checks(struct nouveau_therm
*therm)
 }
 
 int
+nouveau_therm_fan_init(struct nouveau_therm *therm)
+{
+	return 0;
+}
+
+int
+nouveau_therm_fan_fini(struct nouveau_therm *therm, bool suspend)
+{
+	struct nouveau_therm_priv *priv = (void *)therm;
+	struct nouveau_timer *ptimer = nouveau_timer(therm);
+
+	if (suspend)
+		ptimer->alarm_cancel(ptimer, &priv->fan->alarm);
+	return 0;
+}
+
+int
 nouveau_therm_fan_ctor(struct nouveau_therm *therm)
 {
 	struct nouveau_therm_priv *priv = (void *)therm;
@@ -233,7 +250,7 @@ nouveau_therm_fan_ctor(struct nouveau_therm *therm)
 	}
 
 	nv_info(therm, "FAN control: %s\n", priv->fan->type);
-	
+
 	/* read the current speed, it is useful when resuming */
 	priv->fan->percent = nouveau_therm_fan_get(therm);
 
diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/priv.h
b/drivers/gpu/drm/nouveau/core/subdev/therm/priv.h
index 15ca64e..dd38529 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/priv.h
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/priv.h
@@ -113,6 +113,8 @@ void nouveau_therm_ic_ctor(struct nouveau_therm *therm);
 int nouveau_therm_sensor_ctor(struct nouveau_therm *therm);
 
 int nouveau_therm_fan_ctor(struct nouveau_therm *therm);
+int nouveau_therm_fan_init(struct nouveau_therm *therm);
+int nouveau_therm_fan_fini(struct nouveau_therm *therm, bool suspend);
 int nouveau_therm_fan_get(struct nouveau_therm *therm);
 int nouveau_therm_fan_set(struct nouveau_therm *therm, bool now, int percent);
 int nouveau_therm_fan_user_get(struct nouveau_therm *therm);
@@ -122,6 +124,8 @@ int nouveau_therm_fan_sense(struct nouveau_therm *therm);
 
 int nouveau_therm_preinit(struct nouveau_therm *);
 
+int nouveau_therm_sensor_init(struct nouveau_therm *therm);
+int nouveau_therm_sensor_fini(struct nouveau_therm *therm, bool suspend);
 void nouveau_therm_sensor_preinit(struct nouveau_therm *);
 void nouveau_therm_sensor_set_threshold_state(struct nouveau_therm *therm,
 					     enum nouveau_therm_thrs thrs,
diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/temp.c
b/drivers/gpu/drm/nouveau/core/subdev/therm/temp.c
index dde746c..b80a330 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/temp.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/temp.c
@@ -180,6 +180,8 @@ alarm_timer_callback(struct nouveau_alarm *alarm)
 
 	spin_lock_irqsave(&priv->sensor.alarm_program_lock, flags);
 
+	nv_debug(therm, "polling the internal temperature\n");
+
 	nouveau_therm_threshold_hyst_polling(therm, &sensor->thrs_fan_boost,
 					     NOUVEAU_THERM_THRS_FANBOOST);
 
@@ -216,6 +218,25 @@ nouveau_therm_program_alarms_polling(struct nouveau_therm
*therm)
 	alarm_timer_callback(&priv->sensor.therm_poll_alarm);
 }
 
+int
+nouveau_therm_sensor_init(struct nouveau_therm *therm)
+{
+	struct nouveau_therm_priv *priv = (void *)therm;
+	priv->sensor.program_alarms(therm);
+	return 0;
+}
+
+int
+nouveau_therm_sensor_fini(struct nouveau_therm *therm, bool suspend)
+{
+	struct nouveau_therm_priv *priv = (void *)therm;
+	struct nouveau_timer *ptimer = nouveau_timer(therm);
+
+	if (suspend)
+		ptimer->alarm_cancel(ptimer, &priv->sensor.therm_poll_alarm);
+	return 0;
+}
+
 void
 nouveau_therm_sensor_preinit(struct nouveau_therm *therm)
 {
-- 
1.8.3.4