On Mon, 2021-01-11 at 21:38 +0000, Huang, Sean Z wrote: > 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)) This is not what I asked actually. I asked to get the BIT(14) to be defined separated, shift defined as well... and the | and actuall shift operations to be performed in the code and not in the defines > > Best regards, > Sean > > -----Original Message----- > From: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > Sent: Friday, January 8, 2021 3:31 AM > To: Huang, Sean Z <sean.z.huang@xxxxxxxxx>; > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Vivi, Rodrigo < > rodrigo.vivi@xxxxxxxxx> > Subject: Re: [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@xxxxxxxxx> > > > --- > > > 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 Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
