dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Mikko Perttunen <cyndis@kapsi.fi>
To: Dmitry Osipenko <digetx@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jon Hunter <jonathanh@nvidia.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	sumit.semwal@linaro.org, gustavo@padovan.org
Cc: "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	talho@nvidia.com, bhuntsman@nvidia.com,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Erik Faye-Lund <kusmabite@gmail.com>
Subject: Re: [RFC] Host1x/TegraDRM UAPI (sync points)
Date: Wed, 8 Jul 2020 13:06:08 +0300	[thread overview]
Message-ID: <d3a6c005-0d85-e7b9-41a2-3d86595efbc1@kapsi.fi> (raw)
In-Reply-To: <a2b1f9cf-cc0b-1a0f-d0ac-addcb1f28502@gmail.com>

On 7/7/20 2:06 PM, Dmitry Osipenko wrote:
> 02.07.2020 15:10, Mikko Perttunen пишет:
>> Ok, so we would have two kinds of syncpoints for the job; one
>> for kernel job tracking; and one that userspace can
>> manipulate as it wants to.
>>
>> Could we handle the job tracking syncpoint completely inside the kernel,
>> i.e. allocate it in kernel during job submission, and add an increment
>> for it at the end of the job (with condition OP_DONE)? For MLOCKing, the
>> kernel already needs to insert a SYNCPT_INCR(OP_DONE) + WAIT +
>> MLOCK_RELEASE sequence at the end of each job.
> 
> If sync point is allocated within kernel, then we'll need to always
> patch all job's sync point increments with the ID of the allocated sync
> point, regardless of whether firewall enabled or not.

The idea was that the job tracking increment would also be added to the 
pushbuffer in the kernel, so gathers would only have increments for the 
"user syncpoints", if any. I think this should work for THI-based 
engines (e.g. VIC), you probably have better information about 
GR2D/GR3D. On newer Tegras we could use CHANNEL/APPID protection to 
prevent the gathers from incrementing these job tracking syncpoints.

> 
> Secondly, I'm now recalling that only one sync point could be assigned
> to a channel at a time on newer Tegras which support sync point
> protection. So it sounds like we don't really have variants other than
> to allocate one sync point per channel for the jobs usage if we want to
> be able to push multiple jobs into channel's pushbuffer, correct?
> 

The other way around; each syncpoint can be assigned to one channel. One 
channel may have multiple syncpoints.

> ...
>>> Hmm, we actually should be able to have a one sync point per-channel for
>>> the job submission, similarly to what the current driver does!
>>>
>>> I'm keep forgetting about the waitbase existence!
>>
>> Tegra194 doesn't have waitbases, but if we are resubmitting all the jobs
>> anyway, can't we just recalculate wait thresholds at that time?
> 
> Yes, thresholds could be recalculated + job should be re-formed at the
> push-time then.
> 
> It also means that jobs always should be formed only at the push-time if
> wait-command is utilized by cmdstream since the waits always need to be
> patched because we won't know the thresholds until scheduler actually
> runs the job.

Could we keep the job tracking syncpoints entirely within the kernel, 
and have all wait commands and other stuff that userspace does use the 
user syncpoints? Then kernel job tracking and userspace activity would 
be separate from each other.

Alternatively, if we know that jobs can only be removed from the middle 
of pushbuffers, and not added, we could replace any removed jobs with 
syncpoint increments in the pushbuffer and any thresholds would stay intact.

