From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vicente Bergas Subject: Re: [regression] HDMI breakage just before poweroff Date: Thu, 3 May 2018 23:15:59 +0200 Message-ID: References: <5AEA873A.7080701@rock-chips.com> <0284fa4f-abd6-26f6-31e2-1a6d24777733@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <0284fa4f-abd6-26f6-31e2-1a6d24777733-5wv7dgnIgG8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+glpar-linux-rockchip=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Robin Murphy Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, JeffyChen , Klaus Goger , Jakob Unterwurzacher , Heiko Stuebner List-Id: linux-rockchip.vger.kernel.org On Thu, May 3, 2018 at 2: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. > >>> 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.