Hello,

I see, based on Joonas and Rodrigo's feedback.

I made the modification as below, I will still keep the macro in this .c 
instead of i915_reg.h, and this change will be reflected in rev20.

/* KCR register definitions */
 #define KCR_INIT            _MMIO(0x320f0)
-#define KCR_INIT_MASK_SHIFT (16)
+
 /* Setting KCR Init bit is required after system boot */
-#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES (BIT(14) | (BIT(14) << 
KCR_INIT_MASK_SHIFT))
+#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES (BIT(14) | (BIT(14) << 16))

Best regards,
Sean

-----Original Message-----
From: Joonas Lahtinen <[email protected]> 
Sent: Friday, January 8, 2021 3:31 AM
To: Huang, Sean Z <[email protected]>; [email protected]; 
Vivi, Rodrigo <[email protected]>
Subject: Re: [Intel-gfx] [RFC-v19 02/13] drm/i915/pxp: set KCR reg init during 
the boot time

Quoting Vivi, Rodrigo (2021-01-07 17:31:36)
> On Wed, 2021-01-06 at 15:12 -0800, Huang, Sean Z wrote:
> > Set the KCR init during the boot time, which is required by 
> > hardware, to allow us doing further protection operation such as 
> > sending commands to GPU or TEE.
> > 
> > Signed-off-by: Huang, Sean Z <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/pxp/intel_pxp.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > index 9bc3c7e30654..f566a4fda044 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > @@ -6,6 +6,12 @@
> >  #include "intel_pxp.h"
> >  #include "intel_pxp_context.h"
> >  
> > +/* KCR register definitions */
> 
> please define this in i915_reg.h

Generally the trend on the GT side is to contain in a .c file if there are no 
shared users like these. So they should be at this spot, yet the rest of the 
review comments apply.

The spurious comments should be dropped and like Rodrigo pointed out, we should 
be using the appropriate macros for a masked writes, not baking in the #define.

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to