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? > Well, unless we manage to make it just call some simple high > level "suspend display + resume display" pair of functions > and nothing else. That would probably be nice simplification > in general, but iirc currently it's much more ad-hoc than that. I agree, but I'd say that should be done as a follow-up (just calling the same functions during both system supend/resume and reset I suppose) and have a simpler fix for the current issue. > > -- > Ville Syrjälä > Intel
