* [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-04 18:32 Anusha Srivatsa
0 siblings, 0 replies; 12+ messages in thread
From: Anusha Srivatsa @ 2022-05-04 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Anusha Srivatsa, stable, Lucas De Marchi
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
- Add MMIO range for RKL as well.(Anusha)
v5: Use DISPLAY_VER instead of per platform check (Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 42 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..e7437ed2597e 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -374,6 +374,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (DISPLAY_VER(i915) >= 13) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (DISPLAY_VER(i915) >= 12) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +480,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-06 17:35 Anusha Srivatsa
0 siblings, 0 replies; 12+ messages in thread
From: Anusha Srivatsa @ 2022-05-06 17:35 UTC (permalink / raw)
To: intel-gfx; +Cc: Anusha Srivatsa, stable, Lucas De Marchi
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
- Add MMIO range for RKL as well.(Anusha)
v5: Use DISPLAY_VER instead of per platform check (Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 42 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 2f01aca4d981..f545cc7367e3 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -378,6 +378,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (DISPLAY_VER(i915) >= 13) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (DISPLAY_VER(i915) >= 12) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -447,6 +484,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-04 0:13 Anusha Srivatsa
2022-05-04 0:31 ` Lucas De Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Anusha Srivatsa @ 2022-05-04 0:13 UTC (permalink / raw)
To: intel-gfx; +Cc: Anusha Srivatsa, stable, Lucas De Marchi
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
- Add MMIO range for RKL as well.(Anusha)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 43 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..e37ba75e68da 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -374,6 +374,44 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915) ||
+ IS_ROCKETLAKE(i915)) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +481,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-05-04 0:13 Anusha Srivatsa
@ 2022-05-04 0:31 ` Lucas De Marchi
2022-05-04 0:36 ` Srivatsa, Anusha
0 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2022-05-04 0:31 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx, stable
On Tue, May 03, 2022 at 05:13:46PM -0700, Anusha Srivatsa wrote:
>Bspec has added some steps that check forDMC MMIO range before
>programming them
>
>v2: Fix for CI
>v3: move register defines to .h (Anusha)
>- Check MMIO restrictions per pipe
>- Add MMIO restricton for v1 dmc header as well (Lucas)
>v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
>- clean up sanity check logic.(Lucas)
>- Add MMIO range for RKL as well.(Anusha)
>
>BSpec: 49193
>
>Cc: <stable@vger.kernel.org>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dmc.c | 43 +++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
> 2 files changed, 60 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>index 257cf662f9f4..e37ba75e68da 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>@@ -374,6 +374,44 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
> }
> }
>
>+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
>+ u32 mmio_count, int header_ver, u8 dmc_id)
>+{
>+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
>+ u32 start_range, end_range;
>+ int i;
>+
>+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
dmc_id is unsigned and DMC_FW_MAIN is 0. dmc_id < DMC_FW_MAIN can't ever
possibly happen so you can remove it.
>+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
>+ return false;
>+ }
>+
>+ if (header_ver == 1) {
>+ start_range = DMC_MMIO_START_RANGE;
>+ end_range = DMC_MMIO_END_RANGE;
>+ } else if (dmc_id == DMC_FW_MAIN) {
>+ start_range = TGL_MAIN_MMIO_START;
>+ end_range = TGL_MAIN_MMIO_END;
>+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
} else if (DISPLAY_VER(i915) >= 13) {
?
>+ start_range = ADLP_PIPE_MMIO_START;
>+ end_range = ADLP_PIPE_MMIO_END;
>+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915) ||
>+ IS_ROCKETLAKE(i915)) {
} else if (DISPLAY_VER(i915) >= 12) {
?
maintaining the if/else ladder fine grained by platform is somewhat painful.
>+ start_range = TGL_PIPE_MMIO_START(dmc_id);
>+ end_range = TGL_PIPE_MMIO_END(dmc_id);
>+ } else {
>+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
>+ return false;
>+ }
>+
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
>+ return false;
>+ }
>+
>+ return true;
>+}
>+
> static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> const struct intel_dmc_header_base *dmc_header,
> size_t rem_size, u8 dmc_id)
>@@ -443,6 +481,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> return 0;
> }
>
>+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
>+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
>+ return 0;
>+ }
>+
> for (i = 0; i < mmio_count; i++) {
> dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> dmc_info->mmiodata[i] = mmiodata[i];
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>index d65e698832eb..67e14eb96a7a 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>@@ -16,7 +16,23 @@
> #define DMC_LAST_WRITE _MMIO(0x8F034)
> #define DMC_LAST_WRITE_VALUE 0xc003b400
> #define DMC_MMIO_START_RANGE 0x80000
>-#define DMC_MMIO_END_RANGE 0x8FFFF
>+#define DMC_MMIO_END_RANGE 0x8FFFF
>+#define DMC_V1_MMIO_START_RANGE 0x80000
>+#define TGL_MAIN_MMIO_START 0x8F000
>+#define TGL_MAIN_MMIO_END 0x8FFFF
>+#define _TGL_PIPEA_MMIO_START 0x92000
>+#define _TGL_PIPEA_MMIO_END 0x93FFF
>+#define _TGL_PIPEB_MMIO_START 0x96000
>+#define _TGL_PIPEB_MMIO_END 0x97FFF
>+#define ADLP_PIPE_MMIO_START 0x5F000
>+#define ADLP_PIPE_MMIO_END 0x5FFFF
don't we have per-pipe range for ADLP_? Or is there only pipe A?
with the above fixes, feel free to add my Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
in the next version.
Lucas De Marchi
>+
>+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
>+ _TGL_PIPEB_MMIO_START)
>+
>+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
>+ _TGL_PIPEB_MMIO_END)
>+
> #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
>--
>2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-05-04 0:31 ` Lucas De Marchi
@ 2022-05-04 0:36 ` Srivatsa, Anusha
0 siblings, 0 replies; 12+ messages in thread
From: Srivatsa, Anusha @ 2022-05-04 0:36 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx, stable
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Tuesday, May 3, 2022 5:31 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Tue, May 03, 2022 at 05:13:46PM -0700, Anusha Srivatsa wrote:
> >Bspec has added some steps that check forDMC MMIO range before
> >programming them
> >
> >v2: Fix for CI
> >v3: move register defines to .h (Anusha)
> >- Check MMIO restrictions per pipe
> >- Add MMIO restricton for v1 dmc header as well (Lucas)
> >v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
> >- clean up sanity check logic.(Lucas)
> >- Add MMIO range for RKL as well.(Anusha)
> >
> >BSpec: 49193
> >
> >Cc: <stable@vger.kernel.org>
> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >---
> > drivers/gpu/drm/i915/display/intel_dmc.c | 43 +++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
> > 2 files changed, 60 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >index 257cf662f9f4..e37ba75e68da 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >@@ -374,6 +374,44 @@ static void dmc_set_fw_offset(struct intel_dmc
> *dmc,
> > }
> > }
> >
> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> u32 *mmioaddr,
> >+ u32 mmio_count, int header_ver, u8
> dmc_id) {
> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> dmc);
> >+ u32 start_range, end_range;
> >+ int i;
> >+
> >+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
>
> dmc_id is unsigned and DMC_FW_MAIN is 0. dmc_id < DMC_FW_MAIN can't
> ever possibly happen so you can remove it.
>
> >+ drm_warn(&i915->drm, "Unsupported firmware id %u\n",
> dmc_id);
> >+ return false;
> >+ }
> >+
> >+ if (header_ver == 1) {
> >+ start_range = DMC_MMIO_START_RANGE;
> >+ end_range = DMC_MMIO_END_RANGE;
> >+ } else if (dmc_id == DMC_FW_MAIN) {
> >+ start_range = TGL_MAIN_MMIO_START;
> >+ end_range = TGL_MAIN_MMIO_END;
> >+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>
> } else if (DISPLAY_VER(i915) >= 13) {
>
> ?
>
> >+ start_range = ADLP_PIPE_MMIO_START;
> >+ end_range = ADLP_PIPE_MMIO_END;
> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> IS_ALDERLAKE_S(i915) ||
> >+ IS_ROCKETLAKE(i915)) {
>
> } else if (DISPLAY_VER(i915) >= 12) {
>
> ?
>
> maintaining the if/else ladder fine grained by platform is somewhat painful.
Agreed.
> >+ start_range = TGL_PIPE_MMIO_START(dmc_id);
> >+ end_range = TGL_PIPE_MMIO_END(dmc_id);
> >+ } else {
> >+ drm_warn(&i915->drm, "Unknown mmio range for sanity
> check");
> >+ return false;
> >+ }
> >+
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
> >+ return false;
> >+ }
> >+
> >+ return true;
> >+}
> >+
> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> > const struct intel_dmc_header_base
> *dmc_header,
> > size_t rem_size, u8 dmc_id)
> >@@ -443,6 +481,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc
> *dmc,
> > return 0;
> > }
> >
> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
> dmc_header->header_ver, dmc_id)) {
> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
> Addresses\n");
> >+ return 0;
> >+ }
> >+
> > for (i = 0; i < mmio_count; i++) {
> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >index d65e698832eb..67e14eb96a7a 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >@@ -16,7 +16,23 @@
> > #define DMC_LAST_WRITE _MMIO(0x8F034)
> > #define DMC_LAST_WRITE_VALUE 0xc003b400
> > #define DMC_MMIO_START_RANGE 0x80000
> >-#define DMC_MMIO_END_RANGE 0x8FFFF
> >+#define DMC_MMIO_END_RANGE 0x8FFFF
> >+#define DMC_V1_MMIO_START_RANGE 0x80000
> >+#define TGL_MAIN_MMIO_START 0x8F000
> >+#define TGL_MAIN_MMIO_END 0x8FFFF
> >+#define _TGL_PIPEA_MMIO_START 0x92000
> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
> >+#define _TGL_PIPEB_MMIO_START 0x96000
> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
>
> don't we have per-pipe range for ADLP_? Or is there only pipe A?
>
We don't have per-pipe range. We have one big chunk of range for all pipe DMC MMIOs.
> with the above fixes, feel free to add my Reviewed-by: Lucas De Marchi
> <lucas.demarchi@intel.com> in the next version.
Thanks!
> Lucas De Marchi
>
> >+
> >+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1),
> _TGL_PIPEA_MMIO_START,\
> >+ _TGL_PIPEB_MMIO_START)
> >+
> >+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1),
> _TGL_PIPEA_MMIO_END,\
> >+ _TGL_PIPEB_MMIO_END)
> >+
> > #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> > #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> > #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
> >--
> >2.25.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-03 23:36 Anusha Srivatsa
0 siblings, 0 replies; 12+ messages in thread
From: Anusha Srivatsa @ 2022-05-03 23:36 UTC (permalink / raw)
To: intel-gfx; +Cc: Anusha Srivatsa, stable, Lucas De Marchi
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
- Add MMIO range for RKL as well.(Anusha)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 43 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..12d5cb850e39 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -374,6 +374,44 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915) ||
+ IS_ROCKETLAKE(dev_priv)) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +481,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-03 22:04 Anusha Srivatsa
0 siblings, 0 replies; 12+ messages in thread
From: Anusha Srivatsa @ 2022-05-03 22:04 UTC (permalink / raw)
To: intel-gfx; +Cc: Anusha Srivatsa, stable, Lucas De Marchi
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 42 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..7abe4418eeb0 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -374,6 +374,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +480,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-27 0:35 Anusha Srivatsa
2022-04-27 5:41 ` Lucas De Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Anusha Srivatsa @ 2022-04-27 0:35 UTC (permalink / raw)
To: intel-gfx; +Cc: Anusha Srivatsa, stable, Lucas De Marchi
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
2 files changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..ac7896835bfa 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
#define BXT_DMC_MAX_FW_SIZE 0x3000
MODULE_FIRMWARE(BXT_DMC_PATH);
-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
-#define DMC_V1_MAX_MMIO_COUNT 8
-#define DMC_V3_MAX_MMIO_COUNT 20
-#define DMC_V1_MMIO_START_RANGE 0x80000
-
struct intel_css_header {
/* 0x09 for DMC */
u32 module_type;
@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ int i;
+
+ if (header_ver == 1) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < DMC_MMIO_START_RANGE || mmioaddr[i] > DMC_MMIO_END_RANGE)
+ return false;
+ }
+ }
+
+ /* Main DMC MMIO check */
+ if (dmc_id == DMC_FW_MAIN) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
+ return false;
+ }
+ }
+
+ /* Pipe DMC MMIO check */
+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START && mmioaddr[i] > ADLP_PIPE_MMIO_END)
+ return false;
+ }
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
+ return false;
+ }
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..235d1b721334 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -11,12 +11,43 @@
#define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
#define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
#define DMC_HTP_ADDR_SKL 0x00500034
+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
#define DMC_SSP_BASE _MMIO(0x8F074)
#define DMC_HTP_SKL _MMIO(0x8F004)
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
#define DMC_MMIO_END_RANGE 0x8FFFF
+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
+#define DMC_V1_MAX_MMIO_COUNT 8
+#define DMC_V3_MAX_MMIO_COUNT 20
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define _TGL_MAIN_MMIO_START 0x8F000
+#define _TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define _TGL_PIPEC_MMIO_START 0x9A000
+#define _TGL_PIPEC_MMIO_END 0x9BFFF
+#define _TGL_PIPED_MMIO_START 0x9E000
+#define _TGL_PIPED_MMIO_END 0x9FFFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe, _TGL_MAIN_MMIO_START,\
+ _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START,\
+ _TGL_PIPEC_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe, _TGL_MAIN_MMIO_END,\
+ _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END,\
+ _TGL_PIPEC_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-27 0:35 Anusha Srivatsa
@ 2022-04-27 5:41 ` Lucas De Marchi
2022-04-29 20:39 ` Srivatsa, Anusha
0 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2022-04-27 5:41 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx, stable
On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
>Bspec has added some steps that check forDMC MMIO range before
>programming them
>
>v2: Fix for CI
>v3: move register defines to .h (Anusha)
>- Check MMIO restrictions per pipe
>- Add MMIO restricton for v1 dmc header as well (Lucas)
>
>BSpec: 49193
>
>Cc: <stable@vger.kernel.org>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> 2 files changed, 72 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>index 257cf662f9f4..ac7896835bfa 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> #define BXT_DMC_MAX_FW_SIZE 0x3000
> MODULE_FIRMWARE(BXT_DMC_PATH);
>
>-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>-#define DMC_V1_MAX_MMIO_COUNT 8
>-#define DMC_V3_MAX_MMIO_COUNT 20
>-#define DMC_V1_MMIO_START_RANGE 0x80000
>-
> struct intel_css_header {
> /* 0x09 for DMC */
> u32 module_type;
>@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
> }
> }
>
>+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
>+ u32 mmio_count, int header_ver, u8 dmc_id)
>+{
>+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
>+ int i;
>+
>+ if (header_ver == 1) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < DMC_MMIO_START_RANGE || mmioaddr[i] > DMC_MMIO_END_RANGE)
>+ return false;
>+ }
return missing here
>+ }
>+
>+ /* Main DMC MMIO check */
>+ if (dmc_id == DMC_FW_MAIN) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>+ return false;
>+ }
>+ }
>+
>+ /* Pipe DMC MMIO check */
>+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START && mmioaddr[i] > ADLP_PIPE_MMIO_END)
>+ return false;
>+ }
for DG2, main should use TGL_DMC_MMIO_START? and then fail here because
of another missing return above?
>+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>+ return false;
this is handling DMC_FW_MAIN twice.
Maybe something like this:
u32 start, end;
if (header_ver == 1) {
start = DMC_MMIO_START_RANGE;
end = DMC_MMIO_END_RANGE;
} else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
start = TGL_DMC_MMIO_START(dmc_id);
end = TGL_DMC_MMIO_END(dmc_id);
} else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
start = ADLP_PIPE_MMIO_START;
end = ADLP_PIPE_MMIO_END;
} else {
drm_warn(&i915->drm, "Unknown mmio range for sanity check");
return false;
}
for (i = 0; i < mmio_count; i++)
if (mmioaddr[i] < start || mmioaddr[i] > end)
return false;
return true;
... untested, and may need tweaks depending on the answer to the
question above on what range to use for ADL-P/DG2 on main DMC.
>+ }
>+ }
>+
>+ return true;
>+}
>+
> static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> const struct intel_dmc_header_base *dmc_header,
> size_t rem_size, u8 dmc_id)
>@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> return 0;
> }
>
>+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
>+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
>+ return 0;
you don't like DMC and decided to fail it for all platforms?
>+
> for (i = 0; i < mmio_count; i++) {
> dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> dmc_info->mmiodata[i] = mmiodata[i];
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>index d65e698832eb..235d1b721334 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>@@ -11,12 +11,43 @@
> #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> #define DMC_HTP_ADDR_SKL 0x00500034
>+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> #define DMC_SSP_BASE _MMIO(0x8F074)
> #define DMC_HTP_SKL _MMIO(0x8F004)
> #define DMC_LAST_WRITE _MMIO(0x8F034)
> #define DMC_LAST_WRITE_VALUE 0xc003b400
> #define DMC_MMIO_START_RANGE 0x80000
> #define DMC_MMIO_END_RANGE 0x8FFFF
>+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>+#define DMC_V1_MAX_MMIO_COUNT 8
>+#define DMC_V3_MAX_MMIO_COUNT 20
why are you moving these to _regs? these describe the DMC header/blob
>+#define DMC_V1_MMIO_START_RANGE 0x80000
>+#define _TGL_MAIN_MMIO_START 0x8F000
>+#define _TGL_MAIN_MMIO_END 0x8FFFF
>+#define _TGL_PIPEA_MMIO_START 0x92000
>+#define _TGL_PIPEA_MMIO_END 0x93FFF
>+#define _TGL_PIPEB_MMIO_START 0x96000
>+#define _TGL_PIPEB_MMIO_END 0x97FFF
>+#define _TGL_PIPEC_MMIO_START 0x9A000
>+#define _TGL_PIPEC_MMIO_END 0x9BFFF
>+#define _TGL_PIPED_MMIO_START 0x9E000
>+#define _TGL_PIPED_MMIO_END 0x9FFFF
>+#define ADLP_PIPE_MMIO_START 0x5F000
>+#define ADLP_PIPE_MMIO_END 0x5FFFF
>+
>+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe, _TGL_MAIN_MMIO_START,\
_PICK? Did you miss my previous review?
Lucas De Marchi
>+ _TGL_PIPEA_MMIO_START,\
>+ _TGL_PIPEB_MMIO_START,\
>+ _TGL_PIPEC_MMIO_START,\
>+ _TGL_PIPEB_MMIO_START)
>+
>+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe, _TGL_MAIN_MMIO_END,\
>+ _TGL_PIPEA_MMIO_END,\
>+ _TGL_PIPEB_MMIO_END,\
>+ _TGL_PIPEC_MMIO_END,\
>+ _TGL_PIPEB_MMIO_END)
>+
> #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
>--
>2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-27 5:41 ` Lucas De Marchi
@ 2022-04-29 20:39 ` Srivatsa, Anusha
2022-04-29 20:49 ` Lucas De Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Srivatsa, Anusha @ 2022-04-29 20:39 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx, stable
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Tuesday, April 26, 2022 10:42 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
> >Bspec has added some steps that check forDMC MMIO range before
> >programming them
> >
> >v2: Fix for CI
> >v3: move register defines to .h (Anusha)
> >- Check MMIO restrictions per pipe
> >- Add MMIO restricton for v1 dmc header as well (Lucas)
> >
> >BSpec: 49193
> >
> >Cc: <stable@vger.kernel.org>
> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >---
> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> > 2 files changed, 72 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >index 257cf662f9f4..ac7896835bfa 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> > #define BXT_DMC_MAX_FW_SIZE 0x3000
> > MODULE_FIRMWARE(BXT_DMC_PATH);
> >
> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >-#define DMC_V1_MAX_MMIO_COUNT 8
> >-#define DMC_V3_MAX_MMIO_COUNT 20
> >-#define DMC_V1_MMIO_START_RANGE 0x80000
> >-
> > struct intel_css_header {
> > /* 0x09 for DMC */
> > u32 module_type;
> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
> *dmc,
> > }
> > }
> >
> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> u32 *mmioaddr,
> >+ u32 mmio_count, int header_ver, u8
> dmc_id) {
> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> dmc);
> >+ int i;
> >+
> >+ if (header_ver == 1) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
> mmioaddr[i] > DMC_MMIO_END_RANGE)
> >+ return false;
> >+ }
>
> return missing here
>
> >+ }
> >+
> >+ /* Main DMC MMIO check */
> >+ if (dmc_id == DMC_FW_MAIN) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >+ return false;
> >+ }
> >+ }
> >+
> >+ /* Pipe DMC MMIO check */
> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
> mmioaddr[i] > ADLP_PIPE_MMIO_END)
> >+ return false;
> >+ }
>
> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
> because of another missing return above?
>
> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> IS_ALDERLAKE_S(i915)) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >+ return false;
>
> this is handling DMC_FW_MAIN twice.
>
>
> Maybe something like this:
>
> u32 start, end;
>
> if (header_ver == 1) {
> start = DMC_MMIO_START_RANGE;
> end = DMC_MMIO_END_RANGE;
> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
> start = TGL_DMC_MMIO_START(dmc_id);
> end = TGL_DMC_MMIO_END(dmc_id);
> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> start = ADLP_PIPE_MMIO_START;
> end = ADLP_PIPE_MMIO_END;
> } else {
> drm_warn(&i915->drm, "Unknown mmio range for sanity
> check");
> return false;
> }
>
> for (i = 0; i < mmio_count; i++)
> if (mmioaddr[i] < start || mmioaddr[i] > end)
> return false;
>
> return true;
>
>
> ... untested, and may need tweaks depending on the answer to the question
> above on what range to use for ADL-P/DG2 on main DMC.
The above approach is definitely neater.
The main DMC offset is the same for all platforms.
> >+ }
> >+ }
> >+
> >+ return true;
> >+}
> >+
> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> > const struct intel_dmc_header_base
> *dmc_header,
> > size_t rem_size, u8 dmc_id)
> >@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc
> *dmc,
> > return 0;
> > }
> >
> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
> dmc_header->header_ver, dmc_id))
> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
> Addresses\n");
> >+ return 0;
>
> you don't like DMC and decided to fail it for all platforms?
<facepalm>
> >+
> > for (i = 0; i < mmio_count; i++) {
> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >index d65e698832eb..235d1b721334 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >@@ -11,12 +11,43 @@
> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> > #define DMC_HTP_ADDR_SKL 0x00500034
> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> > #define DMC_SSP_BASE _MMIO(0x8F074)
> > #define DMC_HTP_SKL _MMIO(0x8F004)
> > #define DMC_LAST_WRITE _MMIO(0x8F034)
> > #define DMC_LAST_WRITE_VALUE 0xc003b400
> > #define DMC_MMIO_START_RANGE 0x80000
> > #define DMC_MMIO_END_RANGE 0x8FFFF
> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >+#define DMC_V1_MAX_MMIO_COUNT 8
> >+#define DMC_V3_MAX_MMIO_COUNT 20
>
>
> why are you moving these to _regs? these describe the DMC header/blob
Yeah my mistake. While making the change they seemed like the right thing to do.
>
> >+#define DMC_V1_MMIO_START_RANGE 0x80000
> >+#define _TGL_MAIN_MMIO_START 0x8F000
> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
> >+#define _TGL_PIPEA_MMIO_START 0x92000
> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
> >+#define _TGL_PIPEB_MMIO_START 0x96000
> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
> >+#define _TGL_PIPEC_MMIO_START 0x9A000
> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
> >+#define _TGL_PIPED_MMIO_START 0x9E000
> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
> >+
> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
> _TGL_MAIN_MMIO_START,\
>
> _PICK? Did you miss my previous review?
No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So using PICK_EVEN is not the right choice here. We also don't need to do _MMIO really.....
Unless I am missing something, this seems like the right approach.
Anusha
> Lucas De Marchi
>
> >+ _TGL_PIPEA_MMIO_START,\
> >+ _TGL_PIPEB_MMIO_START,\
> >+ _TGL_PIPEC_MMIO_START,\
> >+ _TGL_PIPEB_MMIO_START)
> >+
> >+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe,
> _TGL_MAIN_MMIO_END,\
> >+ _TGL_PIPEA_MMIO_END,\
> >+ _TGL_PIPEB_MMIO_END,\
> >+ _TGL_PIPEC_MMIO_END,\
> >+ _TGL_PIPEB_MMIO_END)
> >+
> > #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> > #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> > #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
> >--
> >2.25.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-29 20:39 ` Srivatsa, Anusha
@ 2022-04-29 20:49 ` Lucas De Marchi
2022-04-29 22:57 ` Srivatsa, Anusha
0 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2022-04-29 20:49 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx, stable
On Fri, Apr 29, 2022 at 01:39:03PM -0700, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi@intel.com>
>> Sent: Tuesday, April 26, 2022 10:42 PM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
>> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>>
>> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
>> >Bspec has added some steps that check forDMC MMIO range before
>> >programming them
>> >
>> >v2: Fix for CI
>> >v3: move register defines to .h (Anusha)
>> >- Check MMIO restrictions per pipe
>> >- Add MMIO restricton for v1 dmc header as well (Lucas)
>> >
>> >BSpec: 49193
>> >
>> >Cc: <stable@vger.kernel.org>
>> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> >---
>> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
>> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
>> > 2 files changed, 72 insertions(+), 7 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >index 257cf662f9f4..ac7896835bfa 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
>> > #define BXT_DMC_MAX_FW_SIZE 0x3000
>> > MODULE_FIRMWARE(BXT_DMC_PATH);
>> >
>> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>> >-#define DMC_V1_MAX_MMIO_COUNT 8
>> >-#define DMC_V3_MAX_MMIO_COUNT 20
>> >-#define DMC_V1_MMIO_START_RANGE 0x80000
>> >-
>> > struct intel_css_header {
>> > /* 0x09 for DMC */
>> > u32 module_type;
>> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
>> *dmc,
>> > }
>> > }
>> >
>> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
>> u32 *mmioaddr,
>> >+ u32 mmio_count, int header_ver, u8
>> dmc_id) {
>> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
>> dmc);
>> >+ int i;
>> >+
>> >+ if (header_ver == 1) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
>> mmioaddr[i] > DMC_MMIO_END_RANGE)
>> >+ return false;
>> >+ }
>>
>> return missing here
>>
>> >+ }
>> >+
>> >+ /* Main DMC MMIO check */
>> >+ if (dmc_id == DMC_FW_MAIN) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
>> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>> >+ return false;
>> >+ }
>> >+ }
>> >+
>> >+ /* Pipe DMC MMIO check */
>> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
>> mmioaddr[i] > ADLP_PIPE_MMIO_END)
>> >+ return false;
>> >+ }
>>
>> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
>> because of another missing return above?
>>
>> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
>> IS_ALDERLAKE_S(i915)) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
>> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>> >+ return false;
>>
>> this is handling DMC_FW_MAIN twice.
>>
>>
>> Maybe something like this:
>>
>> u32 start, end;
>>
>> if (header_ver == 1) {
>> start = DMC_MMIO_START_RANGE;
>> end = DMC_MMIO_END_RANGE;
>> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
>> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
>> start = TGL_DMC_MMIO_START(dmc_id);
>> end = TGL_DMC_MMIO_END(dmc_id);
>> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>> start = ADLP_PIPE_MMIO_START;
>> end = ADLP_PIPE_MMIO_END;
>> } else {
>> drm_warn(&i915->drm, "Unknown mmio range for sanity
>> check");
>> return false;
>> }
>>
>> for (i = 0; i < mmio_count; i++)
>> if (mmioaddr[i] < start || mmioaddr[i] > end)
>> return false;
>>
>> return true;
>>
>>
>> ... untested, and may need tweaks depending on the answer to the question
>> above on what range to use for ADL-P/DG2 on main DMC.
>The above approach is definitely neater.
>The main DMC offset is the same for all platforms.
>
>> >+ }
>> >+ }
>> >+
>> >+ return true;
>> >+}
>> >+
>> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
>> > const struct intel_dmc_header_base
>> *dmc_header,
>> > size_t rem_size, u8 dmc_id)
>> >@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc
>> *dmc,
>> > return 0;
>> > }
>> >
>> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
>> dmc_header->header_ver, dmc_id))
>> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
>> Addresses\n");
>> >+ return 0;
>>
>> you don't like DMC and decided to fail it for all platforms?
>
><facepalm>
>
>> >+
>> > for (i = 0; i < mmio_count; i++) {
>> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
>> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
>> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >index d65e698832eb..235d1b721334 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >@@ -11,12 +11,43 @@
>> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
>> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
>> > #define DMC_HTP_ADDR_SKL 0x00500034
>> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>> > #define DMC_SSP_BASE _MMIO(0x8F074)
>> > #define DMC_HTP_SKL _MMIO(0x8F004)
>> > #define DMC_LAST_WRITE _MMIO(0x8F034)
>> > #define DMC_LAST_WRITE_VALUE 0xc003b400
>> > #define DMC_MMIO_START_RANGE 0x80000
>> > #define DMC_MMIO_END_RANGE 0x8FFFF
>> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>> >+#define DMC_V1_MAX_MMIO_COUNT 8
>> >+#define DMC_V3_MAX_MMIO_COUNT 20
>>
>>
>> why are you moving these to _regs? these describe the DMC header/blob
>
>Yeah my mistake. While making the change they seemed like the right thing to do.
>
>>
>> >+#define DMC_V1_MMIO_START_RANGE 0x80000
>> >+#define _TGL_MAIN_MMIO_START 0x8F000
>> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
>> >+#define _TGL_PIPEA_MMIO_START 0x92000
>> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
>> >+#define _TGL_PIPEB_MMIO_START 0x96000
>> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
>> >+#define _TGL_PIPEC_MMIO_START 0x9A000
>> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
>> >+#define _TGL_PIPED_MMIO_START 0x9E000
>> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
>> >+#define ADLP_PIPE_MMIO_START 0x5F000
>> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
>> >+
>> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
>> _TGL_MAIN_MMIO_START,\
>>
>> _PICK? Did you miss my previous review?
>
>No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So using PICK_EVEN is not the right choice here. We also don't need to do _MMIO really.....
>Unless I am missing something, this seems like the right approach.
Because the name you chose for your variable:
TGL_DMC_MMIO_START(pipe) _PICK(pipe,
I was expecting this to be used only with the pipe DMC address, which
are evenly spaced. It seems the argument you're expecting here is a
dmc_id. But.... you said:
>The main DMC offset is the same for all platforms.
So, maybe just handle that separately and keep using pipe here? Then you
can switch to _PICK_EVEN()
Lucas De Marchi
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-29 20:49 ` Lucas De Marchi
@ 2022-04-29 22:57 ` Srivatsa, Anusha
0 siblings, 0 replies; 12+ messages in thread
From: Srivatsa, Anusha @ 2022-04-29 22:57 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx, stable
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Friday, April 29, 2022 1:50 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Fri, Apr 29, 2022 at 01:39:03PM -0700, Anusha Srivatsa wrote:
> >
> >
> >> -----Original Message-----
> >> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> >> Sent: Tuesday, April 26, 2022 10:42 PM
> >> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> >> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> >> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
> >>
> >> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
> >> >Bspec has added some steps that check forDMC MMIO range before
> >> >programming them
> >> >
> >> >v2: Fix for CI
> >> >v3: move register defines to .h (Anusha)
> >> >- Check MMIO restrictions per pipe
> >> >- Add MMIO restricton for v1 dmc header as well (Lucas)
> >> >
> >> >BSpec: 49193
> >> >
> >> >Cc: <stable@vger.kernel.org>
> >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >> >---
> >> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> >> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> >> > 2 files changed, 72 insertions(+), 7 deletions(-)
> >> >
> >> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >index 257cf662f9f4..ac7896835bfa 100644
> >> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> >> > #define BXT_DMC_MAX_FW_SIZE 0x3000
> >> > MODULE_FIRMWARE(BXT_DMC_PATH);
> >> >
> >> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >> >-#define DMC_V1_MAX_MMIO_COUNT 8
> >> >-#define DMC_V3_MAX_MMIO_COUNT 20
> >> >-#define DMC_V1_MMIO_START_RANGE 0x80000
> >> >-
> >> > struct intel_css_header {
> >> > /* 0x09 for DMC */
> >> > u32 module_type;
> >> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
> >> *dmc,
> >> > }
> >> > }
> >> >
> >> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> >> u32 *mmioaddr,
> >> >+ u32 mmio_count, int header_ver, u8
> >> dmc_id) {
> >> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> >> dmc);
> >> >+ int i;
> >> >+
> >> >+ if (header_ver == 1) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
> >> mmioaddr[i] > DMC_MMIO_END_RANGE)
> >> >+ return false;
> >> >+ }
> >>
> >> return missing here
> >>
> >> >+ }
> >> >+
> >> >+ /* Main DMC MMIO check */
> >> >+ if (dmc_id == DMC_FW_MAIN) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> >> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >> >+ return false;
> >> >+ }
> >> >+ }
> >> >+
> >> >+ /* Pipe DMC MMIO check */
> >> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
> >> mmioaddr[i] > ADLP_PIPE_MMIO_END)
> >> >+ return false;
> >> >+ }
> >>
> >> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
> >> because of another missing return above?
> >>
> >> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> >> IS_ALDERLAKE_S(i915)) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> >> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >> >+ return false;
> >>
> >> this is handling DMC_FW_MAIN twice.
> >>
> >>
> >> Maybe something like this:
> >>
> >> u32 start, end;
> >>
> >> if (header_ver == 1) {
> >> start = DMC_MMIO_START_RANGE;
> >> end = DMC_MMIO_END_RANGE;
> >> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
> >> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
> >> start = TGL_DMC_MMIO_START(dmc_id);
> >> end = TGL_DMC_MMIO_END(dmc_id);
> >> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >> start = ADLP_PIPE_MMIO_START;
> >> end = ADLP_PIPE_MMIO_END;
> >> } else {
> >> drm_warn(&i915->drm, "Unknown mmio range for sanity
> check");
> >> return false;
> >> }
> >>
> >> for (i = 0; i < mmio_count; i++)
> >> if (mmioaddr[i] < start || mmioaddr[i] > end)
> >> return false;
> >>
> >> return true;
> >>
> >>
> >> ... untested, and may need tweaks depending on the answer to the
> >> question above on what range to use for ADL-P/DG2 on main DMC.
> >The above approach is definitely neater.
> >The main DMC offset is the same for all platforms.
> >
> >> >+ }
> >> >+ }
> >> >+
> >> >+ return true;
> >> >+}
> >> >+
> >> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> >> > const struct intel_dmc_header_base
> >> *dmc_header,
> >> > size_t rem_size, u8 dmc_id) @@ -443,6 +473,10
> @@ static
> >> >u32 parse_dmc_fw_header(struct intel_dmc
> >> *dmc,
> >> > return 0;
> >> > }
> >> >
> >> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
> >> dmc_header->header_ver, dmc_id))
> >> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
> >> Addresses\n");
> >> >+ return 0;
> >>
> >> you don't like DMC and decided to fail it for all platforms?
> >
> ><facepalm>
> >
> >> >+
> >> > for (i = 0; i < mmio_count; i++) {
> >> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> >> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
> >> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >index d65e698832eb..235d1b721334 100644
> >> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >@@ -11,12 +11,43 @@
> >> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> >> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> >> > #define DMC_HTP_ADDR_SKL 0x00500034
> >> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >> > #define DMC_SSP_BASE _MMIO(0x8F074)
> >> > #define DMC_HTP_SKL _MMIO(0x8F004)
> >> > #define DMC_LAST_WRITE _MMIO(0x8F034)
> >> > #define DMC_LAST_WRITE_VALUE 0xc003b400
> >> > #define DMC_MMIO_START_RANGE 0x80000
> >> > #define DMC_MMIO_END_RANGE 0x8FFFF
> >> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >> >+#define DMC_V1_MAX_MMIO_COUNT 8
> >> >+#define DMC_V3_MAX_MMIO_COUNT 20
> >>
> >>
> >> why are you moving these to _regs? these describe the DMC
> >> header/blob
> >
> >Yeah my mistake. While making the change they seemed like the right thing
> to do.
> >
> >>
> >> >+#define DMC_V1_MMIO_START_RANGE 0x80000
> >> >+#define _TGL_MAIN_MMIO_START 0x8F000
> >> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
> >> >+#define _TGL_PIPEA_MMIO_START 0x92000
> >> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
> >> >+#define _TGL_PIPEB_MMIO_START 0x96000
> >> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
> >> >+#define _TGL_PIPEC_MMIO_START 0x9A000
> >> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
> >> >+#define _TGL_PIPED_MMIO_START 0x9E000
> >> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
> >> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
> >> >+
> >> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
> >> _TGL_MAIN_MMIO_START,\
> >>
> >> _PICK? Did you miss my previous review?
> >
> >No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So
> using PICK_EVEN is not the right choice here. We also don't need to do
> _MMIO really.....
> >Unless I am missing something, this seems like the right approach.
>
> Because the name you chose for your variable:
>
> TGL_DMC_MMIO_START(pipe) _PICK(pipe,
>
> I was expecting this to be used only with the pipe DMC address, which are
> evenly spaced. It seems the argument you're expecting here is a dmc_id.
Ah, yes. I see the confusion now. It is expecting the dmc_id ,yes. In the usage of the macro in the beginning of the patch though, I am using dmc_id. Thought that would make it clearer, still pipe was the wrong choice of var name.
> But.... you said:
>
> >The main DMC offset is the same for all platforms.
>
> So, maybe just handle that separately and keep using pipe here? Then you
> can switch to _PICK_EVEN()
While the Pipe DMC s are evenly spaced and _PICK_EVEN is the right choice. The dmc_id for PipeA, PipeB ....will be 1,2....and not 0,1,2 so the helper will return the wrong values.
But you are suggesting to use PIPE_A, PIPE_B etc which will be 0 indexed. But here in the code we are parsing the dmc binary to see if it has Pipe DMC and if so what the MMIO offsets are they in and if it is a valid blob or not. The data we can use at this point is the dmc_id...... Unless we do a conversion from dmc-id to the pipe:
If (DMC_FW_PIPEA)
TGL_DMC_MMIO_START(PIPE_A) _PICK(
That will lead to individual such conditions per pipe.
Anusha
> Lucas De Marchi
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-05-06 17:39 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-04 18:32 [PATCH] drm/i915/dmc: Add MMIO range restrictions Anusha Srivatsa
-- strict thread matches above, loose matches on Subject: below --
2022-05-06 17:35 Anusha Srivatsa
2022-05-04 0:13 Anusha Srivatsa
2022-05-04 0:31 ` Lucas De Marchi
2022-05-04 0:36 ` Srivatsa, Anusha
2022-05-03 23:36 Anusha Srivatsa
2022-05-03 22:04 Anusha Srivatsa
2022-04-27 0:35 Anusha Srivatsa
2022-04-27 5:41 ` Lucas De Marchi
2022-04-29 20:39 ` Srivatsa, Anusha
2022-04-29 20:49 ` Lucas De Marchi
2022-04-29 22:57 ` Srivatsa, Anusha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).