Re: [Nouveau] [PATCH v2 16/22] volt: don't require perfect fit

2016-03-28 Thread Martin Peres 

On 21/03/16 18:16, Karol Herbst wrote:

if we calculate the voltage in the table right, we get all kinds of values,
which never fit the hardware steps, so we use the closest higher value the
hardware can do


This is indeed the goal.



Signed-off-by: Karol Herbst 
---
  drm/nouveau/nvkm/subdev/volt/base.c | 25 ++---
  1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/volt/base.c 
b/drm/nouveau/nvkm/subdev/volt/base.c
index e741383..58738e3 100644
--- a/drm/nouveau/nvkm/subdev/volt/base.c
+++ b/drm/nouveau/nvkm/subdev/volt/base.c
@@ -51,18 +51,37 @@ static int
  nvkm_volt_set(struct nvkm_volt *volt, u32 uv)
  {
struct nvkm_subdev *subdev = >subdev;
-   int i, ret = -EINVAL;
+   int i, ret = -EINVAL, err, best = -1;
  
  	if (volt->func->volt_set)

return volt->func->volt_set(volt, uv);
  
  	for (i = 0; i < volt->vid_nr; i++) {

-   if (volt->vid[i].uv == uv) {
-   ret = volt->func->vid_set(volt, volt->vid[i].vid);
+   if (i == 0) {
+   best = 0;
+   err = volt->vid[i].uv - uv;
+   } else {
+   int new_err = volt->vid[i].uv - uv;
+   if (abs(new_err) < abs(err)
+   || (err < 0 && new_err >= 0)) {
+   best = i;
+   err = new_err;
+   }
+   }
+
+   if (err == 0) {
+   ret = volt->func->vid_set(volt, volt->vid[best].vid);
nvkm_debug(subdev, "set %duv: %d\n", uv, ret);
break;
}
}
+
+   if (best != -1) {
+   ret = volt->func->vid_set(volt, volt->vid[best].vid);
+   nvkm_debug(subdev, "set req %duv to %duv: %d\n", uv,
+  volt->vid[best].uv, ret);
+   }
+
return ret;
  }


The code is really weird though :o How about the following? It would 
make it much more readable and actually do what your commit message says.


int best = -1;
u32 best_abs_err = -1;
for (i = 0; i < volt->vid_nr; i++) {
int abs_err = abs(volt->vid[i].uv - uv);
if (volt->vid[i].uv >= uv && abs_err < best_abs_err) {
best = i;
best_abs_err = abs_err; 
if (abs_err == 0)
break;
 }
}

if (best >= 0) {
ret = volt->func->vid_set(volt, volt->vid[best].vid);
nvkm_debug(subdev, "set %duv: %d\n", uv, ret);
} else {
/* return an error to tell that we cannot use the wanted clock since we 
cannot select the needed voltage! */
}


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 18/22] volt: add coefficients I found on my gpu

2016-03-28 Thread Martin Peres 

On 21/03/16 18:16, Karol Herbst wrote:

I am sure that those are a bit different on other GPUs, but while testing
the error range compared to nvidia was around 100%+-3%.

Without this change we are most of the time around 10% below nvidias voltage,
so this change causes no harm and improves the situation a lot already.

The remaining task for this is to figure out which of these constants are
chip specific and from where to get the chip specific factors

Signed-off-by: Karol Herbst 


Instead of landing these coefficients like that, I would really like if 
you could dump pfuse and check if you find these coefficients there. 
What could be done also is to re-do the work you did to find out those 
coefficients on a GPU where your coefficients are not really good and 
find out what values would have needed and then compare pfuse with the 
original card.


If this is to be considered a long task, then I am OK landing those 
coefficients, but I would really appreciate if you could at least have a 
quick look at pfuse :)

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 06/22] volt: parse the both max voltage entries

2016-03-28 Thread Martin Peres 

On 28/03/16 23:49, Martin Peres wrote:

On 21/03/16 18:16, Karol Herbst wrote:

these entries specify a maximum voltage nvidia never exceeds, we shouldn't do
that, too.

Signed-off-by: Karol Herbst 
---
   drm/nouveau/include/nvkm/subdev/bios/vmap.h |  2 ++
   drm/nouveau/include/nvkm/subdev/volt.h  |  2 ++
   drm/nouveau/nvkm/subdev/bios/vmap.c |  5 +
   drm/nouveau/nvkm/subdev/volt/base.c | 11 +++
   4 files changed, 20 insertions(+)

diff --git a/drm/nouveau/include/nvkm/subdev/bios/vmap.h 
b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
index 6633c6d..48fe71d 100644
--- a/drm/nouveau/include/nvkm/subdev/bios/vmap.h
+++ b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
@@ -1,6 +1,8 @@
   #ifndef __NVBIOS_VMAP_H__
   #define __NVBIOS_VMAP_H__
   struct nvbios_vmap {
+   u8  max0;
+   u8  max1;
   };
   
   u16 nvbios_vmap_table(struct nvkm_bios *, u8 *ver, u8 *hdr, u8 *cnt, u8 *len);

diff --git a/drm/nouveau/include/nvkm/subdev/volt.h 
b/drm/nouveau/include/nvkm/subdev/volt.h
index fc68825..3e0f8da 100644
--- a/drm/nouveau/include/nvkm/subdev/volt.h
+++ b/drm/nouveau/include/nvkm/subdev/volt.h
@@ -15,6 +15,8 @@ struct nvkm_volt {
   
   	u32 max_uv;

u32 min_uv;
+   u8 max0_vid;
+   u8 max1_vid;
   };
   
   int nvkm_volt_map_min(struct nvkm_volt *volt, u8 id);

