[Bug 108654] [CI][DRMTIP] igt at pm_rpm@* - dmesg-warn - *ERROR* DBus power enable timeout!

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=108654

--- Comment #5 from Imre Deak <imre.deak at intel.com> ---
I was wondering how this can happen since we have all outputs disabled and
runtime suspended. Then by now icl_dbuf_disable() should have zeroed
dev_priv->wm.skl_hw.ddb.enabled_slices and skl_compute_ddb() copied this
0 to intel_state->wm_results.ddb.enabled_slices during the compute phase
of the problematic commit (since no crtcs are active this 0 should've
been kept as-is). Then here both hw_enabled_slices and required_slices
should be 0, but it's obviously not the case.

What could happen I think is that the store in icl_dbuf_disable() /
icl_dbuf_enable() can race with the load in skl_compute_ddb(). So perhaps
skl_compute_ddb() copied from dev_priv enabled_slices as 1 or 2 (the only
values we compute during a commit), then icl_dbuf_disable() zeroed in a racy
way enabled_slices in dev_priv and we get here required_slices being 1 or 2 and
hw_enabled_slices being 0.

To fix the above I think we would need to do the following:
1. Do not update enabled_slices in icl_dbuf_enable() / icl_dbuf_disable()
   We instead depend on the DDB HW readout to set the correct initial value
   after module loading/resume and any following commit to update it if
   needed (based on new resolution etc.).

   After this we could still end up with stale values in
   wm.skl_hw.ddb.enabled_slices, for instance if it's 2 b/c of two pipes
   being enabled we wouldn't set it to 1 atm when disabling both pipes
   in the same atomic commit. So we'd also need the following 2 changes.

2. Force-enable only a single slice in icl_dbuf_enable() (as the spec
   requires) keeping the 2nd slice enabled only if BIOS has enabled it
   (DDB HW readout will set the correct enabled_slices afterwards).

3. Make sure we always compute the proper enabled_slices in the atomic
   state, by moving the calculation from intel_get_ddb_size() to
   earlier, performing it even if all crtcs are disabled.

Ville also noticed a related problem where we don't recalculate the slice count
during a non-modeset commit, where a plane configuration change would require
this (due to the changed pixel bandwidth).

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx-bugs/attachments/20181130/1d3f5d06/attachment-0001.html>