On Wed, 26 Feb 2025, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote:
> On Wed, Feb 26, 2025 at 12:38:40PM +0200, Jani Nikula wrote:
>> On Tue, 25 Feb 2025, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote:
>> > On Tue, Feb 25, 2025 at 01:14:20PM +0200, Jani Nikula wrote:
>> >> Move the checks for whether display reset is needed as well as
>> >> I915_RESET_MODESET flag handling to gt side of things.
>> >>
>> >> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
>> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> >> ---
>> >> .../drm/i915/display/intel_display_reset.c | 15 --------------
>> >> drivers/gpu/drm/i915/gt/intel_reset.c | 20 +++++++++++++++++++
>> >> 2 files changed, 20 insertions(+), 15 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c
>> >> index b7962f90c21c..362436cd280f 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_display_reset.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c
>> >> @@ -37,15 +37,6 @@ void intel_display_reset_prepare(struct intel_display *display)
>> >> if (!HAS_DISPLAY(display))
>> >> return;
>> >>
>> >> - /* reset doesn't touch the display */
>> >> - if (!intel_display_reset_test(display) &&
>> >> - !gpu_reset_clobbers_display(display))
>> >> - return;
>> >> -
>> >> - /* We have a modeset vs reset deadlock, defensively unbreak it. */
>> >
>> > Doesn't this comment more accurately apply to the 'if' condition below
>> > rather than to the flag updates and wakeup we do before that? Assuming
>> > I'm understanding correctly, it seems like the comment should stay here
>> > and not move to the other file --- saying "We have a ... deadlock" is
>> > only true if we still have a pending pin after we've done that other
>> > stuff. The unbreaking part (by wedging) is still located here too.
>>
>> I'm... not sure.
>>
>> Commit d59cf7bb73f3 ("drm/i915/display: Use dma_fence interfaces instead
>> of i915_sw_fence") seems relevant. We no longer have anyone waiting on
>> I915_RESET_MODESET, and I think we could probably remove the bit from
>> reset flags altogether, and handle this locally in
>> intel_gt_reset_global(). Right?
>
> Yeah, I believe you're right.
So I modified the approach again [1]. Sorry, and thanks for bearing with
me!
BR,
Jani.
[1] https://lore.kernel.org/r/cover.1741001054.git.jani.nikula@xxxxxxxxx
--
Jani Nikula, Intel