diff --git a/drm/nouveau/nvkm/subdev/bios/vmap.c 
b/drm/nouveau/nvkm/subdev/bios/vmap.c
index 2f13db7..f5463b1 100644
--- a/drm/nouveau/nvkm/subdev/bios/vmap.c
+++ b/drm/nouveau/nvkm/subdev/bios/vmap.c
@@ -61,7 +61,12 @@ nvbios_vmap_parse(struct nvkm_bios *bios, u8 *ver, u8 *hdr, 
u8 *cnt, u8 *len,
memset(info, 0x00, sizeof(*info));
switch (!!vmap * *ver) {
case 0x10:
+   info->max0 = 0xff;
+   info->max1 = 0xff;
+   break;
case 0x20:
+   info->max0 = nvbios_rd08(bios, vmap + 0x7);
+   info->max1 = nvbios_rd08(bios, vmap + 0x8);
break;
}
return vmap;
diff --git a/drm/nouveau/nvkm/subdev/volt/base.c 
b/drm/nouveau/nvkm/subdev/volt/base.c
index 71094a9..205dfcf 100644
--- a/drm/nouveau/nvkm/subdev/volt/base.c
+++ b/drm/nouveau/nvkm/subdev/volt/base.c
@@ -217,9 +217,20 @@ nvkm_volt_ctor(const struct nvkm_volt_func *func, struct 
nvkm_device *device,
   
   	/* Assuming the non-bios device should build the voltage table later */

if (bios) {
+   u8 ver, hdr, cnt, len;
+   struct nvbios_vmap vmap;
+
nvkm_volt_parse_bios(bios, volt);
nvkm_debug(>subdev, "min: %iuv max: %iuv\n",
   volt->min_uv, volt->max_uv);
+
+   if (nvbios_vmap_parse(bios, , , , , )) {
+   volt->max0_vid = vmap.max0;
+   volt->max1_vid = vmap.max1;
+   } else {
+   volt->max0_vid = 0xff;
+   volt->max1_vid = 0xff;
+   }
}
   
   	if (volt->vid_nr) {

This is really peculiar that NVIDIA would have 2 max_vid in the same
entry :s

How did you reverse that? Did you really see no differences? Maybe the
two max_vid are for different temperatures?
Ok, after discussions on IRC, I understand better. Max0/1 is actually 
not a vid but an entry in the voltage mapping table, which has different 
coefficient for the temperature. This allows OEMs to clamp the maximum 
voltage with not one but two different functions. I can see why it is OK.


Can you explain this a little more? Maybe as a comment in the code?

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 22/22] debugfs: add boost interface to change the boost_mode

2016-03-28 Thread Martin Peres 

On 21/03/16 18:16, Karol Herbst wrote:

Signed-off-by: Karol Herbst 


21-22 are:

Reviewed-by: Martin Peres 
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 07/22] volt: add min_id parameter to nvkm_volt_set_id

2016-03-28 Thread Martin Peres 

On 28/03/16 23:52, Martin Peres wrote:

On 21/03/16 18:16, Karol Herbst wrote:

min_id indicates a volt map entry which acts as a floor value, this will be
used to set the lower voltage limit through pstates


Please state that this comes that this min_id is different for each 
pstate, hence why volt should not know about this and needs to take it 
as an input!




Signed-off-by: Karol Herbst 

Do we really want to push reclocking logic to the volt subsystem?

To me, volt should just allow you to read back and set a voltage. All
the rest of the logic should be in clk.

Since this is my first NAK, here are my R-b for 3, 4 and 5:

Reviewed-by: Martin Peres 


With this fixed, this patch is Reviewed-by: Martin Peres 


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 07/22] volt: add min_id parameter to nvkm_volt_set_id

2016-03-28 Thread Martin Peres 

On 21/03/16 18:16, Karol Herbst wrote:

min_id indicates a volt map entry which acts as a floor value, this will be
used to set the lower voltage limit through pstates

Signed-off-by: Karol Herbst 


Do we really want to push reclocking logic to the volt subsystem?

To me, volt should just allow you to read back and set a voltage. All 
the rest of the logic should be in clk.


Since this is my first NAK, here are my R-b for 3, 4 and 5:

Reviewed-by: Martin Peres 
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 06/22] volt: parse the both max voltage entries

2016-03-28 Thread Martin Peres 

On 21/03/16 18:16, Karol Herbst wrote:

these entries specify a maximum voltage nvidia never exceeds, we shouldn't do
that, too.

Signed-off-by: Karol Herbst 
---
  drm/nouveau/include/nvkm/subdev/bios/vmap.h |  2 ++
  drm/nouveau/include/nvkm/subdev/volt.h  |  2 ++
  drm/nouveau/nvkm/subdev/bios/vmap.c |  5 +
  drm/nouveau/nvkm/subdev/volt/base.c | 11 +++
  4 files changed, 20 insertions(+)

diff --git a/drm/nouveau/include/nvkm/subdev/bios/vmap.h 
b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
index 6633c6d..48fe71d 100644
--- a/drm/nouveau/include/nvkm/subdev/bios/vmap.h
+++ b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
@@ -1,6 +1,8 @@
  #ifndef __NVBIOS_VMAP_H__
  #define __NVBIOS_VMAP_H__
  struct nvbios_vmap {
+   u8  max0;
+   u8  max1;
  };
  
  u16 nvbios_vmap_table(struct nvkm_bios *, u8 *ver, u8 *hdr, u8 *cnt, u8 *len);

diff --git a/drm/nouveau/include/nvkm/subdev/volt.h 
b/drm/nouveau/include/nvkm/subdev/volt.h
index fc68825..3e0f8da 100644
--- a/drm/nouveau/include/nvkm/subdev/volt.h
+++ b/drm/nouveau/include/nvkm/subdev/volt.h
@@ -15,6 +15,8 @@ struct nvkm_volt {
  
  	u32 max_uv;

u32 min_uv;
+   u8 max0_vid;
+   u8 max1_vid;
  };
  
  int nvkm_volt_map_min(struct nvkm_volt *volt, u8 id);

