[Intel-gfx] [PATCH] drm/i915/icl: Update forcewake firmware ranges
Sripada, Radhakrishna
radhakrishna.sripada at intel.com
Wed Apr 15 14:28:18 UTC 2020
Hi Matt,
> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Tuesday, April 14, 2020 11:40 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/icl: Update forcewake firmware ranges
>
> 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')
Comparing with previous gens, my understanding is the '0' is indicative of uncore range
Of registers. The reserved ranges are marked with "Blitter" force wake range. Am I in the right
Path making that assumption? Hence made all the subsequent changes according to the assumption.
Thanks,
Radhakrishna(RK) Sripada
>
>
> > 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
More information about the Intel-gfx
mailing list