On 09/30/20 14:17, Rob Clark 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.
For me thees 2 properties
1. Run ASAP 2. Finish the work un-interrupted
Scream the workers need to be SCHED_FIFO by default. CFS can't give you these guarantees.
IMO using sched_set_fifo() for these workers is the right thing.
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.
I'm not sure I agree with this. I think it's better to characterize tasks based on their properties/requirements rather than what the rest of the userspace is using.
I do appreciate that maybe some of these tasks have varying requirements during their life time. e.g: they have RT property during specific critical section but otherwise are CFS tasks. I think the UI thread in Android behaves like that.
It's worth IMO trying that approach I pointed out earlier to see if making RT try to pick an idle CPU rather than preempt CFS helps. Not sure if it'd be accepted but IMHO it's a better direction to consider and discuss.
Or maybe you can wrap userspace pipeline critical section lock such that any task holding it will automatically be promoted to SCHED_FIFO and then demoted to CFS once it releases it.
Haven't worked with display pipelines before, so hopefully this makes sense :-)
Thanks
-- Qais Yousef
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.
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