[Intel-gfx] ✗ Fi.CI.BAT: warning for RFC: duct-tape for the gpu reset vs. modeset deadlock

[Maarten Lankhorst]

Op 15-07-17 om 15:45 schreef Daniel Vetter:
> On Sat, Jul 15, 2017 at 3:23 PM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>> On Sat, Jul 15, 2017 at 2:00 PM, Patchwork
>> <patchwork at emeril.freedesktop.org> wrote:
>>> == Series Details ==
>>>
>>> Series: RFC: duct-tape for the gpu reset vs. modeset deadlock
>>> URL   : https://patchwork.freedesktop.org/series/27345/
>>> State : warning
>>>
>>> == Summary ==
>>>
>>> Series 27345v1 RFC: duct-tape for the gpu reset vs. modeset deadlock
>>> https://patchwork.freedesktop.org/api/1.0/series/27345/revisions/1/mbox/
>>>
>>> Test drv_hangman:
>>>         Subgroup error-state-basic:
>>>                 pass       -> DMESG-WARN (fi-blb-e6850)
>>>                 pass       -> DMESG-WARN (fi-pnv-d510)
>>> Test gem_busy:
>>>         Subgroup basic-hang-default:
>>>                 pass       -> DMESG-WARN (fi-blb-e6850)
>>>                 pass       -> DMESG-WARN (fi-pnv-d510)
>>> Test gem_exec_fence:
>>>         Subgroup await-hang-default:
>>>                 pass       -> DMESG-WARN (fi-blb-e6850)
>>>                 pass       -> DMESG-WARN (fi-pnv-d510)
>> Looks like I wreaked something on gpus where the hw reset kills the
>> display. I'll check it out.
> Ok, simplest solution seems to be to just not do the special casing
> between real hw reset and fake reset, and unconditionally shut down
> all the CRTC. Probably a lot more than just the power domain tracking
> will get out of whack. I wasn't really sold on that idea either way,
> so I'll drop both "drm/i915: only disable outputs in simulated gpu
> resets" and "drm/atomic-helper: add sync_all helper for gpu reset"
> from the series. They're not needed really anyway.
> -Daniel

Agreed, sync_all helper should have been done by the disable_all call.

The thing I didn't like was that you didn't shut down the GPU unconditionally, but with that fixed series looks good. :)
I like patch 6/7, it makes everything more clear.

I'm not sure if calling prepare_to_wait twice is allowed, but don't see why not.

So with those taken out and feedback addressed.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>