<html> <head> <base href="https://bugs.freedesktop.org/"> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW - [CI][DRMTIP] igt@pm_rpm@* - dmesg-warn - *ERROR* DBus power enable timeout!" href="https://bugs.freedesktop.org/show_bug.cgi?id=108654#c5">Comment # 5</a> on <a class="bz_bug_link bz_status_NEW " title="NEW - [CI][DRMTIP] igt@pm_rpm@* - dmesg-warn - *ERROR* DBus power enable timeout!" href="https://bugs.freedesktop.org/show_bug.cgi?id=108654">bug 108654</a> from <a class="email" href="mailto:imre.deak@intel.com" title="Imre Deak <imre.deak@intel.com>"> Imre Deak</a> <pre>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).</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the QA Contact for the bug.</li> <li>You are on the CC list for the bug.</li> </ul> </body> </html>