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@xxxxxxxxx> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx> > --- > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
