Re: [regression] HDMI breakage just before poweroff

[Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]

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.

Best regards,
Tomasz