Nouveau - Mar 2014 - [PATCH 1/4] pm/fan: drop the fan lock in fan_update() before rescheduling

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

This should fix a deadlock that has been reported to us where fan_update()
would hold the fan lock and try to grab the alarm_program_lock to reschedule
an update. On an other CPU, the alarm_program_lock would have been taken
before calling fan_update(), leading to a deadlock.

We should Cc: <stable at vger.kernel.org> # 3.9+

Reported-by: Marcin Slusarz <marcin.slusarz at gmail.com>
Tested-by: Timoth?e Ravier <tim at siosm.fr>
Signed-off-by: Martin Peres <martin.peres at free.fr>
---
 nvkm/subdev/therm/fan.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/nvkm/subdev/therm/fan.c b/nvkm/subdev/therm/fan.c
index 95f6129..29d4c41 100644
--- a/nvkm/subdev/therm/fan.c
+++ b/nvkm/subdev/therm/fan.c
@@ -54,8 +54,10 @@ nouveau_fan_update(struct nouveau_fan *fan, bool immediate,
int target)
 
 	/* check that we're not already at the target duty cycle */
 	duty = fan->get(therm);
-	if (duty == target)
-		goto done;
+	if (duty == target) {
+		spin_unlock_irqrestore(&fan->lock, flags);
+		return 0;
+	}
 
 	/* smooth out the fanspeed increase/decrease */
 	if (!immediate && duty >= 0) {
@@ -73,8 +75,15 @@ nouveau_fan_update(struct nouveau_fan *fan, bool immediate,
int target)
 
 	nv_debug(therm, "FAN update: %d\n", duty);
 	ret = fan->set(therm, duty);
-	if (ret)
-		goto done;
+	if (ret) {
+		spin_unlock_irqrestore(&fan->lock, flags);
+		return ret;
+	}
+
+	/* fan speed updated, drop the fan lock before grabbing the
+	 * alarm-scheduling lock and risking a deadlock
+	 */
+	spin_unlock_irqrestore(&fan->lock, flags);
 
 	/* schedule next fan update, if not at target speed already */
 	if (list_empty(&fan->alarm.head) && target != duty) {
@@ -92,8 +101,6 @@ nouveau_fan_update(struct nouveau_fan *fan, bool immediate,
int target)
 		ptimer->alarm(ptimer, delay * 1000 * 1000, &fan->alarm);
 	}
 
-done:
-	spin_unlock_irqrestore(&fan->lock, flags);
 	return ret;
 }
 
-- 
1.9.1

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

This should fix fan management on many nvd7+ chipsets.

Signed-off-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Timoth?e Ravier <tim at siosm.fr>
---
 nvkm/include/subdev/therm.h |  2 +-
 nvkm/subdev/therm/fan.c     |  3 ++-
 nvkm/subdev/therm/fanpwm.c  |  2 +-
 nvkm/subdev/therm/nv50.c    |  2 +-
 nvkm/subdev/therm/nvd0.c    | 40 +++++++++++++++++++++++++++++-----------
 nvkm/subdev/therm/priv.h    |  2 +-
 6 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/nvkm/include/subdev/therm.h b/nvkm/include/subdev/therm.h
