On Fri, Sep 23, 2022 at 03:29:34PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Kernel is trying to eliminate callers of flush_scheduled_work so lets
> try to accommodate.
>
> We currently call it from intel_modeset_driver_remove_noirq on the driver
> remove path but the comment next to it does not tell me what exact work it
> wants to flush.
>
> I can spot three (or four) works using the system_wq:
>
> ..hotplug.reenable_work
> ..hotplug.hotplug_work
Looks like we at least try to shoot those down via
intel_irq_uninstall()
->intel_hpd_cancel_work()
->cancel_delayed_work_sync()
But I'm not sure how broken the hpd disable path is here.
I know hpd cancel vs. irq disable has some known ordering
issues during suspend at least, some of which I think may
have gotten fixed recently. But hpd cancel is still a bit
of a mess in general.
Here we at least do cancel all the hpd works after irqs
have been disabled, so I don't think any further flushing
should help with whatever races we have left in there.
> ..psr.dc3co_work
I think the whole dc3co thing should be disabled atm,
so nothing should ever schedule this. We should
probably garbage collect the whole thing...
> ..crtc->drrs.work
That one should have been killed in
intel_display_driver_unregister()
->drm_atomic_helper_shutdown()
->...
->intel_drrs_deactivate()
->cancel_delayed_work_sync()
> So if I replace it with intel_hpd_cancel_work() that appears would handle
> the first two. What about the other two?
Other stuff that comes to mind is the pps vdd_off work.
But looks like that should get taken down in the
encoder->destroy() hook at the latest (via
intel_mode_config_cleanup()).
psr.work at least has a cancel_work_sync() in intel_psr_disable(),
so should hopefully get killed the same way as drrs.
opregion.asle_work seems to get cancelled from the unregister path.
The ones that look broken to me are dmc.work and fbc underrun_work.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> ---
> I am clueless about the display paths and only send this because Jani
> convinced me to send a patch to kick off the discussion. No expectations
> whatsoever this is correct or complete.
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 2d0018ae34b1..0eb72530a003 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8980,7 +8980,7 @@ void intel_modeset_driver_remove_noirq(struct drm_i915_private *i915)
> intel_unregister_dsm_handler();
>
> /* flush any delayed tasks or pending work */
> - flush_scheduled_work();
> + intel_hpd_cancel_work(i915);
>
> intel_hdcp_component_fini(i915);
>
> --
> 2.34.1
--
Ville Syrjälä
Intel
