Re: [Intel-gfx] [PATCH v7 2/9] drm/i915: Add the has_aux_ccs device property
Hi Andrzej, On Fri, Jul 21, 2023 at 11:41:22AM +0200, Andrzej Hajda wrote: > On 20.07.2023 23:07, Andi Shyti wrote: > > We always assumed that a device might either have AUX or FLAT > > CCS, but this is an approximation that is not always true as it > > requires some further per device checks. > > > > Add the "has_aux_ccs" flag in the intel_device_info structure in > > order to have a per device flag indicating of the AUX CCS. > > As Matt mentioned in v6, aux_ccs is present also in older platforms. > This is about presence/necessity (?) of aux_ccs table invalidation. > Maybe has_aux_ccs_inv, dunno? yes, true! It's aux_ccs_inv. > Moreover you define flag per device, but this is rather per engine, > theoretically could be also: > MTL: > .aux_ccs_inv_mask = BIT(RCS0) | BIT(BCS0) | ... > Others: > .aux_ccs_inv_mask = BIT(RCS0) | ... there is already some engine discrimination that is mandatory later in the series. Doing it here it's a bit replicated. > looks overkill, > maybe helper function would be simpler, up to you. Yes, a helper function that checks on the platform and returns true or false... it looks hardcoded to me, while the info structure is hardcoded by definition and looks easier to maintain by just toggling on/off a single flag in that structure. That's why I decided to place it there. But, because there are already two people suggesting it, then I will try it in v8. Thanks, Andi
Re: [Intel-gfx] [PATCH v7 2/9] drm/i915: Add the has_aux_ccs device property
On 20.07.2023 23:07, Andi Shyti wrote:
We always assumed that a device might either have AUX or FLAT
CCS, but this is an approximation that is not always true as it
requires some further per device checks.
Add the "has_aux_ccs" flag in the intel_device_info structure in
order to have a per device flag indicating of the AUX CCS.
As Matt mentioned in v6, aux_ccs is present also in older platforms.
This is about presence/necessity (?) of aux_ccs table invalidation.
Maybe has_aux_ccs_inv, dunno?
Moreover you define flag per device, but this is rather per engine,
theoretically could be also:
MTL:
.aux_ccs_inv_mask = BIT(RCS0) | BIT(BCS0) | ...
Others:
.aux_ccs_inv_mask = BIT(RCS0) | ...
looks overkill,
maybe helper function would be simpler, up to you.
Regards
Andrzej
Signed-off-by: Andi Shyti
Cc: Matt Roper
Cc: Jonathan Cavitt
Cc: # v5.8+
---
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 4 ++--
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_pci.c | 5 -
drivers/gpu/drm/i915/intel_device_info.h | 1 +
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 563efee055602..0d4d5e0407a2d 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -267,7 +267,7 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
else if (engine->class == COMPUTE_CLASS)
flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
- if (!HAS_FLAT_CCS(rq->engine->i915))
+ if (HAS_AUX_CCS(rq->engine->i915))
count = 8 + 4;
else
count = 8;
@@ -307,7 +307,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode)
if (mode & EMIT_INVALIDATE) {
cmd += 2;
- if (!HAS_FLAT_CCS(rq->engine->i915) &&
+ if (HAS_AUX_CCS(rq->engine->i915) &&
(rq->engine->class == VIDEO_DECODE_CLASS ||
rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) {
aux_inv = rq->engine->mask &
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 682ef2b5c7d59..e9cc048b5727a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -848,6 +848,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
* stored in lmem to support the 3D and media compression formats.
*/
#define HAS_FLAT_CCS(i915) (INTEL_INFO(i915)->has_flat_ccs)
+#define HAS_AUX_CCS(i915)(INTEL_INFO(i915)->has_aux_ccs)
#define HAS_GT_UC(i915) (INTEL_INFO(i915)->has_gt_uc)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index fcacdc21643cf..c9ff1d11a9fce 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -643,7 +643,8 @@ static const struct intel_device_info jsl_info = {
TGL_CACHELEVEL, \
.has_global_mocs = 1, \
.has_pxp = 1, \
- .max_pat_index = 3
+ .max_pat_index = 3, \
+ .has_aux_ccs = 1
static const struct intel_device_info tgl_info = {
GEN12_FEATURES,
@@ -775,6 +776,7 @@ static const struct intel_device_info dg2_info = {
static const struct intel_device_info ats_m_info = {
DG2_FEATURES,
+ .has_aux_ccs = 1,
.require_force_probe = 1,
.tuning_thread_rr_after_dep = 1,
};
@@ -827,6 +829,7 @@ static const struct intel_device_info mtl_info = {
.__runtime.media.ip.ver = 13,
PLATFORM(INTEL_METEORLAKE),
.extra_gt_list = xelpmp_extra_gt,
+ .has_aux_ccs = 1,
.has_flat_ccs = 0,
.has_gmd_id = 1,
.has_guc_deprivilege = 1,
diff --git a/drivers/gpu/drm/i915/intel_device_info.h
b/drivers/gpu/drm/i915/intel_device_info.h
index dbfe6443457b5..93485507506cc 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -151,6 +151,7 @@ enum intel_ppgtt_type {
func(has_reset_engine); \
func(has_3d_pipeline); \
func(has_4tile); \
+ func(has_aux_ccs); \
func(has_flat_ccs); \
func(has_global_mocs); \
func(has_gmd_id); \
[Intel-gfx] [PATCH v7 2/9] drm/i915: Add the has_aux_ccs device property
We always assumed that a device might either have AUX or FLAT
CCS, but this is an approximation that is not always true as it
requires some further per device checks.
Add the "has_aux_ccs" flag in the intel_device_info structure in
order to have a per device flag indicating of the AUX CCS.
Signed-off-by: Andi Shyti
Cc: Matt Roper
Cc: Jonathan Cavitt
Cc: # v5.8+
---
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 4 ++--
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_pci.c | 5 -
drivers/gpu/drm/i915/intel_device_info.h | 1 +
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 563efee055602..0d4d5e0407a2d 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -267,7 +267,7 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
else if (engine->class == COMPUTE_CLASS)
flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
- if (!HAS_FLAT_CCS(rq->engine->i915))
+ if (HAS_AUX_CCS(rq->engine->i915))
count = 8 + 4;
else
count = 8;
@@ -307,7 +307,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode)
if (mode & EMIT_INVALIDATE) {
cmd += 2;
- if (!HAS_FLAT_CCS(rq->engine->i915) &&
+ if (HAS_AUX_CCS(rq->engine->i915) &&
(rq->engine->class == VIDEO_DECODE_CLASS ||
rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) {
aux_inv = rq->engine->mask &
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 682ef2b5c7d59..e9cc048b5727a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -848,6 +848,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
* stored in lmem to support the 3D and media compression formats.
*/
#define HAS_FLAT_CCS(i915) (INTEL_INFO(i915)->has_flat_ccs)
+#define HAS_AUX_CCS(i915)(INTEL_INFO(i915)->has_aux_ccs)
#define HAS_GT_UC(i915)(INTEL_INFO(i915)->has_gt_uc)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index fcacdc21643cf..c9ff1d11a9fce 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -643,7 +643,8 @@ static const struct intel_device_info jsl_info = {
TGL_CACHELEVEL, \
.has_global_mocs = 1, \
.has_pxp = 1, \
- .max_pat_index = 3
+ .max_pat_index = 3, \
+ .has_aux_ccs = 1
static const struct intel_device_info tgl_info = {
GEN12_FEATURES,
@@ -775,6 +776,7 @@ static const struct intel_device_info dg2_info = {
static const struct intel_device_info ats_m_info = {
DG2_FEATURES,
+ .has_aux_ccs = 1,
.require_force_probe = 1,
.tuning_thread_rr_after_dep = 1,
};
@@ -827,6 +829,7 @@ static const struct intel_device_info mtl_info = {
.__runtime.media.ip.ver = 13,
PLATFORM(INTEL_METEORLAKE),
.extra_gt_list = xelpmp_extra_gt,
+ .has_aux_ccs = 1,
.has_flat_ccs = 0,
.has_gmd_id = 1,
.has_guc_deprivilege = 1,
diff --git a/drivers/gpu/drm/i915/intel_device_info.h
b/drivers/gpu/drm/i915/intel_device_info.h
index dbfe6443457b5..93485507506cc 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -151,6 +151,7 @@ enum intel_ppgtt_type {
func(has_reset_engine); \
func(has_3d_pipeline); \
func(has_4tile); \
+ func(has_aux_ccs); \
func(has_flat_ccs); \
func(has_global_mocs); \
func(has_gmd_id); \
--
2.40.1
