Re: [RFC-v3 02/13] drm/i915/pxp: set KCR reg init during the boot time

["Huang, Sean Z" <sean.z.huang@xxxxxxxxx>]

> I'm wondering if some other patch in this series will explain the need for this, but as is right now, this shouldn't be here on this patch... and maybe not even at all.
> Let's minimize the wraps and use the already provided read/write functions.

Thanks for pointing it out. Yes, you're correct. I missed this one and it should be removed from this commit.

Best regards,
Sean

-----Original Message-----
From: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> 
Sent: Wednesday, December 9, 2020 10:08 AM
To: Huang, Sean Z <sean.z.huang@xxxxxxxxx>
Cc: Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re:  [RFC-v3 02/13] drm/i915/pxp: set KCR reg init during the boot time

On Tue, Dec 08, 2020 at 11:02:56PM -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 ++++++++  
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 6 ++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c 
> b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index ba43b2c923c7..c4815950567d 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 */
> +#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))
> +
>  int intel_pxp_init(struct intel_pxp *pxp)  {
>  	struct intel_gt *gt = container_of(pxp, struct intel_gt, pxp); @@ 
> -16,6 +22,8 @@ int intel_pxp_init(struct intel_pxp *pxp)
>  
>  	intel_pxp_ctx_init(&pxp->ctx);
>  
> +	intel_uncore_write(gt->uncore, KCR_INIT, 
> +KCR_INIT_ALLOW_DISPLAY_ME_WRITES);
> +
>  	drm_info(&gt->i915->drm, "Protected Xe Path (PXP) protected content 
> support initialized\n");
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h 
> b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> index 7c3d49a6a3ab..b906bf7dea90 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> @@ -15,6 +15,7 @@ struct intel_pxp {
>  #ifdef CONFIG_DRM_I915_PXP
>  int intel_pxp_init(struct intel_pxp *pxp);  void 
> intel_pxp_uninit(struct intel_pxp *pxp);
> +int intel_pxp_reg_read(struct intel_pxp *pxp, u32 offset, u32 
> +*regval);
>  #else
>  static inline int intel_pxp_init(struct intel_pxp *pxp)  { @@ -24,6 
> +25,11 @@ static inline int intel_pxp_init(struct intel_pxp *pxp)  
> static inline void intel_pxp_uninit(struct intel_pxp *pxp)  {  }
> +
> +static inline int intel_pxp_reg_read(struct intel_pxp *pxp, u32 
> +offset, u32 *regval) {
> +	return 0;
> +}

I'm wondering if some other patch in this series will explain the need for this, but as is right now, this shouldn't be here on this patch... and maybe not even at all.
Let's minimize the wraps and use the already provided read/write functions.

>  #endif
>  
>  #endif /* __INTEL_PXP_PM_H__ */
> --
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx