Re: [Nouveau] Addressing the problem of noisy GPUs under Nouveau

2017-11-14 Thread John Hubbard
Hi Martin,

This is just a quick ACK. I've started an internal email thread and
we'll see if we can get back to you soon.

Yes, our thermal and fan control definitely changes a lot which
the various chip architectures. I'm continually impressed by how much
the SW+HW has been able to improve performance per watt, year after
year, but of course the side effect is a very complex system, as 
you are seeing. But even so, let's see if there is any sort of
simpler approximation that would work for you here...no promises,
because I'm about to be humbled when the thermal experts respond. :)


thanks,
John Hubbard
NVIDIA

On 11/12/2017 06:29 PM, Martin Peres wrote:
> Hello,
> 
> Some users have been complaining for years about their GPU sounding like
> a jet engine at take off. Last year, I finally laid my hand on one of
> these GPUs and have been trying to fix this issue on and off since then.
> 
> After failing to find anything in the HW, I figured out that the duty
> cycle set by nvidia's proprietary driver would be way under the expected
> value. By randomly changing values in the unknown tables of the vbios, I
> found out that there is a fan calibration table at the offset 0x18 in
> the BIT P table (version 2).
> 
> In this table, I identified 2 major 16 bits parameters at offset 0xa and
> 0xc[2]. The first one, I named pwm_max, while naming the latter
> pwm_offset. As expected, these parameters look like a mapping function
> of the form aX + b. However, after gathering more samples, I found out
> that the output was not continuous when linearly increasing pwm_offset
> [1]. Even more funnily, the period of this square function is linear
> with the frequency used for the fan's PWN.
> 
> I tried reverse engineering the formula to describe this function, but
> failed to find a version that would work perfectly for all PWM
> frequency. This is the closest I have got to[3], and I basically stopped
> there about a year ago because I could not figure it out and got
> frustrated :s.
> 
> I started again on this project 2 weeks ago, with the intent of finding
> a good-enough solution for nouveau, and modelling the rest of the
> equation that that would allow me to compute what duty I should set for
> every wanted fan speed (%). I again mostly succeeded... but it would
> seem that the interpretation of the table depends on the generation of
> chipset (Tesla behaves one way, Fermi+ behaves another way). Also, the
> proprietary is not consistent for rules such as what to do when the
> computed duty value is going to be lower than 0 or not (sometimes we
> clamp it to 0, some times we set it to the same value as the divider,
> some times we set it to a slightly lower value than the divider).
> 
> I have been trying to cover all edge cases by generating a randomized
> set of values for the PWM frequency, pwm_max, and pwm_offset values,
> flashed the vbios, and iterate from 0% to 100% fan speed while dumping
> the values set by your driver. Using half a million sample points (which
> took a week to acquire), my model computes 97% of the values correctly
> (ignoring off by ones), while the remaining 3% are worryingly off (by up
> to 100%)... It is clear that the code is not trivial and is full of
> branching, which makes clean-room reverse engineering a chore.
> 
> As a final attempt to make a somewhat complete solution, I tried this
> weekend to make a "safe" model that would still make the GPUs quiet. I
> managed to improve the pass rate from 97 to 99.6%, but the remaining
> failures conflict with my previous findings, which are also way more
> prevalent. In the end, the only completely-safe way of driving the fan
> is the current behaviour of nouveau...
> 
> At this point, I am ready to throw in the towel and hardcode parameters
> in nouveau to address the problem of the loudest GPUs, but this is of
> course suboptimal. This is why I am asking for your help. Would you have
> some documentation about this fan calibration table that could help me
> here? Code would be even more appreciated.
> 
> Thanks a lot in advance,
> Martin
> 
> PS: here is most of the code you may want to see:
> http://fs.mupuf.org/nvidia/fan_calib/
> 
> [1] http://fs.mupuf.org/nvidia/fan_calib/pwm_offset.png
> [2] https://github.com/envytools/envytools/blob/master/nvbios/power.c#L333
> [3] https://github.com/envytools/envytools/blob/master/nvbios/power.c#L298
> 
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [RFC PATCH] nouveau/compiler: Allow to omit line numbers when printing instructions

2017-11-14 Thread Karol Herbst
I think it is better to put this behind an environmental variable,
because that way it can also be used without having to dump the TGSI
first and I don't see a good reason why not to.

On Tue, Nov 14, 2017 at 4:01 PM, Tobias Klausmann
 wrote:
