CMOS/ADTG/Digic register investigation on ISO

Started by a1ex, January 10, 2014, 12:11:01 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

timbytheriver

@names_are_hard That is sooo helpful. :D Thanks so much for taking the time to explain it in terms I think [even] I can understand. Reading your explanation has me musing on whether what I am trying to do is above my present skill level...

Basically what I'm trying to do is code some discovered register values (from the research in this thread) that impact iso response into a preset that can be applied in conjunction with an existing crop_rec setting. So:

Shooting in say, crop_rec 3K mode > Q menu > select new preset 'X' for 'adjusted iso registers' = on or off.

At the moment we have to either make the register adjustments in iso_regs.mo or adtg_gui.mo which is fiddly, doesn't work in crop modes, and needs to be reset on power off. I'm looking for a way to code discovered values into a preset 'X'.

*** EDIT ***
Thinking maybe these could be added to the existing crop mode > Q menu. Am I right in thinking these might be added to crop_rec.c at lines (appx) 166 and 1409 ?



static int32_t  target_yres = 0;
static int32_t  delta_adtg0 = 0;
static int32_t  delta_adtg1 = 0;
static int32_t  delta_head3 = 0;
static int32_t  delta_head4 = 0;
static uint32_t cmos1_lo = 0, cmos1_hi = 0;
static uint32_t cmos2 = 0;




and



static struct menu_entry crop_rec_menu[] =
{
    {
        .name       = "Crop mode",
        .priv       = &crop_preset_index,
        .update     = crop_update,
        .depends_on = DEP_LIVEVIEW,
        .children =  (struct menu_entry[]) {
            {




?
5D3 1.1.3
5D2 2.1.2

names_are_hard

I've never used crop_rec, or any modules with ML, so I don't know how their UI works.  It would likely be many hours work for me to understand the code.  I will say you can probably test those changes in QEMU.  The register manipulation probably doesn't work in QEMU (not checked), but the UI changes should.  Get the UI looking how you want (but the buttons do nothing because you don't need to make code that attempts any register changes, just make it do nothing).  When the UI looks right, attach the UI buttons to real code that changes registers.  Try that in QEMU (even though it probably won't work), and if it looks sensible (doesn't crash at least *before* you press the buttons), try it on a real cam.

timbytheriver

Thanks names_are_hard! QEMU seems a sensible option.  :)

Before I dive into an install, does anyone know whether changes to registers, running adtg_gui.mo or iso_regs.mo can be emulated in QEMU?

Thanks.
5D3 1.1.3
5D2 2.1.2

Danne

I think you are better off experimenting straight in crop_rec. Maybe your regs doesn't even work or need to be adjusted to work.
I can't say if it's safe or not but can speak from my own testing. Whatever adtg reg I fed in there has not affected anything permanently. Always restored on restart. The only time I soft bricked my eosm was when fooling around with movie crop mode registry and bypassing explicit warnings in code(property reg). @a1ex fixed the mess nevertheless).
I can never gurantee anything. Only tell what I've been doing and if I didn't try out a thousand things in crop_rec I would never learnt how to build the presets I did on my eosm.

timbytheriver

Ok. Thanks all for good advice. Will persevere!
5D3 1.1.3
5D2 2.1.2

timbytheriver

I'm working on translating regs in adtg_gui.mo to iso_regs.mo:

I notice that in iso_regs the following regs allow a single value, e.g.


ADTG Gain 100
ADTG Preamp 2
ADTG 0xFE 3 etc.


Whereas in adtg_gui the equivalent (I think) registers allow for a choice of four values x 2 which may vary by a few figures, e.g

ADTG Gain:

ADTG2 [8882] 1083 (dec equiv...)
ADTG2 [8884] 1080
ADTG2 [8886] 1081
ADTG2 [8888] 1082

ADTG4 [8882] 1079
ADTG4 [8884] 1079
ADTG4 [8886] 1079
ADTG4 [8888] 1082

etc...

Does this mean that adtg_gui allows for a finer (per column?) adjustment of the reg values than iso_regs? Is iso_regs averaging all these values into one value?
Since I'm working on hard coding some clean iso values into a preset I'm wondering whether I should be adjusting a different value for each – or using the single value from iso_regs.

Thanks!

5D3 1.1.3
5D2 2.1.2

names_are_hard

I don't have that code so I can't check.  If you link to the lines in the repo you're using I could try and understand it.


Danne

So by looking in adtg_gui you should be able to just add the regs in crop_rec. What's the problem?

timbytheriver

Not a problem. I have already added in crop_rec.c so and they appear to be working.

My specific question was about why the adjustments in adtg_gui allow for four different values to be set per blue/green 1 (ADTG2) and red/green 2 (ADTG4) – whereas in iso_regs they seem summed into a single value. Just seeking to understand properly.  :)
5D3 1.1.3
5D2 2.1.2

