[Intel-gfx] [PATCH 2/2] drm/i915: Test all fw tables during mock selftests

[Chris Wilson]

On Fri, Dec 09, 2016 at 09:41:30AM +0000, Tvrtko Ursulin wrote:
> 
> On 08/12/2016 22:29, Chris Wilson wrote:
> >On Thu, Dec 08, 2016 at 04:52:24PM +0000, Tvrtko Ursulin wrote:
> >>Idea for another late test:
> >>
> >>for (offset = 0; offset < 0x40000; offset++) {
> >>	fw_domain = intel_uncore_forcewake_for_reg(dev_priv, { .reg =
> >>offset }, FW_REG_READ | FW_REG_WRITE);
> >>	if (WARN_ON(fw_domain & ~dev_priv->uncore.fw_domains))
> >>		return -EINVAL;
> >>}
> >>
> >>And then we could convert the existing related WARNs in the live
> >>code base to GEM_WARN_ONs.
> >
> >I liked it enough to type something up... However, isn't the argument
> >for skipping some fw_domains that the associated register banks are
> >invalid/absent on those platforms? i.e. we can't simply walk
> >	for (offset = 0; NEEDS_FORCE_WAKE(offset); offset++)
> >and expect every offset to correspond to a register (and so need a
> >covering fw_domain)?
> 
> I thought it won't be a problem because it would return 0 in those
> cases. So only those offsets that need a forcewake in a platform can
> trigger the fail.
> 
> Oh I see. We would need another layer before the condition checking, like:
> 
> fw_domains = intel_uncore_forcewake_for_reg(...)
> if ((fw_domains & FORCEWAKE_MEDIA) && !HAS_ENGINE(VCS))
> 	fw_domains &= ~FORCEWAKE_MEDIA;

That's almost like saying we should just
	return entry->domain & i915->uncore.fw_domains;
which we do anyway by the filtering in force_wake_auto.

Hmm. Could we use the mmio debug. Something like

	valid_reg = bitmap_create(0x40000)
	FORCEWAKE_ALL
	for_each_offset()
		read reg
		if (!mmio_debug)
			set_bit(valid_reg);

	for_each_bit()
		FORCEWAKE_DISABLE
		read reg using fw_domains
		if (mmio_debug)
			AWOOGA!
?

Hopefully read reg won't cause system hangs, and less liable to corrupt
state than write reg.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre