Re: [PATCH 15/15] drm/i915/irq: add intel_display_irq_handler() to irq funcs

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 29 Apr 2026, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Apr 29, 2026 at 01:24:55PM +0300, Jani Nikula wrote:
>> @@ -2088,6 +2123,28 @@ static void vlv_display_irq_ack(struct intel_display *display,
>>  	i9xx_pipestat_irq_ack(display, state->iir, state->pipe_stats);
>>  }
>>  
>> +static bool vlv_display_irq_handler(struct intel_display *display,
>> +				    const struct intel_display_irq_state *state)
>> +{
>> +	u32 lpe_mask = I915_LPE_PIPE_A_INTERRUPT | I915_LPE_PIPE_B_INTERRUPT;
>> +
>> +	if (display->platform.cherryview)
>> +		lpe_mask |= I915_LPE_PIPE_C_INTERRUPT;
>
> I would prefer a function rather than the extra variable. The other
> option is to just use the CHV mask always. There is nothing on the
> extra bit on VLV so we never unmask it.

Hmm. reset and postinstall also have slight variations for VLV vs. CHV,
but only very slight. I think they're overall close enough that
splitting out a funcs struct and separate funcs for CHV is more
distracting and duplication than helpful.

Or would something like this be satisfactory? Just a couple of small
wrapper functions around intel_lpe_audio_irq_handler() here that the
compiler would just inline:

	if (display->platform.cherryview)
		chv_lpe_audio_irq_handler(display, state->iir);		
	else
		vlv_lpe_audio_irq_handler(display, state->iir);		

Of course this would need to move to the ack part per your other review
comment.


>> @@ -51,14 +51,12 @@ void bdw_disable_vblank(struct drm_crtc *crtc);
>>  
>>  void ilk_display_irq_master_disable(struct intel_display *display, u32 *de_ier, u32 *sde_ier);
>>  void ilk_display_irq_master_enable(struct intel_display *display, u32 de_ier, u32 sde_ier);
>> -bool ilk_display_irq_handler(struct intel_display *display);
>> -void gen8_de_irq_handler(struct intel_display *display, u32 master_ctl);
>> -void gen11_display_irq_handler(struct intel_display *display);
>>  
>>  u32 gen11_gu_misc_irq_ack(struct intel_display *display, const u32 master_ctl);
>>  void gen11_gu_misc_irq_handler(struct intel_display *display, const u32 iir);
>>  
>>  struct intel_display_irq_state {
>> +	u32 master_ctl;
>
> Ideally I'd like a separate structs for different platforms,
> but until I resurrect my old ack vs. handle split for all
> platforms I guess we don't need anything else here for
> ilk+. So good enough for now I suppose.

You mean per-platform substructs/unions within struct
intel_display_irq_state? I can do that.

BR,
Jani.



-- 
Jani Nikula, Intel