This PR addresses issues #313 and #16 with the primary purpose being to ensure that the register updates from the encoder are synchronous with the control code. This PR also incorporates the stale PR #263.

Encoder Modifications

The two register updates that come from the encoder are steps, which are held in the counter register, and position, which is held in the position register. Two new registers, steps_synced and position_synced were added to hold the values of the steps and position from the current state of the encoder relative to AMDC-Firmware timing.

In order to synchronize the PWM Carrier signals with the encoder interface, the first thing to do was pass the internal pwm_carrier_high and pwm_carrier_low signals into the encoder IP core and connect those signals in throughout the modules. Then, the encoder interface was updated to copy the instantaneous register updates to newly created synced registers when the PWM carrier high or low signals are true, based on the points from the triangle carrier wave:

else if (pwm_carrier_low || pwm_carrier_high) begin
    steps_synced <= counter;
    position_synced <= position;
 end

Driver Modifications

In the AXI Verilog file, the read registers were updated to add in the synced values from the encoder interface:

  2'h2   : reg_data_out <= steps_synced;
  2'h3   : reg_data_out <= position_synced;

Thus, the encoder driver was updated to make the defaults for retrieving the steps and position now get the synced values. The old functions, which were previously the default, were updated to a different name with "_instantaneous" added, as that will now need to be called in order to get the immediate value from the registers counter and position.

Code Modification Example

The new default to get the correctly synchronized value of the steps; the offset is set to 2, which, as seen above, reads the value from steps_synced:

void encoder_get_steps(int32_t *steps)
{
    *steps = Xil_In32(ENCODER_BASE_ADDR + 2 * sizeof(uint32_t));
}

The old default which now gets the immediate value of the steps; the offset is set to 0, which reads the value from counter:

void encoder_get_steps_instantaneous(int32_t *steps)
{
    *steps = Xil_In32(ENCODER_BASE_ADDR);
}

Changes Made to Address Issue #16

These changes address the issue that the firmware only supports pulses per revolution that are a power of 2.

The new default created in the FPGA code changes the pulses_per_rev_bits signal to pulses_per_rev and does not shift the pulses when assigning the MAX_POS signal.

The encoder driver was also modified to add a new function which changes the parameter to pulses versus bits:

void encoder_set_pulses_per_rev(uint32_t pulses)
{
    printf("ENC:\tSetting pulses per rev = %ld...\n", pulses);
    Xil_Out32(ENCODER_BASE_ADDR + 2 * sizeof(uint32_t), pulses);
}

encoder_set_pulses_per_rev_bits() now calls the new function with an argument of 1 shifted left by the inputted amount of bits.

Testing

A SystemVerilog test bench was created to model the behavior of the encoder and test the results at different speeds. For this test, six different cases were used, varying the amount of clock cycles between the A and B pulses changing from 5 to 2400 (2.4 million RPM to 5,000 RPM). The direction was also changed. The task to simulate these revolutions took inputs of direction, number of revolutions, cycles, and speed (in clock cycles). In this test bench, the pulses per revolution was set to 12 (decimal), and this was connected to the newly changed input of pulses_per_rev in instantiation of the encoder peripheral.

This was the equation used to find the number of clocks to simulate based on an RPM value and a 5 ns clock period: $clocks = {60 \over 1000{RPM5e-9}}$

Here is an example of when the encoder is simulated at 100,000 RPM:

As can be seen, at the negative edge of the carrier high signal, the steps_synced and position_synced registers now hold the value from counter and position from the last clock cycle.

The test bench was automated to stop and produce an error message if the synced registers' values changed when between assertion of either the carrier high or low signals, therefore indicating that the register updates are no longer synchronized. Falling edge detectors were added to find both the negative edge of the carrier low and high signals; a flop was added to check that the synced values are held for both cases.

Results

At lower speeds, it behaves similarly; the immediate counter and position registers do not change often between the pwm_carrier_high and pwm_carrier_low signals being asserted. Therefore, the synchronization issue is not as clearly seen with these arguments; if the speed is low enough, the immediate values will not change between carrier signals and would read the same value as the copied synced values for a large portion of the time. Here is an example of how this looks at 10,000 RPM:

However, at higher speeds, it is much clearer that synchronization works better. Both the position and counter registers update frequently between the high and low signals; however, the synced values hold the data that was stored at the previous carrier signal. Here is an example of this happening at 100,000 RPM:

A $display() statement was added after each call to the simulation task to show the difference in the synced versus immediate values. It can be seen that at very slow speeds, they are the same, but at higher speeds, they have a greater difference, which further proves the functionality and impact of this synchronization update to the encoder.

The synced registers retain the value from when the high or low signals are asserted, and only change at the next negative clock edge of the next signal. This improves the functionality at higher speeds. In all, the value of the steps and position are now synchronized with the AMDC firmware timing, instead of being relative to the encoder.

Severson-Group / AMDC-Firmware

Synchronize Encoder Updates with PWM Carrier #317