Danne

Oh, I see. Go on then :).
Could you share samples, comparisons?

names_are_hard

Tim, in the comparisons you give, one is an array of known_reg structs, the other is uint32_t type (without looking very hard I suspect this will be treated as an address later on).  They're not directly comparable so it's not clear to me what you mean.  In particular I don't know what you mean by "allowing" numbers of values.

timbytheriver

Ok, so. I guess you know this already but adtg_gui and iso_regs are separate modules that allow for manipulation of the registers.

There is overlap between them. iso_regs is 5D3 only; adtg_gui is other cameras also. For a new-to-coder like myself I am confused by the different way that what appear to be the same registers are called different names in either module.  :o

I started out tweaking iso_regs only and that works just fine. Then I tried adtg_gui and discovered that when in iso_regs the adjustment say for ADTG preamp is '1', in adtg_gui this single value seems to be 'split' among several wherein each has a slightly different value! e.g


ADTG2 [8882] = 0x43b
ADTG2 [8884] = 0x438
ADTG2 [8886] = 0x439
ADTG2 [8888] = 0x43a

ADTG4 [8882] = 0x43b
etc...


I'm just seeking to understand why, and what implications this has for hardcoding a value or set-of into a movie preset! :)

Maybe I've completely misunderstood something fundamental ... just seeking to do it right!
5D3 1.1.3
5D2 2.1.2

names_are_hard

That helps me understand what you're trying to do (I didn't know what the different modules were for), but it is still not clear without the code you're referring to.  You say a "1" is "split" in a different file, but only give one example of code, and it doesn't involve a "1".  The lines you quote aren't in the files you gave links to.  If you don't know, you can click on the line numbers in Bitbucket to get a link direct to that line.  Being able to see *both* things you're referring to would help.

I'm guessing you don't have much experience with C?  I think you might be getting confused with the different ways you deal with a struct and an array of structs?


timbytheriver

Quote from: names_are_hard on December 03, 2019, 08:55:00 PM
I'm guessing you don't have much experience with C?

How did you possibly guess that?  ;)

Yep, I'm a C-noob. I'm coming at this project from a 'visual' angle. Having seen visual benefits to this iso research I'm seeking to skill-up to be able to hard code the tweaks I'm able to do in iso_regs or adtg_gui and add them to a crop_rec (3K, UHD etc) preset.

At present if you're running iso_regs or adtg_gui, they won't allow for crop_rec extended resolutions to work at the same time.

Thanks again for your help! :) What camera are you running btw?
5D3 1.1.3
5D2 2.1.2

timbytheriver

Can someone clever help out with what I think is a maths problem please?  :o

I'm taking a single register value set in iso_regs.mo which is

ADTG Preamp = 2

and trying to duplicate it exactly in adtg_gui.mo. However, the corresponding preamp registers in adtg_gui are split up into several values that seem to be: blue and green channel 1, and red and green channel 2

Below are the default values:


/* ADTG2 adjusts blue and green channel 1 ? */
ADTG2[8] = 96
ADTG2[9] = 97
ADTG2[a] = 98
ADTG2[b] = 96

/* ADTG4 adjusts red and green channel 2 ? */
ADTG4[8] = 99
ADTG4[9] = 101
ADTG4[a] = 99
ADTG4[b] = 99


