diff --git a/Marlin/planner.cpp b/Marlin/planner.cpp index f2d812157..6ae5da6d2 100644 --- a/Marlin/planner.cpp +++ b/Marlin/planner.cpp @@ -740,7 +740,11 @@ void Planner::calculate_trapezoid_for_block(block_t* const block, const float &e const bool was_enabled = STEPPER_ISR_ENABLED(); if (was_enabled) DISABLE_STEPPER_DRIVER_INTERRUPT(); - // Don't update variables if block is busy: It is being interpreted by the planner + // Don't update variables if block is busy; it is being interpreted by the planner. + // If this happens, there's a problem... The block speed is inconsistent. Some values + // have already been updated, but the Stepper ISR is already using the block. Fortunately, + // the values being used by the Stepper ISR weren't touched, so just stop here... + // TODO: There may be a way to update a running block, depending on the stepper ISR position. if (!TEST(block->flag, BLOCK_BIT_BUSY)) { block->accelerate_until = accelerate_steps; block->decelerate_after = accelerate_steps + plateau_steps; @@ -844,10 +848,13 @@ void Planner::reverse_pass_kernel(block_t* const current, const block_t * const ? max_entry_speed_sqr : MIN(max_entry_speed_sqr, max_allowable_speed_sqr(-current->acceleration, next ? next->entry_speed_sqr : sq(MINIMUM_PLANNER_SPEED), current->millimeters)); if (current->entry_speed_sqr != new_entry_speed_sqr) { - current->entry_speed_sqr = new_entry_speed_sqr; - // Need to recalculate the block speed + // Need to recalculate the block speed - Mark it now, so the stepper + // ISR does not consume the block before being recalculated SBI(current->flag, BLOCK_BIT_RECALCULATE); + + // Set the new entry speed + current->entry_speed_sqr = new_entry_speed_sqr; } } } @@ -907,14 +914,15 @@ void Planner::forward_pass_kernel(const block_t* const previous, block_t* const // If true, current block is full-acceleration and we can move the planned pointer forward. if (new_entry_speed_sqr < current->entry_speed_sqr) { + // Mark we need to recompute the trapezoidal shape, and do it now, + // so the stepper ISR does not consume the block before being recalculated + SBI(current->flag, BLOCK_BIT_RECALCULATE); + // Always <= max_entry_speed_sqr. Backward pass sets this. current->entry_speed_sqr = new_entry_speed_sqr; // Always <= max_entry_speed_sqr. Backward pass sets this. // Set optimal plan pointer. block_buffer_planned = block_index; - - // And mark we need to recompute the trapezoidal shape - SBI(current->flag, BLOCK_BIT_RECALCULATE); } } @@ -1001,6 +1009,12 @@ void Planner::recalculate_trapezoids() { if (current) { // Recalculate if current block entry or exit junction speed has changed. if (TEST(current->flag, BLOCK_BIT_RECALCULATE) || TEST(next->flag, BLOCK_BIT_RECALCULATE)) { + + // Mark the current block as RECALCULATE, to protect it from the Stepper ISR running it. + // Note that due to the above condition, there's a chance the current block isn't marked as + // RECALCULATE yet, but the next one is. That's the reason for the following line. + SBI(current->flag, BLOCK_BIT_RECALCULATE); + // NOTE: Entry and exit factors always > 0 by all previous logic operations. const float current_nominal_speed = SQRT(current->nominal_speed_sqr), nomr = 1.0 / current_nominal_speed; @@ -1012,7 +1026,10 @@ void Planner::recalculate_trapezoids() { current->final_adv_steps = next_entry_speed * comp; } #endif - CBI(current->flag, BLOCK_BIT_RECALCULATE); // Reset current only to ensure next trapezoid is computed + + // Reset current only to ensure next trapezoid is computed - The + // stepper is free to use the block from now on. + CBI(current->flag, BLOCK_BIT_RECALCULATE); } } @@ -1025,6 +1042,12 @@ void Planner::recalculate_trapezoids() { // Last/newest block in buffer. Exit speed is set with MINIMUM_PLANNER_SPEED. Always recalculated. if (next) { + + // Mark the next(last) block as RECALCULATE, to prevent the Stepper ISR running it. + // As the last block is always recalculated here, there is a chance the block isn't + // marked as RECALCULATE yet. That's the reason for the following line. + SBI(next->flag, BLOCK_BIT_RECALCULATE); + const float next_nominal_speed = SQRT(next->nominal_speed_sqr), nomr = 1.0 / next_nominal_speed; calculate_trapezoid_for_block(next, next_entry_speed * nomr, (MINIMUM_PLANNER_SPEED) * nomr); @@ -1035,6 +1058,9 @@ void Planner::recalculate_trapezoids() { next->final_adv_steps = (MINIMUM_PLANNER_SPEED) * comp; } #endif + + // Reset next only to ensure its trapezoid is computed - The stepper is free to use + // the block from now on. CBI(next->flag, BLOCK_BIT_RECALCULATE); } } diff --git a/Marlin/stepper.h b/Marlin/stepper.h index 35246885d..4526e5d8b 100644 --- a/Marlin/stepper.h +++ b/Marlin/stepper.h @@ -53,7 +53,7 @@ // #ifndef MINIMUM_STEPPER_PULSE - #define MINIMUM_STEPPER_PULSE 0 + #define MINIMUM_STEPPER_PULSE 0UL #endif #ifndef MAXIMUM_STEPPER_RATE @@ -64,101 +64,93 @@ #endif #endif -#ifdef CPU_32_BIT - - // The base ISR takes 792 cycles - #define ISR_BASE_CYCLES 792UL - - // Linear advance base time is 64 cycles - #if ENABLED(LIN_ADVANCE) - #define ISR_LA_BASE_CYCLES 64UL - #else - #define ISR_LA_BASE_CYCLES 0UL - #endif - - // S curve interpolation adds 40 cycles - #if ENABLED(S_CURVE_ACCELERATION) - #define ISR_S_CURVE_CYCLES 40UL - #else - #define ISR_S_CURVE_CYCLES 0UL - #endif - - // Stepper Loop base cycles - #define ISR_LOOP_BASE_CYCLES 4UL - - // And each stepper takes 16 cycles - #define ISR_STEPPER_CYCLES 16UL +// The base ISR takes 752 cycles +#define ISR_BASE_CYCLES 752UL +// Linear advance base time is 32 cycles +#if ENABLED(LIN_ADVANCE) + #define ISR_LA_BASE_CYCLES 32UL #else - - // The base ISR takes 752 cycles - #define ISR_BASE_CYCLES 752UL - - // Linear advance base time is 32 cycles - #if ENABLED(LIN_ADVANCE) - #define ISR_LA_BASE_CYCLES 32UL - #else - #define ISR_LA_BASE_CYCLES 0UL - #endif - - // S curve interpolation adds 160 cycles - #if ENABLED(S_CURVE_ACCELERATION) - #define ISR_S_CURVE_CYCLES 160UL - #else - #define ISR_S_CURVE_CYCLES 0UL - #endif - - // Stepper Loop base cycles - #define ISR_LOOP_BASE_CYCLES 32UL - - // And each stepper takes 88 cycles - #define ISR_STEPPER_CYCLES 88UL - + #define ISR_LA_BASE_CYCLES 0UL #endif +// S curve interpolation adds 160 cycles +#if ENABLED(S_CURVE_ACCELERATION) + #define ISR_S_CURVE_CYCLES 160UL +#else + #define ISR_S_CURVE_CYCLES 0UL +#endif + +// Stepper Loop base cycles +#define ISR_LOOP_BASE_CYCLES 32UL + +// To start the step pulse, in the worst case takes +#define ISR_START_STEPPER_CYCLES 57UL + +// And each stepper (start + stop pulse) takes in worst case +#define ISR_STEPPER_CYCLES 88UL + // Add time for each stepper #ifdef HAS_X_STEP - #define ISR_X_STEPPER_CYCLES ISR_STEPPER_CYCLES + #define ISR_START_X_STEPPER_CYCLES ISR_START_STEPPER_CYCLES + #define ISR_X_STEPPER_CYCLES ISR_STEPPER_CYCLES #else - #define ISR_X_STEPPER_CYCLES 0UL + #define ISR_START_X_STEPPER_CYCLES 0UL + #define ISR_X_STEPPER_CYCLES 0UL #endif #ifdef HAS_Y_STEP - #define ISR_Y_STEPPER_CYCLES ISR_STEPPER_CYCLES + #define ISR_START_Y_STEPPER_CYCLES ISR_START_STEPPER_CYCLES + #define ISR_Y_STEPPER_CYCLES ISR_STEPPER_CYCLES #else - #define ISR_Y_STEPPER_CYCLES 0UL + #define ISR_START_Y_STEPPER_CYCLES 0UL + #define ISR_Y_STEPPER_CYCLES 0UL #endif #ifdef HAS_Z_STEP - #define ISR_Z_STEPPER_CYCLES ISR_STEPPER_CYCLES + #define ISR_START_Z_STEPPER_CYCLES ISR_START_STEPPER_CYCLES + #define ISR_Z_STEPPER_CYCLES ISR_STEPPER_CYCLES #else - #define ISR_Z_STEPPER_CYCLES 0UL + #define ISR_START_Z_STEPPER_CYCLES 0UL + #define ISR_Z_STEPPER_CYCLES 0UL #endif // E is always interpolated, even for mixing extruders -#define ISR_E_STEPPER_CYCLES ISR_STEPPER_CYCLES +#define ISR_START_E_STEPPER_CYCLES ISR_START_STEPPER_CYCLES +#define ISR_E_STEPPER_CYCLES ISR_STEPPER_CYCLES // If linear advance is disabled, then the loop also handles them #if DISABLED(LIN_ADVANCE) && ENABLED(MIXING_EXTRUDER) + #define ISR_START_MIXING_STEPPER_CYCLES ((MIXING_STEPPERS) * (ISR_START_STEPPER_CYCLES)) #define ISR_MIXING_STEPPER_CYCLES ((MIXING_STEPPERS) * (ISR_STEPPER_CYCLES)) #else + #define ISR_START_MIXING_STEPPER_CYCLES 0UL #define ISR_MIXING_STEPPER_CYCLES 0UL #endif +// Calculate the minimum time to start all stepper pulses in the ISR loop +#define MIN_ISR_START_LOOP_CYCLES (ISR_START_X_STEPPER_CYCLES + ISR_START_Y_STEPPER_CYCLES + ISR_START_Z_STEPPER_CYCLES + ISR_START_E_STEPPER_CYCLES + ISR_START_MIXING_STEPPER_CYCLES) + // And the total minimum loop time, not including the base #define MIN_ISR_LOOP_CYCLES (ISR_X_STEPPER_CYCLES + ISR_Y_STEPPER_CYCLES + ISR_Z_STEPPER_CYCLES + ISR_E_STEPPER_CYCLES + ISR_MIXING_STEPPER_CYCLES) // Calculate the minimum MPU cycles needed per pulse to enforce, limited to the max stepper rate -#define _MIN_STEPPER_PULSE_CYCLES(N) max((F_CPU) / (MAXIMUM_STEPPER_RATE), ((F_CPU) / 500000UL) * (N)) +#define _MIN_STEPPER_PULSE_CYCLES(N) MAX((F_CPU) / (MAXIMUM_STEPPER_RATE), ((F_CPU) / 500000UL) * (N)) #if MINIMUM_STEPPER_PULSE - #define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES(MINIMUM_STEPPER_PULSE) + #define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES((MINIMUM_STEPPER_PULSE)) #else - #define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES(1) + #define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES(1UL) #endif -#define MIN_PULSE_TICKS ((PULSE_TIMER_TICKS_PER_US) * (MINIMUM_STEPPER_PULSE)) -#define ADDED_STEP_TICKS ((MIN_STEPPER_PULSE_CYCLES) / (PULSE_TIMER_PRESCALE) - MIN_PULSE_TICKS) +// Calculate the minimum ticks of the PULSE timer that must elapse with the step pulse enabled +// adding the "start stepper pulse" code section execution cycles to account for that not all +// pulses start at the beginning of the loop, so an extra time must be added to compensate so +// the last generated pulse (usually the extruder stepper) has the right length +#define MIN_PULSE_TICKS (((PULSE_TIMER_TICKS_PER_US) * (MINIMUM_STEPPER_PULSE)) + ((MIN_ISR_START_LOOP_CYCLES) / (PULSE_TIMER_PRESCALE))) + +// Calculate the extra ticks of the PULSE timer between step pulses +#define ADDED_STEP_TICKS (((MIN_STEPPER_PULSE_CYCLES) / (PULSE_TIMER_PRESCALE)) - (MIN_PULSE_TICKS)) // But the user could be enforcing a minimum time, so the loop time is -#define ISR_LOOP_CYCLES (ISR_LOOP_BASE_CYCLES + max(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LOOP_CYCLES)) +#define ISR_LOOP_CYCLES (ISR_LOOP_BASE_CYCLES + MAX(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LOOP_CYCLES)) // If linear advance is enabled, then it is handled separately #if ENABLED(LIN_ADVANCE) @@ -171,7 +163,7 @@ #endif // And the real loop time - #define ISR_LA_LOOP_CYCLES max(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LA_LOOP_CYCLES) + #define ISR_LA_LOOP_CYCLES MAX(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LA_LOOP_CYCLES) #else #define ISR_LA_LOOP_CYCLES 0UL