[Intel-xe] [PATCH] drm/xe: Apply mask to RING IMR

[Upadhyay, Tejas]

> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi at intel.com>
> Sent: Tuesday, December 5, 2023 8:15 PM
> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> Cc: intel-xe at lists.freedesktop.org
> Subject: Re: [Intel-xe] [PATCH] drm/xe: Apply mask to RING IMR
> 
> On Tue, Dec 05, 2023 at 02:10:28PM +0000, Upadhyay, Tejas wrote:
> >
> >
> >> -----Original Message-----
> >> From: De Marchi, Lucas <lucas.demarchi at intel.com>
> >> Sent: Tuesday, December 5, 2023 12:11 AM
> >> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> >> Cc: intel-xe at lists.freedesktop.org
> >> Subject: Re: [Intel-xe] [PATCH] drm/xe: Apply mask to RING IMR
> >>
> >> On Mon, Dec 04, 2023 at 05:15:25PM +0530, Tejas Upadhyay wrote:
> >> >As per bspec RING interrupt mask register will have lower 16 bit
> >> >masked by upper 16 bit.
> >> >
> >> >Bspec 60352
> >>
> >> am I reading the wrong spec? 60352 doesn't say this register has
> >> masked access, i.e. in order to write any of the 0-15 bits we also
> >> need to set the upper bits.
> >>
> >> Compare that to e.g. 60279 where we do have a masked register
> >> BCS_SWCTRL.
> >>
> >> In this register it seems the upper bits are just another field to be
> >> set rather than considering this a masked register.
> >
> >"If an interrupt is unmasked. On occurrence of an interrupt and if the
> interrupt is enabled, will result in generating an interrupt message to the
> host."
> >By above sentence I could understand that this mask for all interrupt bits
> mentioned in lower bits.
> 
> it's an overloaded meaning of "mask". A "masked register" in bspec (and the
> option you are trying to set in xe) has a very specific
> meaning: The upper bits select the corresponding bits in the lower part that
> will be set when you write to the register.
> 
> For those register if you try to write 0x00000001, it won't have any effect as
> the hardware will ignore it. You have to write 0x00010001 for the write to go
> through. This allows writes to different fields in the register to simply not care
> what the other bits are, avoiding a rmw cycle.
> 
> Those registers in bspec are marked as "Access: masked" below the symbol
> name, besides the bitfield documentation.
> 
> In this register here, the mask doesn't have that meaning. It's a normal
> "interrupt masking/unmasking" field.

Ok got it, access relation. Thanks for the information.

Tejas
> 
> Lucas De Marchi
> 
> >
> >Thanks,
> >Tejas
> >
> >>
> >>
> >> Lucas De Marchi
> >>
> >>
> >> >
> >> >Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> >> >---
> >> > drivers/gpu/drm/xe/regs/xe_engine_regs.h | 2 +-
> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> >diff --git a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> >> >b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> >> >index 444ff9b83bb1..d9013c698710 100644
> >> >--- a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> >> >+++ b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> >> >@@ -39,7 +39,7 @@
> >> > #define RING_MI_MODE(base)			XE_REG((base) +
> >> 0x9c)
> >> > #define RING_NOPID(base)			XE_REG((base) +
> 0x94)
> >> >
> >> >-#define RING_IMR(base)				XE_REG((base) +
> >> 0xa8)
> >> >+#define RING_IMR(base)				XE_REG((base) + 0xa8,
> >> XE_REG_OPTION_MASKED)
> >> > #define   RING_MAX_NONPRIV_SLOTS  12
> >> >
> >> > #define RING_EIR(base)				XE_REG((base) +
> >> 0xb0)
> >> >--
> >> >2.25.1
> >> >