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.

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

Jump to bottom

Guard time_spent_in/out_isr with MULTISTEPPING_LIMIT > 1 #27147

Open
wants to merge 1 commit into
base: bugfix-2.1.x
Choose a base branch
from
+25 −38
Open

Guard time_spent_in/out_isr with MULTISTEPPING_LIMIT > 1 #27147

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
56 changes: 22 additions & 34 deletions Marlin/src/module/stepper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,10 @@ uint32_t Stepper::acceleration_time, Stepper::deceleration_time;

#if MULTISTEPPING_LIMIT > 1
uint8_t Stepper::steps_per_isr = 1; // Count of steps to perform per Stepper ISR call
#endif

#if DISABLED(OLD_ADAPTIVE_MULTISTEPPING)
hal_timer_t Stepper::time_spent_in_isr = 0, Stepper::time_spent_out_isr = 0;
#if DISABLED(OLD_ADAPTIVE_MULTISTEPPING)
hal_timer_t Stepper::time_spent_in_isr = 0, Stepper::time_spent_out_isr = 0;
#endif
#endif

#if ENABLED(ADAPTIVE_STEP_SMOOTHING)
Expand Down Expand Up @@ -1671,21 +1671,12 @@ void Stepper::isr() {
*/
min_ticks = HAL_timer_get_count(MF_TIMER_STEP) + hal_timer_t(TERN(__AVR__, 8, 1) * (STEPPER_TIMER_TICKS_PER_US));

#if ENABLED(OLD_ADAPTIVE_MULTISTEPPING)
/**
* NB: If for some reason the stepper monopolizes the MPU, eventually the
* timer will wrap around (and so will 'next_isr_ticks'). So, limit the
* loop to 10 iterations. Beyond that, there's no way to ensure correct pulse
* timing, since the MCU isn't fast enough.
*/
if (!--max_loops) next_isr_ticks = min_ticks;
#endif

// Advance pulses if not enough time to wait for the next ISR
} while (TERN(OLD_ADAPTIVE_MULTISTEPPING, true, --max_loops) && next_isr_ticks < min_ticks);

#if DISABLED(OLD_ADAPTIVE_MULTISTEPPING)
// Advance pulses if not enough time to wait for the next ISR or we've hit the loop limit.
// We limit to max_loop iterations to avoid timer (and 'next_isr_ticks') overflow and to
// avoid this (high priority) ISR completely monopolizing the MPU.
} while (--max_loops && next_isr_ticks < min_ticks);

#if DISABLED(OLD_ADAPTIVE_MULTISTEPPING) && MULTISTEPPING_LIMIT > 1
// Track the time spent in the ISR
const hal_timer_t time_spent = HAL_timer_get_count(MF_TIMER_STEP);
time_spent_in_isr += time_spent;
Expand All @@ -1694,21 +1685,20 @@ void Stepper::isr() {
next_isr_ticks = min_ticks;

// When forced out of the ISR, increase multi-stepping
#if MULTISTEPPING_LIMIT > 1
if (steps_per_isr < MULTISTEPPING_LIMIT) {
steps_per_isr <<= 1;
// ticks_nominal will need to be recalculated if we are in cruise phase
ticks_nominal = 0;
}
#endif
if (steps_per_isr < MULTISTEPPING_LIMIT) {
steps_per_isr <<= 1;
// ticks_nominal will need to be recalculated if we are in cruise phase
ticks_nominal = 0;
}
}
else {
// Track the time spent voluntarily outside the ISR
time_spent_out_isr += next_isr_ticks;
time_spent_out_isr -= time_spent;
}

#endif // !OLD_ADAPTIVE_MULTISTEPPING
#else
NOLESS(next_isr_ticks, min_ticks);
#endif // DISABLED(OLD_ADAPTIVE_MULTISTEPPING) && MULTISTEPPING_LIMIT > 1

// Now 'next_isr_ticks' contains the period to the next Stepper ISR - And we are
// sure that the time has not arrived yet - Warrantied by the scheduler
Expand Down Expand Up @@ -2365,16 +2355,14 @@ void Stepper::set_axis_moved_for_current_block() {
* have been done, so it is less time critical.
*/
hal_timer_t Stepper::block_phase_isr() {
#if DISABLED(OLD_ADAPTIVE_MULTISTEPPING)
#if DISABLED(OLD_ADAPTIVE_MULTISTEPPING) && MULTISTEPPING_LIMIT > 1
// If the ISR uses < 50% of MPU time, halve multi-stepping
const hal_timer_t time_spent = HAL_timer_get_count(MF_TIMER_STEP);
#if MULTISTEPPING_LIMIT > 1
if (steps_per_isr > 1 && time_spent_out_isr >= time_spent_in_isr + time_spent) {
steps_per_isr >>= 1;
// ticks_nominal will need to be recalculated if we are in cruise phase
ticks_nominal = 0;
}
#endif
if (steps_per_isr > 1 && time_spent_out_isr >= time_spent_in_isr + time_spent) {
steps_per_isr >>= 1;
// ticks_nominal will need to be recalculated if we are in cruise phase
ticks_nominal = 0;
}
time_spent_in_isr = -time_spent; // unsigned but guaranteed to be +ve when needed
time_spent_out_isr = 0;
#endif
Expand Down
7 changes: 3 additions & 4 deletions Marlin/src/module/stepper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,10 +374,9 @@ class Stepper {
static constexpr uint8_t steps_per_isr = 1; // Count of steps to perform per Stepper ISR call
#else
static uint8_t steps_per_isr;
#endif

#if DISABLED(OLD_ADAPTIVE_MULTISTEPPING)
static hal_timer_t time_spent_in_isr, time_spent_out_isr;
#if DISABLED(OLD_ADAPTIVE_MULTISTEPPING)
static hal_timer_t time_spent_in_isr, time_spent_out_isr;
#endif
#endif

#if ENABLED(ADAPTIVE_STEP_SMOOTHING)
Expand Down
Loading