From: Rob Clark <robdclark@gmail.com> To: "Ville Syrjälä" <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel@ffwll.ch>, Rob Clark <robdclark@chromium.org>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>, Tim Murray <timmurray@google.com>, dri-devel <dri-devel@lists.freedesktop.org>, Tejun Heo <tj@kernel.org>, Qais Yousef <qais.yousef@arm.com> Subject: Re: [PATCH v2 0/3] drm: commit_work scheduling Date: Fri, 16 Oct 2020 09:27:39 -0700 [thread overview] Message-ID: <CAF6AEGvC+t8RvFRrpWaZGNMNMyz8oY0Y3LzGANHJ-SeDYoXw7Q@mail.gmail.com> (raw) In-Reply-To: <20201008082411.GE6112@intel.com> On Thu, Oct 8, 2020 at 1:24 AM Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Wed, Oct 07, 2020 at 09:44:09AM -0700, Rob Clark wrote: > > On Mon, Oct 5, 2020 at 5:15 AM Ville Syrjälä > > <ville.syrjala@linux.intel.com> wrote: > > > > > > On Fri, Oct 02, 2020 at 10:55:52AM -0700, Rob Clark wrote: > > > > On Fri, Oct 2, 2020 at 4:05 AM Ville Syrjälä > > > > <ville.syrjala@linux.intel.com> wrote: > > > > > > > > > > On Fri, Oct 02, 2020 at 01:52:56PM +0300, Ville Syrjälä wrote: > > > > > > On Thu, Oct 01, 2020 at 05:25:55PM +0200, Daniel Vetter wrote: > > > > > > > On Thu, Oct 1, 2020 at 5:15 PM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > > > > > > > > > I'm leaning towards converting the other drivers over to use the > > > > > > > > per-crtc kwork, and then dropping the 'commit_work` from atomic state. > > > > > > > > I can add a patch to that, but figured I could postpone that churn > > > > > > > > until there is some by-in on this whole idea. > > > > > > > > > > > > > > i915 has its own commit code, it's not even using the current commit > > > > > > > helpers (nor the commit_work). Not sure how much other fun there is. > > > > > > > > > > > > I don't think we want per-crtc threads for this in i915. Seems > > > > > > to me easier to guarantee atomicity across multiple crtcs if > > > > > > we just commit them from the same thread. > > > > > > > > > > Oh, and we may have to commit things in a very specific order > > > > > to guarantee the hw doesn't fall over, so yeah definitely per-crtc > > > > > thread is a no go. > > > > > > > > If I'm understanding the i915 code, this is only the case for modeset > > > > commits? I suppose we could achieve the same result by just deciding > > > > to pick the kthread of the first CRTC for modeset commits. I'm not > > > > really so much concerned about parallelism for modeset. > > > > > > I'm not entirely happy about the random differences between modesets > > > and other commits. Ideally we wouldn't need any. > > > > > > Anyways, even if we ignore modesets we still have the issue with > > > atomicity guarantees across multiple crtcs. So I think we still > > > don't want per-crtc threads, rather it should be thread for each > > > commit. > > > > I don't really see any other way to solve the priority inversion other > > than per-CRTC kthreads. > > What's the problem with just something like a dedicated commit > thread pool? partly, I was trying to avoid re-implementing workqueue. And partly the thread-pool approach seems like it would be harder for userspace to find the tasks which need priority adjustment. But each CRTC is essentially a FIFO, pageflip N+1 on a given CRTC will happen after pageflip N. BR, -R > > I've been thinking about it a bit more, and > > my conclusion is: > > > > (1) There isn't really any use for the N+1'th commit to start running > > before the kthread_work for the N'th commit completes, so I don't mind > > losing the unbound aspect of the workqueue approach > > (2) For cases where there does need to be serialization between > > commits on different CRTCs, since there is a per-CRTC kthread, you > > could achieve this with locking > > > > Since i915 isn't using the atomic helpers here, I suppose it is an > > option for i915 to just continue doing what it is doing. > > > > And I could ofc just stop using the atomic commit helper and do the > > kthreads thing in msm. But my first preference would be that the > > commit helper does generally the right thing. > > > > BR, > > -R > > -- > Ville Syrjälä > Intel
WARNING: multiple messages have this Message-ID (diff)
From: Rob Clark <robdclark@gmail.com> To: "Ville Syrjälä" <ville.syrjala@linux.intel.com> Cc: Rob Clark <robdclark@chromium.org>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>, Tim Murray <timmurray@google.com>, dri-devel <dri-devel@lists.freedesktop.org>, Tejun Heo <tj@kernel.org>, Qais Yousef <qais.yousef@arm.com> Subject: Re: [PATCH v2 0/3] drm: commit_work scheduling Date: Fri, 16 Oct 2020 09:27:39 -0700 [thread overview] Message-ID: <CAF6AEGvC+t8RvFRrpWaZGNMNMyz8oY0Y3LzGANHJ-SeDYoXw7Q@mail.gmail.com> (raw) In-Reply-To: <20201008082411.GE6112@intel.com> On Thu, Oct 8, 2020 at 1:24 AM Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Wed, Oct 07, 2020 at 09:44:09AM -0700, Rob Clark wrote: > > On Mon, Oct 5, 2020 at 5:15 AM Ville Syrjälä > > <ville.syrjala@linux.intel.com> wrote: > > > > > > On Fri, Oct 02, 2020 at 10:55:52AM -0700, Rob Clark wrote: > > > > On Fri, Oct 2, 2020 at 4:05 AM Ville Syrjälä > > > > <ville.syrjala@linux.intel.com> wrote: > > > > > > > > > > On Fri, Oct 02, 2020 at 01:52:56PM +0300, Ville Syrjälä wrote: > > > > > > On Thu, Oct 01, 2020 at 05:25:55PM +0200, Daniel Vetter wrote: > > > > > > > On Thu, Oct 1, 2020 at 5:15 PM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > > > > > > > > > I'm leaning towards converting the other drivers over to use the > > > > > > > > per-crtc kwork, and then dropping the 'commit_work` from atomic state. > > > > > > > > I can add a patch to that, but figured I could postpone that churn > > > > > > > > until there is some by-in on this whole idea. > > > > > > > > > > > > > > i915 has its own commit code, it's not even using the current commit > > > > > > > helpers (nor the commit_work). Not sure how much other fun there is. > > > > > > > > > > > > I don't think we want per-crtc threads for this in i915. Seems > > > > > > to me easier to guarantee atomicity across multiple crtcs if > > > > > > we just commit them from the same thread. > > > > > > > > > > Oh, and we may have to commit things in a very specific order > > > > > to guarantee the hw doesn't fall over, so yeah definitely per-crtc > > > > > thread is a no go. > > > > > > > > If I'm understanding the i915 code, this is only the case for modeset > > > > commits? I suppose we could achieve the same result by just deciding > > > > to pick the kthread of the first CRTC for modeset commits. I'm not > > > > really so much concerned about parallelism for modeset. > > > > > > I'm not entirely happy about the random differences between modesets > > > and other commits. Ideally we wouldn't need any. > > > > > > Anyways, even if we ignore modesets we still have the issue with > > > atomicity guarantees across multiple crtcs. So I think we still > > > don't want per-crtc threads, rather it should be thread for each > > > commit. > > > > I don't really see any other way to solve the priority inversion other > > than per-CRTC kthreads. > > What's the problem with just something like a dedicated commit > thread pool? partly, I was trying to avoid re-implementing workqueue. And partly the thread-pool approach seems like it would be harder for userspace to find the tasks which need priority adjustment. But each CRTC is essentially a FIFO, pageflip N+1 on a given CRTC will happen after pageflip N. BR, -R > > I've been thinking about it a bit more, and > > my conclusion is: > > > > (1) There isn't really any use for the N+1'th commit to start running > > before the kthread_work for the N'th commit completes, so I don't mind > > losing the unbound aspect of the workqueue approach > > (2) For cases where there does need to be serialization between > > commits on different CRTCs, since there is a per-CRTC kthread, you > > could achieve this with locking > > > > Since i915 isn't using the atomic helpers here, I suppose it is an > > option for i915 to just continue doing what it is doing. > > > > And I could ofc just stop using the atomic commit helper and do the > > kthreads thing in msm. But my first preference would be that the > > commit helper does generally the right thing. > > > > BR, > > -R > > -- > Ville Syrjälä > Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-10-16 16:28 UTC|newest] Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-30 21:17 [PATCH v2 0/3] drm: commit_work scheduling Rob Clark 2020-09-30 21:17 ` Rob Clark 2020-09-30 21:17 ` [PATCH v2 1/3] drm/crtc: Introduce per-crtc kworker Rob Clark 2020-09-30 21:17 ` Rob Clark 2020-09-30 21:17 ` [PATCH v2 2/3] drm/atomic: Use kthread worker for nonblocking commits Rob Clark 2020-09-30 21:17 ` Rob Clark 2020-09-30 21:17 ` [PATCH v2 3/3] drm: Expose CRTC's kworker task id Rob Clark 2020-09-30 21:17 ` Rob Clark 2020-10-01 7:25 ` [PATCH v2 0/3] drm: commit_work scheduling Daniel Vetter 2020-10-01 7:25 ` Daniel Vetter 2020-10-01 15:15 ` Rob Clark 2020-10-01 15:15 ` Rob Clark 2020-10-01 15:25 ` Daniel Vetter 2020-10-01 15:25 ` Daniel Vetter 2020-10-02 10:52 ` Ville Syrjälä 2020-10-02 10:52 ` Ville Syrjälä 2020-10-02 11:05 ` Ville Syrjälä 2020-10-02 11:05 ` Ville Syrjälä 2020-10-02 17:55 ` Rob Clark 2020-10-02 17:55 ` Rob Clark 2020-10-05 12:15 ` Ville Syrjälä 2020-10-05 12:15 ` Ville Syrjälä 2020-10-05 14:15 ` Daniel Vetter 2020-10-05 14:15 ` Daniel Vetter 2020-10-05 22:58 ` Rob Clark 2020-10-05 22:58 ` Rob Clark 2020-10-07 16:44 ` Rob Clark 2020-10-07 16:44 ` Rob Clark 2020-10-08 8:24 ` Ville Syrjälä 2020-10-08 8:24 ` Ville Syrjälä 2020-10-16 16:27 ` Rob Clark [this message] 2020-10-16 16:27 ` Rob Clark 2020-10-02 11:01 ` Qais Yousef 2020-10-02 11:01 ` Qais Yousef 2020-10-02 18:07 ` Rob Clark 2020-10-02 18:07 ` Rob Clark 2020-10-05 15:00 ` Qais Yousef 2020-10-05 15:00 ` Qais Yousef 2020-10-05 23:24 ` Rob Clark 2020-10-05 23:24 ` Rob Clark 2020-10-06 9:08 ` Daniel Vetter 2020-10-06 9:08 ` Daniel Vetter 2020-10-06 10:01 ` Peter Zijlstra 2020-10-06 10:01 ` Peter Zijlstra 2020-10-06 10:59 ` Qais Yousef 2020-10-06 10:59 ` Qais Yousef 2020-10-06 20:04 ` Rob Clark 2020-10-06 20:04 ` Rob Clark 2020-10-07 10:36 ` Qais Yousef 2020-10-07 10:36 ` Qais Yousef 2020-10-07 15:57 ` Rob Clark 2020-10-07 15:57 ` Rob Clark 2020-10-07 16:30 ` Qais Yousef 2020-10-07 16:30 ` Qais Yousef 2020-10-08 9:10 ` Daniel Vetter 2020-10-08 9:10 ` Daniel Vetter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAF6AEGvC+t8RvFRrpWaZGNMNMyz8oY0Y3LzGANHJ-SeDYoXw7Q@mail.gmail.com \ --to=robdclark@gmail.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=qais.yousef@arm.com \ --cc=robdclark@chromium.org \ --cc=timmurray@google.com \ --cc=tj@kernel.org \ --cc=ville.syrjala@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.