On Tue, 12 Jun 2018 11:30:49 +0100, Tomasz Figa <tfiga at chromium.org> wrote: > > On Tue, Jun 12, 2018 at 7:27 PM JeffyChen <jeffy.chen at rock-chips.com> 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 at arm.com> 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.