All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>,
	<intel-xe@lists.freedesktop.org>,
	Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Subject: Re: [PATCH 6/7] drm/xe/gt: Drop third submission for default context
Date: Mon, 7 Jul 2025 21:59:57 -0700	[thread overview]
Message-ID: <aGylzRvFQBqCF6be@lstrano-desk.jf.intel.com> (raw)
In-Reply-To: <nxvygecgyeh43k4pru36f4v3g2qcgweh7wsz5mhphiky435dwn@hc2hw4emmzav>

On Mon, Jul 07, 2025 at 09:55:58PM -0500, Lucas De Marchi wrote:
> On Fri, Jul 04, 2025 at 12:21:50PM +0100, Tvrtko Ursulin wrote:
> > 
> > On 03/07/2025 23:41, Lucas De Marchi wrote:
> > > There's no need to submit the nop job again on the first queue. Any
> > > state needed is already saved when the first LRC is switched out. The
> > > comment is a little misleading regarding indirect W/A: first of all
> > > there's still no indirect W/A enabled and secondly, even after they are,
> > > there's no need to submit this job again for having their state
> > > propagated: the indirect W/A will actually run on every LRC switch.
> > > 
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_gt.c | 8 --------
> > >  1 file changed, 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > > index 67425e37c2187..439e7c703ed84 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > > @@ -361,14 +361,6 @@ int xe_gt_record_default_lrcs(struct xe_gt *gt)
> > >  			goto put_nop_q;
> > >  		}
> > > -		/* Reload golden LRC to record the effect of any indirect W/A */
> > > -		err = emit_nop_job(gt, q);
> > > -		if (err) {
> > > -			xe_gt_err(gt, "hwe %s: emit_nop_job failed (%pe) guc_id=%u\n",
> > > -				  hwe->name, ERR_PTR(err), q->guc->id);
> > > -			goto put_nop_q;
> > > -		}
> > > -
> > >  		xe_map_memcpy_from(xe, default_lrc,
> > >  				   &q->lrc[0]->bo->vmap,
> > >  				   xe_lrc_pphwsp_offset(q->lrc[0]),
> > > 
> > 
> > Wasn't it also racy to memcpy from q's LRC without guaranteeing context
> > save had completed? I don't think dma_fence_wait in emit_nop_job
> > guarantees it. If that is so this patch should actually have Fixes:
> > added and commit message adjusted accordingly.
> 
> I don't think it really fixes anything, it's just pointless to do it.
> It would just save the same information from the first time it executed
> even if there was a race.
> 

Agree with Lucas, this pointless yet harmless.

Matt

> Lucas De Marchi
> 
> > 
> > Regards,
> > 
> > Tvrtko
> > 

  reply	other threads:[~2025-07-08  4:58 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-03 22:41 [PATCH 0/7] drm/xe: LRC refactors Lucas De Marchi
2025-07-03 22:41 ` [PATCH 1/7] drm/xe/lrc: Reduce scope of empty lrc data Lucas De Marchi
2025-07-04 10:07   ` Tvrtko Ursulin
2025-07-08  5:01   ` Matthew Brost
2025-07-03 22:41 ` [PATCH 2/7] drm/xe: Count dwords before allocating Lucas De Marchi
2025-07-04 10:20   ` Tvrtko Ursulin
2025-07-07  3:30     ` Lucas De Marchi
2025-07-07 23:09       ` Lucas De Marchi
2025-07-03 22:41 ` [PATCH 3/7] drm/xe/gt: Extract emit_job_sync() Lucas De Marchi
2025-07-04 10:35   ` Tvrtko Ursulin
2025-07-08  0:59     ` Lucas De Marchi
2025-07-03 22:41 ` [PATCH 4/7] drm/xe/lrc: Add table with LRC layout Lucas De Marchi
2025-07-04 10:41   ` Tvrtko Ursulin
2025-07-08  1:02     ` Lucas De Marchi
2025-07-03 22:41 ` [PATCH 5/7] drm/xe/lrc: Remove leftover TODO Lucas De Marchi
2025-07-08  5:08   ` Matthew Brost
2025-07-08 12:46     ` Lucas De Marchi
2025-07-09 19:37   ` Summers, Stuart
2025-07-09 21:24     ` Lucas De Marchi
2025-07-09 21:44       ` Summers, Stuart
2025-07-03 22:41 ` [PATCH 6/7] drm/xe/gt: Drop third submission for default context Lucas De Marchi
2025-07-04 11:21   ` Tvrtko Ursulin
2025-07-08  2:55     ` Lucas De Marchi
2025-07-08  4:59       ` Matthew Brost [this message]
2025-07-08  5:05         ` Matthew Brost
2025-07-09  7:34         ` Tvrtko Ursulin
2025-07-03 22:41 ` [PATCH 7/7] drm/xe: Waste fewer instructions in emit_wa_job() Lucas De Marchi
2025-07-08  7:54   ` Matthew Brost
2025-07-03 22:49 ` ✗ CI.checkpatch: warning for drm/xe: LRC refactors Patchwork
2025-07-03 22:50 ` ✓ CI.KUnit: success " Patchwork
2025-07-03 23:26 ` ✓ Xe.CI.BAT: " Patchwork
2025-07-05 17:33 ` ✗ Xe.CI.Full: failure " Patchwork

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=aGylzRvFQBqCF6be@lstrano-desk.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=tvrtko.ursulin@igalia.com \
    --cc=umesh.nerlige.ramappa@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: link
Be 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.