Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added config registers readers #63

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adjordjevic-TT
Copy link

Added reader functions for configuration registers structs for packer and unpacker

relu_config_u config;

// Get pointer to registers for current state ID
volatile uint tt_reg_ptr *cfg = get_cfg_pointer();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not keep the same approach as for read_pack_config? Where cfg is passed as argument?

@@ -519,4 +519,54 @@ namespace ckernel::packer
TTI_STOREIND (1, 0, p_ind::LD_16B, LO_16(0), p_ind::INC_NONE, p_gpr_pack::TILE_HEADER, p_gpr_pack::OUTPUT_ADDR);
}

// READERS FOR CONFIG STRUCTS

inline pack_config_t read_pack_config(uint32_t reg_addr, const volatile uint tt_reg_ptr* cfg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For other functions this is not passed as input since it is known at compile time.
Can you use the same thing here?

return dest.f;
}

inline pck_edge_offset_t read_pck_edge_offset(uint32_t reg_addr, const volatile uint tt_reg_ptr* cfg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's please not use abbreviation. If anything, we should remove the existing ones. pck -> pack

return edge.f;
}

inline pack_counters_t read_pack_counters(uint32_t reg_addr, const volatile uint tt_reg_ptr* cfg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about arguments as above. cfg could be obtained inside function and reg_addr should be known at compile time. If true, let's use that.
It also reduces possibility for user errors, as currently user can pass a wrong reg_addr and read pack_config instead of counters, for example.

alu_config_u config;
volatile uint tt_reg_ptr *cfg = get_cfg_pointer();

config.val = cfg[ALU_ROUNDING_MODE_Fpu_srnd_en_ADDR32];
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this correctly populate just the FPU rounding bit in the config? Or the whole config, and just the register name is unfortunate?

Copy link
Author

Choose a reason for hiding this comment

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

This should populate whole config. ALU_ROUNDING_MODE_Fpu_srnd_en_ADDR32 is starting address of the whole config (since fpu_srnd_en is first field in structure). As I understood, we pass to cfg starting address and then it returns 32 bits starting from there.

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