diff --git a/drm/nouveau/nvkm/subdev/bios/vmap.c 
b/drm/nouveau/nvkm/subdev/bios/vmap.c
index 2f13db7..f5463b1 100644
--- a/drm/nouveau/nvkm/subdev/bios/vmap.c
+++ b/drm/nouveau/nvkm/subdev/bios/vmap.c
@@ -61,7 +61,12 @@ nvbios_vmap_parse(struct nvkm_bios *bios, u8 *ver, u8 *hdr, 
u8 *cnt, u8 *len,
memset(info, 0x00, sizeof(*info));
switch (!!vmap * *ver) {
case 0x10:
+   info->max0 = 0xff;
+   info->max1 = 0xff;
+   break;
case 0x20:
+   info->max0 = nvbios_rd08(bios, vmap + 0x7);
+   info->max1 = nvbios_rd08(bios, vmap + 0x8);
break;
}
return vmap;
diff --git a/drm/nouveau/nvkm/subdev/volt/base.c 
b/drm/nouveau/nvkm/subdev/volt/base.c
index 71094a9..205dfcf 100644
--- a/drm/nouveau/nvkm/subdev/volt/base.c
+++ b/drm/nouveau/nvkm/subdev/volt/base.c
@@ -217,9 +217,20 @@ nvkm_volt_ctor(const struct nvkm_volt_func *func, struct 
nvkm_device *device,
  
  	/* Assuming the non-bios device should build the voltage table later */

if (bios) {
+   u8 ver, hdr, cnt, len;
+   struct nvbios_vmap vmap;
+
nvkm_volt_parse_bios(bios, volt);
nvkm_debug(>subdev, "min: %iuv max: %iuv\n",
   volt->min_uv, volt->max_uv);
+
+   if (nvbios_vmap_parse(bios, , , , , )) {
+   volt->max0_vid = vmap.max0;
+   volt->max1_vid = vmap.max1;
+   } else {
+   volt->max0_vid = 0xff;
+   volt->max1_vid = 0xff;
+   }
}
  
  	if (volt->vid_nr) {


This is really peculiar that NVIDIA would have 2 max_vid in the same 
entry :s


How did you reverse that? Did you really see no differences? Maybe the 
two max_vid are for different temperatures?


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [Bug 94728] Nouveau doesn't work with Nvidia GTX 960

2016-03-28 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=94728

--- Comment #4 from neilmed...@gmail.com ---
Makes sense. Thanks for the quick replies - and the good work in maintaining
nouveau :)

-- 
You are receiving this mail because:
You are the assignee for the bug.___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [Bug 91526] World of Warcraft (on Wine) has UI corruption with nouveau

2016-03-28 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=91526

Ilia Mirkin  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #15 from Ilia Mirkin  ---
Pushed to master:

commit 41100b6b44e747b9003937f123fce571fd3dec46
Author: Ilia Mirkin 
Date:   Sat Mar 26 22:32:43 2016 -0400

nvc0: disable primitive restart and index bias during blits

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [Bug 94728] Nouveau doesn't work with Nvidia GTX 960

2016-03-28 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=94728

--- Comment #3 from Ilia Mirkin  ---
As you can see, it's falling back to modesetting, as expected. The
xf86-video-nouveau DDX does not support any maxwell GPUs at the moment.

Separately, modesetting isn't able to start up glamor because of the reasons
that Martin pointed out. This means that you'll get swrast when running
applications inside X as well.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [Bug 91526] World of Warcraft (on Wine) has UI corruption with nouveau

2016-03-28 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=91526

--- Comment #14 from Karol Herbst  ---
(In reply to Ilia Mirkin from comment #13)
> As I had originally suspected, this has nothing to actually do with BGRA4
> format. It was just an odd place in the command stream.
> 
> The following patch should fix it:
> https://patchwork.freedesktop.org/patch/78448/
> 
> Please test. I think this has been broken since mesa 10.2 or so =/

works great, thanks :)

Tested-by: Karol Herbst 

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH v2 2/4] iccsense: convert to linked list

2016-03-28 Thread Karol Herbst 
v2: add list_del calls

Signed-off-by: Karol Herbst 
Reviewed-by: Martin Peres 
---
 drm/nouveau/include/nvkm/subdev/iccsense.h |  4 +---
 drm/nouveau/nouveau_hwmon.c|  2 +-
 drm/nouveau/nvkm/subdev/iccsense/base.c| 34 ++
 drm/nouveau/nvkm/subdev/iccsense/priv.h|  1 +
 4 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/iccsense.h 
b/drm/nouveau/include/nvkm/subdev/iccsense.h
index c3defcd..a4c0da0 100644
--- a/drm/nouveau/include/nvkm/subdev/iccsense.h
+++ b/drm/nouveau/include/nvkm/subdev/iccsense.h
@@ -3,12 +3,10 @@
 
 #include 
 
-struct nkvm_iccsense_rail;
 struct nvkm_iccsense {
struct nvkm_subdev subdev;
-   u8 rail_count;
bool data_valid;
-   struct nvkm_iccsense_rail *rails;
+   struct list_head rails;
 };
 
 int gf100_iccsense_new(struct nvkm_device *, int index, struct nvkm_iccsense 
**);
diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c
index 67edd2f..74f237b 100644
--- a/drm/nouveau/nouveau_hwmon.c
+++ b/drm/nouveau/nouveau_hwmon.c
@@ -689,7 +689,7 @@ nouveau_hwmon_init(struct drm_device *dev)
goto error;
}
 
