video: rockchip: mpp: report zero load for idle devices

[rpardini]

The per-device statistics shown in /proc/mpp_service/load are only ever recomputed inside mpp_dev_load(), which is called exclusively from the task-completion paths (mpp_common.c task worker and the rkvdec2 link/ccu workers). Within that function the load and utilization figures are only refreshed when a task finishes and at least one load_interval has elapsed; the result is stored in mpp->load_info and reset for the next window. There is no timer, runtime-PM hook, or reader-side logic that ages the value out, so mpp->load_info.load is a snapshot of the last completed interval, not a live measurement.

As a consequence, once a device stops receiving tasks its stored load is never updated again and /proc/mpp_service/load keeps reporting the last busy interval indefinitely (until load_interval is toggled, which clears the stats via mpp_dev_load_clear()).

This is most visible on the standalone AV1 decoder (fdc70000.av1d). It is driven one task at a time by the default worker, so when playback stops the final frame's completion is genuinely the last event that will ever call mpp_dev_load() for that core, and its load freezes at the busy value. Multi-core rkvdec2 decoders in link/ccu mode keep draining their queued task list after playback ends, which happens to log a further, near-idle interval and pulls the figure back down, so the staleness goes unnoticed there. The underlying defect is common to every device.

Rather than introduce a periodic recompute (a per-core timer with the associated runtime-PM interactions), detect the idle case in the reader. A device whose load_info has not been updated for more than one full load_interval has, by definition, completed no task in that window and is idle; report zero for it instead of the stale snapshot. Devices that never started load tracking (!load_en) are likewise reported as zero.

---

Some eye candy from my WiP node-exporter RKNPU/RKMPP collector + Prometheus + Grafana:

---

Maybe for @nyanmisaka ?

[coderabbitai]

Walkthrough

mpp_show_device_load now reads load and utilization values through local variables, checks whether reporting is enabled and whether the snapshot exceeds twice the configured load interval, and reports zeroes for disabled or stale statistics. Fresh statistics retain the existing output format.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: amazingfate, nyanmisaka

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: reporting zero load for idle Rockchip MPP devices.
Description check	✅ Passed	The description directly explains the idle-load reporting fix and matches the changed behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

drivers/video/rockchip/mpp/mpp_service.c (1)

351-354: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Consider evaluating ktime_get() once outside the loop.

Calling ktime_get() inside the inner loop queries the current time for each core individually. Taking a single snapshot of ktime_get() before iterating through the devices and cores would ensure a perfectly consistent timestamp across all evaluated statistics and provide a marginal performance optimization.

♻️ Proposed refactor

Declare and initialize a single timestamp before the outer loop:

	ktime_t now;
	// ...
	if (!srv->load_interval) {
		seq_puts(file, "please set load_interval first!!!\n");
		// ...
		return 0;
	}

	now = ktime_get();
	for (i = 0; i < MPP_DEVICE_BUTT; i++) {

Then, use now instead of ktime_get() in the staleness check:

 			if (!mpp->load_en ||
-			    ktime_us_delta(ktime_get(), load_info->load_time) >
+			    ktime_us_delta(now, load_info->load_time) >
 			    (s64)srv->load_interval * 1000 * 2)
 				load = load_frac = util = util_frac = 0;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@drivers/video/rockchip/mpp/mpp_service.c` around lines 351 - 354, In the
statistics routine containing the outer device loop and the load staleness
check, declare a ktime_t snapshot and assign it once after validating
load_interval and before iterating devices. Replace the inner-loop ktime_get()
call in the mpp->load_en condition with that shared timestamp, preserving the
existing staleness threshold and load reset behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@drivers/video/rockchip/mpp/mpp_service.c`:
- Around line 351-354: In the statistics routine containing the outer device
loop and the load staleness check, declare a ktime_t snapshot and assign it once
after validating load_interval and before iterating devices. Replace the
inner-loop ktime_get() call in the mpp->load_en condition with that shared
timestamp, preserving the existing staleness threshold and load reset behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5443ce05-eee2-423d-9bec-233668ef3fa1

📥 Commits

Reviewing files that changed from the base of the PR and between 5280f9b and 722dd94.

📒 Files selected for processing (1)

• drivers/video/rockchip/mpp/mpp_service.c

[nyanmisaka]

I noticed this cosmetic issue a long time ago. Since it doesn't affect actual decoding functionality, I didn't look into it further. It would be best to open an issue in the mpp repository as well to let them know.

https://github.com/HermanChen/mpp/issues
https://github.com/rockchip-linux/mpp/issues