diff --git a/drivers/rknpu/include/rknpu_drv.h b/drivers/rknpu/include/rknpu_drv.h index 3f3804ad7..ed44b76ac 100644 --- a/drivers/rknpu/include/rknpu_drv.h +++ b/drivers/rknpu/include/rknpu_drv.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -100,6 +101,10 @@ struct rknpu_subcore_data { struct rknpu_job *job; int64_t task_num; struct rknpu_timer timer; + /* #patch: progress watchdog. wd_last = last PC completed-task counter seen, + * wd_flat = consecutive samples with no advance. */ + u32 wd_last; + u32 wd_flat; }; /** @@ -159,6 +164,11 @@ struct rknpu_device { struct rknpu_debugger debugger; struct hrtimer timer; ktime_t kt; + /* #patch: progress watchdog timer (armed on submit, self-disarms when idle) */ + struct hrtimer wd_timer; + struct work_struct wd_work; + ktime_t wd_kt; + int wd_on; phys_addr_t sram_start; phys_addr_t sram_end; phys_addr_t nbuf_start; @@ -186,4 +196,6 @@ int rknpu_power_get(struct rknpu_device *rknpu_dev); int rknpu_power_put(struct rknpu_device *rknpu_dev); int rknpu_power_put_delay(struct rknpu_device *rknpu_dev); +void rknpu_wd_arm(struct rknpu_device *rknpu_dev); + #endif /* __LINUX_RKNPU_DRV_H_ */ diff --git a/drivers/rknpu/rknpu_drv.c b/drivers/rknpu/rknpu_drv.c index 4090344b4..fca5c9a22 100644 --- a/drivers/rknpu/rknpu_drv.c +++ b/drivers/rknpu/rknpu_drv.c @@ -37,6 +37,7 @@ #include #endif +#include #include "rknpu_ioctl.h" #include "rknpu_reset.h" #include "rknpu_fence.h" @@ -778,17 +779,164 @@ static enum hrtimer_restart hrtimer_handler(struct hrtimer *timer) return HRTIMER_RESTART; } +/* ================= PROGRESS WATCHDOG (#patch) ========================================== + * WHY. rknpu_job_timeout_clean is not a watchdog: it runs only when ANOTHER submit arrives on + * the same core, so a stalled job with no follow-up submit is never noticed (which is why + * userspace has to fire "reap dummy" submits). It also measures TOTAL JOB AGE, meaningless for + * a PC-chained job whose task list is extensible at runtime. "No task completed in N samples" + * is well defined for a chain of any length. + * + * CONFIRMED FINDING: on a doorbell miss the PC completed-task counter reads 0 with the job many + * ms old -- the job is accepted and NEVER DISPATCHES. (Control: at an 8 ms threshold a healthy + * ~30 ms job does NOT report, so the counter genuinely advances and the offset is right.) + * + * POWER SAFETY -- the hard part, and v1/v2 got it wrong. + * v1 read NPU MMIO straight from the hrtimer. A register read on a powered-down block is an + * external abort => instant HARD hang (no ping, power cycle required). It hung the board on + * its first NPU workload. + * v2 guarded with pm_runtime_get_if_in_use(rknpu_dev->dev). STILL WRONG on RK3588: this driver + * keeps its OWN refcount (power_refcount/power_lock) and rknpu_power_off() puts the three + * PER-CORE genpds (genpd_dev_npu0/1/2) directly. A reference on the parent does not hold those + * up, so core i's domain can still drop between the check and the readl on base[i]. + * v3 (this): sample in PROCESS context and hold power_lock across the read. rknpu_power_off() + * takes the same mutex, so the genpds cannot go down while we read. We deliberately do NOT call + * rknpu_power_get(): that would POWER THE NPU ON just to read a counter and pin it up forever. + * mutex_trylock means the watchdog can never delay a submit; a missed sample is simply skipped + * rather than counted as a no-advance. + * + * The hrtimer only decides cadence and queues the work; all MMIO happens in the worker. */ +static unsigned int rknpu_wd_period_us = 1000; +module_param_named(wd_period_us, rknpu_wd_period_us, uint, 0644); +MODULE_PARM_DESC(wd_period_us, "progress watchdog sample period in us (0 disables)"); + +/* diagnostics: is the watchdog actually sampling, and why not when it isn't */ +static unsigned long rknpu_wd_ticks, rknpu_wd_seen_job, rknpu_wd_pm_skip, rknpu_wd_stalls; +module_param_named(wd_ticks, rknpu_wd_ticks, ulong, 0444); +module_param_named(wd_seen_job, rknpu_wd_seen_job, ulong, 0444); +module_param_named(wd_pm_skip, rknpu_wd_pm_skip, ulong, 0444); +module_param_named(wd_stalls, rknpu_wd_stalls, ulong, 0444); + +static unsigned int rknpu_wd_samples = 64; +module_param_named(wd_samples, rknpu_wd_samples, uint, 0644); +MODULE_PARM_DESC(wd_samples, "consecutive no-advance samples before reporting a stall"); + +/* Process context: safe to take power_lock, so safe to touch MMIO. */ +static void rknpu_wd_work_fn(struct work_struct *work) +{ + struct rknpu_device *rknpu_dev = + container_of(work, struct rknpu_device, wd_work); + struct rknpu_subcore_data *subcore_data = NULL; + struct rknpu_job *job = NULL; + unsigned long flags; + int i; + + /* never block a submit: power_lock is held by rknpu_power_get/put on the hot path */ + if (!mutex_trylock(&rknpu_dev->power_lock)) { + rknpu_wd_pm_skip++; + return; + } + /* powered off => nothing is running and MMIO is unsafe; skip without penalising wd_flat */ + if (atomic_read(&rknpu_dev->power_refcount) <= 0) { + rknpu_wd_pm_skip++; + mutex_unlock(&rknpu_dev->power_lock); + return; + } + + for (i = 0; i < rknpu_dev->config->num_irqs; i++) { + void __iomem *rknpu_core_base = rknpu_dev->base[i]; + u32 cnt; + + subcore_data = &rknpu_dev->subcore_datas[i]; + + spin_lock_irqsave(&rknpu_dev->irq_lock, flags); + job = subcore_data->job; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + + if (!job) { + subcore_data->wd_flat = 0; + continue; + } + rknpu_wd_seen_job++; + + cnt = readl(rknpu_core_base + + rknpu_dev->config->pc_task_status_offset) & + rknpu_dev->config->pc_task_number_mask; + + if (cnt == subcore_data->wd_last) { + /* fire ONCE per stall episode (equality, not >=) */ + if (++subcore_data->wd_flat == rknpu_wd_samples) { + rknpu_wd_stalls++; + LOG_ERROR("RKNPU: core %d NO TASK PROGRESS for %u samples (%u us), task counter stuck at %#x, job age %lld us\n", + i, subcore_data->wd_flat, + subcore_data->wd_flat * rknpu_wd_period_us, + cnt, + ktime_us_delta(ktime_get(), job->timestamp)); + } + } else { + subcore_data->wd_last = cnt; + subcore_data->wd_flat = 0; + } + } + + mutex_unlock(&rknpu_dev->power_lock); +} + +/* Cadence only — NO MMIO here. Self-disarms once no core has an outstanding job. */ +static enum hrtimer_restart rknpu_wd_handler(struct hrtimer *timer) +{ + struct rknpu_device *rknpu_dev = + container_of(timer, struct rknpu_device, wd_timer); + unsigned long flags; + int i, active = 0; + + rknpu_wd_ticks++; + + spin_lock_irqsave(&rknpu_dev->irq_lock, flags); + for (i = 0; i < rknpu_dev->config->num_irqs; i++) + if (rknpu_dev->subcore_datas[i].job) + active++; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + + if (!active) { + rknpu_dev->wd_on = 0; + return HRTIMER_NORESTART; + } + /* a no-op if the previous sample is still queued, which self-throttles */ + queue_work(system_wq, &rknpu_dev->wd_work); + + hrtimer_forward_now(timer, rknpu_dev->wd_kt); + return HRTIMER_RESTART; +} + +/* Armed from the submit path; self-disarms in the handler once no core has a job. Re-arming an + * already-running hrtimer is harmless, so the wd_on race is benign. */ +void rknpu_wd_arm(struct rknpu_device *rknpu_dev) +{ + if (!rknpu_wd_period_us || rknpu_dev->wd_on) + return; + rknpu_dev->wd_on = 1; + rknpu_dev->wd_kt = ktime_set(0, (u64)rknpu_wd_period_us * 1000); + hrtimer_start(&rknpu_dev->wd_timer, rknpu_dev->wd_kt, HRTIMER_MODE_REL); +} + static void rknpu_init_timer(struct rknpu_device *rknpu_dev) { rknpu_dev->kt = ktime_set(0, RKNPU_LOAD_INTERVAL); hrtimer_init(&rknpu_dev->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); rknpu_dev->timer.function = hrtimer_handler; hrtimer_start(&rknpu_dev->timer, rknpu_dev->kt, HRTIMER_MODE_REL); + /* #patch: watchdog timer is initialised but NOT started — armed on submit */ + hrtimer_init(&rknpu_dev->wd_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + rknpu_dev->wd_timer.function = rknpu_wd_handler; + rknpu_dev->wd_on = 0; + INIT_WORK(&rknpu_dev->wd_work, rknpu_wd_work_fn); } static void rknpu_cancel_timer(struct rknpu_device *rknpu_dev) { hrtimer_cancel(&rknpu_dev->timer); + hrtimer_cancel(&rknpu_dev->wd_timer); /* #patch */ + cancel_work_sync(&rknpu_dev->wd_work); /* #patch */ } static bool rknpu_is_iommu_enable(struct device *dev)