intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH v2 6/8] drm/i915/reset: move gt related stuff out of display reset
Date: Mon, 03 Mar 2025 13:29:02 +0200	[thread overview]
Message-ID: <87bjui1hfl.fsf@intel.com> (raw)
In-Reply-To: <20250226234611.GO4460@mdroper-desk1.amr.corp.intel.com>

On Wed, 26 Feb 2025, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Wed, Feb 26, 2025 at 12:38:40PM +0200, Jani Nikula wrote:
>> On Tue, 25 Feb 2025, Matt Roper <matthew.d.roper@intel.com> 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@intel.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> ---
>> >>  .../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@intel.com


-- 
Jani Nikula, Intel

  reply	other threads:[~2025-03-03 11:29 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-25 11:14 [PATCH v2 0/8] drm/i915: display reset cleanups Jani Nikula
2025-02-25 11:14 ` [PATCH v2 1/8] drm/i915/display: convert display reset to struct intel_display * Jani Nikula
2025-02-25 11:14 ` [PATCH v2 2/8] drm/i915: move pending_fb_pin to struct intel_display Jani Nikula
2025-02-25 11:14 ` [PATCH v2 3/8] drm/i915/reset: add intel_gt_gpu_reset_clobbers_display() helper Jani Nikula
2025-02-25 11:14 ` [PATCH v2 4/8] drm/i915/reset: add intel_display_reset_test() Jani Nikula
2025-02-25 11:16   ` Jani Nikula
2025-02-25 20:14   ` Matt Roper
2025-02-25 11:14 ` [PATCH v2 5/8] drm/i915/reset: add wrappers for display reset calls Jani Nikula
2025-02-25 20:15   ` Matt Roper
2025-02-25 11:14 ` [PATCH v2 6/8] drm/i915/reset: move gt related stuff out of display reset Jani Nikula
2025-02-25 20:35   ` Matt Roper
2025-02-26 10:38     ` Jani Nikula
2025-02-26 23:46       ` Matt Roper
2025-03-03 11:29         ` Jani Nikula [this message]
2025-02-25 11:14 ` [PATCH v2 7/8] drm/i915/reset: pass test only parameter to intel_display_reset_finish() Jani Nikula
2025-02-25 20:37   ` Matt Roper
2025-02-25 11:14 ` [PATCH v2 8/8] drm/i915/reset: add modeset_stuck callback to intel_display_reset_prepare() Jani Nikula
2025-02-25 20:39   ` Matt Roper
2025-02-25 12:50 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: display reset cleanups (rev2) Patchwork
2025-02-25 12:50 ` ✗ Fi.CI.SPARSE: " Patchwork
2025-02-25 13:10 ` ✓ i915.CI.BAT: success " Patchwork
2025-02-25 15:56 ` ✗ i915.CI.Full: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bjui1hfl.fsf@intel.com \
    --to=jani.nikula@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.d.roper@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).