On Fri, 2026-09-04 at 12:56 +0300, Jani Nikula wrote:
> 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.
Ack. Forgot to remove that!
>
> 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.
Okay. Will update.
BR
vinod
>
> > - 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,
>
