nouveau_subdev & misc patches

nouveau_subdev & misc patches

[Emil Velikov]

Hello all, this series includes a wide range of fixes - from a few
month's old one-liners from Andreas Heider regarding vga_switcheroo, via a
null pointer dereference and double memory allocation, to a buffer overflow.

Please review and comment

---
 drivers/gpu/drm/nouveau/nouveau_acpi.c   |    3 ++-
 drivers/gpu/drm/nouveau/nouveau_device.c |   26 +++++++++++++++-----------
 drivers/gpu/drm/nouveau/nouveau_fb.h     |    1 -
 drivers/gpu/drm/nouveau/nouveau_gpio.h   |    3 ---
 drivers/gpu/drm/nouveau/nouveau_gpuobj.c |   12 ++++--------
 drivers/gpu/drm/nouveau/nouveau_perf.c   |    2 +-
 drivers/gpu/drm/nouveau/nouveau_pm.c     |    6 +++---
 drivers/gpu/drm/nouveau/nouveau_state.c  |    2 ++
 drivers/gpu/drm/nouveau/nouveau_therm.c  |   63 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
 drivers/gpu/drm/nouveau/nv04_instmem.c   |    3 +--
 drivers/gpu/drm/nouveau/nv10_fb.c        |    4 ----
 drivers/gpu/drm/nouveau/nv10_gpio.c      |   16 ++++++++--------
 drivers/gpu/drm/nouveau/nv30_fb.c        |    3 +--
 drivers/gpu/drm/nouveau/nv40_fanpwm.c    |   14 +++++++-------
 drivers/gpu/drm/nouveau/nv40_fb.c        |   11 ++++-------
 drivers/gpu/drm/nouveau/nv50_bar.c       |    2 --
 drivers/gpu/drm/nouveau/nv50_fanpwm.c    |   20 ++++++++++----------
 drivers/gpu/drm/nouveau/nv50_gpio.c      |   16 ++++++++--------
 drivers/gpu/drm/nouveau/nv50_instmem.c   |   10 +++-------
 drivers/gpu/drm/nouveau/nvc0_instmem.c   |   10 +++-------
 drivers/gpu/drm/nouveau/nvd0_gpio.c      |   16 ++++++++--------
 21 files changed, 129 insertions(+), 114 deletions(-)

[PATCH 01/14] drm/nouveau: Check dsm on switcheroo unregister

[Emil Velikov]

From: Andreas Heider <andreas-nJONrVjOgt4@public.gmane.org>

Currently vga_switcheroo_unregister_handler is called unconditionally when
nouveau is unloaded, even when nouveau never registered a handler. This
interferes with other switcheroo handlers, as vga_switcheroo doesn't check who
called unregister_handler, but simply unregisters the current handler. This
patch adds a check so unregister is only called if a handler was registered by
nouveau before.

Signed-off-by: Andreas Heider <andreas-nJONrVjOgt4@public.gmane.org>
---
v2: Rebased on top of master

 drivers/gpu/drm/nouveau/nouveau_acpi.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index 0156d83..38a85b3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -338,7 +338,8 @@ void nouveau_switcheroo_optimus_dsm(void)
 
 void nouveau_unregister_dsm_handler(void)
 {
-	vga_switcheroo_unregister_handler();
+	if (nouveau_dsm_priv.optimus_detected || nouveau_dsm_priv.dsm_detected)
+		vga_switcheroo_unregister_handler();
 }
 
 /* retrieve the ROM in 4k blocks */
-- 
1.7.10.2

[PATCH 02/14] drm/nouveau: Unregister switcheroo client on exit

[Emil Velikov]

From: Andreas Heider <andreas-nJONrVjOgt4@public.gmane.org>

Currently nouveau only registers as a vga_switcheroo client, but never
unregisters. This patch adds the necessary unregister calls.

Signed-off-by: Andreas Heider <andreas-nJONrVjOgt4@public.gmane.org>
---
v2: Rebased on top of master

 drivers/gpu/drm/nouveau/nouveau_state.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index 1a70d07..b84db66 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -344,6 +344,7 @@ out_device_init:
 out_display_early:
 	engine->display.late_takedown(ndev);
 out:
+	vga_switcheroo_unregister_client(dev->pdev);
 	vga_client_register(dev->pdev, NULL, NULL, NULL);
 	return ret;
 }
@@ -387,6 +388,7 @@ static void nouveau_card_takedown(struct nouveau_device *ndev)
 
 	nouveau_irq_fini(ndev);
 
+	vga_switcheroo_unregister_client(dev->pdev);
 	vga_client_register(dev->pdev, NULL, NULL, NULL);
 }
 
-- 
1.7.10.2

[PATCH 03/14] drm/nouveau/device: Simplify init, fini fuction

[Emil Velikov]

