Re: [Intel-gfx] [PXP CLEAN PATCH v06 07/27] drm/i915/pxp: Add PXP-related registers into allowlist

Jani Nikula
2020-11-23T15:06:21Z

On Fri, 13 Nov 2020, Sean Z Huang <sean.z.huang@intel.com> wrote:
> From: "Huang, Sean Z" <sean.z.huang@intel.com>
>
> Add several PXP-related reg into allowlist to allow
> ring3 driver to read the those register values.
>
> Signed-off-by: Huang, Sean Z <sean.z.huang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 8 ++++
> drivers/gpu/drm/i915/intel_uncore.c | 57 +++++++++++++++++++++--------
> 2 files changed, 50 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index faf6b06145fa..5c51c9df8b28 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -12419,4 +12419,12 @@ enum skl_power_gate {
> #define TGL_ROOT_DEVICE_SKU_ULX 0x2
> #define TGL_ROOT_DEVICE_SKU_ULT 0x4
>
> +/* Registers for passlist check */

allowlist

> +#define PXP_REG_01_LOWERBOUND _MMIO(0x32260)
> +#define PXP_REG_01_UPPERBOUND _MMIO(0x32268)
> +#define PXP_REG_02_LOWERBOUND _MMIO(0x32670)
> +#define PXP_REG_02_UPPERBOUND _MMIO(0x32678)
> +#define PXP_REG_03_LOWERBOUND _MMIO(0x32860)
> +#define PXP_REG_03_UPPERBOUND _MMIO(0x32c7c)
> +
> #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index c9ef0025c60e..670856e095c4 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1990,16 +1990,41 @@ void intel_uncore_fini_mmio(struct intel_uncore *uncore)
> }
>
> static const struct reg_allowlist {
> - i915_reg_t offset_ldw;
> + i915_reg_t offset_ldw_lowerbound;
> + i915_reg_t offset_ldw_upperbound;
> i915_reg_t offset_udw;
> u16 gen_mask;
> u8 size;
> -} reg_read_allowlist[] = { {
> - .offset_ldw = RING_TIMESTAMP(RENDER_RING_BASE),
> +} reg_read_allowlist[] = {
> + {
> + .offset_ldw_lowerbound = RING_TIMESTAMP(RENDER_RING_BASE),
> + .offset_ldw_upperbound = RING_TIMESTAMP(RENDER_RING_BASE),
> .offset_udw = RING_TIMESTAMP_UDW(RENDER_RING_BASE),
> .gen_mask = INTEL_GEN_MASK(4, 12),
> .size = 8
> -} };
> + },
> + {
> + .offset_ldw_lowerbound = PXP_REG_01_LOWERBOUND,
> + .offset_ldw_upperbound = PXP_REG_01_UPPERBOUND,
> + .offset_udw = {0},
> + .gen_mask = INTEL_GEN_MASK(4, 12),
> + .size = 4
> + },
> + {
> + .offset_ldw_lowerbound = PXP_REG_02_LOWERBOUND,
> + .offset_ldw_upperbound = PXP_REG_02_UPPERBOUND,
> + .offset_udw = {0},
> + .gen_mask = INTEL_GEN_MASK(4, 12),
> + .size = 4
> + },
> + {
> + .offset_ldw_lowerbound = PXP_REG_03_LOWERBOUND,
> + .offset_ldw_upperbound = PXP_REG_03_UPPERBOUND,
> + .offset_udw = {0},
> + .gen_mask = INTEL_GEN_MASK(4, 12),
> + .size = 4
> + }
> +};
>
> int i915_reg_read_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file)
> @@ -2012,18 +2037,22 @@ int i915_reg_read_ioctl(struct drm_device *dev,
> unsigned int flags;
> int remain;
> int ret = 0;
> + i915_reg_t offset_ldw;
>
> entry = reg_read_allowlist;
> remain = ARRAY_SIZE(reg_read_allowlist);
> while (remain) {
> - u32 entry_offset = i915_mmio_reg_offset(entry->offset_ldw);
> + u32 entry_offset_lb = i915_mmio_reg_offset(entry->offset_ldw_lowerbound);
> + u32 entry_offset_ub = i915_mmio_reg_offset(entry->offset_ldw_upperbound);
>
> GEM_BUG_ON(!is_power_of_2(entry->size));
> GEM_BUG_ON(entry->size > 8);
> - GEM_BUG_ON(entry_offset & (entry->size - 1));
> + GEM_BUG_ON(entry_offset_lb & (entry->size - 1));
> + GEM_BUG_ON(entry_offset_ub & (entry->size - 1));
>
> if (INTEL_INFO(i915)->gen_mask & entry->gen_mask &&
> - entry_offset == (reg->offset & -entry->size))
> + entry_offset_lb <= (reg->offset & -entry->size) &&
> + (reg->offset & -entry->size) <= entry_offset_ub)
> break;
> entry++;
> remain--;
> @@ -2033,23 +2062,21 @@ int i915_reg_read_ioctl(struct drm_device *dev,
> return -EINVAL;
>
> flags = reg->offset & (entry->size - 1);
> + offset_ldw = _MMIO(reg->offset - flags);
>
> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
> if (entry->size == 8 && flags == I915_REG_READ_8B_WA)
> reg->val = intel_uncore_read64_2x32(uncore,
> - entry->offset_ldw,
> + offset_ldw,
> entry->offset_udw);
> else if (entry->size == 8 && flags == 0)
> - reg->val = intel_uncore_read64(uncore,
> - entry->offset_ldw);
> + reg->val = intel_uncore_read64(uncore, offset_ldw);
> else if (entry->size == 4 && flags == 0)
> - reg->val = intel_uncore_read(uncore, entry->offset_ldw);
> + reg->val = intel_uncore_read(uncore, offset_ldw);
> else if (entry->size == 2 && flags == 0)
> - reg->val = intel_uncore_read16(uncore,
> - entry->offset_ldw);
> + reg->val = intel_uncore_read16(uncore, offset_ldw);
> else if (entry->size == 1 && flags == 0)
> - reg->val = intel_uncore_read8(uncore,
> - entry->offset_ldw);
> + reg->val = intel_uncore_read8(uncore, offset_ldw);
> else
> ret = -EINVAL;
> }

--
Jani Nikula, Intel Open Source Graphics Center