On Wed, Sep 30, 2020 at 11:16 PM Rob Clark robdclark@gmail.com wrote:
From: Rob Clark robdclark@chromium.org
The android userspace treats the display pipeline as a realtime problem. And arguably, if your goal is to not miss frame deadlines (ie. vblank), it is. (See https://lwn.net/Articles/809545/ for the best explaination that I found.)
But this presents a problem with using workqueues for non-blocking atomic commit_work(), because the SCHED_FIFO userspace thread(s) can preempt the worker. Which is not really the outcome you want.. once the required fences are scheduled, you want to push the atomic commit down to hw ASAP.
But the decision of whether commit_work should be RT or not really depends on what userspace is doing. For a pure CFS userspace display pipeline, commit_work() should remain SCHED_NORMAL.
To handle this, convert non-blocking commit_work() to use per-CRTC kthread workers, instead of system_unbound_wq. Per-CRTC workers are used to avoid serializing commits when userspace is using a per-CRTC update loop. And the last patch exposes the task id to userspace as a CRTC property, so that userspace can adjust the priority and sched policy to fit it's needs.
v2: Drop client cap and in-kernel setting of priority/policy in favor of exposing the kworker tid to userspace so that user- space can set priority/policy.
Yeah I think this looks more reasonable. Still a bit irky interface, so I'd like to get some kworker/rt ack on this. Other opens: - needs userspace, the usual drill - we need this also for vblank workers, otherwise this wont work for drivers needing those because of another priority inversion. - we probably want some indication of whether this actually does something useful, not all drivers use atomic commit helpers. Not sure how to do that. - not sure whether the vfunc is an awesome idea, I'd frankly just open-code this inline. We have similar special cases already for e.g. dpms (and in multiple places), this isn't the worst. - still feeling we could at least change the default to highpriority niceness. - there's still the problem that commit works can overlap, and a single worker can't do that anymore. So rolling that out for everyone as-is feels a bit risky.
Cheers, Daniel
Rob Clark (3): drm/crtc: Introduce per-crtc kworker drm/atomic: Use kthread worker for nonblocking commits drm: Expose CRTC's kworker task id
drivers/gpu/drm/drm_atomic_helper.c | 13 ++++++++---- drivers/gpu/drm/drm_crtc.c | 14 +++++++++++++ drivers/gpu/drm/drm_mode_config.c | 14 +++++++++++++ drivers/gpu/drm/drm_mode_object.c | 4 ++++ include/drm/drm_atomic.h | 31 +++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 8 ++++++++ include/drm/drm_mode_config.h | 9 +++++++++ include/drm/drm_property.h | 9 +++++++++ 8 files changed, 98 insertions(+), 4 deletions(-)
-- 2.26.2