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 > > > > <[email protected]> > > > > Sent: Friday, March 13, 2026 9:55 AM > > > > To: Kandpal, Suraj <[email protected]>; > > > > [email protected]; [email protected] > > > > Cc: [email protected]; Shankar, Uma > > > > <[email protected]>; Sharma, Swati2 <[email protected]> > > > > 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 > > > > >>>> <[email protected]> > > > > >>>> --- > > > > >>>> 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
