* [regression] HDMI breakage just before poweroff @ 2018-05-02 19:36 Vicente Bergas [not found] ` <CAAMcf8D86ssM+YeFAXaYDm9QwwAQLdaOgWyC2F8yQ-_-UNyY+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-05-02 19:36 UTC (permalink / raw) Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, JeffyChen, Klaus Goger, Jakob Unterwurzacher, Heiko Stuebner Hello, on rk3399-sapphire there is a missing feature: the poweroff command is unable to poweroff the system and the display stays on. When the system is halted, power needs to be removed manually. With Linux kernel 4.16, when reaching the halted state, the HDMI console shows the last messages before halting. With 4.17.0-rc3, when reaching the halted state, the HDMI console shows colorful static noise. Regards, Vicente. ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAMcf8D86ssM+YeFAXaYDm9QwwAQLdaOgWyC2F8yQ-_-UNyY+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <CAAMcf8D86ssM+YeFAXaYDm9QwwAQLdaOgWyC2F8yQ-_-UNyY+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-05-03 3:51 ` JeffyChen [not found] ` <5AEA873A.7080701-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: JeffyChen @ 2018-05-03 3:51 UTC (permalink / raw) To: Vicente Bergas Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Klaus Goger, Jakob Unterwurzacher, Heiko Stuebner Hi Vincente, Thanks for your mail. On 05/03/2018 03:36 AM, Vicente Bergas wrote: > Hello, > on rk3399-sapphire there is a missing feature: the poweroff command is > unable to poweroff the system and the display stays on. > When the system is halted, power needs to be removed manually. according to the dtsi, the rk3399-sapphire is using rk808 pimc right? i think the power off flow would be: 1/ rk808 set the pm_power_off callback: drivers/mfd/rk808.c switch (rk808->variant) { case RK805_ID: ... pm_pwroff_fn = rk805_device_shutdown; ... pm_off = of_property_read_bool(np, "rockchip,system-power-controller"); if (pm_off && !pm_power_off) { rk808_i2c_client = client; pm_power_off = pm_pwroff_fn; } 2/ the poweroff command would call kernel_power_off: kernel/reboot.c: void kernel_power_off(void) { ... machine_power_off(); } 3/ arm64's machine_power_off() would call pm_power_off to let rk808 cutoff the power. > > With Linux kernel 4.16, when reaching the halted state, the HDMI console > shows the last messages before halting. if it ends up halted, maybe something wrong during setup the pm_power_off? kernel/reboot.c: /* Instead of trying to make the power_off code look like * halt when pm_power_off is not set do it the easy way. */ if ((cmd == LINUX_REBOOT_CMD_POWER_OFF) && !pm_power_off) cmd = LINUX_REBOOT_CMD_HALT; > > 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? > Regards, > Vicente. > > > ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <5AEA873A.7080701-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <5AEA873A.7080701-TNX95d0MmH7DzftRWevZcw@public.gmane.org> @ 2018-05-03 12:14 ` Robin Murphy [not found] ` <0284fa4f-abd6-26f6-31e2-1a6d24777733-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Robin Murphy @ 2018-05-03 12:14 UTC (permalink / raw) To: JeffyChen, Vicente Bergas Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Klaus Goger, Jakob Unterwurzacher, Heiko Stuebner On 03/05/18 04:51, JeffyChen wrote: > Hi Vincente, > > Thanks for your mail. > > On 05/03/2018 03:36 AM, Vicente Bergas wrote: >> Hello, >> on rk3399-sapphire there is a missing feature: the poweroff command is >> unable to poweroff the system and the display stays on. >> When the system is halted, power needs to be removed manually. > according to the dtsi, the rk3399-sapphire is using rk808 pimc right? > > i think the power off flow would be: > 1/ rk808 set the pm_power_off callback: > drivers/mfd/rk808.c > switch (rk808->variant) { > case RK805_ID: > ... > pm_pwroff_fn = rk805_device_shutdown; > ... > > pm_off = of_property_read_bool(np, > "rockchip,system-power-controller"); > if (pm_off && !pm_power_off) { > rk808_i2c_client = client; > pm_power_off = pm_pwroff_fn; > } > > 2/ the poweroff command would call kernel_power_off: > kernel/reboot.c: > void kernel_power_off(void) > { > ... > machine_power_off(); > } > > 3/ arm64's machine_power_off() would call pm_power_off to let rk808 > cutoff the power. Doesn't PSCI's pm_power_off take precedence, though? This sounds similar to what my RK3328 does - I looked into that briefly a while ago, and from what I remember it appeared that the firmware SYSTEM_OFF handler tries to turn off the PMIC by driving the SLEEP pin, but the kernel driver never actually configures it into the correct mode for that to work. Thus the box just ends up spinning in the firmware until you pull the plug manually. >> With Linux kernel 4.16, when reaching the halted state, the HDMI console >> shows the last messages before halting. > if it ends up halted, maybe something wrong during setup the pm_power_off? > kernel/reboot.c: > /* Instead of trying to make the power_off code look like > * halt when pm_power_off is not set do it the easy way. > */ > if ((cmd == LINUX_REBOOT_CMD_POWER_OFF) && !pm_power_off) > cmd = LINUX_REBOOT_CMD_HALT; > >> >> 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. Robin. _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <0284fa4f-abd6-26f6-31e2-1a6d24777733-5wv7dgnIgG8@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <0284fa4f-abd6-26f6-31e2-1a6d24777733-5wv7dgnIgG8@public.gmane.org> @ 2018-05-03 15:10 ` JeffyChen [not found] ` <5AEB2670.9090305-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 2018-05-03 21:15 ` Vicente Bergas 2018-06-11 22:04 ` Vicente Bergas 2 siblings, 1 reply; 30+ messages in thread From: JeffyChen @ 2018-05-03 15:10 UTC (permalink / raw) To: Robin Murphy, Vicente Bergas Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Klaus Goger, Jakob Unterwurzacher, Heiko Stuebner Hi Robin, On 05/03/2018 08:14 PM, Robin Murphy wrote: > On 03/05/18 04:51, JeffyChen wrote: >> Hi Vincente, >> >> Thanks for your mail. >> >> On 05/03/2018 03:36 AM, Vicente Bergas wrote: >>> Hello, >>> on rk3399-sapphire there is a missing feature: the poweroff command is >>> unable to poweroff the system and the display stays on. >>> When the system is halted, power needs to be removed manually. >> according to the dtsi, the rk3399-sapphire is using rk808 pimc right? >> >> i think the power off flow would be: >> 1/ rk808 set the pm_power_off callback: >> drivers/mfd/rk808.c >> switch (rk808->variant) { >> case RK805_ID: >> ... >> pm_pwroff_fn = rk805_device_shutdown; >> ... >> >> pm_off = of_property_read_bool(np, >> "rockchip,system-power-controller"); >> if (pm_off && !pm_power_off) { >> rk808_i2c_client = client; >> pm_power_off = pm_pwroff_fn; >> } >> >> 2/ the poweroff command would call kernel_power_off: >> kernel/reboot.c: >> void kernel_power_off(void) >> { >> ... >> machine_power_off(); >> } >> >> 3/ arm64's machine_power_off() would call pm_power_off to let rk808 >> cutoff the power. > > Doesn't PSCI's pm_power_off take precedence, though? This sounds similar > to what my RK3328 does - I looked into that briefly a while ago, and > from what I remember it appeared that the firmware SYSTEM_OFF handler > tries to turn off the PMIC by driving the SLEEP pin, but the kernel > driver never actually configures it into the correct mode for that to > work. Thus the box just ends up spinning in the firmware until you pull > the plug manually. hmmm, right, it should be psci's power off. and i checked the BSP kernel, the rk808 driver switched to use pm_power_off_prepare and syscore_ops->shutdown there > >>> With Linux kernel 4.16, when reaching the halted state, the HDMI console >>> shows the last messages before halting. >> if it ends up halted, maybe something wrong during setup the >> pm_power_off? >> kernel/reboot.c: >> /* Instead of trying to make the power_off code look like >> * halt when pm_power_off is not set do it the easy way. >> */ >> if ((cmd == LINUX_REBOOT_CMD_POWER_OFF) && !pm_power_off) >> cmd = LINUX_REBOOT_CMD_HALT; >> >>> >>> 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. > > Robin. > > > ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <5AEB2670.9090305-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <5AEB2670.9090305-TNX95d0MmH7DzftRWevZcw@public.gmane.org> @ 2018-05-03 20:33 ` Vicente Bergas 0 siblings, 0 replies; 30+ messages in thread From: Vicente Bergas @ 2018-05-03 20:33 UTC (permalink / raw) To: JeffyChen Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Klaus Goger, Robin Murphy, Jakob Unterwurzacher, Heiko Stuebner Hi, On Thu, May 3, 2018 at 5:10 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote: > Hi Robin, > > > On 05/03/2018 08:14 PM, Robin Murphy wrote: >> >> On 03/05/18 04:51, JeffyChen wrote: >>> >>> Hi Vincente, >>> >>> Thanks for your mail. >>> >>> On 05/03/2018 03:36 AM, Vicente Bergas wrote: >>>> >>>> Hello, >>>> on rk3399-sapphire there is a missing feature: the poweroff command is >>>> unable to poweroff the system and the display stays on. >>>> When the system is halted, power needs to be removed manually. >>> >>> according to the dtsi, the rk3399-sapphire is using rk808 pimc right? >>> >>> i think the power off flow would be: >>> 1/ rk808 set the pm_power_off callback: >>> drivers/mfd/rk808.c >>> switch (rk808->variant) { >>> case RK805_ID: >>> ... >>> pm_pwroff_fn = rk805_device_shutdown; >>> ... >>> >>> pm_off = of_property_read_bool(np, >>> "rockchip,system-power-controller"); >>> if (pm_off && !pm_power_off) { >>> rk808_i2c_client = client; >>> pm_power_off = pm_pwroff_fn; >>> } >>> >>> 2/ the poweroff command would call kernel_power_off: >>> kernel/reboot.c: >>> void kernel_power_off(void) >>> { >>> ... >>> machine_power_off(); >>> } >>> >>> 3/ arm64's machine_power_off() would call pm_power_off to let rk808 >>> cutoff the power. >> >> >> Doesn't PSCI's pm_power_off take precedence, though? This sounds similar >> to what my RK3328 does - I looked into that briefly a while ago, and >> from what I remember it appeared that the firmware SYSTEM_OFF handler >> tries to turn off the PMIC by driving the SLEEP pin, but the kernel >> driver never actually configures it into the correct mode for that to >> work. Thus the box just ends up spinning in the firmware until you pull >> the plug manually. > > > hmmm, right, it should be psci's power off. > > and i checked the BSP kernel, the rk808 driver switched to use > pm_power_off_prepare and syscore_ops->shutdown there > PSCI? that is provided by the ATF (ARM Trusted Firmware), right? The Sapphire board this issue appears on has: 1.- As first stage loader the SPL (Secondary Program Loader) from rkbin [1]. 2.- As second stage loader the the ATF from rkbin [1]. 3.- As third stage loader mainline u-boot. I would strongly like to use open source and preferably mainline SPL and ATF, but had no success when trying a few months back. Regards, Vicente. [1] https://github.com/rockchip-linux/rkbin > >> >>>> With Linux kernel 4.16, when reaching the halted state, the HDMI console >>>> shows the last messages before halting. >>> >>> if it ends up halted, maybe something wrong during setup the >>> pm_power_off? >>> kernel/reboot.c: >>> /* Instead of trying to make the power_off code look like >>> * halt when pm_power_off is not set do it the easy way. >>> */ >>> if ((cmd == LINUX_REBOOT_CMD_POWER_OFF) && !pm_power_off) >>> cmd = LINUX_REBOOT_CMD_HALT; >>> >>>> >>>> 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. >> >> Robin. >> >> >> > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [regression] HDMI breakage just before poweroff [not found] ` <0284fa4f-abd6-26f6-31e2-1a6d24777733-5wv7dgnIgG8@public.gmane.org> 2018-05-03 15:10 ` JeffyChen @ 2018-05-03 21:15 ` Vicente Bergas 2018-06-11 22:04 ` Vicente Bergas 2 siblings, 0 replies; 30+ messages in thread From: Vicente Bergas @ 2018-05-03 21:15 UTC (permalink / raw) To: Robin Murphy Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, JeffyChen, Klaus Goger, Jakob Unterwurzacher, Heiko Stuebner 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: >> >> Hi Vincente, >> >> Thanks for your mail. >> >> On 05/03/2018 03:36 AM, Vicente Bergas wrote: >>> >>> Hello, >>> on rk3399-sapphire there is a missing feature: the poweroff command is >>> unable to poweroff the system and the display stays on. >>> When the system is halted, power needs to be removed manually. >> >> according to the dtsi, the rk3399-sapphire is using rk808 pimc right? >> >> i think the power off flow would be: >> 1/ rk808 set the pm_power_off callback: >> drivers/mfd/rk808.c >> switch (rk808->variant) { >> case RK805_ID: >> ... >> pm_pwroff_fn = rk805_device_shutdown; >> ... >> >> pm_off = of_property_read_bool(np, >> "rockchip,system-power-controller"); >> if (pm_off && !pm_power_off) { >> rk808_i2c_client = client; >> pm_power_off = pm_pwroff_fn; >> } >> >> 2/ the poweroff command would call kernel_power_off: >> kernel/reboot.c: >> void kernel_power_off(void) >> { >> ... >> machine_power_off(); >> } >> >> 3/ arm64's machine_power_off() would call pm_power_off to let rk808 cutoff >> the power. > > > Doesn't PSCI's pm_power_off take precedence, though? This sounds similar to > what my RK3328 does - I looked into that briefly a while ago, and from what > I remember it appeared that the firmware SYSTEM_OFF handler tries to turn > off the PMIC by driving the SLEEP pin, but the kernel driver never actually > configures it into the correct mode for that to work. Thus the box just ends > up spinning in the firmware until you pull the plug manually. > >>> With Linux kernel 4.16, when reaching the halted state, the HDMI console >>> shows the last messages before halting. >> >> if it ends up halted, maybe something wrong during setup the pm_power_off? >> kernel/reboot.c: >> /* Instead of trying to make the power_off code look like >> * halt when pm_power_off is not set do it the easy way. >> */ >> if ((cmd == LINUX_REBOOT_CMD_POWER_OFF) && !pm_power_off) >> cmd = LINUX_REBOOT_CMD_HALT; >> >>> >>> 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. > > Robin. I can confirm that --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1269,7 +1264,6 @@ MODULE_DEVICE_TABLE(of, rk_iommu_dt_ids); static struct platform_driver rk_iommu_driver = { .probe = rk_iommu_probe, - .shutdown = rk_iommu_shutdown, .driver = { .name = "rk_iommu", .of_match_table = rk_iommu_dt_ids, fixes the HDMI issue. Regards, Vicente. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [regression] HDMI breakage just before poweroff [not found] ` <0284fa4f-abd6-26f6-31e2-1a6d24777733-5wv7dgnIgG8@public.gmane.org> 2018-05-03 15:10 ` JeffyChen 2018-05-03 21:15 ` Vicente Bergas @ 2018-06-11 22:04 ` Vicente Bergas [not found] ` <CAAMcf8BJ1skbmR7AazyTPkkdyMoyuA0ihj=Gbnhb9nKWf3FSGg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2 siblings, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-06-11 22:04 UTC (permalink / raw) To: Robin Murphy Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, JeffyChen, Klaus Goger, Jakob Unterwurzacher, Heiko Stuebner 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? Regards, Vicente. ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAMcf8BJ1skbmR7AazyTPkkdyMoyuA0ihj=Gbnhb9nKWf3FSGg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <CAAMcf8BJ1skbmR7AazyTPkkdyMoyuA0ihj=Gbnhb9nKWf3FSGg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-06-12 3:52 ` JeffyChen [not found] ` <5B1F4375.7000000-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: JeffyChen @ 2018-06-12 3:52 UTC (permalink / raw) To: Vicente Bergas, Robin Murphy Cc: Heiko Stuebner, Marc Zyngier, 黄家钗, Tomasz Figa, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Klaus Goger, Sean Paul, Jakob Unterwurzacher 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, > Regards, > Vicente. > > > ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <5B1F4375.7000000-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <5B1F4375.7000000-TNX95d0MmH7DzftRWevZcw@public.gmane.org> @ 2018-06-12 8:54 ` Marc Zyngier [not found] ` <77e4c6e1-015f-5fac-66b6-c942bb2dc9d8-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Marc Zyngier @ 2018-06-12 8:54 UTC (permalink / raw) To: JeffyChen, Vicente Bergas, Robin Murphy Cc: Heiko Stuebner, 黄家钗, Tomasz Figa, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sean Paul, Klaus Goger, Jakob Unterwurzacher 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? Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <77e4c6e1-015f-5fac-66b6-c942bb2dc9d8-5wv7dgnIgG8@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <77e4c6e1-015f-5fac-66b6-c942bb2dc9d8-5wv7dgnIgG8@public.gmane.org> @ 2018-06-12 10:27 ` JeffyChen [not found] ` <5B1F9FF7.5070203-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: JeffyChen @ 2018-06-12 10:27 UTC (permalink / raw) To: Marc Zyngier, Vicente Bergas, Robin Murphy Cc: Heiko Stuebner, 黄家钗, Tomasz Figa, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sean Paul, Klaus Goger, Jakob Unterwurzacher 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 > > Thanks, > > M. > ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <5B1F9FF7.5070203-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <5B1F9FF7.5070203-TNX95d0MmH7DzftRWevZcw@public.gmane.org> @ 2018-06-12 10:30 ` Tomasz Figa [not found] ` <CAAFQd5D__ArEYSVoWL5A40X6vbYxJ-vOYpsXxiLujC-XK9u2+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Tomasz Figa @ 2018-06-12 10:30 UTC (permalink / raw) To: Jeffy Cc: Heiko Stübner, marc.zyngier-5wv7dgnIgG8, Sandy Huang, vicencb-Re5JQEeQqe8AvxtiuMwx3w, open list:ARM/Rockchip SoC..., klaus.goger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5, Sean Paul, Robin Murphy, jakob.unterwurzacher-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAFQd5D__ArEYSVoWL5A40X6vbYxJ-vOYpsXxiLujC-XK9u2+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <CAAFQd5D__ArEYSVoWL5A40X6vbYxJ-vOYpsXxiLujC-XK9u2+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-06-12 11:02 ` Marc Zyngier [not found] ` <86efhc8dyg.wl-marc.zyngier-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Marc Zyngier @ 2018-06-12 11:02 UTC (permalink / raw) To: Tomasz Figa Cc: Heiko Stübner, Jeffy, Sandy Huang, vicencb-Re5JQEeQqe8AvxtiuMwx3w, open list:ARM/Rockchip SoC..., klaus.goger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5, Sean Paul, Robin Murphy, jakob.unterwurzacher-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <86efhc8dyg.wl-marc.zyngier-5wv7dgnIgG8@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <86efhc8dyg.wl-marc.zyngier-5wv7dgnIgG8@public.gmane.org> @ 2018-06-13 7:15 ` Vicente Bergas [not found] ` <CAAMcf8AmuMStdSUHGEQhii7BUtTRHOSyLhop43rf_nX0GROnnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-06-13 7:15 UTC (permalink / raw) To: Marc Zyngier Cc: Heiko Stübner, Jeffy, Sandy Huang, Tomasz Figa, open list:ARM/Rockchip SoC..., Klaus Goger, Sean Paul, Robin Murphy, Jakob Unterwurzacher On Tue, Jun 12, 2018 at 1:02 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote: > 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. Hi, just tested it. There was an issue because of 'incompatible pointer type', see the proper patch at the bottom. It seems to do what it is expected to do, that is, it shuts down the display. I am not sure that this is what is wanted. When the system is in the halted state, should not it show the last messages? For this, the display needs to be operational. Regards, Vicente. --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -451,6 +451,7 @@ static struct platform_driver rockchip_drm_platform_driver = { .probe = rockchip_drm_platform_probe, .remove = rockchip_drm_platform_remove, + .shutdown = (void (*)(struct platform_device *))rockchip_drm_platform_remove, .driver = { .name = "rockchip-drm", .of_match_table = rockchip_drm_dt_ids, ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAMcf8AmuMStdSUHGEQhii7BUtTRHOSyLhop43rf_nX0GROnnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <CAAMcf8AmuMStdSUHGEQhii7BUtTRHOSyLhop43rf_nX0GROnnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-06-13 10:26 ` Marc Zyngier [not found] ` <cc147c40-dbad-b714-3fe6-858854a56004-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Marc Zyngier @ 2018-06-13 10:26 UTC (permalink / raw) To: Vicente Bergas Cc: Heiko Stübner, Jeffy, Sandy Huang, Tomasz Figa, open list:ARM/Rockchip SoC..., Klaus Goger, Sean Paul, Robin Murphy, Jakob Unterwurzacher On 13/06/18 08:15, Vicente Bergas wrote: > On Tue, Jun 12, 2018 at 1:02 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote: >> 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. > > Hi, > just tested it. > There was an issue because of 'incompatible pointer type', > see the proper patch at the bottom. > > It seems to do what it is expected to do, that is, it shuts down the display. > I am not sure that this is what is wanted. > When the system is in the halted state, should not it show the last messages? > For this, the display needs to be operational. Well, we can't have everything, unfortunately. We definitely need to stop the IOMMU in order to kexec safely, which is an important use-case. A possibility would be to switch to a directly mapped buffer on shutdown, but I'm not sure how we can enforce that at this stage. > > Regards, > Vicente. > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -451,6 +451,7 @@ > static struct platform_driver rockchip_drm_platform_driver = { > .probe = rockchip_drm_platform_probe, > .remove = rockchip_drm_platform_remove, > + .shutdown = (void (*)(struct platform_device > *))rockchip_drm_platform_remove, No, please... :-( Provide a wrapper that returns void instead. > .driver = { > .name = "rockchip-drm", > .of_match_table = rockchip_drm_dt_ids, > Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <cc147c40-dbad-b714-3fe6-858854a56004-5wv7dgnIgG8@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <cc147c40-dbad-b714-3fe6-858854a56004-5wv7dgnIgG8@public.gmane.org> @ 2018-06-13 10:46 ` JeffyChen [not found] ` <5B20F5F0.9090101-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: JeffyChen @ 2018-06-13 10:46 UTC (permalink / raw) To: Marc Zyngier, Vicente Bergas Cc: Heiko Stübner, Sandy Huang, Tomasz Figa, open list:ARM/Rockchip SoC..., Klaus Goger, Sean Paul, Robin Murphy, Jakob Unterwurzacher Hi Marc & Vicente, On 06/13/2018 06:26 PM, Marc Zyngier wrote: >> > >> >--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >> >+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >> >@@ -451,6 +451,7 @@ >> > static struct platform_driver rockchip_drm_platform_driver = { >> > .probe = rockchip_drm_platform_probe, >> > .remove = rockchip_drm_platform_remove, >> >+ .shutdown = (void (*)(struct platform_device >> >*))rockchip_drm_platform_remove, > No, please...:-( Provide a wrapper that returns void instead. hmmm, this is just a test only hack, i am thinking maybe drm_atomic_helper_shutdown is enough for shutdown() ? something like: +++ 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); +} + > ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <5B20F5F0.9090101-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <5B20F5F0.9090101-TNX95d0MmH7DzftRWevZcw@public.gmane.org> @ 2018-06-15 16:39 ` Vicente Bergas [not found] ` <CAAMcf8D1-7YvOeOMd+rJOWX7XNQwKeAYjfmyAHOR8TC6vc=G1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-08-05 14:09 ` [PATCH] drm/rockchip: shutdown drm subsystem on shutdown Vicente Bergas 1 sibling, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-06-15 16:39 UTC (permalink / raw) To: JeffyChen Cc: Heiko Stübner, Marc Zyngier, Sandy Huang, Tomasz Figa, open list:ARM/Rockchip SoC..., Klaus Goger, Sean Paul, Robin Murphy, Jakob Unterwurzacher On Wed, Jun 13, 2018 at 12:46 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote: > Hi Marc & Vicente, > > On 06/13/2018 06:26 PM, Marc Zyngier wrote: >>> >>> > >>> >--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>> >+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>> >@@ -451,6 +451,7 @@ >>> > static struct platform_driver rockchip_drm_platform_driver = { >>> > .probe = rockchip_drm_platform_probe, >>> > .remove = rockchip_drm_platform_remove, >>> >+ .shutdown = (void (*)(struct platform_device >>> >*))rockchip_drm_platform_remove, >> >> No, please...:-( Provide a wrapper that returns void instead. > > > hmmm, this is just a test only hack, i am thinking maybe > drm_atomic_helper_shutdown is enough for shutdown() ? > > > something like: > +++ 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); > +} > + > Hi, I can confirm that rockchip_drm_platform_shutdown works as fine as rockchip_drm_platform_remove. Just a note: The first time I noticed this error was because it appeared on display, now that the display is off, are we sure the error is fixed? Also, could there be other peripherals that need to be powered off before the iommu? For curiosity: why is the iommu disabled before kexec instead of resetting it after kexec? Regards, Vicente. ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAMcf8D1-7YvOeOMd+rJOWX7XNQwKeAYjfmyAHOR8TC6vc=G1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <CAAMcf8D1-7YvOeOMd+rJOWX7XNQwKeAYjfmyAHOR8TC6vc=G1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-06-15 16:51 ` Marc Zyngier [not found] ` <7ce78e65-51b6-9204-5bb4-e515e36576d4-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Marc Zyngier @ 2018-06-15 16:51 UTC (permalink / raw) To: Vicente Bergas, JeffyChen Cc: Heiko Stübner, Sandy Huang, Tomasz Figa, open list:ARM/Rockchip SoC..., Klaus Goger, Sean Paul, Robin Murphy, Jakob Unterwurzacher On 15/06/18 17:39, Vicente Bergas wrote: > On Wed, Jun 13, 2018 at 12:46 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote: >> Hi Marc & Vicente, >> >> On 06/13/2018 06:26 PM, Marc Zyngier wrote: >>>> >>>>> >>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>>>> @@ -451,6 +451,7 @@ >>>>> static struct platform_driver rockchip_drm_platform_driver = { >>>>> .probe = rockchip_drm_platform_probe, >>>>> .remove = rockchip_drm_platform_remove, >>>>> + .shutdown = (void (*)(struct platform_device >>>>> *))rockchip_drm_platform_remove, >>> >>> No, please...:-( Provide a wrapper that returns void instead. >> >> >> hmmm, this is just a test only hack, i am thinking maybe >> drm_atomic_helper_shutdown is enough for shutdown() ? >> >> >> something like: >> +++ 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); >> +} >> + >> > Hi, > I can confirm that rockchip_drm_platform_shutdown works as fine as > rockchip_drm_platform_remove. > Just a note: The first time I noticed this error was because it > appeared on display, now that the display is off, are we sure the > error is fixed? > Also, could there be other peripherals that need to be powered off > before the iommu? > For curiosity: why is the iommu disabled before kexec instead of > resetting it after kexec? Because by the time you're in the new kernel, you've corrupted the page tables, accessed peripheral address space, and set the system on fire. In general, having a DMA master active across something like kexec is a pretty fatal issue. And when your boot loader is a Linux kernel using kexec, you're really insisting that kexec works correctly. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <7ce78e65-51b6-9204-5bb4-e515e36576d4-5wv7dgnIgG8@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <7ce78e65-51b6-9204-5bb4-e515e36576d4-5wv7dgnIgG8@public.gmane.org> @ 2018-06-15 18:46 ` Vicente Bergas [not found] ` <CAAMcf8C3fS1SdZVKrsu9zRMn69BcAtFL44QvWMJ8zn4CySnfcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-06-15 18:46 UTC (permalink / raw) To: Marc Zyngier Cc: Heiko Stübner, JeffyChen, Sandy Huang, Tomasz Figa, open list:ARM/Rockchip SoC..., Klaus Goger, Sean Paul, Robin Murphy, Jakob Unterwurzacher On Fri, Jun 15, 2018 at 6:51 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote: > On 15/06/18 17:39, Vicente Bergas wrote: >> On Wed, Jun 13, 2018 at 12:46 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote: >>> Hi Marc & Vicente, >>> >>> On 06/13/2018 06:26 PM, Marc Zyngier wrote: >>>>> >>>>>> >>>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c >>>>>> @@ -451,6 +451,7 @@ >>>>>> static struct platform_driver rockchip_drm_platform_driver = { >>>>>> .probe = rockchip_drm_platform_probe, >>>>>> .remove = rockchip_drm_platform_remove, >>>>>> + .shutdown = (void (*)(struct platform_device >>>>>> *))rockchip_drm_platform_remove, >>>> >>>> No, please...:-( Provide a wrapper that returns void instead. >>> >>> >>> hmmm, this is just a test only hack, i am thinking maybe >>> drm_atomic_helper_shutdown is enough for shutdown() ? >>> >>> >>> something like: >>> +++ 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); >>> +} >>> + >>> >> Hi, >> I can confirm that rockchip_drm_platform_shutdown works as fine as >> rockchip_drm_platform_remove. >> Just a note: The first time I noticed this error was because it >> appeared on display, now that the display is off, are we sure the >> error is fixed? >> Also, could there be other peripherals that need to be powered off >> before the iommu? >> For curiosity: why is the iommu disabled before kexec instead of >> resetting it after kexec? > > Because by the time you're in the new kernel, you've corrupted the page > tables, accessed peripheral address space, and set the system on fire. > > In general, having a DMA master active across something like kexec is a > pretty fatal issue. And when your boot loader is a Linux kernel using > kexec, you're really insisting that kexec works correctly. Is it feasible to place the display memory at a virtual address that matches the physical address? ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAMcf8C3fS1SdZVKrsu9zRMn69BcAtFL44QvWMJ8zn4CySnfcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [regression] HDMI breakage just before poweroff [not found] ` <CAAMcf8C3fS1SdZVKrsu9zRMn69BcAtFL44QvWMJ8zn4CySnfcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-06-16 10:35 ` Marc Zyngier 0 siblings, 0 replies; 30+ messages in thread From: Marc Zyngier @ 2018-06-16 10:35 UTC (permalink / raw) To: Vicente Bergas Cc: Heiko Stübner, JeffyChen, Sandy Huang, Tomasz Figa, open list:ARM/Rockchip SoC..., Klaus Goger, Sean Paul, Robin Murphy, Jakob Unterwurzacher On Fri, 15 Jun 2018 19:46:50 +0100, Vicente Bergas <vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > On Fri, Jun 15, 2018 at 6:51 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote: > > On 15/06/18 17:39, Vicente Bergas wrote: > >> On Wed, Jun 13, 2018 at 12:46 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote: > >>> Hi Marc & Vicente, > >>> > >>> On 06/13/2018 06:26 PM, Marc Zyngier wrote: [...] > >> I can confirm that rockchip_drm_platform_shutdown works as fine as > >> rockchip_drm_platform_remove. > >> Just a note: The first time I noticed this error was because it > >> appeared on display, now that the display is off, are we sure the > >> error is fixed? > >> Also, could there be other peripherals that need to be powered off > >> before the iommu? > >> For curiosity: why is the iommu disabled before kexec instead of > >> resetting it after kexec? > > > > Because by the time you're in the new kernel, you've corrupted the page > > tables, accessed peripheral address space, and set the system on fire. > > > > In general, having a DMA master active across something like kexec is a > > pretty fatal issue. And when your boot loader is a Linux kernel using > > kexec, you're really insisting that kexec works correctly. > > Is it feasible to place the display memory at a virtual address that > matches the physical address? That depends on the addressing capability of the end-point, and on the kernel ability to allocate a large, contiguous range of physical memory. I'm not sure it is worth the hassle just to get the display working all the way through shutdown (knowing that the most likely event after that is a power off). M. -- Jazz is not dead, it just smell funny. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] drm/rockchip: shutdown drm subsystem on shutdown [not found] ` <5B20F5F0.9090101-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 2018-06-15 16:39 ` Vicente Bergas @ 2018-08-05 14:09 ` Vicente Bergas [not found] ` <20180805140911.19205-1-vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 1 sibling, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-08-05 14:09 UTC (permalink / raw) To: JeffyChen, Robin Murphy, Heiko Stuebner, Marc Zyngier, Tomasz Figa, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: 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-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); +} + 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, -- 2.18.0 ^ permalink raw reply related [flat|nested] 30+ messages in thread
[parent not found: <20180805140911.19205-1-vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown [not found] ` <20180805140911.19205-1-vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2018-08-05 16:50 ` Marc Zyngier [not found] ` <20180805175038.1d3a0c5e-Fmn/x+r+pSA9//JtdbceeD8Kkb2uy4ct@public.gmane.org> 2018-08-07 12:44 ` Heiko Stuebner 1 sibling, 1 reply; 30+ messages in thread From: Marc Zyngier @ 2018-08-05 16:50 UTC (permalink / raw) To: Vicente Bergas Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, JeffyChen, Robin Murphy, Heiko Stuebner, Tomasz Figa 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <20180805175038.1d3a0c5e-Fmn/x+r+pSA9//JtdbceeD8Kkb2uy4ct@public.gmane.org>]
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown [not found] ` <20180805175038.1d3a0c5e-Fmn/x+r+pSA9//JtdbceeD8Kkb2uy4ct@public.gmane.org> @ 2018-08-05 17:38 ` Vicente Bergas [not found] ` <CAAMcf8CyUri0LigtJWvahLK62ihPUYa_UvA+2q_EA8m+TL2cqw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-08-05 17:38 UTC (permalink / raw) To: Marc Zyngier Cc: open list:ARM/Rockchip SoC..., JeffyChen, Robin Murphy, Heiko Stuebner, Tomasz Figa On Sun, Aug 5, 2018 at 6:50 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote: > 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. 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAMcf8CyUri0LigtJWvahLK62ihPUYa_UvA+2q_EA8m+TL2cqw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown [not found] ` <CAAMcf8CyUri0LigtJWvahLK62ihPUYa_UvA+2q_EA8m+TL2cqw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-08-05 18:23 ` Marc Zyngier 0 siblings, 0 replies; 30+ messages in thread From: Marc Zyngier @ 2018-08-05 18:23 UTC (permalink / raw) To: Vicente Bergas Cc: open list:ARM/Rockchip SoC..., JeffyChen, Robin Murphy, Heiko Stuebner, Tomasz Figa On Sun, 05 Aug 2018 18:38:18 +0100, Vicente Bergas <vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > On Sun, Aug 5, 2018 at 6:50 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote: > > 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. > > 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. -- Jazz is not dead, it just smell funny. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown [not found] ` <20180805140911.19205-1-vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2018-08-05 16:50 ` Marc Zyngier @ 2018-08-07 12:44 ` Heiko Stuebner 2018-08-07 16:05 ` Vicente Bergas 1 sibling, 1 reply; 30+ messages in thread From: Heiko Stuebner @ 2018-08-07 12:44 UTC (permalink / raw) To: Vicente Bergas Cc: Marc Zyngier, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, JeffyChen, Robin Murphy, Tomasz Figa 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-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 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown 2018-08-07 12:44 ` Heiko Stuebner @ 2018-08-07 16:05 ` Vicente Bergas [not found] ` <CAAMcf8D8Vwgi-Amqt8ou9LbDrsRTePjCvwDXWo1JUTGTxLyn_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-08-07 16:05 UTC (permalink / raw) To: Heiko Stuebner Cc: Marc Zyngier, open list:ARM/Rockchip SoC..., JeffyChen, Robin Murphy, Tomasz Figa Hi Heiko, Jeffy, Marc, On Tue, Aug 7, 2018 at 2:44 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-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 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 > > ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAMcf8D8Vwgi-Amqt8ou9LbDrsRTePjCvwDXWo1JUTGTxLyn_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown [not found] ` <CAAMcf8D8Vwgi-Amqt8ou9LbDrsRTePjCvwDXWo1JUTGTxLyn_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-08-07 16:07 ` Heiko Stuebner 2018-08-07 16:20 ` Vicente Bergas 0 siblings, 1 reply; 30+ messages in thread From: Heiko Stuebner @ 2018-08-07 16:07 UTC (permalink / raw) To: Vicente Bergas Cc: Marc Zyngier, open list:ARM/Rockchip SoC..., JeffyChen, Robin Murphy, Tomasz Figa 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-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-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 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown 2018-08-07 16:07 ` Heiko Stuebner @ 2018-08-07 16:20 ` Vicente Bergas [not found] ` <CAAMcf8DJhBk8rG65SmQR4vwtj1zfm7jfKJJ4nAx6OsAeu52T2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Vicente Bergas @ 2018-08-07 16:20 UTC (permalink / raw) To: Heiko Stuebner Cc: Marc Zyngier, open list:ARM/Rockchip SoC..., JeffyChen, Robin Murphy, Tomasz Figa On Tue, Aug 7, 2018 at 6:07 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-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 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> tag. As said, i only tested that on shutdown, the hdmi output is also shut down instead of showing random noise. Regards, Vicente. ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <CAAMcf8DJhBk8rG65SmQR4vwtj1zfm7jfKJJ4nAx6OsAeu52T2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown [not found] ` <CAAMcf8DJhBk8rG65SmQR4vwtj1zfm7jfKJJ4nAx6OsAeu52T2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-09-09 13:43 ` Marc Zyngier [not found] ` <20180909144300.3908a11b-Fmn/x+r+pSA9//JtdbceeD8Kkb2uy4ct@public.gmane.org> 0 siblings, 1 reply; 30+ messages in thread From: Marc Zyngier @ 2018-09-09 13:43 UTC (permalink / raw) To: Vicente Bergas, Heiko Stuebner Cc: open list:ARM/Rockchip SoC..., JeffyChen, Robin Murphy, Tomasz Figa On Tue, 7 Aug 2018 18:20:15 +0200 Vicente Bergas <vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Tue, Aug 7, 2018 at 6:07 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-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 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > 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. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <20180909144300.3908a11b-Fmn/x+r+pSA9//JtdbceeD8Kkb2uy4ct@public.gmane.org>]
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown [not found] ` <20180909144300.3908a11b-Fmn/x+r+pSA9//JtdbceeD8Kkb2uy4ct@public.gmane.org> @ 2018-09-10 9:08 ` Heiko Stuebner 2018-09-10 9:57 ` Marc Zyngier 0 siblings, 1 reply; 30+ messages in thread From: Heiko Stuebner @ 2018-09-10 9:08 UTC (permalink / raw) To: Marc Zyngier Cc: open list:ARM/Rockchip SoC..., JeffyChen, Robin Murphy, Vicente Bergas, Tomasz Figa Am Sonntag, 9. September 2018, 15:43:00 CEST schrieb Marc Zyngier: > On Tue, 7 Aug 2018 18:20:15 +0200 > Vicente Bergas <vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > > On Tue, Aug 7, 2018 at 6:07 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-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 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] drm/rockchip: shutdown drm subsystem on shutdown 2018-09-10 9:08 ` Heiko Stuebner @ 2018-09-10 9:57 ` Marc Zyngier 0 siblings, 0 replies; 30+ messages in thread From: Marc Zyngier @ 2018-09-10 9:57 UTC (permalink / raw) To: Heiko Stuebner Cc: open list:ARM/Rockchip SoC..., JeffyChen, Robin Murphy, Vicente Bergas, Tomasz Figa 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> >>> On Tue, Aug 7, 2018 at 6:07 PM, Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> 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-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 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >>> 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. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2018-09-10 9:57 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-02 19:36 [regression] HDMI breakage just before poweroff Vicente Bergas [not found] ` <CAAMcf8D86ssM+YeFAXaYDm9QwwAQLdaOgWyC2F8yQ-_-UNyY+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-05-03 3:51 ` JeffyChen [not found] ` <5AEA873A.7080701-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 2018-05-03 12:14 ` Robin Murphy [not found] ` <0284fa4f-abd6-26f6-31e2-1a6d24777733-5wv7dgnIgG8@public.gmane.org> 2018-05-03 15:10 ` JeffyChen [not found] ` <5AEB2670.9090305-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 2018-05-03 20:33 ` Vicente Bergas 2018-05-03 21:15 ` Vicente Bergas 2018-06-11 22:04 ` Vicente Bergas [not found] ` <CAAMcf8BJ1skbmR7AazyTPkkdyMoyuA0ihj=Gbnhb9nKWf3FSGg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-06-12 3:52 ` JeffyChen [not found] ` <5B1F4375.7000000-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 2018-06-12 8:54 ` Marc Zyngier [not found] ` <77e4c6e1-015f-5fac-66b6-c942bb2dc9d8-5wv7dgnIgG8@public.gmane.org> 2018-06-12 10:27 ` JeffyChen [not found] ` <5B1F9FF7.5070203-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 2018-06-12 10:30 ` Tomasz Figa [not found] ` <CAAFQd5D__ArEYSVoWL5A40X6vbYxJ-vOYpsXxiLujC-XK9u2+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-06-12 11:02 ` Marc Zyngier [not found] ` <86efhc8dyg.wl-marc.zyngier-5wv7dgnIgG8@public.gmane.org> 2018-06-13 7:15 ` Vicente Bergas [not found] ` <CAAMcf8AmuMStdSUHGEQhii7BUtTRHOSyLhop43rf_nX0GROnnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-06-13 10:26 ` Marc Zyngier [not found] ` <cc147c40-dbad-b714-3fe6-858854a56004-5wv7dgnIgG8@public.gmane.org> 2018-06-13 10:46 ` JeffyChen [not found] ` <5B20F5F0.9090101-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 2018-06-15 16:39 ` Vicente Bergas [not found] ` <CAAMcf8D1-7YvOeOMd+rJOWX7XNQwKeAYjfmyAHOR8TC6vc=G1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-06-15 16:51 ` Marc Zyngier [not found] ` <7ce78e65-51b6-9204-5bb4-e515e36576d4-5wv7dgnIgG8@public.gmane.org> 2018-06-15 18:46 ` Vicente Bergas [not found] ` <CAAMcf8C3fS1SdZVKrsu9zRMn69BcAtFL44QvWMJ8zn4CySnfcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-06-16 10:35 ` Marc Zyngier 2018-08-05 14:09 ` [PATCH] drm/rockchip: shutdown drm subsystem on shutdown Vicente Bergas [not found] ` <20180805140911.19205-1-vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2018-08-05 16:50 ` Marc Zyngier [not found] ` <20180805175038.1d3a0c5e-Fmn/x+r+pSA9//JtdbceeD8Kkb2uy4ct@public.gmane.org> 2018-08-05 17:38 ` Vicente Bergas [not found] ` <CAAMcf8CyUri0LigtJWvahLK62ihPUYa_UvA+2q_EA8m+TL2cqw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-08-05 18:23 ` Marc Zyngier 2018-08-07 12:44 ` Heiko Stuebner 2018-08-07 16:05 ` Vicente Bergas [not found] ` <CAAMcf8D8Vwgi-Amqt8ou9LbDrsRTePjCvwDXWo1JUTGTxLyn_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-08-07 16:07 ` Heiko Stuebner 2018-08-07 16:20 ` Vicente Bergas [not found] ` <CAAMcf8DJhBk8rG65SmQR4vwtj1zfm7jfKJJ4nAx6OsAeu52T2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-09-09 13:43 ` Marc Zyngier [not found] ` <20180909144300.3908a11b-Fmn/x+r+pSA9//JtdbceeD8Kkb2uy4ct@public.gmane.org> 2018-09-10 9:08 ` Heiko Stuebner 2018-09-10 9:57 ` Marc Zyngier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.