If I set all these values at an identical value e.g. = 2 to match the value in iso_regs I get the desired iso gain reduction but with strange yellow blotches / artefacts, and it does something weird to the demosaicing. I have spent many hours of setting/shooting/card pulls trying random combinations of values or ratios between figures – but without losing the artefacts. Maybe there's a quicker way to address this? :'(

Artefacts



The maths I'm looking for is the formula which reduces or sums all the values in the adtg_gui lines into the correct ratios to equal the '2' of iso_regs single value!

Does this makes sense?  :-\


5D3 1.1.3
5D2 2.1.2

names_are_hard

Can you link to the exact lines of code in both files?  If so I'll take a look.  This question is much more clearly asked, you're explaining your assumptions better.

It's totally normal to be super confused if you're not coming from a coding background :)  I used to teach C in university, part time, to first years, so I've seen much worse.

Oh and I have a 200D, working (very slowly now, kind of stuck) on an ML port.

timbytheriver

Thanks @names_are_hard

The preset lives in crop_rec.mo. I'm working from one of Danne's earlier crop_rec.c as my template: https://bitbucket.org/Dannephoto/magic-lantern/src/9b8625caa3554d09003bd8a89dee7f43d66a634e/modules/crop_rec/crop_rec.c

Line 880 is where I started from...

My own version is local only at present. I have set up a bitbucket account today, once I get my head around it I can put the code in my own repo! :)

My own version I added to from about line 880 (with your earlier help! :) )to include the following code. I think this the main chunk.


switch (crop_preset)
        {
            /* all 1:1 modes (3x, 3K, 4K...) */
            case CROP_PRESET_3X:
            case CROP_PRESET_3X_TALL:
            case CROP_PRESET_3K:
            case CROP_PRESET_UHD:
            case CROP_PRESET_4K_HFPS:
            case CROP_PRESET_FULLRES_LV:
                /* ADTG2/4[0x8000] = 5 (set in one call) */
                /* ADTG2[0x8806] = 0x6088 on 5D3 (artifacts without it) */
                adtg_new[2] = (struct adtg_new) {6, 0x8000, 5};
                if (is_5D3) {
                    /* this register is model-specific */
                    adtg_new[3] = (struct adtg_new) {2, 0x8806, 0x6088};

                    /* timebytheriver edit below for testing cleaner iso on UHD preset. These relate to ADTG2/4 Preamps 8,9,a,b and ADTG4 0XFE gain */
                   
                    /* ADTG2 adjusts blue and green channel 1? */
                    adtg_new[13] = (struct adtg_new) {2, 0x8, 2}; /* was id 23 originally */
                    adtg_new[14] = (struct adtg_new) {2, 0x9, 2};
                    adtg_new[15] = (struct adtg_new) {2, 0xA, 2};
                    adtg_new[16] = (struct adtg_new) {2, 0xB, 2};
                     /* ADTG4 adjusts red and green channel 2? */
                    adtg_new[17] = (struct adtg_new) {4, 0x8, 2};
                    adtg_new[18] = (struct adtg_new) {4, 0x9, 2};
                    adtg_new[19] = (struct adtg_new) {4, 0xA, 2}; 
                    adtg_new[20] = (struct adtg_new) {4, 0xB, 2};

                    adtg_new[21] = (struct adtg_new) {2, 0xFE, 0};

                    /* end timebytheriver edit below for testing clean iso on UHD preset */
                }
                break;



5D3 1.1.3
5D2 2.1.2

Danne

Looking at your line up it seems you are using "maybe" already used slots. Didn´t look too carefully. For instance:
adtg_new[13] could be used somewhere else already. I would recommend to expand from this:
/* expand this as required */
    struct adtg_new adtg_new[30] = {{0}};

to this:
/* expand this as required */
    struct adtg_new adtg_new[40] = {{0}};

And then maybe start using slots from something like this:
                    adtg_new[30] = (struct adtg_new) {2, 0x8, 2}; /* was id 23 originally */
                    adtg_new[31] = (struct adtg_new) {2, 0x9, 2};



Edit:
Also note that you can write values like follows too:
Example:
adtg_new[13] = (struct adtg_new) {6, 0x8882, 0x438};
0x438

I am also a noob here, believe me ;)

names_are_hard

I still need to see where this part comes from:
Quote
ADTG Preamp = 2

I can't understand what you're comparing against without that.  You say it's iso_regs related but I can't find it in iso_regs.c.

Once again, a disclaimer that I know basically nothing about ADTG, I'm just reading the code.  Don't trust me very much :)  So, while I can comment on correctness of your code from a C perspective, I dunno what it's going to do on a real cam.

