* [PATCH 0/2] Disable automatic load CCS load balancing
@ 2024-02-20 14:20 Andi Shyti
2024-02-20 14:20 ` [PATCH 1/2] drm/i915/gt: Disable HW load balancing for CCS Andi Shyti
` (6 more replies)
0 siblings, 7 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-20 14:20 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, John Harrison,
Tvrtko Ursulin, stable, Andi Shyti, Andi Shyti
Hi,
this series does basically two things:
1. Disables automatic load balancing as adviced by the hardware
workaround.
2. Forces the sharing of the load submitted to CCS among all the
CCS available (as of now only DG2 has more than one CCS). This
way the user, when sending a query, will see only one CCS
available.
Andi
Andi Shyti (2):
drm/i915/gt: Disable HW load balancing for CCS
drm/i915/gt: Set default CCS mode '1'
drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 ++++++
drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
drivers/gpu/drm/i915/i915_query.c | 5 +++--
5 files changed, 40 insertions(+), 2 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] drm/i915/gt: Disable HW load balancing for CCS
2024-02-20 14:20 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
@ 2024-02-20 14:20 ` Andi Shyti
2024-02-20 14:20 ` [PATCH 2/2] drm/i915/gt: Set default CCS mode '1' Andi Shyti
` (5 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-20 14:20 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, John Harrison,
Tvrtko Ursulin, stable, Andi Shyti, Andi Shyti
The hardware should not dynamically balance the load between CCS
engines. Wa_16016805146 recommends disabling it across all
platforms.
Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: <stable@vger.kernel.org> # v6.2+
---
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 50962cfd1353..cf709f6c05ae 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1478,6 +1478,7 @@
#define GEN12_RCU_MODE _MMIO(0x14800)
#define GEN12_RCU_MODE_CCS_ENABLE REG_BIT(0)
+#define XEHP_RCU_MODE_FIXED_SLICE_CCS_MODE REG_BIT(1)
#define CHV_FUSE_GT _MMIO(VLV_GUNIT_BASE + 0x2168)
#define CHV_FGT_DISABLE_SS0 (1 << 10)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index d67d44611c28..7f42c8015f71 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2988,6 +2988,12 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li
wa_mcr_masked_en(wal, GEN8_HALF_SLICE_CHICKEN1,
GEN7_PSD_SINGLE_PORT_DISPATCH_ENABLE);
}
+
+ /*
+ * Wa_16016805146: disable the CCS load balancing
+ * indiscriminately for all the platforms
+ */
+ wa_masked_en(wal, GEN12_RCU_MODE, XEHP_RCU_MODE_FIXED_SLICE_CCS_MODE);
}
static void
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-20 14:20 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
2024-02-20 14:20 ` [PATCH 1/2] drm/i915/gt: Disable HW load balancing for CCS Andi Shyti
@ 2024-02-20 14:20 ` Andi Shyti
2024-02-20 14:27 ` Tvrtko Ursulin
2024-02-27 12:18 ` Jani Nikula
2024-02-20 14:23 ` [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
` (4 subsequent siblings)
6 siblings, 2 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-20 14:20 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, John Harrison,
Tvrtko Ursulin, stable, Andi Shyti, Andi Shyti
Since CCS automatic load balancing is disabled, we will impose a
fixed balancing policy that involves setting all the CCS engines
to work together on the same load.
Simultaneously, the user will see only 1 CCS rather than the
actual number. As of now, this change affects only DG2.
Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: <stable@vger.kernel.org> # v6.2+
---
drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
drivers/gpu/drm/i915/i915_query.c | 5 +++--
4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index a425db5ed3a2..e19df4ef47f6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
}
}
+static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
+{
+ if (!IS_DG2(gt->i915))
+ return;
+
+ intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
+}
+
int intel_gt_init_hw(struct intel_gt *gt)
{
struct drm_i915_private *i915 = gt->i915;
@@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
intel_gt_init_swizzling(gt);
+ /* Configure CCS mode */
+ intel_gt_apply_ccs_mode(gt);
+
/*
* At least 830 can leave some of the unused rings
* "active" (ie. head != tail) after resume which
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index cf709f6c05ae..c148113770ea 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1605,6 +1605,8 @@
#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
#define GEN12_CAGF_MASK REG_GENMASK(19, 11)
+#define XEHP_CCS_MODE _MMIO(0x14804)
+
#define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
#define GEN11_CSME (31)
#define GEN12_HECI_2 (30)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e81b3b2858ac..0853ffd3cb8d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
(engine__); \
(engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
+/*
+ * Exclude unavailable engines.
+ *
+ * Only the first CCS engine is utilized due to the disabling of CCS auto load
+ * balancing. As a result, all CCS engines operate collectively, functioning
+ * essentially as a single CCS engine, hence the count of active CCS engines is
+ * considered '1'.
+ * Currently, this applies to platforms with more than one CCS engine,
+ * specifically DG2.
+ */
+#define for_each_available_uabi_engine(engine__, i915__) \
+ for_each_uabi_engine(engine__, i915__) \
+ if ((IS_DG2(i915__)) && \
+ ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
+ ((engine__)->uabi_instance)) { } \
+ else
+
#define INTEL_INFO(i915) ((i915)->__info)
#define RUNTIME_INFO(i915) (&(i915)->__runtime)
#define DRIVER_CAPS(i915) (&(i915)->caps)
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index fa3e937ed3f5..2d41bda626a6 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
}
+
static int
query_engine_info(struct drm_i915_private *i915,
struct drm_i915_query_item *query_item)
@@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
if (query_item->flags)
return -EINVAL;
- for_each_uabi_engine(engine, i915)
+ for_each_available_uabi_engine(engine, i915)
num_uabi_engines++;
len = struct_size(query_ptr, engines, num_uabi_engines);
@@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
info_ptr = &query_ptr->engines[0];
- for_each_uabi_engine(engine, i915) {
+ for_each_available_uabi_engine(engine, i915) {
info.engine.engine_class = engine->uabi_class;
info.engine.engine_instance = engine->uabi_instance;
info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 0/2] Disable automatic load CCS load balancing
2024-02-20 14:20 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
2024-02-20 14:20 ` [PATCH 1/2] drm/i915/gt: Disable HW load balancing for CCS Andi Shyti
2024-02-20 14:20 ` [PATCH 2/2] drm/i915/gt: Set default CCS mode '1' Andi Shyti
@ 2024-02-20 14:23 ` Andi Shyti
2024-02-20 14:34 ` Andi Shyti
` (3 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-20 14:23 UTC (permalink / raw)
To: Andi Shyti
Cc: intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen, Matt Roper,
John Harrison, Tvrtko Ursulin, stable, Andi Shyti
Hi,
I'm sorry, I forgot to add the changelog. Here it is:
v1 -> v2
========
- In Patch 1 use the correct workaround number (thanks Matt).
- In Patch 2 do not add the extra CCS engines to the exposed UABI
engine list and adapt the engine counting accordingly (thanks
Tvrtko).
- Reword the commit of Patch 2 (thanks John).
On Tue, Feb 20, 2024 at 03:20:32PM +0100, Andi Shyti wrote:
> Hi,
>
> this series does basically two things:
>
> 1. Disables automatic load balancing as adviced by the hardware
> workaround.
>
> 2. Forces the sharing of the load submitted to CCS among all the
> CCS available (as of now only DG2 has more than one CCS). This
> way the user, when sending a query, will see only one CCS
> available.
>
> Andi
>
> Andi Shyti (2):
> drm/i915/gt: Disable HW load balancing for CCS
> drm/i915/gt: Set default CCS mode '1'
>
> drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 ++++++
> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
> drivers/gpu/drm/i915/i915_query.c | 5 +++--
> 5 files changed, 40 insertions(+), 2 deletions(-)
>
> --
> 2.43.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-20 14:20 ` [PATCH 2/2] drm/i915/gt: Set default CCS mode '1' Andi Shyti
@ 2024-02-20 14:27 ` Tvrtko Ursulin
2024-02-20 14:33 ` Andi Shyti
2024-02-27 12:18 ` Jani Nikula
1 sibling, 1 reply; 21+ messages in thread
From: Tvrtko Ursulin @ 2024-02-20 14:27 UTC (permalink / raw)
To: Andi Shyti, intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, John Harrison, stable,
Andi Shyti
On 20/02/2024 14:20, Andi Shyti wrote:
> Since CCS automatic load balancing is disabled, we will impose a
> fixed balancing policy that involves setting all the CCS engines
> to work together on the same load.
Erm *all* CSS engines work together..
> Simultaneously, the user will see only 1 CCS rather than the
> actual number. As of now, this change affects only DG2.
... *one* CCS engine.
>
> Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # v6.2+
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
> drivers/gpu/drm/i915/i915_query.c | 5 +++--
> 4 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index a425db5ed3a2..e19df4ef47f6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
> }
> }
>
> +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
> +{
> + if (!IS_DG2(gt->i915))
> + return;
> +
> + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
> +}
> +
> int intel_gt_init_hw(struct intel_gt *gt)
> {
> struct drm_i915_private *i915 = gt->i915;
> @@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
>
> intel_gt_init_swizzling(gt);
>
> + /* Configure CCS mode */
> + intel_gt_apply_ccs_mode(gt);
> +
> /*
> * At least 830 can leave some of the unused rings
> * "active" (ie. head != tail) after resume which
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index cf709f6c05ae..c148113770ea 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1605,6 +1605,8 @@
> #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
>
> +#define XEHP_CCS_MODE _MMIO(0x14804)
> +
> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
> #define GEN11_CSME (31)
> #define GEN12_HECI_2 (30)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e81b3b2858ac..0853ffd3cb8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
> (engine__); \
> (engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
>
> +/*
> + * Exclude unavailable engines.
> + *
> + * Only the first CCS engine is utilized due to the disabling of CCS auto load
> + * balancing. As a result, all CCS engines operate collectively, functioning
> + * essentially as a single CCS engine, hence the count of active CCS engines is
> + * considered '1'.
> + * Currently, this applies to platforms with more than one CCS engine,
> + * specifically DG2.
> + */
> +#define for_each_available_uabi_engine(engine__, i915__) \
> + for_each_uabi_engine(engine__, i915__) \
> + if ((IS_DG2(i915__)) && \
> + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
> + ((engine__)->uabi_instance)) { } \
> + else
> +
I thought the plan was to simply not register the engine. Like that it
would be a simpler patch.
> #define INTEL_INFO(i915) ((i915)->__info)
> #define RUNTIME_INFO(i915) (&(i915)->__runtime)
> #define DRIVER_CAPS(i915) (&(i915)->caps)
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> index fa3e937ed3f5..2d41bda626a6 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
> return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
> }
>
> +
!
> static int
> query_engine_info(struct drm_i915_private *i915,
> struct drm_i915_query_item *query_item)
> @@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
> if (query_item->flags)
> return -EINVAL;
>
> - for_each_uabi_engine(engine, i915)
> + for_each_available_uabi_engine(engine, i915)
> num_uabi_engines++;
>
> len = struct_size(query_ptr, engines, num_uabi_engines);
> @@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
>
> info_ptr = &query_ptr->engines[0];
>
> - for_each_uabi_engine(engine, i915) {
> + for_each_available_uabi_engine(engine, i915) {
> info.engine.engine_class = engine->uabi_class;
> info.engine.engine_instance = engine->uabi_instance;
> info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
I thought you agreed that this still misses to hide the engine on direct
lookup from userspace such as context map, PMU, SSEU. All of those would
automatically be handled by not registering the engine.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-20 14:27 ` Tvrtko Ursulin
@ 2024-02-20 14:33 ` Andi Shyti
0 siblings, 0 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-20 14:33 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: Andi Shyti, intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen,
Matt Roper, John Harrison, stable, Andi Shyti
On Tue, Feb 20, 2024 at 02:27:07PM +0000, Tvrtko Ursulin wrote:
>
> On 20/02/2024 14:20, Andi Shyti wrote:
> > Since CCS automatic load balancing is disabled, we will impose a
> > fixed balancing policy that involves setting all the CCS engines
> > to work together on the same load.
>
> Erm *all* CSS engines work together..
>
> > Simultaneously, the user will see only 1 CCS rather than the
> > actual number. As of now, this change affects only DG2.
>
> ... *one* CCS engine.
ops... I sent V1 again!
Sorry, I will send v2 now
Thanks!
Andi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/2] Disable automatic load CCS load balancing
2024-02-20 14:20 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
` (2 preceding siblings ...)
2024-02-20 14:23 ` [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
@ 2024-02-20 14:34 ` Andi Shyti
2024-02-20 14:47 ` ✗ Fi.CI.CHECKPATCH: warning for Disable automatic load CCS load balancing (rev2) Patchwork
` (2 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-20 14:34 UTC (permalink / raw)
To: Andi Shyti
Cc: intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen, Matt Roper,
John Harrison, Tvrtko Ursulin, stable, Andi Shyti
Please, ignore, I sent V1 again.
Sorry about the noise!
Andi
On Tue, Feb 20, 2024 at 03:20:32PM +0100, Andi Shyti wrote:
> Hi,
>
> this series does basically two things:
>
> 1. Disables automatic load balancing as adviced by the hardware
> workaround.
>
> 2. Forces the sharing of the load submitted to CCS among all the
> CCS available (as of now only DG2 has more than one CCS). This
> way the user, when sending a query, will see only one CCS
> available.
>
> Andi
>
> Andi Shyti (2):
> drm/i915/gt: Disable HW load balancing for CCS
> drm/i915/gt: Set default CCS mode '1'
>
> drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 ++++++
> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
> drivers/gpu/drm/i915/i915_query.c | 5 +++--
> 5 files changed, 40 insertions(+), 2 deletions(-)
>
> --
> 2.43.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for Disable automatic load CCS load balancing (rev2)
2024-02-20 14:20 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
` (3 preceding siblings ...)
2024-02-20 14:34 ` Andi Shyti
@ 2024-02-20 14:47 ` Patchwork
2024-02-20 14:47 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-02-20 15:00 ` ✗ Fi.CI.BAT: failure " Patchwork
6 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2024-02-20 14:47 UTC (permalink / raw)
To: Andi Shyti; +Cc: intel-gfx
== Series Details ==
Series: Disable automatic load CCS load balancing (rev2)
URL : https://patchwork.freedesktop.org/series/129951/
State : warning
== Summary ==
Error: dim checkpatch failed
2065c72a8acd drm/i915/gt: Disable HW load balancing for CCS
5e600e4db29b drm/i915/gt: Set default CCS mode '1'
-:80: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#80: FILE: drivers/gpu/drm/i915/i915_drv.h:409:
+#define for_each_available_uabi_engine(engine__, i915__) \
+ for_each_uabi_engine(engine__, i915__) \
+ if ((IS_DG2(i915__)) && \
+ ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
+ ((engine__)->uabi_instance)) { } \
+ else
-:80: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'engine__' - possible side-effects?
#80: FILE: drivers/gpu/drm/i915/i915_drv.h:409:
+#define for_each_available_uabi_engine(engine__, i915__) \
+ for_each_uabi_engine(engine__, i915__) \
+ if ((IS_DG2(i915__)) && \
+ ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
+ ((engine__)->uabi_instance)) { } \
+ else
-:80: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'i915__' - possible side-effects?
#80: FILE: drivers/gpu/drm/i915/i915_drv.h:409:
+#define for_each_available_uabi_engine(engine__, i915__) \
+ for_each_uabi_engine(engine__, i915__) \
+ if ((IS_DG2(i915__)) && \
+ ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
+ ((engine__)->uabi_instance)) { } \
+ else
-:82: ERROR:TRAILING_STATEMENTS: trailing statements should be on next line
#82: FILE: drivers/gpu/drm/i915/i915_drv.h:411:
+ if ((IS_DG2(i915__)) && \
[...]
+ ((engine__)->uabi_instance)) { } \
-:98: CHECK:LINE_SPACING: Please don't use multiple blank lines
#98: FILE: drivers/gpu/drm/i915/i915_query.c:127:
+
total: 2 errors, 0 warnings, 3 checks, 77 lines checked
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.SPARSE: warning for Disable automatic load CCS load balancing (rev2)
2024-02-20 14:20 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
` (4 preceding siblings ...)
2024-02-20 14:47 ` ✗ Fi.CI.CHECKPATCH: warning for Disable automatic load CCS load balancing (rev2) Patchwork
@ 2024-02-20 14:47 ` Patchwork
2024-02-20 15:00 ` ✗ Fi.CI.BAT: failure " Patchwork
6 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2024-02-20 14:47 UTC (permalink / raw)
To: Andi Shyti; +Cc: intel-gfx
== Series Details ==
Series: Disable automatic load CCS load balancing (rev2)
URL : https://patchwork.freedesktop.org/series/129951/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.BAT: failure for Disable automatic load CCS load balancing (rev2)
2024-02-20 14:20 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
` (5 preceding siblings ...)
2024-02-20 14:47 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2024-02-20 15:00 ` Patchwork
6 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2024-02-20 15:00 UTC (permalink / raw)
To: Andi Shyti; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 8390 bytes --]
== Series Details ==
Series: Disable automatic load CCS load balancing (rev2)
URL : https://patchwork.freedesktop.org/series/129951/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_14300 -> Patchwork_129951v2
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_129951v2 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_129951v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/index.html
Participating hosts (40 -> 38)
------------------------------
Missing (2): fi-glk-j4005 fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_129951v2:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@load:
- fi-bsw-n3050: [PASS][1] -> [ABORT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-bsw-n3050/igt@i915_module_load@load.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-bsw-n3050/igt@i915_module_load@load.html
- fi-skl-6600u: [PASS][3] -> [ABORT][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-skl-6600u/igt@i915_module_load@load.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-skl-6600u/igt@i915_module_load@load.html
- fi-apl-guc: NOTRUN -> [ABORT][5]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-apl-guc/igt@i915_module_load@load.html
- fi-skl-guc: [PASS][6] -> [ABORT][7]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-skl-guc/igt@i915_module_load@load.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-skl-guc/igt@i915_module_load@load.html
- fi-kbl-7567u: [PASS][8] -> [ABORT][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-kbl-7567u/igt@i915_module_load@load.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-kbl-7567u/igt@i915_module_load@load.html
- fi-cfl-8700k: [PASS][10] -> [ABORT][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-cfl-8700k/igt@i915_module_load@load.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-cfl-8700k/igt@i915_module_load@load.html
- fi-bsw-nick: [PASS][12] -> [ABORT][13]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-bsw-nick/igt@i915_module_load@load.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-bsw-nick/igt@i915_module_load@load.html
- bat-kbl-2: [PASS][14] -> [ABORT][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/bat-kbl-2/igt@i915_module_load@load.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/bat-kbl-2/igt@i915_module_load@load.html
- fi-cfl-guc: [PASS][16] -> [ABORT][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-cfl-guc/igt@i915_module_load@load.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-cfl-guc/igt@i915_module_load@load.html
- fi-kbl-x1275: [PASS][18] -> [ABORT][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-kbl-x1275/igt@i915_module_load@load.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-kbl-x1275/igt@i915_module_load@load.html
- fi-cfl-8109u: [PASS][20] -> [ABORT][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-cfl-8109u/igt@i915_module_load@load.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-cfl-8109u/igt@i915_module_load@load.html
- fi-ivb-3770: [PASS][22] -> [ABORT][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-ivb-3770/igt@i915_module_load@load.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-ivb-3770/igt@i915_module_load@load.html
- fi-kbl-guc: [PASS][24] -> [ABORT][25]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-kbl-guc/igt@i915_module_load@load.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-kbl-guc/igt@i915_module_load@load.html
* igt@i915_selftest@live@client:
- fi-elk-e7500: [PASS][26] -> [DMESG-WARN][27] +42 other tests dmesg-warn
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-elk-e7500/igt@i915_selftest@live@client.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-elk-e7500/igt@i915_selftest@live@client.html
* igt@i915_selftest@live@coherency:
- bat-jsl-3: [PASS][28] -> [DMESG-WARN][29] +40 other tests dmesg-warn
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/bat-jsl-3/igt@i915_selftest@live@coherency.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/bat-jsl-3/igt@i915_selftest@live@coherency.html
* igt@i915_selftest@live@gt_contexts:
- fi-ilk-650: [PASS][30] -> [DMESG-WARN][31] +42 other tests dmesg-warn
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-ilk-650/igt@i915_selftest@live@gt_contexts.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-ilk-650/igt@i915_selftest@live@gt_contexts.html
* igt@i915_selftest@live@gt_pm:
- bat-jsl-1: [PASS][32] -> [DMESG-WARN][33] +40 other tests dmesg-warn
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/bat-jsl-1/igt@i915_selftest@live@gt_pm.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/bat-jsl-1/igt@i915_selftest@live@gt_pm.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@runner@aborted:
- {bat-arls-2}: [FAIL][34] ([i915#10237]) -> [FAIL][35]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/bat-arls-2/igt@runner@aborted.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/bat-arls-2/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_129951v2 that come from known issues:
### CI changes ###
#### Possible fixes ####
* boot:
- fi-apl-guc: [FAIL][36] ([i915#8293]) -> [PASS][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/fi-apl-guc/boot.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/fi-apl-guc/boot.html
### IGT changes ###
#### Issues hit ####
* igt@i915_module_load@reload:
- bat-jsl-3: [PASS][38] -> [DMESG-WARN][39] ([i915#1982])
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/bat-jsl-3/igt@i915_module_load@reload.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/bat-jsl-3/igt@i915_module_load@reload.html
- bat-jsl-1: [PASS][40] -> [DMESG-WARN][41] ([i915#1982])
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14300/bat-jsl-1/igt@i915_module_load@reload.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/bat-jsl-1/igt@i915_module_load@reload.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#10237]: https://gitlab.freedesktop.org/drm/intel/issues/10237
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
Build changes
-------------
* Linux: CI_DRM_14300 -> Patchwork_129951v2
CI-20190529: 20190529
CI_DRM_14300: e2b02e89746d8eff8c244f938eecd0f1db8eb805 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7718: 40e8b9122853f455c84afcfa56469a6bc9a0d564 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_129951v2: e2b02e89746d8eff8c244f938eecd0f1db8eb805 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
26221b83843a drm/i915/gt: Set default CCS mode '1'
c1e3306767c1 drm/i915/gt: Disable HW load balancing for CCS
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129951v2/index.html
[-- Attachment #2: Type: text/html, Size: 9343 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-20 14:20 ` [PATCH 2/2] drm/i915/gt: Set default CCS mode '1' Andi Shyti
2024-02-20 14:27 ` Tvrtko Ursulin
@ 2024-02-27 12:18 ` Jani Nikula
2024-02-27 13:01 ` Andi Shyti
1 sibling, 1 reply; 21+ messages in thread
From: Jani Nikula @ 2024-02-27 12:18 UTC (permalink / raw)
To: Andi Shyti, intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, John Harrison,
Tvrtko Ursulin, stable, Andi Shyti, Andi Shyti
On Tue, 20 Feb 2024, Andi Shyti <andi.shyti@linux.intel.com> wrote:
> Since CCS automatic load balancing is disabled, we will impose a
> fixed balancing policy that involves setting all the CCS engines
> to work together on the same load.
>
> Simultaneously, the user will see only 1 CCS rather than the
> actual number. As of now, this change affects only DG2.
>
> Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # v6.2+
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
> drivers/gpu/drm/i915/i915_query.c | 5 +++--
> 4 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index a425db5ed3a2..e19df4ef47f6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
> }
> }
>
> +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
> +{
> + if (!IS_DG2(gt->i915))
> + return;
> +
> + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
> +}
> +
> int intel_gt_init_hw(struct intel_gt *gt)
> {
> struct drm_i915_private *i915 = gt->i915;
> @@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
>
> intel_gt_init_swizzling(gt);
>
> + /* Configure CCS mode */
> + intel_gt_apply_ccs_mode(gt);
> +
> /*
> * At least 830 can leave some of the unused rings
> * "active" (ie. head != tail) after resume which
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index cf709f6c05ae..c148113770ea 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1605,6 +1605,8 @@
> #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
>
> +#define XEHP_CCS_MODE _MMIO(0x14804)
> +
> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
> #define GEN11_CSME (31)
> #define GEN12_HECI_2 (30)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e81b3b2858ac..0853ffd3cb8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
> (engine__); \
> (engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
>
> +/*
> + * Exclude unavailable engines.
> + *
> + * Only the first CCS engine is utilized due to the disabling of CCS auto load
> + * balancing. As a result, all CCS engines operate collectively, functioning
> + * essentially as a single CCS engine, hence the count of active CCS engines is
> + * considered '1'.
> + * Currently, this applies to platforms with more than one CCS engine,
> + * specifically DG2.
> + */
> +#define for_each_available_uabi_engine(engine__, i915__) \
Hrmh, I've been trying to pester folks to move the existing engine
iterator macros away from i915_drv.h, so not happy to see more.
But since this is Cc: stable, better do that in a follow-up. Please?
> + for_each_uabi_engine(engine__, i915__) \
> + if ((IS_DG2(i915__)) && \
> + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
> + ((engine__)->uabi_instance)) { } \
> + else
We have for_each_if for this.
> +
> #define INTEL_INFO(i915) ((i915)->__info)
> #define RUNTIME_INFO(i915) (&(i915)->__runtime)
> #define DRIVER_CAPS(i915) (&(i915)->caps)
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> index fa3e937ed3f5..2d41bda626a6 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
> return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
> }
>
> +
Superfluous newline change.
> static int
> query_engine_info(struct drm_i915_private *i915,
> struct drm_i915_query_item *query_item)
> @@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
> if (query_item->flags)
> return -EINVAL;
>
> - for_each_uabi_engine(engine, i915)
> + for_each_available_uabi_engine(engine, i915)
> num_uabi_engines++;
>
> len = struct_size(query_ptr, engines, num_uabi_engines);
> @@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
>
> info_ptr = &query_ptr->engines[0];
>
> - for_each_uabi_engine(engine, i915) {
> + for_each_available_uabi_engine(engine, i915) {
> info.engine.engine_class = engine->uabi_class;
> info.engine.engine_instance = engine->uabi_instance;
> info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-27 12:18 ` Jani Nikula
@ 2024-02-27 13:01 ` Andi Shyti
0 siblings, 0 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-27 13:01 UTC (permalink / raw)
To: Jani Nikula
Cc: Andi Shyti, intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen,
Matt Roper, John Harrison, Tvrtko Ursulin, stable, Andi Shyti
Hi Jani,
thanks, there has been a v2 after this and your comments have
been addressed somehow.
There will be a v3, as well.
Thanks,
Andi
On Tue, Feb 27, 2024 at 02:18:01PM +0200, Jani Nikula wrote:
> On Tue, 20 Feb 2024, Andi Shyti <andi.shyti@linux.intel.com> wrote:
> > Since CCS automatic load balancing is disabled, we will impose a
> > fixed balancing policy that involves setting all the CCS engines
> > to work together on the same load.
> >
> > Simultaneously, the user will see only 1 CCS rather than the
> > actual number. As of now, this change affects only DG2.
> >
> > Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: <stable@vger.kernel.org> # v6.2+
> > ---
> > drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
> > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
> > drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
> > drivers/gpu/drm/i915/i915_query.c | 5 +++--
> > 4 files changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> > index a425db5ed3a2..e19df4ef47f6 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> > @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
> > }
> > }
> >
> > +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
> > +{
> > + if (!IS_DG2(gt->i915))
> > + return;
> > +
> > + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
> > +}
> > +
> > int intel_gt_init_hw(struct intel_gt *gt)
> > {
> > struct drm_i915_private *i915 = gt->i915;
> > @@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
> >
> > intel_gt_init_swizzling(gt);
> >
> > + /* Configure CCS mode */
> > + intel_gt_apply_ccs_mode(gt);
> > +
> > /*
> > * At least 830 can leave some of the unused rings
> > * "active" (ie. head != tail) after resume which
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index cf709f6c05ae..c148113770ea 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1605,6 +1605,8 @@
> > #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> > #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
> >
> > +#define XEHP_CCS_MODE _MMIO(0x14804)
> > +
> > #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
> > #define GEN11_CSME (31)
> > #define GEN12_HECI_2 (30)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index e81b3b2858ac..0853ffd3cb8d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
> > (engine__); \
> > (engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
> >
> > +/*
> > + * Exclude unavailable engines.
> > + *
> > + * Only the first CCS engine is utilized due to the disabling of CCS auto load
> > + * balancing. As a result, all CCS engines operate collectively, functioning
> > + * essentially as a single CCS engine, hence the count of active CCS engines is
> > + * considered '1'.
> > + * Currently, this applies to platforms with more than one CCS engine,
> > + * specifically DG2.
> > + */
> > +#define for_each_available_uabi_engine(engine__, i915__) \
>
> Hrmh, I've been trying to pester folks to move the existing engine
> iterator macros away from i915_drv.h, so not happy to see more.
>
> But since this is Cc: stable, better do that in a follow-up. Please?
>
> > + for_each_uabi_engine(engine__, i915__) \
> > + if ((IS_DG2(i915__)) && \
> > + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
> > + ((engine__)->uabi_instance)) { } \
> > + else
>
> We have for_each_if for this.
>
> > +
> > #define INTEL_INFO(i915) ((i915)->__info)
> > #define RUNTIME_INFO(i915) (&(i915)->__runtime)
> > #define DRIVER_CAPS(i915) (&(i915)->caps)
> > diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> > index fa3e937ed3f5..2d41bda626a6 100644
> > --- a/drivers/gpu/drm/i915/i915_query.c
> > +++ b/drivers/gpu/drm/i915/i915_query.c
> > @@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
> > return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
> > }
> >
> > +
>
> Superfluous newline change.
>
> > static int
> > query_engine_info(struct drm_i915_private *i915,
> > struct drm_i915_query_item *query_item)
> > @@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
> > if (query_item->flags)
> > return -EINVAL;
> >
> > - for_each_uabi_engine(engine, i915)
> > + for_each_available_uabi_engine(engine, i915)
> > num_uabi_engines++;
> >
> > len = struct_size(query_ptr, engines, num_uabi_engines);
> > @@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
> >
> > info_ptr = &query_ptr->engines[0];
> >
> > - for_each_uabi_engine(engine, i915) {
> > + for_each_available_uabi_engine(engine, i915) {
> > info.engine.engine_class = engine->uabi_class;
> > info.engine.engine_instance = engine->uabi_instance;
> > info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-20 11:15 ` Tvrtko Ursulin
@ 2024-02-20 11:21 ` Andi Shyti
0 siblings, 0 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-20 11:21 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: Andi Shyti, intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen,
Matt Roper, stable, Andi Shyti
On Tue, Feb 20, 2024 at 11:15:05AM +0000, Tvrtko Ursulin wrote:
> On 20/02/2024 10:11, Andi Shyti wrote:
> > On Mon, Feb 19, 2024 at 12:51:44PM +0000, Tvrtko Ursulin wrote:
> > > On 19/02/2024 11:16, Tvrtko Ursulin wrote:
> > > > On 15/02/2024 13:59, Andi Shyti wrote:
> >
> > ...
> >
> > > > > +/*
> > > > > + * Exclude unavailable engines.
> > > > > + *
> > > > > + * Only the first CCS engine is utilized due to the disabling of
> > > > > CCS auto load
> > > > > + * balancing. As a result, all CCS engines operate collectively,
> > > > > functioning
> > > > > + * essentially as a single CCS engine, hence the count of active
> > > > > CCS engines is
> > > > > + * considered '1'.
> > > > > + * Currently, this applies to platforms with more than one CCS engine,
> > > > > + * specifically DG2.
> > > > > + */
> > > > > +#define for_each_available_uabi_engine(engine__, i915__) \
> > > > > + for_each_uabi_engine(engine__, i915__) \
> > > > > + if ((IS_DG2(i915__)) && \
> > > > > + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
> > > > > + ((engine__)->uabi_instance)) { } \
> > > > > + else
> > > > > +
> > > >
> > > > If you don't want userspace to see some engines, just don't add them to
> > > > the uabi list in intel_engines_driver_register or thereabouts?
> >
> > It will be dynamic. In next series I am preparing the user will
> > be able to increase the number of CCS engines he wants to use.
>
> Oh tricky and new. Does it need to be at runtime or could be boot time?
At boot time the CCS mode has to be set to '1', i.e. only one CCS
will be visible to the user. Then, during the normal execution of
the driver, the user can decide to change the mode and therefore
we would need to expose more than one CCS engine.
> If you are aiming to make the static single CCS only into the 6.9 release,
> and you feel running out of time, you could always do a simple solution for
> now. The one I mentioned of simply not registering on the uabi list. Then
> you can refine more leisurely for the next release.
Thanks. I started working on a dynamic rebuilt of the
uabi_engines, but in this series it wouldn't fit.
I will add the limitation during the list creation.
Thanks a lot,
Andi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-20 10:11 ` Andi Shyti
@ 2024-02-20 11:15 ` Tvrtko Ursulin
2024-02-20 11:21 ` Andi Shyti
0 siblings, 1 reply; 21+ messages in thread
From: Tvrtko Ursulin @ 2024-02-20 11:15 UTC (permalink / raw)
To: Andi Shyti
Cc: intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen, Matt Roper,
stable, Andi Shyti
On 20/02/2024 10:11, Andi Shyti wrote:
> Hi Tvrtko,
>
> On Mon, Feb 19, 2024 at 12:51:44PM +0000, Tvrtko Ursulin wrote:
>> On 19/02/2024 11:16, Tvrtko Ursulin wrote:
>>> On 15/02/2024 13:59, Andi Shyti wrote:
>
> ...
>
>>>> +/*
>>>> + * Exclude unavailable engines.
>>>> + *
>>>> + * Only the first CCS engine is utilized due to the disabling of
>>>> CCS auto load
>>>> + * balancing. As a result, all CCS engines operate collectively,
>>>> functioning
>>>> + * essentially as a single CCS engine, hence the count of active
>>>> CCS engines is
>>>> + * considered '1'.
>>>> + * Currently, this applies to platforms with more than one CCS engine,
>>>> + * specifically DG2.
>>>> + */
>>>> +#define for_each_available_uabi_engine(engine__, i915__) \
>>>> + for_each_uabi_engine(engine__, i915__) \
>>>> + if ((IS_DG2(i915__)) && \
>>>> + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
>>>> + ((engine__)->uabi_instance)) { } \
>>>> + else
>>>> +
>>>
>>> If you don't want userspace to see some engines, just don't add them to
>>> the uabi list in intel_engines_driver_register or thereabouts?
>
> It will be dynamic. In next series I am preparing the user will
> be able to increase the number of CCS engines he wants to use.
Oh tricky and new. Does it need to be at runtime or could be boot time?
If you are aiming to make the static single CCS only into the 6.9
release, and you feel running out of time, you could always do a simple
solution for now. The one I mentioned of simply not registering on the
uabi list. Then you can refine more leisurely for the next release.
Regards,
Tvrtko
>
>>> Similar as we do for gsc which uses I915_NO_UABI_CLASS, although for ccs
>>> you can choose a different approach, whatever is more elegant.
>>>
>>> That is also needed for i915->engine_uabi_class_count to be right, so
>>> userspace stats which rely on it are correct.
>
> Oh yes. Will update it.
>
>> I later realized it is more than that - everything that uses
>> intel_engine_lookup_user to look up class instance passed in from userspace
>> relies on the engine not being on the user list otherwise userspace could
>> bypass the fact engine query does not list it. Like PMU, Perf/POA, context
>> engine map and SSEU context query.
>
> Correct, will look into that, thank you!
>
> Andi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-19 12:51 ` Tvrtko Ursulin
@ 2024-02-20 10:11 ` Andi Shyti
2024-02-20 11:15 ` Tvrtko Ursulin
0 siblings, 1 reply; 21+ messages in thread
From: Andi Shyti @ 2024-02-20 10:11 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: Andi Shyti, intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen,
Matt Roper, stable, Andi Shyti
Hi Tvrtko,
On Mon, Feb 19, 2024 at 12:51:44PM +0000, Tvrtko Ursulin wrote:
> On 19/02/2024 11:16, Tvrtko Ursulin wrote:
> > On 15/02/2024 13:59, Andi Shyti wrote:
...
> > > +/*
> > > + * Exclude unavailable engines.
> > > + *
> > > + * Only the first CCS engine is utilized due to the disabling of
> > > CCS auto load
> > > + * balancing. As a result, all CCS engines operate collectively,
> > > functioning
> > > + * essentially as a single CCS engine, hence the count of active
> > > CCS engines is
> > > + * considered '1'.
> > > + * Currently, this applies to platforms with more than one CCS engine,
> > > + * specifically DG2.
> > > + */
> > > +#define for_each_available_uabi_engine(engine__, i915__) \
> > > + for_each_uabi_engine(engine__, i915__) \
> > > + if ((IS_DG2(i915__)) && \
> > > + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
> > > + ((engine__)->uabi_instance)) { } \
> > > + else
> > > +
> >
> > If you don't want userspace to see some engines, just don't add them to
> > the uabi list in intel_engines_driver_register or thereabouts?
It will be dynamic. In next series I am preparing the user will
be able to increase the number of CCS engines he wants to use.
> > Similar as we do for gsc which uses I915_NO_UABI_CLASS, although for ccs
> > you can choose a different approach, whatever is more elegant.
> >
> > That is also needed for i915->engine_uabi_class_count to be right, so
> > userspace stats which rely on it are correct.
Oh yes. Will update it.
> I later realized it is more than that - everything that uses
> intel_engine_lookup_user to look up class instance passed in from userspace
> relies on the engine not being on the user list otherwise userspace could
> bypass the fact engine query does not list it. Like PMU, Perf/POA, context
> engine map and SSEU context query.
Correct, will look into that, thank you!
Andi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-19 11:16 ` Tvrtko Ursulin
@ 2024-02-19 12:51 ` Tvrtko Ursulin
2024-02-20 10:11 ` Andi Shyti
0 siblings, 1 reply; 21+ messages in thread
From: Tvrtko Ursulin @ 2024-02-19 12:51 UTC (permalink / raw)
To: Andi Shyti, intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, stable, Andi Shyti
On 19/02/2024 11:16, Tvrtko Ursulin wrote:
>
> On 15/02/2024 13:59, Andi Shyti wrote:
>> Since CCS automatic load balancing is disabled, we will impose a
>> fixed balancing policy that involves setting all the CCS engines
>> to work together on the same load.
>>
>> Simultaneously, the user will see only 1 CCS rather than the
>> actual number. As of now, this change affects only DG2.
>>
>> Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: <stable@vger.kernel.org> # v6.2+
>> ---
>> drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
>> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
>> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
>> drivers/gpu/drm/i915/i915_query.c | 5 +++--
>> 4 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
>> b/drivers/gpu/drm/i915/gt/intel_gt.c
>> index a425db5ed3a2..e19df4ef47f6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>> @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
>> }
>> }
>> +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
>> +{
>> + if (!IS_DG2(gt->i915))
>> + return;
>> +
>> + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
>> +}
>> +
>> int intel_gt_init_hw(struct intel_gt *gt)
>> {
>> struct drm_i915_private *i915 = gt->i915;
>> @@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
>> intel_gt_init_swizzling(gt);
>> + /* Configure CCS mode */
>> + intel_gt_apply_ccs_mode(gt);
>> +
>> /*
>> * At least 830 can leave some of the unused rings
>> * "active" (ie. head != tail) after resume which
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> index cf709f6c05ae..c148113770ea 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> @@ -1605,6 +1605,8 @@
>> #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
>> #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
>> +#define XEHP_CCS_MODE _MMIO(0x14804)
>> +
>> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
>> #define GEN11_CSME (31)
>> #define GEN12_HECI_2 (30)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index e81b3b2858ac..0853ffd3cb8d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct
>> drm_i915_private *i915)
>> (engine__); \
>> (engine__) =
>> rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
>> +/*
>> + * Exclude unavailable engines.
>> + *
>> + * Only the first CCS engine is utilized due to the disabling of CCS
>> auto load
>> + * balancing. As a result, all CCS engines operate collectively,
>> functioning
>> + * essentially as a single CCS engine, hence the count of active CCS
>> engines is
>> + * considered '1'.
>> + * Currently, this applies to platforms with more than one CCS engine,
>> + * specifically DG2.
>> + */
>> +#define for_each_available_uabi_engine(engine__, i915__) \
>> + for_each_uabi_engine(engine__, i915__) \
>> + if ((IS_DG2(i915__)) && \
>> + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
>> + ((engine__)->uabi_instance)) { } \
>> + else
>> +
>
> If you don't want userspace to see some engines, just don't add them to
> the uabi list in intel_engines_driver_register or thereabouts?
>
> Similar as we do for gsc which uses I915_NO_UABI_CLASS, although for ccs
> you can choose a different approach, whatever is more elegant.
>
> That is also needed for i915->engine_uabi_class_count to be right, so
> userspace stats which rely on it are correct.
I later realized it is more than that - everything that uses
intel_engine_lookup_user to look up class instance passed in from
userspace relies on the engine not being on the user list otherwise
userspace could bypass the fact engine query does not list it. Like PMU,
Perf/POA, context engine map and SSEU context query.
Regards,
Tvrtko
>
> Regards,
>
> Tvrtko
>
>> #define INTEL_INFO(i915) ((i915)->__info)
>> #define RUNTIME_INFO(i915) (&(i915)->__runtime)
>> #define DRIVER_CAPS(i915) (&(i915)->caps)
>> diff --git a/drivers/gpu/drm/i915/i915_query.c
>> b/drivers/gpu/drm/i915/i915_query.c
>> index fa3e937ed3f5..2d41bda626a6 100644
>> --- a/drivers/gpu/drm/i915/i915_query.c
>> +++ b/drivers/gpu/drm/i915/i915_query.c
>> @@ -124,6 +124,7 @@ static int query_geometry_subslices(struct
>> drm_i915_private *i915,
>> return fill_topology_info(sseu, query_item,
>> sseu->geometry_subslice_mask);
>> }
>> +
>> static int
>> query_engine_info(struct drm_i915_private *i915,
>> struct drm_i915_query_item *query_item)
>> @@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
>> if (query_item->flags)
>> return -EINVAL;
>> - for_each_uabi_engine(engine, i915)
>> + for_each_available_uabi_engine(engine, i915)
>> num_uabi_engines++;
>> len = struct_size(query_ptr, engines, num_uabi_engines);
>> @@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
>> info_ptr = &query_ptr->engines[0];
>> - for_each_uabi_engine(engine, i915) {
>> + for_each_available_uabi_engine(engine, i915) {
>> info.engine.engine_class = engine->uabi_class;
>> info.engine.engine_instance = engine->uabi_instance;
>> info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-15 13:59 ` [PATCH 2/2] drm/i915/gt: Set default CCS mode '1' Andi Shyti
2024-02-15 21:23 ` John Harrison
@ 2024-02-19 11:16 ` Tvrtko Ursulin
2024-02-19 12:51 ` Tvrtko Ursulin
1 sibling, 1 reply; 21+ messages in thread
From: Tvrtko Ursulin @ 2024-02-19 11:16 UTC (permalink / raw)
To: Andi Shyti, intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, stable, Andi Shyti
On 15/02/2024 13:59, Andi Shyti wrote:
> Since CCS automatic load balancing is disabled, we will impose a
> fixed balancing policy that involves setting all the CCS engines
> to work together on the same load.
>
> Simultaneously, the user will see only 1 CCS rather than the
> actual number. As of now, this change affects only DG2.
>
> Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # v6.2+
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
> drivers/gpu/drm/i915/i915_query.c | 5 +++--
> 4 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index a425db5ed3a2..e19df4ef47f6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
> }
> }
>
> +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
> +{
> + if (!IS_DG2(gt->i915))
> + return;
> +
> + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
> +}
> +
> int intel_gt_init_hw(struct intel_gt *gt)
> {
> struct drm_i915_private *i915 = gt->i915;
> @@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
>
> intel_gt_init_swizzling(gt);
>
> + /* Configure CCS mode */
> + intel_gt_apply_ccs_mode(gt);
> +
> /*
> * At least 830 can leave some of the unused rings
> * "active" (ie. head != tail) after resume which
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index cf709f6c05ae..c148113770ea 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1605,6 +1605,8 @@
> #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
>
> +#define XEHP_CCS_MODE _MMIO(0x14804)
> +
> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
> #define GEN11_CSME (31)
> #define GEN12_HECI_2 (30)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e81b3b2858ac..0853ffd3cb8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
> (engine__); \
> (engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
>
> +/*
> + * Exclude unavailable engines.
> + *
> + * Only the first CCS engine is utilized due to the disabling of CCS auto load
> + * balancing. As a result, all CCS engines operate collectively, functioning
> + * essentially as a single CCS engine, hence the count of active CCS engines is
> + * considered '1'.
> + * Currently, this applies to platforms with more than one CCS engine,
> + * specifically DG2.
> + */
> +#define for_each_available_uabi_engine(engine__, i915__) \
> + for_each_uabi_engine(engine__, i915__) \
> + if ((IS_DG2(i915__)) && \
> + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
> + ((engine__)->uabi_instance)) { } \
> + else
> +
If you don't want userspace to see some engines, just don't add them to
the uabi list in intel_engines_driver_register or thereabouts?
Similar as we do for gsc which uses I915_NO_UABI_CLASS, although for ccs
you can choose a different approach, whatever is more elegant.
That is also needed for i915->engine_uabi_class_count to be right, so
userspace stats which rely on it are correct.
Regards,
Tvrtko
> #define INTEL_INFO(i915) ((i915)->__info)
> #define RUNTIME_INFO(i915) (&(i915)->__runtime)
> #define DRIVER_CAPS(i915) (&(i915)->caps)
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> index fa3e937ed3f5..2d41bda626a6 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
> return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
> }
>
> +
> static int
> query_engine_info(struct drm_i915_private *i915,
> struct drm_i915_query_item *query_item)
> @@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
> if (query_item->flags)
> return -EINVAL;
>
> - for_each_uabi_engine(engine, i915)
> + for_each_available_uabi_engine(engine, i915)
> num_uabi_engines++;
>
> len = struct_size(query_ptr, engines, num_uabi_engines);
> @@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
>
> info_ptr = &query_ptr->engines[0];
>
> - for_each_uabi_engine(engine, i915) {
> + for_each_available_uabi_engine(engine, i915) {
> info.engine.engine_class = engine->uabi_class;
> info.engine.engine_instance = engine->uabi_instance;
> info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-15 22:34 ` Andi Shyti
@ 2024-02-15 22:55 ` John Harrison
0 siblings, 0 replies; 21+ messages in thread
From: John Harrison @ 2024-02-15 22:55 UTC (permalink / raw)
To: Andi Shyti
Cc: intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen, Matt Roper,
stable, Andi Shyti
On 2/15/2024 14:34, Andi Shyti wrote:
> Hi John,
>
> On Thu, Feb 15, 2024 at 01:23:24PM -0800, John Harrison wrote:
>> On 2/15/2024 05:59, Andi Shyti wrote:
>>> Since CCS automatic load balancing is disabled, we will impose a
>>> fixed balancing policy that involves setting all the CCS engines
>>> to work together on the same load.
>>>
>>> Simultaneously, the user will see only 1 CCS rather than the
>>> actual number. As of now, this change affects only DG2.
>> These two paragraphs are mutually exclusive. You can't have four CCS engines
>> 'working together' if only one engine exists. I think you are meaning that
>> we only export 1 CCS engine and that single engine is configured to control
>> all the EUs. As opposed to running in 4 CCS engine mode where the EUs are
>> (dynamically or statically) divided amongst those four engines.
> The balancing is done statically. The dynamic balancing is
> disabled in patch 1.
>
> The 2 or 4 CCS engines will share the same workload.
But they don't.
In i915, we use 'engine' to refer to a command streamer and all the
associated hardware. This is distinct from the EUs which sit behind and
can be driven by one or more command streamers. Saying that multiple
engines are sharing a workload implies that you are submitting the
context to multiple command streamers in parallel. I.e. a similar
process to media frame split where they have a set of LRCA contexts
bound together which are submitted in parallel to two or more video
decode engines (VCS0, VCS1, etc.). That is not what is happening here.
Here, you are submitting a single context with a singe ring buffer to a
single engine - CCS0. That engine is configured to own all EUs. Which
actually means that submitting a compute task to another CCS engine will
achieve nothing because there are no EUs available to those other
engines. They will simply hang when waiting for the walker instruction
to complete.
>
> Because the user won't be able anymore to select the CCS engine
> he wants to use, he will see only one CCS.
>
> I think we are saying the same thing using different words :)
But words are important.
John.
> I can try in v2 to reword the commit better.
>
> Thanks for looking into this.
> Andi
>
>> John.
>>
>>> Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
>>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>>> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Cc: <stable@vger.kernel.org> # v6.2+
>>> ---
>>> drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
>>> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
>>> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
>>> drivers/gpu/drm/i915/i915_query.c | 5 +++--
>>> 4 files changed, 33 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
>>> index a425db5ed3a2..e19df4ef47f6 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>>> @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
>>> }
>>> }
>>> +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
>>> +{
>>> + if (!IS_DG2(gt->i915))
>>> + return;
>>> +
>>> + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
>>> +}
>>> +
>>> int intel_gt_init_hw(struct intel_gt *gt)
>>> {
>>> struct drm_i915_private *i915 = gt->i915;
>>> @@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
>>> intel_gt_init_swizzling(gt);
>>> + /* Configure CCS mode */
>>> + intel_gt_apply_ccs_mode(gt);
>>> +
>>> /*
>>> * At least 830 can leave some of the unused rings
>>> * "active" (ie. head != tail) after resume which
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> index cf709f6c05ae..c148113770ea 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> @@ -1605,6 +1605,8 @@
>>> #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
>>> #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
>>> +#define XEHP_CCS_MODE _MMIO(0x14804)
>>> +
>>> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
>>> #define GEN11_CSME (31)
>>> #define GEN12_HECI_2 (30)
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index e81b3b2858ac..0853ffd3cb8d 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
>>> (engine__); \
>>> (engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
>>> +/*
>>> + * Exclude unavailable engines.
>>> + *
>>> + * Only the first CCS engine is utilized due to the disabling of CCS auto load
>>> + * balancing. As a result, all CCS engines operate collectively, functioning
>>> + * essentially as a single CCS engine, hence the count of active CCS engines is
>>> + * considered '1'.
>>> + * Currently, this applies to platforms with more than one CCS engine,
>>> + * specifically DG2.
>>> + */
>>> +#define for_each_available_uabi_engine(engine__, i915__) \
>>> + for_each_uabi_engine(engine__, i915__) \
>>> + if ((IS_DG2(i915__)) && \
>>> + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
>>> + ((engine__)->uabi_instance)) { } \
>>> + else
>>> +
>>> #define INTEL_INFO(i915) ((i915)->__info)
>>> #define RUNTIME_INFO(i915) (&(i915)->__runtime)
>>> #define DRIVER_CAPS(i915) (&(i915)->caps)
>>> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
>>> index fa3e937ed3f5..2d41bda626a6 100644
>>> --- a/drivers/gpu/drm/i915/i915_query.c
>>> +++ b/drivers/gpu/drm/i915/i915_query.c
>>> @@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
>>> return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
>>> }
>>> +
>>> static int
>>> query_engine_info(struct drm_i915_private *i915,
>>> struct drm_i915_query_item *query_item)
>>> @@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
>>> if (query_item->flags)
>>> return -EINVAL;
>>> - for_each_uabi_engine(engine, i915)
>>> + for_each_available_uabi_engine(engine, i915)
>>> num_uabi_engines++;
>>> len = struct_size(query_ptr, engines, num_uabi_engines);
>>> @@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
>>> info_ptr = &query_ptr->engines[0];
>>> - for_each_uabi_engine(engine, i915) {
>>> + for_each_available_uabi_engine(engine, i915) {
>>> info.engine.engine_class = engine->uabi_class;
>>> info.engine.engine_instance = engine->uabi_instance;
>>> info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-15 21:23 ` John Harrison
@ 2024-02-15 22:34 ` Andi Shyti
2024-02-15 22:55 ` John Harrison
0 siblings, 1 reply; 21+ messages in thread
From: Andi Shyti @ 2024-02-15 22:34 UTC (permalink / raw)
To: John Harrison
Cc: Andi Shyti, intel-gfx, dri-devel, Chris Wilson, Joonas Lahtinen,
Matt Roper, stable, Andi Shyti
Hi John,
On Thu, Feb 15, 2024 at 01:23:24PM -0800, John Harrison wrote:
> On 2/15/2024 05:59, Andi Shyti wrote:
> > Since CCS automatic load balancing is disabled, we will impose a
> > fixed balancing policy that involves setting all the CCS engines
> > to work together on the same load.
> >
> > Simultaneously, the user will see only 1 CCS rather than the
> > actual number. As of now, this change affects only DG2.
> These two paragraphs are mutually exclusive. You can't have four CCS engines
> 'working together' if only one engine exists. I think you are meaning that
> we only export 1 CCS engine and that single engine is configured to control
> all the EUs. As opposed to running in 4 CCS engine mode where the EUs are
> (dynamically or statically) divided amongst those four engines.
The balancing is done statically. The dynamic balancing is
disabled in patch 1.
The 2 or 4 CCS engines will share the same workload.
Because the user won't be able anymore to select the CCS engine
he wants to use, he will see only one CCS.
I think we are saying the same thing using different words :)
I can try in v2 to reword the commit better.
Thanks for looking into this.
Andi
> John.
>
> >
> > Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: <stable@vger.kernel.org> # v6.2+
> > ---
> > drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
> > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
> > drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
> > drivers/gpu/drm/i915/i915_query.c | 5 +++--
> > 4 files changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> > index a425db5ed3a2..e19df4ef47f6 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> > @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
> > }
> > }
> > +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
> > +{
> > + if (!IS_DG2(gt->i915))
> > + return;
> > +
> > + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
> > +}
> > +
> > int intel_gt_init_hw(struct intel_gt *gt)
> > {
> > struct drm_i915_private *i915 = gt->i915;
> > @@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
> > intel_gt_init_swizzling(gt);
> > + /* Configure CCS mode */
> > + intel_gt_apply_ccs_mode(gt);
> > +
> > /*
> > * At least 830 can leave some of the unused rings
> > * "active" (ie. head != tail) after resume which
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index cf709f6c05ae..c148113770ea 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1605,6 +1605,8 @@
> > #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> > #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
> > +#define XEHP_CCS_MODE _MMIO(0x14804)
> > +
> > #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
> > #define GEN11_CSME (31)
> > #define GEN12_HECI_2 (30)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index e81b3b2858ac..0853ffd3cb8d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
> > (engine__); \
> > (engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
> > +/*
> > + * Exclude unavailable engines.
> > + *
> > + * Only the first CCS engine is utilized due to the disabling of CCS auto load
> > + * balancing. As a result, all CCS engines operate collectively, functioning
> > + * essentially as a single CCS engine, hence the count of active CCS engines is
> > + * considered '1'.
> > + * Currently, this applies to platforms with more than one CCS engine,
> > + * specifically DG2.
> > + */
> > +#define for_each_available_uabi_engine(engine__, i915__) \
> > + for_each_uabi_engine(engine__, i915__) \
> > + if ((IS_DG2(i915__)) && \
> > + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
> > + ((engine__)->uabi_instance)) { } \
> > + else
> > +
> > #define INTEL_INFO(i915) ((i915)->__info)
> > #define RUNTIME_INFO(i915) (&(i915)->__runtime)
> > #define DRIVER_CAPS(i915) (&(i915)->caps)
> > diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> > index fa3e937ed3f5..2d41bda626a6 100644
> > --- a/drivers/gpu/drm/i915/i915_query.c
> > +++ b/drivers/gpu/drm/i915/i915_query.c
> > @@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
> > return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
> > }
> > +
> > static int
> > query_engine_info(struct drm_i915_private *i915,
> > struct drm_i915_query_item *query_item)
> > @@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
> > if (query_item->flags)
> > return -EINVAL;
> > - for_each_uabi_engine(engine, i915)
> > + for_each_available_uabi_engine(engine, i915)
> > num_uabi_engines++;
> > len = struct_size(query_ptr, engines, num_uabi_engines);
> > @@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
> > info_ptr = &query_ptr->engines[0];
> > - for_each_uabi_engine(engine, i915) {
> > + for_each_available_uabi_engine(engine, i915) {
> > info.engine.engine_class = engine->uabi_class;
> > info.engine.engine_instance = engine->uabi_instance;
> > info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-15 13:59 ` [PATCH 2/2] drm/i915/gt: Set default CCS mode '1' Andi Shyti
@ 2024-02-15 21:23 ` John Harrison
2024-02-15 22:34 ` Andi Shyti
2024-02-19 11:16 ` Tvrtko Ursulin
1 sibling, 1 reply; 21+ messages in thread
From: John Harrison @ 2024-02-15 21:23 UTC (permalink / raw)
To: Andi Shyti, intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, stable, Andi Shyti
On 2/15/2024 05:59, Andi Shyti wrote:
> Since CCS automatic load balancing is disabled, we will impose a
> fixed balancing policy that involves setting all the CCS engines
> to work together on the same load.
>
> Simultaneously, the user will see only 1 CCS rather than the
> actual number. As of now, this change affects only DG2.
These two paragraphs are mutually exclusive. You can't have four CCS
engines 'working together' if only one engine exists. I think you are
meaning that we only export 1 CCS engine and that single engine is
configured to control all the EUs. As opposed to running in 4 CCS engine
mode where the EUs are (dynamically or statically) divided amongst those
four engines.
John.
>
> Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # v6.2+
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
> drivers/gpu/drm/i915/i915_query.c | 5 +++--
> 4 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index a425db5ed3a2..e19df4ef47f6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
> }
> }
>
> +static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
> +{
> + if (!IS_DG2(gt->i915))
> + return;
> +
> + intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
> +}
> +
> int intel_gt_init_hw(struct intel_gt *gt)
> {
> struct drm_i915_private *i915 = gt->i915;
> @@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
>
> intel_gt_init_swizzling(gt);
>
> + /* Configure CCS mode */
> + intel_gt_apply_ccs_mode(gt);
> +
> /*
> * At least 830 can leave some of the unused rings
> * "active" (ie. head != tail) after resume which
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index cf709f6c05ae..c148113770ea 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1605,6 +1605,8 @@
> #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
>
> +#define XEHP_CCS_MODE _MMIO(0x14804)
> +
> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
> #define GEN11_CSME (31)
> #define GEN12_HECI_2 (30)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e81b3b2858ac..0853ffd3cb8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
> (engine__); \
> (engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
>
> +/*
> + * Exclude unavailable engines.
> + *
> + * Only the first CCS engine is utilized due to the disabling of CCS auto load
> + * balancing. As a result, all CCS engines operate collectively, functioning
> + * essentially as a single CCS engine, hence the count of active CCS engines is
> + * considered '1'.
> + * Currently, this applies to platforms with more than one CCS engine,
> + * specifically DG2.
> + */
> +#define for_each_available_uabi_engine(engine__, i915__) \
> + for_each_uabi_engine(engine__, i915__) \
> + if ((IS_DG2(i915__)) && \
> + ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
> + ((engine__)->uabi_instance)) { } \
> + else
> +
> #define INTEL_INFO(i915) ((i915)->__info)
> #define RUNTIME_INFO(i915) (&(i915)->__runtime)
> #define DRIVER_CAPS(i915) (&(i915)->caps)
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> index fa3e937ed3f5..2d41bda626a6 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
> return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
> }
>
> +
> static int
> query_engine_info(struct drm_i915_private *i915,
> struct drm_i915_query_item *query_item)
> @@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
> if (query_item->flags)
> return -EINVAL;
>
> - for_each_uabi_engine(engine, i915)
> + for_each_available_uabi_engine(engine, i915)
> num_uabi_engines++;
>
> len = struct_size(query_ptr, engines, num_uabi_engines);
> @@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
>
> info_ptr = &query_ptr->engines[0];
>
> - for_each_uabi_engine(engine, i915) {
> + for_each_available_uabi_engine(engine, i915) {
> info.engine.engine_class = engine->uabi_class;
> info.engine.engine_instance = engine->uabi_instance;
> info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/2] drm/i915/gt: Set default CCS mode '1'
2024-02-15 13:59 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
@ 2024-02-15 13:59 ` Andi Shyti
2024-02-15 21:23 ` John Harrison
2024-02-19 11:16 ` Tvrtko Ursulin
0 siblings, 2 replies; 21+ messages in thread
From: Andi Shyti @ 2024-02-15 13:59 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: Chris Wilson, Joonas Lahtinen, Matt Roper, stable, Andi Shyti,
Andi Shyti
Since CCS automatic load balancing is disabled, we will impose a
fixed balancing policy that involves setting all the CCS engines
to work together on the same load.
Simultaneously, the user will see only 1 CCS rather than the
actual number. As of now, this change affects only DG2.
Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: <stable@vger.kernel.org> # v6.2+
---
drivers/gpu/drm/i915/gt/intel_gt.c | 11 +++++++++++
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
drivers/gpu/drm/i915/i915_query.c | 5 +++--
4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index a425db5ed3a2..e19df4ef47f6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -168,6 +168,14 @@ static void init_unused_rings(struct intel_gt *gt)
}
}
+static void intel_gt_apply_ccs_mode(struct intel_gt *gt)
+{
+ if (!IS_DG2(gt->i915))
+ return;
+
+ intel_uncore_write(gt->uncore, XEHP_CCS_MODE, 0);
+}
+
int intel_gt_init_hw(struct intel_gt *gt)
{
struct drm_i915_private *i915 = gt->i915;
@@ -195,6 +203,9 @@ int intel_gt_init_hw(struct intel_gt *gt)
intel_gt_init_swizzling(gt);
+ /* Configure CCS mode */
+ intel_gt_apply_ccs_mode(gt);
+
/*
* At least 830 can leave some of the unused rings
* "active" (ie. head != tail) after resume which
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index cf709f6c05ae..c148113770ea 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1605,6 +1605,8 @@
#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
#define GEN12_CAGF_MASK REG_GENMASK(19, 11)
+#define XEHP_CCS_MODE _MMIO(0x14804)
+
#define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
#define GEN11_CSME (31)
#define GEN12_HECI_2 (30)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e81b3b2858ac..0853ffd3cb8d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -396,6 +396,23 @@ static inline struct intel_gt *to_gt(const struct drm_i915_private *i915)
(engine__); \
(engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
+/*
+ * Exclude unavailable engines.
+ *
+ * Only the first CCS engine is utilized due to the disabling of CCS auto load
+ * balancing. As a result, all CCS engines operate collectively, functioning
+ * essentially as a single CCS engine, hence the count of active CCS engines is
+ * considered '1'.
+ * Currently, this applies to platforms with more than one CCS engine,
+ * specifically DG2.
+ */
+#define for_each_available_uabi_engine(engine__, i915__) \
+ for_each_uabi_engine(engine__, i915__) \
+ if ((IS_DG2(i915__)) && \
+ ((engine__)->uabi_class == I915_ENGINE_CLASS_COMPUTE) && \
+ ((engine__)->uabi_instance)) { } \
+ else
+
#define INTEL_INFO(i915) ((i915)->__info)
#define RUNTIME_INFO(i915) (&(i915)->__runtime)
#define DRIVER_CAPS(i915) (&(i915)->caps)
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index fa3e937ed3f5..2d41bda626a6 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -124,6 +124,7 @@ static int query_geometry_subslices(struct drm_i915_private *i915,
return fill_topology_info(sseu, query_item, sseu->geometry_subslice_mask);
}
+
static int
query_engine_info(struct drm_i915_private *i915,
struct drm_i915_query_item *query_item)
@@ -140,7 +141,7 @@ query_engine_info(struct drm_i915_private *i915,
if (query_item->flags)
return -EINVAL;
- for_each_uabi_engine(engine, i915)
+ for_each_available_uabi_engine(engine, i915)
num_uabi_engines++;
len = struct_size(query_ptr, engines, num_uabi_engines);
@@ -155,7 +156,7 @@ query_engine_info(struct drm_i915_private *i915,
info_ptr = &query_ptr->engines[0];
- for_each_uabi_engine(engine, i915) {
+ for_each_available_uabi_engine(engine, i915) {
info.engine.engine_class = engine->uabi_class;
info.engine.engine_instance = engine->uabi_instance;
info.flags = I915_ENGINE_INFO_HAS_LOGICAL_INSTANCE;
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2024-02-27 13:01 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-20 14:20 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
2024-02-20 14:20 ` [PATCH 1/2] drm/i915/gt: Disable HW load balancing for CCS Andi Shyti
2024-02-20 14:20 ` [PATCH 2/2] drm/i915/gt: Set default CCS mode '1' Andi Shyti
2024-02-20 14:27 ` Tvrtko Ursulin
2024-02-20 14:33 ` Andi Shyti
2024-02-27 12:18 ` Jani Nikula
2024-02-27 13:01 ` Andi Shyti
2024-02-20 14:23 ` [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
2024-02-20 14:34 ` Andi Shyti
2024-02-20 14:47 ` ✗ Fi.CI.CHECKPATCH: warning for Disable automatic load CCS load balancing (rev2) Patchwork
2024-02-20 14:47 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-02-20 15:00 ` ✗ Fi.CI.BAT: failure " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2024-02-15 13:59 [PATCH 0/2] Disable automatic load CCS load balancing Andi Shyti
2024-02-15 13:59 ` [PATCH 2/2] drm/i915/gt: Set default CCS mode '1' Andi Shyti
2024-02-15 21:23 ` John Harrison
2024-02-15 22:34 ` Andi Shyti
2024-02-15 22:55 ` John Harrison
2024-02-19 11:16 ` Tvrtko Ursulin
2024-02-19 12:51 ` Tvrtko Ursulin
2024-02-20 10:11 ` Andi Shyti
2024-02-20 11:15 ` Tvrtko Ursulin
2024-02-20 11:21 ` Andi Shyti
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.