Capture makeover

[coreyeng]

Hello,

I've done a bit of work on the captures and overlays (mostly captures though) but I'm hitting some points where I'm questioning what the user interface should look like. My own use cases are very direct: I'll be using reg.capture() in patterns or pin.capture() in drivers, expecting the capture character to override whatever's in the final output. But, I'd like a bit feedback on what more needs to be supported.

So far, I've got the underlying nodes, processors, and the V93K renderer in place and I think the foundation is good. Capturing is actually a tester-oriented action where reg.capture() andpin.capture() are really just shortcut/convenience functions to simplify things.

Some Implementation Details

I further extended the transaction to also hold a sub-struct, called a capture, which contains only the details needed to perform a capture. A capture can be standalone, used primarily for internal handling, or packaged along with a transaction, replacing the previous capture_enables field.

Capture requests from the patterns just add a Capture node. Following what @ginty said during our last meeting, this doesn't do anything fancy - just acts as record of what the pattern wanted to do. A processor specific to captures handles de-compressing vectors to account for capturing and tossing in EndCapture nodes that make for more easier rendering.

To-Do: Optimizing Captures

Just to address this up front, the rendered output is not guaranteed to be the most vector-compressed. The original vector compression uses the non-capture state and the vector de-compression only decompresses the minimal amount. The stuff in place though should be able to handle this, just not something I've done yet. Example:

https://github.com/Origen-SDK/o2/blob/2cb4709329ac903e7febef4f7f23f3ceb14736c0/test_apps/python_app/approved/v93ksmt7/captures.avc#L304-L319

Ideally, this would be a single R32 if vector compression is enabled.

While on the subject, I think we need a "compress" option anyway to force these to be compressed from the pattern. Another reason why I haven't worried much about this yet.

Python Interface

Okay... so now to get into why I opened this PR this early in the first place:

I've have an example pattern which shows the basic use cases. But, as I was doing this, I started questioning what's really needed and how certain corner cases should resolve.

Capturing on the Tester

As touched on above, captures and handled internally as a tester-level action. In Python, this looks like this:

https://github.com/Origen-SDK/o2/blob/2cb4709329ac903e7febef4f7f23f3ceb14736c0/test_apps/python_app/example/patterns/captures.py#L10-L24

