这是indexloc提供的服务，不要输入任何密码
Skip to content

Introduce tuner transaction interface #9

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.

Sign up for GitHub

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

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

Introduce tuner transaction interface #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions lib/verilog/tuner/tuner_ctrl_txn_adapter.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
//==============================================================================
// Adapter between tuner_txn_if and existing tuner_ctrl_arb_if
// Moves the micro handshaking logic into one place so controllers only
// issue a single transaction.
//==============================================================================

`timescale 1ns/1ps
`default_nettype none

module tuner_ctrl_txn_adapter #(
parameter int DAC_WIDTH = 8,
parameter int ADC_WIDTH = 8,
parameter tuner_phy_pkg::tuner_ctrl_ch_e CHANNEL = tuner_phy_pkg::CH_SEARCH
) (
input logic i_clk,
input logic i_rst,
tuner_txn_if.arb txn_if,
tuner_ctrl_arb_if.producer ctrl_if
);
import tuner_phy_pkg::*;

typedef enum logic [1:0] {IDLE, WAIT_TUNE, WAIT_COMMIT, RESP} state_e;
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

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

The state enumeration uses 4 states but only declares 2 bits, which limits future extensibility. Consider using logic [2:0] to provide room for additional states or use an explicit width declaration that matches the actual number of states needed.

Suggested change
typedef enum logic [1:0] {IDLE, WAIT_TUNE, WAIT_COMMIT, RESP} state_e;
typedef enum logic [2:0] {IDLE, WAIT_TUNE, WAIT_COMMIT, RESP} state_e;

Copilot uses AI. Check for mistakes.
state_e state, state_next;
logic refresh_pulse;

// State machine
always_ff @(posedge i_clk or posedge i_rst) begin
if (i_rst) begin
state <= IDLE;
end else begin
state <= state_next;
end
end

always_comb begin
state_next = state;
unique case (state)
IDLE: if (txn_if.val) state_next = WAIT_TUNE;
WAIT_TUNE: if (ctrl_if.get_ctrl_tune_ack(CHANNEL)) state_next = WAIT_COMMIT;
WAIT_COMMIT: if (ctrl_if.get_ctrl_commit_ack(CHANNEL)) state_next = RESP;
RESP: if (txn_if.fire()) state_next = IDLE;
Comment on lines +38 to +41
Copy link

@gemini-code-assist gemini-code-assist bot Jul 9, 2025

Choose a reason for hiding this comment

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

medium

The state machine could potentially get stuck in WAIT_TUNE or WAIT_COMMIT states if the expected ack signals from ctrl_if are not asserted. Consider adding a timeout mechanism to handle such scenarios and prevent potential deadlocks.

endcase
end

// Refresh pulse when new transaction begins
always_ff @(posedge i_clk or posedge i_rst) begin
if (i_rst) refresh_pulse <= 1'b0;
else refresh_pulse <= (state == IDLE) && (state_next == WAIT_TUNE);
end

// Drive control arbiter interface
assign ctrl_if.ctrl_active[CHANNEL] = (state != IDLE);
assign ctrl_if.ctrl_refresh[CHANNEL] = refresh_pulse;

assign ctrl_if.ring_tune[CHANNEL] = txn_if.tune_code;
assign ctrl_if.tune_val[CHANNEL] = (state == WAIT_TUNE) && txn_if.val;

assign ctrl_if.commit_rdy[CHANNEL] = (state == WAIT_COMMIT);

// Pass measurement back on RESP state
assign txn_if.rdy = (state == RESP);
assign txn_if.meas_power = ctrl_if.pwr_commit;

endmodule

