[Intel-xe] [PATCH] drm/xe: add has_pcode flag

[Lucas De Marchi]

On Sun, Oct 15, 2023 at 03:23:07PM +0300, Koby Elbaz wrote:
>Per device, this flag indicates whether PCODE uC exists.
>
>Signed-off-by: Koby Elbaz <kelbaz at habana.ai>
>---
> drivers/gpu/drm/xe/xe_device_types.h |  2 ++
> drivers/gpu/drm/xe/xe_pci.c          | 13 +++++++++++++
> drivers/gpu/drm/xe/xe_pcode.c        |  9 +++++++++
> 3 files changed, 24 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>index bc375ddda5a7..8e327673c05c 100644
>--- a/drivers/gpu/drm/xe/xe_device_types.h
>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>@@ -257,6 +257,8 @@ struct xe_device {
> 		u8 bypass_mtcfg:1;
> 		/** @supports_mmio_ext: supports MMIO extension/s */
> 		u8 supports_mmio_ext:1;
>+		/** @has_pcode: device has a PCODE uC */
>+		u8 has_pcode:1;

please group has_pcode with the other has_* flags, although I wonder if
we could have some other name so we don't have to update all the current
platforms (and probably miss some that are WIP).

>
> #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> 		struct {
>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>index 99ccee07f8cd..2aac0f4df50c 100644
>--- a/drivers/gpu/drm/xe/xe_pci.c
>+++ b/drivers/gpu/drm/xe/xe_pci.c
>@@ -61,6 +61,7 @@ struct xe_device_desc {
> 	u8 has_llc:1;
> 	u8 bypass_mtcfg:1;
> 	u8 supports_mmio_ext:1;
>+	u8 has_pcode:1;

ditto

> };
>
> __diag_push();
>@@ -212,6 +213,7 @@ static const struct xe_device_desc tgl_desc = {
> 	.has_display = true,
> 	.has_llc = true,
> 	.require_force_probe = true,
>+	.has_pcode = true,
> };
>
> static const struct xe_device_desc rkl_desc = {
>@@ -221,6 +223,7 @@ static const struct xe_device_desc rkl_desc = {
> 	.has_display = true,
> 	.has_llc = true,
> 	.require_force_probe = true,
>+	.has_pcode = true,
> };
>
> static const u16 adls_rpls_ids[] = { XE_RPLS_IDS(NOP), 0 };
>@@ -236,6 +239,7 @@ static const struct xe_device_desc adl_s_desc = {
> 		{ XE_SUBPLATFORM_ALDERLAKE_S_RPLS, "RPLS", adls_rpls_ids },
> 		{},
> 	},
>+	.has_pcode = true,
> };
>
> static const u16 adlp_rplu_ids[] = { XE_RPLU_IDS(NOP), 0 };
>@@ -251,6 +255,7 @@ static const struct xe_device_desc adl_p_desc = {
> 		{ XE_SUBPLATFORM_ALDERLAKE_P_RPLU, "RPLU", adlp_rplu_ids },
> 		{},
> 	},
>+	.has_pcode = true,
> };
>
> static const struct xe_device_desc adl_n_desc = {
>@@ -260,6 +265,7 @@ static const struct xe_device_desc adl_n_desc = {
> 	.has_display = true,
> 	.has_llc = true,
> 	.require_force_probe = true,
>+	.has_pcode = true,
> };
>
> #define DGFX_FEATURES \
>@@ -272,6 +278,7 @@ static const struct xe_device_desc dg1_desc = {
> 	PLATFORM(XE_DG1),
> 	.has_display = true,
> 	.require_force_probe = true,
>+	.has_pcode = true,
> };
>
> static const u16 dg2_g10_ids[] = { XE_DG2_G10_IDS(NOP), XE_ATS_M150_IDS(NOP), 0 };
>@@ -295,6 +302,7 @@ static const struct xe_device_desc ats_m_desc = {
>
> 	DG2_FEATURES,
> 	.has_display = false,
>+	.has_pcode = true,
> };
>
> static const struct xe_device_desc dg2_desc = {
>@@ -304,6 +312,7 @@ static const struct xe_device_desc dg2_desc = {
>
> 	DG2_FEATURES,
> 	.has_display = true,
>+	.has_pcode = true,
> };
>
> static const struct xe_device_desc pvc_desc = {
>@@ -312,6 +321,7 @@ static const struct xe_device_desc pvc_desc = {
> 	PLATFORM(XE_PVC),
> 	.has_display = false,
> 	.require_force_probe = true,
>+	.has_pcode = true,
> };
>
> static const struct xe_device_desc mtl_desc = {
>@@ -319,11 +329,13 @@ static const struct xe_device_desc mtl_desc = {
> 	.require_force_probe = true,
> 	PLATFORM(XE_METEORLAKE),
> 	.has_display = true,
>+	.has_pcode = true,
> };
>
> static const struct xe_device_desc lnl_desc = {
> 	PLATFORM(XE_LUNARLAKE),
> 	.require_force_probe = true,
>+	.has_pcode = true,
> };
>
> #undef PLATFORM
>@@ -576,6 +588,7 @@ static int xe_info_init(struct xe_device *xe,
> 	xe->info.bypass_mtcfg = desc->bypass_mtcfg;
> 	xe->info.supports_mmio_ext = desc->supports_mmio_ext;
> 	xe->info.tile_mmio_ext_size = graphics_desc->tile_mmio_ext_size;
>+	xe->info.has_pcode = desc->has_pcode;
>
> 	xe->info.dma_mask_size = graphics_desc->dma_mask_size;
> 	xe->info.vram_flags = graphics_desc->vram_flags;
>diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
>index 4a240acf7625..d895cb1e16c7 100644
>--- a/drivers/gpu/drm/xe/xe_pcode.c
>+++ b/drivers/gpu/drm/xe/xe_pcode.c
>@@ -61,6 +61,9 @@ static int pcode_mailbox_rw(struct xe_gt *gt, u32 mbox, u32 *data0, u32 *data1,
> {
> 	int err;
>
>+	if (!gt_to_xe(gt)->info.has_pcode)
>+		return 0;

I think we should rather protect the functions that are exported from
xe_pcode.c, not this one.

Lucas De Marchi

>+
> 	lockdep_assert_held(&gt->pcode.lock);
>
> 	if ((xe_mmio_read32(gt, PCODE_MAILBOX) & PCODE_READY) != 0)
>@@ -249,6 +252,9 @@ int xe_pcode_init(struct xe_gt *gt)
> 	int timeout_us = 180000000; /* 3 min */
> 	int ret;
>
>+	if (!gt_to_xe(gt)->info.has_pcode)
>+		return 0;
>+
> 	if (!IS_DGFX(gt_to_xe(gt)))
> 		return 0;
>
>@@ -280,6 +286,9 @@ int xe_pcode_probe(struct xe_gt *gt)
> {
> 	drmm_mutex_init(&gt_to_xe(gt)->drm, &gt->pcode.lock);
>
>+	if (!gt_to_xe(gt)->info.has_pcode)
>+		return 0;
>+
> 	if (!IS_DGFX(gt_to_xe(gt)))
> 		return 0;
>
>-- 
>2.34.1
>