-   if (iccsense && iccsense->data_valid && iccsense->rail_count) {
+   if (iccsense && iccsense->data_valid && !list_empty(>rails)) {
ret = sysfs_create_group(_dev->kobj,
 _power_attrgroup);
if (ret)
diff --git a/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drm/nouveau/nvkm/subdev/iccsense/base.c
index bf1b94e..f2a210a 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -98,25 +98,21 @@ nvkm_iccsense_ina3221_read(struct nvkm_iccsense *iccsense,
 int
 nvkm_iccsense_read_all(struct nvkm_iccsense *iccsense)
 {
-   int result = 0, i;
+   int result = 0;
+   struct nvkm_iccsense_rail *rail;
 
if (!iccsense)
return -EINVAL;
 
-   if (iccsense->rail_count == 0)
-   return -ENODEV;
-
-   for (i = 0; i < iccsense->rail_count; ++i) {
+   list_for_each_entry(rail, >rails, head) {
int res;
-   struct nvkm_iccsense_rail *rail = >rails[i];
if (!rail->read)
return -ENODEV;
 
res = rail->read(iccsense, rail);
-   if (res >= 0)
-   result += res;
-   else
+   if (res < 0)
return res;
+   result += res;
}
return result;
 }
@@ -125,9 +121,12 @@ static void *
 nvkm_iccsense_dtor(struct nvkm_subdev *subdev)
 {
struct nvkm_iccsense *iccsense = nvkm_iccsense(subdev);
+   struct nvkm_iccsense_rail *rail, *tmp;
 
-   if (iccsense->rails)
-   kfree(iccsense->rails);
+   list_for_each_entry_safe(rail, tmp, >rails, head) {
+   list_del(>head);
+   kfree(rail);
+   }
 
return iccsense;
 }
@@ -145,11 +144,6 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
|| !stbl.nr_entry)
return 0;
 
-   iccsense->rails = kmalloc(sizeof(*iccsense->rails) * stbl.nr_entry,
- GFP_KERNEL);
-   if (!iccsense->rails)
-   return -ENOMEM;
-
iccsense->data_valid = true;
for (i = 0; i < stbl.nr_entry; ++i) {
struct pwr_rail_t *r = [i];
@@ -184,7 +178,10 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
continue;
}
 
-   rail = >rails[iccsense->rail_count];
+   rail = kmalloc(sizeof(*rail), GFP_KERNEL);
+   if (!rail)
+   return -ENOMEM;
+
switch (extdev.type) {
case NVBIOS_EXTDEV_INA209:
rail->read = nvkm_iccsense_ina209_read;
@@ -201,7 +198,7 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
rail->rail = r->rail;
rail->mohm = r->resistor_mohm;
rail->i2c = _bus->i2c;
-   ++iccsense->rail_count;
+   list_add_tail(>head, >rails);
}
return 0;
 }
@@ -224,6 +221,7 @@ nvkm_iccsense_new_(struct nvkm_device *device, int index,
 {
if (!(*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL)))
return -ENOMEM;
+   INIT_LIST_HEAD(&(*iccsense)->rails);
nvkm_iccsense_ctor(device, index, *iccsense);
return 0;
 }
diff --git a/drm/nouveau/nvkm/subdev/iccsense/priv.h 
b/drm/nouveau/nvkm/subdev/iccsense/priv.h
index ed398b8..e479128 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/priv.h
+++ b/drm/nouveau/nvkm/subdev/iccsense/priv.h
@@ -4,6 +4,7 @@
 #include 
 
 struct nvkm_iccsense_rail {
+   struct list_head head;
int (*read)(struct nvkm_iccsense *, struct nvkm_iccsense_rail *);
struct

[Nouveau] [PATCH v2 4/4] iccsense: configure sensors like nvidia does

2016-03-28 Thread Karol Herbst 
v2: rename ina209/ina219 read function

Signed-off-by: Karol Herbst 
Reviewed-by: Martin Peres 
---
 drm/nouveau/nvkm/subdev/iccsense/base.c | 68 +
 1 file changed, 68 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drm/nouveau/nvkm/subdev/iccsense/base.c
index 03eef9d..55f36f5 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -95,6 +95,63 @@ nvkm_iccsense_ina3221_read(struct nvkm_iccsense *iccsense,
   40 * 8);
 }
 
+static void
+nvkm_iccsense_ina209_config(struct nvkm_iccsense *iccsense,
+   struct nvkm_iccsense_sensor *sensor)
+{
+   struct nvkm_subdev *subdev = >subdev;
+   /* configuration:
+* 0x0007: 0x0007 shunt and bus continous
+* 0x0078: 0x0078 128 samples shunt
+* 0x0780: 0x0780 128 samples bus
+* 0x1800: 0x +-40 mV shunt range
+* 0x2000: 0x 16V FSR
+ */
+   u16 value = 0x07ff;
+   nvkm_debug(subdev, "config for sensor id %i: 0x%x\n", sensor->id, 
value);
+   nv_wr16i2cr(sensor->i2c, sensor->addr, 0x00, value);
+}
+
+static void
+nvkm_iccsense_ina3221_config(struct nvkm_iccsense *iccsense,
+struct nvkm_iccsense_sensor *sensor)
+{
+   struct nvkm_subdev *subdev = >subdev;
+   /* configuration:
+* 0x0007: 0x0007 shunt and bus continous
+* 0x0031: 0x 140 us conversion time shunt
+* 0x01c0: 0x 140 us conversion time bus
+* 0x0f00: 0x0f00 1024 samples
+* 0x7000: 0x?000 channels
+ */
+   u16 value = 0x0e07;
+   if (sensor->rail_mask & 0x1)
+   value |= 0x1 << 14;
+   if (sensor->rail_mask & 0x2)
+   value |= 0x1 << 13;
+   if (sensor->rail_mask & 0x4)
+   value |= 0x1 << 12;
+   nvkm_debug(subdev, "config for sensor id %i: 0x%x\n", sensor->id, 
value);
+   nv_wr16i2cr(sensor->i2c, sensor->addr, 0x00, value);
+}
+
+static void
+nvkm_iccsense_sensor_config(struct nvkm_iccsense *iccsense,
+   struct nvkm_iccsense_sensor *sensor)
+{
+   switch (sensor->type) {
+   case NVBIOS_EXTDEV_INA209:
+   case NVBIOS_EXTDEV_INA219:
+   nvkm_iccsense_ina209_config(iccsense, sensor);
+   break;
+   case NVBIOS_EXTDEV_INA3221:
+   nvkm_iccsense_ina3221_config(iccsense, sensor);
+   break;
+   default:
+   break;
+   }
+}
+
 int
 nvkm_iccsense_read_all(struct nvkm_iccsense *iccsense)
 {
@@ -260,8 +317,19 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
return 0;
 }
 
+static int
+nvkm_iccsense_init(struct nvkm_subdev *subdev)
+{
+   struct nvkm_iccsense *iccsense = nvkm_iccsense(subdev);
+   struct nvkm_iccsense_sensor *sensor;
+   list_for_each_entry(sensor, >sensors, head)
+   nvkm_iccsense_sensor_config(iccsense, sensor);
+   return 0;
+}
+
 struct nvkm_subdev_func iccsense_func = {
.oneinit = nvkm_iccsense_oneinit,
+   .init = nvkm_iccsense_init,
.dtor = nvkm_iccsense_dtor,
 };
 
-- 
2.7.4

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH v2 0/4] Configure Power Sensors

2016-03-28 Thread Karol Herbst 
The power sensors can be configured to sample the readout values over time.
Nvidia does this too, so nouveau should probably do that too.

v2: use list_del and rework an error message

Karol Herbst (4):
  iccsense: remove read function
  iccsense: convert to linked list
  iccsense: split sensor into own struct
  iccsense: configure sensors like nvidia does

 drm/nouveau/include/nvkm/subdev/iccsense.h |   6 +-
 drm/nouveau/nouveau_hwmon.c|   2 +-
 drm/nouveau/nvkm/subdev/iccsense/base.c| 249 +
 drm/nouveau/nvkm/subdev/iccsense/priv.h|  16 +-
 4 files changed, 201 insertions(+), 72 deletions(-)

-- 
2.7.4

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH v2 1/4] iccsense: remove read function