`default_nettype wire
21 changes: 20 additions & 1 deletion lib/verilog/tuner/tuner_phy.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ module tuner_phy #(
.i_clk(i_clk),
.i_rst(i_rst)
);

tuner_txn_if #(
.DAC_WIDTH(DAC_WIDTH),
.ADC_WIDTH(ADC_WIDTH)
) search_txn_if (
.i_clk(i_clk),
.i_rst(i_rst)
);
// ----------------------------------------------------------------------

// ----------------------------------------------------------------------
Expand Down Expand Up @@ -102,6 +110,17 @@ module tuner_phy #(
.o_afe_ring_tune_val()
);

tuner_ctrl_txn_adapter #(
.DAC_WIDTH(DAC_WIDTH),
.ADC_WIDTH(ADC_WIDTH),
.CHANNEL(CH_SEARCH)
) search_txn_adapter (
.i_clk(i_clk),
.i_rst(i_rst),
.txn_if(search_txn_if.arb),
.ctrl_if(ctrl_arb_if.producer)
);

tuner_search_phy #(
.DAC_WIDTH(DAC_WIDTH),
.ADC_WIDTH(ADC_WIDTH),
Expand All @@ -116,7 +135,7 @@ module tuner_phy #(
.i_dig_ring_tune_end(i_cfg_ring_tune_end),
.i_dig_ring_tune_stride(i_cfg_ring_tune_stride),

.ctrl_arb_if(ctrl_arb_if.producer),
.txn_if(search_txn_if.ctrl),
.search_if(search_if),
.o_dig_ring_tune(search_phy_ring_tune)
);
Expand Down
129 changes: 25 additions & 104 deletions lib/verilog/tuner/tuner_search_phy.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ module tuner_search_phy #(
/*// Power Detector Interface
*tuner_pwr_detect_if.consumer pwr_detect_if,*/

// Controller Arbiter Interface
tuner_ctrl_arb_if.producer ctrl_arb_if,
// Tuning transaction interface
tuner_txn_if.ctrl txn_if,

// Search Interface for local search
tuner_search_if.producer search_if,
Expand Down Expand Up @@ -81,9 +81,6 @@ module tuner_search_phy #(
/*typedef tuner_pwr_detect_if.pwr_detect_state_e search_active_state_e;*/
/*typedef tuner_phy_detect_if_state_e search_active_state_e;*/

// Arbiter I/F state for communication with ring tuner and power detector
// This is currently the only substate in SEARCH_ACTIVE
typedef tuner_phy_ctrl_arb_if_state_e search_active_state_e;

// First half [0:SEARCH_PEAK_WINDOW_HALFSIZE-1]
// Second half [SEARCH_PEAK_WINDOW_HALFSIZE:2*SEARCH_PEAK_WINDOW_HALFSIZE-1]
Expand All @@ -110,29 +107,17 @@ module tuner_search_phy #(

/*logic pwr_read_fire;*/
/*logic pwr_detect_fire;*/
logic is_ctrl_active_state;
logic is_tune_state;
logic is_update_state;
logic tune_fire;
logic commit_fire;
logic tune_compute;
logic tune_compute_done;
logic update_commit_done;

// Refactored into pwr_detect_if
// Internal state within SEARCH_ACTIVE
// Reintroduced for controller arbiter i/f
search_active_state_e search_active_state, search_active_state_next;
logic txn_valid;

int search_active_cnt;
int search_active_cnt_max;
logic search_active_update;
logic search_active_done;
logic is_ctrl_active_state;

logic [DAC_WIDTH-1:0] ring_tune_step;
logic [DAC_WIDTH-1:0] ring_tune;
/*logic [DAC_WIDTH-1:0] ring_tune_prev;*/
logic [DAC_WIDTH-1:0] ring_tune_next;

/*logic [ADC_WIDTH-1:0] ring_pwr_detected;
*logic [ADC_WIDTH-1:0] ring_pwr_detected_prev;*/
Expand Down Expand Up @@ -255,103 +240,32 @@ module tuner_search_phy #(
*assign pwr_detect_if.pwr_detect_refresh = (state == SEARCH_INIT);
*assign search_active_update = pwr_detect_if.pwr_detect_update;*/
// ----------------------------------------------------------------------

// ----------------------------------------------------------------------
// SEARCH_ACTIVE - Controller Arbiter I/F
// SEARCH_ACTIVE - Transaction I/F
// ----------------------------------------------------------------------
// Super-state for active control
assign is_ctrl_active_state = (state == SEARCH_ACTIVE);
// Update sub-state: receive committed ring tune and power
assign is_update_state = is_ctrl_active_state && (search_active_state == CTRL_UPDATE);
// Tune sub-state: compute tuner code
assign is_tune_state = is_ctrl_active_state && (search_active_state == CTRL_TUNE);

assign tune_fire = ctrl_arb_if.get_ctrl_tune_ack(CH_SEARCH);
assign commit_fire = ctrl_arb_if.get_ctrl_commit_ack(CH_SEARCH);

// Internal state for controller arbiter
always_ff @(posedge i_clk or posedge i_rst) begin
if (i_rst) begin
search_active_state <= CTRL_TUNE; // Start with CTRL_TUNE
end
else if (search_refresh) begin
search_active_state <= CTRL_TUNE; // Reset to CTRL_TUNE on refresh
end
else begin
search_active_state <= search_active_state_next;
end
end

// Advance states only at SEARCH_ACTIVE
always_comb begin
search_active_state_next = search_active_state;
if (is_ctrl_active_state) begin
case (search_active_state)
CTRL_TUNE: if (tune_fire) search_active_state_next = CTRL_UPDATE;
CTRL_UPDATE: if (commit_fire) search_active_state_next = CTRL_TUNE;
default: search_active_state_next = CTRL_UPDATE; // Reset to CTRL_TUNE on error
endcase
end
end

/*assign ctrl_arb_if.ctrl_active = is_ctrl_active_state;
*assign ctrl_arb_if.ctrl_refresh = search_refresh;*/
assign ctrl_arb_if.ctrl_active[CH_SEARCH] = is_ctrl_active_state;
assign ctrl_arb_if.ctrl_refresh[CH_SEARCH] = search_refresh;

assign ctrl_arb_if.tune_val[CH_SEARCH] = is_tune_state && tune_compute_done;
assign search_active_update = txn_if.fire();
assign txn_if.val = is_ctrl_active_state && txn_valid;
assign txn_if.tune_code = ring_tune;

// commit is instantaneous
assign update_commit_done = is_update_state;
assign ctrl_arb_if.commit_rdy[CH_SEARCH] = is_update_state && update_commit_done;

/*assign search_active_update = is_ctrl_active_state && ctrl_arb_if.get_ctrl_commit_ack(CH_SEARCH);*/
assign search_active_update = is_ctrl_active_state && commit_fire;
// compute start at the next cycle after commit
/*assign tune_compute_start = search_active_update;*/
// ----------------------------------------------------------------------

// ----------------------------------------------------------------------
// SEARCH_ACTIVE - Ring Tuning
// ----------------------------------------------------------------------
// Step ring tuner at pwr detect (let pwr detector to deal with analog delays)
assign ring_tune_step = (1 << i_dig_ring_tune_stride);
assign tune_compute = is_tune_state && !tune_compute_done;

always_ff @(posedge i_clk or posedge i_rst) begin
if (i_rst) begin
ring_tune <= '0;
end
else if (search_refresh) begin
end else if (search_refresh) begin
ring_tune <= i_dig_ring_tune_start;
end
else if (tune_compute) begin
end else if (txn_if.fire()) begin
Comment on lines +260 to +262
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

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

[nitpick] The else if statements should be formatted consistently. Line 260 uses end else if while line 262 uses end else if - consider using the same spacing pattern throughout the conditional chain.

Copilot uses AI. Check for mistakes.
ring_tune <= ring_tune + ring_tune_step;
end
end

always_ff @(posedge i_clk or posedge i_rst) begin
if (i_rst) begin
tune_compute_done <= 1'b0;
end
else if (search_refresh) begin
tune_compute_done <= 1'b0;
end
// Assume single-cycle tuner code compute (toggle done immediately)
else if (is_tune_state) begin
tune_compute_done <= 1'b1;
end
else if (is_update_state) begin
tune_compute_done <= 1'b0;
end
end

/*assign o_dig_ring_tune = ring_tune;*/
assign ctrl_arb_if.ring_tune[CH_SEARCH] = ring_tune;
// ----------------------------------------------------------------------
assign o_dig_ring_tune = ring_tune;

// ----------------------------------------------------------------------
// SEARCH_ACTIVE - Count
// ----------------------------------------------------------------------
// Count the number of power detections taken during SEARCH_ACTIVE
assign search_active_cnt_max = (i_dig_ring_tune_end - i_dig_ring_tune_start) >> i_dig_ring_tune_stride;
Expand All @@ -368,6 +282,18 @@ module tuner_search_phy #(
search_active_cnt <= search_active_cnt + 1;
end
end

always_ff @(posedge i_clk or posedge i_rst) begin
if (i_rst) begin
txn_valid <= 1'b0;
end
else if (search_refresh) begin
txn_valid <= 1'b1;
end
else if (txn_if.fire()) begin
txn_valid <= ~search_active_done;
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

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

[nitpick] Using bitwise negation ~ on a single-bit signal is less clear than logical negation !. Consider using !search_active_done for better readability.

Suggested change
txn_valid <= ~search_active_done;
txn_valid <= !search_active_done;

Copilot uses AI. Check for mistakes.
end
end
// ----------------------------------------------------------------------

// ----------------------------------------------------------------------
Expand All @@ -387,13 +313,8 @@ module tuner_search_phy #(
end
else if (search_active_update) begin
// Receive the committed ring tune and power from the controller arbiter
pwr_det_track <= ctrl_arb_if.pwr_commit;
ring_tune_track <= ctrl_arb_if.ring_tune_commit;
end
end

// Majority-Vote
/*assign pwr_incremented = (i_dig_pwr_detected > pwr_detected_track_window[0]);*/
pwr_det_track <= txn_if.meas_power;
ring_tune_track <= ring_tune;
Copy link

@gemini-code-assist gemini-code-assist bot Jul 9, 2025

Choose a reason for hiding this comment

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

critical

The always_ff block starting on line 304 and the else if block starting on line 314 are missing their closing end keywords, which is causing a syntax error and build failure.

      pwr_det_track   <= txn_if.meas_power;
      ring_tune_track <= ring_tune;
    end
  end

/*assign pwr_decremented = (i_dig_pwr_detected < pwr_detected_track_window[0]);*/
assign pwr_incremented = pwr_det_track_win[0] > pwr_det_track_win[1];
assign pwr_decremented = pwr_det_track_win[0] < pwr_det_track_win[1];
Expand Down
44 changes: 44 additions & 0 deletions lib/verilog/tuner/tuner_txn_if.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//==============================================================================
// Tuner transaction interface
// Provides a combined request/response handshake for tuning operations.
//==============================================================================

`timescale 1ns/1ps
`default_nettype none

interface tuner_txn_if #(
parameter int DAC_WIDTH = 8,
parameter int ADC_WIDTH = 8
) (
input logic i_clk,
input logic i_rst
);
logic val;
logic rdy;
logic [DAC_WIDTH-1:0] tune_code;
logic [ADC_WIDTH-1:0] meas_power;

function automatic logic fire();
return val & rdy;
endfunction

// Controller side
modport ctrl(
output val,
output tune_code,
input rdy,
input meas_power,
import fire
);

// Arbiter side
modport arb(
input val,
input tune_code,
output rdy,
output meas_power,
import fire
);
endinterface

`default_nettype wire