Re: [PATCH v2] drm/xe/nvm: enable cri platform

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Dec 16, 2025 at 01:10:34PM +0200, Alexander Usyskin wrote:
> Mark CRI as one that have the CSC NVM device.
> Update the writable override flow to take the information from
> the scratch register for CRI.
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
> 
> V2: Fix commit message format (Raag)
>     Drop fallthrough; (Raag)
> 
>  drivers/gpu/drm/xe/xe_nvm.c | 34 +++++++++++++++++++++++-----------
>  drivers/gpu/drm/xe/xe_pci.c |  1 +
>  2 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_nvm.c b/drivers/gpu/drm/xe/xe_nvm.c
> index 33f4ac82fc80..01510061d4d4 100644
> --- a/drivers/gpu/drm/xe/xe_nvm.c
> +++ b/drivers/gpu/drm/xe/xe_nvm.c
> @@ -10,6 +10,7 @@
>  #include "xe_device_types.h"
>  #include "xe_mmio.h"
>  #include "xe_nvm.h"
> +#include "xe_pcode_api.h"
>  #include "regs/xe_gsc_regs.h"
>  #include "xe_sriov.h"
>  
> @@ -45,39 +46,50 @@ static bool xe_nvm_non_posted_erase(struct xe_device *xe)
>  {
>  	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>  
> -	if (xe->info.platform != XE_BATTLEMAGE)
> +	switch (xe->info.platform) {
> +	case XE_CRESCENTISLAND:
> +	case XE_BATTLEMAGE:
> +		return !(xe_mmio_read32(mmio, XE_REG(GEN12_CNTL_PROTECTED_NVM_REG)) &
> +			 NVM_NON_POSTED_ERASE_CHICKEN_BIT);
> +	default:
>  		return false;
> -	return !(xe_mmio_read32(mmio, XE_REG(GEN12_CNTL_PROTECTED_NVM_REG)) &
> -		 NVM_NON_POSTED_ERASE_CHICKEN_BIT);
> +	}
>  }
>  
>  static bool xe_nvm_writable_override(struct xe_device *xe)
>  {
>  	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>  	bool writable_override;
> -	resource_size_t base;
> +	struct xe_reg reg;
> +	u32 test_bit;
>  
>  	switch (xe->info.platform) {
> +	case XE_CRESCENTISLAND:
> +		reg = PCODE_SCRATCH(0);
> +		test_bit = FDO_MODE;

thanks for pointing out this bit offline....

> +		break;
>  	case XE_BATTLEMAGE:
> -		base = DG2_GSC_HECI2_BASE;
> +		reg = HECI_FWSTS2(DG2_GSC_HECI2_BASE);
> +		test_bit = HECI_FW_STATUS_2_NVM_ACCESS_MODE;
>  		break;
>  	case XE_PVC:
> -		base = PVC_GSC_HECI2_BASE;
> +		reg = HECI_FWSTS2(PVC_GSC_HECI2_BASE);
> +		test_bit = HECI_FW_STATUS_2_NVM_ACCESS_MODE;
>  		break;
>  	case XE_DG2:
> -		base = DG2_GSC_HECI2_BASE;
> +		reg = HECI_FWSTS2(DG2_GSC_HECI2_BASE);
> +		test_bit = HECI_FW_STATUS_2_NVM_ACCESS_MODE;
>  		break;
>  	case XE_DG1:
> -		base = DG1_GSC_HECI2_BASE;
> +		reg = HECI_FWSTS2(DG1_GSC_HECI2_BASE);
> +		test_bit = HECI_FW_STATUS_2_NVM_ACCESS_MODE;
>  		break;
>  	default:
>  		drm_err(&xe->drm, "Unknown platform\n");
>  		return true;
>  	}
>  
> -	writable_override =
> -		!(xe_mmio_read32(mmio, HECI_FWSTS2(base)) &
> -		  HECI_FW_STATUS_2_NVM_ACCESS_MODE);
> +	writable_override = !(xe_mmio_read32(mmio, reg) & test_bit);

I was wondering if we should have another name for this variable...
perhaps read_only_mode... but I couldn't come with anything that
is global, so...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

>  	if (writable_override)
>  		drm_info(&xe->drm, "NVM access overridden by jumper\n");
>  	return writable_override;
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 0887d1912283..535325796067 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -416,6 +416,7 @@ static const struct xe_device_desc cri_desc = {
>  	.dma_mask_size = 52,
>  	.has_display = false,
>  	.has_flat_ccs = false,
> +	.has_gsc_nvm = 1,
>  	.has_i2c = true,
>  	.has_mbx_power_limits = true,
>  	.has_mert = true,
> -- 
> 2.43.0
>