Which generates a (preprocessed) AST:

    Comment(1, "Basics")
    Comment(1, "---")
    Comment(1, "Capture a single cycle")
    Capture(Capture { symbol: None, cycles: None, enables: None, pin_ids: None }, None)
    Cycle(1, true)
    Cycle(1, true)
    Comment(1, "Capture a single cycle on portc")
    Capture(Capture { symbol: None, cycles: None, enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(1, true)
    Cycle(1, true)
    Comment(1, "Capture two cycles on portc")
    Capture(Capture { symbol: None, cycles: Some(2), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(2, true)
    Comment(1, "Capture three cycles on portc with symbol \'A\'")
    Capture(Capture { symbol: Some("A"), cycles: Some(3), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(3, true)
    Comment(1, "Capture four cycles on portc with symbol \'B\'")
    Capture(Capture { symbol: Some("B"), cycles: Some(4), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(2, true)
    Cycle(2, true)
    Comment(1, "Capture four cycles on portc and clk")
    Capture(Capture { symbol: None, cycles: Some(4), enables: None, pin_ids: Some([6, 7, 8]) }, None)
    Cycle(10, true)

And ultimately renderers V93K vectors of:

https://github.com/Origen-SDK/o2/blob/2cb4709329ac903e7febef4f7f23f3ceb14736c0/test_apps/python_app/approved/v93ksmt7/captures.avc#L28-L41

When capturing this way, everything is optional. If no cycles are given, its up to the render to figure out what it wants to do with that, but the vector-based renderer will treat it as a single-cycle capture. Pins and a custom capture symbol are also available. This is the low-level or most-direct capture mechanism.

This yields a corner case though where two back-to-back captures could step on each other. If the pins are different then its not really an issue, since those are tracked independently, but pins stepping on each other causes some tricky resolutions. This is more easily shown when capturing on a pin, so I've got more details below.

Capturing On Pins

A shortcut for applying captures when you have a handle on the pins already. pin.capture(**kwargs) equates in the AST to calling tester.capture(pins=[pin], **kwargs):

https://github.com/Origen-SDK/o2/blob/2cb4709329ac903e7febef4f7f23f3ceb14736c0/test_apps/python_app/example/patterns/captures.py#L26-L39

Which yields:

    Comment(1, "Basics with implied pins (portc)")
    Comment(1, "---")
    Comment(1, "Capture next cycle (portc)")
    Capture(Capture { symbol: None, cycles: Some(1), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(1, true)
    Cycle(1, true)
    Comment(1, "Capture next two cycles (portc)")
    Capture(Capture { symbol: None, cycles: Some(2), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(2, true)
    Cycle(1, true)
    Comment(1, "Capture next two cycles with symbol \'A\' (portc)")
    Capture(Capture { symbol: Some("A"), cycles: Some(2), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(2, true)
    Cycle(1, true)
    Comment(1, "Capture next two cycles with symbol \'B\' masking the second bit (portc)")
    Capture(Capture { symbol: Some("B"), cycles: Some(2), enables: Some(BigUint { data: [2] }), pin_ids: Some([6, 7]) }, None)
    Cycle(2, true)
    Cycle(1, true)

In the AST, and:

https://github.com/Origen-SDK/o2/blob/2cb4709329ac903e7febef4f7f23f3ceb14736c0/test_apps/python_app/approved/v93ksmt7/captures.avc#L42-L55

In the V93K pattern.

This has the same corner cases as above where you can override a previous capture without warning. We could do whatever we wanted here with the current architecture: allow it, raise an error, raise a warning, stack them (when possible), etc., but I'm not sure which would be the best.

An example of what I'm talking about:

https://github.com/Origen-SDK/o2/blob/2cb4709329ac903e7febef4f7f23f3ceb14736c0/test_apps/python_app/example/patterns/captures.py#L41-L55

    Comment(1, "Corner case: two captures stepping on each other")
    Comment(1, "---")
    Comment(1, "(capture on portc with symbol \'A\' is essentially ignored)")
    Capture(Capture { symbol: Some("A"), cycles: Some(2), enables: None, pin_ids: Some([6, 7]) }, None)
    Comment(1, "Capture on portc with symbol \'B\'")
    Capture(Capture { symbol: Some("B"), cycles: Some(2), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(2, true)
    Cycle(1, true)
    Comment(1, "Two captures with symbols")
    Comment(1, "---")
    Comment(1, "This however, is fine.")
    Capture(Capture { symbol: Some("A"), cycles: Some(2), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(2, true)
    Capture(Capture { symbol: Some("B"), cycles: Some(2), enables: None, pin_ids: Some([6, 7]) }, None)
    Cycle(2, true)
    Cycle(1, true)

https://github.com/Origen-SDK/o2/blob/2cb4709329ac903e7febef4f7f23f3ceb14736c0/test_apps/python_app/approved/v93ksmt7/captures.avc#L56-L67

Notice that both captures appear in the AST, but only the latter is rendered since it "overwrite" the former.

I don't like the idea of allowing it as its very likely that it was overridden by mistake. However, this is actually following pin behavior. For example, pins can override their state without cycling and I don't know that we should make a distinction between pin.capture() and pin.drive() regarding overrides. I think that'd end up confusing to have some pin methods allowing one thing and some others behaving differently. We could very easily do this, just a matter of if we should. For this PR, the easiest case was implemented - do nothing and let a future node overwrite a previous one.

What do we want to do for this case? I could see justification for all the options I listed, or if there's other ideas, throw 'em into the mix as well.

Additionally, a side effect of changing mentalities is that [these test cases] are now incorrect. Though, I think this makes sense in the new context.

Capturing On Registers

This too, is just a shortcut to generating equivalent nodes. A general RegCapture node takes a full transaction, allowing the underlying driver to still see the address and other needed parameters and then selecting where the capture should actually occur.

For example, ArmDebug's verify function checks if its Capture or a Verify and generates the data transaction appropriately. Handling captures from registers is a driver-level detail though. For example, ArmDebug will go through its rigmarole as normal before deferring to the generic drive transaction method, which will add the capture nodes.

In this PR, reg.capture() generates a specific ]Capture Transaction](https://github.com/Origen-SDK/o2/blob/capture_makeover/rust/origen/src/core/tester/api_structs.rs#L14-L19), which can handle uninitialized data. Otherwise, trying to capture a register without setting data results an error. I think capturing an uninitialized register should be allowed though.

This has the side effect of ArmDebug treating all reg.captures() as having no data. On the V93K, this isn't an issue because we can't do both anyway, but that's not universally the case.

I do plan to support something like reg.verify(capture=True) for this, but I don't know what kind of other options we should support with this. For example, this PR allows for pin.capture() or tester.capture() to also pass in a mask. I started on that here, but stopped a bit short as I'm not sure what that would like in the context of non-capture-character testers. Same with capturing on pins and the tester though and a similar story with the capture character.

Capture Character

The capture character has come up in discussions of captures some, but I'm not sure that we need it. I would think that a Capture Node should be sufficient and it’s a renderer-specific detail for if a capture character is needed or what to do with it.

Aside: Verbosity Keywords

I was often commenting/uncommenting lines to dump the AST, either before processing or after processing, and, although dut.test_ast is available, it can't dynamically print out the AST in various processing phases that occur on the Rust side.

I added the ability to pass in verbosity_keywords to the logger, which can be queried from the backed and perform logger-stuff appropriately. Although a bit overkill for just debugging the ASTs I'm working on, I think this would be useful in the log term as stacking up -v's doesn't allow you to hone in on certain things.

I'm thinking something like this will really help in early debug stages or if Origen catches on more as we may not have access to the full project to replicate issues and adding hooks to, for example, dump the pre-processed and post-processed AST from the offending environment only, without also looking at a ton of other non-related debug stuff.

This should also come in handy for plugins we don't own (internal ones) that take excessive liberty with printing data. If anyone remembers all the stuff Rosetta Stone would dump, it basically made running with --verbose useless for anything that occurred before RS ran as it could print 10k+ lines for large register models. This feature allows for more focused debug.

As many keywords as needed can be stacked. For example, to get both the pre/post processed ASTs, you can run:

origen g .\example\patterns\captures.py -k show_unprocessed_ast -k vector_based_dump_final_ast -v.

Note, this internally uses the logger.info statement, so at least a single -v is required, but that's up to the implementer to decide. Could just as well be a println, but I think for core stuff we should stick to using the logger so it ends up in the log file as well.

I'm not sold on the name of this. I spend a few minutes thinking up a name but everything either sounded too cute or didn't end up relating to what these actually do. So, I settled on verbosity_keywords, or -k for short. Naming recommendations are welcome though.

Lastly, we can build this up as needed. This was just a quick prototype. If we like this, we can delve into it more and support a verbosity list or something like that, or add some error handling for registering/clashing keywords. I've got some ideas, but didn't do anything beyond the base case for this.

Summary/Current State

I opened this as a draft PR because I believe the Rust-backend side, processors, and rendering is legitimate and I don't see frontend requests that couldn't be supported with that base architecture.

But, the Python interface and drivers have a lot of dangling pieces. As I was implementing, the questions above came up and I didn't want to put work into an interface I wasn't 100% on. I'd like to use this PR to discuss those points, making whatever changes are needed, then upgrade this to a real one.

These same questions will come up for overlays but, save for captures and overlays conflicting (such as giving both a capture character and an overlay character, if these stick around), the implementation will be very similar, but I'll do that in its own PR.

So my plan here:

• Use this space to figure out what the user interface should look like.
• Fix ^
• Change to a real PR & request merging.
• Add similar support for overlaying. If the interface is simplified a ton, then this could probably be quickly done here as well, but if not, I'll probably do it separately.

[pderouen]

Hi @coreyeng, thanks for all this work. I think you're on the right track. I made modifications to O1/Testers/drivers a few years back to make the capture and overlay interface transparent to the implementation.

Capture is simpler and documented here. I think the only thing missing from what you already have is a way to configure the size (for de-serialization when using digcap) and capture style. Right now only uflex digcap uses either of those settings. But, I made the configuration interface common to all testers to avoid the need for if statements. These information nodes can easily be added after your work is done though.

Overlay is a little more complex. There needs to be a way to specify when the overlay character needs to be used in the output pattern and a way to specify when new data should be sent. The o1 description is here. I would think in o2 with the node structure it could be simpler. There could be open/close overlay nodes and a new data node to indicate new data.

Drivers need to be written to include all of this information. @ginty had me start a running list of driver requirements.

[coreyeng]

Hi @pderouen,

I made modifications to O1/Testers/drivers a few years back to make the capture and overlay interface transparent to the implementation.

You can still get quite a bit of transparency, but its ultimately up to the driver to separate the pieces it wants to capture from other "boiler-plate" stuff. This is how arm debug works in O1 as a single "transaction" is actually 3 or 4 mini-transactions and its not til the end you actually want to capture or overlay something.

I think the only thing missing from what you already have is a way to configure the size (for de-serialization when using digcap) and capture style

[Edit] Nevermind. This is how it is in O1. Didn't look close enough.

Are those things we want in the actual capture methods though? I see those as more tester configuration things. And, they can also be quite tester specific. I saw those more as being set in an app's startup. For example, something like:

def startup(self):
  tester.set_capture_style("digcap")
  ...

And later make use of the tester-specific stuff @ginty has been adding to complain if a capture style (or whatever else) is given to a tester that doesn't know what to do with it. I am assuming though that those setting don't change through the course of a pattern.

That said, those could easily be added as options and tossed into the Capture struct, Its ultimately the renderer's problem to figure out what to do with it.

size (for de-serialization when using digcap)

Isn't this the same as the cycles option here? Specifying how long to capture for? If the renderer is one that needs to also know the number of pins, it can calculate that itself as it will have the pins being captured and how many cycles you want to capture for.

There needs to be a way to specify when the overlay character needs to be used in the output pattern and a way to specify when new data should be sent.

These are the same issues for captures though. That's why I stopped short of adding overlays. The same structure will support both as its possible on some platforms to read and capture. I think this possible on the Teradyne testers, but I've not used those in a while and I'm pretty sure a V93K method could be written to do this as well. But that would be another tester-specific thing and I don't think you'd mix those methodologies in the same pattern source.

As I was doing for captures, I would add something like:

reg.overlay("ovl") #=> implies "don't care" data.
reg.verify(overlay="ovl") #=> overlay with whatever data is in the register
reg.verify(0xCE, overlay="ovl") #=> set the data and overlay. Will both set the register data (internal state is updated) and acts as the "default data".

# Same for writes

This is more thinking out loud though. I really don't know what the best place for this stuff is. But, adding these options blows up the complexity for things that seem like more tester-level config values.

[pderouen]

I think the only thing missing from what you already have is a way to configure the size (for de-serialization when using digcap) and capture style

~Are those things we want in the actual capture methods though? I see those as more tester configuration things. And, they can also be quite tester specific. I saw those more as being set in an app's startup. For example, something like:~

def startup(self):
  tester.set_capture_style("digcap")
  ...

Yes this interface would be perfect. It doesn't need to be specified every time. But, we may want a way to temporarily override that setting.

~That said, those could easily be added as options and tossed into the Capture struct, Its ultimately the renderer's problem to figure out what to do with it.~

That seems reasonable.

size (for de-serialization when using digcap)

Isn't this the same as the cycles option here? Specifying how long to capture for?

No. This size option is used to construct the instrument statement in the pattern header. It is used to tell the tester to turn 1 pin (like TDO) with 256 captures into 8 32-bit words (for example). Dig src/cap of the Uflex is the only case I'm aware of where this gets configured in the pattern header. It was made part of the common API in o1 with the assumption that some future test system may need it and others can ignore it.

There also needs to be a way to specify any other arbitrary instrument settings (msb/lsb first, 2's complement, 1's complement, binary, etc.) for a given capture/source pin/group. This also remains the same throughout the pattern and is unique to the UFlex. But, I could see it being useful in other scenarios. Ideally some of this information should be provided by the protocol driver (JTAG driver knows data is presented LSB first).

There needs to be a way to specify when the overlay character needs to be used in the output pattern and a way to specify when new data should be sent.

These are the same issues for captures though. That's why I stopped short of adding overlays. The same structure will support both as its possible on some platforms to read and capture. I think this possible on the Teradyne testers, but I've not used those in a while and I'm pretty sure a V93K method could be written to do this as well. But that would be another tester-specific thing and I don't think you'd mix those methodologies in the same pattern source.

Lost me there. Most of my original comment was talking about the AST content with an aim toward making sure it can describe the more complicated Dig src/cap configuration and use. This is a verbose restatement of what I was intending to say about overlay:

As far as I know Teradyne and the NI test system are the only ones that have digital source overlay implemented in o1. In all other cases all you need to know is which cycles are marked for overlay and you don't care if the driver is presenting a new piece of data or holding the most recent data on the pin. With digsrc it is very possible for a single bit of overlay data to be held for multiple cycles. The AST needs to be able to mark the vectors for overlay and mark which vectors have new data. To handle it I think you need the AST to look something like this (imagine it is JTAG with a TCK multiple of 4):

<Begin overlay, pin: blah>
    <New Overlay data, pin: blah>
    <cycle>
    <cycle>
    <drive tck = 1>
    <cycle>
    <cycle>
    <drive tck = 0>
    <New Overlay data, pin: blah>
    <cycle>
    <cycle>
    <drive tck = 1>
    <cycle>
    <cycle>
    <drive tck = 0>
<End Overlay, pin:blah>

Which turns into these vectors on uflex (tck, tms, tdi, tdo):

(TDI:DigSrc Send)          > tp0 00DX
                           > tp0 00DX
                           > tp0 10DX
                           > tp0 10DX
(TDI:DigSrc Send)          > tp0 00DX
                           > tp0 00DX
                           > tp0 10DX
                           > tp0 10DX

[coreyeng]

Are the digital capture settings known up front? And are they able to change during course of the pattern? I'm still wondering if that should be nodes or just variables on the tester. From a renderer's perspective, you may have to do two passes to figure out how to render the pattern. Multiple passes is fine, but they may add up and a pass specifically for static, upfront settings seems like a waste.

We don't currently have a way to support arbitrary settings in the AST. Eveything needs to be enumerated. I was going to work on that as I have an idea as to how to implement it, but that's a bit out of scope for this PR and, when that's ready, it should be available for just about anything.

The overlay thing makes sense, and that's close to how the V93K currently handles the captures. That's a renderer level detail though, but it can be pulled into either the vector-based trait, or a Teradyne-specific one.

What is the call that you would expect to generate the overlay AST/output provided? Forgive me, but I know virtually nothing about the uflex anymore so I'm not seeing what options you'd expect to put to get that out. With what I was planning, you do this as:

tdi.overlay("some str", cycles=2)
tclk.drive(0).repeat(2)
tdi.overlay("some str", cycles=2)
tclk.drive(1).repeat(2)

tdi.overlay("some str", cycles=2)
tclk.drive(0).repeat(2)
tdi.overlay("some str", cycles=2)
tclk.drive(0).repeat(2)

It would be the uflex renderer that has to go shovel the 'U' characters into the final output.

[ginty]

Hi @coreyeng, this is great stuff.

Really like the API and how the pin/reg methods are just syntax sugar, and how it is all represented in the AST.

On how to handle the capture overwriting, I agree that we should probably error this as it is most likely a mistake. Generally, for things like this where I'm not sure what the right approach is I tend to make it raise an error. That means that you have dealt with it safely and won't forget about it, and if someone complains in future then they will be able to provide you with a real life scenario and a test case which you can use to further reason about and refine how it is handled.

As an aside, I've struggled a bit recently from wanting to add too much error checking on the Python side. Specifically, not allowing conflicting tester.eq blocks and not allowing just any hash of options to be thrown at a test object and have it pick up only the keys it understands. I ultimately backed out of both of those as it felt pretty un-Pythonic or un-Origen-like within application code and it was a reflection of the new stricter me who has now been heavily influenced by Rust being such a stickler for correctness. It's an interesting question as to how much of the backend strictness we want to expose on to the frontend. It's probably a bit of a Goldilocks problem, but with the general goal that if users are happy writing Python then it should feel like Python.

Otherwise, trying to capture a register without setting data results an error. I think capturing an uninitialized register should be allowed though.

I agree, and honestly I also expect to get a lot of complaints when folk start using the new register model which requires reset data to be specified instead of an implied reset value of 0. My plan was just to add some API to allow app's to declare that 0 is an implied reset value if not specified. Probably capture requires specialy handling to not care about initial data in both cases.

What you have for the capture character looks fine to me, the V93K should assume 'C' if not specified, for IG-XL force it to X regardless of what's specified. The V93K char can then be overridden via your option argument for something special on the V93K if required.

I really like that logger feature, definitely a good prototype.

On how testers should implement overlays/captures, I think this should be a good use case for the tester-specific patgen APIs, so something like:

with tester().eq("uflex") do uflex:
  uflex.capture_style = "digcap"
  uflex.capture_blah_blah = 10

That would just go straight into the AST and the UFlex renderer should pick it up and do its thing.

@pderouen is the expert on these capture/store features on Teradyne and I know little about them either. I would recommend that you mainly just put in the hooks to enable the above, maybe with a placeholder example, and then let Paul define exactly what the options should be and how they should render.

[pderouen]

What you have for the capture character looks fine to me, the V93K should assume 'C' if not specified, for IG-XL force it to X regardless of what's specified. The V93K char can then be overridden via your option argument for something special on the V93K if required.

Don't do that. For J750 IG-XL 'X' is fine. For UFlex that will shut off the comparator resulting in no capture. It needs to be 'V' for UFlex. The renderer handles that though, right?

[ginty]

You know I nearly wrote "J750" instead of "IGXL", but thought I would try and sound more relevant - should've stuck to what I know about! So yeah the UFlex renderer would implement that tester-specific detail.

[pderouen]

Sorry, I guess I never posted my responses.

Are the digital capture settings known up front? And are they able to change during course of the pattern? I'm still wondering if that should be nodes or just variables on the tester. From a renderer's perspective, you may have to do two passes to figure out how to render the pattern. Multiple passes is fine, but they may add up and a pass specifically for static, upfront settings seems like a waste.

The capture and overlay configuration settings are known up front, but can be (and usually are) different from pattern to pattern. I probably won't do a pass through the AST just looking for configuration nodes. That does seem wasteful. Most likely I'd collect that info during any of the other passes.

We don't currently have a way to support arbitrary settings in the AST. Eveything needs to be enumerated. I was going to work on that as I have an idea as to how to implement it, but that's a bit out of scope for this PR and, when that's ready, it should be available for just about anything.

I might have chosen for it to be something like an instrument configure node with name, type, and maybe 1 or 2 configuration fields that are strings. But, an arbitrary setting node would likely work fine as well. I agree this doesn't need to go in this PR.

What is the call that you would expect to generate the overlay AST/output provided? Forgive me, but I know virtually nothing about the uflex anymore so I'm not seeing what options you'd expect to put to get that out. With what I was planning, you do this as:

tdi.overlay("some str", cycles=2)
tclk.drive(0).repeat(2)
tdi.overlay("some str", cycles=2)
tclk.drive(1).repeat(2)

tdi.overlay("some str", cycles=2)
tclk.drive(0).repeat(2)
tdi.overlay("some str", cycles=2)
tclk.drive(0).repeat(2)

It would be the uflex renderer that has to go shovel the 'U' characters into the final output.

I think that interface is fine. I'd just need 1 more piece of information. I need to know whether a given cycle represents a new piece of overlay data. So, maybe something like this?

tdi.overlay("some str", cycles=2, new_data=false)
tclk.drive(0).repeat(2)

[coreyeng]

Thanks for all the feedback! I've got some loose ends to tie up yet, but good to know its on the right track.

@pderouen, I'll look at adding some tester config stuff. A separate node for this probably will be the best then if its subject to change as the pattern progresses. We'll likely need to grow it in the future, but it should be a decent starting point.