[Intel-gfx] [RFC-v19 02/13] drm/i915/pxp: set KCR reg init during the boot time
jani.nikula at linux.intel.com
Tue Jan 12 15:36:02 UTC 2021
On Tue, 12 Jan 2021, "Vivi, Rodrigo" <rodrigo.vivi at intel.com> wrote:
> On Mon, 2021-01-11 at 21:38 +0000, Huang, Sean Z wrote:
>> 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) <<
>> +#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES (BIT(14) | (BIT(14) << 16))
BIT(14) << 16 is actually BIT(14+16), or BIT(30). The above is
Also, you'll end up with problems by using BIT() instead of REG_BIT()
defined in i915_reg.h due to BIT() using unsigned long.
Also, read the big style comment near the top of i915_reg.h.
> 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,
>> -----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
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
Jani Nikula, Intel Open Source Graphics Center
More information about the Intel-gfx