LDMX-Software / pflib

Library and tool for interacting with polarfires.
https://ldmx-software.github.io/pflib/
4 stars 0 forks source link

Bit 11 of CALIB_DAC seems unset #63

Closed tomeichlersmith closed 2 years ago

tomeichlersmith commented 2 years ago

Describe the bug Referencd in elog note and brought to my attention by @jmmans . Can see the effects in a 4/10 elog post. Great linearity up to a the bit-11 threshold, then drop down and start linearity up from 0 again.

Expected behavior We don't want a jump discontinuity...

Already Done I have checked the compiler code and saw basic validation that it seems to not be the issue:

# calib_dac.yaml
reference_voltage_0:
  calib_dac: 0b100000000000
  intctest: 1
  extctest: 0

Then running pfcompile --no-defaults calib_dac.yaml produces

# This register settings file was generated by pfcompile
#    The columns are: page, register, value (in hex)
296,6,0x00
296,7,0x48

which is the expected result.

tomeichlersmith commented 2 years ago

The short test above validates that the page-internal look-up table is good and aligned with the manual-defined behavior.

https://github.com/LDMX-Software/pflib/blob/b14cc1cb05c6eb2375fc23de70862d2ec53e223d/src/pflib/Compile.cxx#L109-L124

No other page is given the same page number as either reference voltage page in the full parameter LUT.

https://github.com/LDMX-Software/pflib/blob/b14cc1cb05c6eb2375fc23de70862d2ec53e223d/src/pflib/Compile.cxx#L288

I am comfortable concluding that the compilation is not the issue. We could still have a software issue where somehow register 7 of the reference voltage page is not being set correctly. Perhaps in the ROC::applyParameters function:

https://github.com/LDMX-Software/pflib/blob/b14cc1cb05c6eb2375fc23de70862d2ec53e223d/src/pflib/ROC.cxx#L70-L97

tomeichlersmith commented 2 years ago

Adding some printouts to ROC::applyParameters and using POKE_PARAM in the ROC menu seems to do the correct behavior:

 > POKE_PARAM
Page:  [Global_Analog_0] reference_voltage_0
Parameter: calib_dac
New value: 2048
Touched: 
  296 6 80
  296 7 0
Setting: 
  296 6 0
  296 7 8
 > PAGE 
Which page?  [0] 296
Length? [8] 
00 : fc
01 : 00
02 : 6c
03 : 1c
04 : 38
05 : 28
06 : 00
07 : 08

where my printouts look like

diff --git a/src/pflib/ROC.cxx b/src/pflib/ROC.cxx
index f51a954a..caa12283 100644
--- a/src/pflib/ROC.cxx
+++ b/src/pflib/ROC.cxx
@@ -71,15 +71,18 @@ void ROC::applyParameters(const std::map<std::string,std::map<std::string,int>>&
   // 1. get registers YAML file contains by compiling without defaults
   auto touched_registers = compile(parameters);
   // 2. get the current register values on the chip
+  std::cout << "Touched: " << std::endl;
   std::map<int,std::map<int,uint8_t>> chip_reg;
   for (auto& page : touched_registers) {
     int page_id = page.first;
     std::vector<uint8_t> on_chip_reg_values = this->readPage(page_id, 16);
     for (int i{0}; i < 16; i++) {
       // skip un-touched registers
       if (page.second.find(i) == page.second.end()) 
         continue;
       chip_reg[page_id][i] = on_chip_reg_values.at(i);
+      std::cout << "  " << page_id << " " << i << " " << std::hex << int(on_chip_reg_values.at(i)) << std::dec << std::endl;
     }
   }
   // 3. compile this parameter onto those register values
@@ -92,6 +95,13 @@ void ROC::applyParameters(const std::map<std::string,std::map<std::string,int>>&
       compile(page_name,upper_cp(param.first),param.second,chip_reg);
     }
   }
+  std::cout << "Setting: " << std::endl;
+  for (auto& page : chip_reg) {
+    for (auto& reg : page.second) {
+      std::cout << "  " << page.first << " " << reg.first << " " 
+        << std::hex << int(reg.second) << std::dec << std::endl;
+    }
+  }
   // 4. put these updated values onto the chip
   this->setRegisters(chip_reg);
 }

onto checking the SCANCHARGE command...

tomeichlersmith commented 2 years ago

