RE: [PATCH 2/2] drm/i915/dmc: Enable PIPEDMC_ERROR interrupt

Kandpal, Suraj
2026-03-13T10:08:52Z

> Subject: Re: [PATCH 2/2] drm/i915/dmc: Enable PIPEDMC_ERROR interrupt
>
> On Fri, Mar 13, 2026 at 05:04:46AM +0000, Kandpal, Suraj wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dibin Moolakadan Subrahmanian
> > > <dibin.moolakadan.subrahmanian@intel.com>
> > > Sent: Friday, March 13, 2026 9:55 AM
> > > To: Kandpal, Suraj <suraj.kandpal@intel.com>;
> > > intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> > > Cc: ville.syrjala@linux.intel.com; Shankar, Uma
> > > <uma.shankar@intel.com>; Sharma, Swati2 <swati2.sharma@intel.com>
> > > Subject: Re: [PATCH 2/2] drm/i915/dmc: Enable PIPEDMC_ERROR
> > > interrupt
> > >
> > >
> > > On 13-03-2026 08:56, Kandpal, Suraj wrote:
> > > >> On 12-03-2026 08:48, Kandpal, Suraj wrote:
> > > >>>> Subject: [PATCH 2/2] drm/i915/dmc: Enable PIPEDMC_ERROR
> > > >>>> interrupt
> > > >>>>
> > > >>>> Enable PIPEDMC_ERROR interrupt bit for display version 35+.
> > > >>>>
> > > >>> Add same Bspec link here too
> > > >>>
> > > >>>> Signed-off-by: Dibin Moolakadan Subrahmanian
> > > >>>> <dibin.moolakadan.subrahmanian@intel.com>
> > > >>>> ---
> > > >>>> drivers/gpu/drm/i915/display/intel_dmc.c | 3 ++-
> > > >>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >>>>
> > > >>>> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > >>>> b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > >>>> index 38b284a0db82..e60f1f977070 100644
> > > >>>> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > >>>> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > >>>> @@ -510,7 +510,8 @@ static void pipedmc_clock_gating_wa(struct
> > > >>>> intel_display *display, bool enable) static u32
> > > >>>> pipedmc_interrupt_mask(struct intel_display *display) {
> > > >>>> if (DISPLAY_VER(display) >= 35)
> > > >>>> - return PIPEDMC_FLIPQ_PROG_DONE;
> > > >>>> + return PIPEDMC_FLIPQ_PROG_DONE |
> > > >>>> + PIPEDMC_ERROR;
> > > >>>>
> > > >>> Mostly looks okay but here's my question:
> > > >>> I know LNL pipe B had an issue with PIPEDMC_ERROR being
> > > >>> triggered on LNL pipe B, As I can see from Ville's commit
> > > >>> message, but is it still the case for
> > > >> PTL ?
> > > >>> Can we have that tested ?
> > > >>> If that works we can add the PIPEDMC_ERROR from PTL onwards.
> > > >>> Then here we can change code to create a mask and then return it
> > > >>> finally like
> > > >> :
> > > >>> mask = PIPEDMC_FLIPQ_PROG_DONE
> > > >>>
> > > >>> if display ver >= 30
> > > >>> mask |= PIPEDMC_ERROR
> > > >>>
> > > >>> if display ver < 35
> > > >>> mask |= PIPEDMC_GTT_FAULT |
> > > >>> PIPEDMC_ATS_FAULT;
> > > >>>
> > > >>> Return mask;
> > > >>>
> > > >>> Obviously that is if PIPEDMC_ERROR works on PTL properly.
> > > >> Thank you for spotting this, I think its better to add above
> > > >> logic in new series rather than combing with 35+ bit mask update.
> > > >>
> > > >> Regards,
> > > >> Dibin
> > > > If that is the case then I think its better to drop this patch altogether.
> > > > We have a justification of why we remove bits in first patch, that
> > > > was a change
> > > in NVL H/w.
> > > > But this change was introduced in LNL.
> > > > Without a strong reasoning of why you are enabling this is in NVL
> > > > and not in PTL (which I don’t see in this patch series) I suggest
> > > > you add this patch with as a part of the series where you have a
> > > > use case for it. And if
> > > there too you only add it for NVL You will need to add a comments as
> > > to why this is not enabled for PTL.
> > >
> > > This patch intent to fix the interrupt mask for 35+.
> > > I dont see any reason to disable this bit as
> > > 1) error bit warning is already present in interrupt handler.
> > > 2) bit is defined in bsepc.
> > > 3) LNL it was mentioned disabled because pipeB triggering it during
> > > first DC state transition which did not see in this case.
> >
> > In that case the interrupt handler is made to report errors if this bit is
> unmasked for >= LNL.
> > Now this bit is introduced in LNL timeframe for which the reason to not add it
> is mentioned in comment and documented.
> > Similarly if you want to skip PTL you will need this to be documented
> > with the reason. Which means the FIXME comment needs to be modified In
> the least. If this patch is to go through.
> > Also Ville can you shed some light, on what the H/w folks had to say
> > regarding this and if they had mentioned any WA for LNL, and if this is fixed In
> LNL+.
>
> I suspect it might be some kind of issue in the DMC firmware where it's
> accessing unpowered registers. But it was never investigated properly.
>
> It would be good if someone could take that up and actually figure out what's
> going on. The problem is figuring out what exactly is the register that causes
> this. I don't think LNL has any kind of RM_CAPTURE register/etc available for
> the DMC that would directly tell us that :(
>
> IIRC the Windows driver did seem to enable the error interrupt on LNL, but
> either they just ignore all the reported errors, or somehow the way they use
> the hardware/firmware doesn't trigger them.

Hmm would it be okay if we can move with enabling the bit for NVL+ since Dibin says we don’t see this issue
Anymore, while we add or TODO or FIXME in the comment to investigate this further for PTL and LNL

Regards,
Suraj Kandpal

>
> --
> Ville Syrjälä
> Intel