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

[Lucas De Marchi]

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.

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
>> >