[PATCH v6 07/11] drm/xe: Assert runnable state in handle_sched_done
Matthew Brost
matthew.brost at intel.com
Wed Jun 12 22:27:40 UTC 2024
On Wed, Jun 12, 2024 at 02:25:40PM -0700, John Harrison wrote:
> On 6/11/2024 07:40, Matthew Brost wrote:
> > Ensure G2H and KMD GuC machine match.
> >
> > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_guc_submit.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index afd22a8d815d..ab0dc93d7740 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -1592,16 +1592,21 @@ static void deregister_exec_queue(struct xe_guc *guc, struct xe_exec_queue *q)
> > xe_guc_ct_send_g2h_handler(&guc->ct, action, ARRAY_SIZE(action));
> > }
> > -static void handle_sched_done(struct xe_guc *guc, struct xe_exec_queue *q)
> > +static void handle_sched_done(struct xe_guc *guc, struct xe_exec_queue *q,
> > + u32 runnable_state)
> > {
> > trace_xe_exec_queue_scheduling_done(q);
> > if (exec_queue_pending_enable(q)) {
> > + xe_gt_assert(guc_to_gt(guc), runnable_state == 1);
> > +
> > q->guc->resume_time = ktime_get();
> > clear_exec_queue_pending_enable(q);
> > smp_wmb();
> > wake_up_all(&guc->ct.wq);
> > } else {
> > + xe_gt_assert(guc_to_gt(guc), runnable_state == 0);
> > +
> Isn't this the wrong way around?
>
These asserts are per my testing and CI.
> You made an earlier comment that sounded like it is legal for an enable to
> be queued while a disable is still pending? If so, then you would get in
Other way around, a disable can be sent when an enable is still in
flight in the case of a preempt fence.
Enables cannot be issued when a pending disable is in flight.
So I believe this patch is correct.
Matt
> here for the disable notification but with both enable_pending and
> disable_pending flags set. That would hit the assert. Whereas, if the if
> checks the runnable_state parameter and the assert then checks for pending,
> you will not hit the assert and the code will do the correct thing.
>
> John.
>
> > clear_exec_queue_pending_disable(q);
> > if (q->guc->suspend_pending) {
> > suspend_fence_signal(q);
> > @@ -1640,7 +1645,7 @@ int xe_guc_sched_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
> > return -EPROTO;
> > }
> > - handle_sched_done(guc, q);
> > + handle_sched_done(guc, q, runnable_state);
> > return 0;
> > }
>
More information about the Intel-xe
mailing list