[Intel-gfx] [PATCH v2 06/50] drm/i915/xehp: Extra media engines - Part 1 (engine definitions)

John Harrison john.c.harrison at intel.com
Tue Jul 20 23:40:52 UTC 2021


On 7/20/2021 16:03, Lucas De Marchi wrote:
> On Tue, Jul 13, 2021 at 08:14:56PM -0700, Matt Roper wrote:
>> From: John Harrison <John.C.Harrison at Intel.com>
>>
>> Xe_HP can have a lot of extra media engines. This patch adds the basic
>> definitions for them.
>>
>> v2:
>> - Re-order intel_gt_info and intel_device_info slightly to avoid
>>   unnecessary padding now that we've increased the size of
>>   intel_engine_mask_t.  (Tvrtko)
>>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>> Signed-off-by: Tomas Winkler <tomas.winkler at intel.com>
>> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/gen8_engine_cs.c     |  7 ++-
>> drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 50 ++++++++++++++++++++
>> drivers/gpu/drm/i915/gt/intel_engine_types.h | 14 ++++--
>> drivers/gpu/drm/i915/gt/intel_gt_types.h     |  5 +-
>> drivers/gpu/drm/i915/i915_reg.h              |  6 +++
>> drivers/gpu/drm/i915/intel_device_info.h     |  3 +-
>> 6 files changed, 74 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c 
>> b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>> index 87b06572fd2e..35edc55720f4 100644
>> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>> @@ -279,7 +279,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, 
>> u32 mode)
>>     if (mode & EMIT_INVALIDATE)
>>         aux_inv = rq->engine->mask & ~BIT(BCS0);
>>     if (aux_inv)
>> -        cmd += 2 * hweight8(aux_inv) + 2;
>> +        cmd += 2 * hweight32(aux_inv) + 2;
>>
>>     cs = intel_ring_begin(rq, cmd);
>>     if (IS_ERR(cs))
>> @@ -313,9 +313,8 @@ int gen12_emit_flush_xcs(struct i915_request *rq, 
>> u32 mode)
>>         struct intel_engine_cs *engine;
>>         unsigned int tmp;
>>
>> -        *cs++ = MI_LOAD_REGISTER_IMM(hweight8(aux_inv));
>> -        for_each_engine_masked(engine, rq->engine->gt,
>> -                       aux_inv, tmp) {
>> +        *cs++ = MI_LOAD_REGISTER_IMM(hweight32(aux_inv));
>> +        for_each_engine_masked(engine, rq->engine->gt, aux_inv, tmp) {
>>             *cs++ = i915_mmio_reg_offset(aux_inv_reg(engine));
>>             *cs++ = AUX_INV;
>>         }
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> index 3f8013612a08..6c2cb1400c8c 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> @@ -104,6 +104,38 @@ static const struct engine_info intel_engines[] = {
>>             { .graphics_ver = 11, .base = GEN11_BSD4_RING_BASE }
>>         },
>>     },
>> +    [VCS4] = {
>> +        .hw_id = 0, /* not used in GEN12+, see MI_SEMAPHORE_SIGNAL */
>
> I may be misreading this, but hw_id is only used by
> RING_FAULT_REG() which is not actually used since
> gen8... they are using GEN8_RING_FAULT_REG().
>
> I'm having a hard time to understand what this comment "see
> MI_SEMAPHORE_SIGNAL" actually means.
I vaguely recall something about being told the hw_id field was used in 
semaphore messages from one engine to another. I.e. if engine X is 
blocked on a semaphore that is signalled by engine Y then the MI_ 
instruction executed on Y to do the signal needs to specify X as the 
target. Whereas, on newer hardware this requirement was no longer 
applicable because MI_SEMAPHORE_SIGNAL uses memory mailboxes instead of 
directed engine messages. Maybe that information was wrong or maybe that 
code has since been removed or reworked?


>
>
> I'd just remove all these `.hw_id = 0, ...` together with the comment
> since it will be zero-initiliazed.
Yeah, the reason for explicitly setting it to zero was to avoid 
confusion over whether it had just been forgotten or not. I.e. to say 
'we know semaphores used to use this field but honest guv, we didn't 
forget to add it, it's just that newer hardware doesn't need it'.

John.


>
> Lucas De Marchi
>
>
>> +        .class = VIDEO_DECODE_CLASS,
>> +        .instance = 4,
>> +        .mmio_bases = {
>> +            { .graphics_ver = 11, .base = XEHP_BSD5_RING_BASE }
>> +        },
>> +    },
>> +    [VCS5] = {
>> +        .hw_id = 0, /* not used in GEN12+, see MI_SEMAPHORE_SIGNAL */
>> +        .class = VIDEO_DECODE_CLASS,
>> +        .instance = 5,
>> +        .mmio_bases = {
>> +            { .graphics_ver = 12, .base = XEHP_BSD6_RING_BASE }
>> +        },
>> +    },
>> +    [VCS6] = {
>> +        .hw_id = 0, /* not used in GEN12+, see MI_SEMAPHORE_SIGNAL */
>> +        .class = VIDEO_DECODE_CLASS,
>> +        .instance = 6,
>> +        .mmio_bases = {
>> +            { .graphics_ver = 12, .base = XEHP_BSD7_RING_BASE }
>> +        },
>> +    },
>> +    [VCS7] = {
>> +        .hw_id = 0, /* not used in GEN12+, see MI_SEMAPHORE_SIGNAL */
>> +        .class = VIDEO_DECODE_CLASS,
>> +        .instance = 7,
>> +        .mmio_bases = {
>> +            { .graphics_ver = 12, .base = XEHP_BSD8_RING_BASE }
>> +        },
>> +    },
>>     [VECS0] = {
>>         .hw_id = VECS0_HW,
>>         .class = VIDEO_ENHANCEMENT_CLASS,
>> @@ -121,6 +153,22 @@ static const struct engine_info intel_engines[] = {
>>             { .graphics_ver = 11, .base = GEN11_VEBOX2_RING_BASE }
>>         },
>>     },
>> +    [VECS2] = {
>> +        .hw_id = 0, /* not used in GEN12+, see MI_SEMAPHORE_SIGNAL */
>> +        .class = VIDEO_ENHANCEMENT_CLASS,
>> +        .instance = 2,
>> +        .mmio_bases = {
>> +            { .graphics_ver = 12, .base = XEHP_VEBOX3_RING_BASE }
>> +        },
>> +    },
>> +    [VECS3] = {
>> +        .hw_id = 0, /* not used in GEN12+, see MI_SEMAPHORE_SIGNAL */
>> +        .class = VIDEO_ENHANCEMENT_CLASS,
>> +        .instance = 3,
>> +        .mmio_bases = {
>> +            { .graphics_ver = 12, .base = XEHP_VEBOX4_RING_BASE }
>> +        },
>> +    },
>> };
>>
>> /**
>> @@ -269,6 +317,8 @@ static int intel_engine_setup(struct intel_gt 
>> *gt, enum intel_engine_id id)
>>
>>     BUILD_BUG_ON(MAX_ENGINE_CLASS >= BIT(GEN11_ENGINE_CLASS_WIDTH));
>>     BUILD_BUG_ON(MAX_ENGINE_INSTANCE >= 
>> BIT(GEN11_ENGINE_INSTANCE_WIDTH));
>> +    BUILD_BUG_ON(I915_MAX_VCS > (MAX_ENGINE_INSTANCE + 1));
>> +    BUILD_BUG_ON(I915_MAX_VECS > (MAX_ENGINE_INSTANCE + 1));
>>
>>     if (GEM_DEBUG_WARN_ON(id >= ARRAY_SIZE(gt->engine)))
>>         return -EINVAL;
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> index 1cb9c3b70b29..e0b1cbdbadce 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> @@ -46,7 +46,7 @@
>> #define COPY_ENGINE_CLASS    3
>> #define OTHER_CLASS        4
>> #define MAX_ENGINE_CLASS    4
>> -#define MAX_ENGINE_INSTANCE    3
>> +#define MAX_ENGINE_INSTANCE    7
>>
>> #define I915_MAX_SLICES    3
>> #define I915_MAX_SUBSLICES 8
>> @@ -64,7 +64,7 @@ struct intel_gt;
>> struct intel_ring;
>> struct intel_uncore;
>>
>> -typedef u8 intel_engine_mask_t;
>> +typedef u32 intel_engine_mask_t;
>> #define ALL_ENGINES ((intel_engine_mask_t)~0ul)
>>
>> struct intel_hw_status_page {
>> @@ -101,8 +101,8 @@ struct i915_ctx_workarounds {
>>     struct i915_vma *vma;
>> };
>>
>> -#define I915_MAX_VCS    4
>> -#define I915_MAX_VECS    2
>> +#define I915_MAX_VCS    8
>> +#define I915_MAX_VECS    4
>>
>> /*
>>  * Engine IDs definitions.
>> @@ -115,9 +115,15 @@ enum intel_engine_id {
>>     VCS1,
>>     VCS2,
>>     VCS3,
>> +    VCS4,
>> +    VCS5,
>> +    VCS6,
>> +    VCS7,
>> #define _VCS(n) (VCS0 + (n))
>>     VECS0,
>>     VECS1,
>> +    VECS2,
>> +    VECS3,
>> #define _VECS(n) (VECS0 + (n))
>>     I915_NUM_ENGINES
>> #define INVALID_ENGINE ((enum intel_engine_id)-1)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h 
>> b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> index d93d578a4105..97a5075288d2 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> @@ -174,13 +174,14 @@ struct intel_gt {
>>
>>     struct intel_gt_info {
>>         intel_engine_mask_t engine_mask;
>> +
>> +        u32 l3bank_mask;
>> +
>>         u8 num_engines;
>>
>>         /* Media engine access to SFC per instance */
>>         u8 vdbox_sfc_access;
>>
>> -        u32 l3bank_mask;
>> -
>>         /* Slice/subslice/EU info */
>>         struct sseu_dev_info sseu;
>>     } info;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 2274e9c01d61..de0c27d67e15 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2516,9 +2516,15 @@ static inline bool 
>> i915_mmio_reg_valid(i915_reg_t reg)
>> #define GEN11_BSD2_RING_BASE    0x1c4000
>> #define GEN11_BSD3_RING_BASE    0x1d0000
>> #define GEN11_BSD4_RING_BASE    0x1d4000
>> +#define XEHP_BSD5_RING_BASE    0x1e0000
>> +#define XEHP_BSD6_RING_BASE    0x1e4000
>> +#define XEHP_BSD7_RING_BASE    0x1f0000
>> +#define XEHP_BSD8_RING_BASE    0x1f4000
>> #define VEBOX_RING_BASE        0x1a000
>> #define GEN11_VEBOX_RING_BASE        0x1c8000
>> #define GEN11_VEBOX2_RING_BASE        0x1d8000
>> +#define XEHP_VEBOX3_RING_BASE        0x1e8000
>> +#define XEHP_VEBOX4_RING_BASE        0x1f8000
>> #define BLT_RING_BASE        0x22000
>> #define RING_TAIL(base)        _MMIO((base) + 0x30)
>> #define RING_HEAD(base)        _MMIO((base) + 0x34)
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.h 
>> b/drivers/gpu/drm/i915/intel_device_info.h
>> index ba7483acc3f7..75d225d0db47 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.h
>> +++ b/drivers/gpu/drm/i915/intel_device_info.h
>> @@ -166,7 +166,6 @@ struct intel_device_info {
>>     u8 media_ver;
>>     u8 media_rel;
>>
>> -    u8 gt; /* GT number, 0 if undefined */
>>     intel_engine_mask_t platform_engine_mask; /* Engines supported by 
>> the HW */
>>
>>     enum intel_platform platform;
>> @@ -182,6 +181,8 @@ struct intel_device_info {
>>
>>     u32 display_mmio_offset;
>>
>> +    u8 gt; /* GT number, 0 if undefined */
>> +
>>     u8 pipe_mask;
>>     u8 cpu_transcoder_mask;
>>
>> -- 
>> 2.25.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



More information about the Intel-gfx mailing list