Re: [regression] HDMI breakage just before poweroff

[Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>]

On Tue, 12 Jun 2018 11:30:49 +0100,
Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> 
> On Tue, Jun 12, 2018 at 7:27 PM JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> >
> > Hi Marc,
> >
> > On 06/12/2018 04:54 PM, Marc Zyngier wrote:
> > > On 12/06/18 04:52, JeffyChen wrote:
> > >> Hi Vicente,
> > >>
> > >> On 06/12/2018 06:04 AM, Vicente Bergas wrote:
> > >>> On Thu, May 3, 2018 at 2:14 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> > >>>> On 03/05/18 04:51, JeffyChen wrote:
> > >>>>> On 05/03/2018 03:36 AM, Vicente Bergas wrote:
> > >>> [snip]
> > >>>>>> With 4.17.0-rc3, when reaching the halted state, the HDMI console
> > >>>>>> shows colorful static noise.
> > >>>>>>
> > >>>>> we've added a shutdown() to the iommu driver:
> > >>>>> https://patchwork.kernel.org/patch/10230817/
> > >>>>>
> > >>>>> any chance related?
> > >>>>
> > >>>> For sure - the IOMMU shutdown disables paging, so if the VOP is still
> > >>>> scanning out then that will result in whatever IOVAs it was using now going
> > >>>> straight out onto the bus as physical addresses. Between the RK3399 memory
> > >>>> map and the way the IOVA allocator works, that probably means it's reading
> > >>>> from all over the peripherals region, which, er, isn't ideal.
> > >>>
> > >>> Hi,
> > >>> just wondering if there has been any progress on that front?
> > >>>
> > >>
> > >> maybe we can add .shutdown() to rockchip_drm_drv too? (maybe just call
> > >> drm_atomic_helper_shutdown())
> > >>
> > >> could you help to try this hack:
> > >>
> > >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> > >> @@ -451,6 +451,7 @@ MODULE_DEVICE_TABLE(of, rockchip_drm_dt_ids);
> > >>    static struct platform_driver rockchip_drm_platform_driver = {
> > >>           .probe = rockchip_drm_platform_probe,
> > >>           .remove = rockchip_drm_platform_remove,
> > >> +       .shutdown = rockchip_drm_platform_remove,
> > >>
> > >
> > > Is there any mechanism guaranteeing the ordering of shutdown between VOP
> > > and IOMMU?
> >
> > it seems like the device_shutdown() will walk the devices_kset->list
> > backward.
> >
> > and the devices_kset->list's order is based on the probe
> > order(drivers/base/dd.c -> really_probe() -> devices_kset_move_last())
> >
> > and we are using of_iommu for rockchip-iommu, which would make sure
> > iommu probed before iommu master, so the vop iommu would be
> > shutdown after vop
> 
> Rather than shutting down the IOMMU, shouldn't we shut down all the
> respective master? The latter would automatically imply shutting down
> the IOMMUs, so we could remove the shutdown callback from the IOMMU
> driver.

As long as you can definitely ensure that you cannot have a active
IOMMU by the time you hit reboot/halt/kexec, that should work. But
experience seems to indicate that this is not a universal truth.

I'd be more confident if we had some form of fallback that would work
in the kexec/kdump use case.

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.