Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown

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

Hi Vicente,

On Sun,  5 Aug 2018 16:09:11 +0200
Vicente Bergas <vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> As explained by Robin Murphy:
> > 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.  
> 
> Suggested-by: JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Suggested-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: Vicente Bergas <vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index f814d37b1db2..00a06768edb2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -442,6 +442,14 @@ static int rockchip_drm_platform_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static void rockchip_drm_platform_shutdown(struct platform_device *pdev)
> +{
> +	struct drm_device *drm = platform_get_drvdata(pdev);
> +
> +	if (drm)
> +		drm_atomic_helper_shutdown(drm);

I'm not completely sure this is the right thing to do here. We want
shutdown to teardown the whole thing, not just the DRM context.

The driver already calls drm_atomic_helper_shutdown as part of the
unbind sequence, which looks very much like what we're trying to
achieve here.

I've already posted a patch[1] which calls the .remove handler,
ensuring that the whole infrastructure gets torn down at shutdown time.

Thanks,

	M.

[1] https://www.spinics.net/lists/arm-kernel/msg670229.html
-- 
Without deviation from the norm, progress is not possible.