On Mon, May 02, 2022 at 11:39:48AM -0700, Lucas De Marchi wrote:
On Mon, May 02, 2022 at 09:50:23AM -0700, Matt Roper wrote:
On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
From: Ayaz A Siddiqui ayaz.siddiqui@intel.com
Bspec: 45101, 72161 Signed-off-by: Ayaz A Siddiqui ayaz.siddiqui@intel.com Signed-off-by: Fei Yang fei.yang@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 + drivers/gpu/drm/i915/gt/intel_mocs.c | 24 ++++++++++++++++++++- drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 ++++++++--- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 3 ++- drivers/gpu/drm/i915/intel_device_info.h | 1 + 6 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index b06611c1d4ad..7853ea194ea6 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -221,6 +221,7 @@ struct intel_gt {
struct { u8 uc_index;
u8 wb_index; /* Only for platforms listed in Bspec: 72161 */} mocs;
struct intel_pxp pxp;
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c index c4c37585ae8c..265812589f87 100644 --- a/drivers/gpu/drm/i915/gt/intel_mocs.c +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c @@ -23,6 +23,7 @@ struct drm_i915_mocs_table { unsigned int n_entries; const struct drm_i915_mocs_entry *table; u8 uc_index;
- u8 wb_index; /* Only for platforms listed in Bspec: 72161 */ u8 unused_entries_index;
};
@@ -47,6 +48,7 @@ struct drm_i915_mocs_table {
/* Helper defines */ #define GEN9_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but configured. */ +#define PVC_NUM_MOCS_ENTRIES 3
Should this be 4? The value here should reflect the number of entries that can defined in hardware rather than the size of the table we're asked to program. Since there are two registers (each with a high and a low entry), that would imply we should set 4 here to ensure that the fourth entry is initialized according to unused_entries_index rather than left at whatever the hardware defaults might be.
not sure I understand what you mean here. The n_entries specifies, as you said, the number of entries we can have. Bspec 45101 shows entries for indexes 0, 1 and 2. As does the pvc_mocs_table below.
Also, from bspec 44509: "For PVC, only 3 MOCS states are supported. The allowed index values are in range [0, 2]..."
So, I don't think we want to program any fourth entry.
We don't have a choice; the fourth entry lives in the same register as the third entry, so no matter what we're writing _something_ to those bits. The question is whether we should write all 0's or whether we should treat it like other platforms and ensure it's initialized to the unused entry values. Entry #4 isn't supposed to be used, but if buggy userspace tries to use it, we probably still want well-defined behavior, just like it an invalid entry gets used on any other platform.
Matt
Lucas De Marchi