Removes unnecessary goto statement
fini subdevices before destroying them

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nouveau_device.c |   26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_device.c b/drivers/gpu/drm/nouveau/nouveau_device.c
index 9cc21d1..38ed71a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_device.c
+++ b/drivers/gpu/drm/nouveau/nouveau_device.c
@@ -55,13 +55,13 @@ nouveau_device_init(struct nouveau_device *ndev)
 
 	for (i = 0; i < NVDEV_SUBDEV_NR; i++) {
 		ret = nouveau_subdev_init(ndev, i, 0);
-		if (ret)
-			goto error;
+		if (ret) {
+			for (--i; i >= 0; i--)
+				nouveau_subdev_fini(ndev, i, false);
+			break;
+		}
 	}
 
-error:
-	for (--i; ret && i >= 0; i--)
-		nouveau_subdev_fini(ndev, i, false);
 	return ret;
 }
 
@@ -72,13 +72,13 @@ nouveau_device_fini(struct nouveau_device *ndev, bool suspend)
 
 	for (i = NVDEV_SUBDEV_NR - 1; i >= 0; i--) {
 		ret = nouveau_subdev_fini(ndev, i, suspend);
-		if (ret)
-			goto error;
+		if (ret) {
+			for (--i; i >= 0; i--)
+				nouveau_subdev_init(ndev, i, 0);
+			break;
+		}
 	}
 
-error:
-	for (--i; ret && i >= 0; i--)
-		nouveau_subdev_init(ndev, i, 0);
 	return ret;
 }
 
