Message ID | 20180805140911.19205-1-vicencb@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/rockchip: shutdown drm subsystem on shutdown | expand |
Hi Vicente, On Sun, 5 Aug 2018 16:09:11 +0200 Vicente Bergas <vicencb@gmail.com> 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@rock-chips.com> > Suggested-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Vicente Bergas <vicencb@gmail.com> > --- > 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
On Sun, Aug 5, 2018 at 6:50 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > Hi Vicente, > > On Sun, 5 Aug 2018 16:09:11 +0200 > Vicente Bergas <vicencb@gmail.com> 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@rock-chips.com> >> Suggested-by: Robin Murphy <robin.murphy@arm.com> >> Signed-off-by: Vicente Bergas <vicencb@gmail.com> >> --- >> 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. OK, but maybe it is calling drm_atomic_helper_shutdown too late because this patch makes a difference and does indeed fix the issue. Anyways, please, apply your version if you have reasons to think it is better suited to do the task. > > I've already posted a patch[1] which calls the .remove handler, > ensuring that the whole infrastructure gets torn down at shutdown time. That is funny: after months of having reported the issue, we both sent a patch to fix it in less than 3 hours difference! Regards, Vicente. > > Thanks, > > M. > > [1] https://www.spinics.net/lists/arm-kernel/msg670229.html > -- > Without deviation from the norm, progress is not possible.
On Sun, 05 Aug 2018 18:38:18 +0100, Vicente Bergas <vicencb@gmail.com> wrote: > > On Sun, Aug 5, 2018 at 6:50 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > > Hi Vicente, > > > > On Sun, 5 Aug 2018 16:09:11 +0200 > > Vicente Bergas <vicencb@gmail.com> 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@rock-chips.com> > >> Suggested-by: Robin Murphy <robin.murphy@arm.com> > >> Signed-off-by: Vicente Bergas <vicencb@gmail.com> > >> --- > >> 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. > > OK, but maybe it is calling drm_atomic_helper_shutdown too late > because this patch makes a difference and does indeed fix the issue. I'm not saying it doesn't fix your issue. All I'm saying is that we may want something that is closer to a full remove than just a shallow DRM teardown. At the moment, unbind is simply *never* called. > Anyways, please, apply your version if you have reasons to think it > is better suited to do the task. That would be for the DRM maintainers to decide. For that particular subsystem, I'm just a random contributor. > > I've already posted a patch[1] which calls the .remove handler, > > ensuring that the whole infrastructure gets torn down at shutdown time. > > That is funny: after months of having reported the issue, we both > sent a patch to fix it in less than 3 hours difference! I was hoping you'd do that earlier, but given that 4.18 is just a week away and that we still don't have a proper fix for this, I've taken the matter into my own hands. Without this, I cannot reliably use kexec anymore, which is just the same as not being able to boot at all. Thanks, M.
Hi Vicente, Am Sonntag, 5. August 2018, 16:09:11 CEST schrieb Vicente Bergas: > 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@rock-chips.com> > Suggested-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Vicente Bergas <vicencb@gmail.com> > --- > 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 tend to side with Marc's more drastic approach, especially as this one should also nicely unbind the encoders used. Are you ok with us going with Marc's patch or do you have concerns? Providing a Tested-by tag would also be great ;-) Thanks Heiko
Hi Heiko, Jeffy, Marc, On Tue, Aug 7, 2018 at 2:44 PM, Heiko Stuebner <heiko@sntech.de> wrote: > Hi Vicente, > > Am Sonntag, 5. August 2018, 16:09:11 CEST schrieb Vicente Bergas: >> 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@rock-chips.com> >> Suggested-by: Robin Murphy <robin.murphy@arm.com> >> Signed-off-by: Vicente Bergas <vicencb@gmail.com> >> --- >> 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 tend to side with Marc's more drastic approach, especially as this one > should also nicely unbind the encoders used. Are you ok with us going > with Marc's patch or do you have concerns? The patch i posted comes from Jeffy, as is, no modifications. So, if he has no concerns about it, then it is also fine for me. > > Providing a Tested-by tag would also be great ;-) OK, i'll reply to his patch with a Tested-by tag, but i was only aware of this issue affecting hdmi on power-off, so, the only testing performed was checking only this. I have done no kexec-related test. Only one issue related to this: Marc, how can i reply to your patch if i was not a recipient? Regards, Vicente. > > > Thanks > Heiko > >
Am Dienstag, 7. August 2018, 18:05:13 CEST schrieb Vicente Bergas: > Hi Heiko, Jeffy, Marc, > > On Tue, Aug 7, 2018 at 2:44 PM, Heiko Stuebner <heiko@sntech.de> wrote: > > Hi Vicente, > > > > Am Sonntag, 5. August 2018, 16:09:11 CEST schrieb Vicente Bergas: > >> 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@rock-chips.com> > >> Suggested-by: Robin Murphy <robin.murphy@arm.com> > >> Signed-off-by: Vicente Bergas <vicencb@gmail.com> > >> --- > >> 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 tend to side with Marc's more drastic approach, especially as this one > > should also nicely unbind the encoders used. Are you ok with us going > > with Marc's patch or do you have concerns? > > The patch i posted comes from Jeffy, as is, no modifications. > So, if he has no concerns about it, then it is also fine for me. > > > > > Providing a Tested-by tag would also be great ;-) > > OK, i'll reply to his patch with a Tested-by tag, but i was only > aware of this issue affecting hdmi on power-off, so, the only testing > performed was checking only this. I have done no kexec-related test. > > Only one issue related to this: Marc, how can i reply to your patch > if i was not a recipient? You can also just post it here. Together with Sandy I'm carrying the drm-maintainer hat, so I'm probably the one that applies either one of the patches and can pick up a tag from here as well :-D Heiko
On Tue, Aug 7, 2018 at 6:07 PM, Heiko Stuebner <heiko@sntech.de> wrote: > Am Dienstag, 7. August 2018, 18:05:13 CEST schrieb Vicente Bergas: >> Hi Heiko, Jeffy, Marc, >> >> On Tue, Aug 7, 2018 at 2:44 PM, Heiko Stuebner <heiko@sntech.de> wrote: >> > Hi Vicente, >> > >> > Am Sonntag, 5. August 2018, 16:09:11 CEST schrieb Vicente Bergas: >> >> 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@rock-chips.com> >> >> Suggested-by: Robin Murphy <robin.murphy@arm.com> >> >> Signed-off-by: Vicente Bergas <vicencb@gmail.com> >> >> --- >> >> 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 tend to side with Marc's more drastic approach, especially as this one >> > should also nicely unbind the encoders used. Are you ok with us going >> > with Marc's patch or do you have concerns? >> >> The patch i posted comes from Jeffy, as is, no modifications. >> So, if he has no concerns about it, then it is also fine for me. >> >> > >> > Providing a Tested-by tag would also be great ;-) >> >> OK, i'll reply to his patch with a Tested-by tag, but i was only >> aware of this issue affecting hdmi on power-off, so, the only testing >> performed was checking only this. I have done no kexec-related test. >> >> Only one issue related to this: Marc, how can i reply to your patch >> if i was not a recipient? > > You can also just post it here. Together with Sandy I'm carrying the > drm-maintainer hat, so I'm probably the one that applies either one > of the patches and can pick up a tag from here as well :-D > > > Heiko OK, perfect, so, for this patch: https://www.spinics.net/lists/arm-kernel/msg670229.html here is my Tested-by: Vicente Bergas <vicencb@gmail.com> tag. As said, i only tested that on shutdown, the hdmi output is also shut down instead of showing random noise. Regards, Vicente.
On Tue, 7 Aug 2018 18:20:15 +0200 Vicente Bergas <vicencb@gmail.com> wrote: > On Tue, Aug 7, 2018 at 6:07 PM, Heiko Stuebner <heiko@sntech.de> wrote: > > Am Dienstag, 7. August 2018, 18:05:13 CEST schrieb Vicente Bergas: > >> Hi Heiko, Jeffy, Marc, > >> > >> On Tue, Aug 7, 2018 at 2:44 PM, Heiko Stuebner <heiko@sntech.de> wrote: > >> > Hi Vicente, > >> > > >> > Am Sonntag, 5. August 2018, 16:09:11 CEST schrieb Vicente Bergas: > >> >> 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@rock-chips.com> > >> >> Suggested-by: Robin Murphy <robin.murphy@arm.com> > >> >> Signed-off-by: Vicente Bergas <vicencb@gmail.com> > >> >> --- > >> >> 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 tend to side with Marc's more drastic approach, especially as this one > >> > should also nicely unbind the encoders used. Are you ok with us going > >> > with Marc's patch or do you have concerns? > >> > >> The patch i posted comes from Jeffy, as is, no modifications. > >> So, if he has no concerns about it, then it is also fine for me. > >> > >> > > >> > Providing a Tested-by tag would also be great ;-) > >> > >> OK, i'll reply to his patch with a Tested-by tag, but i was only > >> aware of this issue affecting hdmi on power-off, so, the only testing > >> performed was checking only this. I have done no kexec-related test. > >> > >> Only one issue related to this: Marc, how can i reply to your patch > >> if i was not a recipient? > > > > You can also just post it here. Together with Sandy I'm carrying the > > drm-maintainer hat, so I'm probably the one that applies either one > > of the patches and can pick up a tag from here as well :-D > > > > > > Heiko > > OK, perfect, so, for this patch: > https://www.spinics.net/lists/arm-kernel/msg670229.html > here is my > Tested-by: Vicente Bergas <vicencb@gmail.com> > tag. > As said, i only tested that on shutdown, the hdmi output > is also shut down instead of showing random noise. Any update on this patch? I was hoping to see it in 4.18, but so far nothing has happened. Thanks, M.
Am Sonntag, 9. September 2018, 15:43:00 CEST schrieb Marc Zyngier: > On Tue, 7 Aug 2018 18:20:15 +0200 > Vicente Bergas <vicencb@gmail.com> wrote: > > > On Tue, Aug 7, 2018 at 6:07 PM, Heiko Stuebner <heiko@sntech.de> wrote: > > > Am Dienstag, 7. August 2018, 18:05:13 CEST schrieb Vicente Bergas: > > >> Hi Heiko, Jeffy, Marc, > > >> > > >> On Tue, Aug 7, 2018 at 2:44 PM, Heiko Stuebner <heiko@sntech.de> wrote: > > >> > Hi Vicente, > > >> > > > >> > Am Sonntag, 5. August 2018, 16:09:11 CEST schrieb Vicente Bergas: > > >> >> 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@rock-chips.com> > > >> >> Suggested-by: Robin Murphy <robin.murphy@arm.com> > > >> >> Signed-off-by: Vicente Bergas <vicencb@gmail.com> > > >> >> --- > > >> >> 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 tend to side with Marc's more drastic approach, especially as this one > > >> > should also nicely unbind the encoders used. Are you ok with us going > > >> > with Marc's patch or do you have concerns? > > >> > > >> The patch i posted comes from Jeffy, as is, no modifications. > > >> So, if he has no concerns about it, then it is also fine for me. > > >> > > >> > > > >> > Providing a Tested-by tag would also be great ;-) > > >> > > >> OK, i'll reply to his patch with a Tested-by tag, but i was only > > >> aware of this issue affecting hdmi on power-off, so, the only testing > > >> performed was checking only this. I have done no kexec-related test. > > >> > > >> Only one issue related to this: Marc, how can i reply to your patch > > >> if i was not a recipient? > > > > > > You can also just post it here. Together with Sandy I'm carrying the > > > drm-maintainer hat, so I'm probably the one that applies either one > > > of the patches and can pick up a tag from here as well :-D > > > > > > > > > Heiko > > > > OK, perfect, so, for this patch: > > https://www.spinics.net/lists/arm-kernel/msg670229.html > > here is my > > Tested-by: Vicente Bergas <vicencb@gmail.com> > > tag. > > As said, i only tested that on shutdown, the hdmi output > > is also shut down instead of showing random noise. > > Any update on this patch? I was hoping to see it in 4.18, but so far > nothing has happened. it somehow slipped through my grasp, but I've applied it now. With a stable tag added so it should also trickle down to some stable kernels hopefully. Sorry for the holdup Heiko
On 10/09/18 10:08, Heiko Stuebner wrote: > Am Sonntag, 9. September 2018, 15:43:00 CEST schrieb Marc Zyngier: >> On Tue, 7 Aug 2018 18:20:15 +0200 >> Vicente Bergas <vicencb@gmail.com> wrote: >> >>> On Tue, Aug 7, 2018 at 6:07 PM, Heiko Stuebner <heiko@sntech.de> wrote: >>>> Am Dienstag, 7. August 2018, 18:05:13 CEST schrieb Vicente Bergas: >>>>> Hi Heiko, Jeffy, Marc, >>>>> >>>>> On Tue, Aug 7, 2018 at 2:44 PM, Heiko Stuebner <heiko@sntech.de> wrote: >>>>>> Hi Vicente, >>>>>> >>>>>> Am Sonntag, 5. August 2018, 16:09:11 CEST schrieb Vicente Bergas: >>>>>>> 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@rock-chips.com> >>>>>>> Suggested-by: Robin Murphy <robin.murphy@arm.com> >>>>>>> Signed-off-by: Vicente Bergas <vicencb@gmail.com> >>>>>>> --- >>>>>>> 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 tend to side with Marc's more drastic approach, especially as this one >>>>>> should also nicely unbind the encoders used. Are you ok with us going >>>>>> with Marc's patch or do you have concerns? >>>>> >>>>> The patch i posted comes from Jeffy, as is, no modifications. >>>>> So, if he has no concerns about it, then it is also fine for me. >>>>> >>>>>> >>>>>> Providing a Tested-by tag would also be great ;-) >>>>> >>>>> OK, i'll reply to his patch with a Tested-by tag, but i was only >>>>> aware of this issue affecting hdmi on power-off, so, the only testing >>>>> performed was checking only this. I have done no kexec-related test. >>>>> >>>>> Only one issue related to this: Marc, how can i reply to your patch >>>>> if i was not a recipient? >>>> >>>> You can also just post it here. Together with Sandy I'm carrying the >>>> drm-maintainer hat, so I'm probably the one that applies either one >>>> of the patches and can pick up a tag from here as well :-D >>>> >>>> >>>> Heiko >>> >>> OK, perfect, so, for this patch: >>> https://www.spinics.net/lists/arm-kernel/msg670229.html >>> here is my >>> Tested-by: Vicente Bergas <vicencb@gmail.com> >>> tag. >>> As said, i only tested that on shutdown, the hdmi output >>> is also shut down instead of showing random noise. >> >> Any update on this patch? I was hoping to see it in 4.18, but so far >> nothing has happened. > > it somehow slipped through my grasp, but I've applied it now. > With a stable tag added so it should also trickle down to some > stable kernels hopefully. > > > Sorry for the holdup No worries, and thanks for the stable tag! :-) M.
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); +} + static const struct of_device_id rockchip_drm_dt_ids[] = { { .compatible = "rockchip,display-subsystem", }, { /* sentinel */ }, @@ -451,6 +459,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_shutdown, .driver = { .name = "rockchip-drm", .of_match_table = rockchip_drm_dt_ids,