SCANCHARGE is a little weird...

 > SCANCHARGE 
CALIB_DAC valid range is 0...4095
Smallest value of CALIB_DAC? [10] 2047
Largest value of CALIB_DAC? [500] 2049
Use HighRange?  (Y/N)  [N] 
Number of steps? [10] 3
Events per step? [100] 
Filename :   [scan_CALIB_DAC_20220411_093800.csv] 
 Clearing charge injection on all channels (ground-state)...
 Enabling IntCTest...
 Scan CALIB_DAC=2047...
ref0 : 47ff
ref1 : 47ff
 Scan CALIB_DAC=2048...
ref0 : 47ff
ref1 : 47ff
 Scan CALIB_DAC=2049...
ref0 : 48 0
ref1 : 48 0
 Diabling IntCTest...

where my printouts are

diff --git a/tool/pftool.cc b/tool/pftool.cc
index c4da64b6..7a89aa5a 100644
--- a/tool/pftool.cc
+++ b/tool/pftool.cc
@@ -1152,6 +1152,12 @@ static void tasks( const std::string& cmd, PolarfireTarget* pft ) {
       int value=low_value+step*(high_value-low_value)/std::max(1,steps-1);

       printf(" Scan %s=%d...\n",valuename.c_str(),value);
+      std::vector<uint8_t> ref0 = pft->hcal.roc(0).readPage(296,8);
+      std::vector<uint8_t> ref1 = pft->hcal.roc(0).readPage( 40,8);
+      std::cout << std::hex
+        << "ref0 : " << std::setw(2) << int(ref0.at(7)) << std::setw(2) << int(ref0.at(6)) << "\n"
+        << "ref1 : " << std::setw(2) << int(ref1.at(7)) << std::setw(2) << int(ref1.at(6))
+        << std::endl;

       for (int ilink=0; ilink<pft->hcal.elinks().nlinks(); ilink++) {
         if (!pft->hcal.elinks().isActive(ilink)) continue;
tomeichlersmith commented 2 years ago

Nevermind, I had my printouts too early in the function, now it is

 > SCANCHARGE 
CALIB_DAC valid range is 0...4095
Smallest value of CALIB_DAC? [10] 2047
Largest value of CALIB_DAC? [500] 2050
Use HighRange?  (Y/N)  [N] 
Number of steps? [10] 4
Events per step? [100] 1
Filename :   [scan_CALIB_DAC_20220411_095032.csv] 
 Clearing charge injection on all channels (ground-state)...
 Enabling IntCTest...
 Scan CALIB_DAC=2047...
ref0 : 47ff
ref1 : 47ff
 Scan CALIB_DAC=2048...
ref0 : 48 0
ref1 : 48 0
 Scan CALIB_DAC=2049...
ref0 : 48 1
ref1 : 48 1
 Scan CALIB_DAC=2050...
ref0 : 48 2
ref1 : 48 2
 Diabling IntCTest...

where my printouts are

diff --git a/tool/pftool.cc b/tool/pftool.cc
index c4da64b6..93955d4d 100644
--- a/tool/pftool.cc
+++ b/tool/pftool.cc
@@ -1162,6 +1162,12 @@ static void tasks( const std::string& cmd, PolarfireTarget* pft ) {
         // set the value
         pft->hcal.roc(iroc).applyParameter(pagename, valuename, value);
       }
+      std::vector<uint8_t> ref0 = pft->hcal.roc(0).readPage(296,8);
+      std::vector<uint8_t> ref1 = pft->hcal.roc(0).readPage( 40,8);
+      std::cout << std::hex
+        << "ref0 : " << std::setw(2) << int(ref0.at(7)) << std::setw(2) << int(ref0.at(6)) << "\n"
+        << "ref1 : " << std::setw(2) << int(ref1.at(7)) << std::setw(2) << int(ref1.at(6))
+        << std::endl;
       ////////////////////////////////////////////////////////////

       pft->prepareNewRun();

This is printing registers 7 and 6 next to each other from the reference voltage page of the hgc roc. The highest hex digit should be 4 signaling that the internal charge injection is selected. The lower three hex digis are supposed to set the 12-bit calib_dac value. You see in the printouts that the calib_dac iterates across the 11th bit barrier correctly.

tomeichlersmith commented 2 years ago

The CALIB DAC if v2 HGC ROC is only 11 bits. It will be 12 bits in v3.