[Intel-gfx] [PATCH] drm/i915/icl: Update forcewake firmware ranges

Tue Apr 14 18:39:55 UTC 2020

On Mon, Apr 13, 2020 at 02:00:03AM -0700, Radhakrishna Sripada wrote:
> Some workarounds are not sticking across suspend resume cycles. The
> forcewake ranges table has been updated and would reflect the hardware
> appropriately.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1222

It's unfortunate that they still haven't updated the bspec for this yet.
A few comments below based on my understanding of the document we
received from the hardware team.

> 
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 55 +++++++++++++++++++++--------
>  1 file changed, 41 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index fa86b7ab2d99..c0e21697a44c 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1098,32 +1098,59 @@ static const struct intel_forcewake_range __gen11_fw_ranges[] = {

Looks like the first range in this table (0x0 - 0xaff) needs to be
changed to '0' (or rather combined with the following entry for a
combined 0x0 - 0x1fff range set to '0')

>  	GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x4000, 0x51ff, FORCEWAKE_BLITTER),
> -	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x5200, 0x53ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x5400, 0x54ff, FORCEWAKE_BLITTER),

In the version of the document I'm looking at, the wake target for this
entry is blank, which usually means no wake target is required.  AFAICS,
no registers actually fall in this range on gen11, so I'd either set
this to '0' to explicitly match the document, or just leave it combined
with the two surrounding render ranges for simplicity (and smaller table
size).

> +	GEN_FW_RANGE(0x5500, 0x7fff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x8000, 0x813f, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x8140, 0x815f, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x8160, 0x82ff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x8500, 0x8bff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x8500, 0x87ff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x8800, 0x883f, 0),
> +	GEN_FW_RANGE(0x8840, 0x8bff, FORCEWAKE_BLITTER),

I see 0x8840 - 0x8bff as a reserved range with no forcewake target so we
should be able to combine it with the range before it.

>  	GEN_FW_RANGE(0x8c00, 0x8cff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x8d00, 0x93ff, FORCEWAKE_BLITTER),
> -	GEN_FW_RANGE(0x9400, 0x97ff, FORCEWAKE_ALL),
> -	GEN_FW_RANGE(0x9800, 0xafff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x8d00, 0x94cf, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x94d0, 0x955f, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x9560, 0x95ff, 0),
> +	GEN_FW_RANGE(0x9600, 0xafff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0xb480, 0xdeff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0xdf00, 0xe8ff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0xe900, 0x16dff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x16e00, 0x19fff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x1a000, 0x243ff, FORCEWAKE_BLITTER),
> -	GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x24800, 0x3ffff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x1a000, 0x23fff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x24000, 0x2407f, 0),
> +	GEN_FW_RANGE(0x24080, 0x2417f, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x24180, 0x242ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x24300, 0x243ff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x24400, 0x245ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x24600, 0x2467f, FORCEWAKE_BLITTER),

This one is a reserved range with no registers, so we can just squash it
into the two render ranges on either side of it.

> +	GEN_FW_RANGE(0x24680, 0x247ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x24800, 0x249ff, FORCEWAKE_BLITTER),

Also reserved with no registers.

> +	GEN_FW_RANGE(0x24a00, 0x24a7f, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x24a80, 0x24dff, FORCEWAKE_BLITTER),

Also reserved with no registers.

So I think we should be able to have just use one render range that runs
from 0x24400 - 0x24fff.

> +	GEN_FW_RANGE(0x24e00, 0x24fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x25000, 0x3ffff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x40000, 0x1bffff, 0),
> -	GEN_FW_RANGE(0x1c0000, 0x1c3fff, FORCEWAKE_MEDIA_VDBOX0),
> -	GEN_FW_RANGE(0x1c4000, 0x1c7fff, FORCEWAKE_MEDIA_VDBOX1),
> -	GEN_FW_RANGE(0x1c8000, 0x1cbfff, FORCEWAKE_MEDIA_VEBOX0),
> +	GEN_FW_RANGE(0x1c0000, 0x1c2bff, FORCEWAKE_MEDIA_VDBOX0),
> +	GEN_FW_RANGE(0x1c2c00, 0x1c3eff, FORCEWAKE_BLITTER),

This is another reserved range that can be combined into the ranges
around it (0x1c0000 - 0x1c3fff).

> +	GEN_FW_RANGE(0x1c3f00, 0x1c3fff, FORCEWAKE_MEDIA_VDBOX0),
> +	GEN_FW_RANGE(0x1c4000, 0x1c6bff, FORCEWAKE_MEDIA_VDBOX1),
> +	GEN_FW_RANGE(0x1c6c00, 0x1c7eff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x1c7f00, 0x1c7fff, FORCEWAKE_MEDIA_VDBOX1),

0x1c4000 - 0x1c7fff are reserved for vdbox1, but that doesn't exist on
this platform (we only have vdbox0 and vdbox2 on ICL and only vdbox0 on
EHL/JSL).  So the three entries above should be combined into a single
range with target '0' here.

> +	GEN_FW_RANGE(0x1c8000, 0x1ca0ff, FORCEWAKE_MEDIA_VEBOX0),
> +	GEN_FW_RANGE(0x1ca100, 0x1cbeff, FORCEWAKE_BLITTER),

Another reserved range; can squash with the two surrounding ranges.

> +	GEN_FW_RANGE(0x1cbf00, 0x1cbfff, FORCEWAKE_MEDIA_VEBOX0),
>  	GEN_FW_RANGE(0x1cc000, 0x1cffff, FORCEWAKE_BLITTER),

Another reserved range.  Either set to 0 or combine with one of the
ranges before/after it.

> -	GEN_FW_RANGE(0x1d0000, 0x1d3fff, FORCEWAKE_MEDIA_VDBOX2),
> -	GEN_FW_RANGE(0x1d4000, 0x1d7fff, FORCEWAKE_MEDIA_VDBOX3),
> -	GEN_FW_RANGE(0x1d8000, 0x1dbfff, FORCEWAKE_MEDIA_VEBOX1)
> +	GEN_FW_RANGE(0x1d0000, 0x1d2bff, FORCEWAKE_MEDIA_VDBOX2),
> +	GEN_FW_RANGE(0x1d2c00, 0x1d3eff, FORCEWAKE_BLITTER),

Reserved range; can squash with ranges around it.

> +	GEN_FW_RANGE(0x1d3f00, 0x1d3fff, FORCEWAKE_MEDIA_VDBOX2),
> +	GEN_FW_RANGE(0x1d4000, 0x1d6bff, FORCEWAKE_MEDIA_VDBOX3),
> +	GEN_FW_RANGE(0x1d6c00, 0x1d7eff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x1d7f00, 0x1d7fff, FORCEWAKE_MEDIA_VDBOX3),
> +	GEN_FW_RANGE(0x1d8000, 0x1da0ff, FORCEWAKE_MEDIA_VEBOX1),
> +	GEN_FW_RANGE(0x1da100, 0x1dbeff, FORCEWAKE_BLITTER),
> +	GEN_FW_RANGE(0x1dbf00, 0x1dbfff, FORCEWAKE_MEDIA_VEBOX1)

As noted above, gen11 doesn't have vdbox1 or vdbox3, so these entries
should just be set to '0' (including the reserved ranges that you have
listed as blitter right now).

>  };
>  
>  /* *Must* be sorted by offset ranges! See intel_fw_table_check(). */
> -- 
> 2.20.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795