Re: [PATCH v3 1/5] drm/i915/uncore: Reorganize and document shadow and forcewake tables

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 10, 2022 at 11:02:24PM -0700, Matt Roper wrote:
> Let's reorganize some of the forcewake/shadow handling in intel_uncore.c
> and consolidate the cargo-cult comments on each table into more general
> comments that apply to all tables.
> 
> We'll probably move forcewake handling to its own dedicated file in the
> near future and further enhance this with true kerneldoc.  But this is a
> good intermediate step to help clarify the behavior a bit.
> 
> Cc: Stuart Summers <stuart.summers@xxxxxxxxx>
Reviewed-by: Matt Atwood <matthew.s.atwood@xxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 125 ++++++++++++++++++----------
>  1 file changed, 80 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 83517a703eb6..095e071e4053 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -938,36 +938,32 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
>  	return entry->domains;
>  }
>  
> -#define GEN_FW_RANGE(s, e, d) \
> -	{ .start = (s), .end = (e), .domains = (d) }
> -
> -/* *Must* be sorted by offset ranges! See intel_fw_table_check(). */
> -static const struct intel_forcewake_range __vlv_fw_ranges[] = {
> -	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x5000, 0x7fff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0xb000, 0x11fff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0x22000, 0x23fff, FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0x2e000, 0x2ffff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
> -};
> -
> -#define __fwtable_reg_read_fw_domains(uncore, offset) \
> -({ \
> -	enum forcewake_domains __fwd = 0; \
> -	if (NEEDS_FORCE_WAKE((offset))) \
> -		__fwd = find_fw_domain(uncore, offset); \
> -	__fwd; \
> -})
> +/*
> + * Shadowed register tables describe special register ranges that i915 is
> + * allowed to write to without acquiring forcewake.  If these registers' power
> + * wells are down, the hardware will save values written by i915 to a shadow
> + * copy and automatically transfer them into the real register the next time
> + * the power well is woken up.  Shadowing only applies to writes; forcewake
> + * must still be acquired when reading from registers in these ranges.
> + *
> + * The documentation for shadowed registers is somewhat spotty on older
> + * platforms.  However missing registers from these lists is non-fatal; it just
> + * means we'll wake up the hardware for some register accesses where we didn't
> + * really need to.
> + *
> + * The ranges listed in these tables must be sorted by offset.
> + *
> + * When adding new tables here, please also add them to
> + * intel_shadow_table_check() in selftests/intel_uncore.c so that they will be
> + * scanned for obvious mistakes or typos by the selftests.
> + */
>  
> -/* *Must* be sorted by offset! See intel_shadow_table_check(). */
>  static const struct i915_range gen8_shadowed_regs[] = {
>  	{ .start =  0x2030, .end =  0x2030 },
>  	{ .start =  0xA008, .end =  0xA00C },
>  	{ .start = 0x12030, .end = 0x12030 },
>  	{ .start = 0x1a030, .end = 0x1a030 },
>  	{ .start = 0x22030, .end = 0x22030 },
> -	/* TODO: Other registers are not yet used */
>  };
>  
>  static const struct i915_range gen11_shadowed_regs[] = {
> @@ -1107,11 +1103,71 @@ gen6_reg_write_fw_domains(struct intel_uncore *uncore, i915_reg_t reg)
>  	return FORCEWAKE_RENDER;
>  }
>  
> +#define __fwtable_reg_read_fw_domains(uncore, offset) \
> +({ \
> +	enum forcewake_domains __fwd = 0; \
> +	if (NEEDS_FORCE_WAKE((offset))) \
> +		__fwd = find_fw_domain(uncore, offset); \
> +	__fwd; \
> +})
> +
> +#define __fwtable_reg_write_fw_domains(uncore, offset) \
> +({ \
> +	enum forcewake_domains __fwd = 0; \
> +	const u32 __offset = (offset); \
> +	if (NEEDS_FORCE_WAKE((__offset)) && !is_shadowed(uncore, __offset)) \
> +		__fwd = find_fw_domain(uncore, __offset); \
> +	__fwd; \
> +})
> +
> +#define GEN_FW_RANGE(s, e, d) \
> +	{ .start = (s), .end = (e), .domains = (d) }
> +
> +/*
> + * All platforms' forcewake tables below must be sorted by offset ranges.
> + * Furthermore, new forcewake tables added should be "watertight" and have
> + * no gaps between ranges.
> + *
> + * When there are multiple consecutive ranges listed in the bspec with
> + * the same forcewake domain, it is customary to combine them into a single
> + * row in the tables below to keep the tables small and lookups fast.
> + * Likewise, reserved/unused ranges may be combined with the preceding and/or
> + * following ranges since the driver will never be making MMIO accesses in
> + * those ranges.
> + *
> + * For example, if the bspec were to list:
> + *
> + *    ...
> + *    0x1000 - 0x1fff:  GT
> + *    0x2000 - 0x2cff:  GT
> + *    0x2d00 - 0x2fff:  unused/reserved
> + *    0x3000 - 0xffff:  GT
> + *    ...
> + *
> + * these could all be represented by a single line in the code:
> + *
> + *   GEN_FW_RANGE(0x1000, 0xffff, FORCEWAKE_GT)
> + *
> + * When adding new forcewake tables here, please also add them to
> + * intel_uncore_mock_selftests in selftests/intel_uncore.c so that they will be
> + * scanned for obvious mistakes or typos by the selftests.
> + */
> +
>  static const struct intel_forcewake_range __gen6_fw_ranges[] = {
>  	GEN_FW_RANGE(0x0, 0x3ffff, FORCEWAKE_RENDER),
>  };
>  
> -/* *Must* be sorted by offset ranges! See intel_fw_table_check(). */
> +static const struct intel_forcewake_range __vlv_fw_ranges[] = {
> +	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x5000, 0x7fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0xb000, 0x11fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x22000, 0x23fff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x2e000, 0x2ffff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
> +};
> +
> +
>  static const struct intel_forcewake_range __chv_fw_ranges[] = {
>  	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x4000, 0x4fff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
> @@ -1131,16 +1187,6 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
>  	GEN_FW_RANGE(0x30000, 0x37fff, FORCEWAKE_MEDIA),
>  };
>  
> -#define __fwtable_reg_write_fw_domains(uncore, offset) \
> -({ \
> -	enum forcewake_domains __fwd = 0; \
> -	const u32 __offset = (offset); \
> -	if (NEEDS_FORCE_WAKE((__offset)) && !is_shadowed(uncore, __offset)) \
> -		__fwd = find_fw_domain(uncore, __offset); \
> -	__fwd; \
> -})
> -
> -/* *Must* be sorted by offset ranges! See intel_fw_table_check(). */
>  static const struct intel_forcewake_range __gen9_fw_ranges[] = {
>  	GEN_FW_RANGE(0x0, 0xaff, FORCEWAKE_GT),
>  	GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
> @@ -1176,7 +1222,6 @@ static const struct intel_forcewake_range __gen9_fw_ranges[] = {
>  	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
>  };
>  
> -/* *Must* be sorted by offset ranges! See intel_fw_table_check(). */
>  static const struct intel_forcewake_range __gen11_fw_ranges[] = {
>  	GEN_FW_RANGE(0x0, 0x1fff, 0), /* uncore range */
>  	GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
> @@ -1215,14 +1260,6 @@ static const struct intel_forcewake_range __gen11_fw_ranges[] = {
>  	GEN_FW_RANGE(0x1d4000, 0x1dbfff, 0)
>  };
>  
> -/*
> - * *Must* be sorted by offset ranges! See intel_fw_table_check().
> - *
> - * Note that the spec lists several reserved/unused ranges that don't
> - * actually contain any registers.  In the table below we'll combine those
> - * reserved ranges with either the preceding or following range to keep the
> - * table small and lookups fast.
> - */
>  static const struct intel_forcewake_range __gen12_fw_ranges[] = {
>  	GEN_FW_RANGE(0x0, 0x1fff, 0), /*
>  		0x0   -  0xaff: reserved
> @@ -1327,8 +1364,6 @@ static const struct intel_forcewake_range __gen12_fw_ranges[] = {
>  /*
>   * Graphics IP version 12.55 brings a slight change to the 0xd800 range,
>   * switching it from the GT domain to the render domain.
> - *
> - * *Must* be sorted by offset ranges! See intel_fw_table_check().
>   */
>  #define XEHP_FWRANGES(FW_RANGE_D800)					\
>  	GEN_FW_RANGE(0x0, 0x1fff, 0), /*					\
> -- 
> 2.35.1
> 



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux