Re: [PATCH 3/9] drm/xe/rtp: Keep track of non-OA nonpriv slots

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Fri, 29 May 2026 18:51:47 -0700, Dixit, Ashutosh wrote:
>
> On Fri, 29 May 2026 16:24:30 -0700, Umesh Nerlige Ramappa wrote:
> >
> > On Fri, May 29, 2026 at 01:45:12PM -0700, Dixit, Ashutosh wrote:
> > > On Fri, 29 May 2026 11:30:34 -0700, Umesh Nerlige Ramappa wrote:
> > >>
> > >> On Mon, May 18, 2026 at 04:47:10PM -0700, Ashutosh Dixit wrote:
> > >> > In order to dynamically whitelist/dewhitelist OA registers on OA stream
> > >> > open/close, we need to keep track of nonpriv slots occupied by non-OA
> > >> > register whitelists.
> > >>
> > >> Can we maintain the slot index within hwe, so that the caller does not need
> > >> to keep track of it? I am assuming this will only be used in the probe
> > >> path. For all other paths the *_sr registers are used directly which
> > >> already have the registre<->slot association stored within them.
> > >>
> > >> i.e.
> > >>
> > >> hwe->slot;
> > >
> > > Hmm, when I first wrote the code, I had this in hwe: hwe->nonpriv_slots.
> > > But later realized it is only needed in xe_reg_whitelist_process_engine(),
> > > so moved it to the local variable nonpriv_slots there.
> > >
> > > Also, nonpriv_slots is actually number of slots occupied by non-OA
> > > registers. So if it part of hwe, the question becomes more complicated:
> > > what do we store there, number of slots excluding OA or including OA? Since
> > > we only need number of slots excluding OA.
> >
> > I would just think of it as the next available nonpriv slot for the caller
> > to use. It's just a state w.r.t the hwe. When you let the caller (even if
> > internal to the KMD) manage it, it's prone to errors - not about this
> > series or current code, but in general.
>
> OK, will do this in v2. Thanks.

I have done it but note that, because OA registers are added to two lists
(oa_sr and reg_sr), some involvement from the caller cannot be
eliminated. Hopefully the arguments and return value from
whitelist_apply_to_hwe() in v2 are sufficient to clarify this involvement
of the caller.

>
> > >
> > > Therefore, to avoid these complications, I decided that the best way was a
> > > temporary local variable.
> > >
> > > What do you think we'd gain by maintaining it as part of hwe? If things
> > > change later, we could add it, but for now the local variable seems
> > > sufficient?
> >
> > >
> > > Thanks.
> > > --
> > > Ashutosh
> > >
> > >> >
> > >> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > >> > ---
> > >> > drivers/gpu/drm/xe/xe_reg_whitelist.c | 8 ++++++--
> > >> > 1 file changed, 6 insertions(+), 2 deletions(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/xe/xe_reg_whitelist.c b/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > >> > index 6cc81f53fc601..1e788e2ee4014 100644
> > >> > --- a/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > >> > +++ b/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > >> > @@ -159,7 +159,7 @@ static const struct xe_rtp_entry_sr oa_whitelist[] = {
> > >> >	},
> > >> > };
> > >> >
> > >> > -static void whitelist_apply_to_hwe(struct xe_hw_engine *hwe)
> > >> > +static int whitelist_apply_to_hwe(struct xe_hw_engine *hwe)
> > >> > {
> > >> >	struct xe_reg_sr *sr = &hwe->reg_whitelist;
> > >> >	struct xe_reg_sr_entry *entry;
> > >> > @@ -191,6 +191,8 @@ static void whitelist_apply_to_hwe(struct xe_hw_engine *hwe)
> > >> >
> > >> >		slot++;
> > >> >	}
> > >> > +
> > >> > +	return slot;
> > >> > }
> > >> >
> > >> > /**
> > >> > @@ -203,11 +205,13 @@ static void whitelist_apply_to_hwe(struct xe_hw_engine *hwe)
> > >> >  */
> > >> > void xe_reg_whitelist_process_engine(struct xe_hw_engine *hwe)
> > >> > {
> > >> > +	int nonpriv_slots;
> > >> > +
> > >> >	struct xe_rtp_process_ctx ctx = XE_RTP_PROCESS_CTX_INITIALIZER(hwe);
> > >> >
> > >> >	xe_rtp_process_to_sr(&ctx, register_whitelist, ARRAY_SIZE(register_whitelist),
> > >> >			     &hwe->reg_whitelist, false);
> > >> > -	whitelist_apply_to_hwe(hwe);
> > >> > +	nonpriv_slots = whitelist_apply_to_hwe(hwe);
> > >> >
> > >> >	xe_rtp_process_to_sr(&ctx, oa_whitelist, ARRAY_SIZE(oa_whitelist),
> > >> >			     &hwe->oa_whitelist, false);
> > >> > --
> > >> > 2.54.0
> > >> >