Skip to content

Conversation

@Biancaa-R
Copy link

This patch introduces batch.c and batch.h to implement the skip-rti-nop mechanism for RISC-V DTM transactions. The new logic optimizes DMI command batching by skipping redundant RTI and NOP sequences, improving overall debug interface efficiency.

Files added:

  • src/target/riscv/batch.c
  • src/target/riscv/batch.h

Fixes the issue : #1308

Reading the value of batch->fields->outvalue before setting delay :
image

Also runs without issue:
image

This patch introduces batch.c and batch.h to implement the skip-rti-nop mechanism for RISC-V DTM transactions. The new logic optimizes DMI command batching by skipping redundant RTI and NOP sequences, improving overall debug interface efficiency.

Files added:
- src/target/riscv/batch.c
- src/target/riscv/batch.h

Signed-off-by: Biancaa-R <biancaa2210329@ssn.edu.in>
Copy link
Collaborator

@en-sc en-sc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

Regarding the part about Abstract Commands -- i've mistaken, it's irrelevant here -- a NOP can not start an Abstract Command.

const unsigned int out_op = buf_get_u32(batch->fields->out_value,
DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH);

if (out_op == DTM_DMI_OP_NOP) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to keep the logic around delay times inside get_delay(). The main reason -- here the operations are scheduled, but the results are reported separately, in the log_batch() function. With the current implementation the delay will be logged incorrectly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@en-sc so you want me to add this same logic but inside get_delay() right? so the timing logs and scheduling stay consistent.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

if (bscan_tunnel_ir_width != 0)
riscv_add_bscan_tunneled_scan(batch->target->tap, batch->fields + i,
batch->bscan_ctxt + i);
batch->bscan_ctxt + i);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem like a correct formatting fix -- AFAIK, throughout the codebase the prevailing pattern is +2 TABs when inside the braces of a function call, e.g.:
https://github.com/Biancaa-R/riscv-openocd/blob/f9ef41bf7622d65a14066056eda3876a0b9d29cc/src/target/target.c#L738-L739


// TODO: DTM_DMI_MAX_ADDRESS_LENGTH should be reduced to 32 (per the debug spec)
#define DTM_DMI_MAX_ADDRESS_LENGTH ((1<<DTM_DTMCS_ABITS_LENGTH)-1)
#define DTM_DMI_MAX_ADDRESS_LENGTH ((1 << DTM_DTMCS_ABITS_LENGTH) - 1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's separate formatting fixes from functional once for the sake of a sane git blame.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants