On Thu, 03 Sep 2026, Vinod Govindapillai <[email protected]> wrote:
> If a device supports generating PME from HPDs, resetting HPD IRQs
> will be counter productive as HPDs itself will be lost. During
> suspend routines, all the IRQs are reset. So if the device is
> capable of generating PME rom HPDs, keep the HPD related IRQs from
> reset based on the PME capability of the device on a target power
> state. PME capability will be assessed and updated separately.
>
> Bspec: 52979, 52980, 68857, 68867, 68970
> Assisted-by: GitHub_Copilot:claude-opus-5
> Signed-off-by: Vinod Govindapillai <[email protected]>
> ---
> .../gpu/drm/i915/display/intel_display_irq.c | 20 ++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c
> b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index a59b75830bd1..fc50b19d5ddf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -22,6 +22,7 @@
> #include "intel_fdi_regs.h"
> #include "intel_fifo_underrun.h"
> #include "intel_gmbus.h"
> +#include "intel_hotplug.h"
> #include "intel_hotplug_irq.h"
> #include "intel_lpe_audio.h"
> #include "intel_parent.h"
> @@ -2217,8 +2218,11 @@ static void gen11_display_irq_reset(struct
> intel_display *display)
> enum pipe pipe;
> u32 trans_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
> BIT(TRANSCODER_C) | BIT(TRANSCODER_D);
> + /* If HPD can generate PME, we don't want to reset HPD related IRQs */
Please don't add comments in the middle of the declarations. IMO it's a
distraction rather than helpful.
To be fair, one comment like this here or there would be fine, but it
takes only a few to set an example both for humans and the language
models to start littering the declarations with comments, and I don't
want that.
> + bool keep_hpd = intel_hpd_can_generate_pme(display);
bool reset_hpd = !intel_hpd_can_generate_pme(display);
IMO this would be more clear.
> - intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
> + if (!keep_hpd)
> + intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
>
> if (DISPLAY_VER(display) >= 12) {
> enum transcoder trans;
> @@ -2250,13 +2254,15 @@ static void gen11_display_irq_reset(struct
> intel_display *display)
> irq_reset(display, GEN8_DE_PORT_IRQ_REGS);
> irq_reset(display, GEN8_DE_MISC_IRQ_REGS);
>
> - if (DISPLAY_VER(display) >= 14)
> - irq_reset(display, PICAINTERRUPT_IRQ_REGS);
> - else
> - irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
> + if (!keep_hpd) {
> + if (DISPLAY_VER(display) >= 14)
> + irq_reset(display, PICAINTERRUPT_IRQ_REGS);
> + else
> + irq_reset(display, GEN11_DE_HPD_IRQ_REGS);
>
> - if (INTEL_PCH_TYPE(display) >= PCH_ICP)
> - irq_reset(display, SDE_IRQ_REGS);
> + if (INTEL_PCH_TYPE(display) >= PCH_ICP)
> + irq_reset(display, SDE_IRQ_REGS);
> + }
> }
>
> void gen8_irq_power_well_post_enable(struct intel_display *display,
--
Jani Nikula, Intel