2016-03-28 Thread Karol Herbst 
Signed-off-by: Karol Herbst 
Reviewed-by: Martin Peres 
---
 drm/nouveau/include/nvkm/subdev/iccsense.h |  1 -
 drm/nouveau/nvkm/subdev/iccsense/base.c| 23 ++-
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/iccsense.h 
b/drm/nouveau/include/nvkm/subdev/iccsense.h
index 530c621..c3defcd 100644
--- a/drm/nouveau/include/nvkm/subdev/iccsense.h
+++ b/drm/nouveau/include/nvkm/subdev/iccsense.h
@@ -12,6 +12,5 @@ struct nvkm_iccsense {
 };
 
 int gf100_iccsense_new(struct nvkm_device *, int index, struct nvkm_iccsense 
**);
-int nvkm_iccsense_read(struct nvkm_iccsense *iccsense, u8 idx);
 int nvkm_iccsense_read_all(struct nvkm_iccsense *iccsense);
 #endif
diff --git a/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drm/nouveau/nvkm/subdev/iccsense/base.c
index c44a852..bf1b94e 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -96,26 +96,23 @@ nvkm_iccsense_ina3221_read(struct nvkm_iccsense *iccsense,
 }
 
 int
-nvkm_iccsense_read(struct nvkm_iccsense *iccsense, u8 idx)
+nvkm_iccsense_read_all(struct nvkm_iccsense *iccsense)
 {
-   struct nvkm_iccsense_rail *rail;
+   int result = 0, i;
 
-   if (!iccsense || idx >= iccsense->rail_count)
+   if (!iccsense)
return -EINVAL;
 
-   rail = >rails[idx];
-   if (!rail->read)
+   if (iccsense->rail_count == 0)
return -ENODEV;
 
-   return rail->read(iccsense, rail);
-}
-
-int
-nvkm_iccsense_read_all(struct nvkm_iccsense *iccsense)
-{
-   int result = 0, i;
for (i = 0; i < iccsense->rail_count; ++i) {
-   int res = nvkm_iccsense_read(iccsense, i);
+   int res;
+   struct nvkm_iccsense_rail *rail = >rails[i];
+   if (!rail->read)
+   return -ENODEV;
+
+   res = rail->read(iccsense, rail);
if (res >= 0)
result += res;
else
-- 
2.7.4

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH v2 3/4] iccsense: split sensor into own struct

2016-03-28 Thread Karol Herbst 
v2: add list_del call, reword error message

Signed-off-by: Karol Herbst 
Reviewed-by: Martin Peres 
---
 drm/nouveau/include/nvkm/subdev/iccsense.h |   1 +
 drm/nouveau/nvkm/subdev/iccsense/base.c| 142 -
 drm/nouveau/nvkm/subdev/iccsense/priv.h|  15 ++-
 3 files changed, 113 insertions(+), 45 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/iccsense.h 
b/drm/nouveau/include/nvkm/subdev/iccsense.h
index a4c0da0..3c2ddd9 100644
--- a/drm/nouveau/include/nvkm/subdev/iccsense.h
+++ b/drm/nouveau/include/nvkm/subdev/iccsense.h
@@ -6,6 +6,7 @@
 struct nvkm_iccsense {
struct nvkm_subdev subdev;
bool data_valid;
+   struct list_head sensors;
struct list_head rails;
 };
 
diff --git a/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drm/nouveau/nvkm/subdev/iccsense/base.c
index f2a210a..03eef9d 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -30,15 +30,14 @@
 
 static bool
 nvkm_iccsense_validate_device(struct i2c_adapter *i2c, u8 addr,
- enum nvbios_extdev_type type, u8 rail)
+ enum nvbios_extdev_type type)
 {
switch (type) {
case NVBIOS_EXTDEV_INA209:
case NVBIOS_EXTDEV_INA219:
-   return rail == 0 && nv_rd16i2cr(i2c, addr, 0x0) >= 0;
+   return nv_rd16i2cr(i2c, addr, 0x0) >= 0;
case NVBIOS_EXTDEV_INA3221:
-   return rail <= 3 &&
-  nv_rd16i2cr(i2c, addr, 0xff) == 0x3220 &&
+   return nv_rd16i2cr(i2c, addr, 0xff) == 0x3220 &&
   nv_rd16i2cr(i2c, addr, 0xfe) == 0x5449;
default:
return false;
@@ -67,8 +66,9 @@ nvkm_iccsense_ina2x9_read(struct nvkm_iccsense *iccsense,
   struct nvkm_iccsense_rail *rail,
  u8 shunt_reg, u8 bus_reg)
 {
-   return nvkm_iccsense_poll_lane(rail->i2c, rail->addr, shunt_reg, 0,
-  bus_reg, 3, rail->mohm, 10 * 4);
+   return nvkm_iccsense_poll_lane(rail->sensor->i2c, rail->sensor->addr,
+  shunt_reg, 0, bus_reg, 3, rail->mohm,
+  10 * 4);
 }
 
 static int
@@ -89,9 +89,9 @@ static int
 nvkm_iccsense_ina3221_read(struct nvkm_iccsense *iccsense,
   struct nvkm_iccsense_rail *rail)
 {
-   return nvkm_iccsense_poll_lane(rail->i2c, rail->addr,
-  1 + (rail->rail * 2), 3,
-  2 + (rail->rail * 2), 3, rail->mohm,
+   return nvkm_iccsense_poll_lane(rail->sensor->i2c, rail->sensor->addr,
+  1 + (rail->idx * 2), 3,
+  2 + (rail->idx * 2), 3, rail->mohm,
   40 * 8);
 }
 
@@ -121,9 +121,14 @@ static void *
 nvkm_iccsense_dtor(struct nvkm_subdev *subdev)
 {
struct nvkm_iccsense *iccsense = nvkm_iccsense(subdev);
-   struct nvkm_iccsense_rail *rail, *tmp;
+   struct nvkm_iccsense_sensor *sensor, *tmps;
+   struct nvkm_iccsense_rail *rail, *tmpr;
 
-   list_for_each_entry_safe(rail, tmp, >rails, head) {
+   list_for_each_entry_safe(sensor, tmps, >sensors, head) {
+   list_del(>head);
+   kfree(sensor);
+   }
+   list_for_each_entry_safe(rail, tmpr, >rails, head) {
list_del(>head);
kfree(rail);
}
@@ -131,73 +136,125 @@ nvkm_iccsense_dtor(struct nvkm_subdev *subdev)
return iccsense;
 }
 
+static struct nvkm_iccsense_sensor*
+nvkm_iccsense_create_sensor(struct nvkm_iccsense *iccsense, u8 id)
+{
+
+   struct nvkm_subdev *subdev = >subdev;
+   struct nvkm_bios *bios = subdev->device->bios;
+   struct nvkm_i2c *i2c = subdev->device->i2c;
+   struct nvbios_extdev_func extdev;
+   struct nvkm_i2c_bus *i2c_bus;
+   struct nvkm_iccsense_sensor *sensor;
+   u8 addr;
+
+   if (!i2c || !bios || nvbios_extdev_parse(bios, id, ))
+   return NULL;
+
+   if (extdev.type == 0xff)
+   return NULL;
+
+   if (extdev.type != NVBIOS_EXTDEV_INA209 &&
+   extdev.type != NVBIOS_EXTDEV_INA219 &&
+   extdev.type != NVBIOS_EXTDEV_INA3221) {
+   iccsense->data_valid = false;
+   nvkm_error(subdev, "Unknown sensor type %x, power reading "
+  "disabled\n", extdev.type);
+   return NULL;
+   }
+
+   if (extdev.bus)
+   i2c_bus = nvkm_i2c_bus_find(i2c, NVKM_I2C_BUS_SEC);
+   else
+   i2c_bus = nvkm_i2c_bus_find(i2c, NVKM_I2C_BUS_PRI);
+   if (!i2c_bus)
+   return NULL;
+
+   addr = extdev.addr >> 1;
+   if (!nvkm_iccsense_validate_device(_bus->i2c, addr,
+

Re: [Nouveau] [PATCH 4/4] iccsense: configure sensors like nvidia does

2016-03-28 Thread Martin Peres 

On 25/03/16 13:19, Karol Herbst wrote:

Signed-off-by: Karol Herbst 
---
  drm/nouveau/nvkm/subdev/iccsense/base.c | 68 +
  1 file changed, 68 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drm/nouveau/nvkm/subdev/iccsense/base.c
index b6f6222..6f3709e 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -95,6 +95,63 @@ nvkm_iccsense_ina3221_read(struct nvkm_iccsense *iccsense,
   40 * 8);
  }
  
+static void

+nvkm_iccsense_ina2x9_config(struct nvkm_iccsense *iccsense,
+   struct nvkm_iccsense_sensor *sensor)


Maybe calling the sensor ina209 and using it also for 219 would be less 
confusing, especially if a 229 is later released by TI.

+{
+   struct nvkm_subdev *subdev = >subdev;
+   /* configuration:
+* 0x0007: 0x0007 shunt and bus continous
+* 0x0078: 0x0078 128 samples shunt
+* 0x0780: 0x0780 128 samples bus
+* 0x1800: 0x +-40 mV shunt range
+* 0x2000: 0x 16V FSR
+ */
+   u16 value = 0x07ff;
+   nvkm_debug(subdev, "config for sensor id %i: 0x%x\n", sensor->id, 
value);
+   nv_wr16i2cr(sensor->i2c, sensor->addr, 0x00, value);
+}
+
+static void
+nvkm_iccsense_ina3221_config(struct nvkm_iccsense *iccsense,
+struct nvkm_iccsense_sensor *sensor)
+{
+   struct nvkm_subdev *subdev = >subdev;
+   /* configuration:
+* 0x0007: 0x0007 shunt and bus continous
+* 0x0031: 0x 140 us conversion time shunt
+* 0x01c0: 0x 140 us conversion time bus
+* 0x0f00: 0x0f00 1024 samples
+* 0x7000: 0x?000 channels
+ */
+   u16 value = 0x0e07;
+   if (sensor->rail_mask & 0x1)
+   value |= 0x1 << 14;
+   if (sensor->rail_mask & 0x2)
+   value |= 0x1 << 13;
+   if (sensor->rail_mask & 0x4)
+   value |= 0x1 << 12;
+   nvkm_debug(subdev, "config for sensor id %i: 0x%x\n", sensor->id, 
value);
+   nv_wr16i2cr(sensor->i2c, sensor->addr, 0x00, value);
+}
+
+static void
+nvkm_iccsense_sensor_config(struct nvkm_iccsense *iccsense,
+   struct nvkm_iccsense_sensor *sensor)
+{
+   switch (sensor->type) {
+   case NVBIOS_EXTDEV_INA209:
+   case NVBIOS_EXTDEV_INA219:
+   nvkm_iccsense_ina2x9_config(iccsense, sensor);
+   break;
+   case NVBIOS_EXTDEV_INA3221:
+   nvkm_iccsense_ina3221_config(iccsense, sensor);
+   break;
+   default:
+   break;
+   }
+}
+
  int
  nvkm_iccsense_read_all(struct nvkm_iccsense *iccsense)
  {
@@ -257,8 +314,19 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
return 0;
  }
  
+static int

+nvkm_iccsense_init(struct nvkm_subdev *subdev)
+{
+   struct nvkm_iccsense *iccsense = nvkm_iccsense(subdev);
+   struct nvkm_iccsense_sensor *sensor;
+   list_for_each_entry(sensor, >sensors, head)
+   nvkm_iccsense_sensor_config(iccsense, sensor);
+   return 0;
+}
+
  struct nvkm_subdev_func iccsense_func = {
.oneinit = nvkm_iccsense_oneinit,
+   .init = nvkm_iccsense_init,
.dtor = nvkm_iccsense_dtor,
  };
  


Looks like a good cleanup and improvement to me!

With the free-ing the lists fixed and maybe the change in name for the 
ina2x9, this is:

Martin Peres 
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH 3/4] iccsense: split sensor into own struct

2016-03-28 Thread Martin Peres 

On 25/03/16 13:19, Karol Herbst wrote:

Signed-off-by: Karol Herbst 
---
  drm/nouveau/include/nvkm/subdev/iccsense.h |   1 +
  drm/nouveau/nvkm/subdev/iccsense/base.c| 141 -
  drm/nouveau/nvkm/subdev/iccsense/priv.h|  15 ++-
  3 files changed, 112 insertions(+), 45 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/iccsense.h 
b/drm/nouveau/include/nvkm/subdev/iccsense.h
index a4c0da0..3c2ddd9 100644
--- a/drm/nouveau/include/nvkm/subdev/iccsense.h
+++ b/drm/nouveau/include/nvkm/subdev/iccsense.h
@@ -6,6 +6,7 @@
  struct nvkm_iccsense {
struct nvkm_subdev subdev;
bool data_valid;
+   struct list_head sensors;
struct list_head rails;
  };
  
diff --git a/drm/nouveau/nvkm/subdev/iccsense/base.c b/drm/nouveau/nvkm/subdev/iccsense/base.c

index 6fde68d..b6f6222 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -30,15 +30,14 @@
  
  static bool

  nvkm_iccsense_validate_device(struct i2c_adapter *i2c, u8 addr,
- enum nvbios_extdev_type type, u8 rail)
+ enum nvbios_extdev_type type)
  {
switch (type) {
case NVBIOS_EXTDEV_INA209:
case NVBIOS_EXTDEV_INA219:
-   return rail == 0 && nv_rd16i2cr(i2c, addr, 0x0) >= 0;
+   return nv_rd16i2cr(i2c, addr, 0x0) >= 0;
case NVBIOS_EXTDEV_INA3221:
-   return rail <= 3 &&
-  nv_rd16i2cr(i2c, addr, 0xff) == 0x3220 &&
+   return nv_rd16i2cr(i2c, addr, 0xff) == 0x3220 &&
   nv_rd16i2cr(i2c, addr, 0xfe) == 0x5449;
default:
return false;
@@ -67,8 +66,9 @@ nvkm_iccsense_ina2x9_read(struct nvkm_iccsense *iccsense,
struct nvkm_iccsense_rail *rail,
  u8 shunt_reg, u8 bus_reg)
  {
-   return nvkm_iccsense_poll_lane(rail->i2c, rail->addr, shunt_reg, 0,
-  bus_reg, 3, rail->mohm, 10 * 4);
+   return nvkm_iccsense_poll_lane(rail->sensor->i2c, rail->sensor->addr,
+  shunt_reg, 0, bus_reg, 3, rail->mohm,
+  10 * 4);
  }
  
  static int

@@ -89,9 +89,9 @@ static int
  nvkm_iccsense_ina3221_read(struct nvkm_iccsense *iccsense,
   struct nvkm_iccsense_rail *rail)
  {
-   return nvkm_iccsense_poll_lane(rail->i2c, rail->addr,
-  1 + (rail->rail * 2), 3,
-  2 + (rail->rail * 2), 3, rail->mohm,
+   return nvkm_iccsense_poll_lane(rail->sensor->i2c, rail->sensor->addr,
+  1 + (rail->idx * 2), 3,
+  2 + (rail->idx * 2), 3, rail->mohm,
   40 * 8);
  }
  
@@ -121,81 +121,137 @@ static void *

  nvkm_iccsense_dtor(struct nvkm_subdev *subdev)
  {
struct nvkm_iccsense *iccsense = nvkm_iccsense(subdev);
-   struct nvkm_iccsense_rail *rail, *tmp;
+   struct nvkm_iccsense_sensor *sensor, *tmps;
+   struct nvkm_iccsense_rail *rail, *tmpr;
  
-	list_for_each_entry_safe(rail, tmp, >rails, head)

+   list_for_each_entry_safe(sensor, tmps, >sensors, head)
+   kfree(sensor);
+   list_for_each_entry_safe(rail, tmpr, >rails, head)
kfree(rail);


Same problem as the last patch.

  
  	return iccsense;

  }
  
+static struct nvkm_iccsense_sensor*

+nvkm_iccsense_create_sensor(struct nvkm_iccsense *iccsense, u8 id)
+{
+
+   struct nvkm_subdev *subdev = >subdev;
+   struct nvkm_bios *bios = subdev->device->bios;
+   struct nvkm_i2c *i2c = subdev->device->i2c;
+   struct nvbios_extdev_func extdev;
+   struct nvkm_i2c_bus *i2c_bus;
+   struct nvkm_iccsense_sensor *sensor;
+   u8 addr;
+
+   if (!i2c || !bios || nvbios_extdev_parse(bios, id, ))
+   return NULL;
+
+   if (extdev.type == 0xff)
+   return NULL;
+
+   if (extdev.type != NVBIOS_EXTDEV_INA209 &&
+   extdev.type != NVBIOS_EXTDEV_INA219 &&
+   extdev.type != NVBIOS_EXTDEV_INA3221) {
+   iccsense->data_valid = false;
+   nvkm_error(subdev, "found unknown sensor type %x, "
+  "power reading might be invalid\n",
+  extdev.type);


You do not create the sensor if it is unknown, so why do you say the 
report might be invalid?


Please change it to "Unknown sensor type %x".


+   return NULL;
+   }
+
+   if (extdev.bus)
+   i2c_bus = nvkm_i2c_bus_find(i2c, NVKM_I2C_BUS_SEC);
+   else
+   i2c_bus = nvkm_i2c_bus_find(i2c, NVKM_I2C_BUS_PRI);
+   if (!i2c_bus)
+   return NULL;
+
+   addr = extdev.addr >> 1;
+   if (!nvkm_iccsense_validate_device(_bus->i2c, addr,
+

Re: [Nouveau] [PATCH 2/4] iccsense: convert to linked list

2016-03-28 Thread Martin Peres 

On 25/03/16 13:19, Karol Herbst wrote:

Signed-off-by: Karol Herbst 
---
  drm/nouveau/include/nvkm/subdev/iccsense.h |  4 +---
  drm/nouveau/nouveau_hwmon.c|  2 +-
  drm/nouveau/nvkm/subdev/iccsense/base.c| 32 +-
  drm/nouveau/nvkm/subdev/iccsense/priv.h|  1 +
  4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/iccsense.h 
b/drm/nouveau/include/nvkm/subdev/iccsense.h
index c3defcd..a4c0da0 100644
--- a/drm/nouveau/include/nvkm/subdev/iccsense.h
+++ b/drm/nouveau/include/nvkm/subdev/iccsense.h
@@ -3,12 +3,10 @@
  
  #include 
  
-struct nkvm_iccsense_rail;

  struct nvkm_iccsense {
struct nvkm_subdev subdev;
-   u8 rail_count;
bool data_valid;
-   struct nvkm_iccsense_rail *rails;
+   struct list_head rails;
  };
  
  int gf100_iccsense_new(struct nvkm_device *, int index, struct nvkm_iccsense **);

diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c
index 67edd2f..74f237b 100644
--- a/drm/nouveau/nouveau_hwmon.c
+++ b/drm/nouveau/nouveau_hwmon.c
@@ -689,7 +689,7 @@ nouveau_hwmon_init(struct drm_device *dev)
goto error;
}
  
-	if (iccsense && iccsense->data_valid && iccsense->rail_count) {

+   if (iccsense && iccsense->data_valid && !list_empty(>rails)) {
ret = sysfs_create_group(_dev->kobj,
 _power_attrgroup);
if (ret)
diff --git a/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drm/nouveau/nvkm/subdev/iccsense/base.c
index bf1b94e..6fde68d 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -98,25 +98,21 @@ nvkm_iccsense_ina3221_read(struct nvkm_iccsense *iccsense,
  int
  nvkm_iccsense_read_all(struct nvkm_iccsense *iccsense)
  {
-   int result = 0, i;
+   int result = 0;
+   struct nvkm_iccsense_rail *rail;
  
  	if (!iccsense)

return -EINVAL;
  
-	if (iccsense->rail_count == 0)

-   return -ENODEV;
-
-   for (i = 0; i < iccsense->rail_count; ++i) {
+   list_for_each_entry(rail, >rails, head) {
int res;
-   struct nvkm_iccsense_rail *rail = >rails[i];
if (!rail->read)
return -ENODEV;
  
  		res = rail->read(iccsense, rail);

-   if (res >= 0)
-   result += res;
-   else
+   if (res < 0)
return res;
+   result += res;
}
return result;
  }
@@ -125,9 +121,10 @@ static void *
  nvkm_iccsense_dtor(struct nvkm_subdev *subdev)
  {
struct nvkm_iccsense *iccsense = nvkm_iccsense(subdev);
+   struct nvkm_iccsense_rail *rail, *tmp;
  
-	if (iccsense->rails)

-   kfree(iccsense->rails);
+   list_for_each_entry_safe(rail, tmp, >rails, head)
+   kfree(rail);
The rails list is filled with invalid pointers at this point. Please add 
list_del(rail); before kfree(rail);


It has the added benefit of adding poisonous pointers that will show any 
access after freeing.
  
  	return iccsense;

  }
@@ -145,11 +142,6 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
|| !stbl.nr_entry)
return 0;
  
-	iccsense->rails = kmalloc(sizeof(*iccsense->rails) * stbl.nr_entry,

- GFP_KERNEL);
-   if (!iccsense->rails)
-   return -ENOMEM;
-
iccsense->data_valid = true;
for (i = 0; i < stbl.nr_entry; ++i) {
struct pwr_rail_t *r = [i];
@@ -184,7 +176,10 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
continue;
}
  
-		rail = >rails[iccsense->rail_count];

+   rail = kmalloc(sizeof(*rail), GFP_KERNEL);
+   if (!rail)
+   return -ENOMEM;
+
switch (extdev.type) {
case NVBIOS_EXTDEV_INA209:
rail->read = nvkm_iccsense_ina209_read;
@@ -201,7 +196,7 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
rail->rail = r->rail;
rail->mohm = r->resistor_mohm;
rail->i2c = _bus->i2c;
-   ++iccsense->rail_count;
+   list_add_tail(>head, >rails);
}
return 0;
  }
@@ -224,6 +219,7 @@ nvkm_iccsense_new_(struct nvkm_device *device, int index,
  {
if (!(*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL)))
return -ENOMEM;
+   INIT_LIST_HEAD(&(*iccsense)->rails);
nvkm_iccsense_ctor(device, index, *iccsense);
return 0;
  }
diff --git a/drm/nouveau/nvkm/subdev/iccsense/priv.h 
b/drm/nouveau/nvkm/subdev/iccsense/priv.h
index ed398b8..e479128 100644
--- a/drm/nouveau/nvkm/subdev/iccsense/priv.h
+++ b/drm/nouveau/nvkm/subdev/iccsense/priv.h
@@ -4,6 +4,7 @@
  #include 
  
  struct nvkm_iccsense_rail {

+

[Nouveau] [Bug 94728] Nouveau doesn't work with Nvidia GTX 960

2016-03-28 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=94728

Martin Peres  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |NOTABUG

--- Comment #2 from Martin Peres  ---
Yes, you need a more up-to-date kernel and mesa version. That is to say Linux
4.6 and mesa 11.2 which are not released yet. You will also need to update
linux-firmware.

Maybe rawhide would work for you?

-- 
You are receiving this mail because:
You are the assignee for the bug.___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [Bug 94728] New: Nouveau doesn't work with Nvidia GTX 960

2016-03-28 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=94728

Bug ID: 94728
   Summary: Nouveau doesn't work with Nvidia GTX 960
   Product: xorg
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Driver/nouveau
  Assignee: nouveau@lists.freedesktop.org
  Reporter: neilmed...@gmail.com
QA Contact: xorg-t...@lists.x.org

The nouveau driver doesn't seem to work with my GTX 960 card - I'm getting
"Unknown chipset: NV126" message in the logs. Looks like it's then falling back
to mesa (gnome-control-center shows graphics as Gallium 0.4 on llvmpipe (LLVM
3.7, 256 bits)). 

I'm on the latest version Fedora 23, although I haven't seen this work since I
got the card. Output of journalctl -b _COMM=gdm-x-session attached. Let me know
if you need anymore info - many thanks. 


(II) [drm] nouveau interface version: 1.3.1
(EE) Unknown chipset: NV126
(II) [drm] nouveau interface version: 1.3.1
(EE) Unknown chipset: NV126
(II) [drm] nouveau interface version: 1.3.1
(EE) Unknown chipset: NV126
(EE) [drm] Failed to open DRM device for pci::01:00.0: -13
(EE) [drm] Failed to open DRM device for pci::01:00.0: -13

-- 
You are receiving this mail because:
You are the assignee for the bug.___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau