Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: Gustavo Sousa <gustavo.sousa@intel.com>,
	intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Cc: ville.syrjala@linux.intel.com
Subject: Re: [PATCH 8/8] drm/i915/irq: move i915->irq_lock to display->irq.lock
Date: Wed, 07 May 2025 11:34:30 +0300	[thread overview]
Message-ID: <87o6w4vmy1.fsf@intel.com> (raw)
In-Reply-To: <174656703321.1401.8627403371256302933@intel.com>

On Tue, 06 May 2025, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Quoting Jani Nikula (2025-05-06 10:06:50-03:00)
>>Observe that i915->irq_lock is no longer used to protect anything
>>outside of display. Make it a display thing.
>>
>>This allows us to remove the ugly #define irq_lock irq.lock hack from xe
>>compat header.
>>
>>Note that this is slightly more subtle than it first looks. For i915,
>>there's no functional change here. The lock is moved. However, for xe,
>>we'll now have *two* locks, xe->irq.lock and display->irq.lock. These
>>should protect different things, though. Indeed, nesting in the past
>>would've lead to a deadlock because they were the same lock.
>>
>>With the i915 references gone, we can make a handful more files
>>independent of i915_drv.h.
>>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Besides reviewing the patch itself, I also did a git-grep to check for
> lexical references to irq_lock in the code after this patch is applied.
>
> I found 2 references in comments:
>
>  (1) A reference to "drm_i915_private::irq_lock" in the comment for member
>      detection_work_enabled of struct intel_hotplug. I think we can
>      simply refer to "intel_display::irq::lock" now.
>
>  (2) A reference to "i915->irq_lock" in a comment inside struct intel_rps.
>      Looking at the history, it looks like we started using gt->irq_lock
>      with commit d762043f7ab1 ("drm/i915: Extract GT powermanagement
>      interrupt handling"), which failed to update the comment. I think
>      we can update the comment to make it more accurate. I guess that
>      could be on a patch of its own...

Thanks for spotting these!

I sent a separate fix for (2), because it is kind of separate [1].

> So, with the small tweak suggested in (1),
>
>   Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

Thanks a lot for the reviews! I took the liberty of tweaking the comment
(1) while applying, and pushed the series to din.

BR,
Jani.


[1] https://lore.kernel.org/r/20250507083136.1987023-1-jani.nikula@intel.com


-- 
Jani Nikula, Intel

  reply	other threads:[~2025-05-07  8:34 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-06 13:06 [PATCH 0/8] drm/i915: irq_lock refactoring, move to display Jani Nikula
2025-05-06 13:06 ` [PATCH 1/8] drm/i915/irq: move locking inside vlv_display_irq_reset() Jani Nikula
2025-05-06 21:38   ` Gustavo Sousa
2025-05-06 13:06 ` [PATCH 2/8] drm/i915/irq: move locking inside valleyview_{enable, disable}_display_irqs() Jani Nikula
2025-05-06 21:38   ` Gustavo Sousa
2025-05-06 13:06 ` [PATCH 3/8] drm/i915/irq: move locking inside vlv_display_irq_postinstall() Jani Nikula
2025-05-06 21:39   ` Gustavo Sousa
2025-05-06 13:06 ` [PATCH 4/8] drm/i915/irq: split out i915_display_irq_postinstall() Jani Nikula
2025-05-06 21:39   ` Gustavo Sousa
2025-05-06 13:06 ` [PATCH 5/8] drm/i915/irq: split out i965_display_irq_postinstall() Jani Nikula
2025-05-06 21:40   ` Gustavo Sousa
2025-05-06 13:06 ` [PATCH 6/8] drm/i915/irq: make i915_enable_asle_pipestat() static Jani Nikula
2025-05-06 21:40   ` Gustavo Sousa
2025-05-06 13:06 ` [PATCH 7/8] drm/i915/rps: refactor display rps support Jani Nikula
2025-05-06 21:41   ` Gustavo Sousa
2025-05-06 13:06 ` [PATCH 8/8] drm/i915/irq: move i915->irq_lock to display->irq.lock Jani Nikula
2025-05-06 21:30   ` Gustavo Sousa
2025-05-07  8:34     ` Jani Nikula [this message]
2025-05-06 13:13 ` ✓ CI.Patch_applied: success for drm/i915: irq_lock refactoring, move to display Patchwork
2025-05-06 13:14 ` ✗ CI.checkpatch: warning " Patchwork
2025-05-06 13:15 ` ✓ CI.KUnit: success " Patchwork
2025-05-06 13:23 ` ✓ CI.Build: " Patchwork
2025-05-06 13:26 ` ✓ CI.Hooks: " Patchwork
2025-05-06 13:27 ` ✓ CI.checksparse: " Patchwork
2025-05-06 13:54 ` ✓ Xe.CI.BAT: " Patchwork
2025-05-06 16:15 ` ✗ Xe.CI.Full: failure " Patchwork
2025-05-26 16:00 ` ✗ CI.Patch_applied: " 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=87o6w4vmy1.fsf@intel.com \
    --to=jani.nikula@intel.com \
    --cc=gustavo.sousa@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=ville.syrjala@linux.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