On Thu, Oct 06, 2022 at 12:01:17AM +0300, Imre Deak wrote: > On Wed, Oct 05, 2022 at 10:14:43PM +0300, Ville Syrjälä wrote: > > On Wed, Oct 05, 2022 at 08:52:51PM +0300, Imre Deak wrote: > > > The GPU reset involves a display suspend/resume sequence, but this is > > > done without suspending/resuming the encoders. > > > > The display reset path is there for the old platforms which > > can't reset the gt stuff separately from the display engine. > > And the only reason we started to force that codepath on more > > modern platforms was to make sure it doesn't break all the time. > > That used to happen quite regularly, but not sure if we even had > > any pre-g4x hw in CI at the time. > > > > I suspect it's probably a mistake to start piling on more > > code in there just to make it work on really modern hw. > > The old hw where it actually matters doesn't need any of > > that code after all. > > Ok, but for the !clobbers_display case the current resume sequence is > broken imo. So if this fix is not acceptable how about only restoring > modeset_restore_state in this case without reading out the HW state > first (to keep some test coverage still) or removing the > force_reset_modeset_test? So the conclusion from our chat was to nuke all the extra junk from the simulated path and leave it with just the commit_duplicated_state(). I think that's still sufficient test of the display vs. reset path since it should still grab the modeset locks and whatnot. Well, sufficient assuming it actually works :) -- Ville Syrjälä Intel
