Re: [PATCH 1/2] drm/xe/oa: Rename last argument of WHITELIST_OA_MMIO_TRG

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

On Wed, 03 Jun 2026 13:21:35 -0700, Dixit, Ashutosh wrote:
>
> On Wed, 03 Jun 2026 12:09:59 -0700, Umesh Nerlige Ramappa wrote:
> >
> > On Tue, Jun 02, 2026 at 04:09:07PM -0700, Ashutosh Dixit wrote:
> > > OA head pointer registers are not used by UMD's and do not need to be
> > > whitelisted, the last argument of WHITELIST_OA_MMIO_TRG is actually used
> > > for whitelisting tail pointer and OA buffer registers. Rename the argument
> > > to tail_buf to highlight this. OA head pointer is sometimes provided to the
> > > WHITELIST_OA_MMIO_TRG to have the correct register offset alignment (16)
> > > for RING_FORCE_TO_NONPRIV_RANGE_4.
> > >
> > > Fixes: ed455775c5a6 ("drm/xe/rtp: Refactor OAG MMIO trigger register whitelisting")
> > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > ---
> > > drivers/gpu/drm/xe/xe_reg_whitelist.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_reg_whitelist.c b/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > > index 2e84b1c49f374..a17ebacc1455b 100644
> > > --- a/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > > +++ b/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > > @@ -104,10 +104,10 @@ static const struct xe_rtp_table_sr register_whitelist = XE_RTP_TABLE_SR(
> > >				   RING_FORCE_TO_NONPRIV_ACCESS_RW))
> > >	},
> > >
> > > -#define WHITELIST_OA_MMIO_TRG(trg, status, head) \
> > > +#define WHITELIST_OA_MMIO_TRG(trg, status, tail_buf) \
> > >	WHITELIST(trg, RING_FORCE_TO_NONPRIV_ACCESS_RW), \
> > >	WHITELIST(status, RING_FORCE_TO_NONPRIV_ACCESS_RD), \
> > > -	WHITELIST(head, RING_FORCE_TO_NONPRIV_ACCESS_RD | RING_FORCE_TO_NONPRIV_RANGE_4)
> > > +	WHITELIST(tail_buf, RING_FORCE_TO_NONPRIV_ACCESS_RD | RING_FORCE_TO_NONPRIV_RANGE_4)
> >
> > I think this would be a good time to split the regs and do away with the
> > RANGE_4.  If HEAD and STATUS are not used, then we should just use 2 slots
> > - one for OA_TAIL and one for OA_BUFFER (single reg mode). Any idea if
> > STATUS is needed? I thought the KMD already returns the status in an IOCTL.
>
> OK, yes I think OASTATUS doesn't need to be whitelisted and we can also do
> away with RANGE_4.

Actually, we can save one slot by retaining RANGE_4. So isn't it better to
retain it? Since the whitlisting will anyway be gated by paranoid.

Since there is a multiplicative effect: say OAM command streamers will
whitelist registers for all 3 OAM units, so there is a x3 multiplier. With
RANGE_4 we will use up 6 slots and without RANGE_4 we will use up 9 slots.

So just thought I'll ask about this too. Thanks.


>
> But can we review and merge Patch 2 for now, since that is a bug fix and
> will need to be propagated also to previous kernel versions. So if we can
> merge that first and then make these changes, it will simplify life quite a
> bit.
>
> Thanks.
> --
> Ashutosh
>
>
> >
> > >
> > > #define WHITELIST_OAG_MMIO_TRG \
> > >	WHITELIST_OA_MMIO_TRG(OAG_MMIOTRIGGER, OAG_OASTATUS, OAG_OAHEADPTR)
> > > --
> > > 2.54.0
> > >