* [PATCH 1/4] drm/i915/gt: fix platform prefix
2021-07-20 23:20 [PATCH 0/4] Nuke legacy hw_id Lucas De Marchi
@ 2021-07-20 23:20 ` Lucas De Marchi
2021-07-21 9:11 ` [Intel-gfx] " Tvrtko Ursulin
2021-07-21 22:34 ` Matt Roper
2021-07-20 23:20 ` [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id Lucas De Marchi
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin
gen8_clear_engine_error_register() is actually not used by
GRAPHICS_VER >= 8, since for those we are using another register that is
not engine-dependent. Fix the platform prefix, to make clear we are not
using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index e714e21c0a4d..a8efdd44e9cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
intel_uncore_rmw(uncore, reg, 0, 0);
}
-static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
+static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
{
GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
GEN6_RING_FAULT_REG_POSTING_READ(engine);
@@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
enum intel_engine_id id;
for_each_engine_masked(engine, gt, engine_mask, id)
- gen8_clear_engine_error_register(engine);
+ gen6_clear_engine_error_register(engine);
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/gt: fix platform prefix
2021-07-20 23:20 ` [PATCH 1/4] drm/i915/gt: fix platform prefix Lucas De Marchi
@ 2021-07-21 9:11 ` Tvrtko Ursulin
2021-07-21 22:34 ` Matt Roper
1 sibling, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21 9:11 UTC (permalink / raw)
To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel
On 21/07/2021 00:20, Lucas De Marchi wrote:
> gen8_clear_engine_error_register() is actually not used by
> GRAPHICS_VER >= 8, since for those we are using another register that is
> not engine-dependent. Fix the platform prefix, to make clear we are not
> using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index e714e21c0a4d..a8efdd44e9cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
> intel_uncore_rmw(uncore, reg, 0, 0);
> }
>
> -static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> +static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
> {
> GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
> GEN6_RING_FAULT_REG_POSTING_READ(engine);
> @@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
> enum intel_engine_id id;
>
> for_each_engine_masked(engine, gt, engine_mask, id)
> - gen8_clear_engine_error_register(engine);
> + gen6_clear_engine_error_register(engine);
> }
> }
>
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] drm/i915/gt: fix platform prefix
2021-07-20 23:20 ` [PATCH 1/4] drm/i915/gt: fix platform prefix Lucas De Marchi
2021-07-21 9:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-07-21 22:34 ` Matt Roper
1 sibling, 0 replies; 17+ messages in thread
From: Matt Roper @ 2021-07-21 22:34 UTC (permalink / raw)
To: Lucas De Marchi
Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison
On Tue, Jul 20, 2021 at 04:20:11PM -0700, Lucas De Marchi wrote:
> gen8_clear_engine_error_register() is actually not used by
> GRAPHICS_VER >= 8, since for those we are using another register that is
> not engine-dependent. Fix the platform prefix, to make clear we are not
> using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index e714e21c0a4d..a8efdd44e9cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
> intel_uncore_rmw(uncore, reg, 0, 0);
> }
>
> -static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> +static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
> {
> GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
> GEN6_RING_FAULT_REG_POSTING_READ(engine);
> @@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
> enum intel_engine_id id;
>
> for_each_engine_masked(engine, gt, engine_mask, id)
> - gen8_clear_engine_error_register(engine);
> + gen6_clear_engine_error_register(engine);
> }
> }
>
> --
> 2.31.1
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
2021-07-20 23:20 [PATCH 0/4] Nuke legacy hw_id Lucas De Marchi
2021-07-20 23:20 ` [PATCH 1/4] drm/i915/gt: fix platform prefix Lucas De Marchi
@ 2021-07-20 23:20 ` Lucas De Marchi
2021-07-21 9:18 ` [Intel-gfx] " Tvrtko Ursulin
2021-07-21 22:47 ` Matt Roper
2021-07-20 23:20 ` [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id Lucas De Marchi
2021-07-20 23:20 ` [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id Lucas De Marchi
3 siblings, 2 replies; 17+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin
The engine hw_id is only used by RING_FAULT_REG(), which is not used
since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
consistent, but let's try to remove them and let them defined to 0 when
not used.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 ----
drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
2 files changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index d561573ed98c..a11f69f2e46e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
},
},
[VCS1] = {
- .hw_id = VCS1_HW,
.class = VIDEO_DECODE_CLASS,
.instance = 1,
.mmio_bases = {
@@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
},
},
[VCS2] = {
- .hw_id = VCS2_HW,
.class = VIDEO_DECODE_CLASS,
.instance = 2,
.mmio_bases = {
@@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
},
},
[VCS3] = {
- .hw_id = VCS3_HW,
.class = VIDEO_DECODE_CLASS,
.instance = 3,
.mmio_bases = {
@@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
},
},
[VECS1] = {
- .hw_id = VECS1_HW,
.class = VIDEO_ENHANCEMENT_CLASS,
.instance = 1,
.mmio_bases = {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 1cb9c3b70b29..a107eb58ffa2 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -34,10 +34,6 @@
#define VCS0_HW 1
#define BCS0_HW 2
#define VECS0_HW 3
-#define VCS1_HW 4
-#define VCS2_HW 6
-#define VCS3_HW 7
-#define VECS1_HW 12
/* Gen11+ HW Engine class + instance */
#define RENDER_CLASS 0
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
2021-07-20 23:20 ` [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id Lucas De Marchi
@ 2021-07-21 9:18 ` Tvrtko Ursulin
2021-07-21 22:47 ` Matt Roper
1 sibling, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21 9:18 UTC (permalink / raw)
To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel
On 21/07/2021 00:20, Lucas De Marchi wrote:
> The engine hw_id is only used by RING_FAULT_REG(), which is not used
> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
> consistent, but let's try to remove them and let them defined to 0 when
> not used.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 ----
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
> 2 files changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index d561573ed98c..a11f69f2e46e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS1] = {
> - .hw_id = VCS1_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 1,
> .mmio_bases = {
> @@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS2] = {
> - .hw_id = VCS2_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 2,
> .mmio_bases = {
> @@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS3] = {
> - .hw_id = VCS3_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 3,
> .mmio_bases = {
> @@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VECS1] = {
> - .hw_id = VECS1_HW,
> .class = VIDEO_ENHANCEMENT_CLASS,
> .instance = 1,
> .mmio_bases = {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 1cb9c3b70b29..a107eb58ffa2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -34,10 +34,6 @@
> #define VCS0_HW 1
> #define BCS0_HW 2
> #define VECS0_HW 3
> -#define VCS1_HW 4
> -#define VCS2_HW 6
> -#define VCS3_HW 7
> -#define VECS1_HW 12
>
> /* Gen11+ HW Engine class + instance */
> #define RENDER_CLASS 0
>
Story checks out AFAICS.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
2021-07-20 23:20 ` [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id Lucas De Marchi
2021-07-21 9:18 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-07-21 22:47 ` Matt Roper
2021-07-21 23:17 ` Lucas De Marchi
1 sibling, 1 reply; 17+ messages in thread
From: Matt Roper @ 2021-07-21 22:47 UTC (permalink / raw)
To: Lucas De Marchi
Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison
On Tue, Jul 20, 2021 at 04:20:12PM -0700, Lucas De Marchi wrote:
> The engine hw_id is only used by RING_FAULT_REG(), which is not used
> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
> consistent, but let's try to remove them and let them defined to 0 when
> not used.
s/when not used/for engines that only exist on gen8+ platforms/
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
For historical reference, we did use hw_id on gen8+ platforms too until
relatively recently --- it was used to set the engine's guc_id as well
up until:
commit c784e5249e773689e38d2bc1749f08b986621a26
Author: John Harrison <John.C.Harrison@Intel.com>
Date: Wed Oct 28 07:58:24 2020 -0700
drm/i915/guc: Update to use firmware v49.0.1
Matt
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 ----
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
> 2 files changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index d561573ed98c..a11f69f2e46e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS1] = {
> - .hw_id = VCS1_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 1,
> .mmio_bases = {
> @@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS2] = {
> - .hw_id = VCS2_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 2,
> .mmio_bases = {
> @@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS3] = {
> - .hw_id = VCS3_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 3,
> .mmio_bases = {
> @@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VECS1] = {
> - .hw_id = VECS1_HW,
> .class = VIDEO_ENHANCEMENT_CLASS,
> .instance = 1,
> .mmio_bases = {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 1cb9c3b70b29..a107eb58ffa2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -34,10 +34,6 @@
> #define VCS0_HW 1
> #define BCS0_HW 2
> #define VECS0_HW 3
> -#define VCS1_HW 4
> -#define VCS2_HW 6
> -#define VCS3_HW 7
> -#define VECS1_HW 12
>
> /* Gen11+ HW Engine class + instance */
> #define RENDER_CLASS 0
> --
> 2.31.1
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
2021-07-21 22:47 ` Matt Roper
@ 2021-07-21 23:17 ` Lucas De Marchi
0 siblings, 0 replies; 17+ messages in thread
From: Lucas De Marchi @ 2021-07-21 23:17 UTC (permalink / raw)
To: Matt Roper
Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison
On Wed, Jul 21, 2021 at 03:47:22PM -0700, Matt Roper wrote:
>On Tue, Jul 20, 2021 at 04:20:12PM -0700, Lucas De Marchi wrote:
>> The engine hw_id is only used by RING_FAULT_REG(), which is not used
>> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
>> consistent, but let's try to remove them and let them defined to 0 when
>> not used.
>
>s/when not used/for engines that only exist on gen8+ platforms/
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
>For historical reference, we did use hw_id on gen8+ platforms too until
>relatively recently --- it was used to set the engine's guc_id as well
>up until:
>
> commit c784e5249e773689e38d2bc1749f08b986621a26
> Author: John Harrison <John.C.Harrison@Intel.com>
> Date: Wed Oct 28 07:58:24 2020 -0700
>
> drm/i915/guc: Update to use firmware v49.0.1
thanks for digging this, I will add that to the commit message as well.
Lucas De Marchi
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
2021-07-20 23:20 [PATCH 0/4] Nuke legacy hw_id Lucas De Marchi
2021-07-20 23:20 ` [PATCH 1/4] drm/i915/gt: fix platform prefix Lucas De Marchi
2021-07-20 23:20 ` [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id Lucas De Marchi
@ 2021-07-20 23:20 ` Lucas De Marchi
2021-07-21 9:20 ` [Intel-gfx] " Tvrtko Ursulin
2021-07-21 22:51 ` Matt Roper
2021-07-20 23:20 ` [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id Lucas De Marchi
3 siblings, 2 replies; 17+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin
We kept adding new engines and for that increasing hw_id unnecessarily:
it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
try to pack it in the structs to give a hint this field is actually not
used in recent platforms.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 12 ++++++------
drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index a11f69f2e46e..508221de411c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -42,7 +42,7 @@
#define MAX_MMIO_BASES 3
struct engine_info {
- unsigned int hw_id;
+ u8 gen6_hw_id;
u8 class;
u8 instance;
/* mmio bases table *must* be sorted in reverse graphics_ver order */
@@ -54,7 +54,7 @@ struct engine_info {
static const struct engine_info intel_engines[] = {
[RCS0] = {
- .hw_id = RCS0_HW,
+ .gen6_hw_id = RCS0_HW,
.class = RENDER_CLASS,
.instance = 0,
.mmio_bases = {
@@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
},
},
[BCS0] = {
- .hw_id = BCS0_HW,
+ .gen6_hw_id = BCS0_HW,
.class = COPY_ENGINE_CLASS,
.instance = 0,
.mmio_bases = {
@@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
},
},
[VCS0] = {
- .hw_id = VCS0_HW,
+ .gen6_hw_id = VCS0_HW,
.class = VIDEO_DECODE_CLASS,
.instance = 0,
.mmio_bases = {
@@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
},
},
[VECS0] = {
- .hw_id = VECS0_HW,
+ .gen6_hw_id = VECS0_HW,
.class = VIDEO_ENHANCEMENT_CLASS,
.instance = 0,
.mmio_bases = {
@@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
engine->i915 = i915;
engine->gt = gt;
engine->uncore = gt->uncore;
- engine->hw_id = info->hw_id;
+ engine->gen6_hw_id = info->gen6_hw_id;
guc_class = engine_class_to_guc_class(info->class);
engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index a107eb58ffa2..266422d8d1b1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -264,11 +264,11 @@ struct intel_engine_cs {
enum intel_engine_id id;
enum intel_engine_id legacy_idx;
- unsigned int hw_id;
unsigned int guc_id;
intel_engine_mask_t mask;
+ u8 gen6_hw_id;
u8 class;
u8 instance;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 943fe485c662..8750ffce9d61 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define ARB_MODE_BWGTLB_DISABLE (1 << 9)
#define ARB_MODE_SWIZZLE_BDW (1 << 1)
#define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
-#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->hw_id)
+#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
#define GEN8_RING_FAULT_REG _MMIO(0x4094)
#define GEN12_RING_FAULT_REG _MMIO(0xcec4)
#define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
2021-07-20 23:20 ` [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id Lucas De Marchi
@ 2021-07-21 9:20 ` Tvrtko Ursulin
2021-07-21 22:51 ` Matt Roper
1 sibling, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21 9:20 UTC (permalink / raw)
To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel
On 21/07/2021 00:20, Lucas De Marchi wrote:
> We kept adding new engines and for that increasing hw_id unnecessarily:
> it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> try to pack it in the structs to give a hint this field is actually not
> used in recent platforms.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 12 ++++++------
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 2 +-
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index a11f69f2e46e..508221de411c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,7 @@
>
> #define MAX_MMIO_BASES 3
> struct engine_info {
> - unsigned int hw_id;
> + u8 gen6_hw_id;
> u8 class;
> u8 instance;
> /* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +54,7 @@ struct engine_info {
>
> static const struct engine_info intel_engines[] = {
> [RCS0] = {
> - .hw_id = RCS0_HW,
> + .gen6_hw_id = RCS0_HW,
> .class = RENDER_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [BCS0] = {
> - .hw_id = BCS0_HW,
> + .gen6_hw_id = BCS0_HW,
> .class = COPY_ENGINE_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS0] = {
> - .hw_id = VCS0_HW,
> + .gen6_hw_id = VCS0_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VECS0] = {
> - .hw_id = VECS0_HW,
> + .gen6_hw_id = VECS0_HW,
> .class = VIDEO_ENHANCEMENT_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> engine->i915 = i915;
> engine->gt = gt;
> engine->uncore = gt->uncore;
> - engine->hw_id = info->hw_id;
> + engine->gen6_hw_id = info->gen6_hw_id;
> guc_class = engine_class_to_guc_class(info->class);
> engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index a107eb58ffa2..266422d8d1b1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -264,11 +264,11 @@ struct intel_engine_cs {
> enum intel_engine_id id;
> enum intel_engine_id legacy_idx;
>
> - unsigned int hw_id;
> unsigned int guc_id;
>
> intel_engine_mask_t mask;
>
> + u8 gen6_hw_id;
> u8 class;
> u8 instance;
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 943fe485c662..8750ffce9d61 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define ARB_MODE_BWGTLB_DISABLE (1 << 9)
> #define ARB_MODE_SWIZZLE_BDW (1 << 1)
> #define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
> -#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->hw_id)
> +#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> #define GEN8_RING_FAULT_REG _MMIO(0x4094)
> #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
2021-07-20 23:20 ` [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id Lucas De Marchi
2021-07-21 9:20 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-07-21 22:51 ` Matt Roper
2021-07-22 5:11 ` [Intel-gfx] " Lucas De Marchi
1 sibling, 1 reply; 17+ messages in thread
From: Matt Roper @ 2021-07-21 22:51 UTC (permalink / raw)
To: Lucas De Marchi
Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison
On Tue, Jul 20, 2021 at 04:20:13PM -0700, Lucas De Marchi wrote:
> We kept adding new engines and for that increasing hw_id unnecessarily:
> it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> try to pack it in the structs to give a hint this field is actually not
> used in recent platforms.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
although if we apply patch #4 we could probably drop this intermediate
step.
Matt
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 12 ++++++------
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 2 +-
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index a11f69f2e46e..508221de411c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,7 @@
>
> #define MAX_MMIO_BASES 3
> struct engine_info {
> - unsigned int hw_id;
> + u8 gen6_hw_id;
> u8 class;
> u8 instance;
> /* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +54,7 @@ struct engine_info {
>
> static const struct engine_info intel_engines[] = {
> [RCS0] = {
> - .hw_id = RCS0_HW,
> + .gen6_hw_id = RCS0_HW,
> .class = RENDER_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [BCS0] = {
> - .hw_id = BCS0_HW,
> + .gen6_hw_id = BCS0_HW,
> .class = COPY_ENGINE_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS0] = {
> - .hw_id = VCS0_HW,
> + .gen6_hw_id = VCS0_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VECS0] = {
> - .hw_id = VECS0_HW,
> + .gen6_hw_id = VECS0_HW,
> .class = VIDEO_ENHANCEMENT_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> engine->i915 = i915;
> engine->gt = gt;
> engine->uncore = gt->uncore;
> - engine->hw_id = info->hw_id;
> + engine->gen6_hw_id = info->gen6_hw_id;
> guc_class = engine_class_to_guc_class(info->class);
> engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index a107eb58ffa2..266422d8d1b1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -264,11 +264,11 @@ struct intel_engine_cs {
> enum intel_engine_id id;
> enum intel_engine_id legacy_idx;
>
> - unsigned int hw_id;
> unsigned int guc_id;
>
> intel_engine_mask_t mask;
>
> + u8 gen6_hw_id;
> u8 class;
> u8 instance;
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 943fe485c662..8750ffce9d61 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define ARB_MODE_BWGTLB_DISABLE (1 << 9)
> #define ARB_MODE_SWIZZLE_BDW (1 << 1)
> #define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
> -#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->hw_id)
> +#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> #define GEN8_RING_FAULT_REG _MMIO(0x4094)
> #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
> --
> 2.31.1
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
2021-07-21 22:51 ` Matt Roper
@ 2021-07-22 5:11 ` Lucas De Marchi
0 siblings, 0 replies; 17+ messages in thread
From: Lucas De Marchi @ 2021-07-22 5:11 UTC (permalink / raw)
To: Matt Roper; +Cc: Intel Graphics, Lucas De Marchi, Tomas Winkler, DRI
On Wed, Jul 21, 2021 at 3:51 PM Matt Roper <matthew.d.roper@intel.com> wrote:
>
> On Tue, Jul 20, 2021 at 04:20:13PM -0700, Lucas De Marchi wrote:
> > We kept adding new engines and for that increasing hw_id unnecessarily:
> > it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> > try to pack it in the structs to give a hint this field is actually not
> > used in recent platforms.
> >
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
> although if we apply patch #4 we could probably drop this intermediate
I was not so confident people would agree with that patch. Adding the macros to
the header as suggested helps it being more palatable though.
thanks
Lucas De Marchi
> step.
>
>
> Matt
>
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 12 ++++++------
> > drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 +-
> > drivers/gpu/drm/i915/i915_reg.h | 2 +-
> > 3 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index a11f69f2e46e..508221de411c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -42,7 +42,7 @@
> >
> > #define MAX_MMIO_BASES 3
> > struct engine_info {
> > - unsigned int hw_id;
> > + u8 gen6_hw_id;
> > u8 class;
> > u8 instance;
> > /* mmio bases table *must* be sorted in reverse graphics_ver order */
> > @@ -54,7 +54,7 @@ struct engine_info {
> >
> > static const struct engine_info intel_engines[] = {
> > [RCS0] = {
> > - .hw_id = RCS0_HW,
> > + .gen6_hw_id = RCS0_HW,
> > .class = RENDER_CLASS,
> > .instance = 0,
> > .mmio_bases = {
> > @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
> > },
> > },
> > [BCS0] = {
> > - .hw_id = BCS0_HW,
> > + .gen6_hw_id = BCS0_HW,
> > .class = COPY_ENGINE_CLASS,
> > .instance = 0,
> > .mmio_bases = {
> > @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
> > },
> > },
> > [VCS0] = {
> > - .hw_id = VCS0_HW,
> > + .gen6_hw_id = VCS0_HW,
> > .class = VIDEO_DECODE_CLASS,
> > .instance = 0,
> > .mmio_bases = {
> > @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
> > },
> > },
> > [VECS0] = {
> > - .hw_id = VECS0_HW,
> > + .gen6_hw_id = VECS0_HW,
> > .class = VIDEO_ENHANCEMENT_CLASS,
> > .instance = 0,
> > .mmio_bases = {
> > @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> > engine->i915 = i915;
> > engine->gt = gt;
> > engine->uncore = gt->uncore;
> > - engine->hw_id = info->hw_id;
> > + engine->gen6_hw_id = info->gen6_hw_id;
> > guc_class = engine_class_to_guc_class(info->class);
> > engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
> > engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > index a107eb58ffa2..266422d8d1b1 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > @@ -264,11 +264,11 @@ struct intel_engine_cs {
> > enum intel_engine_id id;
> > enum intel_engine_id legacy_idx;
> >
> > - unsigned int hw_id;
> > unsigned int guc_id;
> >
> > intel_engine_mask_t mask;
> >
> > + u8 gen6_hw_id;
> > u8 class;
> > u8 instance;
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 943fe485c662..8750ffce9d61 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> > #define ARB_MODE_BWGTLB_DISABLE (1 << 9)
> > #define ARB_MODE_SWIZZLE_BDW (1 << 1)
> > #define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
> > -#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->hw_id)
> > +#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> > #define GEN8_RING_FAULT_REG _MMIO(0x4094)
> > #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> > #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
> > --
> > 2.31.1
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
2021-07-20 23:20 [PATCH 0/4] Nuke legacy hw_id Lucas De Marchi
` (2 preceding siblings ...)
2021-07-20 23:20 ` [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id Lucas De Marchi
@ 2021-07-20 23:20 ` Lucas De Marchi
2021-07-21 9:25 ` [Intel-gfx] " Tvrtko Ursulin
2021-07-21 23:02 ` Matt Roper
3 siblings, 2 replies; 17+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin
This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
recent platforms do not depend on this field, so it doesn't make much
sense to keep it generic like that. Instead, just do a mapping from
engine class to HW ID in the single place that is needed.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------
drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
drivers/gpu/drm/i915/i915_reg.h | 4 +++-
3 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 508221de411c..0a04e8d90e9e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -42,7 +42,6 @@
#define MAX_MMIO_BASES 3
struct engine_info {
- u8 gen6_hw_id;
u8 class;
u8 instance;
/* mmio bases table *must* be sorted in reverse graphics_ver order */
@@ -54,7 +53,6 @@ struct engine_info {
static const struct engine_info intel_engines[] = {
[RCS0] = {
- .gen6_hw_id = RCS0_HW,
.class = RENDER_CLASS,
.instance = 0,
.mmio_bases = {
@@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
},
},
[BCS0] = {
- .gen6_hw_id = BCS0_HW,
.class = COPY_ENGINE_CLASS,
.instance = 0,
.mmio_bases = {
@@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
},
},
[VCS0] = {
- .gen6_hw_id = VCS0_HW,
.class = VIDEO_DECODE_CLASS,
.instance = 0,
.mmio_bases = {
@@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
},
},
[VECS0] = {
- .gen6_hw_id = VECS0_HW,
.class = VIDEO_ENHANCEMENT_CLASS,
.instance = 0,
.mmio_bases = {
@@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
engine->i915 = i915;
engine->gt = gt;
engine->uncore = gt->uncore;
- engine->gen6_hw_id = info->gen6_hw_id;
guc_class = engine_class_to_guc_class(info->class);
engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 266422d8d1b1..64330bfb7641 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -28,13 +28,6 @@
#include "intel_wakeref.h"
#include "intel_workarounds_types.h"
-/* Legacy HW Engine ID */
-
-#define RCS0_HW 0
-#define VCS0_HW 1
-#define BCS0_HW 2
-#define VECS0_HW 3
-
/* Gen11+ HW Engine class + instance */
#define RENDER_CLASS 0
#define VIDEO_DECODE_CLASS 1
@@ -268,7 +261,6 @@ struct intel_engine_cs {
intel_engine_mask_t mask;
- u8 gen6_hw_id;
u8 class;
u8 instance;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8750ffce9d61..d91386f4828e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define ARB_MODE_BWGTLB_DISABLE (1 << 9)
#define ARB_MODE_SWIZZLE_BDW (1 << 1)
#define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
-#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
+
+#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
+#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
#define GEN8_RING_FAULT_REG _MMIO(0x4094)
#define GEN12_RING_FAULT_REG _MMIO(0xcec4)
#define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
2021-07-20 23:20 ` [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id Lucas De Marchi
@ 2021-07-21 9:25 ` Tvrtko Ursulin
2021-07-21 18:44 ` Lucas De Marchi
2021-07-21 23:02 ` Matt Roper
1 sibling, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21 9:25 UTC (permalink / raw)
To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel
On 21/07/2021 00:20, Lucas De Marchi wrote:
> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
> recent platforms do not depend on this field, so it doesn't make much
> sense to keep it generic like that. Instead, just do a mapping from
> engine class to HW ID in the single place that is needed.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
> drivers/gpu/drm/i915/i915_reg.h | 4 +++-
> 3 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 508221de411c..0a04e8d90e9e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,6 @@
>
> #define MAX_MMIO_BASES 3
> struct engine_info {
> - u8 gen6_hw_id;
> u8 class;
> u8 instance;
> /* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +53,6 @@ struct engine_info {
>
> static const struct engine_info intel_engines[] = {
> [RCS0] = {
> - .gen6_hw_id = RCS0_HW,
> .class = RENDER_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [BCS0] = {
> - .gen6_hw_id = BCS0_HW,
> .class = COPY_ENGINE_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS0] = {
> - .gen6_hw_id = VCS0_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VECS0] = {
> - .gen6_hw_id = VECS0_HW,
> .class = VIDEO_ENHANCEMENT_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> engine->i915 = i915;
> engine->gt = gt;
> engine->uncore = gt->uncore;
> - engine->gen6_hw_id = info->gen6_hw_id;
> guc_class = engine_class_to_guc_class(info->class);
> engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 266422d8d1b1..64330bfb7641 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -28,13 +28,6 @@
> #include "intel_wakeref.h"
> #include "intel_workarounds_types.h"
>
> -/* Legacy HW Engine ID */
> -
> -#define RCS0_HW 0
> -#define VCS0_HW 1
> -#define BCS0_HW 2
> -#define VECS0_HW 3
> -
> /* Gen11+ HW Engine class + instance */
> #define RENDER_CLASS 0
> #define VIDEO_DECODE_CLASS 1
> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>
> intel_engine_mask_t mask;
>
> - u8 gen6_hw_id;
> u8 class;
> u8 instance;
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8750ffce9d61..d91386f4828e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define ARB_MODE_BWGTLB_DISABLE (1 << 9)
> #define ARB_MODE_SWIZZLE_BDW (1 << 1)
> #define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
> -#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> +
> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
> +#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
Makes sense to me.
Maybe HW_ID and HW_CLASS in the macro name? Not sure.
Only open I have is why the "Gen11+ HW Engine class + instance" comment
and now we would tie that, allegedly Gen11 concept, with Gen6-7. Care to
do some digging?
Regards,
Tvrtko
> #define GEN8_RING_FAULT_REG _MMIO(0x4094)
> #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
2021-07-21 9:25 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-07-21 18:44 ` Lucas De Marchi
2021-07-22 8:27 ` Tvrtko Ursulin
0 siblings, 1 reply; 17+ messages in thread
From: Lucas De Marchi @ 2021-07-21 18:44 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: intel-gfx, Daniele Ceraolo Spurio, Tomas Winkler, dri-devel
On Wed, Jul 21, 2021 at 10:25:59AM +0100, Tvrtko Ursulin wrote:
>
>On 21/07/2021 00:20, Lucas De Marchi wrote:
>>This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
>>recent platforms do not depend on this field, so it doesn't make much
>>sense to keep it generic like that. Instead, just do a mapping from
>>engine class to HW ID in the single place that is needed.
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>---
>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------
>> drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>> drivers/gpu/drm/i915/i915_reg.h | 4 +++-
>> 3 files changed, 3 insertions(+), 15 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>index 508221de411c..0a04e8d90e9e 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>@@ -42,7 +42,6 @@
>> #define MAX_MMIO_BASES 3
>> struct engine_info {
>>- u8 gen6_hw_id;
>> u8 class;
>> u8 instance;
>> /* mmio bases table *must* be sorted in reverse graphics_ver order */
>>@@ -54,7 +53,6 @@ struct engine_info {
>> static const struct engine_info intel_engines[] = {
>> [RCS0] = {
>>- .gen6_hw_id = RCS0_HW,
>> .class = RENDER_CLASS,
>> .instance = 0,
>> .mmio_bases = {
>>@@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>> },
>> },
>> [BCS0] = {
>>- .gen6_hw_id = BCS0_HW,
>> .class = COPY_ENGINE_CLASS,
>> .instance = 0,
>> .mmio_bases = {
>>@@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>> },
>> },
>> [VCS0] = {
>>- .gen6_hw_id = VCS0_HW,
>> .class = VIDEO_DECODE_CLASS,
>> .instance = 0,
>> .mmio_bases = {
>>@@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>> },
>> },
>> [VECS0] = {
>>- .gen6_hw_id = VECS0_HW,
>> .class = VIDEO_ENHANCEMENT_CLASS,
>> .instance = 0,
>> .mmio_bases = {
>>@@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>> engine->i915 = i915;
>> engine->gt = gt;
>> engine->uncore = gt->uncore;
>>- engine->gen6_hw_id = info->gen6_hw_id;
>> guc_class = engine_class_to_guc_class(info->class);
>> engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
>>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>index 266422d8d1b1..64330bfb7641 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>@@ -28,13 +28,6 @@
>> #include "intel_wakeref.h"
>> #include "intel_workarounds_types.h"
>>-/* Legacy HW Engine ID */
>>-
>>-#define RCS0_HW 0
>>-#define VCS0_HW 1
>>-#define BCS0_HW 2
>>-#define VECS0_HW 3
>>-
>> /* Gen11+ HW Engine class + instance */
>> #define RENDER_CLASS 0
>> #define VIDEO_DECODE_CLASS 1
>>@@ -268,7 +261,6 @@ struct intel_engine_cs {
>> intel_engine_mask_t mask;
>>- u8 gen6_hw_id;
>> u8 class;
>> u8 instance;
>>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>index 8750ffce9d61..d91386f4828e 100644
>>--- a/drivers/gpu/drm/i915/i915_reg.h
>>+++ b/drivers/gpu/drm/i915/i915_reg.h
>>@@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>> #define ARB_MODE_BWGTLB_DISABLE (1 << 9)
>> #define ARB_MODE_SWIZZLE_BDW (1 << 1)
>> #define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
>>-#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
>>+
>>+#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
>>+#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
>
>Makes sense to me.
>
>Maybe HW_ID and HW_CLASS in the macro name? Not sure.
I can do that... I think I avoided it because it makes the macro
very big. Anyway, this should be called in just one place, so it doesn't
matter much... I can add it.
>
>Only open I have is why the "Gen11+ HW Engine class + instance"
>comment and now we would tie that, allegedly Gen11 concept, with
>Gen6-7. Care to do some digging?
Not sure. This comes from 3d7b3039741d ("drm/i915: Move engine IDs out of i915_reg.h")
that I reviewed :-o
Cc'ing Daniele. I don't see "class" as a Gen11+ thing. Is it just that
those numbers started to make sense for gen11? Since
a) we are using the class even for GRAPHICS_VER < 11
b) the legacy HW IDs shouldn't be used anywhere else anymore
we could
1) move the legacy defines back to i915_reg.h
2) use them in the macro above (IMO would slightly improve the
readability of that _PICK() call)
3) Remove the "Gen11+" comment in the _CLASS macros to avoid
misunderstandings
Thoughts?
thanks
Lucas De Marchi
>
>Regards,
>
>Tvrtko
>
>> #define GEN8_RING_FAULT_REG _MMIO(0x4094)
>> #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
>> #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
2021-07-21 18:44 ` Lucas De Marchi
@ 2021-07-22 8:27 ` Tvrtko Ursulin
0 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2021-07-22 8:27 UTC (permalink / raw)
To: Lucas De Marchi
Cc: intel-gfx, Daniele Ceraolo Spurio, Tomas Winkler, dri-devel
On 21/07/2021 19:44, Lucas De Marchi wrote:
> On Wed, Jul 21, 2021 at 10:25:59AM +0100, Tvrtko Ursulin wrote:
>>
>> On 21/07/2021 00:20, Lucas De Marchi wrote:
>>> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
>>> recent platforms do not depend on this field, so it doesn't make much
>>> sense to keep it generic like that. Instead, just do a mapping from
>>> engine class to HW ID in the single place that is needed.
>>>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------
>>> drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>>> drivers/gpu/drm/i915/i915_reg.h | 4 +++-
>>> 3 files changed, 3 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> index 508221de411c..0a04e8d90e9e 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> @@ -42,7 +42,6 @@
>>> #define MAX_MMIO_BASES 3
>>> struct engine_info {
>>> - u8 gen6_hw_id;
>>> u8 class;
>>> u8 instance;
>>> /* mmio bases table *must* be sorted in reverse graphics_ver
>>> order */
>>> @@ -54,7 +53,6 @@ struct engine_info {
>>> static const struct engine_info intel_engines[] = {
>>> [RCS0] = {
>>> - .gen6_hw_id = RCS0_HW,
>>> .class = RENDER_CLASS,
>>> .instance = 0,
>>> .mmio_bases = {
>>> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>>> },
>>> },
>>> [BCS0] = {
>>> - .gen6_hw_id = BCS0_HW,
>>> .class = COPY_ENGINE_CLASS,
>>> .instance = 0,
>>> .mmio_bases = {
>>> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>>> },
>>> },
>>> [VCS0] = {
>>> - .gen6_hw_id = VCS0_HW,
>>> .class = VIDEO_DECODE_CLASS,
>>> .instance = 0,
>>> .mmio_bases = {
>>> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>>> },
>>> },
>>> [VECS0] = {
>>> - .gen6_hw_id = VECS0_HW,
>>> .class = VIDEO_ENHANCEMENT_CLASS,
>>> .instance = 0,
>>> .mmio_bases = {
>>> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt
>>> *gt, enum intel_engine_id id)
>>> engine->i915 = i915;
>>> engine->gt = gt;
>>> engine->uncore = gt->uncore;
>>> - engine->gen6_hw_id = info->gen6_hw_id;
>>> guc_class = engine_class_to_guc_class(info->class);
>>> engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>>> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> index 266422d8d1b1..64330bfb7641 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> @@ -28,13 +28,6 @@
>>> #include "intel_wakeref.h"
>>> #include "intel_workarounds_types.h"
>>> -/* Legacy HW Engine ID */
>>> -
>>> -#define RCS0_HW 0
>>> -#define VCS0_HW 1
>>> -#define BCS0_HW 2
>>> -#define VECS0_HW 3
>>> -
>>> /* Gen11+ HW Engine class + instance */
>>> #define RENDER_CLASS 0
>>> #define VIDEO_DECODE_CLASS 1
>>> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>>> intel_engine_mask_t mask;
>>> - u8 gen6_hw_id;
>>> u8 class;
>>> u8 instance;
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index 8750ffce9d61..d91386f4828e 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -2572,7 +2572,9 @@ static inline bool
>>> i915_mmio_reg_valid(i915_reg_t reg)
>>> #define ARB_MODE_BWGTLB_DISABLE (1 << 9)
>>> #define ARB_MODE_SWIZZLE_BDW (1 << 1)
>>> #define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
>>> -#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 *
>>> (engine)->gen6_hw_id)
>>> +
>>> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
>>> +#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 *
>>> _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
>>
>> Makes sense to me.
>>
>> Maybe HW_ID and HW_CLASS in the macro name? Not sure.
>
> I can do that... I think I avoided it because it makes the macro
> very big. Anyway, this should be called in just one place, so it doesn't
> matter much... I can add it.
>
>>
>> Only open I have is why the "Gen11+ HW Engine class + instance"
>> comment and now we would tie that, allegedly Gen11 concept, with
>> Gen6-7. Care to do some digging?
>
> Not sure. This comes from 3d7b3039741d ("drm/i915: Move engine IDs out
> of i915_reg.h")
> that I reviewed :-o
>
> Cc'ing Daniele. I don't see "class" as a Gen11+ thing. Is it just that
> those numbers started to make sense for gen11? Since
>
> a) we are using the class even for GRAPHICS_VER < 11
> b) the legacy HW IDs shouldn't be used anywhere else anymore
>
>
> we could
>
> 1) move the legacy defines back to i915_reg.h
> 2) use them in the macro above (IMO would slightly improve the
> readability of that _PICK() call)
> 3) Remove the "Gen11+" comment in the _CLASS macros to avoid
> misunderstandings
>
>
> Thoughts?
What Matt suggested sounds fine to me (using _PICK with
_RING_FAULT_REG_RCS etc). It retires the concept of hw id from the code
base completely and hopefully the need for it does not re-surface.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
2021-07-20 23:20 ` [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id Lucas De Marchi
2021-07-21 9:25 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-07-21 23:02 ` Matt Roper
1 sibling, 0 replies; 17+ messages in thread
From: Matt Roper @ 2021-07-21 23:02 UTC (permalink / raw)
To: Lucas De Marchi
Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison
On Tue, Jul 20, 2021 at 04:20:14PM -0700, Lucas De Marchi wrote:
> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
> recent platforms do not depend on this field, so it doesn't make much
> sense to keep it generic like that. Instead, just do a mapping from
> engine class to HW ID in the single place that is needed.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 ------
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
> drivers/gpu/drm/i915/i915_reg.h | 4 +++-
> 3 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 508221de411c..0a04e8d90e9e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,6 @@
>
> #define MAX_MMIO_BASES 3
> struct engine_info {
> - u8 gen6_hw_id;
> u8 class;
> u8 instance;
> /* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +53,6 @@ struct engine_info {
>
> static const struct engine_info intel_engines[] = {
> [RCS0] = {
> - .gen6_hw_id = RCS0_HW,
> .class = RENDER_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [BCS0] = {
> - .gen6_hw_id = BCS0_HW,
> .class = COPY_ENGINE_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VCS0] = {
> - .gen6_hw_id = VCS0_HW,
> .class = VIDEO_DECODE_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
> },
> },
> [VECS0] = {
> - .gen6_hw_id = VECS0_HW,
> .class = VIDEO_ENHANCEMENT_CLASS,
> .instance = 0,
> .mmio_bases = {
> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> engine->i915 = i915;
> engine->gt = gt;
> engine->uncore = gt->uncore;
> - engine->gen6_hw_id = info->gen6_hw_id;
> guc_class = engine_class_to_guc_class(info->class);
> engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 266422d8d1b1..64330bfb7641 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -28,13 +28,6 @@
> #include "intel_wakeref.h"
> #include "intel_workarounds_types.h"
>
> -/* Legacy HW Engine ID */
> -
> -#define RCS0_HW 0
> -#define VCS0_HW 1
> -#define BCS0_HW 2
> -#define VECS0_HW 3
> -
> /* Gen11+ HW Engine class + instance */
> #define RENDER_CLASS 0
> #define VIDEO_DECODE_CLASS 1
> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>
> intel_engine_mask_t mask;
>
> - u8 gen6_hw_id;
> u8 class;
> u8 instance;
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8750ffce9d61..d91386f4828e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define ARB_MODE_BWGTLB_DISABLE (1 << 9)
> #define ARB_MODE_SWIZZLE_BDW (1 << 1)
> #define RENDER_HWS_PGA_GEN7 _MMIO(0x04080)
> -#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> +
> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
> +#define RING_FAULT_REG(engine) _MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
If you want to make this more clear to someone reading it down the road,
you could always do something explicit like:
#define _RING_FAULT_REG_RCS 0x4094
#define _RING_FAULT_REG_VCS 0x4194
#define _RING_FAULT_REG_BCS 0x4294
#define _RING_FAULT_REG_VECS 0x4394
#define RING_FAULT_REG(engine) _MMIO(_PICK((engine)->class, \
_RING_FAULT_REG_RCS, \
_RING_FAULT_REG_VCS, \
_RING_FAULT_REG_VECS, \
_RING_FAULT_REG_BCS))
But in general,
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> #define GEN8_RING_FAULT_REG _MMIO(0x4094)
> #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
> --
> 2.31.1
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
^ permalink raw reply [flat|nested] 17+ messages in thread