> 
>> Maybe a more detailed sequence list or diagram of what happens during
>> submission and recovery would be useful.
> 
> The textual form + code is already good enough to me. A diagram could be
> nice to have, although it may take a bit too much effort to create +
> maintain it. But I don't mind at all if you'd want to make one :)
> 
> ...
>>>> * We should be able to keep the syncpoint refcounting based on fences.
>>>
>>> The fence doesn't need the sync point itself, it only needs to get a
>>> signal when the threshold is reached or when sync point is ceased.
>>>
>>> Imagine:
>>>
>>>     - Process A creates sync point
>>>     - Process A creates dma-fence from this sync point
>>>     - Process A exports dma-fence to process B
>>>     - Process A dies
>>>
>>> What should happen to process B?
>>>
>>>     - Should dma-fence of the process B get a error signal when process A
>>> dies?
>>>     - Should process B get stuck waiting endlessly for the dma-fence?
>>>
>>> This is one example of why I'm proposing that fence shouldn't be coupled
>>> tightly to a sync point.
>>
>> As a baseline, we should consider process B to get stuck endlessly
>> (until a timeout of its choosing) for the fence. In this case it is
>> avoidable, but if the ID/threshold pair is exported out of the fence and
>> is waited for otherwise, it is unavoidable. I.e. once the ID/threshold
>> are exported out of a fence, the waiter can only see the fence being
>> signaled by the threshold being reached, not by the syncpoint getting
>> freed.
> 
> This is correct. If sync point's FD is exported or once sync point is
> resolved from a dma-fence, then sync point will stay alive until the
> last reference to the sync point is dropped. I.e. if Process A dies
> *after* process B started to wait on the sync point, then it will get stuck.
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-07-08 10:06 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23 12:09 [RFC] Host1x/TegraDRM UAPI Mikko Perttunen
2020-06-24 20:55 ` Dmitry Osipenko
2020-06-25  9:17   ` Mikko Perttunen
2020-06-24 22:33 ` Dmitry Osipenko
2020-06-25  9:27   ` Mikko Perttunen
2020-06-25 22:50     ` [RFC] Host1x/TegraDRM UAPI (drm_tegra_submit_relocation) Dmitry Osipenko
2020-06-26  9:01       ` Mikko Perttunen
2020-06-24 23:11 ` [RFC] Host1x/TegraDRM UAPI Dmitry Osipenko
2020-06-25  9:16   ` Mikko Perttunen
2020-06-25 23:24     ` Dmitry Osipenko
2020-06-26  9:05       ` Mikko Perttunen
2020-06-24 23:18 ` Dmitry Osipenko
2020-06-25  0:59   ` Dmitry Osipenko
2020-06-24 23:23 ` Dmitry Osipenko
2020-06-25  9:19   ` Mikko Perttunen
2020-06-25  0:47 ` Dmitry Osipenko
2020-06-25  9:23   ` Mikko Perttunen
2020-06-25 22:47 ` [RFC] Host1x/TegraDRM UAPI (drm_tegra_channel_map) Dmitry Osipenko
2020-06-26  7:34   ` Thierry Reding
2020-06-26 16:35     ` Dmitry Osipenko
2020-06-28 11:16       ` Mikko Perttunen
2020-06-28 22:59         ` Dmitry Osipenko
2020-06-30 10:55           ` Mikko Perttunen
2020-06-30 19:48             ` Dmitry Osipenko
2020-06-26 11:06 ` [RFC] Host1x/TegraDRM UAPI Karol Herbst
2020-06-26 11:13   ` Mikko Perttunen
2020-06-26 11:16     ` Karol Herbst
2020-06-26 11:40   ` Thierry Reding
2020-06-26 13:38     ` Daniel Vetter
2020-06-26 13:59       ` Dmitry Osipenko
2020-06-30  9:09         ` Daniel Vetter
2020-06-27 21:47 ` [RFC] Host1x/TegraDRM UAPI (drm_tegra_submit_syncpt_incr) Dmitry Osipenko
2020-06-28 11:10   ` Mikko Perttunen
2020-06-27 22:32 ` [RFC] Host1x/TegraDRM UAPI (drm_tegra_submit_command) Dmitry Osipenko
2020-06-28 10:28   ` Mikko Perttunen
2020-06-29  0:00     ` Dmitry Osipenko
2020-06-30 10:40       ` Mikko Perttunen
2020-06-27 23:27 ` [RFC] Host1x/TegraDRM UAPI (sync points) Dmitry Osipenko
2020-06-28  9:44   ` Mikko Perttunen
2020-06-29  2:36     ` Dmitry Osipenko
2020-06-29 10:27       ` Mikko Perttunen
2020-06-29 19:28         ` Dmitry Osipenko
2020-06-29 19:33         ` Dmitry Osipenko
2020-06-29 19:42         ` Dmitry Osipenko
2020-06-30 10:26           ` Mikko Perttunen
2020-07-01  0:22             ` Dmitry Osipenko
2020-07-02 12:10               ` Mikko Perttunen
2020-07-07 11:06                 ` Dmitry Osipenko
2020-07-08 10:06                   ` Mikko Perttunen [this message]
2020-07-09  9:28                     ` Dmitry Osipenko

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=d3a6c005-0d85-e7b9-41a2-3d86595efbc1@kapsi.fi \
    --to=cyndis@kapsi.fi \
    --cc=airlied@linux.ie \
    --cc=bhuntsman@nvidia.com \
    --cc=daniel@ffwll.ch \
    --cc=digetx@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gustavo@padovan.org \
    --cc=jonathanh@nvidia.com \
    --cc=kusmabite@gmail.com \
    --cc=linux-tegra@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=talho@nvidia.com \
    --cc=thierry.reding@gmail.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).