[Intel-gfx] [RFC-v19 02/13] drm/i915/pxp: set KCR reg init during the boot time

[Huang, Sean Z]

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 <joonas.lahtinen at linux.intel.com> 
Sent: Friday, January 8, 2021 3:31 AM
To: Huang, Sean Z <sean.z.huang at intel.com>; Intel-gfx at lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi at intel.com>
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 <sean.z.huang at intel.com>
> > ---
> >  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