On Fri, Aug 01, 2025 at 06:26:04PM +0300, Imre Deak wrote:
> Hi Greg and Shradha,
>
> could you please check the comment below about the 4ad8d57d902f backport
> commit in the v6.1.y stable tree and if you agree with the reasoning why
> it has an issue, then suggest a way to resolve it?
>
> Also, AFAICS, other stable trees are not affected, since the original
> 5abffb66d12bcac commit got only backported to the above v6.1.y tree, but
> please confirm this.
>
> On Fri, Aug 01, 2025 at 02:37:04PM +0000, nicusor.huhu...@siemens.com wrote:
> > > -----Original Message-----
> > > From: Imre Deak <imre.d...@intel.com>
> > > Sent: Wednesday, July 30, 2025 11:02 PM
> > > To: Nicusor Liviu Huhulea (FT FDS CES LX PBU 1)
> > > <nicusor.huhu...@siemens.com>
> > > Cc: sta...@vger.kernel.org; dri-de...@lists.freedesktop.org;
> > > intel-gfx@lists.freedesktop.org; cip-...@lists.cip-project.org;
> > > jouni.hogan...@intel.com; neil.armstr...@linaro.org;
> > > jani.nik...@linux.intel.com;
> > > maarten.lankho...@linux.intel.com; mrip...@kernel.org;
> > > tzimmerm...@suse.de;
> > > airl...@gmail.com; dan...@ffwll.ch; joonas.lahti...@linux.intel.com;
> > > rodrigo.v...@intel.com; tvrtko.ursu...@linux.intel.com;
> > > laurentiu.pa...@oss.nxp.com;
> > > Cedric Hombourger (FT FDS CES LX) <cedric.hombour...@siemens.com>;
> > > Shrikant Krishnarao Bobade (FT FDS CES LX PBU 1)
> > > <shrikant.bob...@siemens.com>
> > > Subject: Re: [PATCH 0/5] drm/i915: fixes for i915 Hot Plug Detection and
> > > build/runtime issues
> > >
> > > Hi Nicusor,
> > >
> > > thanks for the report and the root causing effort. The patchset itself
> > > has a few
> > > issues:
> > >
> > > - commit cfd48ad8c4a9 ("drm/i915: Fix HPD polling, reenabling the output
> > > poll work as needed") you backport fixes d33a54e3991d
> > > ("drm/probe_helper: sort out poll_running vs poll_enabled"), but this
> > > fixed commit is not part of the 6.1.y stable tree which you are
> > > targeting.
> > >
> > > Similarly commit d33a54e3991d fixes c8268795c9a9 ("drm/probe-helper:
> > > enable and disable HPD on connectors"), which is not part of 6.1.y
> > > either.
> > >
> > > This means the issue commit cfd48ad8c4a9 is fixing is not present in
> > > the 6.1.y tree, as the changes introducing that issue are not present
> > > in that tree either.
> > >
> > > - The compile errors the patches in your patchset introduce would
> > > prevent bisection, so fixing up these compile errors only at the end
> > > of the patchset is not ok; the tree should compile without errors at
> > > each patch/commit.
> > >
> > > Looking at v6.1.y and the patchset I suspect the actual issue is the
> > >
> > > commit 4ad8d57d902f ("drm: Check output polling initialized before
> > > disabling") backport in v6.1.y, which had the
> > >
> > > - if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
> > > + if (drm_WARN_ON_ONCE(dev, !dev->mode_config.poll_enabled) ||
> > > + !drm_kms_helper_poll || dev->mode_config.poll_running)
> > >
> > > change, not part of the original
> > >
> > > commit 5abffb66d12b ("drm: Check output polling initialized before
> > > disabling"). i.e.
> > > the original patch didn't add the check for
> > > dev->mode_config.poll_running. So could you try on top of v6.1.147
> > > (w/o the changes in the patchset you posted):
> > >
> > > diff --git a/drivers/gpu/drm/drm_probe_helper.c
> > > b/drivers/gpu/drm/drm_probe_helper.c
> > > index 0e5eadc6d44d..a515b78f839e 100644
> > > --- a/drivers/gpu/drm/drm_probe_helper.c
> > > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > > @@ -250,7 +250,7 @@ void drm_kms_helper_poll_enable(struct drm_device
> > > *dev)
> > > unsigned long delay = DRM_OUTPUT_POLL_PERIOD;
> > >
> > > if (drm_WARN_ON_ONCE(dev, !dev->mode_config.poll_enabled) ||
> > > - !drm_kms_helper_poll || dev->mode_config.poll_running)
> > > + !drm_kms_helper_poll)
> > > return;
> > >
> > > drm_connector_list_iter_begin(dev, &conn_iter);
> >
> > Thank you for your thorough explanation, especially regarding the
> > bisecting issue. I hadn't anticipated that by fixing compile errors
> > only at the end of the series, I would make bisection unreliable.
> >
> > I have tested your idea/fix and it works. HPD polling works reliably
> > on both devices. I can see the polling in logs when display cable is
> > not connected.
> >
> > Since this fix is mostly your solution, would you prefer to submit
> > yourself, or would you like me to resubmit it as a v2 and credit you
> > appropriately ?
>
> Thanks again Nicusor for the effort to root cause this and for all the
> tests.
>
> Greg, Shradha, as described above I think in the 4ad8d57d902f backport
> commit in v6.1.y it was incorrect to add the
>
> dev->mode_config.poll_running
>
> condition, as the original 5abffb66d12b commit was not the one adding
> this, in that commit that condition was only part of the diff context.
> OTOH, adding the check for this condition causes an issue in the i915
> driver's IRQ storm handling in the v6.1.y stable tree: after
> dev->mode_config.poll_running gets set (during the first connector
> detection in drm_helper_probe_single_connector_modes()), the
>
> drm_kms_helper_poll_enable()
>
> call in intel_hpd_irq_storm_switch_to_polling() will not any more
> schedule the output_poll_work as expected. Thus after an IRQ storm, the
> HPD IRQs get disabled, but the HPD polling will not run and so the HPD
> detection will not work as Nicusor described above.
>
> If you agree with the above and the above proposed solution to remove
> the dev->mode_config.poll_running check from the v6.1.y tree, then what
> would be Greg the correct way to do this?
Send whatever fix is needed please.