index 69891d4..d4a6817 100644
--- a/nvkm/include/subdev/therm.h
+++ b/nvkm/include/subdev/therm.h
@@ -31,7 +31,7 @@ struct nouveau_therm {
 	int (*pwm_ctrl)(struct nouveau_therm *, int line, bool);
 	int (*pwm_get)(struct nouveau_therm *, int line, u32 *, u32 *);
 	int (*pwm_set)(struct nouveau_therm *, int line, u32, u32);
-	int (*pwm_clock)(struct nouveau_therm *);
+	int (*pwm_clock)(struct nouveau_therm *, int line);
 
 	int (*fan_get)(struct nouveau_therm *);
 	int (*fan_set)(struct nouveau_therm *, int);
diff --git a/nvkm/subdev/therm/fan.c b/nvkm/subdev/therm/fan.c
index 29d4c41..ceb8528 100644
--- a/nvkm/subdev/therm/fan.c
+++ b/nvkm/subdev/therm/fan.c
@@ -242,7 +242,8 @@ nouveau_therm_fan_ctor(struct nouveau_therm *therm)
 	/* attempt to locate a drivable fan, and determine control method */
 	ret = gpio->find(gpio, 0, DCB_GPIO_FAN, 0xff, &func);
 	if (ret == 0) {
-		if (func.log[0] & DCB_GPIO_LOG_DIR_IN) {
+		/* FIXME: is this really the place to perform such checks ? */
+		if (func.line != 16 && func.log[0] & DCB_GPIO_LOG_DIR_IN) {
 			nv_debug(therm, "GPIO_FAN is in input mode\n");
 			ret = -EINVAL;
 		} else {
diff --git a/nvkm/subdev/therm/fanpwm.c b/nvkm/subdev/therm/fanpwm.c
index 5f71db8..9a5c073 100644
--- a/nvkm/subdev/therm/fanpwm.c
+++ b/nvkm/subdev/therm/fanpwm.c
@@ -67,7 +67,7 @@ nouveau_fanpwm_set(struct nouveau_therm *therm, int percent)
 	if (priv->base.bios.pwm_freq) {
 		divs = 1;
 		if (therm->pwm_clock)
-			divs = therm->pwm_clock(therm);
+			divs = therm->pwm_clock(therm, priv->func.line);
 		divs /= priv->base.bios.pwm_freq;
 	}
 
diff --git a/nvkm/subdev/therm/nv50.c b/nvkm/subdev/therm/nv50.c
index 8cf7597..321db92 100644
--- a/nvkm/subdev/therm/nv50.c
+++ b/nvkm/subdev/therm/nv50.c
@@ -93,7 +93,7 @@ nv50_fan_pwm_set(struct nouveau_therm *therm, int line, u32
divs, u32 duty)
 }
 
 int
-nv50_fan_pwm_clock(struct nouveau_therm *therm)
+nv50_fan_pwm_clock(struct nouveau_therm *therm, int line)
 {
 	int chipset = nv_device(therm)->chipset;
 	int crystal = nv_device(therm)->crystal;
diff --git a/nvkm/subdev/therm/nvd0.c b/nvkm/subdev/therm/nvd0.c
index 4dd4f81..43fec17 100644
--- a/nvkm/subdev/therm/nvd0.c
+++ b/nvkm/subdev/therm/nvd0.c
@@ -32,10 +32,12 @@ static int
 pwm_info(struct nouveau_therm *therm, int line)
 {
 	u32 gpio = nv_rd32(therm, 0x00d610 + (line * 0x04));
+
 	switch (gpio & 0x000000c0) {
 	case 0x00000000: /* normal mode, possibly pwm forced off by us */
 	case 0x00000040: /* nvio special */
 		switch (gpio & 0x0000001f) {
+		case 0x00: return 2;
 		case 0x19: return 1;
 		case 0x1c: return 0;
 		default:
@@ -56,8 +58,9 @@ nvd0_fan_pwm_ctrl(struct nouveau_therm *therm, int line, bool
enable)
 	int indx = pwm_info(therm, line);
 	if (indx < 0)
 		return indx;
-
-	nv_mask(therm, 0x00d610 + (line * 0x04), 0x000000c0, data);
+	else if (indx < 2)
+		nv_mask(therm, 0x00d610 + (line * 0x04), 0x000000c0, data);
+	/* nothing to do for indx == 2, it seems hardwired to PTHERM */
 	return 0;
 }
 
@@ -67,10 +70,15 @@ nvd0_fan_pwm_get(struct nouveau_therm *therm, int line, u32
*divs, u32 *duty)
 	int indx = pwm_info(therm, line);
 	if (indx < 0)
 		return indx;
-
-	if (nv_rd32(therm, 0x00d610 + (line * 0x04)) & 0x00000040) {
-		*divs = nv_rd32(therm, 0x00e114 + (indx * 8));
-		*duty = nv_rd32(therm, 0x00e118 + (indx * 8));
+	else if (indx < 2) {
+		if (nv_rd32(therm, 0x00d610 + (line * 0x04)) & 0x00000040) {
+			*divs = nv_rd32(therm, 0x00e114 + (indx * 8));
+			*duty = nv_rd32(therm, 0x00e118 + (indx * 8));
+			return 0;
+		}
+	} else if (indx == 2) {
+		*divs = nv_rd32(therm, 0x0200d8) & 0x1fff;
+		*duty = nv_rd32(therm, 0x0200dc) & 0x1fff;
 		return 0;
 	}
 
@@ -83,16 +91,26 @@ nvd0_fan_pwm_set(struct nouveau_therm *therm, int line, u32
divs, u32 duty)
 	int indx = pwm_info(therm, line);
 	if (indx < 0)
 		return indx;
-
-	nv_wr32(therm, 0x00e114 + (indx * 8), divs);
-	nv_wr32(therm, 0x00e118 + (indx * 8), duty | 0x80000000);
+	else if (indx < 2) {
+		nv_wr32(therm, 0x00e114 + (indx * 8), divs);
+		nv_wr32(therm, 0x00e118 + (indx * 8), duty | 0x80000000);
+	} else if (indx == 2) {
+		nv_mask(therm, 0x0200d8, 0x1fff, divs); /* keep the high bits */
+		nv_wr32(therm, 0x0200dc, duty | 0x40000000);
+	}
 	return 0;
 }
 
 static int
-nvd0_fan_pwm_clock(struct nouveau_therm *therm)
+nvd0_fan_pwm_clock(struct nouveau_therm *therm, int line)
 {
-	return (nv_device(therm)->crystal * 1000) / 20;
+	int indx = pwm_info(therm, line);
+	if (indx < 0)
+		return 0;
+	else if (indx < 2)
+		return (nv_device(therm)->crystal * 1000) / 20;
+	else
+		return nv_device(therm)->crystal * 1000 / 10;
 }
 
 static int
diff --git a/nvkm/subdev/therm/priv.h b/nvkm/subdev/therm/priv.h
index 96f8f95..916fca5 100644
--- a/nvkm/subdev/therm/priv.h
+++ b/nvkm/subdev/therm/priv.h
@@ -143,7 +143,7 @@ void nv40_therm_intr(struct nouveau_subdev *);
 int nv50_fan_pwm_ctrl(struct nouveau_therm *, int, bool);
 int nv50_fan_pwm_get(struct nouveau_therm *, int, u32 *, u32 *);
 int nv50_fan_pwm_set(struct nouveau_therm *, int, u32, u32);
-int nv50_fan_pwm_clock(struct nouveau_therm *);
+int nv50_fan_pwm_clock(struct nouveau_therm *, int);
 int nv84_temp_get(struct nouveau_therm *therm);
 int nv84_therm_fini(struct nouveau_object *object, bool suspend);
 
-- 
1.9.1

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

This should fix automatic fan management on fermi cards who do not have
0x46 entries in the thermal table.

On my nve6, the blob sets the default linear range from 40?C to 100?C
but my nvcf's default values are 40?C to 85?C. Let's keep 85 as a
default
for everyone.

Signed-off-by: Martin Peres <martin.peres at labri.fr>
Tested-by: Timoth?e Ravier <tim at siosm.fr>
---
 nvkm/include/subdev/bios/therm.h |  7 +++++++
 nvkm/subdev/bios/therm.c         | 11 +++++++++++
 nvkm/subdev/therm/base.c         | 12 +++++++-----
 nvkm/subdev/therm/fan.c          |  3 ---
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/nvkm/include/subdev/bios/therm.h b/nvkm/include/subdev/bios/therm.h
index 083541d..8dc5051 100644
--- a/nvkm/include/subdev/bios/therm.h
+++ b/nvkm/include/subdev/bios/therm.h
@@ -31,6 +31,12 @@ struct nouveau_therm_trip_point {
 	int hysteresis;
 };
 
+enum nvbios_therm_fan_mode {
+	NVBIOS_THERM_FAN_TRIP = 0,
+	NVBIOS_THERM_FAN_LINEAR = 1,
+	NVBIOS_THERM_FAN_OTHER = 2,
+};
+
 struct nvbios_therm_fan {
 	u16 pwm_freq;
 
@@ -40,6 +46,7 @@ struct nvbios_therm_fan {
 	u16 bump_period;
 	u16 slow_down_period;
 
+	enum nvbios_therm_fan_mode fan_mode;
 	struct nouveau_therm_trip_point trip[NOUVEAU_TEMP_FAN_TRIP_MAX];
 	u8 nr_fan_trip;
 	u8 linear_min_temp;
diff --git a/nvkm/subdev/bios/therm.c b/nvkm/subdev/bios/therm.c
index 22ac6db..d158540 100644
--- a/nvkm/subdev/bios/therm.c
+++ b/nvkm/subdev/bios/therm.c
@@ -164,6 +164,7 @@ nvbios_therm_fan_parse(struct nouveau_bios *bios,
 
 	i = 0;
 	fan->nr_fan_trip = 0;
+	fan->fan_mode = NVBIOS_THERM_FAN_OTHER;
 	while ((entry = nvbios_therm_entry(bios, i++, &ver, &len))) {
 		s16 value = nv_ro16(bios, entry + 1);
 
@@ -174,6 +175,8 @@ nvbios_therm_fan_parse(struct nouveau_bios *bios,
 			break;
 		case 0x24:
 			fan->nr_fan_trip++;
+			if (fan->fan_mode > NVBIOS_THERM_FAN_TRIP)
+				fan->fan_mode = NVBIOS_THERM_FAN_TRIP;
 			cur_trip = &fan->trip[fan->nr_fan_trip - 1];
 			cur_trip->hysteresis = value & 0xf;
 			cur_trip->temp = (value & 0xff0) >> 4;
@@ -194,11 +197,19 @@ nvbios_therm_fan_parse(struct nouveau_bios *bios,
 			fan->slow_down_period = value;
 			break;
 		case 0x46:
+			if (fan->fan_mode > NVBIOS_THERM_FAN_LINEAR)
+				fan->fan_mode = NVBIOS_THERM_FAN_LINEAR;
 			fan->linear_min_temp = nv_ro08(bios, entry + 1);
 			fan->linear_max_temp = nv_ro08(bios, entry + 2);
 			break;
 		}
 	}
 
+	/* starting from fermi, fan management is always linear */
+	if (nv_device(bios)->card_type >= NV_C0 &&
+		fan->fan_mode == NVBIOS_THERM_FAN_OTHER) {
+		fan->fan_mode = NVBIOS_THERM_FAN_LINEAR;
+	}
+
 	return 0;
 }
diff --git a/nvkm/subdev/therm/base.c b/nvkm/subdev/therm/base.c
index 80e584a..e709b23 100644
--- a/nvkm/subdev/therm/base.c
+++ b/nvkm/subdev/therm/base.c
@@ -110,16 +110,18 @@ nouveau_therm_update(struct nouveau_therm *therm, int
mode)
 		poll = false;
 		break;
 	case NOUVEAU_THERM_CTRL_AUTO:
-		if (priv->fan->bios.nr_fan_trip) {
+		switch(priv->fan->bios.fan_mode) {
+		case NVBIOS_THERM_FAN_TRIP:
 			duty = nouveau_therm_update_trip(therm);
-		} else
-		if (priv->fan->bios.linear_min_temp ||
-		    priv->fan->bios.linear_max_temp) {
+			break;
+		case NVBIOS_THERM_FAN_LINEAR:
 			duty = nouveau_therm_update_linear(therm);
-		} else {
+			break;
+		case NVBIOS_THERM_FAN_OTHER:
 			if (priv->cstate)
 				duty = priv->cstate;
 			poll = false;
+			break;
 		}
 		immd = false;
 		break;
diff --git a/nvkm/subdev/therm/fan.c b/nvkm/subdev/therm/fan.c
index ceb8528..016990a 100644
--- a/nvkm/subdev/therm/fan.c
+++ b/nvkm/subdev/therm/fan.c
@@ -192,11 +192,8 @@ nouveau_therm_fan_set_defaults(struct nouveau_therm *therm)
 	priv->fan->bios.max_duty = 100;
 	priv->fan->bios.bump_period = 500;
 	priv->fan->bios.slow_down_period = 2000;
-/*XXX: talk to mupuf */
-#if 0
 	priv->fan->bios.linear_min_temp = 40;
 	priv->fan->bios.linear_max_temp = 85;
-#endif
 }
 
 static void
-- 
1.9.1

At boot time, on one of my desktop PC and one of my card, Nouveau fails to
boot because the vbios is corrupted. If I blacklist the module and load it
after, it works fine. Also, this card on another computer works fine and other
cars on this desktop work directly (I would need to check more cards though).

I tried several things to make it work at boot time:
- use 32 bits reads instead of 8
- disable IRQs while we fetch the bios
- add a read to check the status of 88050 (but it doesn't change on its own)
- increase the number of initial reads for the signature

None of them worked so I tried something more extreme, trying to read the
vbios 16 times when reading it from PROM. It seems to work all the time now.

I'm posting this patch to leave a written trace of what I tried. If someone
needs this patch too, please let me know!

No sign off for this, this is a hack.
---
 nvkm/subdev/bios/base.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/nvkm/subdev/bios/base.c b/nvkm/subdev/bios/base.c
index 5608530..96e2eaf 100644
--- a/nvkm/subdev/bios/base.c
+++ b/nvkm/subdev/bios/base.c
@@ -139,7 +139,7 @@ out:
 }
 
 static void
-nouveau_bios_shadow_prom(struct nouveau_bios *bios)
+nouveau_bios_shadow_prom_attempt(struct nouveau_bios *bios)
 {
 	struct nouveau_device *device = nv_device(bios);
 	u32 pcireg, access;
@@ -196,6 +196,17 @@ out:
 	nv_wr32(bios, pcireg, access);
 }
 
+static void
+nouveau_bios_shadow_prom(struct nouveau_bios *bios)
+{
+	int i = 0;
+	do {
+		nouveau_bios_shadow_prom_attempt(bios);
+		if(!nvbios_checksum(bios->data, bios->size))
+			break;
+	} while (i++ < 16);
+}
+
 #if defined(CONFIG_ACPI) && defined(CONFIG_X86)
 int nouveau_acpi_get_bios_chunk(uint8_t *bios, int offset, int len);
 bool nouveau_acpi_rom_supported(struct pci_dev *pdev);
-- 
1.9.1

Le 24/03/2014 02:03, Martin Peres a ?crit :
> From: Martin Peres <martin.peres at labri.fr>
>
> This should fix a deadlock that has been reported to us where fan_update()
> would hold the fan lock and try to grab the alarm_program_lock to
reschedule
> an update. On an other CPU, the alarm_program_lock would have been taken
> before calling fan_update(), leading to a deadlock.
>
> We should Cc: <stable at vger.kernel.org> # 3.9+
>
> Reported-by: Marcin Slusarz <marcin.slusarz at gmail.com>
> Tested-by: Timoth?e Ravier <tim at siosm.fr>
Tested-by: Boris Fersing (IRC nick fersingb, email is private)
> Signed-off-by: Martin Peres <martin.peres at free.fr>
> ---
>   nvkm/subdev/therm/fan.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/nvkm/subdev/therm/fan.c b/nvkm/subdev/therm/fan.c
> index 95f6129..29d4c41 100644
> --- a/nvkm/subdev/therm/fan.c
> +++ b/nvkm/subdev/therm/fan.c
> @@ -54,8 +54,10 @@ nouveau_fan_update(struct nouveau_fan *fan, bool
immediate, int target)
>
>   	/* check that we're not already at the target duty cycle */
>   	duty = fan->get(therm);
> -	if (duty == target)
> -		goto done;
> +	if (duty == target) {
> +		spin_unlock_irqrestore(&fan->lock, flags);
> +		return 0;
> +	}
>
>   	/* smooth out the fanspeed increase/decrease */
>   	if (!immediate && duty >= 0) {
> @@ -73,8 +75,15 @@ nouveau_fan_update(struct nouveau_fan *fan, bool
immediate, int target)
>
>   	nv_debug(therm, "FAN update: %d\n", duty);
>   	ret = fan->set(therm, duty);
> -	if (ret)
> -		goto done;
> +	if (ret) {
> +		spin_unlock_irqrestore(&fan->lock, flags);
> +		return ret;
> +	}
> +
> +	/* fan speed updated, drop the fan lock before grabbing the
> +	 * alarm-scheduling lock and risking a deadlock
> +	 */
> +	spin_unlock_irqrestore(&fan->lock, flags);
>
>   	/* schedule next fan update, if not at target speed already */
>   	if (list_empty(&fan->alarm.head) && target != duty) {
> @@ -92,8 +101,6 @@ nouveau_fan_update(struct nouveau_fan *fan, bool
immediate, int target)
>   		ptimer->alarm(ptimer, delay * 1000 * 1000, &fan->alarm);
>   	}
>
> -done:
> -	spin_unlock_irqrestore(&fan->lock, flags);
>   	return ret;
>   }
>
>