Re: [regression] HDMI breakage just before poweroff

[Vicente Bergas <vicencb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

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.