In your added code it looks like you always want to set ADTG2 and ADTG4 to the same values.  If so, just use 6 for the mask field and only have one set of the four similar lines:
Quote
                    /* ADTG2/4 adjusts blue and green channel 1? */
                    adtg_new[13] = (struct adtg_new) {6, 0x8, 2}; /* was id 23 originally */
                    adtg_new[14] = (struct adtg_new) {6, 0x9, 2};
                    adtg_new[15] = (struct adtg_new) {6, 0xA, 2};
                    adtg_new[16] = (struct adtg_new) {6, 0xB, 2};
To understand how that works, read my comment about masks and learn about bit operations in C.  Specifically you want to learn about "bitwise and".  You'd also want to learn about hexadecimal / binary / decimal conversions to understand how flipping single bits changes the number.

You have a discrepancy between one of your comments and one of your lines of code.  You say "ADTG4 0xFE gain", but your code has "adtg_new[21] = (struct adtg_new) {2, 0xFE, 0};", which is trying to modify ADTG2 (the mask, which is the first field in the struct, is 2).  I don't know which you intended.  Perhaps this should be 6 as well?

Your code is inside the is_5D3 guard.  This implies a) you have a 5D3 (if not, your changes shouldn't do anything for you) b) the changes you are making only work on 5D3.  Point b doesn't matter right now since it's just you using it, but if you expect this to work on multiple cams you'd want to move it outside of the guard at some point (after testing, etc).

By the way, I am guessing it's four lines because in bayer there's BGGR, four channels.  Here each line sets one register (presumably 0x8, 0x9, 0xa, 0xb are offsets into some set of registers), I assume the ADTG chip later checks each register for each colour channel setting.  Each register will control some different property of the hardware. I could easily be wrong; just guesses around how embedded stuff often works.  You could test by only setting one and seeing if it affects one colour only (or half of the green pixels).  Doesn't matter to me, might be interesting for you.

To illustrate what I mean about registers within ADTG chip, these lines suggest register 0x800c controls if ADTG should skip lines in the sensor when reading it:
Quote
                /* ADTG2/4[0x800C] = 0: read every line */
                adtg_new[2] = (struct adtg_new) {6, 0x800C, 0};
That only needs one line, because you either skip lines or you don't.  For colour related stuff, since ADTG (seems to) give the ability to affect each colour channel separately, you can set four offsets / registers if you want to affect all four colour channels.  Perhaps this is where you're getting confused and what your references to 1 vs 4 mean?

The switch / case code is quite hard to read due to the funky indenting :(  I guess the file has mixed tabs and spaces :(

timbytheriver

Thanks plenty @Danne & @names_are_hard Will need to digest all of this good stuff. :)

@names_are_hard ADTG Preamp = 2 is a value I set in the iso_regs front-end gui.

Refs to it in the actual code are: https://bitbucket.org/hudson/magic-lantern/src/75c1f3fe29273f1057ca932f71446d75be8937b8/modules/iso_regs/iso_regs.c?at=iso-research

Lines, 27,36,243,375,513,582,599,675,765
5D3 1.1.3
5D2 2.1.2

names_are_hard

Oh, adtg_preamp!  No wonder I couldn't find "ADTG Preamp"!

That fits my theory on your 1 vs 4 confusion being to do with colour channels.  The preamp presumably applies to the whole sensor readout.

timbytheriver

Quote from: Danne on December 04, 2019, 06:20:12 PM
Edit:
Also note that you can write values like follows too:
Example:
adtg_new[13] = (struct adtg_new) {6, 0x8882, 0x438};


@Danne Thanks! Does using the Hex syntax make any operational difference? I see it used both ways in the same module!  :o
5D3 1.1.3
5D2 2.1.2

Danne

Quote from: timbytheriver on December 05, 2019, 10:20:08 AM
@Danne Thanks! Does using the Hex syntax make any operational difference? I see it used both ways in the same module!  :o
Do tests and you´ll see what happens. That´s how I do things.

timbytheriver

Have done now – must be hundreds!  :o

Hex/Dec doesn't seem to make a difference here... :)
5D3 1.1.3
5D2 2.1.2