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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 13, 2026 at 10:08:47AM +0000, Kandpal, Suraj wrote:
> > 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

Yeah, I think the sooner we enable this on NVL the better. We want to
catch the issues early. For PTL someone should just send a patch to
enable it (separately from the NVL changes) and hopefully CI will tell
us whether it's still a problem there or not.

-- 
Ville Syrjälä
Intel