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?
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