> This comes in handy when checking "NV50_PROG_DEBUG=1" outputs with diff!
>
> Signed-off-by: Tobias Klausmann 
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir.cpp|  6 +++---
>  src/gallium/drivers/nouveau/codegen/nv50_ir.h  |  2 +-
>  src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h   |  1 +
>  src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp  | 12 
>  src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp |  2 +-
>  src/gallium/drivers/nouveau/nouveau_compiler.c |  8 ++--
>  src/gallium/drivers/nouveau/nv50/nv50_program.c|  1 +
>  src/gallium/drivers/nouveau/nvc0/nvc0_program.c|  1 +
>  8 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
> index e9363101bf..4bf6c73837 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
> @@ -1249,7 +1249,7 @@ nv50_ir_generate_code(struct nv50_ir_prog_info *info)
> if (ret < 0)
>goto out;
> if (prog->dbgFlags & NV50_IR_DEBUG_VERBOSE)
> -  prog->print();
> +  prog->print(info->omitLineNum);
>
> targ->parseDriverInfo(info);
> prog->getTarget()->runLegalizePass(prog, nv50_ir::CG_STAGE_PRE_SSA);
> @@ -1257,13 +1257,13 @@ nv50_ir_generate_code(struct nv50_ir_prog_info *info)
> prog->convertToSSA();
>
> if (prog->dbgFlags & NV50_IR_DEBUG_VERBOSE)
> -  prog->print();
> +  prog->print(info->omitLineNum);
>
> prog->optimizeSSA(info->optLevel);
> prog->getTarget()->runLegalizePass(prog, nv50_ir::CG_STAGE_SSA);
>
> if (prog->dbgFlags & NV50_IR_DEBUG_BASIC)
> -  prog->print();
> +  prog->print(info->omitLineNum);
>
> if (!prog->registerAllocation()) {
>ret = -4;
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.h 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
> index f2ce16d882..a3c7fd2f94 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
> @@ -1249,7 +1249,7 @@ public:
> Program(Type type, Target *targ);
> ~Program();
>
> -   void print();
> +   void print(bool omitLineNum);
>
> Type getType() const { return progType; }
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h
> index ffd53c9cd3..604a22ba89 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h
> @@ -82,6 +82,7 @@ struct nv50_ir_prog_info
>
> uint8_t optLevel; /* optimization level (0 to 3) */
> uint8_t dbgFlags;
> +   bool omitLineNum;
>
> struct {
>int16_t maxGPR; /* may be -1 if none used */
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp
> index f5253b3745..a42fb44940 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp
> @@ -689,7 +689,7 @@ void Instruction::print() const
>  class PrintPass : public Pass
>  {
>  public:
> -   PrintPass() : serial(0) { }
> +   PrintPass(bool omitLineNum = false) : serial(0), omit_serial(omitLineNum) 
> { }
>
> virtual bool visit(Function *);
> virtual bool visit(BasicBlock *);
> @@ -697,6 +697,7 @@ public:
>
>  private:
> int serial;
> +   bool omit_serial;
>  };
>
>  bool
> @@ -760,7 +761,10 @@ PrintPass::visit(BasicBlock *bb)
>  bool
>  PrintPass::visit(Instruction *insn)
>  {
> -   INFO("%3i: ", serial++);
> +   if (omit_serial)
> +  INFO("   ");
> +   else
> +  INFO("%3i: ", serial++);
> insn->print();
> return true;
>  }
> @@ -773,9 +777,9 @@ Function::print()
>  }
>
>  void
> -Program::print()
> +Program::print(bool omitLineNum)
>  {
> -   PrintPass pass;
> +   PrintPass pass(omitLineNum);
> init_colours();
> pass.run(this, true, false);
>  }
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp
> index 298e7c6ef9..96ad70d28a 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp
> @@ -371,7 +371,7 @@ Program::emitBinary(struct nv50_ir_prog_info *info)
> emit->prepareEmission(this);
>
> if (dbgFlags & NV50_IR_DEBUG_BASIC)
> -  this->print();
> +  this->print(info->omitLineNum);
>
> if (!binSize) {
>code = NULL;
> diff --git a/src/gallium/drivers/nouveau/nouveau_compiler.c 
> 

[Nouveau] [RFC PATCH] nouveau/compiler: Allow to omit line numbers when printing instructions

2017-11-14 Thread Tobias Klausmann
This comes in handy when checking "NV50_PROG_DEBUG=1" outputs with diff!

Signed-off-by: Tobias Klausmann 
---
 src/gallium/drivers/nouveau/codegen/nv50_ir.cpp|  6 +++---
 src/gallium/drivers/nouveau/codegen/nv50_ir.h  |  2 +-
 src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h   |  1 +
 src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp  | 12 
 src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp |  2 +-
 src/gallium/drivers/nouveau/nouveau_compiler.c |  8 ++--
 src/gallium/drivers/nouveau/nv50/nv50_program.c|  1 +
 src/gallium/drivers/nouveau/nvc0/nvc0_program.c|  1 +
 8 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
index e9363101bf..4bf6c73837 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
@@ -1249,7 +1249,7 @@ nv50_ir_generate_code(struct nv50_ir_prog_info *info)
if (ret < 0)
   goto out;
if (prog->dbgFlags & NV50_IR_DEBUG_VERBOSE)
-  prog->print();
+  prog->print(info->omitLineNum);
 
targ->parseDriverInfo(info);
prog->getTarget()->runLegalizePass(prog, nv50_ir::CG_STAGE_PRE_SSA);
@@ -1257,13 +1257,13 @@ nv50_ir_generate_code(struct nv50_ir_prog_info *info)
prog->convertToSSA();
 
if (prog->dbgFlags & NV50_IR_DEBUG_VERBOSE)
-  prog->print();
+  prog->print(info->omitLineNum);
 
prog->optimizeSSA(info->optLevel);
prog->getTarget()->runLegalizePass(prog, nv50_ir::CG_STAGE_SSA);
 
if (prog->dbgFlags & NV50_IR_DEBUG_BASIC)
-  prog->print();
+  prog->print(info->omitLineNum);
 
if (!prog->registerAllocation()) {
   ret = -4;
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.h 
b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
index f2ce16d882..a3c7fd2f94 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
@@ -1249,7 +1249,7 @@ public:
Program(Type type, Target *targ);
~Program();
 
-   void print();
+   void print(bool omitLineNum);
 
Type getType() const { return progType; }
 
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h
index ffd53c9cd3..604a22ba89 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h
@@ -82,6 +82,7 @@ struct nv50_ir_prog_info
 
uint8_t optLevel; /* optimization level (0 to 3) */
uint8_t dbgFlags;
+   bool omitLineNum;
 
struct {
   int16_t maxGPR; /* may be -1 if none used */
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp
index f5253b3745..a42fb44940 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_print.cpp
@@ -689,7 +689,7 @@ void Instruction::print() const
 class PrintPass : public Pass
 {
 public:
-   PrintPass() : serial(0) { }
+   PrintPass(bool omitLineNum = false) : serial(0), omit_serial(omitLineNum) { 
}
 
virtual bool visit(Function *);
virtual bool visit(BasicBlock *);
@@ -697,6 +697,7 @@ public:
 
 private:
int serial;
+   bool omit_serial;
 };
 
 bool
@@ -760,7 +761,10 @@ PrintPass::visit(BasicBlock *bb)
 bool
 PrintPass::visit(Instruction *insn)
 {
-   INFO("%3i: ", serial++);
+   if (omit_serial)
+  INFO("   ");
+   else
+  INFO("%3i: ", serial++);
insn->print();
return true;
 }
@@ -773,9 +777,9 @@ Function::print()
 }
 
 void
-Program::print()
+Program::print(bool omitLineNum)
 {
-   PrintPass pass;
+   PrintPass pass(omitLineNum);
init_colours();
pass.run(this, true, false);
 }
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp
index 298e7c6ef9..96ad70d28a 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.cpp
@@ -371,7 +371,7 @@ Program::emitBinary(struct nv50_ir_prog_info *info)
emit->prepareEmission(this);
 
if (dbgFlags & NV50_IR_DEBUG_BASIC)
-  this->print();
+  this->print(info->omitLineNum);
 
if (!binSize) {
   code = NULL;
diff --git a/src/gallium/drivers/nouveau/nouveau_compiler.c 
b/src/gallium/drivers/nouveau/nouveau_compiler.c
index 3151a6f420..ed68031383 100644
--- a/src/gallium/drivers/nouveau/nouveau_compiler.c
+++ b/src/gallium/drivers/nouveau/nouveau_compiler.c
@@ -103,7 +103,7 @@ dummy_assign_slots(struct nv50_ir_prog_info *info)
 
 static int
 nouveau_codegen(int chipset, int type, struct tgsi_token tokens[],
-unsigned *size, unsigned **code) {
+unsigned *size, unsigned **code, bool omitLineNum) {
struct nv50_ir_prog_info info = {0};
int ret;
 
@@ -122,6 +122,7 @@