mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: phasta@kernel.org,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Matthew Brost" <matthew.brost@intel.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Christian König" <ckoenig.leichtzumerken@gmail.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>
Cc: dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org
Subject: Re: [PATCH] drm/sched: Extend and update documentation
Date: Mon, 11 Aug 2025 16:17:20 +0200	[thread overview]
Message-ID: <7f620860-bd4d-4b14-9832-2fa25fd4b086@amd.com> (raw)
In-Reply-To: <90c89caeb8ec3ac0fcae583df722bad20fa72827.camel@mailbox.org>

On 11.08.25 11:50, Philipp Stanner wrote:
>>>>
>>>>> But the original draft from Christian hinted at that. So, @Christian,
>>>>> this would be an opportunity to discuss this matter.
>>>>>
>>>>> Otherwise I'd drop this docu section in v2. What users don't know, they
>>>>> cannot misuse.
>>>>
>>>> I would rather like to keep that to avoid misusing the job as the object for tracking the submission lifetime.
>>>
>>> Why would a driver ever want to access struct drm_sched_fence? The
>>> driver knows when it signaled the hardware fence, and it knows when its
>>> callbacks run_job() and free_job() were invoked.
>>>
>>> I'm open to learn what amdgpu does there and why.
>>
>> The simplest use case is performance optimization. You sometimes have submissions which ideally run with others at the same time.
>>
>> So we have AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES which basically tries to cast a fence to a scheduler fence and then only waits for the dependency to be pushed to the HW instead of waiting for it to finish (see amdgpu_cs.c).
> 
> But the driver recognizes that a certain fence got / gets pushed right
> now through backend_ops.run_job(), doesn't it?

Yeah, but how does that helps?

>>
>> Another example are gang submissions (where I still have the TODO to actually fix the code to not crash in an OOM situation).
>>
>> Here we have a gang leader and gang members which are guaranteed to run together on the HW at the same time.
>>
>> This works by adding scheduled dependencies to the gang leader so that the scheduler pushes it to the HW only after all gang members have been pushed.
>>
>> The first gang member pushed now triggers a dependency handling which makes sure that no other gang can be pushed until gang leader is pushed as well.
> 
> You mean amdgpu registers callbacks to drm_sched_fence?

No, we give it as dependency to drm_sched_job_add_dependency().

>>> That's rather vague. Regarding this TODO, "racing between timing out
>>> and signaling the fence" can now be corrected by the driver. Are there
>>> more issues? If so, we want to add a new FIXME for them.
>>
>> Yeah good point. We basically worked around all those issues now.
>>
>> It's just that I still see that we are missing a general concept. E.g. we applied workaround on top of workaround until it didn't crashed any more instead of saying ok that is the design does that work? Is it valid? etc...
> 
> Yes, that seems to have been our destiny for a while now :) :(
> 
> What I'm afraid of right now is that with the callbacks vs.
> drm_sched_fence we now potentially have several distinct mechanisms for
> doing things. The hardware fence is clearly the relevant
> synchronization object for telling when a job is completed; yet, we
> also have s_fence->finished.

Not quite, the s_fence->finished is what is relevant to the outside. The HW fence is only relevant to the inside of the scheduler.

> Using it (for what?) is even encouraged by the docu:
> 
>         /**
>          * @finished: this fence is what will be signaled by the scheduler
>          * when the job is completed.
>          *
>          * When setting up an out fence for the job, you should use
>          * this, since it's available immediately upon
>          * drm_sched_job_init(), and the fence returned by the driver
>          * from run_job() won't be created until the dependencies have
>          * resolved.
>          */

That comment sounds correct to me. Drivers should mostly use s_fence->finished and not the HW fence to determine if something is done.

> Anyways.
> I think this is a big topic very suitable for our work shop at XDC. I
> also have some ideas about paths forward that I want to present.

Sounds good.

Halve a year ago I was more or less ready to suggest to rip out the scheduler and start from scratch again, but now it more and more looks like we have light at the end of the tunnel.

Christian.

> 
> 
> P.
> 
>>
>>> That said, such an RFC would obviously be great. We can discuss the
>>> paragraph above there, if you want.
>>
>> I will try to hack something together. Not necessarily complete but it should show the direction.
>>
>> Christian.
>>
>>>
>>>
>>> Regards
>>> P.
> 


  reply	other threads:[~2025-08-11 14:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-24 14:01 Philipp Stanner
2025-07-24 15:07 ` Philipp Stanner
2025-08-05  9:05   ` Christian König
2025-08-05 10:22     ` Philipp Stanner
2025-08-07 14:15       ` Christian König
2025-08-11  9:50         ` Philipp Stanner
2025-08-11 14:17           ` Christian König [this message]
2025-08-04 15:41 ` Christian König
  -- strict thread matches above, loose matches on Subject: below --
2024-11-15 10:35 Philipp Stanner
2024-11-21 16:05 ` Alice Ryhl
2024-12-05 13:20   ` Philipp Stanner
2024-11-26 16:31 ` Tvrtko Ursulin

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=7f620860-bd4d-4b14-9832-2fa25fd4b086@amd.com \
    --to=christian.koenig@amd.com \
    --cc=airlied@gmail.com \
    --cc=ckoenig.leichtzumerken@gmail.com \
    --cc=corbet@lwn.net \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=matthew.brost@intel.com \
    --cc=mripard@kernel.org \
    --cc=phasta@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=sumit.semwal@linaro.org \
    --cc=tzimmermann@suse.de \
    /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

all inboxes | Powered by JetHome®