@@ -96,6 +96,7 @@ nouveau_device_create(struct nouveau_device *ndev)
 {
 	int disable = nouveau_noaccel;
 	int ret = 0;
+	int i;
 
 	/* mask out any engines that are known not to work as they should,
 	 * these can be overridden by the user
@@ -553,7 +554,10 @@ nouveau_device_create(struct nouveau_device *ndev)
 		break;
 	}
 
-	if (ret)
+	if (ret) {
+		for (i = NVDEV_SUBDEV_NR - 1; i >= 0; i--)
+			nouveau_subdev_fini(ndev, i, false);
 		nouveau_device_destroy(ndev);
+	}
 	return ret;
 }
-- 
1.7.10.2

[PATCH 04/14] drm/nouveau: Remove non-relevant function prototypes

[Emil Velikov]

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nouveau_fb.h   |    1 -
 drivers/gpu/drm/nouveau/nouveau_gpio.h |    3 ---
 2 files changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fb.h b/drivers/gpu/drm/nouveau/nouveau_fb.h
index b804fd0..5f62743 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fb.h
+++ b/drivers/gpu/drm/nouveau/nouveau_fb.h
@@ -37,7 +37,6 @@ void nv30_fb_free_tile_region(struct nouveau_fb *, int i);
 int  nv40_fb_create(struct nouveau_device *, int);
 
 int  nv50_fb_create(struct nouveau_device *, int);
-void nv50_fb_vram_fini(struct nouveau_fb *);
 void nv50_fb_vram_del(struct nouveau_fb *, struct nouveau_mem **);
 void nv50_fb_vm_trap(struct nouveau_device *, int display);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_gpio.h b/drivers/gpu/drm/nouveau/nouveau_gpio.h
index 1814c19..5bd66e8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gpio.h
+++ b/drivers/gpu/drm/nouveau/nouveau_gpio.h
@@ -53,9 +53,6 @@ struct gpio_func {
 
 /* nouveau_gpio.c */
 int  nouveau_gpio_create(struct nouveau_device *, int subdev);
-void nouveau_gpio_destroy(struct nouveau_device *);
-int  nouveau_gpio_init(struct nouveau_device *);
-void nouveau_gpio_fini(struct nouveau_device *);
 void nouveau_gpio_reset(struct nouveau_device *);
 int  nouveau_gpio_drive(struct nouveau_device *, int idx, int line,
 			int dir, int out);
-- 
1.7.10.2

[PATCH 05/14] drm/nouveau/gpuobj: Do not handle gpuobj_init during fail path in gpuobj_fini

[Emil Velikov]

This approach is already handled by the subdev system. Remove
unneeded variable in the process

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nouveau_gpuobj.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gpuobj.c b/drivers/gpu/drm/nouveau/nouveau_gpuobj.c
index 2eed5c3..43a7cd2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gpuobj.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gpuobj.c
@@ -757,7 +757,7 @@ nouveau_gpuobj_fini(struct nouveau_device *ndev, int subdev, bool suspend)
 {
 	struct nouveau_gpuobj_priv *priv = nv_subdev(ndev, subdev);
 	struct nouveau_gpuobj *gpuobj;
-	int i, ret = 0;
+	int i;
 
 	if (suspend) {
 		list_for_each_entry(gpuobj, &priv->list, list) {
@@ -766,8 +766,7 @@ nouveau_gpuobj_fini(struct nouveau_device *ndev, int subdev, bool suspend)
 
 			gpuobj->suspend = vmalloc(gpuobj->size);
 			if (!gpuobj->suspend) {
-				ret = -ENOMEM;
-				goto error;
+				return -ENOMEM;
 			}
 
 			for (i = 0; i < gpuobj->size; i += 4)
@@ -775,16 +774,13 @@ nouveau_gpuobj_fini(struct nouveau_device *ndev, int subdev, bool suspend)
 		}
 	}
 
-error:
-	if (ret)
-		priv->base.init(ndev, subdev);
-	return ret;
+	return 0;
 }
 
 static void
 nouveau_gpuobj_destroy(struct nouveau_device *ndev, int subdev)
 {
-	struct nouveau_gpuobj_priv *priv = nv_subdev(ndev, NVDEV_SUBDEV_GPUOBJ);
+	struct nouveau_gpuobj_priv *priv = nv_subdev(ndev, subdev);
 	struct nouveau_gpuobj_method *om, *tm;
 	struct nouveau_gpuobj_class *oc, *tc;
 
-- 
1.7.10.2

[PATCH 06/14] drm/nouveau/instmem: Do not handle instmem_init during fail path in instmem_fini

[Emil Velikov]

This approach is already handled by the subdev system. Remove
unneeded variable in the process

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nv50_instmem.c |   10 +++-------
 drivers/gpu/drm/nouveau/nvc0_instmem.c |   10 +++-------
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_instmem.c b/drivers/gpu/drm/nouveau/nv50_instmem.c
index f954e87..79e78d0 100644
--- a/drivers/gpu/drm/nouveau/nv50_instmem.c
+++ b/drivers/gpu/drm/nouveau/nv50_instmem.c
@@ -201,7 +201,7 @@ static int
 nv50_instmem_init(struct nouveau_device *ndev, int subdev)
 {
 	struct nv50_instmem_priv *priv = nv_subdev(ndev, subdev);
-	int ret = 0, i;
+	int i;
 
 	nv_wr32(ndev, 0x001700, priv->mem->offset >> 16);
 
@@ -224,15 +224,11 @@ nv50_instmem_init(struct nouveau_device *ndev, int subdev)
 	for (i = 0; i < 64 * 1024; i += 4) {
 		if (nv_rd32(ndev, 0x705000 + i) != nv_ri32(ndev, i)) {
 			NV_ERROR(ndev, "INSTMEM: readback failed\n");
-			ret = -EIO;
-			goto error;
+			return -EIO;
 		}
 	}
 
-error:
-	if (ret)
-		priv->base.base.fini(ndev, subdev, false);
-	return ret;
+	return 0;
 }
 
 int
diff --git a/drivers/gpu/drm/nouveau/nvc0_instmem.c b/drivers/gpu/drm/nouveau/nvc0_instmem.c
index 2df078b..59e28f7 100644
--- a/drivers/gpu/drm/nouveau/nvc0_instmem.c
+++ b/drivers/gpu/drm/nouveau/nvc0_instmem.c
@@ -107,7 +107,7 @@ static int
 nvc0_instmem_init(struct nouveau_device *ndev, int subdev)
 {
 	struct nvc0_instmem_priv *priv = nv_subdev(ndev, subdev);
-	int ret = 0, i;
+	int i;
 
 	nv_wr32(ndev, 0x001700, priv->mem->offset >> 16);
 
@@ -128,15 +128,11 @@ nvc0_instmem_init(struct nouveau_device *ndev, int subdev)
 	for (i = 0; i < 64 * 1024; i += 4) {
 		if (nv_rd32(ndev, 0x702000 + i) != nv_ri32(ndev, i)) {
 			NV_ERROR(ndev, "INSTMEM: readback failed\n");
-			ret = -EIO;
-			goto error;
+			return -EIO;
 		}
 	}
 
-error:
-	if (ret)
-		priv->base.base.fini(ndev, subdev, false);
-	return ret;
+	return 0;
 }
 
 int
-- 
1.7.10.2

[PATCH 07/14] drm/nouveau/volt: Purge volt->get and volt->set checks

[Emil Velikov]

Both functions are constantly present if the volt subdev is created

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nouveau_pm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_pm.c b/drivers/gpu/drm/nouveau/nouveau_pm.c
index b6bde18..9dd34fe 100644
--- a/drivers/gpu/drm/nouveau/nouveau_pm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_pm.c
@@ -60,7 +60,7 @@ nouveau_pm_perflvl_aux(struct nouveau_device *ndev, struct nouveau_pm_level *per
 		}
 	}
 
-	if (pvolt && pvolt->set) {
+	if (pvolt) {
 		if (perflvl->volt_min && b->volt_min > a->volt_min) {
 			ret = pvolt->set(pvolt, perflvl->volt_min);
 			if (ret) {
@@ -234,7 +234,7 @@ nouveau_pm_perflvl_get(struct nouveau_device *ndev, struct nouveau_pm_level *per
 			return ret;
 	}
 
-	if (pvolt && pvolt->get) {
+	if (pvolt) {
 		ret = pvolt->get(pvolt);
 		if (ret > 0) {
 			perflvl->volt_min = ret;
-- 
1.7.10.2

[PATCH 08/14] drm/nv50_bar: Remove duplicate assignments

[Emil Velikov]

Both variables are already set in nouveau_subdev_create()

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nv50_bar.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_bar.c b/drivers/gpu/drm/nouveau/nv50_bar.c
index 0dc1895..6499a30 100644
--- a/drivers/gpu/drm/nouveau/nv50_bar.c
+++ b/drivers/gpu/drm/nouveau/nv50_bar.c
@@ -92,8 +92,6 @@ nv50_bar_create(struct nouveau_device *ndev, int subdev)
 	if (ret)
 		return ret;
 
-	priv->base.base.name = "BAR";
-	priv->base.base.device = ndev;
 	priv->base.base.destroy = nv50_bar_destroy;
 	priv->base.base.init = nv50_bar_init;
 	priv->base.map = nv50_bar_map;
-- 
1.7.10.2

[PATCH 09/14] drm/nv04_instmem: Remove duplicate assignment

[Emil Velikov]

The variable is already set in nouveau_subdev_create()

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nv04_instmem.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nv04_instmem.c b/drivers/gpu/drm/nouveau/nv04_instmem.c
index 362b23f..47d70fe 100644
--- a/drivers/gpu/drm/nouveau/nv04_instmem.c
+++ b/drivers/gpu/drm/nouveau/nv04_instmem.c
@@ -108,7 +108,6 @@ nv04_instmem_create(struct nouveau_device *ndev, int subdev)
 	if (ret)
 		return ret;
 
-	priv->base.base.device = ndev;
 	priv->base.base.destroy = nv04_instmem_destroy;
 	priv->base.get = nv04_instmem_get;
 	priv->base.put = nv04_instmem_put;
-- 
1.7.10.2

[PATCH 10/14] drm/nv30_fb: Purge optional variable

[Emil Velikov]

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nv30_fb.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv30_fb.c b/drivers/gpu/drm/nouveau/nv30_fb.c
index ce08f08..bc1c65b 100644
--- a/drivers/gpu/drm/nouveau/nv30_fb.c
+++ b/drivers/gpu/drm/nouveau/nv30_fb.c
@@ -113,7 +113,6 @@ nv30_fb_init(struct nouveau_device *ndev, int subdev)
 int
 nv30_fb_create(struct nouveau_device *ndev, int subdev)
 {
-	u32 mem_size = nv_rd32(ndev, 0x10020c);
 	u32 pbus1218 = nv_rd32(ndev, 0x001218);
 	struct nv30_fb_priv *priv;
 	int ret;
@@ -128,7 +127,7 @@ nv30_fb_create(struct nouveau_device *ndev, int subdev)
 	case 0x00000200: ndev->vram_type = NV_MEM_TYPE_GDDR3; break;
 	case 0x00000300: ndev->vram_type = NV_MEM_TYPE_GDDR2; break;
 	}
-	ndev->vram_size = mem_size & 0xff000000;
+	ndev->vram_size = nv_rd32(ndev, 0x10020c) & 0xff000000;
 
 	priv->base.base.destroy = nv10_fb_destroy;
 	priv->base.base.init = nv30_fb_init;
-- 
1.7.10.2

[PATCH 11/14] drm/nv40/fb: Blend if statement within the switch

[Emil Velikov]

No functional change, just bike-shedding

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nv40_fb.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv40_fb.c b/drivers/gpu/drm/nouveau/nv40_fb.c
index 879678b..4f2d436 100644
--- a/drivers/gpu/drm/nouveau/nv40_fb.c
+++ b/drivers/gpu/drm/nouveau/nv40_fb.c
@@ -107,13 +107,6 @@ nv40_fb_init(struct nouveau_device *ndev, int subdev)
 	u32 tmp;
 	int i;
 
-	if (ndev->chipset != 0x40 && ndev->chipset != 0x45) {
-		if (nv44_graph_class(ndev))
-			nv44_fb_init_gart(ndev);
-		else
-			nv40_fb_init_gart(ndev);
-	}
-
 	switch (ndev->chipset) {
 	case 0x40:
 	case 0x45:
@@ -121,6 +114,10 @@ nv40_fb_init(struct nouveau_device *ndev, int subdev)
 		nv_wr32(ndev, NV10_PFB_CLOSE_PAGE2, tmp & ~(1 << 15));
 		break;
 	default:
+		if (nv44_graph_class(ndev))
+			nv44_fb_init_gart(ndev);
+		else
+			nv40_fb_init_gart(ndev);
 		break;
 	}
 
-- 
1.7.10.2

[PATCH 12/14] drm/nv10/fb: Prevent double memory allocation

[Emil Velikov]

Appropriate memory is allocated in nouveau_subdev_create()

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nv10_fb.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv10_fb.c b/drivers/gpu/drm/nouveau/nv10_fb.c
index a886098..80b3e41 100644
--- a/drivers/gpu/drm/nouveau/nv10_fb.c
+++ b/drivers/gpu/drm/nouveau/nv10_fb.c
@@ -132,10 +132,6 @@ nv10_fb_create(struct nouveau_device *ndev, int subdev)
 		ndev->vram_size = data & NV10_PFB_FIFO_DATA_RAM_AMOUNT_MB_MASK;
 	}
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
 	priv->base.base.destroy = nv10_fb_destroy;
 	priv->base.base.init = nv10_fb_init;
 	priv->base.memtype_valid = nv04_fb_memtype_valid;
-- 
1.7.10.2

[PATCH 13/14] drm/nouveau/perf: Prevent buffer oveflow

[Emil Velikov]

If the vbios is corrupted it can indicate more performance entries than the
ones statically allocated, causing overwriting of kernel memory

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nouveau_perf.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_perf.c b/drivers/gpu/drm/nouveau/nouveau_perf.c
index af03fb4..36a9ae3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_perf.c
+++ b/drivers/gpu/drm/nouveau/nouveau_perf.c
@@ -61,7 +61,7 @@ nouveau_perf_entry(struct nouveau_device *ndev, int idx,
 		   u8 *ver, u8 *hdr, u8 *cnt, u8 *len)
 {
 	u8 *perf = nouveau_perf_table(ndev, ver);
-	if (perf) {
+	if (perf && idx < NOUVEAU_PM_MAX_LEVEL) {
 		if (*ver >= 0x12 && *ver < 0x20 && idx < perf[2]) {
 			*hdr = perf[3];
 			*cnt = 0;
-- 
1.7.10.2

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

[Emil Velikov]

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

Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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;
+}
+
 static int
 nv40_sensor_setup(struct nouveau_therm *ptherm)
 {
@@ -181,7 +187,7 @@ nouveau_therm_vbios_parse(struct nouveau_therm *ptherm, u8 *temp)
 	temp = temp + headerlen;
 
 	/* Read the entries from the table */
-	for (i = 0; i < entries; i++) {
+	for (i = 0; i < entries; i++, temp += recordlen) {
 		s16 value = ROM16(temp[1]);
 
 		switch (temp[0]) {
@@ -228,7 +234,6 @@ nouveau_therm_vbios_parse(struct nouveau_therm *ptherm, u8 *temp)
 			ptherm->fan.pwm_freq = value;
 			break;
 		}
-		temp += recordlen;
 	}
 
 	nouveau_therm_safety_checks(ptherm);
@@ -299,6 +304,12 @@ nouveau_therm_probe_i2c(struct nouveau_device *ndev)
 			     probe_monitoring_device, NV_I2C_DEFAULT(0));
 }
 
+static void
+nouveau_ptherm_destroy(struct nouveau_device *ndev, int subdev)
+{
+// XXX: Undo probe_monitoring_device
+}
+
 int
 nouveau_therm_create(struct nouveau_device *ndev, int subdev)
 {
@@ -307,29 +318,53 @@ nouveau_therm_create(struct nouveau_device *ndev, int subdev)
 	u8 *temp = NULL;
 	int ret;
 
-	ret = nouveau_subdev_create(ndev, subdev, "THERM", "thermal", &ptherm);
-	if (ret)
-		return ret;
+	if (bios->type == NVBIOS_BIT) {
+		if (bit_table(ndev, 'P', &P))
+			return 0;
 
-	if (ndev->chipset >= 0x40 && ndev->chipset < 0x84)
-		ptherm->temp_get = nv40_therm_temp_get;
-	else
-	if (ndev->chipset <= 0xd9)
-		ptherm->temp_get = nv84_therm_temp_get;
-
-	if (bit_table(ndev, 'P', &P) == 0) {
 		if (P.version == 1)
 			temp = ROMPTR(ndev, P.data[12]);
 		else
 		if (P.version == 2)
 			temp = ROMPTR(ndev, P.data[16]);
-		else
+		else {
 			NV_WARN(ndev, "unknown temp for BIT P %d\n", P.version);
+		}
+	} else {
+		return 0;
+	}
 
-		nouveau_therm_vbios_parse(ptherm, temp);
+	if (!temp) {
+		NV_DEBUG(ndev, "temp table pointer invalid\n");
+		return 0;
 	}
 
+	ret = nouveau_subdev_create(ndev, subdev, "THERM", "thermal", &ptherm);
+	if (ret)
+		return ret;
+
+	nouveau_therm_vbios_parse(ptherm, temp);
 	nouveau_therm_probe_i2c(ndev);
 
+	ptherm->base.destroy = nouveau_ptherm_destroy;
+	switch (ndev->card_type) {
+	case NV_40:
+	case NV_50:
+	case NV_C0:
+	case NV_D0:
+	case NV_E0:
+		if (ndev->chipset < 0x84) {
+			ptherm->temp_get = nv40_therm_temp_get;
+			break;
+		} else
+		if (ndev->chipset <= 0xd9) {
+			ptherm->temp_get = nv84_therm_temp_get;
+			break;
+		}
+	default:
+		ptherm->temp_get = dummy_therm_temp_get;
+		break;
+	}
+
 	return nouveau_subdev_init(ndev, subdev, ret);
 }
-- 
1.7.10.2

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

[Emil Velikov]

On Mon, 21 May 2012 07:36:13 +0100, Maarten Maathuis  
<madman2003-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Does chipset 0x50 really use the NV40 function?
>

Unfortunately it does.

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

[Emil Velikov]

On Mon, 21 May 2012 07:30:32 +0100, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  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

Re: nouveau_subdev & misc patches

[Ben Skeggs]

On Mon, May 21, 2012 at 12:14:49AM +0100, Emil Velikov wrote:
> Hello all, this series includes a wide range of fixes - from a few
> month's old one-liners from Andreas Heider regarding vga_switcheroo, via a
> null pointer dereference and double memory allocation, to a buffer overflow.
Hey,

Thanks for these!  I've pulled most of them directly for the moment, comments
on specific patches will follow.

Cheers,
Ben.

> 
> Please review and comment
> 
> ---
>  drivers/gpu/drm/nouveau/nouveau_acpi.c   |    3 ++-
>  drivers/gpu/drm/nouveau/nouveau_device.c |   26 +++++++++++++++-----------
>  drivers/gpu/drm/nouveau/nouveau_fb.h     |    1 -
>  drivers/gpu/drm/nouveau/nouveau_gpio.h   |    3 ---
>  drivers/gpu/drm/nouveau/nouveau_gpuobj.c |   12 ++++--------
>  drivers/gpu/drm/nouveau/nouveau_perf.c   |    2 +-
>  drivers/gpu/drm/nouveau/nouveau_pm.c     |    6 +++---
>  drivers/gpu/drm/nouveau/nouveau_state.c  |    2 ++
>  drivers/gpu/drm/nouveau/nouveau_therm.c  |   63 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  drivers/gpu/drm/nouveau/nv04_instmem.c   |    3 +--
>  drivers/gpu/drm/nouveau/nv10_fb.c        |    4 ----
>  drivers/gpu/drm/nouveau/nv10_gpio.c      |   16 ++++++++--------
>  drivers/gpu/drm/nouveau/nv30_fb.c        |    3 +--
>  drivers/gpu/drm/nouveau/nv40_fanpwm.c    |   14 +++++++-------
>  drivers/gpu/drm/nouveau/nv40_fb.c        |   11 ++++-------
>  drivers/gpu/drm/nouveau/nv50_bar.c       |    2 --
>  drivers/gpu/drm/nouveau/nv50_fanpwm.c    |   20 ++++++++++----------
>  drivers/gpu/drm/nouveau/nv50_gpio.c      |   16 ++++++++--------
>  drivers/gpu/drm/nouveau/nv50_instmem.c   |   10 +++-------
>  drivers/gpu/drm/nouveau/nvc0_instmem.c   |   10 +++-------
>  drivers/gpu/drm/nouveau/nvd0_gpio.c      |   16 ++++++++--------
>  21 files changed, 129 insertions(+), 114 deletions(-)
> 
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 03/14] drm/nouveau/device: Simplify init, fini fuction

[Ben Skeggs]

On Mon, May 21, 2012 at 12:14:52AM +0100, Emil Velikov wrote:
> Removes unnecessary goto statement
I dropped these changes from this patch for the moment, I might want them yet.

> fini subdevices before destroying them
I took this hunk, and reworded the commit message to match.

> 
> Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpu/drm/nouveau/nouveau_device.c |   26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_device.c b/drivers/gpu/drm/nouveau/nouveau_device.c
> index 9cc21d1..38ed71a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_device.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_device.c
> @@ -55,13 +55,13 @@ nouveau_device_init(struct nouveau_device *ndev)
>  
>  	for (i = 0; i < NVDEV_SUBDEV_NR; i++) {
>  		ret = nouveau_subdev_init(ndev, i, 0);
> -		if (ret)
> -			goto error;
> +		if (ret) {
> +			for (--i; i >= 0; i--)
> +				nouveau_subdev_fini(ndev, i, false);
> +			break;
> +		}
>  	}
>  
> -error:
> -	for (--i; ret && i >= 0; i--)
> -		nouveau_subdev_fini(ndev, i, false);
>  	return ret;
>  }
>  
> @@ -72,13 +72,13 @@ nouveau_device_fini(struct nouveau_device *ndev, bool suspend)
>  
>  	for (i = NVDEV_SUBDEV_NR - 1; i >= 0; i--) {
>  		ret = nouveau_subdev_fini(ndev, i, suspend);
> -		if (ret)
> -			goto error;
> +		if (ret) {
> +			for (--i; i >= 0; i--)
> +				nouveau_subdev_init(ndev, i, 0);
> +			break;
> +		}
>  	}
>  
> -error:
> -	for (--i; ret && i >= 0; i--)
> -		nouveau_subdev_init(ndev, i, 0);
>  	return ret;
>  }
>  
> @@ -96,6 +96,7 @@ nouveau_device_create(struct nouveau_device *ndev)
>  {
>  	int disable = nouveau_noaccel;
>  	int ret = 0;
> +	int i;
>  
>  	/* mask out any engines that are known not to work as they should,
>  	 * these can be overridden by the user
> @@ -553,7 +554,10 @@ nouveau_device_create(struct nouveau_device *ndev)
>  		break;
>  	}
>  
> -	if (ret)
> +	if (ret) {
> +		for (i = NVDEV_SUBDEV_NR - 1; i >= 0; i--)
> +			nouveau_subdev_fini(ndev, i, false);
>  		nouveau_device_destroy(ndev);
> +	}
>  	return ret;
>  }
> -- 
> 1.7.10.2
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Ben Skeggs]

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.

> 
> Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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?

>  static int
>  nv40_sensor_setup(struct nouveau_therm *ptherm)
>  {
> @@ -181,7 +187,7 @@ nouveau_therm_vbios_parse(struct nouveau_therm *ptherm, u8 *temp)
>  	temp = temp + headerlen;
>  
>  	/* Read the entries from the table */
> -	for (i = 0; i < entries; i++) {
> +	for (i = 0; i < entries; i++, temp += recordlen) {
>  		s16 value = ROM16(temp[1]);
>  
>  		switch (temp[0]) {
> @@ -228,7 +234,6 @@ nouveau_therm_vbios_parse(struct nouveau_therm *ptherm, u8 *temp)
>  			ptherm->fan.pwm_freq = value;
>  			break;
>  		}
> -		temp += recordlen;
>  	}
>  
>  	nouveau_therm_safety_checks(ptherm);
> @@ -299,6 +304,12 @@ nouveau_therm_probe_i2c(struct nouveau_device *ndev)
>  			     probe_monitoring_device, NV_I2C_DEFAULT(0));
>  }
>  
> +static void
> +nouveau_ptherm_destroy(struct nouveau_device *ndev, int subdev)
> +{
> +// XXX: Undo probe_monitoring_device
> +}
> +
>  int
>  nouveau_therm_create(struct nouveau_device *ndev, int subdev)
>  {
> @@ -307,29 +318,53 @@ nouveau_therm_create(struct nouveau_device *ndev, int subdev)
>  	u8 *temp = NULL;
>  	int ret;
>  
> -	ret = nouveau_subdev_create(ndev, subdev, "THERM", "thermal", &ptherm);
> -	if (ret)
> -		return ret;
> +	if (bios->type == NVBIOS_BIT) {
> +		if (bit_table(ndev, 'P', &P))
> +			return 0;
The BIT check isn't necessary, bit_table() will fail if it's not a BIT BIOS.

>  
> -	if (ndev->chipset >= 0x40 && ndev->chipset < 0x84)
> -		ptherm->temp_get = nv40_therm_temp_get;
> -	else
> -	if (ndev->chipset <= 0xd9)
> -		ptherm->temp_get = nv84_therm_temp_get;
> -
> -	if (bit_table(ndev, 'P', &P) == 0) {
>  		if (P.version == 1)
>  			temp = ROMPTR(ndev, P.data[12]);
>  		else
>  		if (P.version == 2)
>  			temp = ROMPTR(ndev, P.data[16]);
> -		else
> +		else {
>  			NV_WARN(ndev, "unknown temp for BIT P %d\n", P.version);
> +		}
> +	} else {
> +		return 0;
> +	}
>  
> -		nouveau_therm_vbios_parse(ptherm, temp);
> +	if (!temp) {
> +		NV_DEBUG(ndev, "temp table pointer invalid\n");
> +		return 0;
>  	}
>  
> +	ret = nouveau_subdev_create(ndev, subdev, "THERM", "thermal", &ptherm);
> +	if (ret)
> +		return ret;
> +
> +	nouveau_therm_vbios_parse(ptherm, temp);
>  	nouveau_therm_probe_i2c(ndev);
>  
> +	ptherm->base.destroy = nouveau_ptherm_destroy;
> +	switch (ndev->card_type) {
> +	case NV_40:
> +	case NV_50:
> +	case NV_C0:
> +	case NV_D0:
> +	case NV_E0:
> +		if (ndev->chipset < 0x84) {
> +			ptherm->temp_get = nv40_therm_temp_get;
> +			break;
> +		} else
> +		if (ndev->chipset <= 0xd9) {
> +			ptherm->temp_get = nv84_therm_temp_get;
> +			break;
> +		}
> +	default:
> +		ptherm->temp_get = dummy_therm_temp_get;
> +		break;
> +	}
> +
>  	return nouveau_subdev_init(ndev, subdev, ret);
>  }
> -- 
> 1.7.10.2
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Maarten Maathuis]

On Mon, May 21, 2012 at 8:30 AM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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.
>
>>
>> Signed-off-by: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  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?
>
>>  static int
>>  nv40_sensor_setup(struct nouveau_therm *ptherm)
>>  {
>> @@ -181,7 +187,7 @@ nouveau_therm_vbios_parse(struct nouveau_therm *ptherm, u8 *temp)
>>       temp = temp + headerlen;
>>
>>       /* Read the entries from the table */
>> -     for (i = 0; i < entries; i++) {
>> +     for (i = 0; i < entries; i++, temp += recordlen) {
>>               s16 value = ROM16(temp[1]);
>>
>>               switch (temp[0]) {
>> @@ -228,7 +234,6 @@ nouveau_therm_vbios_parse(struct nouveau_therm *ptherm, u8 *temp)
>>                       ptherm->fan.pwm_freq = value;
>>                       break;
>>               }
>> -             temp += recordlen;
>>       }
>>
>>       nouveau_therm_safety_checks(ptherm);
>> @@ -299,6 +304,12 @@ nouveau_therm_probe_i2c(struct nouveau_device *ndev)
>>                            probe_monitoring_device, NV_I2C_DEFAULT(0));
>>  }
>>
>> +static void
>> +nouveau_ptherm_destroy(struct nouveau_device *ndev, int subdev)
>> +{
>> +// XXX: Undo probe_monitoring_device
>> +}
>> +
>>  int
>>  nouveau_therm_create(struct nouveau_device *ndev, int subdev)
>>  {
>> @@ -307,29 +318,53 @@ nouveau_therm_create(struct nouveau_device *ndev, int subdev)
>>       u8 *temp = NULL;
>>       int ret;
>>
>> -     ret = nouveau_subdev_create(ndev, subdev, "THERM", "thermal", &ptherm);
>> -     if (ret)
>> -             return ret;
>> +     if (bios->type == NVBIOS_BIT) {
>> +             if (bit_table(ndev, 'P', &P))
>> +                     return 0;
> The BIT check isn't necessary, bit_table() will fail if it's not a BIT BIOS.
>
>>
>> -     if (ndev->chipset >= 0x40 && ndev->chipset < 0x84)
>> -             ptherm->temp_get = nv40_therm_temp_get;
>> -     else
>> -     if (ndev->chipset <= 0xd9)
>> -             ptherm->temp_get = nv84_therm_temp_get;
>> -
>> -     if (bit_table(ndev, 'P', &P) == 0) {
>>               if (P.version == 1)
>>                       temp = ROMPTR(ndev, P.data[12]);
>>               else
>>               if (P.version == 2)
>>                       temp = ROMPTR(ndev, P.data[16]);
>> -             else
>> +             else {
>>                       NV_WARN(ndev, "unknown temp for BIT P %d\n", P.version);
>> +             }
>> +     } else {
>> +             return 0;
>> +     }
>>
>> -             nouveau_therm_vbios_parse(ptherm, temp);
>> +     if (!temp) {
>> +             NV_DEBUG(ndev, "temp table pointer invalid\n");
>> +             return 0;
>>       }
>>
>> +     ret = nouveau_subdev_create(ndev, subdev, "THERM", "thermal", &ptherm);
>> +     if (ret)
>> +             return ret;
>> +
>> +     nouveau_therm_vbios_parse(ptherm, temp);
>>       nouveau_therm_probe_i2c(ndev);
>>
>> +     ptherm->base.destroy = nouveau_ptherm_destroy;
>> +     switch (ndev->card_type) {
>> +     case NV_40:
>> +     case NV_50:
>> +     case NV_C0:
>> +     case NV_D0:
>> +     case NV_E0:
>> +             if (ndev->chipset < 0x84) {
>> +                     ptherm->temp_get = nv40_therm_temp_get;
>> +                     break;
>> +             } else
>> +             if (ndev->chipset <= 0xd9) {
>> +                     ptherm->temp_get = nv84_therm_temp_get;
>> +                     break;
>> +             }
>> +     default:
>> +             ptherm->temp_get = dummy_therm_temp_get;
>> +             break;
>> +     }
>> +
>>       return nouveau_subdev_init(ndev, subdev, ret);
>>  }
>> --
>> 1.7.10.2
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
> _______________________________________________
> Nouveau mailing list
> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Does chipset 0x50 really use the NV40 function?

-- 
Far away from the primal instinct, the song seems to fade away, the
river get wider between your thoughts and the things we do and say.

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

[Ben Skeggs]

On Mon, May 21, 2012 at 07:13:54AM +0100, Emil Velikov wrote:
> On Mon, 21 May 2012 07:30:32 +0100, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>---
> >> 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