[PATCH V4 2/2] drm/vc4: v3d: add PM suspend/resume support

[Stefan Wahren]

Add suspend/resume support for the VC4 V3D component in order
to handle suspend to idle properly.

Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 2423826c89eb..8057b06c1f16 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -368,7 +368,6 @@ void vc4_v3d_bin_bo_put(struct vc4_dev *vc4)
mutex_unlock(&vc4->bin_bo_lock);
 }

-#ifdef CONFIG_PM
 static int vc4_v3d_runtime_suspend(struct device *dev)
 {
struct vc4_v3d *v3d = dev_get_drvdata(dev);
@@ -397,7 +396,6 @@ static int vc4_v3d_runtime_resume(struct device *dev)

return 0;
 }
-#endif

 int vc4_v3d_debugfs_init(struct drm_minor *minor)
 {
@@ -507,7 +505,8 @@ static void vc4_v3d_unbind(struct device *dev, struct 
device *master,
 }

 static const struct dev_pm_ops vc4_v3d_pm_ops = {
-   SET_RUNTIME_PM_OPS(vc4_v3d_runtime_suspend, vc4_v3d_runtime_resume, 
NULL)
+   RUNTIME_PM_OPS(vc4_v3d_runtime_suspend, vc4_v3d_runtime_resume, NULL)
+   SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
 };

 static const struct component_ops vc4_v3d_ops = {
@@ -538,6 +537,6 @@ struct platform_driver vc4_v3d_driver = {
.driver = {
.name = "vc4_v3d",
.of_match_table = vc4_v3d_dt_match,
-   .pm = &vc4_v3d_pm_ops,
+   .pm = pm_ptr(&vc4_v3d_pm_ops),
},
 };
--
2.34.1

[PATCH V4 0/2] drm/vc4: add PM suspend/resume support

[Stefan Wahren]

This series implement the initial S2Idle support for VC4,
which was a long time on my TODO list [1]. These patches were
part of a bigger series [2], but after discovering a critical USB
issue [3], i decided to split these changes out. There are no
dependencies to the rest of the patches.

Changes in V4:
- fix compile warnings for VC4 & V3D patches

[1] - https://github.com/lategoodbye/rpi-zero/issues/9
[2] - 
https://lore.kernel.org/linux-arm-kernel/20240821214052.6800-1-wahre...@gmx.net/
[3] - 
https://lore.kernel.org/linux-usb/2c9b6e0c-cb7d-4ab2-9d7a-e2f90e642...@gmx.net/T/

Stefan Wahren (2):
  drm/vc4: hdmi: add PM suspend/resume support
  drm/vc4: v3d: add PM suspend/resume support

 drivers/gpu/drm/vc4/vc4_hdmi.c | 32 
 drivers/gpu/drm/vc4/vc4_v3d.c  |  7 +++
 2 files changed, 31 insertions(+), 8 deletions(-)

--
2.34.1

[PATCH V4 1/2] drm/vc4: hdmi: add PM suspend/resume support

[Stefan Wahren]

Add suspend/resume support for the VC4 HDMI component in order
to handle suspend to idle properly. Since the HDMI power domain
is powered down during suspend, this makes connector status polling
pointless.

Link: 
https://lore.kernel.org/dri-devel/7003512d-7303-4f41-b0d6-a8af5bf8e...@gmx.net/
Signed-off-by: Stefan Wahren 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 32 
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 62b82b1eeb36..5e5d1c609858 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -3117,6 +3117,31 @@ static int vc5_hdmi_init_resources(struct drm_device 
*drm,
return 0;
 }

+static int vc4_hdmi_suspend(struct device *dev)
+{
+   struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
+
+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_disable(drm);
+
+   return pm_runtime_force_suspend(dev);
+}
+
+static int vc4_hdmi_resume(struct device *dev)
+{
+   struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
+   int ret;
+
+   ret = pm_runtime_force_resume(dev);
+
+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_enable(drm);
+
+   return ret;
+}
+
 static int vc4_hdmi_runtime_suspend(struct device *dev)
 {
struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
@@ -3415,9 +3440,8 @@ static const struct of_device_id vc4_hdmi_dt_match[] = {
 };

 static const struct dev_pm_ops vc4_hdmi_pm_ops = {
-   SET_RUNTIME_PM_OPS(vc4_hdmi_runtime_suspend,
-  vc4_hdmi_runtime_resume,
-  NULL)
+   RUNTIME_PM_OPS(vc4_hdmi_runtime_suspend, vc4_hdmi_runtime_resume, NULL)
+   SYSTEM_SLEEP_PM_OPS(vc4_hdmi_suspend, vc4_hdmi_resume)
 };

 struct platform_driver vc4_hdmi_driver = {
@@ -3426,6 +3450,6 @@ struct platform_driver vc4_hdmi_driver = {
.driver = {
.name = "vc4_hdmi",
.of_match_table = vc4_hdmi_dt_match,
-   .pm = &vc4_hdmi_pm_ops,
+   .pm = pm_ptr(&vc4_hdmi_pm_ops),
},
 };
--
2.34.1

Re: [PATCH V3 1/9] mailbox: bcm2835: Fix timeout during suspend mode

[Stefan Wahren]

Hi Jassi,

Am 01.09.24 um 22:26 schrieb Jassi Brar:

On Sat, Aug 31, 2024 at 4:19 AM Stefan Wahren  wrote:

Hi Jassi,

Am 21.08.24 um 23:40 schrieb Stefan Wahren:

During noirq suspend phase the Raspberry Pi power driver suffer of
firmware property timeouts. The reason is that the IRQ of the underlying
BCM2835 mailbox is disabled and rpi_firmware_property_list() will always
run into a timeout [1].

Since the VideoCore side isn't consider as a wakeup source, set the
IRQF_NO_SUSPEND flag for the mailbox IRQ in order to keep it enabled
during suspend-resume cycle.

[1]
PM: late suspend of devices complete after 1.754 msecs
WARNING: CPU: 0 PID: 438 at drivers/firmware/raspberrypi.c:128
   rpi_firmware_property_list+0x204/0x22c
Firmware transaction 0x00028001 timeout
Modules linked in:
CPU: 0 PID: 438 Comm: bash Tainted: G C 6.9.3-dirty #17
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x34/0x44
dump_stack_lvl from __warn+0x88/0xec
__warn from warn_slowpath_fmt+0x7c/0xb0
warn_slowpath_fmt from rpi_firmware_property_list+0x204/0x22c
rpi_firmware_property_list from rpi_firmware_property+0x68/0x8c
rpi_firmware_property from rpi_firmware_set_power+0x54/0xc0
rpi_firmware_set_power from _genpd_power_off+0xe4/0x148
_genpd_power_off from genpd_sync_power_off+0x7c/0x11c
genpd_sync_power_off from genpd_finish_suspend+0xcc/0xe0
genpd_finish_suspend from dpm_run_callback+0x78/0xd0
dpm_run_callback from device_suspend_noirq+0xc0/0x238
device_suspend_noirq from dpm_suspend_noirq+0xb0/0x168
dpm_suspend_noirq from suspend_devices_and_enter+0x1b8/0x5ac
suspend_devices_and_enter from pm_suspend+0x254/0x2e4
pm_suspend from state_store+0xa8/0xd4
state_store from kernfs_fop_write_iter+0x154/0x1a0
kernfs_fop_write_iter from vfs_write+0x12c/0x184
vfs_write from ksys_write+0x78/0xc0
ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0xcc93dfa8 to 0xcc93dff0)
[...]
PM: noirq suspend of devices complete after 3095.584 msecs

Link: https://github.com/raspberrypi/firmware/issues/1894
Fixes: 0bae6af6d704 ("mailbox: Enable BCM2835 mailbox support")
Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 

gentle ping

This sounds like a fix but also a part of 9 patches update. Do you
want this merged as a bugfix now or into the next window.

there is no dependency to the rest of the series. Since this is late in
the 6.11 cycle, i'm fine with merging it for the next window.

Thanks


thanks

Re: [PATCH V3 1/9] mailbox: bcm2835: Fix timeout during suspend mode

[Stefan Wahren]

Hi Jassi,

Am 21.08.24 um 23:40 schrieb Stefan Wahren:

During noirq suspend phase the Raspberry Pi power driver suffer of
firmware property timeouts. The reason is that the IRQ of the underlying
BCM2835 mailbox is disabled and rpi_firmware_property_list() will always
run into a timeout [1].

Since the VideoCore side isn't consider as a wakeup source, set the
IRQF_NO_SUSPEND flag for the mailbox IRQ in order to keep it enabled
during suspend-resume cycle.

[1]
PM: late suspend of devices complete after 1.754 msecs
WARNING: CPU: 0 PID: 438 at drivers/firmware/raspberrypi.c:128
  rpi_firmware_property_list+0x204/0x22c
Firmware transaction 0x00028001 timeout
Modules linked in:
CPU: 0 PID: 438 Comm: bash Tainted: G C 6.9.3-dirty #17
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x34/0x44
dump_stack_lvl from __warn+0x88/0xec
__warn from warn_slowpath_fmt+0x7c/0xb0
warn_slowpath_fmt from rpi_firmware_property_list+0x204/0x22c
rpi_firmware_property_list from rpi_firmware_property+0x68/0x8c
rpi_firmware_property from rpi_firmware_set_power+0x54/0xc0
rpi_firmware_set_power from _genpd_power_off+0xe4/0x148
_genpd_power_off from genpd_sync_power_off+0x7c/0x11c
genpd_sync_power_off from genpd_finish_suspend+0xcc/0xe0
genpd_finish_suspend from dpm_run_callback+0x78/0xd0
dpm_run_callback from device_suspend_noirq+0xc0/0x238
device_suspend_noirq from dpm_suspend_noirq+0xb0/0x168
dpm_suspend_noirq from suspend_devices_and_enter+0x1b8/0x5ac
suspend_devices_and_enter from pm_suspend+0x254/0x2e4
pm_suspend from state_store+0xa8/0xd4
state_store from kernfs_fop_write_iter+0x154/0x1a0
kernfs_fop_write_iter from vfs_write+0x12c/0x184
vfs_write from ksys_write+0x78/0xc0
ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0xcc93dfa8 to 0xcc93dff0)
[...]
PM: noirq suspend of devices complete after 3095.584 msecs

Link: https://github.com/raspberrypi/firmware/issues/1894
Fixes: 0bae6af6d704 ("mailbox: Enable BCM2835 mailbox support")
Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 

gentle ping

---
  drivers/mailbox/bcm2835-mailbox.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/bcm2835-mailbox.c 
b/drivers/mailbox/bcm2835-mailbox.c
index fbfd0202047c..ea12fb8d2401 100644
--- a/drivers/mailbox/bcm2835-mailbox.c
+++ b/drivers/mailbox/bcm2835-mailbox.c
@@ -145,7 +145,8 @@ static int bcm2835_mbox_probe(struct platform_device *pdev)
spin_lock_init(&mbox->lock);

ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
-  bcm2835_mbox_irq, 0, dev_name(dev), mbox);
+  bcm2835_mbox_irq, IRQF_NO_SUSPEND, dev_name(dev),
+  mbox);
if (ret) {
dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
ret);
--
2.34.1

Re: [PATCH V3 7/9] usb: dwc2: Refactor backup/restore of registers

[Stefan Wahren]

Am 21.08.24 um 23:40 schrieb Stefan Wahren:

The DWC2 runtime PM code reuses similar patterns to backup and
restore the registers. So consolidate them in USB mode specific
variants. This also has the advantage it is usable for further
PM improvements.

Signed-off-by: Stefan Wahren 
---
  drivers/usb/dwc2/core.h   |  12 +
  drivers/usb/dwc2/gadget.c | 101 +++---
  drivers/usb/dwc2/hcd.c|  99 +++--
  3 files changed, 114 insertions(+), 98 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2bd74f3033ed..81e8632f29ed 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1435,6 +1435,8 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg 
*hsotg);
  int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg);
  void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg);
  void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_backup_critical_registers(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_restore_critical_registers(struct dwc2_hsotg *hsotg);
  static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg)
  { hsotg->fifo_map = 0; }
  #else
@@ -1482,6 +1484,10 @@ static inline int 
dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg)
  { return 0; }
  static inline void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg) {}
  static inline void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_gadget_backup_critical_registers(struct dwc2_hsotg 
*hsotg)
+{ return 0; }
+static inline int dwc2_gadget_restore_critical_registers(struct dwc2_hsotg 
*hsotg)
+{ return 0; }
  static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg) {}
  #endif

@@ -1505,6 +1511,8 @@ int dwc2_host_exit_partial_power_down(struct dwc2_hsotg 
*hsotg,
  void dwc2_host_enter_clock_gating(struct dwc2_hsotg *hsotg);
  void dwc2_host_exit_clock_gating(struct dwc2_hsotg *hsotg, int rem_wakeup);
  bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2);
+int dwc2_host_backup_critical_registers(struct dwc2_hsotg *hsotg);
+int dwc2_host_restore_critical_registers(struct dwc2_hsotg *hsotg);
  static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg)
  { schedule_work(&hsotg->phy_reset_work); }
  #else
@@ -1544,6 +1552,10 @@ static inline void dwc2_host_exit_clock_gating(struct 
dwc2_hsotg *hsotg,
   int rem_wakeup) {}
  static inline bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2)
  { return false; }
+static inline int dwc2_host_backup_critical_registers(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_host_restore_critical_registers(struct dwc2_hsotg 
*hsotg)
+{ return 0; }
  static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg) {}

  #endif
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e7bf9cc635be..0bff748bcf74 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -5309,6 +5309,49 @@ void dwc2_gadget_program_ref_clk(struct dwc2_hsotg 
*hsotg)
dev_dbg(hsotg->dev, "GREFCLK=0x%08x\n", dwc2_readl(hsotg, GREFCLK));
  }

+int dwc2_gadget_backup_critical_registers(struct dwc2_hsotg *hsotg)
+{
+   int ret;
+
+   /* Backup all registers */
+   ret = dwc2_backup_global_registers(hsotg);
+   if (ret) {
+   dev_err(hsotg->dev, "%s: failed to backup global registers\n",
+   __func__);
+   return ret;
+   }
+
+   ret = dwc2_backup_device_registers(hsotg);
+   if (ret) {
+   dev_err(hsotg->dev, "%s: failed to backup device registers\n",
+   __func__);
+   return ret;
+   }
+
+   return 0;
+}
+
+int dwc2_gadget_restore_critical_registers(struct dwc2_hsotg *hsotg)
+{
+   int ret;
+
+   ret = dwc2_restore_global_registers(hsotg);
+   if (ret) {
+   dev_err(hsotg->dev, "%s: failed to restore registers\n",
+   __func__);
+   return ret;
+   }
+
+   ret = dwc2_restore_device_registers(hsotg, 0);
+   if (ret) {
+   dev_err(hsotg->dev, "%s: failed to restore device registers\n",
+   __func__);
+   return ret;
+   }
+
+   return 0;
+}
+
  /**
   * dwc2_gadget_enter_hibernation() - Put controller in Hibernation.
   *
@@ -5326,18 +5369,9 @@ int dwc2_gadget_enter_hibernation(struct dwc2_hsotg 
*hsotg)
/* Change to L2(suspend) state */
hsotg->lx_state = DWC2_L2;
dev_dbg(hsotg->dev, "Start of hibernation completed\n");
-   ret = dwc2_backup_global_registers(hsotg);
-   if (ret) {
-   dev_err(hsotg->dev, "%s: failed to backup global registers\n",
-   __func__);
-   return ret;
-   }
-   ret = dwc2_back

[PATCH V3 9/9] ARM: bcm2835_defconfig: Enable SUSPEND

[Stefan Wahren]

Since the Raspberry Pi supports Suspend-To-Idle now, this option
should be enabled. This should make power management testing easier.

Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 
---
 arch/arm/configs/bcm2835_defconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/configs/bcm2835_defconfig 
b/arch/arm/configs/bcm2835_defconfig
index b5f0bd8dd536..97632dee1ab3 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -38,8 +38,6 @@ CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_CPUFREQ_DT=y
 CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
 CONFIG_VFP=y
-# CONFIG_SUSPEND is not set
-CONFIG_PM=y
 CONFIG_JUMP_LABEL=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
--
2.34.1

[PATCH V3 7/9] usb: dwc2: Refactor backup/restore of registers

[Stefan Wahren]

The DWC2 runtime PM code reuses similar patterns to backup and
restore the registers. So consolidate them in USB mode specific
variants. This also has the advantage it is usable for further
PM improvements.

Signed-off-by: Stefan Wahren 
---
 drivers/usb/dwc2/core.h   |  12 +
 drivers/usb/dwc2/gadget.c | 101 +++---
 drivers/usb/dwc2/hcd.c|  99 +++--
 3 files changed, 114 insertions(+), 98 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2bd74f3033ed..81e8632f29ed 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1435,6 +1435,8 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg 
*hsotg);
 int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg);
 void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg);
 void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_backup_critical_registers(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_restore_critical_registers(struct dwc2_hsotg *hsotg);
 static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg)
 { hsotg->fifo_map = 0; }
 #else
@@ -1482,6 +1484,10 @@ static inline int 
dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg)
 { return 0; }
 static inline void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg) {}
 static inline void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_gadget_backup_critical_registers(struct dwc2_hsotg 
*hsotg)
+{ return 0; }
+static inline int dwc2_gadget_restore_critical_registers(struct dwc2_hsotg 
*hsotg)
+{ return 0; }
 static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg) {}
 #endif

@@ -1505,6 +1511,8 @@ int dwc2_host_exit_partial_power_down(struct dwc2_hsotg 
*hsotg,
 void dwc2_host_enter_clock_gating(struct dwc2_hsotg *hsotg);
 void dwc2_host_exit_clock_gating(struct dwc2_hsotg *hsotg, int rem_wakeup);
 bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2);
+int dwc2_host_backup_critical_registers(struct dwc2_hsotg *hsotg);
+int dwc2_host_restore_critical_registers(struct dwc2_hsotg *hsotg);
 static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg)
 { schedule_work(&hsotg->phy_reset_work); }
 #else
@@ -1544,6 +1552,10 @@ static inline void dwc2_host_exit_clock_gating(struct 
dwc2_hsotg *hsotg,
   int rem_wakeup) {}
 static inline bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2)
 { return false; }
+static inline int dwc2_host_backup_critical_registers(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_host_restore_critical_registers(struct dwc2_hsotg 
*hsotg)
+{ return 0; }
 static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg) {}

 #endif
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e7bf9cc635be..0bff748bcf74 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -5309,6 +5309,49 @@ void dwc2_gadget_program_ref_clk(struct dwc2_hsotg 
*hsotg)
dev_dbg(hsotg->dev, "GREFCLK=0x%08x\n", dwc2_readl(hsotg, GREFCLK));
 }

+int dwc2_gadget_backup_critical_registers(struct dwc2_hsotg *hsotg)
+{
+   int ret;
+
+   /* Backup all registers */
+   ret = dwc2_backup_global_registers(hsotg);
+   if (ret) {
+   dev_err(hsotg->dev, "%s: failed to backup global registers\n",
+   __func__);
+   return ret;
+   }
+
+   ret = dwc2_backup_device_registers(hsotg);
+   if (ret) {
+   dev_err(hsotg->dev, "%s: failed to backup device registers\n",
+   __func__);
+   return ret;
+   }
+
+   return 0;
+}
+
+int dwc2_gadget_restore_critical_registers(struct dwc2_hsotg *hsotg)
+{
+   int ret;
+
+   ret = dwc2_restore_global_registers(hsotg);
+   if (ret) {
+   dev_err(hsotg->dev, "%s: failed to restore registers\n",
+   __func__);
+   return ret;
+   }
+
+   ret = dwc2_restore_device_registers(hsotg, 0);
+   if (ret) {
+   dev_err(hsotg->dev, "%s: failed to restore device registers\n",
+   __func__);
+   return ret;
+   }
+
+   return 0;
+}
+
 /**
  * dwc2_gadget_enter_hibernation() - Put controller in Hibernation.
  *
@@ -5326,18 +5369,9 @@ int dwc2_gadget_enter_hibernation(struct dwc2_hsotg 
*hsotg)
/* Change to L2(suspend) state */
hsotg->lx_state = DWC2_L2;
dev_dbg(hsotg->dev, "Start of hibernation completed\n");
-   ret = dwc2_backup_global_registers(hsotg);
-   if (ret) {
-   dev_err(hsotg->dev, "%s: failed to backup global registers\n",
-   __func__);
-   return ret;
-   }
-   ret = dwc2_backup_device_registers(hsotg);
-   if (ret) {
-   dev_e

[PATCH V3 RFC 8/9] usb: dwc2: Implement recovery after PM domain off

[Stefan Wahren]

According to the dt-bindings there are some platforms, which have a
dedicated USB power domain for DWC2 IP core supply. If the power domain
is switched off during system suspend then all USB register will lose
their settings.

Use GUSBCFG_TOUTCAL as a canary to detect that the power domain has
been powered off during suspend. Since the GOTGCTL_CURMODE_HOST doesn't
match on all platform with the current mode, additionally backup
GINTSTS. This works reliable to decide which registers should be
restored.

Signed-off-by: Stefan Wahren 
---
 drivers/usb/dwc2/core.c |  1 +
 drivers/usb/dwc2/core.h |  2 ++
 drivers/usb/dwc2/platform.c | 38 +
 3 files changed, 41 insertions(+)

Hi, would be nice to get some feedback about the approach and
test results on other DWC2 platforms.

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 9919ab725d54..c3d24312db0f 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -43,6 +43,7 @@ int dwc2_backup_global_registers(struct dwc2_hsotg *hsotg)
/* Backup global regs */
gr = &hsotg->gr_backup;

+   gr->gintsts = dwc2_readl(hsotg, GINTSTS);
gr->gotgctl = dwc2_readl(hsotg, GOTGCTL);
gr->gintmsk = dwc2_readl(hsotg, GINTMSK);
gr->gahbcfg = dwc2_readl(hsotg, GAHBCFG);
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 81e8632f29ed..e65a74853bb0 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -667,6 +667,7 @@ struct dwc2_hw_params {
 /**
  * struct dwc2_gregs_backup - Holds global registers state before
  * entering partial power down
+ * @gintsts:   Backup of GINTSTS register
  * @gotgctl:   Backup of GOTGCTL register
  * @gintmsk:   Backup of GINTMSK register
  * @gahbcfg:   Backup of GAHBCFG register
@@ -683,6 +684,7 @@ struct dwc2_hw_params {
  * @valid: True if registers values backuped.
  */
 struct dwc2_gregs_backup {
+   u32 gintsts;
u32 gotgctl;
u32 gintmsk;
u32 gahbcfg;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 7b84416dfc2b..39e9064b6cfe 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -683,6 +683,14 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
regulator_disable(dwc2->usb33d);
}

+   if (is_device_mode)
+   ret = dwc2_gadget_backup_critical_registers(dwc2);
+   else
+   ret = dwc2_host_backup_critical_registers(dwc2);
+
+   if (ret)
+   return ret;
+
if (dwc2->ll_hw_enabled &&
(is_device_mode || dwc2_host_can_poweroff_phy(dwc2))) {
ret = __dwc2_lowlevel_hw_disable(dwc2);
@@ -692,6 +700,24 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
return ret;
 }

+static int dwc2_restore_critical_registers(struct dwc2_hsotg *hsotg)
+{
+   struct dwc2_gregs_backup *gr;
+
+   gr = &hsotg->gr_backup;
+
+   if (!gr->valid) {
+   dev_err(hsotg->dev, "%s: no registers to restore\n",
+   __func__);
+   return -EINVAL;
+   }
+
+   if (gr->gintsts & GINTSTS_CURMODE_HOST)
+   return dwc2_host_restore_critical_registers(hsotg);
+
+   return dwc2_gadget_restore_critical_registers(hsotg);
+}
+
 static int __maybe_unused dwc2_resume(struct device *dev)
 {
struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
@@ -704,6 +730,18 @@ static int __maybe_unused dwc2_resume(struct device *dev)
}
dwc2->phy_off_for_suspend = false;

+   /*
+* During suspend it's possible that the power domain for the
+* DWC2 controller is disabled and all register values get lost.
+* In case the GUSBCFG register is not initialized, it's clear the
+* registers must be restored.
+*/
+   if (!(dwc2_readl(dwc2, GUSBCFG) & GUSBCFG_TOUTCAL_MASK)) {
+   ret = dwc2_restore_critical_registers(dwc2);
+   if (ret)
+   return ret;
+   }
+
if (dwc2->params.activate_stm_id_vb_detection) {
unsigned long flags;
u32 ggpio, gotgctl;
--
2.34.1

[PATCH V3 0/9] ARM: bcm2835: Implement initial S2Idle for Raspberry Pi

[Stefan Wahren]

This series implement the initial S2Idle support for
the Raspberry Pi, which was a long time on my TODO list [1]. The
changes allow to suspend and resume the Raspberry Pi via debug UART.
The focus is on the BCM2835 SoC, because it's less complex than its
successors and have enough documentation.

The series can be roughly separated in 3 parts:
 1. base patches (1, 9) which allows S2Idle support for BCM2835
 2. drm vc4 patches (2 - 6)  which implement S2Idle support
 3. dwc2 patches (7, 8) which handle BCM2835 specific issues

Cherry-picking of patches should be fine.

Test steps:
- configure debug console (pl011 or mini UART) as wakeup source
- send system to idle state

  echo freeze > /sys/power/state

- wakeup system by console traffic

The DWC2 part based on an idea of Doug Anderson and its implementation
should be mostly finished now, but still RFC. The USB domain is now powered
down and the USB devices are still usable after resume. There is still room
for improvements, but at least the system won't freeze forever as before.

Here are some figures for the Raspberry Pi 1 (without any
devices connected except of a debug UART):

running but CPU idle = 1.67 W
S2Idle   = 1.33 W

In comparison with HDMI & USB keyboard connected (but neither active
nor wakeup source):

running but CPU idle = 1.82 W
S2Idle   = 1.33 W

The series has been successfully tested on the following platforms:
Raspberry Pi 1 B
Raspberry Pi 3 B+

Changes in V3:
- added Reviewed-by & Acked-by from Florian & Maíra
- dropped applied pmdomain & bcm2835aux patches
- address comments by Maíra (patch 3 & 5)
- replace old USB recovery patch with canary approach [3], which should
  work with other platforms

Changes in V2:
- rebased against todays mainline
- added Reviewed-by from Florian
- added Acked-by from Minas
- dropped "irqchip/bcm2835: Enable SKIP_SET_WAKE and MASK_ON_SUSPEND"
  because it has been applied by Thomas Gleixner
- dropped "pmdomain: raspberrypi-power: Avoid powering down USB"
  because this workaround has been replaced by patch 14
- use drm_err_once instead of DRM_ERROR and return connector_status_unknown
  in patch 6
- add new patch in order to clean-up all DRM_ERROR
- add new patch to improve raspberrypi-power logging
- add new patch to simplify V3D clock retrieval
- add new patch 5 to avoid power down of wakeup devices
- add new patch 12 to avoid confusion about ACPI ID of BCM283x USB
- add new patch 8 & 10 which address the problem that HDMI
  is not functional after s2idle
- add more links and fix typo in patch 13
- add new WIP patch 14 which recover DWC2 register after power down
- take care of UART clock in patch 15 as commented by Florian
- use SYSTEM_SLEEP_PM_OPS in patch 15

[1] - https://github.com/lategoodbye/rpi-zero/issues/9
[2] - https://bugzilla.redhat.com/show_bug.cgi?id=2283978
[3] - 
https://lore.kernel.org/linux-usb/CAD=FV=w7sdi1+shfhy6rrjk32r8iage4w+o_u5sp982vgbu...@mail.gmail.com/

Stefan Wahren (9):
  mailbox: bcm2835: Fix timeout during suspend mode
  drm/vc4: hdmi: Handle error case of pm_runtime_resume_and_get
  drm/vc4: Get the rid of DRM_ERROR()
  drm/vc4: hdmi: add PM suspend/resume support
  drm/vc4: v3d: simplify clock retrieval
  drm/vc4: v3d: add PM suspend/resume support
  usb: dwc2: Refactor backup/restore of registers
  usb: dwc2: Implement recovery after PM domain off
  ARM: bcm2835_defconfig: Enable SUSPEND

 arch/arm/configs/bcm2835_defconfig |   2 -
 drivers/gpu/drm/vc4/vc4_bo.c   |  14 ++--
 drivers/gpu/drm/vc4/vc4_dpi.c  |  14 ++--
 drivers/gpu/drm/vc4/vc4_dsi.c  |  32 +
 drivers/gpu/drm/vc4/vc4_gem.c  |  11 ++--
 drivers/gpu/drm/vc4/vc4_hdmi.c |  70 ++--
 drivers/gpu/drm/vc4/vc4_hvs.c  |   4 +-
 drivers/gpu/drm/vc4/vc4_irq.c  |   2 +-
 drivers/gpu/drm/vc4/vc4_v3d.c  |  26 +++-
 drivers/gpu/drm/vc4/vc4_validate.c |   8 +--
 drivers/gpu/drm/vc4/vc4_vec.c  |  10 +--
 drivers/mailbox/bcm2835-mailbox.c  |   3 +-
 drivers/usb/dwc2/core.c|   1 +
 drivers/usb/dwc2/core.h|  14 
 drivers/usb/dwc2/gadget.c  | 101 +++--
 drivers/usb/dwc2/hcd.c |  99 ++--
 drivers/usb/dwc2/platform.c|  38 +++
 17 files changed, 265 insertions(+), 184 deletions(-)

--
2.34.1

[PATCH V3 2/9] drm/vc4: hdmi: Handle error case of pm_runtime_resume_and_get

[Stefan Wahren]

The commit 0f5251339eda ("drm/vc4: hdmi: Make sure the controller is
powered in detect") introduced the necessary power management handling
to avoid register access while controller is powered down.
Unfortunately it just print a warning if pm_runtime_resume_and_get()
fails and proceed anyway.

This could happen during suspend to idle. So we must assume it is unsafe
to access the HDMI register. So bail out properly.

Fixes: 0f5251339eda ("drm/vc4: hdmi: Make sure the controller is powered in 
detect")
Signed-off-by: Stefan Wahren 
Reviewed-by: Maíra Canal 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d57c4a5948c8..cb424604484f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -429,6 +429,7 @@ static int vc4_hdmi_connector_detect_ctx(struct 
drm_connector *connector,
 {
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
enum drm_connector_status status = connector_status_disconnected;
+   int ret;

/*
 * NOTE: This function should really take vc4_hdmi->mutex, but
@@ -441,7 +442,12 @@ static int vc4_hdmi_connector_detect_ctx(struct 
drm_connector *connector,
 * the lock for now.
 */

-   WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
+   ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev);
+   if (ret) {
+   drm_err_once(connector->dev, "Failed to retain HDMI power 
domain: %d\n",
+ret);
+   return connector_status_unknown;
+   }

if (vc4_hdmi->hpd_gpio) {
if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
--
2.34.1

[PATCH V3 6/9] drm/vc4: v3d: add PM suspend/resume support

[Stefan Wahren]

Add suspend/resume support for the VC4 V3D component in order
to handle suspend to idle properly.

Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index bf5c4e36c94e..03c790f7ffc6 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -508,6 +508,8 @@ static void vc4_v3d_unbind(struct device *dev, struct 
device *master,

 static const struct dev_pm_ops vc4_v3d_pm_ops = {
SET_RUNTIME_PM_OPS(vc4_v3d_runtime_suspend, vc4_v3d_runtime_resume, 
NULL)
+   SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+   pm_runtime_force_resume)
 };

 static const struct component_ops vc4_v3d_ops = {
--
2.34.1

[PATCH V3 3/9] drm/vc4: Get the rid of DRM_ERROR()

[Stefan Wahren]

DRM_ERROR() has been deprecated in favor of pr_err(). However, we
should prefer to use drm_err() whenever possible so we get device-
specific output with the error message. In error case of kcalloc,
we can simply drop DRM_ERROR(), because kcalloc already logs errors.

Suggested-by: Maíra Canal 
Signed-off-by: Stefan Wahren 
Reviewed-by: Maxime Ripard 
Reviewed-by: Maíra Canal 
---
 drivers/gpu/drm/vc4/vc4_bo.c   | 14 ++--
 drivers/gpu/drm/vc4/vc4_dpi.c  | 14 ++--
 drivers/gpu/drm/vc4/vc4_dsi.c  | 32 ++
 drivers/gpu/drm/vc4/vc4_gem.c  | 11 +
 drivers/gpu/drm/vc4/vc4_hdmi.c | 36 +++---
 drivers/gpu/drm/vc4/vc4_hvs.c  |  4 ++--
 drivers/gpu/drm/vc4/vc4_irq.c  |  2 +-
 drivers/gpu/drm/vc4/vc4_v3d.c  |  6 ++---
 drivers/gpu/drm/vc4/vc4_validate.c |  8 +++
 drivers/gpu/drm/vc4/vc4_vec.c  | 10 -
 10 files changed, 70 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 86d629e45307..3f72be7490d5 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -469,7 +469,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t 
unaligned_size,

if (IS_ERR(dma_obj)) {
struct drm_printer p = drm_info_printer(vc4->base.dev);
-   DRM_ERROR("Failed to allocate from GEM DMA helper:\n");
+   drm_err(dev, "Failed to allocate from GEM DMA helper:\n");
vc4_bo_stats_print(&p, vc4);
return ERR_PTR(-ENOMEM);
}
@@ -702,7 +702,7 @@ static struct dma_buf *vc4_prime_export(struct 
drm_gem_object *obj, int flags)
 */
ret = vc4_bo_inc_usecnt(bo);
if (ret) {
-   DRM_ERROR("Failed to increment BO usecnt\n");
+   drm_err(obj->dev, "Failed to increment BO usecnt\n");
return ERR_PTR(ret);
}

@@ -1050,10 +1050,10 @@ static void vc4_bo_cache_destroy(struct drm_device 
*dev, void *unused)

for (i = 0; i < vc4->num_labels; i++) {
if (vc4->bo_labels[i].num_allocated) {
-   DRM_ERROR("Destroying BO cache with %d %s "
- "BOs still allocated\n",
- vc4->bo_labels[i].num_allocated,
- vc4->bo_labels[i].name);
+   drm_err(dev, "Destroying BO cache with %d %s "
+   "BOs still allocated\n",
+   vc4->bo_labels[i].num_allocated,
+   vc4->bo_labels[i].name);
}

if (is_user_label(i))
@@ -1083,7 +1083,7 @@ int vc4_label_bo_ioctl(struct drm_device *dev, void *data,

gem_obj = drm_gem_object_lookup(file_priv, args->handle);
if (!gem_obj) {
-   DRM_ERROR("Failed to look up GEM BO %d\n", args->handle);
+   drm_err(dev, "Failed to look up GEM BO %d\n", args->handle);
kfree(name);
return -ENOENT;
}
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 39152e755a13..a382dc4654bd 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -199,8 +199,8 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
*encoder)
   DPI_FORMAT);
break;
default:
-   DRM_ERROR("Unknown media bus format %d\n",
- bus_format);
+   drm_err(dev, "Unknown media bus format %d\n",
+   bus_format);
break;
}
}
@@ -236,11 +236,11 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
*encoder)

ret = clk_set_rate(dpi->pixel_clock, mode->clock * 1000);
if (ret)
-   DRM_ERROR("Failed to set clock rate: %d\n", ret);
+   drm_err(dev, "Failed to set clock rate: %d\n", ret);

ret = clk_prepare_enable(dpi->pixel_clock);
if (ret)
-   DRM_ERROR("Failed to set clock rate: %d\n", ret);
+   drm_err(dev, "Failed to set clock rate: %d\n", ret);

drm_dev_exit(idx);
 }
@@ -339,7 +339,7 @@ static int vc4_dpi_bind(struct device *dev, struct device 
*master, void *data)
if (IS_ERR(dpi->core_clock)) {
ret = PTR_ERR(dpi->core_clock);
if (ret != -EPROBE_DEFER)
-   DRM_ERROR("Failed to get core clock: %d\n", ret);
+   drm_err(drm, "Failed to get core cloc

[PATCH V3 5/9] drm/vc4: v3d: simplify clock retrieval

[Stefan Wahren]

Common pattern of handling deferred probe can be simplified with
dev_err_probe() and devm_clk_get_optional(). This results in much
less code.

Signed-off-by: Stefan Wahren 
Reviewed-by: Maíra Canal 
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 6e566584afbf..bf5c4e36c94e 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -441,21 +441,9 @@ static int vc4_v3d_bind(struct device *dev, struct device 
*master, void *data)
vc4->v3d = v3d;
v3d->vc4 = vc4;

-   v3d->clk = devm_clk_get(dev, NULL);
-   if (IS_ERR(v3d->clk)) {
-   int ret = PTR_ERR(v3d->clk);
-
-   if (ret == -ENOENT) {
-   /* bcm2835 didn't have a clock reference in the DT. */
-   ret = 0;
-   v3d->clk = NULL;
-   } else {
-   if (ret != -EPROBE_DEFER)
-   dev_err(dev, "Failed to get V3D clock: %d\n",
-   ret);
-   return ret;
-   }
-   }
+   v3d->clk = devm_clk_get_optional(dev, NULL);
+   if (IS_ERR(v3d->clk))
+   return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D 
clock\n");

ret = platform_get_irq(pdev, 0);
if (ret < 0)
--
2.34.1

[PATCH V3 4/9] drm/vc4: hdmi: add PM suspend/resume support

[Stefan Wahren]

Add suspend/resume support for the VC4 HDMI component in order
to handle suspend to idle properly. Since the HDMI power domain
is powered down during suspend, this makes connector status polling
pointless.

Link: 
https://lore.kernel.org/dri-devel/7003512d-7303-4f41-b0d6-a8af5bf8e...@gmx.net/
Signed-off-by: Stefan Wahren 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 6611ab7c26a6..f7a4ed16094e 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -3104,6 +3104,31 @@ static int vc5_hdmi_init_resources(struct drm_device 
*drm,
return 0;
 }

+static int vc4_hdmi_suspend(struct device *dev)
+{
+   struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
+
+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_disable(drm);
+
+   return pm_runtime_force_suspend(dev);
+}
+
+static int vc4_hdmi_resume(struct device *dev)
+{
+   struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
+   int ret;
+
+   ret = pm_runtime_force_resume(dev);
+
+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_enable(drm);
+
+   return ret;
+}
+
 static int vc4_hdmi_runtime_suspend(struct device *dev)
 {
struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
@@ -3405,6 +3430,7 @@ static const struct dev_pm_ops vc4_hdmi_pm_ops = {
SET_RUNTIME_PM_OPS(vc4_hdmi_runtime_suspend,
   vc4_hdmi_runtime_resume,
   NULL)
+   SET_SYSTEM_SLEEP_PM_OPS(vc4_hdmi_suspend, vc4_hdmi_resume)
 };

 struct platform_driver vc4_hdmi_driver = {
--
2.34.1

[PATCH V3 1/9] mailbox: bcm2835: Fix timeout during suspend mode

[Stefan Wahren]

During noirq suspend phase the Raspberry Pi power driver suffer of
firmware property timeouts. The reason is that the IRQ of the underlying
BCM2835 mailbox is disabled and rpi_firmware_property_list() will always
run into a timeout [1].

Since the VideoCore side isn't consider as a wakeup source, set the
IRQF_NO_SUSPEND flag for the mailbox IRQ in order to keep it enabled
during suspend-resume cycle.

[1]
PM: late suspend of devices complete after 1.754 msecs
WARNING: CPU: 0 PID: 438 at drivers/firmware/raspberrypi.c:128
 rpi_firmware_property_list+0x204/0x22c
Firmware transaction 0x00028001 timeout
Modules linked in:
CPU: 0 PID: 438 Comm: bash Tainted: G C 6.9.3-dirty #17
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x34/0x44
dump_stack_lvl from __warn+0x88/0xec
__warn from warn_slowpath_fmt+0x7c/0xb0
warn_slowpath_fmt from rpi_firmware_property_list+0x204/0x22c
rpi_firmware_property_list from rpi_firmware_property+0x68/0x8c
rpi_firmware_property from rpi_firmware_set_power+0x54/0xc0
rpi_firmware_set_power from _genpd_power_off+0xe4/0x148
_genpd_power_off from genpd_sync_power_off+0x7c/0x11c
genpd_sync_power_off from genpd_finish_suspend+0xcc/0xe0
genpd_finish_suspend from dpm_run_callback+0x78/0xd0
dpm_run_callback from device_suspend_noirq+0xc0/0x238
device_suspend_noirq from dpm_suspend_noirq+0xb0/0x168
dpm_suspend_noirq from suspend_devices_and_enter+0x1b8/0x5ac
suspend_devices_and_enter from pm_suspend+0x254/0x2e4
pm_suspend from state_store+0xa8/0xd4
state_store from kernfs_fop_write_iter+0x154/0x1a0
kernfs_fop_write_iter from vfs_write+0x12c/0x184
vfs_write from ksys_write+0x78/0xc0
ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0xcc93dfa8 to 0xcc93dff0)
[...]
PM: noirq suspend of devices complete after 3095.584 msecs

Link: https://github.com/raspberrypi/firmware/issues/1894
Fixes: 0bae6af6d704 ("mailbox: Enable BCM2835 mailbox support")
Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 
---
 drivers/mailbox/bcm2835-mailbox.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/bcm2835-mailbox.c 
b/drivers/mailbox/bcm2835-mailbox.c
index fbfd0202047c..ea12fb8d2401 100644
--- a/drivers/mailbox/bcm2835-mailbox.c
+++ b/drivers/mailbox/bcm2835-mailbox.c
@@ -145,7 +145,8 @@ static int bcm2835_mbox_probe(struct platform_device *pdev)
spin_lock_init(&mbox->lock);

ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
-  bcm2835_mbox_irq, 0, dev_name(dev), mbox);
+  bcm2835_mbox_irq, IRQF_NO_SUSPEND, dev_name(dev),
+  mbox);
if (ret) {
dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
ret);
--
2.34.1

Re: [PATCH V2 14/16] WIP: usb: dwc2: Implement recovery after PM domain off

[Stefan Wahren]

Hi Doug,

Am 15.08.24 um 21:37 schrieb Doug Anderson:

Hi,

On Wed, Aug 14, 2024 at 2:48 PM Stefan Wahren  wrote:

You're saying that your
registers get saved _unless_ the power domain gets turned off, right?

On BCM2835 there is no need to store the registers because there is no
power management supported by USB core except of the power domain. So
DWC2 don't expect a register loss.

...and the device core keeps power domains on for suspended devices if
they are wakeup sources, which makes sense.

So with that, your patch sounds like a plausible way to do it. I guess
one other way to do it would be some sort of "canary" approach. You
could _always_ save registers and then, at resume time, you could
detect if some "canary" register had reset to its power-on default. If
you see this then you can assume power was lost and re-init all the
registers. This could be pretty much any register that you know won't
be its power on default. In some ways a "canary" approach is uglier
but it also might be more reliable across more configurations?

I don't have enough knowledge about DWC2 and i also don't have the
databook to figure out if there is a magic register which could be used
for the canary approach. But all these different platforms, host vs
gadget role, different low modes let me think the resulting solution
would be also fragile and ugly.

I won't admit to having a DWC2 databook. ;-)

you convinced me of the canary approach. I missed one critical point the
whole time. The used power domain notifier can/will be called while the
USB clock is disabled. So this worked for the Raspberry Pi because the
clock is fixed.

...but don't think it's too hard to find a good canary. What about
"GAHBCFG_GLBL_INTR_EN" ? From a quick glance it looks like the driver
seems to set that bit during driver startup and then it stays on until
driver shutdown. The databook that I definitely won't admit to having
almost certainly says that this register resets to 0 on all hardware
and it's applicable to both host and device. I think you could say
that if the register is 0 at resume time that registers must have been
lost and you can restore them.

There are several reason to not use the GAHBCFG_GLBL_INTR_EN. One is the
fact that the driver disabled this flag at several places, not just on
shutdown. Another reason is that the register layout of GAHBCFG on
BCM2835 is customized ( see last page of datasheet [1]).

I dumped the relevant registers with a unmodified dwc2 driver and the
outcome is a little bit unexpected (0 = PRE_POWER_OFF, 3 = POWER_ON):

[  169.101071] dwc2 2098.usb: dwc2_suspend enter GAHBCFG = 0031
[  169.101143] dwc2 2098.usb: dwc2_suspend enter GUSBCFG = 20001707
[  169.101172] dwc2 2098.usb: dwc2_suspend enter GINTMSK = f3000806
[  169.105888] dwc2 2098.usb: dwc2_power_notifier: 0 GAHBCFG = 0031
[  169.105962] dwc2 2098.usb: dwc2_power_notifier: 0 GUSBCFG = 20001707
[  169.105994] dwc2 2098.usb: dwc2_power_notifier: 0 GINTMSK = f3000806
[  174.248046] dwc2 2098.usb: dwc2_power_notifier: 3 GAHBCFG = 001f
[  174.248118] dwc2 2098.usb: dwc2_power_notifier: 3 GUSBCFG = 20402700
[  174.248148] dwc2 2098.usb: dwc2_power_notifier: 3 GINTMSK = f3000806
[  174.253086] dwc2 2098.usb: dwc2_resume enter: GAHBCFG = 001f
[  174.253162] dwc2 2098.usb: dwc2_resume enter: GUSBCFG = 20402700
[  174.253190] dwc2 2098.usb: dwc2_resume enter: GINTMSK = f3000806

I guess if that doesn't work then "GUSBCFG_TOUTCAL" could be used (I
think that resets to 0 but must be initted to non-0 by the driver).

Yes this looks good and match with the trace above. The driver seems to
initialize this once and a quick test seems to work so far. I will stick
to this.

Yet another register that could probably work as a canary would be
"GINTMSK". I believe that inits to all 0 (everything is masked) and
obviously to use the device we've got to unmask _some_ interrupts.

I don't know why but this didn't worked according to trace, but i also
didn't noticed a interrupt after enabling of the power domain.Thanks [1]
-
https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf

I can look for more, if need be.

-Doug

Re: [PATCH V2 14/16] WIP: usb: dwc2: Implement recovery after PM domain off

[Stefan Wahren]

Am 14.08.24 um 14:01 schrieb Ulf Hansson:

On Tue, 13 Aug 2024 at 21:57, Doug Anderson  wrote:

Hi,

On Mon, Aug 12, 2024 at 4:48 PM Stefan Wahren  wrote:

Hi Doug,

Am 28.07.24 um 15:00 schrieb Stefan Wahren:

DO NOT MERGE

According to the dt-bindings there are some platforms, which have a
dedicated USB power domain for DWC2 IP core supply. If the power domain
is switched off during system suspend then all USB register will lose
their settings.

So use the power on/off notifier in order to save & restore the USB
registers during system suspend.

sorry for bothering you with this DWC2 stuff, but it would great if you
can gave some feedback about this patch.

Boy, it's been _ages_ since I looked at anything to do with dwc2, but
I still have some fondness in my heart for the crufty old driver :-P I
know I was involved with some of the patches to get
wakeup-from-suspend working on dwc2 host controllers in the past but,
if I remember correctly, I mostly shepherded / fixed patches from
Rockchip. Not sure I can spend the days trawling through the driver
and testing things with printk that really answering properly would
need, but let's see...

Yes, this was more a cry for help, because i didn't get much feedback
yet here [1] [2]. So i searched for the most elegant solution via trial
& error and this patch is the outcome. One reason why this is still WIP,
is that i didn't test the gadget role path yet.



I was working a lot to get
suspend to idle working on Raspberry Pi. And this patch is the most
complex part of the series.

Would you agree with this approach or did i miss something?

The problem is that the power domain driver acts independent from dwc2,
so we cannot prevent the USB domain power down except declaring a USB
device as wakeup source. So i decided to use the notifier approach. This
has been successful tested on some older Raspberry Pi boards.

My genpd knowledge is probably not as good as it should be. Don't tell
anyone (aside from all the people and lists CCed here). ;-)

...so I guess you're relying on the fact that
dev_pm_genpd_add_notifier() will return an error if a power-domain
wasn't specified for dwc2 in the device tree, then you ignore that
error and your callback will never happen. You assume that the power
domain isn't specified then the dwc2 registers will be saved?

Yes, on Raspberry Pi we needed to implement the power domain driver to
enable USB at the first place.

I guess one thing is that I'd wonder if that's really reliable. Maybe
some dwc2 controllers lose their registers over system suspend but
_don't_ specify a power domain? Maybe the USB controller just gets its
power yanked as part of system suspend. Maybe that's why the functions
for saving / restoring registers are already there? It looks like
there are ways for various platforms to specify that registers are
lost in some cases...

Yes, the DT property "snps,need-phy-for-wake" is such a case. But the
PHY on Raspberry Pi is currently modeled as a no-op.

...but I guess you can't use the existing ways to say that registers
are lost because you're trying to be dynamic.

Yes, before this patch the DWC2 doesn't know if the USB domain is
powered down during suspend. So the notifier seems the most elegant
solution to me.

You're saying that your
registers get saved _unless_ the power domain gets turned off, right?

On BCM2835 there is no need to store the registers because there is no
power management supported by USB core except of the power domain. So
DWC2 don't expect a register loss.

...and the device core keeps power domains on for suspended devices if
they are wakeup sources, which makes sense.

So with that, your patch sounds like a plausible way to do it. I guess
one other way to do it would be some sort of "canary" approach. You
could _always_ save registers and then, at resume time, you could
detect if some "canary" register had reset to its power-on default. If
you see this then you can assume power was lost and re-init all the
registers. This could be pretty much any register that you know won't
be its power on default. In some ways a "canary" approach is uglier
but it also might be more reliable across more configurations?

I don't have enough knowledge about DWC2 and i also don't have the
databook to figure out if there is a magic register which could be used
for the canary approach. But all these different platforms, host vs
gadget role, different low modes let me think the resulting solution
would be also fragile and ugly.

I guess those would be my main thoughts on the topic. Is that roughly
the feedback you were looking for?

Yes, thank you. This was very helpful.

Thanks Doug for sharing your thoughts. For the record, I agree with
these suggestions.

Using the genpd on/off notifiers is certainly fine, but doing a
save/restore unconditionally via some of the PM 

Re: [PATCH V2 15/16] serial: 8250_bcm2835aux: add PM suspend/resume support

[Stefan Wahren]

Am 14.08.24 um 14:18 schrieb Ulf Hansson:

On Sun, 28 Jul 2024 at 15:07, Stefan Wahren  wrote:

This adds suspend/resume support for the 8250_bcm2835aux
driver to provide power management support on attached
devices.

Signed-off-by: Stefan Wahren 
---
  drivers/tty/serial/8250/8250_bcm2835aux.c | 37 +++
  1 file changed, 37 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c 
b/drivers/tty/serial/8250/8250_bcm2835aux.c
index 121a5ce86050..36e2bb34d82b 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -13,6 +13,7 @@
   */

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -213,11 +214,47 @@ static const struct acpi_device_id 
bcm2835aux_serial_acpi_match[] = {
  };
  MODULE_DEVICE_TABLE(acpi, bcm2835aux_serial_acpi_match);

+static int bcm2835aux_suspend(struct device *dev)
+{
+   struct bcm2835aux_data *data = dev_get_drvdata(dev);
+   struct uart_8250_port *up = serial8250_get_port(data->line);
+
+   serial8250_suspend_port(data->line);
+
+   if (device_may_wakeup(dev))
+   return 0;
+
+   if (uart_console(&up->port) && !console_suspend_enabled)
+   return 0;
+
+   clk_disable_unprepare(data->clk);
+   return 0;
+}
+
+static int bcm2835aux_resume(struct device *dev)
+{
+   struct bcm2835aux_data *data = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_prepare_enable(data->clk);

Doesn't this create clk prepare/enable - unprepare/disable imbalance
problem when the uart is configured for system wakeup?

Thanks, i will send a follow-up

Regards



+   if (ret)
+   return ret;
+
+   serial8250_resume_port(data->line);
+
+   return 0;
+}
+
+static const struct dev_pm_ops bcm2835aux_dev_pm_ops = {
+   SYSTEM_SLEEP_PM_OPS(bcm2835aux_suspend, bcm2835aux_resume)
+};
+
  static struct platform_driver bcm2835aux_serial_driver = {
 .driver = {
 .name = "bcm2835-aux-uart",
 .of_match_table = bcm2835aux_serial_match,
 .acpi_match_table = bcm2835aux_serial_acpi_match,
+   .pm = pm_ptr(&bcm2835aux_dev_pm_ops),
 },
 .probe  = bcm2835aux_serial_probe,
 .remove_new = bcm2835aux_serial_remove,
--
2.34.1


Kind regards
Uffe

Re: [PATCH V2 14/16] WIP: usb: dwc2: Implement recovery after PM domain off

[Stefan Wahren]

Hi Doug,

Am 28.07.24 um 15:00 schrieb Stefan Wahren:

DO NOT MERGE

According to the dt-bindings there are some platforms, which have a
dedicated USB power domain for DWC2 IP core supply. If the power domain
is switched off during system suspend then all USB register will lose
their settings.

So use the power on/off notifier in order to save & restore the USB
registers during system suspend.

sorry for bothering you with this DWC2 stuff, but it would great if you
can gave some feedback about this patch. I was working a lot to get
suspend to idle working on Raspberry Pi. And this patch is the most
complex part of the series.

Would you agree with this approach or did i miss something?

The problem is that the power domain driver acts independent from dwc2,
so we cannot prevent the USB domain power down except declaring a USB
device as wakeup source. So i decided to use the notifier approach. This
has been successful tested on some older Raspberry Pi boards.

Best regards


Signed-off-by: Stefan Wahren 
---

Any feedback is appreciated.

  drivers/usb/dwc2/core.c | 16 
  drivers/usb/dwc2/core.h | 17 +
  drivers/usb/dwc2/gadget.c   | 49 +
  drivers/usb/dwc2/hcd.c  | 49 +
  drivers/usb/dwc2/platform.c | 32 
  5 files changed, 163 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 9919ab725d54..a3263cfdedac 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -391,6 +391,22 @@ int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, int 
rem_wakeup,
return dwc2_gadget_exit_hibernation(hsotg, rem_wakeup, reset);
  }

+int dwc2_enter_poweroff(struct dwc2_hsotg *hsotg)
+{
+   if (dwc2_is_host_mode(hsotg))
+   return dwc2_host_enter_poweroff(hsotg);
+   else
+   return dwc2_gadget_enter_poweroff(hsotg);
+}
+
+int dwc2_exit_poweroff(struct dwc2_hsotg *hsotg)
+{
+   if (dwc2_is_host_mode(hsotg))
+   return dwc2_host_exit_poweroff(hsotg);
+   else
+   return dwc2_gadget_exit_poweroff(hsotg);
+}
+
  /*
   * Do core a soft reset of the core.  Be careful with this because it
   * resets all the internal state machines of the core.
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2bd74f3033ed..9ab755cc3081 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -9,6 +9,7 @@
  #define __DWC2_CORE_H__

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -1080,6 +1081,8 @@ struct dwc2_hsotg {
struct regulator *vbus_supply;
struct regulator *usb33d;

+   struct notifier_block genpd_nb;
+
spinlock_t lock;
void *priv;
int irq;
@@ -1316,6 +1319,8 @@ int dwc2_exit_partial_power_down(struct dwc2_hsotg 
*hsotg, int rem_wakeup,
  int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg, int is_host);
  int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, int rem_wakeup,
int reset, int is_host);
+int dwc2_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_exit_poweroff(struct dwc2_hsotg *hsotg);
  void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg);
  int dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy);

@@ -1435,6 +1440,8 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg 
*hsotg);
  int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg);
  void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg);
  void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_exit_poweroff(struct dwc2_hsotg *hsotg);
  static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg)
  { hsotg->fifo_map = 0; }
  #else
@@ -1482,6 +1489,10 @@ static inline int 
dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg)
  { return 0; }
  static inline void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg) {}
  static inline void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_gadget_enter_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_gadget_exit_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
  static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg) {}
  #endif

@@ -1505,6 +1516,8 @@ int dwc2_host_exit_partial_power_down(struct dwc2_hsotg 
*hsotg,
  void dwc2_host_enter_clock_gating(struct dwc2_hsotg *hsotg);
  void dwc2_host_exit_clock_gating(struct dwc2_hsotg *hsotg, int rem_wakeup);
  bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2);
+int dwc2_host_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_host_exit_poweroff(struct dwc2_hsotg *hsotg);
  static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg)
  { schedule_work(&hsotg->phy_reset_work); }
  #else
@@ -1544,6 +1557,10 @@ static inline void dwc2_host_exit_clock_gating(struct 
dwc2_hsotg *hsotg,

Re: [PATCH V2 05/16] pmdomain: raspberrypi-power: set flag GENPD_FLAG_ACTIVE_WAKEUP

[Stefan Wahren]

Hi,

Am 28.07.24 um 13:41 schrieb Stefan Wahren:

Set flag GENPD_FLAG_ACTIVE_WAKEUP to rpi_power genpd, then when a device
is set as wakeup source using device_set_wakeup_enable, the power
domain could be kept on to make sure the device could wakeup the system.

Signed-off-by: Stefan Wahren 

I know a lot developer are in holidays, but it would be nice to get a
review for the new pmdomain parts before i send V3.

Best regards

---
  drivers/pmdomain/bcm/raspberrypi-power.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/pmdomain/bcm/raspberrypi-power.c 
b/drivers/pmdomain/bcm/raspberrypi-power.c
index fadedfc9c645..b87ea7adb7be 100644
--- a/drivers/pmdomain/bcm/raspberrypi-power.c
+++ b/drivers/pmdomain/bcm/raspberrypi-power.c
@@ -91,6 +91,7 @@ static void rpi_common_init_power_domain(struct 
rpi_power_domains *rpi_domains,
dom->fw = rpi_domains->fw;

dom->base.name = name;
+   dom->base.flags = GENPD_FLAG_ACTIVE_WAKEUP;
dom->base.power_on = rpi_domain_on;
dom->base.power_off = rpi_domain_off;

--
2.34.1

Re: [PATCH V2 02/16] mailbox: bcm2835: Fix timeout during suspend mode

[Stefan Wahren]

Hi Jassi,

Am 28.07.24 um 13:41 schrieb Stefan Wahren:

During noirq suspend phase the Raspberry Pi power driver suffer of
firmware property timeouts. The reason is that the IRQ of the underlying
BCM2835 mailbox is disabled and rpi_firmware_property_list() will always
run into a timeout [1].

Since the VideoCore side isn't consider as a wakeup source, set the
IRQF_NO_SUSPEND flag for the mailbox IRQ in order to keep it enabled
during suspend-resume cycle.

[1]
PM: late suspend of devices complete after 1.754 msecs
WARNING: CPU: 0 PID: 438 at drivers/firmware/raspberrypi.c:128
  rpi_firmware_property_list+0x204/0x22c
Firmware transaction 0x00028001 timeout
Modules linked in:
CPU: 0 PID: 438 Comm: bash Tainted: G C 6.9.3-dirty #17
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x34/0x44
dump_stack_lvl from __warn+0x88/0xec
__warn from warn_slowpath_fmt+0x7c/0xb0
warn_slowpath_fmt from rpi_firmware_property_list+0x204/0x22c
rpi_firmware_property_list from rpi_firmware_property+0x68/0x8c
rpi_firmware_property from rpi_firmware_set_power+0x54/0xc0
rpi_firmware_set_power from _genpd_power_off+0xe4/0x148
_genpd_power_off from genpd_sync_power_off+0x7c/0x11c
genpd_sync_power_off from genpd_finish_suspend+0xcc/0xe0
genpd_finish_suspend from dpm_run_callback+0x78/0xd0
dpm_run_callback from device_suspend_noirq+0xc0/0x238
device_suspend_noirq from dpm_suspend_noirq+0xb0/0x168
dpm_suspend_noirq from suspend_devices_and_enter+0x1b8/0x5ac
suspend_devices_and_enter from pm_suspend+0x254/0x2e4
pm_suspend from state_store+0xa8/0xd4
state_store from kernfs_fop_write_iter+0x154/0x1a0
kernfs_fop_write_iter from vfs_write+0x12c/0x184
vfs_write from ksys_write+0x78/0xc0
ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0xcc93dfa8 to 0xcc93dff0)
[...]
PM: noirq suspend of devices complete after 3095.584 msecs

Link: https://github.com/raspberrypi/firmware/issues/1894
Fixes: 0bae6af6d704 ("mailbox: Enable BCM2835 mailbox support")
Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 

Are there any objections with this patch?

I consider this solution as settled.

Best regards

---
  drivers/mailbox/bcm2835-mailbox.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/bcm2835-mailbox.c 
b/drivers/mailbox/bcm2835-mailbox.c
index fbfd0202047c..ea12fb8d2401 100644
--- a/drivers/mailbox/bcm2835-mailbox.c
+++ b/drivers/mailbox/bcm2835-mailbox.c
@@ -145,7 +145,8 @@ static int bcm2835_mbox_probe(struct platform_device *pdev)
spin_lock_init(&mbox->lock);

ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
-  bcm2835_mbox_irq, 0, dev_name(dev), mbox);
+  bcm2835_mbox_irq, IRQF_NO_SUSPEND, dev_name(dev),
+  mbox);
if (ret) {
dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
ret);
--
2.34.1

Re: [PATCH V2 01/16] firmware: raspberrypi: Improve timeout warning

[Stefan Wahren]

Hi Florian,

Am 28.07.24 um 13:41 schrieb Stefan Wahren:

Recent work on raspberry-power driver showed that even the
stacktrace on firmware property timeout doesn't provide
enough information. So add the first tag name to the warning
to be in line with a status error.

Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 

Are there any concerns, because i assumed this patch would go via your tree?

Best regards

---
  drivers/firmware/raspberrypi.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index ac34876a97f8..18cc34987108 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -62,7 +62,6 @@ rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, 
u32 data)
ret = 0;
} else {
ret = -ETIMEDOUT;
-   WARN_ONCE(1, "Firmware transaction timeout");
}
} else {
dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
@@ -125,6 +124,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
buf[2], buf[1]);
ret = -EINVAL;
+   } else if (ret == -ETIMEDOUT) {
+   WARN_ONCE(1, "Firmware transaction 0x%08x timeout", buf[2]);
}

dma_free_coherent(fw->chan->mbox->dev, PAGE_ALIGN(size), buf, bus_addr);
--
2.34.1

Re: [PATCH V2 09/16] drm/vc4: v3d: simplify clock retrieval

[Stefan Wahren]

Hi Maíra,

Am 07.08.24 um 16:31 schrieb Maíra Canal:

Hi Stefan,

On 8/2/24 10:00, Stefan Wahren wrote:

Hi Maíra,

Am 02.08.24 um 14:56 schrieb Maíra Canal:

Hi Stefan,

On 7/31/24 13:41, Stefan Wahren wrote:

Hi Maíra,

Am 30.07.24 um 13:23 schrieb Maíra Canal:

On 7/28/24 10:00, Stefan Wahren wrote:

Common pattern of handling deferred probe can be simplified with
dev_err_probe() and devm_clk_get_optional(). This results in much
less code.

Signed-off-by: Stefan Wahren 
---
  drivers/gpu/drm/vc4/vc4_v3d.c | 13 ++---
  1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c
b/drivers/gpu/drm/vc4/vc4_v3d.c
index 1ede508a67d3..4bf3a8d24770 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -441,20 +441,11 @@ static int vc4_v3d_bind(struct device *dev,
struct device *master, void *data)
  vc4->v3d = v3d;
  v3d->vc4 = vc4;

-    v3d->clk = devm_clk_get(dev, NULL);
+    v3d->clk = devm_clk_get_optional(dev, NULL);
  if (IS_ERR(v3d->clk)) {
  int ret = PTR_ERR(v3d->clk);



Super nit: you could delete this line ^

Can you please explain? ret is required for dev_err_probe or do you
mean
the empty line after the declaration?


Just deleting the empty line after the declaration. It is a super small
nit indeed.

AFAIK an empty line after a declaration is part of the coding style. Or
is different in drm?


TBH I just checked the result of `git grep "dev_err_probe"` and I
noticed that most of the times, we don't add an empty line after the
declaration in this case or we don't even create a variable, something
like:

return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");

i will go for the latter variant.

I will send a new version which also addresses your comments regarding
patch 7, so they can be applied at once.

But i still need to wait for some feedback for patch 14 before sending
v3, which is the most important part of the series. But I also hope that
some of the firmware/mailbox/pmdomain patches at the beginning are also
applied before.



But it is a pretty small nit. Feel free to ignore it.

Also, let me know if you need me to apply any patches to drm-misc-next.


Yes, this would be nice to apply the vc4/v3d stuff in the next version,
so the series becomes shorter and easier to handle.

Best regards

Re: [PATCH V2 09/16] drm/vc4: v3d: simplify clock retrieval

[Stefan Wahren]

Hi Maíra,

Am 02.08.24 um 14:56 schrieb Maíra Canal:

Hi Stefan,

On 7/31/24 13:41, Stefan Wahren wrote:

Hi Maíra,

Am 30.07.24 um 13:23 schrieb Maíra Canal:

On 7/28/24 10:00, Stefan Wahren wrote:

Common pattern of handling deferred probe can be simplified with
dev_err_probe() and devm_clk_get_optional(). This results in much
less code.

Signed-off-by: Stefan Wahren 
---
  drivers/gpu/drm/vc4/vc4_v3d.c | 13 ++---
  1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c
b/drivers/gpu/drm/vc4/vc4_v3d.c
index 1ede508a67d3..4bf3a8d24770 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -441,20 +441,11 @@ static int vc4_v3d_bind(struct device *dev,
struct device *master, void *data)
  vc4->v3d = v3d;
  v3d->vc4 = vc4;

-    v3d->clk = devm_clk_get(dev, NULL);
+    v3d->clk = devm_clk_get_optional(dev, NULL);
  if (IS_ERR(v3d->clk)) {
  int ret = PTR_ERR(v3d->clk);



Super nit: you could delete this line ^

Can you please explain? ret is required for dev_err_probe or do you mean
the empty line after the declaration?


Just deleting the empty line after the declaration. It is a super small
nit indeed.

AFAIK an empty line after a declaration is part of the coding style. Or
is different in drm?

Best regards


Best Regards,
- Maíra



Reviewed-by: Maíra Canal 

Best Regards,
- Maíra


-    if (ret == -ENOENT) {
-    /* bcm2835 didn't have a clock reference in the DT. */
-    ret = 0;
-    v3d->clk = NULL;
-    } else {
-    if (ret != -EPROBE_DEFER)
-    dev_err(dev, "Failed to get V3D clock: %d\n",
-    ret);
-    return ret;
-    }
+    return dev_err_probe(dev, ret, "Failed to get V3D clock\n");
  }

  ret = platform_get_irq(pdev, 0);
--
2.34.1

Re: [PATCH V2 09/16] drm/vc4: v3d: simplify clock retrieval

[Stefan Wahren]

Hi Maíra,

Am 30.07.24 um 13:23 schrieb Maíra Canal:

On 7/28/24 10:00, Stefan Wahren wrote:

Common pattern of handling deferred probe can be simplified with
dev_err_probe() and devm_clk_get_optional(). This results in much
less code.

Signed-off-by: Stefan Wahren 
---
  drivers/gpu/drm/vc4/vc4_v3d.c | 13 ++---
  1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c
b/drivers/gpu/drm/vc4/vc4_v3d.c
index 1ede508a67d3..4bf3a8d24770 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -441,20 +441,11 @@ static int vc4_v3d_bind(struct device *dev,
struct device *master, void *data)
  vc4->v3d = v3d;
  v3d->vc4 = vc4;

-    v3d->clk = devm_clk_get(dev, NULL);
+    v3d->clk = devm_clk_get_optional(dev, NULL);
  if (IS_ERR(v3d->clk)) {
  int ret = PTR_ERR(v3d->clk);



Super nit: you could delete this line ^

Can you please explain? ret is required for dev_err_probe or do you mean
the empty line after the declaration?


Reviewed-by: Maíra Canal 

Best Regards,
- Maíra


-    if (ret == -ENOENT) {
-    /* bcm2835 didn't have a clock reference in the DT. */
-    ret = 0;
-    v3d->clk = NULL;
-    } else {
-    if (ret != -EPROBE_DEFER)
-    dev_err(dev, "Failed to get V3D clock: %d\n",
-    ret);
-    return ret;
-    }
+    return dev_err_probe(dev, ret, "Failed to get V3D clock\n");
  }

  ret = platform_get_irq(pdev, 0);
--
2.34.1

[PATCH V2 16/16] ARM: bcm2835_defconfig: Enable SUSPEND

[Stefan Wahren]

Since the Raspberry Pi supports Suspend-To-Idle now, this option
should be enabled. This should make power management testing easier.

Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 
---
 arch/arm/configs/bcm2835_defconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/configs/bcm2835_defconfig 
b/arch/arm/configs/bcm2835_defconfig
index b5f0bd8dd536..97632dee1ab3 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -38,8 +38,6 @@ CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_CPUFREQ_DT=y
 CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
 CONFIG_VFP=y
-# CONFIG_SUSPEND is not set
-CONFIG_PM=y
 CONFIG_JUMP_LABEL=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
--
2.34.1

[PATCH V2 10/16] drm/vc4: v3d: add PM suspend/resume support

[Stefan Wahren]

Add suspend/resume support for the VC4 V3D component in order
to handle suspend to idle properly.

Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 4bf3a8d24770..309c8af08fd0 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -511,6 +511,8 @@ static void vc4_v3d_unbind(struct device *dev, struct 
device *master,

 static const struct dev_pm_ops vc4_v3d_pm_ops = {
SET_RUNTIME_PM_OPS(vc4_v3d_runtime_suspend, vc4_v3d_runtime_resume, 
NULL)
+   SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+   pm_runtime_force_resume)
 };

 static const struct component_ops vc4_v3d_ops = {
--
2.34.1

[PATCH V2 15/16] serial: 8250_bcm2835aux: add PM suspend/resume support

[Stefan Wahren]

This adds suspend/resume support for the 8250_bcm2835aux
driver to provide power management support on attached
devices.

Signed-off-by: Stefan Wahren 
---
 drivers/tty/serial/8250/8250_bcm2835aux.c | 37 +++
 1 file changed, 37 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c 
b/drivers/tty/serial/8250/8250_bcm2835aux.c
index 121a5ce86050..36e2bb34d82b 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -13,6 +13,7 @@
  */

 #include 
+#include 
 #include 
 #include 
 #include 
@@ -213,11 +214,47 @@ static const struct acpi_device_id 
bcm2835aux_serial_acpi_match[] = {
 };
 MODULE_DEVICE_TABLE(acpi, bcm2835aux_serial_acpi_match);

+static int bcm2835aux_suspend(struct device *dev)
+{
+   struct bcm2835aux_data *data = dev_get_drvdata(dev);
+   struct uart_8250_port *up = serial8250_get_port(data->line);
+
+   serial8250_suspend_port(data->line);
+
+   if (device_may_wakeup(dev))
+   return 0;
+
+   if (uart_console(&up->port) && !console_suspend_enabled)
+   return 0;
+
+   clk_disable_unprepare(data->clk);
+   return 0;
+}
+
+static int bcm2835aux_resume(struct device *dev)
+{
+   struct bcm2835aux_data *data = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_prepare_enable(data->clk);
+   if (ret)
+   return ret;
+
+   serial8250_resume_port(data->line);
+
+   return 0;
+}
+
+static const struct dev_pm_ops bcm2835aux_dev_pm_ops = {
+   SYSTEM_SLEEP_PM_OPS(bcm2835aux_suspend, bcm2835aux_resume)
+};
+
 static struct platform_driver bcm2835aux_serial_driver = {
.driver = {
.name = "bcm2835-aux-uart",
.of_match_table = bcm2835aux_serial_match,
.acpi_match_table = bcm2835aux_serial_acpi_match,
+   .pm = pm_ptr(&bcm2835aux_dev_pm_ops),
},
.probe  = bcm2835aux_serial_probe,
.remove_new = bcm2835aux_serial_remove,
--
2.34.1

[PATCH V2 14/16] WIP: usb: dwc2: Implement recovery after PM domain off

[Stefan Wahren]

DO NOT MERGE

According to the dt-bindings there are some platforms, which have a
dedicated USB power domain for DWC2 IP core supply. If the power domain
is switched off during system suspend then all USB register will lose
their settings.

So use the power on/off notifier in order to save & restore the USB
registers during system suspend.

Signed-off-by: Stefan Wahren 
---

Any feedback is appreciated.

 drivers/usb/dwc2/core.c | 16 
 drivers/usb/dwc2/core.h | 17 +
 drivers/usb/dwc2/gadget.c   | 49 +
 drivers/usb/dwc2/hcd.c  | 49 +
 drivers/usb/dwc2/platform.c | 32 
 5 files changed, 163 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 9919ab725d54..a3263cfdedac 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -391,6 +391,22 @@ int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, int 
rem_wakeup,
return dwc2_gadget_exit_hibernation(hsotg, rem_wakeup, reset);
 }

+int dwc2_enter_poweroff(struct dwc2_hsotg *hsotg)
+{
+   if (dwc2_is_host_mode(hsotg))
+   return dwc2_host_enter_poweroff(hsotg);
+   else
+   return dwc2_gadget_enter_poweroff(hsotg);
+}
+
+int dwc2_exit_poweroff(struct dwc2_hsotg *hsotg)
+{
+   if (dwc2_is_host_mode(hsotg))
+   return dwc2_host_exit_poweroff(hsotg);
+   else
+   return dwc2_gadget_exit_poweroff(hsotg);
+}
+
 /*
  * Do core a soft reset of the core.  Be careful with this because it
  * resets all the internal state machines of the core.
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2bd74f3033ed..9ab755cc3081 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -9,6 +9,7 @@
 #define __DWC2_CORE_H__

 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1080,6 +1081,8 @@ struct dwc2_hsotg {
struct regulator *vbus_supply;
struct regulator *usb33d;

+   struct notifier_block genpd_nb;
+
spinlock_t lock;
void *priv;
int irq;
@@ -1316,6 +1319,8 @@ int dwc2_exit_partial_power_down(struct dwc2_hsotg 
*hsotg, int rem_wakeup,
 int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg, int is_host);
 int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, int rem_wakeup,
int reset, int is_host);
+int dwc2_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_exit_poweroff(struct dwc2_hsotg *hsotg);
 void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg);
 int dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy);

@@ -1435,6 +1440,8 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg 
*hsotg);
 int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg);
 void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg);
 void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_exit_poweroff(struct dwc2_hsotg *hsotg);
 static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg)
 { hsotg->fifo_map = 0; }
 #else
@@ -1482,6 +1489,10 @@ static inline int 
dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg)
 { return 0; }
 static inline void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg) {}
 static inline void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_gadget_enter_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_gadget_exit_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
 static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg) {}
 #endif

@@ -1505,6 +1516,8 @@ int dwc2_host_exit_partial_power_down(struct dwc2_hsotg 
*hsotg,
 void dwc2_host_enter_clock_gating(struct dwc2_hsotg *hsotg);
 void dwc2_host_exit_clock_gating(struct dwc2_hsotg *hsotg, int rem_wakeup);
 bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2);
+int dwc2_host_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_host_exit_poweroff(struct dwc2_hsotg *hsotg);
 static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg)
 { schedule_work(&hsotg->phy_reset_work); }
 #else
@@ -1544,6 +1557,10 @@ static inline void dwc2_host_exit_clock_gating(struct 
dwc2_hsotg *hsotg,
   int rem_wakeup) {}
 static inline bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2)
 { return false; }
+static inline int dwc2_host_enter_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_host_exit_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
 static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg) {}

 #endif
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e7bf9cc635be..38f0112970fe 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -5710,3 +5710,52 @@ void dwc2_gadget_exit_clock_gating(struct dwc2_hsotg 
*hsotg, int rem_wakeup)
hsotg->lx_state = DWC2_

[PATCH V2 13/16] usb: dwc2: Skip clock gating on Broadcom SoCs

[Stefan Wahren]

On resume of the Raspberry Pi the dwc2 driver fails to enable
HCD_FLAG_HW_ACCESSIBLE before re-enabling the interrupts.
This causes a situation where both handler ignore a incoming port
interrupt and force the upper layers to disable the dwc2 interrupt line.
This leaves the USB interface in a unusable state:

irq 66: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W  6.10.0-rc3
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x50/0x64
dump_stack_lvl from __report_bad_irq+0x38/0xc0
__report_bad_irq from note_interrupt+0x2ac/0x2f4
note_interrupt from handle_irq_event+0x88/0x8c
handle_irq_event from handle_level_irq+0xb4/0x1ac
handle_level_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from bcm2836_chained_handle_irq+0x24/0x28
bcm2836_chained_handle_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from generic_handle_arch_irq+0x34/0x44
generic_handle_arch_irq from __irq_svc+0x88/0xb0
Exception stack(0xc1b01f20 to 0xc1b01f68)
1f20: 0005c0d4 0001   c1b09780 c1d6b32c c1b04e54 c1a5eae8
1f40: c1b04e90    c1d6a8a0 c1b01f70 c11d2da8 c11d4160
1f60: 6013 
__irq_svc from default_idle_call+0x1c/0xb0
default_idle_call from do_idle+0x21c/0x284
do_idle from cpu_startup_entry+0x28/0x2c
cpu_startup_entry from kernel_init+0x0/0x12c
handlers:
[] dwc2_handle_common_intr
[<75cd278b>] usb_hcd_irq
Disabling IRQ #66

Disabling clock gating workaround this issue.

Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.")
Link: 
https://lore.kernel.org/linux-usb/3fd0c2fb-4752-45b3-94eb-42352703e...@gmx.net/T/
Link: 
https://lore.kernel.org/all/5e8cbce0-3260-2971-484f-fc73a3b2b...@synopsys.com/
Signed-off-by: Stefan Wahren 
Acked-by: Minas Harutyunyan 
---
 drivers/usb/dwc2/params.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 4d73fae80b12..68226defdc60 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -23,6 +23,7 @@ static void dwc2_set_bcm_params(struct dwc2_hsotg *hsotg)
p->max_transfer_size = 65535;
p->max_packet_count = 511;
p->ahbcfg = 0x10;
+   p->no_clock_gating = true;
 }

 static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
--
2.34.1

[PATCH V2 12/16] usb: dwc2: Add comment about BCM2848 ACPI ID

[Stefan Wahren]

During recent code review the different naming between ACPI and OF
IDs led to confusion. So add a clarifying comment.

Link: 
https://lore.kernel.org/linux-usb/38e46b44-6248-45e8-bdf9-66008a2fe...@arm.com/
Signed-off-by: Stefan Wahren 
---
 drivers/usb/dwc2/params.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index a937eadbc9b3..4d73fae80b12 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -352,6 +352,7 @@ const struct of_device_id dwc2_of_match_table[] = {
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);

 const struct acpi_device_id dwc2_acpi_match[] = {
+   /* This ID refers to the same USB IP as of_device_id brcm,bcm2835-usb */
{ "BCM2848", (kernel_ulong_t)dwc2_set_bcm_params },
{ },
 };
--
2.34.1

[PATCH V2 11/16] usb: dwc2: debugfs: Print parameter no_clock_gating

[Stefan Wahren]

The commit c4a0f7a6ab54 ("usb: dwc2: Skip clock gating on Samsung
SoCs") introduced a parameter to skip enabling clock gating mode
even the hardware platform should supports it.

In order to make this more visible also print this in show
parameters of debugfs.

Signed-off-by: Stefan Wahren 
Acked-by: Minas Harutyunyan 
Reviewed-by: Florian Fainelli 
---
 drivers/usb/dwc2/debugfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
index 7c82ab590401..3116ac72747f 100644
--- a/drivers/usb/dwc2/debugfs.c
+++ b/drivers/usb/dwc2/debugfs.c
@@ -702,6 +702,7 @@ static int params_show(struct seq_file *seq, void *v)
print_param(seq, p, uframe_sched);
print_param(seq, p, external_id_pin_ctl);
print_param(seq, p, power_down);
+   print_param(seq, p, no_clock_gating);
print_param(seq, p, lpm);
print_param(seq, p, lpm_clock_gating);
print_param(seq, p, besl);
--
2.34.1

[PATCH V2 09/16] drm/vc4: v3d: simplify clock retrieval

[Stefan Wahren]

Common pattern of handling deferred probe can be simplified with
dev_err_probe() and devm_clk_get_optional(). This results in much
less code.

Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 1ede508a67d3..4bf3a8d24770 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -441,20 +441,11 @@ static int vc4_v3d_bind(struct device *dev, struct device 
*master, void *data)
vc4->v3d = v3d;
v3d->vc4 = vc4;

-   v3d->clk = devm_clk_get(dev, NULL);
+   v3d->clk = devm_clk_get_optional(dev, NULL);
if (IS_ERR(v3d->clk)) {
int ret = PTR_ERR(v3d->clk);

-   if (ret == -ENOENT) {
-   /* bcm2835 didn't have a clock reference in the DT. */
-   ret = 0;
-   v3d->clk = NULL;
-   } else {
-   if (ret != -EPROBE_DEFER)
-   dev_err(dev, "Failed to get V3D clock: %d\n",
-   ret);
-   return ret;
-   }
+   return dev_err_probe(dev, ret, "Failed to get V3D clock\n");
}

ret = platform_get_irq(pdev, 0);
--
2.34.1

[PATCH V2 08/16] drm/vc4: hdmi: add PM suspend/resume support

[Stefan Wahren]

Add suspend/resume support for the VC4 HDMI component in order
to handle suspend to idle properly. Since the HDMI power domain
is powered down during suspend, this makes connector status polling
pointless.

Link: 
https://lore.kernel.org/dri-devel/7003512d-7303-4f41-b0d6-a8af5bf8e...@gmx.net/
Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 6611ab7c26a6..f7a4ed16094e 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -3104,6 +3104,31 @@ static int vc5_hdmi_init_resources(struct drm_device 
*drm,
return 0;
 }

+static int vc4_hdmi_suspend(struct device *dev)
+{
+   struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
+
+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_disable(drm);
+
+   return pm_runtime_force_suspend(dev);
+}
+
+static int vc4_hdmi_resume(struct device *dev)
+{
+   struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
+   int ret;
+
+   ret = pm_runtime_force_resume(dev);
+
+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_enable(drm);
+
+   return ret;
+}
+
 static int vc4_hdmi_runtime_suspend(struct device *dev)
 {
struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
@@ -3405,6 +3430,7 @@ static const struct dev_pm_ops vc4_hdmi_pm_ops = {
SET_RUNTIME_PM_OPS(vc4_hdmi_runtime_suspend,
   vc4_hdmi_runtime_resume,
   NULL)
+   SET_SYSTEM_SLEEP_PM_OPS(vc4_hdmi_suspend, vc4_hdmi_resume)
 };

 struct platform_driver vc4_hdmi_driver = {
--
2.34.1

[PATCH V2 07/16] drm/vc4: Get the rid of DRM_ERROR()

[Stefan Wahren]

DRM_ERROR() has been deprecated in favor of pr_err(). However, we
should prefer to use drm_err() whenever possible so we get device-
specific output with the error message. In error case of kcalloc,
we can simply drop DRM_ERROR(), because kcalloc already logs errors.

Suggested-by: Maíra Canal 
Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_bo.c   | 14 ++--
 drivers/gpu/drm/vc4/vc4_dpi.c  | 14 ++--
 drivers/gpu/drm/vc4/vc4_dsi.c  | 32 ++
 drivers/gpu/drm/vc4/vc4_gem.c  | 11 +
 drivers/gpu/drm/vc4/vc4_hdmi.c | 36 +++---
 drivers/gpu/drm/vc4/vc4_hvs.c  |  4 ++--
 drivers/gpu/drm/vc4/vc4_irq.c  |  2 +-
 drivers/gpu/drm/vc4/vc4_v3d.c  |  6 ++---
 drivers/gpu/drm/vc4/vc4_validate.c |  8 +++
 drivers/gpu/drm/vc4/vc4_vec.c  | 10 -
 10 files changed, 70 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 86d629e45307..952953b4cdf8 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -469,7 +469,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t 
unaligned_size,

if (IS_ERR(dma_obj)) {
struct drm_printer p = drm_info_printer(vc4->base.dev);
-   DRM_ERROR("Failed to allocate from GEM DMA helper:\n");
+   drm_err(dev, "Failed to allocate from GEM DMA helper:\n");
vc4_bo_stats_print(&p, vc4);
return ERR_PTR(-ENOMEM);
}
@@ -702,7 +702,7 @@ static struct dma_buf *vc4_prime_export(struct 
drm_gem_object *obj, int flags)
 */
ret = vc4_bo_inc_usecnt(bo);
if (ret) {
-   DRM_ERROR("Failed to increment BO usecnt\n");
+   drm_err(obj->dev, "Failed to increment BO usecnt\n");
return ERR_PTR(ret);
}

@@ -1050,10 +1050,10 @@ static void vc4_bo_cache_destroy(struct drm_device 
*dev, void *unused)

for (i = 0; i < vc4->num_labels; i++) {
if (vc4->bo_labels[i].num_allocated) {
-   DRM_ERROR("Destroying BO cache with %d %s "
- "BOs still allocated\n",
- vc4->bo_labels[i].num_allocated,
- vc4->bo_labels[i].name);
+   drm_err(dev, "Destroying BO cache with %d %s "
+"BOs still allocated\n",
+vc4->bo_labels[i].num_allocated,
+vc4->bo_labels[i].name);
}

if (is_user_label(i))
@@ -1083,7 +1083,7 @@ int vc4_label_bo_ioctl(struct drm_device *dev, void *data,

gem_obj = drm_gem_object_lookup(file_priv, args->handle);
if (!gem_obj) {
-   DRM_ERROR("Failed to look up GEM BO %d\n", args->handle);
+   drm_err(dev, "Failed to look up GEM BO %d\n", args->handle);
kfree(name);
return -ENOENT;
}
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 39152e755a13..688bfddbfb8f 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -199,8 +199,8 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
*encoder)
   DPI_FORMAT);
break;
default:
-   DRM_ERROR("Unknown media bus format %d\n",
- bus_format);
+   drm_err(dev, "Unknown media bus format %d\n",
+bus_format);
break;
}
}
@@ -236,11 +236,11 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
*encoder)

ret = clk_set_rate(dpi->pixel_clock, mode->clock * 1000);
if (ret)
-   DRM_ERROR("Failed to set clock rate: %d\n", ret);
+   drm_err(dev, "Failed to set clock rate: %d\n", ret);

ret = clk_prepare_enable(dpi->pixel_clock);
if (ret)
-   DRM_ERROR("Failed to set clock rate: %d\n", ret);
+   drm_err(dev, "Failed to set clock rate: %d\n", ret);

drm_dev_exit(idx);
 }
@@ -339,7 +339,7 @@ static int vc4_dpi_bind(struct device *dev, struct device 
*master, void *data)
if (IS_ERR(dpi->core_clock)) {
ret = PTR_ERR(dpi->core_clock);
if (ret != -EPROBE_DEFER)
-   DRM_ERROR("Failed to get core clock: %d\n", ret);
+   drm_err(drm, "Failed to get core clock: %d\n", ret);
 

[PATCH V2 06/16] drm/vc4: hdmi: Handle error case of pm_runtime_resume_and_get

[Stefan Wahren]

The commit 0f5251339eda ("drm/vc4: hdmi: Make sure the controller is
powered in detect") introduced the necessary power management handling
to avoid register access while controller is powered down.
Unfortunately it just print a warning if pm_runtime_resume_and_get()
fails and proceed anyway.

This could happen during suspend to idle. So we must assume it is unsafe
to access the HDMI register. So bail out properly.

Fixes: 0f5251339eda ("drm/vc4: hdmi: Make sure the controller is powered in 
detect")
Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d57c4a5948c8..cb424604484f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -429,6 +429,7 @@ static int vc4_hdmi_connector_detect_ctx(struct 
drm_connector *connector,
 {
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
enum drm_connector_status status = connector_status_disconnected;
+   int ret;

/*
 * NOTE: This function should really take vc4_hdmi->mutex, but
@@ -441,7 +442,12 @@ static int vc4_hdmi_connector_detect_ctx(struct 
drm_connector *connector,
 * the lock for now.
 */

-   WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
+   ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev);
+   if (ret) {
+   drm_err_once(connector->dev, "Failed to retain HDMI power 
domain: %d\n",
+ret);
+   return connector_status_unknown;
+   }

if (vc4_hdmi->hpd_gpio) {
if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
--
2.34.1

[PATCH V2 05/16] pmdomain: raspberrypi-power: set flag GENPD_FLAG_ACTIVE_WAKEUP

[Stefan Wahren]

Set flag GENPD_FLAG_ACTIVE_WAKEUP to rpi_power genpd, then when a device
is set as wakeup source using device_set_wakeup_enable, the power
domain could be kept on to make sure the device could wakeup the system.

Signed-off-by: Stefan Wahren 
---
 drivers/pmdomain/bcm/raspberrypi-power.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pmdomain/bcm/raspberrypi-power.c 
b/drivers/pmdomain/bcm/raspberrypi-power.c
index fadedfc9c645..b87ea7adb7be 100644
--- a/drivers/pmdomain/bcm/raspberrypi-power.c
+++ b/drivers/pmdomain/bcm/raspberrypi-power.c
@@ -91,6 +91,7 @@ static void rpi_common_init_power_domain(struct 
rpi_power_domains *rpi_domains,
dom->fw = rpi_domains->fw;

dom->base.name = name;
+   dom->base.flags = GENPD_FLAG_ACTIVE_WAKEUP;
dom->base.power_on = rpi_domain_on;
dom->base.power_off = rpi_domain_off;

--
2.34.1

[PATCH V2 04/16] pmdomain: raspberrypi-power: Add logging to rpi_firmware_set_power

[Stefan Wahren]

The Raspberry Pi power driver heavily relies on the logging of the
underlying firmware driver. This results in disadvantages like unspecific
error messages or limited debugging options. So implement the logging for
the most important function rpi_firmware_set_power.

Signed-off-by: Stefan Wahren 
---
 drivers/pmdomain/bcm/raspberrypi-power.c | 34 ++--
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/pmdomain/bcm/raspberrypi-power.c 
b/drivers/pmdomain/bcm/raspberrypi-power.c
index 39d9a52200c3..fadedfc9c645 100644
--- a/drivers/pmdomain/bcm/raspberrypi-power.c
+++ b/drivers/pmdomain/bcm/raspberrypi-power.c
@@ -48,33 +48,39 @@ struct rpi_power_domain_packet {
  * Asks the firmware to enable or disable power on a specific power
  * domain.
  */
-static int rpi_firmware_set_power(struct rpi_power_domain *rpi_domain, bool on)
+static int rpi_firmware_set_power(struct generic_pm_domain *domain, bool on)
 {
+   struct rpi_power_domain *rpi_domain =
+   container_of(domain, struct rpi_power_domain, base);
+   bool old_interface = rpi_domain->old_interface;
struct rpi_power_domain_packet packet;
+   int ret;

packet.domain = rpi_domain->domain;
packet.state = on;
-   return rpi_firmware_property(rpi_domain->fw,
-rpi_domain->old_interface ?
-RPI_FIRMWARE_SET_POWER_STATE :
-RPI_FIRMWARE_SET_DOMAIN_STATE,
-&packet, sizeof(packet));
+
+   ret = rpi_firmware_property(rpi_domain->fw, old_interface ?
+   RPI_FIRMWARE_SET_POWER_STATE :
+   RPI_FIRMWARE_SET_DOMAIN_STATE,
+   &packet, sizeof(packet));
+   if (ret)
+   dev_err(&domain->dev, "Failed to set %s to %u (%d)\n",
+   old_interface ? "power" : "domain", on, ret);
+   else
+   dev_dbg(&domain->dev, "Set %s to %u\n",
+   old_interface ? "power" : "domain", on);
+
+   return ret;
 }

 static int rpi_domain_off(struct generic_pm_domain *domain)
 {
-   struct rpi_power_domain *rpi_domain =
-   container_of(domain, struct rpi_power_domain, base);
-
-   return rpi_firmware_set_power(rpi_domain, false);
+   return rpi_firmware_set_power(domain, false);
 }

 static int rpi_domain_on(struct generic_pm_domain *domain)
 {
-   struct rpi_power_domain *rpi_domain =
-   container_of(domain, struct rpi_power_domain, base);
-
-   return rpi_firmware_set_power(rpi_domain, true);
+   return rpi_firmware_set_power(domain, true);
 }

 static void rpi_common_init_power_domain(struct rpi_power_domains *rpi_domains,
--
2.34.1

[PATCH V2 03/16] pmdomain: raspberrypi-power: Adjust packet definition

[Stefan Wahren]

According to the official Mailbox property interface the second part
of RPI_FIRMWARE_SET_POWER_STATE ( and so on ...) is named state because
it represent u32 flags and just the lowest bit is for on/off. So rename it
to align with documentation and prepare the driver for further changes.

Link: https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface
Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 
---
 drivers/pmdomain/bcm/raspberrypi-power.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pmdomain/bcm/raspberrypi-power.c 
b/drivers/pmdomain/bcm/raspberrypi-power.c
index 06196ebfe03b..39d9a52200c3 100644
--- a/drivers/pmdomain/bcm/raspberrypi-power.c
+++ b/drivers/pmdomain/bcm/raspberrypi-power.c
@@ -41,7 +41,7 @@ struct rpi_power_domains {
  */
 struct rpi_power_domain_packet {
u32 domain;
-   u32 on;
+   u32 state;
 };

 /*
@@ -53,7 +53,7 @@ static int rpi_firmware_set_power(struct rpi_power_domain 
*rpi_domain, bool on)
struct rpi_power_domain_packet packet;

packet.domain = rpi_domain->domain;
-   packet.on = on;
+   packet.state = on;
return rpi_firmware_property(rpi_domain->fw,
 rpi_domain->old_interface ?
 RPI_FIRMWARE_SET_POWER_STATE :
@@ -142,13 +142,13 @@ rpi_has_new_domain_support(struct rpi_power_domains 
*rpi_domains)
int ret;

packet.domain = RPI_POWER_DOMAIN_ARM;
-   packet.on = ~0;
+   packet.state = ~0;

ret = rpi_firmware_property(rpi_domains->fw,
RPI_FIRMWARE_GET_DOMAIN_STATE,
&packet, sizeof(packet));

-   return ret == 0 && packet.on != ~0;
+   return ret == 0 && packet.state != ~0;
 }

 static int rpi_power_probe(struct platform_device *pdev)
--
2.34.1

[PATCH V2 02/16] mailbox: bcm2835: Fix timeout during suspend mode

[Stefan Wahren]

During noirq suspend phase the Raspberry Pi power driver suffer of
firmware property timeouts. The reason is that the IRQ of the underlying
BCM2835 mailbox is disabled and rpi_firmware_property_list() will always
run into a timeout [1].

Since the VideoCore side isn't consider as a wakeup source, set the
IRQF_NO_SUSPEND flag for the mailbox IRQ in order to keep it enabled
during suspend-resume cycle.

[1]
PM: late suspend of devices complete after 1.754 msecs
WARNING: CPU: 0 PID: 438 at drivers/firmware/raspberrypi.c:128
 rpi_firmware_property_list+0x204/0x22c
Firmware transaction 0x00028001 timeout
Modules linked in:
CPU: 0 PID: 438 Comm: bash Tainted: G C 6.9.3-dirty #17
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x34/0x44
dump_stack_lvl from __warn+0x88/0xec
__warn from warn_slowpath_fmt+0x7c/0xb0
warn_slowpath_fmt from rpi_firmware_property_list+0x204/0x22c
rpi_firmware_property_list from rpi_firmware_property+0x68/0x8c
rpi_firmware_property from rpi_firmware_set_power+0x54/0xc0
rpi_firmware_set_power from _genpd_power_off+0xe4/0x148
_genpd_power_off from genpd_sync_power_off+0x7c/0x11c
genpd_sync_power_off from genpd_finish_suspend+0xcc/0xe0
genpd_finish_suspend from dpm_run_callback+0x78/0xd0
dpm_run_callback from device_suspend_noirq+0xc0/0x238
device_suspend_noirq from dpm_suspend_noirq+0xb0/0x168
dpm_suspend_noirq from suspend_devices_and_enter+0x1b8/0x5ac
suspend_devices_and_enter from pm_suspend+0x254/0x2e4
pm_suspend from state_store+0xa8/0xd4
state_store from kernfs_fop_write_iter+0x154/0x1a0
kernfs_fop_write_iter from vfs_write+0x12c/0x184
vfs_write from ksys_write+0x78/0xc0
ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0xcc93dfa8 to 0xcc93dff0)
[...]
PM: noirq suspend of devices complete after 3095.584 msecs

Link: https://github.com/raspberrypi/firmware/issues/1894
Fixes: 0bae6af6d704 ("mailbox: Enable BCM2835 mailbox support")
Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 
---
 drivers/mailbox/bcm2835-mailbox.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/bcm2835-mailbox.c 
b/drivers/mailbox/bcm2835-mailbox.c
index fbfd0202047c..ea12fb8d2401 100644
--- a/drivers/mailbox/bcm2835-mailbox.c
+++ b/drivers/mailbox/bcm2835-mailbox.c
@@ -145,7 +145,8 @@ static int bcm2835_mbox_probe(struct platform_device *pdev)
spin_lock_init(&mbox->lock);

ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
-  bcm2835_mbox_irq, 0, dev_name(dev), mbox);
+  bcm2835_mbox_irq, IRQF_NO_SUSPEND, dev_name(dev),
+  mbox);
if (ret) {
dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
ret);
--
2.34.1

[PATCH V2 01/16] firmware: raspberrypi: Improve timeout warning

[Stefan Wahren]

Recent work on raspberry-power driver showed that even the
stacktrace on firmware property timeout doesn't provide
enough information. So add the first tag name to the warning
to be in line with a status error.

Signed-off-by: Stefan Wahren 
Reviewed-by: Florian Fainelli 
---
 drivers/firmware/raspberrypi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index ac34876a97f8..18cc34987108 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -62,7 +62,6 @@ rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, 
u32 data)
ret = 0;
} else {
ret = -ETIMEDOUT;
-   WARN_ONCE(1, "Firmware transaction timeout");
}
} else {
dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
@@ -125,6 +124,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
buf[2], buf[1]);
ret = -EINVAL;
+   } else if (ret == -ETIMEDOUT) {
+   WARN_ONCE(1, "Firmware transaction 0x%08x timeout", buf[2]);
}

dma_free_coherent(fw->chan->mbox->dev, PAGE_ALIGN(size), buf, bus_addr);
--
2.34.1

[PATCH V2 00/16] ARM: bcm2835: Implement initial S2Idle for Raspberry Pi

[Stefan Wahren]

This series implement the initial S2Idle support for
the Raspberry Pi, which was a long time on my TODO list [1]. The
changes allow to suspend and resume the Raspberry Pi via debug UART.
The focus is on the BCM2835 SoC, because it's less complex than its
successors and have enough documentation.

The series can be roughly separated in 3 parts:
 1. base patches (1-5, 15, 16) which allows S2Idle support for BCM2835
 2. drm vc4 patches which implement S2Idle support (mostly new and not
really BCM2835 specific)
 3. dwc2 patches which handle BCM2835 specific issues (still in progress,
but works fine)

Cherry-picking of patches should be fine.

Test steps:
- configure debug console (pl011 or mini UART) as wakeup source
- send system to idle state

  echo freeze > /sys/power/state

- wakeup system by console traffic

The DWC2 part of the implementation isn't complete yet (especially patch 14),
but much better than the first version. The USB domain is now powered down and
the USB devices are still usable after resume. There is still room for
improvements, but at least the system won't freeze forever as before [2].

Here are some figures for the Raspberry Pi 1 (without any
devices connected except of a debug UART):

running but CPU idle = 1.67 W
S2Idle   = 1.33 W

In comparison with HDMI & USB keyboard connected (but neither active
nor wakeup source):

running but CPU idle = 1.82 W
S2Idle   = 1.33 W

The series has been successfully tested on the following platforms:
Raspberry Pi 1 B
Raspberry Pi 3 B+

Changes in V2:
- rebased against todays mainline
- added Reviewed-by from Florian
- added Acked-by from Minas
- dropped "irqchip/bcm2835: Enable SKIP_SET_WAKE and MASK_ON_SUSPEND"
  because it has been applied by Thomas Gleixner
- dropped "pmdomain: raspberrypi-power: Avoid powering down USB"
  because this workaround has been replaced by patch 14
- use drm_err_once instead of DRM_ERROR and return connector_status_unknown
  in patch 6
- add new patch in order to clean-up all DRM_ERROR
- add new patch to improve raspberrypi-power logging
- add new patch to simplify V3D clock retrieval
- add new patch 5 to avoid power down of wakeup devices
- add new patch 12 to avoid confusion about ACPI ID of BCM283x USB
- add new patch 8 & 10 which address the problem that HDMI
  is not functional after s2idle
- add more links and fix typo in patch 13
- add new WIP patch 14 which recover DWC2 register after power down
- take care of UART clock in patch 15 as commented by Florian
- use SYSTEM_SLEEP_PM_OPS in patch 15

[1] - https://github.com/lategoodbye/rpi-zero/issues/9
[2] - https://bugzilla.redhat.com/show_bug.cgi?id=2283978

Stefan Wahren (16):
  firmware: raspberrypi: Improve timeout warning
  mailbox: bcm2835: Fix timeout during suspend mode
  pmdomain: raspberrypi-power: Adjust packet definition
  pmdomain: raspberrypi-power: Add logging to rpi_firmware_set_power
  pmdomain: raspberrypi-power: set flag GENPD_FLAG_ACTIVE_WAKEUP
  drm/vc4: hdmi: Handle error case of pm_runtime_resume_and_get
  drm/vc4: Get the rid of DRM_ERROR()
  drm/vc4: hdmi: add PM suspend/resume support
  drm/vc4: v3d: simplify clock retrieval
  drm/vc4: v3d: add PM suspend/resume support
  usb: dwc2: debugfs: Print parameter no_clock_gating
  usb: dwc2: Add comment about BCM2848 ACPI ID
  usb: dwc2: Skip clock gating on Broadcom SoCs
  WIP: usb: dwc2: Implement recovery after PM domain off
  serial: 8250_bcm2835aux: add PM suspend/resume support
  ARM: bcm2835_defconfig: Enable SUSPEND

 arch/arm/configs/bcm2835_defconfig|  2 -
 drivers/firmware/raspberrypi.c|  3 +-
 drivers/gpu/drm/vc4/vc4_bo.c  | 14 ++---
 drivers/gpu/drm/vc4/vc4_dpi.c | 14 ++---
 drivers/gpu/drm/vc4/vc4_dsi.c | 32 ++-
 drivers/gpu/drm/vc4/vc4_gem.c | 11 ++--
 drivers/gpu/drm/vc4/vc4_hdmi.c| 70 +--
 drivers/gpu/drm/vc4/vc4_hvs.c |  4 +-
 drivers/gpu/drm/vc4/vc4_irq.c |  2 +-
 drivers/gpu/drm/vc4/vc4_v3d.c | 21 +++
 drivers/gpu/drm/vc4/vc4_validate.c|  8 +--
 drivers/gpu/drm/vc4/vc4_vec.c | 10 ++--
 drivers/mailbox/bcm2835-mailbox.c |  3 +-
 drivers/pmdomain/bcm/raspberrypi-power.c  | 43 --
 drivers/tty/serial/8250/8250_bcm2835aux.c | 37 
 drivers/usb/dwc2/core.c   | 16 ++
 drivers/usb/dwc2/core.h   | 17 ++
 drivers/usb/dwc2/debugfs.c|  1 +
 drivers/usb/dwc2/gadget.c | 49 
 drivers/usb/dwc2/hcd.c| 49 
 drivers/usb/dwc2/params.c |  2 +
 drivers/usb/dwc2/platform.c   | 32 +++
 22 files changed, 339 insertions(+), 101 deletions(-)

--
2.34.1

Re: [PATCH 06/11] drm/vc4: hdmi: Handle error case of pm_runtime_resume_and_get

[Stefan Wahren]

Hello,

Am 30.06.24 um 17:36 schrieb Stefan Wahren:

The commit 0f5251339eda ("drm/vc4: hdmi: Make sure the controller is
powered in detect") introduced the necessary power management handling
to avoid register access while controller is powered down.
Unfortunately it just print a warning if pm_runtime_resume_and_get()
fails and proceed anyway.

This could happen during suspend to idle. So we must assume it is unsafe
to access the HDMI register. So bail out properly.

Fixes: 0f5251339eda ("drm/vc4: hdmi: Make sure the controller is powered in 
detect")
Signed-off-by: Stefan Wahren 
---
  drivers/gpu/drm/vc4/vc4_hdmi.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d57c4a5948c8..b3a42b709718 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -429,6 +429,7 @@ static int vc4_hdmi_connector_detect_ctx(struct 
drm_connector *connector,
  {
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
enum drm_connector_status status = connector_status_disconnected;
+   int ret;

/*
 * NOTE: This function should really take vc4_hdmi->mutex, but
@@ -441,7 +442,11 @@ static int vc4_hdmi_connector_detect_ctx(struct 
drm_connector *connector,
 * the lock for now.
 */

-   WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
+   ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev);
+   if (ret) {
+   DRM_ERROR("Failed to retain HDMI power domain: %d\n", ret);
+   return status;

I noticed today that the enum drm_connector_status also supports
connector_status_unknown. Wouldn't this be a more appropriate return
value in this error case?

Why isn't status initialized with connector_status_unknown at all?

Best regards

+   }

if (vc4_hdmi->hpd_gpio) {
if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
--
2.34.1

Re: [PATCH 09/11] usb: dwc2: Skip clock gating on Broadcom SoCs

[Stefan Wahren]

Am 05.07.24 um 23:14 schrieb Lukas Wunner:

On Fri, Jul 05, 2024 at 12:16:14PM -0500, Jeremy Linton wrote:

Am 05.07.24 um 17:03 schrieb Lukas Wunner:

Careful there, the patch vaguely says...

 With that added and identified as "BCM2848",
 an id in use by other OSs for this device, the dw2
 controller on the BCM2711 will work.

...which sounds like they copy-pasted the BCM2848 id from somewhere else.
I would assume that BCM2848 is really a different SoC and not just
a different name for the BCM2835, but hopefully BroadCom folks will
be able to confirm or deny this (and thus the necessity of the quirk
on BCM2848 and not just on BCM2835).

This id comes from the edk2-platforms ACPI tables and is currently used by
both the rpi3 and rpi4, and AFAIK nothing else as the rpi5-dev work is
currently only exposing XHCI.

The ID is strictly the USB controller not the SoC. Its a bit confusingly
named, but something we inherited from the much older windows/edk2 port,
where it appears that the peripheral HID's were just picked in numerical
order.

[0] 
https://github.com/tianocore/edk2-platforms/blob/12f68d29abdc9d703f67bd743fdec23ebb1e966e/Platform/RaspberryPi/AcpiTables/GpuDevs.asl#L15

So BCM2848, BCM2849, BCM2850 and so on are just made-up IDs
for a Windows/EDK2 port that got cargo-culted into the kernel?
Yikes!

Has anyone checked whether they collide with actual Broadcom products?

Using public available information like this [1], I wasn't able to find
any collision.

[1] - https://github.com/anholt/linux/wiki/Devices-with-Videocore-graphics

Re: vc4: Increased CMA usage after replace drm_gem_dma_object for drm_gem_object in vc4_exec_info

[Stefan Wahren]

Hi,

Am 05.07.24 um 17:19 schrieb Daniel Vetter:

Adding dri-devel

On Fri, 5 Jul 2024 at 17:14, Stefan Wahren  wrote:

Hi,
last year my Raspberry Pi 3B Plus died, so i didn't noticed this
sooner.
If I ran Raspberry Pi OS Bullseye with X11 and a recent Mainline
Kernel
(arm/multi_v7_defconfig) on my new Raspberry Pi 3 B+, i'm getting the
following errors:

[    0.00] Booting Linux on physical CPU 0x0
[    0.00] Linux version 6.10.0-rc1-g685505219723
(stefanw@stefanw-SCHENKER) (arm-linux-gnueabihf-gcc (GCC) 11.3.1
20220604 [releases/gcc-11 revision
591c0f4b92548e3ae2e8173f4f93984b1c7f62bb], GNU ld
(Linaro_Binutils-2022.06) 2.37.20220122) #1 SMP Wed Jul  3
19:57:14 CEST
2024
[    0.00] CPU: ARMv7 Processor [410fd034] revision 4 (ARMv7),
cr=10c5383d
[    0.00] CPU: div instructions available: patching division code
[    0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[    0.00] OF: fdt: Machine model: Raspberry Pi 3 Model B Plus
Rev 1.3
[    0.00] random: crng init done
[    0.00] Memory policy: Data cache writealloc
[    0.00] efi: UEFI not found.
[    0.00] Reserved memory: created CMA memory pool at 0x3700,
size 64 MiB
[    0.00] OF: reserved mem: initialized node linux,cma,
compatible
id shared-dma-pool
[    0.00] OF: reserved mem: 0x3700..0x3aff (65536
KiB) map
reusable linux,cma
[    0.00] Zone ranges:
[    0.00]   DMA  [mem 0x-0x2fff]
[    0.00]   Normal   empty
[    0.00]   HighMem  [mem 0x3000-0x3b3f]
[    0.00] Movable zone start for each node
[    0.00] Early memory node ranges
[    0.00]   node   0: [mem 0x-0x3b3f]
[    0.00] Initmem setup node 0 [mem
0x-0x3b3f]
[    0.00] percpu: Embedded 17 pages/cpu s40404 r8192 d21036
u69632
[    0.00] pcpu-alloc: s40404 r8192 d21036 u69632 alloc=17*4096
[    0.00] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3
[    0.00] Kernel command line: video=HDMI-A-1:1920x1200M@60
dma.dmachans=0x7ff5 bcm2709.boardrev=0xa020d3
bcm2709.serial=0x51972538
bcm2709.uart_clock=4800 bcm2709.disk_led_gpio=29
bcm2709.disk_led_active_low=0 vc_mem.mem_base=0x3ec0
vc_mem.mem_size=0x4000 console=ttyS1,115200 console=tty1
root=PARTUUID=ecfed651-02 rootfstype=ext4 fsck.repair=yes rootwait
[    0.00] Dentry cache hash table entries: 131072 (order: 7,
524288
bytes, linear)
[    0.00] Inode-cache hash table entries: 65536 (order: 6, 262144
bytes, linear)
[    0.00] Built 1 zonelists, mobility grouping on. Total
pages: 242688
[    0.00] mem auto-init: stack:off, heap alloc:off, heap
free:off,
mlocked free:off
[    0.00] Memory: 868880K/970752K available (15360K kernel code,
2535K rwdata, 6708K rodata, 2048K init, 426K bss, 36336K reserved,
65536K cma-reserved, 118784K highmem)
...
[   56.187787] [drm:vc4_bo_create [vc4]] *ERROR* Failed to
allocate from
GEM DMA helper:
[   56.187870] vc4-drm soc:gpu: [drm] V3D:  36316kb BOs (40)
[   56.187884] vc4-drm soc:gpu: [drm] V3D shader:
124kb BOs (31)
[   56.187893] vc4-drm soc:gpu: [drm] dumb:   4564kb BOs (5)
[   56.187900] vc4-drm soc:gpu: [drm] binner:  16384kb BOs (1)
[   56.187909] vc4-drm soc:gpu: [drm]    total purged BO:
19540kb BOs (15)
[   56.188269] [drm:vc4_bo_create [vc4]] *ERROR* Failed to
allocate from
GEM DMA helper:
[   56.188315] vc4-drm soc:gpu: [drm] V3D:  35072kb BOs (30)
[   56.188325] vc4-drm soc:gpu: [drm] V3D shader:
124kb BOs (31)
[   56.188337] vc4-drm soc:gpu: [drm] dumb:   4564kb BOs (5)
[   56.188345] vc4-drm soc:gpu: [drm] binner:  16384kb BOs (1)
[   56.188354] vc4-drm soc:gpu: [drm]    total purged BO:
19540kb BOs (15)
...

Except of these errors, X11 works as expected. Since I knew that this
setup shouldn't throw these errors without CMA modifications, I
bisected
this to this commit:

commit 47c07e46c86f310bed73b9c895155a49df3d5e71 (HEAD)
Author: Maíra Canal 
Date:   Thu Feb 2 08:19:43 2023 -0300

drm/vc4: replace drm_gem_dma_object for drm_gem_object in
vc4_exec_info

After that I increased the CMA to 128 MB via Kernel cmdline and
compared
/proc/meminfo before and after the patch:

Before
CmaTotal: 131072 kB
CmaFree:   20132 kB

After
CmaTotal: 131072 kB
CmaFree:    5644 kB

So my question: is this noticable CMA increase expected by this
change?


No, it shouldn't change anything. All it does is change the type we
use to iterate over objects from

Re: [PATCH 09/11] usb: dwc2: Skip clock gating on Broadcom SoCs

[Stefan Wahren]

Hi Jeremy,

Am 05.07.24 um 17:03 schrieb Lukas Wunner:

On Fri, Jul 05, 2024 at 12:22:33PM +0200, Stefan Wahren wrote:

Am 05.07.24 um 10:48 schrieb Lukas Wunner:

The real question is whether BCM2848 platforms likewise cannot disable
the clock of the dwc2 controller or whether this is specific to the
BCM2835.  Right now dwc2_set_bcm_params() is applied to both the
BCM2848 and BCM2835.  If the BCM2848 behaves differently in this
regard, we'd have to duplicate dwc2_set_bcm_params() for the BCM2835.

 From my understand BCM2848 refers to the same SoC, but the ACPI
implementation uses a different ID [2]. So I think this is safe.
[2] -
https://patches.linaro.org/project/linux-usb/patch/20210413215834.3126447-2-jeremy.lin...@arm.com/

Careful there, the patch vaguely says...

 With that added and identified as "BCM2848",
 an id in use by other OSs for this device, the dw2
 controller on the BCM2711 will work.

...which sounds like they copy-pasted the BCM2848 id from somewhere else.
I would assume that BCM2848 is really a different SoC and not just
a different name for the BCM2835, but hopefully BroadCom folks will
be able to confirm or deny this (and thus the necessity of the quirk
on BCM2848 and not just on BCM2835).

could you please clarify this situation?

Thanks


Thanks,

Lukas

Re: [PATCH 09/11] usb: dwc2: Skip clock gating on Broadcom SoCs

[Stefan Wahren]

Am 05.07.24 um 10:48 schrieb Lukas Wunner:

On Thu, Jul 04, 2024 at 03:14:50PM +0100, Florian Fainelli wrote:

On 6/30/2024 4:36 PM, Stefan Wahren wrote:

On resume of the Raspberry Pi the dwc2 driver fails to enable
HCD_FLAG_HW_ACCESSIBLE before re-enabling the interrupts.
This causes a situation where both handler ignore a incoming port
interrupt and force the upper layers to disable the dwc2 interrupt line.
This leaves the USB interface in a unusable state:

irq 66: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W  6.10.0-rc3
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x50/0x64
dump_stack_lvl from __report_bad_irq+0x38/0xc0
__report_bad_irq from note_interrupt+0x2ac/0x2f4
note_interrupt from handle_irq_event+0x88/0x8c
handle_irq_event from handle_level_irq+0xb4/0x1ac
handle_level_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from bcm2836_chained_handle_irq+0x24/0x28
bcm2836_chained_handle_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from generic_handle_arch_irq+0x34/0x44
generic_handle_arch_irq from __irq_svc+0x88/0xb0

A similar issue was reported for Agilex platforms back in 2021:

https://lore.kernel.org/all/5e8cbce0-3260-2971-484f-fc73a3b2b...@synopsys.com/

It was fixed by commit 3d8d3504d233 ("usb: dwc2: Add platform specific
data for Intel's Agilex"), which sets the no_clock_gating flag on that
platform.

Looking at drivers/usb/dwc2/params.c, numerous other platforms need
the same flag.

Please amend the commit message to mention the Agilex issue and
resulting commit.

From my understanding Samsung noticed this issue at first and
introduced the no_clock_gating flag [1] and they referenced 0112b7ce68ea
("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") as I did in
this patch. Later some platforms like Rockchip and Agilex followed.

Should i better refer to the Samsung bugfix instead of the Agilex bugfix?




--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -23,6 +23,7 @@ static void dwc2_set_bcm_params(struct dwc2_hsotg *hsotg)
p->max_transfer_size = 65535;
p->max_packet_count = 511;
p->ahbcfg = 0x10;
+   p->no_clock_gating = true;

Could we set this depending upon whether the dwc2 host controller is a
wake-up source for the system or not?

The flag seems to mean whether the platform is actually capable of
disabling the clock of the USB controller.  BCM2835 seems to be
incapable and as a result, even though dwc2_host_enter_clock_gating()
is called, the chip signals interrupts.

That's why I was asking about this in the initial bug report. Since I
didn't get a reply, I submitted this workaround.

There doesn't seem to be a relation to using the controller as a
wakeup source, so your comment doesn't seem to make sense.
If the clock can't be gated, the chip can always serve as a
wakeup source.

I wouldn't conclude that the chip can always serve as a wakeup source
(e.g. power down the USB domain would also prevent this), but i agree
creating a relation between wakeup source and clock gating is a bad idea.

The real question is whether BCM2848 platforms likewise cannot disable
the clock of the dwc2 controller or whether this is specific to the
BCM2835.  Right now dwc2_set_bcm_params() is applied to both the
BCM2848 and BCM2835.  If the BCM2848 behaves differently in this
regard, we'd have to duplicate dwc2_set_bcm_params() for the BCM2835.

From my understand BCM2848 refers to the same SoC, but the ACPI
implementation uses a different ID [2]. So I think this is safe.


Thanks,

Lukas


[1] -
https://lore.kernel.org/linux-usb/20210716050127.4406-1-m.szyprow...@samsung.com/
[2] -
https://patches.linaro.org/project/linux-usb/patch/20210413215834.3126447-2-jeremy.lin...@arm.com/

Re: [PATCH 07/11] drm/vc4: hdmi: Disable connector status polling during suspend

[Stefan Wahren]

Hi Peter,

Am 02.07.24 um 22:02 schrieb Peter Robinson:

Hi Stefan,


Suspend of VC4 HDMI will likely triggers a warning from
vc4_hdmi_connector_detect_ctx() during poll of connector status.
The power management will prevent the resume and keep the relevant
power domain disabled.

Since there is no reason to poll the connector status during
suspend, the polling should be disabled during this.

What about HDMI-CEC? I don't know well enough how CEC integrates at
this level but CEC can wake up the device over HDMI from a TV display
for example so if this affects that, while it's maybe not required for
first pass, I know the rpi is used in a lot of media use cases so the
ability to wake up via CEC would certainly be welcomed.

unfortunately i don't know much about HDMI-CEC, too. The only thing I
noticed was that I currently cannot configure vc4 as a wakeup source.

Re: [PATCH RFT 10/11] serial: 8250_bcm2835aux: add PM suspend/resume support

[Stefan Wahren]

Hi Florian,

Am 04.07.24 um 16:12 schrieb Florian Fainelli:



On 6/30/2024 5:53 PM, Stefan Wahren wrote:

This adds suspend/resume support for the 8250_bcm2835aux
driver to provide power management support on attached
devices.

Signed-off-by: Stefan Wahren 
---

Since i don't have a RS485 setup, any test feedback would be great.

  drivers/tty/serial/8250/8250_bcm2835aux.c | 23 +++
  1 file changed, 23 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c
b/drivers/tty/serial/8250/8250_bcm2835aux.c
index 121a5ce86050..cccd2a09cb6f 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -213,11 +213,34 @@ static const struct acpi_device_id
bcm2835aux_serial_acpi_match[] = {
  };
  MODULE_DEVICE_TABLE(acpi, bcm2835aux_serial_acpi_match);

+static int __maybe_unused bcm2835aux_suspend(struct device *dev)
+{
+    struct bcm2835aux_data *data = dev_get_drvdata(dev);
+
+    serial8250_suspend_port(data->line);


Don't you also need to disable the clock here, unless the device is a
wake-up source, and conversely re-enable the clock upon resume?

at first I experiment with the pm implementation from 8250_uniphier.c,
but this didn't work as soon as I drop "no_console_suspend" from the
Kernel cmdline. Maybe that's the wrong pattern.

Re: [PATCH 09/11] usb: dwc2: Skip clock gating on Broadcom SoCs

[Stefan Wahren]

Hi Florian,

Am 04.07.24 um 16:14 schrieb Florian Fainelli:



On 6/30/2024 4:36 PM, Stefan Wahren wrote:

On resume of the Raspberry Pi the dwc2 driver fails to enable
HCD_FLAG_HW_ACCESSIBLE before re-enabling the interrupts.
This causes a situation where both handler ignore a incoming port
interrupt and force the upper layers to disable the dwc2 interrupt line.
This leaves the USB interface in a unusable state:

irq 66: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W  6.10.0-rc3
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x50/0x64
dump_stack_lvl from __report_bad_irq+0x38/0xc0
__report_bad_irq from note_interrupt+0x2ac/0x2f4
note_interrupt from handle_irq_event+0x88/0x8c
handle_irq_event from handle_level_irq+0xb4/0x1ac
handle_level_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from bcm2836_chained_handle_irq+0x24/0x28
bcm2836_chained_handle_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from generic_handle_arch_irq+0x34/0x44
generic_handle_arch_irq from __irq_svc+0x88/0xb0
Exception stack(0xc1b01f20 to 0xc1b01f68)
1f20: 0005c0d4 0001   c1b09780 c1d6b32c c1b04e54
c1a5eae8
1f40: c1b04e90    c1d6a8a0 c1b01f70 c11d2da8
c11d4160
1f60: 6013 
__irq_svc from default_idle_call+0x1c/0xb0
default_idle_call from do_idle+0x21c/0x284
do_idle from cpu_startup_entry+0x28/0x2c
cpu_startup_entry from kernel_init+0x0/0x12c
handlers:
[] dwc2_handle_common_intr
[<75cd278b>] usb_hcd_irq
Disabling IRQ #66

Disabling clock gatling workaround this issue.


Typo: gatling/gating.



Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr
function.")
Link:
https://lore.kernel.org/linux-usb/3fd0c2fb-4752-45b3-94eb-42352703e...@gmx.net/T/
Signed-off-by: Stefan Wahren 
---
  drivers/usb/dwc2/params.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 5a1500d0bdd9..66580de52882 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -23,6 +23,7 @@ static void dwc2_set_bcm_params(struct dwc2_hsotg
*hsotg)
  p->max_transfer_size = 65535;
  p->max_packet_count = 511;
  p->ahbcfg = 0x10;
+    p->no_clock_gating = true;


Could we set this depending upon whether the dwc2 host controller is a
wake-up source for the system or not?


I would prefer to fix the suspend/resume behavior reported here [1]
instead of making tricky workarounds. But i don't have an idea how to
achieve this.

[1] -
https://lore.kernel.org/linux-usb/3fd0c2fb-4752-45b3-94eb-42352703e...@gmx.net/

Re: [PATCH 07/11] drm/vc4: hdmi: Disable connector status polling during suspend

[Stefan Wahren]

Am 03.07.24 um 12:28 schrieb Stefan Wahren:

Hi Maxime,

Am 02.07.24 um 15:48 schrieb Maxime Ripard:

Hi,

On Sun, Jun 30, 2024 at 05:36:48PM GMT, Stefan Wahren wrote:

Suspend of VC4 HDMI will likely triggers a warning from
vc4_hdmi_connector_detect_ctx() during poll of connector status.
The power management will prevent the resume and keep the relevant
power domain disabled.

Since there is no reason to poll the connector status during
suspend, the polling should be disabled during this.

It not possible to use drm_mode_config_helper_suspend() here,
because the callbacks might be called during bind phase and not all
components are fully initialized.

Link:
https://lore.kernel.org/dri-devel/7003512d-7303-4f41-b0d6-a8af5bf8e...@gmx.net/
Signed-off-by: Stefan Wahren 
---
  drivers/gpu/drm/vc4/vc4_hdmi.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c
b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b3a42b709718..e80495cea6ac 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -3106,6 +3106,13 @@ static int vc5_hdmi_init_resources(struct
drm_device *drm,
  static int vc4_hdmi_runtime_suspend(struct device *dev)
  {
  struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+    struct drm_device *drm = vc4_hdmi->connector.dev;
+
+    /*
+ * Don't disable polling if it was never initialized
+ */
+    if (drm && drm->mode_config.poll_enabled)
+    drm_kms_helper_poll_disable(drm);

Does it make sense to add it to runtime_suspend?

i saw that other drm drivers used drm_mode_config_helper_suspend() in
the RUNTIME_PM_OPS. But i agree, it should be better moved to
SYSTEM_SLEEP_PM_OPS.

What if the board boots without a display connected, and only after a
while one is connected? Wouldn't that prevent the driver from detecting
it?

tbh I noticed that HDMI re-detection didn't worked in my setup
6.10-rcX before this series. I need to investigate ...

Okay, this patch breaks definitely HDMI re-detection. So please ignore
this patch. Sorry, about this mess.

There is another minor issue which already exists before that cause the
following log entry on HDMI reconnect:

[   74.078745] vc4-drm soc:gpu: [drm] User-defined mode not supported:
"1920x1200": 60 154000 1920 1968 2000 2080 1200 1203 1209 1235 0x68 0x9

But in this case HDMI works.

Regards


Maxime

Re: [PATCH 07/11] drm/vc4: hdmi: Disable connector status polling during suspend

[Stefan Wahren]

Hi Maxime,

Am 02.07.24 um 15:48 schrieb Maxime Ripard:

Hi,

On Sun, Jun 30, 2024 at 05:36:48PM GMT, Stefan Wahren wrote:

Suspend of VC4 HDMI will likely triggers a warning from
vc4_hdmi_connector_detect_ctx() during poll of connector status.
The power management will prevent the resume and keep the relevant
power domain disabled.

Since there is no reason to poll the connector status during
suspend, the polling should be disabled during this.

It not possible to use drm_mode_config_helper_suspend() here,
because the callbacks might be called during bind phase and not all
components are fully initialized.

Link: 
https://lore.kernel.org/dri-devel/7003512d-7303-4f41-b0d6-a8af5bf8e...@gmx.net/
Signed-off-by: Stefan Wahren 
---
  drivers/gpu/drm/vc4/vc4_hdmi.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b3a42b709718..e80495cea6ac 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -3106,6 +3106,13 @@ static int vc5_hdmi_init_resources(struct drm_device 
*drm,
  static int vc4_hdmi_runtime_suspend(struct device *dev)
  {
struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
+
+   /*
+* Don't disable polling if it was never initialized
+*/
+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_disable(drm);

Does it make sense to add it to runtime_suspend?

i saw that other drm drivers used drm_mode_config_helper_suspend() in
the RUNTIME_PM_OPS. But i agree, it should be better moved to
SYSTEM_SLEEP_PM_OPS.

What if the board boots without a display connected, and only after a
while one is connected? Wouldn't that prevent the driver from detecting
it?

tbh I noticed that HDMI re-detection didn't worked in my setup 6.10-rcX
before this series. I need to investigate ...


Maxime

[PATCH 11/11] ARM: bcm2835_defconfig: Enable SUSPEND

[Stefan Wahren]

Since the Raspberry Pi supports Suspend-To-Idle now, this option
should be enabled. This should make power management testing easier.

Signed-off-by: Stefan Wahren 
---
 arch/arm/configs/bcm2835_defconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/configs/bcm2835_defconfig 
b/arch/arm/configs/bcm2835_defconfig
index b5f0bd8dd536..97632dee1ab3 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -38,8 +38,6 @@ CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_CPUFREQ_DT=y
 CONFIG_ARM_RASPBERRYPI_CPUFREQ=y
 CONFIG_VFP=y
-# CONFIG_SUSPEND is not set
-CONFIG_PM=y
 CONFIG_JUMP_LABEL=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
--
2.34.1

[PATCH RFT 10/11] serial: 8250_bcm2835aux: add PM suspend/resume support

[Stefan Wahren]

This adds suspend/resume support for the 8250_bcm2835aux
driver to provide power management support on attached
devices.

Signed-off-by: Stefan Wahren 
---

Since i don't have a RS485 setup, any test feedback would be great.

 drivers/tty/serial/8250/8250_bcm2835aux.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c 
b/drivers/tty/serial/8250/8250_bcm2835aux.c
index 121a5ce86050..cccd2a09cb6f 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -213,11 +213,34 @@ static const struct acpi_device_id 
bcm2835aux_serial_acpi_match[] = {
 };
 MODULE_DEVICE_TABLE(acpi, bcm2835aux_serial_acpi_match);

+static int __maybe_unused bcm2835aux_suspend(struct device *dev)
+{
+   struct bcm2835aux_data *data = dev_get_drvdata(dev);
+
+   serial8250_suspend_port(data->line);
+
+   return 0;
+}
+
+static int __maybe_unused bcm2835aux_resume(struct device *dev)
+{
+   struct bcm2835aux_data *data = dev_get_drvdata(dev);
+
+   serial8250_resume_port(data->line);
+
+   return 0;
+}
+
+static const struct dev_pm_ops bcm2835aux_dev_pm_ops = {
+   SET_SYSTEM_SLEEP_PM_OPS(bcm2835aux_suspend, bcm2835aux_resume)
+};
+
 static struct platform_driver bcm2835aux_serial_driver = {
.driver = {
.name = "bcm2835-aux-uart",
.of_match_table = bcm2835aux_serial_match,
.acpi_match_table = bcm2835aux_serial_acpi_match,
+   .pm = &bcm2835aux_dev_pm_ops,
},
.probe  = bcm2835aux_serial_probe,
.remove_new = bcm2835aux_serial_remove,
--
2.34.1

[PATCH 04/11] pmdomain: raspberrypi-power: Avoid powering down USB

[Stefan Wahren]

During supend to idle any request to power off the USB domain
leads to a timeout. As a temporary workaround don't register
the relevant power off handler.

Link: https://github.com/raspberrypi/firmware/issues/1894
Signed-off-by: Stefan Wahren 
---
 drivers/pmdomain/bcm/raspberrypi-power.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pmdomain/bcm/raspberrypi-power.c 
b/drivers/pmdomain/bcm/raspberrypi-power.c
index 39d9a52200c3..3e7b84006acc 100644
--- a/drivers/pmdomain/bcm/raspberrypi-power.c
+++ b/drivers/pmdomain/bcm/raspberrypi-power.c
@@ -86,7 +86,14 @@ static void rpi_common_init_power_domain(struct 
rpi_power_domains *rpi_domains,

dom->base.name = name;
dom->base.power_on = rpi_domain_on;
-   dom->base.power_off = rpi_domain_off;
+
+   /*
+* During supend to idle any request to power off the USB domain
+* leads to a timeout. As a temporary workaround don't register
+* the relevant power off handler.
+*/
+   if (strcmp("USB", name))
+   dom->base.power_off = rpi_domain_off;

/*
 * Treat all power domains as off at boot.
--
2.34.1

[PATCH 05/11] irqchip/bcm2835: Enable SKIP_SET_WAKE and MASK_ON_SUSPEND

[Stefan Wahren]

The BCM2835 ARMCTRL interrupt controller doesn't provide any facility
to configure the wakeup sources. That's the reason why this
implementation lacks the irq_set_wake implementation. But this prevent
us from properly entering power management states like "suspend to
idle".

So enable the flags IRQCHIP_SKIP_SET_WAKE and
IRQCHIP_MASK_ON_SUSPEND to let the irqchip core allows and handles
the power management.

Signed-off-by: Stefan Wahren 
---
 drivers/irqchip/irq-bcm2835.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c
index e94e2882286c..6c20604c2242 100644
--- a/drivers/irqchip/irq-bcm2835.c
+++ b/drivers/irqchip/irq-bcm2835.c
@@ -102,7 +102,9 @@ static void armctrl_unmask_irq(struct irq_data *d)
 static struct irq_chip armctrl_chip = {
.name = "ARMCTRL-level",
.irq_mask = armctrl_mask_irq,
-   .irq_unmask = armctrl_unmask_irq
+   .irq_unmask = armctrl_unmask_irq,
+   .flags = IRQCHIP_MASK_ON_SUSPEND |
+IRQCHIP_SKIP_SET_WAKE,
 };

 static int armctrl_xlate(struct irq_domain *d, struct device_node *ctrlr,
--
2.34.1

[PATCH 06/11] drm/vc4: hdmi: Handle error case of pm_runtime_resume_and_get

[Stefan Wahren]

The commit 0f5251339eda ("drm/vc4: hdmi: Make sure the controller is
powered in detect") introduced the necessary power management handling
to avoid register access while controller is powered down.
Unfortunately it just print a warning if pm_runtime_resume_and_get()
fails and proceed anyway.

This could happen during suspend to idle. So we must assume it is unsafe
to access the HDMI register. So bail out properly.

Fixes: 0f5251339eda ("drm/vc4: hdmi: Make sure the controller is powered in 
detect")
Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d57c4a5948c8..b3a42b709718 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -429,6 +429,7 @@ static int vc4_hdmi_connector_detect_ctx(struct 
drm_connector *connector,
 {
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
enum drm_connector_status status = connector_status_disconnected;
+   int ret;

/*
 * NOTE: This function should really take vc4_hdmi->mutex, but
@@ -441,7 +442,11 @@ static int vc4_hdmi_connector_detect_ctx(struct 
drm_connector *connector,
 * the lock for now.
 */

-   WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
+   ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev);
+   if (ret) {
+   DRM_ERROR("Failed to retain HDMI power domain: %d\n", ret);
+   return status;
+   }

if (vc4_hdmi->hpd_gpio) {
if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
--
2.34.1

[PATCH 08/11] usb: dwc2: debugfs: Print parameter no_clock_gating

[Stefan Wahren]

The commit c4a0f7a6ab54 ("usb: dwc2: Skip clock gating on Samsung
SoCs") introduced a parameter to skip enabling clock gating mode
even the hardware platform should supports it.

In order to make this more visible also print this in show
parameters of debugfs.

Signed-off-by: Stefan Wahren 
---
 drivers/usb/dwc2/debugfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
index 7c82ab590401..3116ac72747f 100644
--- a/drivers/usb/dwc2/debugfs.c
+++ b/drivers/usb/dwc2/debugfs.c
@@ -702,6 +702,7 @@ static int params_show(struct seq_file *seq, void *v)
print_param(seq, p, uframe_sched);
print_param(seq, p, external_id_pin_ctl);
print_param(seq, p, power_down);
+   print_param(seq, p, no_clock_gating);
print_param(seq, p, lpm);
print_param(seq, p, lpm_clock_gating);
print_param(seq, p, besl);
--
2.34.1

[PATCH 07/11] drm/vc4: hdmi: Disable connector status polling during suspend

[Stefan Wahren]

Suspend of VC4 HDMI will likely triggers a warning from
vc4_hdmi_connector_detect_ctx() during poll of connector status.
The power management will prevent the resume and keep the relevant
power domain disabled.

Since there is no reason to poll the connector status during
suspend, the polling should be disabled during this.

It not possible to use drm_mode_config_helper_suspend() here,
because the callbacks might be called during bind phase and not all
components are fully initialized.

Link: 
https://lore.kernel.org/dri-devel/7003512d-7303-4f41-b0d6-a8af5bf8e...@gmx.net/
Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b3a42b709718..e80495cea6ac 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -3106,6 +3106,13 @@ static int vc5_hdmi_init_resources(struct drm_device 
*drm,
 static int vc4_hdmi_runtime_suspend(struct device *dev)
 {
struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
+
+   /*
+* Don't disable polling if it was never initialized
+*/
+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_disable(drm);

clk_disable_unprepare(vc4_hdmi->hsm_clock);

@@ -3115,6 +3122,7 @@ static int vc4_hdmi_runtime_suspend(struct device *dev)
 static int vc4_hdmi_runtime_resume(struct device *dev)
 {
struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+   struct drm_device *drm = vc4_hdmi->connector.dev;
unsigned long __maybe_unused flags;
u32 __maybe_unused value;
unsigned long rate;
@@ -3159,6 +3167,9 @@ static int vc4_hdmi_runtime_resume(struct device *dev)
}
 #endif

+   if (drm && drm->mode_config.poll_enabled)
+   drm_kms_helper_poll_enable(drm);
+
return 0;

 err_disable_clk:
--
2.34.1

[PATCH 09/11] usb: dwc2: Skip clock gating on Broadcom SoCs

[Stefan Wahren]

On resume of the Raspberry Pi the dwc2 driver fails to enable
HCD_FLAG_HW_ACCESSIBLE before re-enabling the interrupts.
This causes a situation where both handler ignore a incoming port
interrupt and force the upper layers to disable the dwc2 interrupt line.
This leaves the USB interface in a unusable state:

irq 66: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W  6.10.0-rc3
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x50/0x64
dump_stack_lvl from __report_bad_irq+0x38/0xc0
__report_bad_irq from note_interrupt+0x2ac/0x2f4
note_interrupt from handle_irq_event+0x88/0x8c
handle_irq_event from handle_level_irq+0xb4/0x1ac
handle_level_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from bcm2836_chained_handle_irq+0x24/0x28
bcm2836_chained_handle_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from generic_handle_arch_irq+0x34/0x44
generic_handle_arch_irq from __irq_svc+0x88/0xb0
Exception stack(0xc1b01f20 to 0xc1b01f68)
1f20: 0005c0d4 0001   c1b09780 c1d6b32c c1b04e54 c1a5eae8
1f40: c1b04e90    c1d6a8a0 c1b01f70 c11d2da8 c11d4160
1f60: 6013 
__irq_svc from default_idle_call+0x1c/0xb0
default_idle_call from do_idle+0x21c/0x284
do_idle from cpu_startup_entry+0x28/0x2c
cpu_startup_entry from kernel_init+0x0/0x12c
handlers:
[] dwc2_handle_common_intr
[<75cd278b>] usb_hcd_irq
Disabling IRQ #66

Disabling clock gatling workaround this issue.

Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.")
Link: 
https://lore.kernel.org/linux-usb/3fd0c2fb-4752-45b3-94eb-42352703e...@gmx.net/T/
Signed-off-by: Stefan Wahren 
---
 drivers/usb/dwc2/params.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 5a1500d0bdd9..66580de52882 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -23,6 +23,7 @@ static void dwc2_set_bcm_params(struct dwc2_hsotg *hsotg)
p->max_transfer_size = 65535;
p->max_packet_count = 511;
p->ahbcfg = 0x10;
+   p->no_clock_gating = true;
 }

 static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
--
2.34.1

[PATCH 02/11] mailbox: bcm2835: Fix timeout during suspend mode

[Stefan Wahren]

During noirq suspend phase the Raspberry Pi power driver suffer of
firmware property timeouts. The reason is that the IRQ of the underlying
BCM2835 mailbox is disabled and rpi_firmware_property_list() will always
run into a timeout [1].

Since the VideoCore side isn't consider as a wakeup source, set the
IRQF_NO_SUSPEND flag for the mailbox IRQ in order to keep it enabled
during suspend-resume cycle.

[1]
PM: late suspend of devices complete after 1.754 msecs
WARNING: CPU: 0 PID: 438 at drivers/firmware/raspberrypi.c:128
 rpi_firmware_property_list+0x204/0x22c
Firmware transaction 0x00028001 timeout
Modules linked in:
CPU: 0 PID: 438 Comm: bash Tainted: G C 6.9.3-dirty #17
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x34/0x44
dump_stack_lvl from __warn+0x88/0xec
__warn from warn_slowpath_fmt+0x7c/0xb0
warn_slowpath_fmt from rpi_firmware_property_list+0x204/0x22c
rpi_firmware_property_list from rpi_firmware_property+0x68/0x8c
rpi_firmware_property from rpi_firmware_set_power+0x54/0xc0
rpi_firmware_set_power from _genpd_power_off+0xe4/0x148
_genpd_power_off from genpd_sync_power_off+0x7c/0x11c
genpd_sync_power_off from genpd_finish_suspend+0xcc/0xe0
genpd_finish_suspend from dpm_run_callback+0x78/0xd0
dpm_run_callback from device_suspend_noirq+0xc0/0x238
device_suspend_noirq from dpm_suspend_noirq+0xb0/0x168
dpm_suspend_noirq from suspend_devices_and_enter+0x1b8/0x5ac
suspend_devices_and_enter from pm_suspend+0x254/0x2e4
pm_suspend from state_store+0xa8/0xd4
state_store from kernfs_fop_write_iter+0x154/0x1a0
kernfs_fop_write_iter from vfs_write+0x12c/0x184
vfs_write from ksys_write+0x78/0xc0
ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0xcc93dfa8 to 0xcc93dff0)
[...]
PM: noirq suspend of devices complete after 3095.584 msecs

Link: https://github.com/raspberrypi/firmware/issues/1894
Fixes: 0bae6af6d704 ("mailbox: Enable BCM2835 mailbox support")
Signed-off-by: Stefan Wahren 
---
 drivers/mailbox/bcm2835-mailbox.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/bcm2835-mailbox.c 
b/drivers/mailbox/bcm2835-mailbox.c
index fbfd0202047c..ea12fb8d2401 100644
--- a/drivers/mailbox/bcm2835-mailbox.c
+++ b/drivers/mailbox/bcm2835-mailbox.c
@@ -145,7 +145,8 @@ static int bcm2835_mbox_probe(struct platform_device *pdev)
spin_lock_init(&mbox->lock);

ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
-  bcm2835_mbox_irq, 0, dev_name(dev), mbox);
+  bcm2835_mbox_irq, IRQF_NO_SUSPEND, dev_name(dev),
+  mbox);
if (ret) {
dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
ret);
--
2.34.1

[PATCH 03/11] pmdomain: raspberrypi-power: Adjust packet definition

[Stefan Wahren]

According to the official Mailbox property interface the second part
of RPI_FIRMWARE_SET_POWER_STATE ( and so on ...) is named state because
it represent u32 flags and just the lowest bit is for on/off. So rename it
to align with documentation and prepare the driver for further changes.

Link: https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface
Signed-off-by: Stefan Wahren 
---
 drivers/pmdomain/bcm/raspberrypi-power.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pmdomain/bcm/raspberrypi-power.c 
b/drivers/pmdomain/bcm/raspberrypi-power.c
index 06196ebfe03b..39d9a52200c3 100644
--- a/drivers/pmdomain/bcm/raspberrypi-power.c
+++ b/drivers/pmdomain/bcm/raspberrypi-power.c
@@ -41,7 +41,7 @@ struct rpi_power_domains {
  */
 struct rpi_power_domain_packet {
u32 domain;
-   u32 on;
+   u32 state;
 };

 /*
@@ -53,7 +53,7 @@ static int rpi_firmware_set_power(struct rpi_power_domain 
*rpi_domain, bool on)
struct rpi_power_domain_packet packet;

packet.domain = rpi_domain->domain;
-   packet.on = on;
+   packet.state = on;
return rpi_firmware_property(rpi_domain->fw,
 rpi_domain->old_interface ?
 RPI_FIRMWARE_SET_POWER_STATE :
@@ -142,13 +142,13 @@ rpi_has_new_domain_support(struct rpi_power_domains 
*rpi_domains)
int ret;

packet.domain = RPI_POWER_DOMAIN_ARM;
-   packet.on = ~0;
+   packet.state = ~0;

ret = rpi_firmware_property(rpi_domains->fw,
RPI_FIRMWARE_GET_DOMAIN_STATE,
&packet, sizeof(packet));

-   return ret == 0 && packet.on != ~0;
+   return ret == 0 && packet.state != ~0;
 }

 static int rpi_power_probe(struct platform_device *pdev)
--
2.34.1

[PATCH 00/11] ARM: bcm2835: Implement initial S2Idle for Raspberry Pi

[Stefan Wahren]

This series implement the initial Suspend-To-Idle support for
the Raspberry Pi, which was a long time on my TODO list [1]. The
changes allow to suspend and resume the Raspberry Pi via debug UART.
The focus is on the BCM2835 SoC, because it's less complex than its
successors and have enough documentation.

The series is a loose collection of fixes and improvements.
So cherry-picking should be fine.

Test steps:
- configure debug console (pl011 or mini UART) as wakeup source
- send system to idle state

  echo freeze > /sys/power/state

- wakeup system by console traffic

The implementation isn't perfect and contains workarounds like
patch 4 and 9. So there is still room for improvements, but
at least the system won't freeze forever as before [2].

Here are some figures for the Raspberry Pi 1 (without any
devices connected except of a debug UART):

running but CPU idle = 1.67 W
suspend to idle  = 1.33 W

The series has been tested on the following platforms:
Raspberry Pi 1 B
Raspberry Pi 3 A+
Raspberry Pi 3 B+

Known issues:
- currently it's not possible to power down the USB domain [3]
- there seems to be an issue with the DWC2 suspend handling [4]

[1] - https://github.com/lategoodbye/rpi-zero/issues/9
[2] - https://bugzilla.redhat.com/show_bug.cgi?id=2283978
[3] - https://github.com/raspberrypi/firmware/issues/1894
[4] - 
https://lore.kernel.org/linux-usb/3fd0c2fb-4752-45b3-94eb-42352703e...@gmx.net/T/

Stefan Wahren (11):
  firmware: raspberrypi: Improve timeout warning
  mailbox: bcm2835: Fix timeout during suspend mode
  pmdomain: raspberrypi-power: Adjust packet definition
  pmdomain: raspberrypi-power: Avoid powering down USB
  irqchip/bcm2835: Enable SKIP_SET_WAKE and MASK_ON_SUSPEND
  drm/vc4: hdmi: Handle error case of pm_runtime_resume_and_get
  drm/vc4: hdmi: Disable connector status polling during suspend
  usb: dwc2: debugfs: Print parameter no_clock_gating
  usb: dwc2: Skip clock gating on Broadcom SoCs
  serial: 8250_bcm2835aux: add PM suspend/resume support
  ARM: bcm2835_defconfig: Enable SUSPEND

 arch/arm/configs/bcm2835_defconfig|  2 --
 drivers/firmware/raspberrypi.c|  3 ++-
 drivers/gpu/drm/vc4/vc4_hdmi.c| 18 +-
 drivers/irqchip/irq-bcm2835.c |  4 +++-
 drivers/mailbox/bcm2835-mailbox.c |  3 ++-
 drivers/pmdomain/bcm/raspberrypi-power.c  | 17 -
 drivers/tty/serial/8250/8250_bcm2835aux.c | 23 +++
 drivers/usb/dwc2/debugfs.c|  1 +
 drivers/usb/dwc2/params.c |  1 +
 9 files changed, 61 insertions(+), 11 deletions(-)

--
2.34.1

[PATCH 01/11] firmware: raspberrypi: Improve timeout warning

[Stefan Wahren]

Recent work on raspberry-power driver showed that even the
stacktrace on firmware property timeout doesn't provide
enough information. So add the first tag name to the warning
to be in line with a status error.

Signed-off-by: Stefan Wahren 
---
 drivers/firmware/raspberrypi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index ac34876a97f8..18cc34987108 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -62,7 +62,6 @@ rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, 
u32 data)
ret = 0;
} else {
ret = -ETIMEDOUT;
-   WARN_ONCE(1, "Firmware transaction timeout");
}
} else {
dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
@@ -125,6 +124,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
buf[2], buf[1]);
ret = -EINVAL;
+   } else if (ret == -ETIMEDOUT) {
+   WARN_ONCE(1, "Firmware transaction 0x%08x timeout", buf[2]);
}

dma_free_coherent(fw->chan->mbox->dev, PAGE_ALIGN(size), buf, bus_addr);
--
2.34.1

Re: vc4: WARNING during suspend to idle of Raspberry Pi 3 Plus

[Stefan Wahren]

Am 27.06.24 um 11:58 schrieb Maxime Ripard:

On Tue, Jun 25, 2024 at 04:21:11PM GMT, Stefan Wahren wrote:

Am 24.06.24 um 14:57 schrieb Maxime Ripard:

On Sun, Jun 16, 2024 at 11:27:08AM GMT, Stefan Wahren wrote:

Hi,
i'm currently experiment with suspend to idle on the Raspberry Pi [1].

During my tests, i noticed a WARNING of the vc4 during suspend incl.
Runtime PM usage count underflow. It would be nice if someone can look
at it. In case you want to reproduce it, i can prepare a branch with
some improvements/hacks. For example i disabled dwc2/USB because it
cause a lot of trouble during resume.

Here are the steps to trigger this issue:
- make sure necessary kernel options are enabled ( CONFIG_SUSPEND,
CONFIG_PM_DEBUG, CONFIG_PM_ADVANCED_DEBUG )
- make sure no HDMI cable is connected to Raspberry Pi 3 Plus

sorry, i forgot the specific type of the Raspberry Pi 3 B Plus, but the
issue on Pi 3 A Plus is the same.

- Add "no_console_suspend" to cmdline.txt and reboot
- Connect via Debug UART:

echo 1 > /sys/power/pm_debug_messages
echo platform > /sys/power/pm_test
echo freeze > /sys/power/state

Here is the output:

[   75.538497] PM: suspend entry (s2idle)
[   76.915786] Filesystems sync: 1.377 seconds
[   79.678262] rpi_firmware_set_power: Set HDMI to 1
[   79.678634] rpi_firmware_set_power: Set HDMI to 0
[   79.850949] Freezing user space processes
[   79.852460] Freezing user space processes completed (elapsed 0.001
seconds)
[   79.852484] OOM killer disabled.
[   79.852493] Freezing remaining freezable tasks
[   79.853684] Freezing remaining freezable tasks completed (elapsed
0.001 seconds)
[   80.892819] ieee80211 phy0: brcmf_fil_cmd_data: bus is down. we have
nothing to do.
[   80.892843] ieee80211 phy0: brcmf_cfg80211_get_tx_power: error (-5)
[   81.514053] PM: suspend of devices complete after 1659.336 msecs
[   81.514089] PM: start suspend of devices complete after 1660.386 msecs
[   81.515616] PM: late suspend of devices complete after 1.509 msecs
[   81.515938] rpi_firmware_set_power: Set VEC to 0
[   81.516010] rpi_firmware_set_power: Set V3D to 0
[   81.516998] PM: noirq suspend of devices complete after 1.239 msecs
[   81.517016] PM: suspend debug: Waiting for 5 second(s).
[   89.598310] rpi_firmware_set_power: Set V3D to 1
[   90.078228] [ cut here ]
[   90.078240] WARNING: CPU: 1 PID: 216 at
drivers/gpu/drm/vc4/vc4_hdmi.c:477
vc4_hdmi_connector_detect_ctx+0x2e4/0x34c [vc4]
[   90.078344] Modules linked in: aes_arm aes_generic cbc crypto_simd
cryptd algif_skcipher af_alg brcmfmac_wcc brcmfmac vc4 brcmutil
snd_soc_hdmi_codec snd_soc_core ac97_bus sha256_generic
snd_pcm_dmaengine libsha256 snd_pcm sha256_arm snd_timer hci_uart
cfg80211 btbcm snd bluetooth soundcore drm_dma_helper crc32_arm_ce
ecdh_generic ecc raspberrypi_hwmon libaes bcm2835_thermal
[   90.078551] CPU: 1 PID: 216 Comm: kworker/1:2 Not tainted 6.9.3-dirty #30
[   90.078568] Hardware name: BCM2835
[   90.078580] Workqueue: events output_poll_execute
[   90.078610] Call trace:
[   90.078624]  unwind_backtrace from show_stack+0x10/0x14
[   90.078660]  show_stack from dump_stack_lvl+0x50/0x64
[   90.078688]  dump_stack_lvl from __warn+0x7c/0x124
[   90.078723]  __warn from warn_slowpath_fmt+0x170/0x178
[   90.078760]  warn_slowpath_fmt from
vc4_hdmi_connector_detect_ctx+0x2e4/0x34c [vc4]
[   90.078862]  vc4_hdmi_connector_detect_ctx [vc4] from
drm_helper_probe_detect_ctx+0x40/0x120
[   90.078951]  drm_helper_probe_detect_ctx from
output_poll_execute+0x160/0x24c
[   90.078982]  output_poll_execute from process_one_work+0x16c/0x3b4
[   90.079012]  process_one_work from worker_thread+0x270/0x488
[   90.079036]  worker_thread from kthread+0xe0/0xfc
[   90.079060]  kthread from ret_from_fork+0x14/0x28
[   90.079080] Exception stack(0xf0af9fb0 to 0xf0af9ff8)
[   90.079096] 9fa0: 
  
[   90.079113] 9fc0:     
  
[   90.079129] 9fe0:     0013 
[   90.079141] ---[ end trace  ]---
[   90.079155] vc4_hdmi 3f902000.hdmi: vc4_hdmi_connector_detect_ctx:
pm_runtime_resume_and_get failed: -13
[   92.638262] rpi_firmware_set_power: Set HDMI to 1
[   95.678251] rpi_firmware_set_power: Set VEC to 1
[   95.678380] PM: noirq resume of devices complete after 9160.930 msecs
[   95.679604] PM: early resume of devices complete after 1.069 msecs
[   95.812230] brcmfmac: brcmf_fw_alloc_request: using
brcm/brcmfmac43455-sdio for chip BCM4345/6
[   95.812282] PM: resume of devices complete after 132.657 msecs
[   95.813246] vc4_hdmi 3f902000.hdmi: Runtime PM usage count underflow!
[   95.814456] OOM killer enabled.
[   95.814466] Restarting tasks ... done.
[   95.817193] random: crng reseeded on system resumption
[   95.819813] rpi_firmware_set_power: Set HDMI to 0
[   95.827808] PM: suspend exit

[1] - https://github.

Re: vc4: WARNING during suspend to idle of Raspberry Pi 3 Plus

[Stefan Wahren]

Hi Maxime,

Am 24.06.24 um 14:57 schrieb Maxime Ripard:

On Sun, Jun 16, 2024 at 11:27:08AM GMT, Stefan Wahren wrote:

Hi,
i'm currently experiment with suspend to idle on the Raspberry Pi [1].

During my tests, i noticed a WARNING of the vc4 during suspend incl.
Runtime PM usage count underflow. It would be nice if someone can look
at it. In case you want to reproduce it, i can prepare a branch with
some improvements/hacks. For example i disabled dwc2/USB because it
cause a lot of trouble during resume.

Here are the steps to trigger this issue:
- make sure necessary kernel options are enabled ( CONFIG_SUSPEND,
CONFIG_PM_DEBUG, CONFIG_PM_ADVANCED_DEBUG )
- make sure no HDMI cable is connected to Raspberry Pi 3 Plus

sorry, i forgot the specific type of the Raspberry Pi 3 B Plus, but the
issue on Pi 3 A Plus is the same.

- Add "no_console_suspend" to cmdline.txt and reboot
- Connect via Debug UART:

echo 1 > /sys/power/pm_debug_messages
echo platform > /sys/power/pm_test
echo freeze > /sys/power/state

Here is the output:

[   75.538497] PM: suspend entry (s2idle)
[   76.915786] Filesystems sync: 1.377 seconds
[   79.678262] rpi_firmware_set_power: Set HDMI to 1
[   79.678634] rpi_firmware_set_power: Set HDMI to 0
[   79.850949] Freezing user space processes
[   79.852460] Freezing user space processes completed (elapsed 0.001
seconds)
[   79.852484] OOM killer disabled.
[   79.852493] Freezing remaining freezable tasks
[   79.853684] Freezing remaining freezable tasks completed (elapsed
0.001 seconds)
[   80.892819] ieee80211 phy0: brcmf_fil_cmd_data: bus is down. we have
nothing to do.
[   80.892843] ieee80211 phy0: brcmf_cfg80211_get_tx_power: error (-5)
[   81.514053] PM: suspend of devices complete after 1659.336 msecs
[   81.514089] PM: start suspend of devices complete after 1660.386 msecs
[   81.515616] PM: late suspend of devices complete after 1.509 msecs
[   81.515938] rpi_firmware_set_power: Set VEC to 0
[   81.516010] rpi_firmware_set_power: Set V3D to 0
[   81.516998] PM: noirq suspend of devices complete after 1.239 msecs
[   81.517016] PM: suspend debug: Waiting for 5 second(s).
[   89.598310] rpi_firmware_set_power: Set V3D to 1
[   90.078228] [ cut here ]
[   90.078240] WARNING: CPU: 1 PID: 216 at
drivers/gpu/drm/vc4/vc4_hdmi.c:477
vc4_hdmi_connector_detect_ctx+0x2e4/0x34c [vc4]
[   90.078344] Modules linked in: aes_arm aes_generic cbc crypto_simd
cryptd algif_skcipher af_alg brcmfmac_wcc brcmfmac vc4 brcmutil
snd_soc_hdmi_codec snd_soc_core ac97_bus sha256_generic
snd_pcm_dmaengine libsha256 snd_pcm sha256_arm snd_timer hci_uart
cfg80211 btbcm snd bluetooth soundcore drm_dma_helper crc32_arm_ce
ecdh_generic ecc raspberrypi_hwmon libaes bcm2835_thermal
[   90.078551] CPU: 1 PID: 216 Comm: kworker/1:2 Not tainted 6.9.3-dirty #30
[   90.078568] Hardware name: BCM2835
[   90.078580] Workqueue: events output_poll_execute
[   90.078610] Call trace:
[   90.078624]  unwind_backtrace from show_stack+0x10/0x14
[   90.078660]  show_stack from dump_stack_lvl+0x50/0x64
[   90.078688]  dump_stack_lvl from __warn+0x7c/0x124
[   90.078723]  __warn from warn_slowpath_fmt+0x170/0x178
[   90.078760]  warn_slowpath_fmt from
vc4_hdmi_connector_detect_ctx+0x2e4/0x34c [vc4]
[   90.078862]  vc4_hdmi_connector_detect_ctx [vc4] from
drm_helper_probe_detect_ctx+0x40/0x120
[   90.078951]  drm_helper_probe_detect_ctx from
output_poll_execute+0x160/0x24c
[   90.078982]  output_poll_execute from process_one_work+0x16c/0x3b4
[   90.079012]  process_one_work from worker_thread+0x270/0x488
[   90.079036]  worker_thread from kthread+0xe0/0xfc
[   90.079060]  kthread from ret_from_fork+0x14/0x28
[   90.079080] Exception stack(0xf0af9fb0 to 0xf0af9ff8)
[   90.079096] 9fa0: 
  
[   90.079113] 9fc0:     
  
[   90.079129] 9fe0:     0013 
[   90.079141] ---[ end trace  ]---
[   90.079155] vc4_hdmi 3f902000.hdmi: vc4_hdmi_connector_detect_ctx:
pm_runtime_resume_and_get failed: -13
[   92.638262] rpi_firmware_set_power: Set HDMI to 1
[   95.678251] rpi_firmware_set_power: Set VEC to 1
[   95.678380] PM: noirq resume of devices complete after 9160.930 msecs
[   95.679604] PM: early resume of devices complete after 1.069 msecs
[   95.812230] brcmfmac: brcmf_fw_alloc_request: using
brcm/brcmfmac43455-sdio for chip BCM4345/6
[   95.812282] PM: resume of devices complete after 132.657 msecs
[   95.813246] vc4_hdmi 3f902000.hdmi: Runtime PM usage count underflow!
[   95.814456] OOM killer enabled.
[   95.814466] Restarting tasks ... done.
[   95.817193] random: crng reseeded on system resumption
[   95.819813] rpi_firmware_set_power: Set HDMI to 0
[   95.827808] PM: suspend exit

[1] - https://github.com/raspberrypi/firmware/issues/1894

The code itself looks fine to me still, but It's n

vc4: WARNING during suspend to idle of Raspberry Pi 3 Plus

[Stefan Wahren]

Hi,
i'm currently experiment with suspend to idle on the Raspberry Pi [1].

During my tests, i noticed a WARNING of the vc4 during suspend incl.
Runtime PM usage count underflow. It would be nice if someone can look
at it. In case you want to reproduce it, i can prepare a branch with
some improvements/hacks. For example i disabled dwc2/USB because it
cause a lot of trouble during resume.

Here are the steps to trigger this issue:
- make sure necessary kernel options are enabled ( CONFIG_SUSPEND,
CONFIG_PM_DEBUG, CONFIG_PM_ADVANCED_DEBUG )
- make sure no HDMI cable is connected to Raspberry Pi 3 Plus
- Add "no_console_suspend" to cmdline.txt and reboot
- Connect via Debug UART:

echo 1 > /sys/power/pm_debug_messages
echo platform > /sys/power/pm_test
echo freeze > /sys/power/state

Here is the output:

[   75.538497] PM: suspend entry (s2idle)
[   76.915786] Filesystems sync: 1.377 seconds
[   79.678262] rpi_firmware_set_power: Set HDMI to 1
[   79.678634] rpi_firmware_set_power: Set HDMI to 0
[   79.850949] Freezing user space processes
[   79.852460] Freezing user space processes completed (elapsed 0.001
seconds)
[   79.852484] OOM killer disabled.
[   79.852493] Freezing remaining freezable tasks
[   79.853684] Freezing remaining freezable tasks completed (elapsed
0.001 seconds)
[   80.892819] ieee80211 phy0: brcmf_fil_cmd_data: bus is down. we have
nothing to do.
[   80.892843] ieee80211 phy0: brcmf_cfg80211_get_tx_power: error (-5)
[   81.514053] PM: suspend of devices complete after 1659.336 msecs
[   81.514089] PM: start suspend of devices complete after 1660.386 msecs
[   81.515616] PM: late suspend of devices complete after 1.509 msecs
[   81.515938] rpi_firmware_set_power: Set VEC to 0
[   81.516010] rpi_firmware_set_power: Set V3D to 0
[   81.516998] PM: noirq suspend of devices complete after 1.239 msecs
[   81.517016] PM: suspend debug: Waiting for 5 second(s).
[   89.598310] rpi_firmware_set_power: Set V3D to 1
[   90.078228] [ cut here ]
[   90.078240] WARNING: CPU: 1 PID: 216 at
drivers/gpu/drm/vc4/vc4_hdmi.c:477
vc4_hdmi_connector_detect_ctx+0x2e4/0x34c [vc4]
[   90.078344] Modules linked in: aes_arm aes_generic cbc crypto_simd
cryptd algif_skcipher af_alg brcmfmac_wcc brcmfmac vc4 brcmutil
snd_soc_hdmi_codec snd_soc_core ac97_bus sha256_generic
snd_pcm_dmaengine libsha256 snd_pcm sha256_arm snd_timer hci_uart
cfg80211 btbcm snd bluetooth soundcore drm_dma_helper crc32_arm_ce
ecdh_generic ecc raspberrypi_hwmon libaes bcm2835_thermal
[   90.078551] CPU: 1 PID: 216 Comm: kworker/1:2 Not tainted 6.9.3-dirty #30
[   90.078568] Hardware name: BCM2835
[   90.078580] Workqueue: events output_poll_execute
[   90.078610] Call trace:
[   90.078624]  unwind_backtrace from show_stack+0x10/0x14
[   90.078660]  show_stack from dump_stack_lvl+0x50/0x64
[   90.078688]  dump_stack_lvl from __warn+0x7c/0x124
[   90.078723]  __warn from warn_slowpath_fmt+0x170/0x178
[   90.078760]  warn_slowpath_fmt from
vc4_hdmi_connector_detect_ctx+0x2e4/0x34c [vc4]
[   90.078862]  vc4_hdmi_connector_detect_ctx [vc4] from
drm_helper_probe_detect_ctx+0x40/0x120
[   90.078951]  drm_helper_probe_detect_ctx from
output_poll_execute+0x160/0x24c
[   90.078982]  output_poll_execute from process_one_work+0x16c/0x3b4
[   90.079012]  process_one_work from worker_thread+0x270/0x488
[   90.079036]  worker_thread from kthread+0xe0/0xfc
[   90.079060]  kthread from ret_from_fork+0x14/0x28
[   90.079080] Exception stack(0xf0af9fb0 to 0xf0af9ff8)
[   90.079096] 9fa0: 
  
[   90.079113] 9fc0:     
  
[   90.079129] 9fe0:     0013 
[   90.079141] ---[ end trace  ]---
[   90.079155] vc4_hdmi 3f902000.hdmi: vc4_hdmi_connector_detect_ctx:
pm_runtime_resume_and_get failed: -13
[   92.638262] rpi_firmware_set_power: Set HDMI to 1
[   95.678251] rpi_firmware_set_power: Set VEC to 1
[   95.678380] PM: noirq resume of devices complete after 9160.930 msecs
[   95.679604] PM: early resume of devices complete after 1.069 msecs
[   95.812230] brcmfmac: brcmf_fw_alloc_request: using
brcm/brcmfmac43455-sdio for chip BCM4345/6
[   95.812282] PM: resume of devices complete after 132.657 msecs
[   95.813246] vc4_hdmi 3f902000.hdmi: Runtime PM usage count underflow!
[   95.814456] OOM killer enabled.
[   95.814466] Restarting tasks ... done.
[   95.817193] random: crng reseeded on system resumption
[   95.819813] rpi_firmware_set_power: Set HDMI to 0
[   95.827808] PM: suspend exit

[1] - https://github.com/raspberrypi/firmware/issues/1894

Re: [PATCH v2] ARM: dts: bcm2835: Enable 3D rendering through V3D

[Stefan Wahren]

Hi Maíra,

Am 16.04.24 um 03:02 schrieb Maíra Canal:

On 4/15/24 13:54, Andre Przywara wrote:

On Mon, 15 Apr 2024 13:00:39 -0300
Maíra Canal  wrote:

Hi,


RPi 0-3 is packed with a GPU that provides 3D rendering capabilities to
the RPi. Currently, the downstream kernel uses an overlay to enable the
GPU and use GPU hardware acceleration. When deploying a mainline kernel
to the RPi 0-3, we end up without any GPU hardware acceleration
(essentially, we can't use the OpenGL driver).

Therefore, enable the V3D core for the RPi 0-3 in the mainline kernel.


So I think Krzysztof's initial comment still stands: What does that
patch
actually change? If I build those DTBs as of now, none of them has a
status property in the v3d node. Which means it's enabled:
https://github.com/devicetree-org/devicetree-specification/blob/main/source/chapter2-devicetree-basics.rst#status

So adding an explicit 'status = "okay";' doesn't make a difference.

What do I miss here?


As mentioned by Stefan in the last version, in Raspberry Pi OS, there is
a systemd script which is trying to check for the V3D driver (/usr/lib
/systemd/scripts/gldriver_test.sh). Within the first check, "raspi-
config nonint is_kms" is called, which always seems to fail. What
"raspi-config" does is check if
/proc/device-tree/soc/v3d@7ec0/status is equal to "okay". As
/proc/device-tree/soc/v3d@7ec0/status doesn't exists, it returns
false.

yes, but i also mention that the V3D driver starts without this patch.
The commit message of this patch suggests this is a DT issue, which is not.

I hadn't the time to update my SD card to Bookworm yet. Does the issue
still exists with this version?


I'll send if I can improve the userspace tool by just checking if the
folder /proc/device-tree/soc/v3d@7ec0/ exists.

Thanks for the explanation!

Best Regards,
- Maíra



Cheers,
Andre


Signed-off-by: Maíra Canal 
---

v1 -> v2:
https://lore.kernel.org/dri-devel/41694292-af1f-4760-a7b6-101ed5dd6...@gmx.net/T/

* As mentioned by Krzysztof, enabling should be done in last place of
override/extend. Therefore, I'm disabling V3D in the common dtsi
and enabling in the last place of extend, i.e. the RPi DTS files.

  arch/arm/boot/dts/broadcom/bcm2835-common.dtsi  | 1 +
  arch/arm/boot/dts/broadcom/bcm2835-rpi-a-plus.dts   | 4 
  arch/arm/boot/dts/broadcom/bcm2835-rpi-a.dts    | 4 
  arch/arm/boot/dts/broadcom/bcm2835-rpi-b-plus.dts   | 4 
  arch/arm/boot/dts/broadcom/bcm2835-rpi-b-rev2.dts   | 4 
  arch/arm/boot/dts/broadcom/bcm2835-rpi-b.dts    | 4 
  arch/arm/boot/dts/broadcom/bcm2835-rpi-cm1-io1.dts  | 4 
  arch/arm/boot/dts/broadcom/bcm2835-rpi-zero-w.dts   | 4 
  arch/arm/boot/dts/broadcom/bcm2835-rpi-zero.dts | 4 
  arch/arm/boot/dts/broadcom/bcm2836-rpi-2-b.dts  | 4 
  arch/arm/boot/dts/broadcom/bcm2837-rpi-3-a-plus.dts | 4 
  arch/arm/boot/dts/broadcom/bcm2837-rpi-3-b-plus.dts | 4 
  arch/arm/boot/dts/broadcom/bcm2837-rpi-3-b.dts  | 4 
  arch/arm/boot/dts/broadcom/bcm2837-rpi-cm3-io3.dts  | 4 
  arch/arm/boot/dts/broadcom/bcm2837-rpi-zero-2-w.dts | 4 
  15 files changed, 57 insertions(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
index 9261b67dbee1..69e34831de51 100644
--- a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
@@ -139,6 +139,7 @@ v3d: v3d@7ec0 {
  compatible = "brcm,bcm2835-v3d";
  reg = <0x7ec0 0x1000>;
  interrupts = <1 10>;
+    status = "disabled";
  };
    vc4: gpu {
diff --git a/arch/arm/boot/dts/broadcom/bcm2835-rpi-a-plus.dts
b/arch/arm/boot/dts/broadcom/bcm2835-rpi-a-plus.dts
index 069b48272aa5..495ab1dfd2ce 100644
--- a/arch/arm/boot/dts/broadcom/bcm2835-rpi-a-plus.dts
+++ b/arch/arm/boot/dts/broadcom/bcm2835-rpi-a-plus.dts
@@ -128,3 +128,7 @@ &uart0 {
  pinctrl-0 = <&uart0_gpio14>;
  status = "okay";
  };
+
+&v3d {
+    status = "okay";
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm2835-rpi-a.dts
b/arch/arm/boot/dts/broadcom/bcm2835-rpi-a.dts
index 2726c00431e8..4634d88ce3af 100644
--- a/arch/arm/boot/dts/broadcom/bcm2835-rpi-a.dts
+++ b/arch/arm/boot/dts/broadcom/bcm2835-rpi-a.dts
@@ -121,3 +121,7 @@ &uart0 {
  pinctrl-0 = <&uart0_gpio14>;
  status = "okay";
  };
+
+&v3d {
+    status = "okay";
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm2835-rpi-b-plus.dts
b/arch/arm/boot/dts/broadcom/bcm2835-rpi-b-plus.dts
index c57b999a4520..45fa0f6851fc 100644
--- a/arch/arm/boot/dts/broadcom/bcm2835-rpi-b-plus.dts
+++ b/arch/arm/boot/dts/broadcom/bcm2835-rpi-b-plus.dts
@@ -130,3 +130,7 @@ &uart0 {
  pinctrl-0 = <&uart0_gpio14>;
  status = "okay";
  };
+
+&v3d {
+    status = "okay";
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm2835-rpi-b-rev2.dts
b/arch/arm/boot/dts/broadcom/bcm2835-rpi-b-rev2.dts
index ae6d3a9586ab..c1dac5d704aa 100644
--- a/arch

Re: [PATCH] ARM: dts: bcm2835: Enable 3D rendering through V3D

[Stefan Wahren]

Hi Phil,

Am 14.04.24 um 20:43 schrieb Phil Elwell:

Hello all,

On Fri, 12 Apr 2024 at 18:17, Stefan Wahren  wrote:

Hi Maíra,

[add Phil & Dave]

Am 12.04.24 um 15:25 schrieb Maíra Canal:

RPi 0-3 is packed with a GPU that provides 3D rendering capabilities to
the RPi. Currently, the downstream kernel uses an overlay to enable the
GPU and use GPU hardware acceleration. When deploying a mainline kernel
to the RPi 0-3, we end up without any GPU hardware acceleration
(essentially, we can't use the OpenGL driver).

Therefore, enable the V3D core for the RPi 0-3 in the mainline kernel.

thanks for trying to improve the combination Raspberry Pi OS + Mainline
Kernel. I think i'm able to reproduce the issue with Raspberry Pi 3 B +
on Buster.

Buster? We launched Buster with 4.19 and ended on 5.10. We've moved
onto Bookworm now. A lot has changed in that time...

Sorry, i meant Bullseye but yes it's not up to date. Anyway i cannot see
a problem with the devicetree.

Re: [PATCH] ARM: dts: bcm2835: Enable 3D rendering through V3D

[Stefan Wahren]

Hi Maíra,

[add Phil & Dave]

Am 12.04.24 um 15:25 schrieb Maíra Canal:

RPi 0-3 is packed with a GPU that provides 3D rendering capabilities to
the RPi. Currently, the downstream kernel uses an overlay to enable the
GPU and use GPU hardware acceleration. When deploying a mainline kernel
to the RPi 0-3, we end up without any GPU hardware acceleration
(essentially, we can't use the OpenGL driver).

Therefore, enable the V3D core for the RPi 0-3 in the mainline kernel.

thanks for trying to improve the combination Raspberry Pi OS + Mainline
Kernel. I think i'm able to reproduce the issue with Raspberry Pi 3 B +
on Buster. From the kernel side everything looks good:

[   11.054833] vc4-drm soc:gpu: bound 3f902000.hdmi (ops vc4_hdmi_ops [vc4])
[   11.055118] vc4-drm soc:gpu: bound 3f806000.vec (ops vc4_vec_ops [vc4])
[   11.055340] vc4-drm soc:gpu: bound 3f004000.txp (ops vc4_txp_ops [vc4])
[   11.055521] vc4-drm soc:gpu: bound 3f206000.pixelvalve (ops
vc4_crtc_ops [vc4])
[   11.055695] vc4-drm soc:gpu: bound 3f207000.pixelvalve (ops
vc4_crtc_ops [vc4])
[   11.055874] vc4-drm soc:gpu: bound 3f807000.pixelvalve (ops
vc4_crtc_ops [vc4])
[   11.056020] vc4-drm soc:gpu: bound 3fc0.v3d (ops vc4_v3d_ops [vc4])
[   11.063277] Bluetooth: hci0: BCM4345C0
'brcm/BCM4345C0.raspberrypi,3-model-b-plus.hcd' Patch
[   11.070466] [drm] Initialized vc4 0.0.0 20140616 for soc:gpu on minor 0
[   11.174803] Console: switching to colour frame buffer device 240x75
[   11.205125] vc4-drm soc:gpu: [drm] fb0: vc4drmfb frame buffer device

But in Raspberry Pi OS there is a systemd script which is trying to
check for the V3D driver /usr/lib/systemd/scripts/gldriver_test.sh
Within the first check "raspi-config nonint is_kms" is called, which
always seems to fail. If i run strace on this command it seems to check
for /proc/device-tree/soc/v3d@7ec0/status which doesn't exists in
the Mainline device tree.

Maybe there is a chance to improve the userspace tool?



Signed-off-by: Maíra Canal 
---

I decided to add the status property to the `bcm2835-common.dtsi`, but
there are two other options:

1. To add the status property to the `bcm2835-rpi-common.dtsi` file
2. To add the status property to each individual RPi model, e.g.
`bcm2837-rpi-3-b.dts`.

Let me know which option is more suitable, and if `bcm2835-common.dtsi`
is not the best option, I can send a v2.

Best Regards,
- Maíra

  arch/arm/boot/dts/broadcom/bcm2835-common.dtsi | 1 +
  1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi 
b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
index 9261b67dbee1..851a6bce1939 100644
--- a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
@@ -139,6 +139,7 @@ v3d: v3d@7ec0 {
compatible = "brcm,bcm2835-v3d";
reg = <0x7ec0 0x1000>;
interrupts = <1 10>;
+   status = "okay";
};

vc4: gpu {

[PATCH] drm/vc4: drv: Avoid possible NPD when booted without KMS

[Stefan Wahren]

From: Dom Cobley 

In case there is no matching KMS, the function vc4_match_add_driver
won't change the match pointer which is initialized with NULL.
Since component_master_add_with_match doesn't expected match
to be a NULL pointer, this results into a NPD.

Fixes: c8b75bca92cb ("drm/vc4: Add KMS support for Raspberry Pi.")
Signed-off-by: Dom Cobley 
Signed-off-by: Stefan Wahren 
---
 drivers/gpu/drm/vc4/vc4_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index c133e96b8aca..4f17840df9d3 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -439,6 +439,8 @@ static int vc4_platform_drm_probe(struct platform_device 
*pdev)

vc4_match_add_drivers(dev, &match,
  component_drivers, ARRAY_SIZE(component_drivers));
+   if (!match)
+   return -ENODEV;

return component_master_add_with_match(dev, &vc4_drm_ops, match);
 }
--
2.34.1

Re: [PATCH v2 0/2] Add waveshare 7inch touchscreen panel support

[Stefan Wahren]

Hi Neil,

Am 09.01.24 um 12:19 schrieb neil.armstr...@linaro.org:

Hi,

On 09/01/2024 08:09, Shengyang Chen wrote:

This patchset adds waveshare 7inch touchscreen panel support
for the StarFive JH7110 SoC.


Could you precise which SKU you're referring to ? is it 19885 =>
https://www.waveshare.com/7inch-dsi-lcd.htm ?

Are you sure it requires different timings from the RPi one ? In the
Waveshare
wiki it explicitly uses the rpi setup (vc4-kms-dsi-7inch) to drive it:
https://www.waveshare.com/wiki/7inch_DSI_LCD

i don't have an anser for your question, but the Raspberry Pi vendor
tree use different timings than the Mainline kernel:

https://github.com/raspberrypi/linux/commit/222b9baa97cc4c880d040a8c6a5da80d6a42c8e8

Additionally the
arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso suggests
that it uses the Raspberry Pi 7inch, but uses the timings of
powertip,ph800480t013-idf02 from panel-simple.

Maybe Shengyang could test these timings with the Waveshare touch. At
the end this rely on a proper implementation on the underlying drivers.

Sorry, for adding more confusion.

Regards


Neil




changes since v1:
- Rebased on tag v6.7.

patch 1:
- Gave up original changing.
- Changed the commit message.
- Add compatible in panel-simple.yaml

patch 2:
- Gave up original changing.
- Changed the commit message.
- Add new mode for the panel in panel-simple.c

v1:
https://patchwork.kernel.org/project/dri-devel/cover/20231124104451.44271-1-shengyang.c...@starfivetech.com/

Shengyang Chen (2):
   dt-bindings: display: panel: panel-simple: Add compatible property
for
 waveshare 7inch touchscreen panel
   gpu: drm: panel: panel-simple: add new display mode for waveshare
 7inch touchscreen panel

  .../bindings/display/panel/panel-simple.yaml  |  2 ++
  drivers/gpu/drm/panel/panel-simple.c  | 28 +++
  2 files changed, 30 insertions(+)

Re: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support

[Stefan Wahren]

Hi Shengyang,

[fix address of Emma]

Am 24.11.23 um 11:44 schrieb Shengyang Chen:

This patchset adds waveshare 7inch touchscreen panel support
for the StarFive JH7110 SoC.

Patch 1 add new compatible for the raspberrypi panel driver and its dt-binding.
Patch 2 add new display mode and new probing process for raspberrypi panel 
driver.

Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
which can be drived by raspberrypi panel driver.

The series has been tested on the VisionFive 2 board.

surprisingly i was recently working on the official Raspberry Pi
touchscreen and was able to get it running the new way.

What do i mean with the new way. There is almost nothing special to the
Raspberry Pi touchscreen, so we should try to use/extend existing
components like:

CONFIG_DRM_PANEL_SIMPLE
CONFIG_TOUCHSCREEN_EDT_FT5X06
CONFIG_DRM_TOSHIBA_TC358762

The only special part is the Attiny on the connector PCB which requires:

CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY

So the whole point is to avoid writing monolitic drivers for simple
panel like that.

There is a WIP branch based on top of Linux 6.7-rcX, which should
demonstrate this approach [1]. Unfortunately it is not ready for
upstreaming, but it has been tested on a Raspberry Pi 3 B Plus. Maybe
this is helpful for your case.

Actually i consider panel-raspberrypi-touchscreen.c as a dead end, which
shouldn't be extended.

Btw there are already DT overlays in mainline which seems to use the
Raspberry Pi 7inch panel (without touch function yet) [2].

[1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
[2] -
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474



Shengyang Chen (2):
   dt-bindings: display: panel: raspberrypi: Add compatible property for
 waveshare 7inch touchscreen panel
   gpu: drm: panel: raspberrypi: add new display mode and new probing
 process

  .../panel/raspberrypi,7inch-touchscreen.yaml  |  4 +-
  .../drm/panel/panel-raspberrypi-touchscreen.c | 99 ---
  2 files changed, 91 insertions(+), 12 deletions(-)

Re: [PATCH v2 3/4] dt-bindings: gpu: v3d: Add BCM2712's compatible

[Stefan Wahren]

Hi Iago,

Am 30.10.23 um 11:18 schrieb Iago Toral:

El lun, 30-10-2023 a las 10:57 +0100, Stefan Wahren escribió:

Hi Iago,

Am 30.10.23 um 09:28 schrieb Iago Toral Quiroga:

BCM2712, Raspberry Pi 5's SoC, contains a V3D core. So add its
specific
compatible to the bindings.

v2: new, requested by Stefan Wahren.

Thanks for sending this but the line above belongs below --- since
it's
not relevant after the patch has been applied.

Signed-off-by: Iago Toral Quiroga 

Unfortunately this patch will be ignored because the relevant
devicetree
people are missing to give their Ack.

Please use scripts/get_maintainers.pl


Sorry about that, my bad, should I resend the patch (as part of a v3)
or is it enough to add the relevant maintainers in a reply to the
original v2 patch?

since the merge window for 6.7 is open, please resent as part of v3.


Iago


Best regards

---
   Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml | 1 +
   1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-
v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
index dae55b8a267b..dc078ceeca9a 100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
@@ -17,6 +17,7 @@ properties:
     compatible:
   enum:
     - brcm,2711-v3d
+  - brcm,2712-v3d
     - brcm,7268-v3d
     - brcm,7278-v3d

Re: [PATCH v2 3/4] dt-bindings: gpu: v3d: Add BCM2712's compatible

[Stefan Wahren]

Hi Iago,

Am 30.10.23 um 09:28 schrieb Iago Toral Quiroga:

BCM2712, Raspberry Pi 5's SoC, contains a V3D core. So add its specific
compatible to the bindings.

v2: new, requested by Stefan Wahren.

Thanks for sending this but the line above belongs below --- since it's
not relevant after the patch has been applied.


Signed-off-by: Iago Toral Quiroga 

Unfortunately this patch will be ignored because the relevant devicetree
people are missing to give their Ack.

Please use scripts/get_maintainers.pl

Best regards

---
  Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml | 1 +
  1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml 
b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
index dae55b8a267b..dc078ceeca9a 100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
@@ -17,6 +17,7 @@ properties:
compatible:
  enum:
- brcm,2711-v3d
+  - brcm,2712-v3d
- brcm,7268-v3d
- brcm,7278-v3d

Re: [PATCH v2 2/4] drm/v3d: fix up register addresses for V3D 7.x

[Stefan Wahren]

Hi Iago,

Am 30.10.23 um 09:28 schrieb Iago Toral Quiroga:

This patch updates a number of register addresses that have
been changed in Raspberry Pi 5 (V3D 7.1) and updates the
code to use the corresponding registers and addresses based
on the actual V3D version.

v2:
  - added s-o-b and commit message. (Maíra Canal)
  - Used macro that takes version as argument and returns
appropriate values instead of two different definitions
for post-v71 and pre-v71 hardware when possible. (Maíra Canal)
  - fixed style warnings from checkpatch.pl. (Maíra Canal)

Signed-off-by: Iago Toral Quiroga 
---
  drivers/gpu/drm/v3d/v3d_debugfs.c | 178 +-
  drivers/gpu/drm/v3d/v3d_gem.c |   4 +-
  drivers/gpu/drm/v3d/v3d_irq.c |  46 
  drivers/gpu/drm/v3d/v3d_regs.h|  94 +---
  drivers/gpu/drm/v3d/v3d_sched.c   |  38 ---
  5 files changed, 204 insertions(+), 156 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c 
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 330669f51fa7..f843a50d5dce 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -12,69 +12,83 @@
  #include "v3d_drv.h"
  #include "v3d_regs.h"

-#define REGDEF(reg) { reg, #reg }
+#define REGDEF(min_ver, max_ver, reg) { min_ver, max_ver, reg, #reg }
  struct v3d_reg_def {
+   u32 min_ver;
+   u32 max_ver;

Is this documented some where which SoC has which V3D version?

Re: [PATCH v2 2/4] drm/v3d: fix up register addresses for V3D 7.x

[Stefan Wahren]

Hi Iago,

Am 30.10.23 um 11:14 schrieb Iago Toral:

Hi Stefan,

El lun, 30-10-2023 a las 10:58 +0100, Stefan Wahren escribió:

Hi Iago,

Am 30.10.23 um 09:28 schrieb Iago Toral Quiroga:

This patch updates a number of register addresses that have
been changed in Raspberry Pi 5 (V3D 7.1) and updates the
code to use the corresponding registers and addresses based
on the actual V3D version.

v2:
   - added s-o-b and commit message. (Maíra Canal)
   - Used macro that takes version as argument and returns
     appropriate values instead of two different definitions
     for post-v71 and pre-v71 hardware when possible. (Maíra Canal)
   - fixed style warnings from checkpatch.pl. (Maíra Canal)

Signed-off-by: Iago Toral Quiroga 
---
   drivers/gpu/drm/v3d/v3d_debugfs.c | 178 +
-
   drivers/gpu/drm/v3d/v3d_gem.c |   4 +-
   drivers/gpu/drm/v3d/v3d_irq.c |  46 
   drivers/gpu/drm/v3d/v3d_regs.h    |  94 +---
   drivers/gpu/drm/v3d/v3d_sched.c   |  38 ---
   5 files changed, 204 insertions(+), 156 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 330669f51fa7..f843a50d5dce 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -12,69 +12,83 @@
   #include "v3d_drv.h"
   #include "v3d_regs.h"

-#define REGDEF(reg) { reg, #reg }
+#define REGDEF(min_ver, max_ver, reg) { min_ver, max_ver, reg,
#reg }
   struct v3d_reg_def {
+   u32 min_ver;
+   u32 max_ver;

Is this documented some where which SoC has which V3D version?


Not that I am aware of.

There are really only two Raspberry Pi SoCs supported by v3d: bcm2711
is Raspberry Pi 4 which is V3D 4.2 (compatible with 4.1), and bcm2712
is Raspberry Pi 5 which is V3D 7.1.

okay, in this case the best source would be Emma's wiki [1]. Maybe these
should be added as well to the wiki.

[1] - https://github.com/anholt/linux/wiki/Devices-with-Videocore-graphics


I don't know what SoCs are supported by versions of V3D before 4.1, I
think those were targetting set-top-box hardware that Emma used while
setting up the driver before the SoC for Raspberry Pi 4 was available.

Iago

Re: [PATCH v2 4/4] drm/v3d: add brcm,2712-v3d as a compatible V3D device

[Stefan Wahren]

Hi Iago,

Am 30.10.23 um 09:28 schrieb Iago Toral Quiroga:

This is required to get the V3D module to load with Raspberry Pi 5.

v2:
  - added s-o-b and commit message. (Maíra)
  - keep order of compatible strings. (Stefan Wahren)

as in the other patch, please move the changelog below --- or in the
cover letter.

Except of this:

Reviewed-by: Stefan Wahren 


Signed-off-by: Iago Toral Quiroga 
---
  drivers/gpu/drm/v3d/v3d_drv.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index ffbbe9d527d3..1ab46bdf8ad7 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -187,6 +187,7 @@ static const struct drm_driver v3d_drm_driver = {

  static const struct of_device_id v3d_of_match[] = {
{ .compatible = "brcm,2711-v3d" },
+   { .compatible = "brcm,2712-v3d" },
{ .compatible = "brcm,7268-v3d" },
{ .compatible = "brcm,7278-v3d" },
{},

Re: [PATCH 3/3] drm/v3d: add brcm, 2712-v3d as a compatible V3D device

[Stefan Wahren]

Hi Iago,

additional to Maria's comments:

Please keep the order of the compatible strings.

Also you need to update the device tree binding before this patch:
Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml

Also make sure that the series is send to the maintainers, not just
dri-devel by using scripts/get_maintainer.pl

Best regards

Re: [PATCH] drm/vc4: drop all currently held locks if deadlock happens

[Stefan Wahren]

Hi Maíra,

Am 29.12.22 um 20:46 schrieb Maíra Canal:

If vc4_hdmi_reset_link() returns -EDEADLK, it means that a deadlock
happened in the locking context. This situation should be addressed by
dropping all currently held locks and block until the contended lock
becomes available. Currently, vc4 is not dealing with the deadlock
properly, producing the following output when PROVE_LOCKING is enabled:

[  825.612809] [ cut here ]
[  825.612852] WARNING: CPU: 1 PID: 116 at 
drivers/gpu/drm/drm_modeset_lock.c:276 drm_modeset_drop_locks+0x60/0x68 [drm]
[  825.613458] Modules linked in: 8021q mrp garp stp llc
raspberrypi_cpufreq brcmfmac brcmutil crct10dif_ce hci_uart cfg80211
btqca btbcm bluetooth vc4 raspberrypi_hwmon snd_soc_hdmi_codec cec
clk_raspberrypi ecdh_generic drm_display_helper ecc rfkill
drm_dma_helper drm_kms_helper pwm_bcm2835 bcm2835_thermal bcm2835_rng
rng_core i2c_bcm2835 drm fuse ip_tables x_tables ipv6
[  825.613735] CPU: 1 PID: 116 Comm: kworker/1:2 Tainted: GW 
6.1.0-rc6-01399-g941aae326315 #3
[  825.613759] Hardware name: Raspberry Pi 3 Model B Rev 1.2 (DT)
[  825.613777] Workqueue: events output_poll_execute [drm_kms_helper]
[  825.614038] pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  825.614063] pc : drm_modeset_drop_locks+0x60/0x68 [drm]
[  825.614603] lr : drm_helper_probe_detect+0x120/0x1b4 [drm_kms_helper]
[  825.614829] sp : 88313bf0
[  825.614844] x29: 88313bf0 x28: cd7778b8b000 x27: 
[  825.614883] x26: 0001 x25: 0001 x24: 677cc35c2758
[  825.614920] x23: cd7707d01430 x22: cd7707c3edc7 x21: 0001
[  825.614958] x20:  x19: 88313c10 x18: b6d3
[  825.614995] x17: cd777835e214 x16: cdcef870 x15: f810
[  825.615033] x14:  x13: 0099 x12: 0002
[  825.615070] x11: 72917988020af800 x10: 72917988020af800 x9 : 72917988020af800
[  825.615108] x8 : 677cc665e0a8 x7 : d00a8c18110c x6 : cd4c0054
[  825.615145] x5 :  x4 : 0001 x3 : 
[  825.615181] x2 : 677cc55e1880 x1 : cdcef8ec x0 : 88313c10
[  825.615219] Call trace:
[  825.615232]  drm_modeset_drop_locks+0x60/0x68 [drm]
[  825.615773]  drm_helper_probe_detect+0x120/0x1b4 [drm_kms_helper]
[  825.616003]  output_poll_execute+0xe4/0x224 [drm_kms_helper]
[  825.616233]  process_one_work+0x2b4/0x618
[  825.616264]  worker_thread+0x24c/0x464
[  825.616288]  kthread+0xec/0x110
[  825.616310]  ret_from_fork+0x10/0x20
[  825.616335] irq event stamp: 7634
[  825.616349] hardirqs last  enabled at (7633): [] 
_raw_spin_unlock_irq+0x3c/0x78
[  825.616384] hardirqs last disabled at (7634): [] 
__schedule+0x134/0x9f0
[  825.616411] softirqs last  enabled at (7630): [] 
local_bh_enable+0x4/0x30 [ipv6]
[  825.617019] softirqs last disabled at (7618): [] 
local_bh_disable+0x4/0x30 [ipv6]
[  825.617586] ---[ end trace  ]---

Therefore, deal with the deadlock as suggested by [1], using the
function drm_modeset_backoff().

[1] https://docs.kernel.org/gpu/drm-kms.html?highlight=kms#kms-locking

Fixes: 6bed2ea3cb38 ("drm/vc4: hdmi: Reset link on hotplug")
Reported-by: Stefan Wahren 
Signed-off-by: Maíra Canal 


i cannot provide a review, but at least you can have my:

Tested-by: Stefan Wahren 

Re: WARNING: CPU: 2 PID: 42 at drivers/gpu/drm/drm_modeset_lock.c:276

[Stefan Wahren]

Hi Maíra,

Am 28.12.22 um 20:49 schrieb Maíra Canal:

Hi Stefan,

I was able to reproduce this error on drm-misc-next. I bisected,
and I got into commit 6bed2ea3cb38. I noticed that the crtc->mutex is
being locked twice, and this might be causing the problem. I wrote a
patch to try to fix this issue, and after applying the patch, I wasn't
able to reproduce the error anymore.

Let me know if you were able to reproduce the warning after applying 
this patch.


the patch works as expected and avoid the warning. I tested it on top of 
v6.1 with RPi 3 B+ and RPi 4 B.


In case you send the patch please add the Fixes tag so the patch get 
backported to stable.


Thanks a lot

Stefan



Best Regards,
- Maíra Canal

---

From f6c910327d060e2314947e7e456db363a6164a49 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ADra=20Canal?= 
Date: Wed, 28 Dec 2022 16:14:34 -0300
Subject: [PATCH] drm/vc4: don't lock crtc's mutex on reset link
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Current, crtc->mutex is being locked twice: first, by
vc4_hdmi_reset_link() and them, internally, by
drm_atomic_get_crtc_state(). This produces the following output when
PROVE_LOCKING is enabled:

[  825.612809] [ cut here ]
[  825.612852] WARNING: CPU: 1 PID: 116 at 
drivers/gpu/drm/drm_modeset_lock.c:276 
drm_modeset_drop_locks+0x60/0x68 [drm]

[  825.613458] Modules linked in: 8021q mrp garp stp llc
raspberrypi_cpufreq brcmfmac brcmutil crct10dif_ce hci_uart cfg80211
btqca btbcm bluetooth vc4 raspberrypi_hwmon snd_soc_hdmi_codec cec
clk_raspberrypi ecdh_generic drm_display_helper ecc rfkill
drm_dma_helper drm_kms_helper pwm_bcm2835 bcm2835_thermal bcm2835_rng
rng_core i2c_bcm2835 drm fuse ip_tables x_tables ipv6
[  825.613735] CPU: 1 PID: 116 Comm: kworker/1:2 Tainted: G W 
6.1.0-rc6-01399-g941aae326315 #3

[  825.613759] Hardware name: Raspberry Pi 3 Model B Rev 1.2 (DT)
[  825.613777] Workqueue: events output_poll_execute [drm_kms_helper]
[  825.614038] pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS 
BTYPE=--)

[  825.614063] pc : drm_modeset_drop_locks+0x60/0x68 [drm]
[  825.614603] lr : drm_helper_probe_detect+0x120/0x1b4 [drm_kms_helper]
[  825.614829] sp : 88313bf0
[  825.614844] x29: 88313bf0 x28: cd7778b8b000 x27: 

[  825.614883] x26: 0001 x25: 0001 x24: 
677cc35c2758
[  825.614920] x23: cd7707d01430 x22: cd7707c3edc7 x21: 
0001
[  825.614958] x20:  x19: 88313c10 x18: 
b6d3
[  825.614995] x17: cd777835e214 x16: cdcef870 x15: 
f810
[  825.615033] x14:  x13: 0099 x12: 
0002
[  825.615070] x11: 72917988020af800 x10: 72917988020af800 x9 : 
72917988020af800
[  825.615108] x8 : 677cc665e0a8 x7 : d00a8c18110c x6 : 
cd4c0054
[  825.615145] x5 :  x4 : 0001 x3 : 

[  825.615181] x2 : 677cc55e1880 x1 : cdcef8ec x0 : 
88313c10

[  825.615219] Call trace:
[  825.615232]  drm_modeset_drop_locks+0x60/0x68 [drm]
[  825.615773]  drm_helper_probe_detect+0x120/0x1b4 [drm_kms_helper]
[  825.616003]  output_poll_execute+0xe4/0x224 [drm_kms_helper]
[  825.616233]  process_one_work+0x2b4/0x618
[  825.616264]  worker_thread+0x24c/0x464
[  825.616288]  kthread+0xec/0x110
[  825.616310]  ret_from_fork+0x10/0x20
[  825.616335] irq event stamp: 7634
[  825.616349] hardirqs last  enabled at (7633): [] 
_raw_spin_unlock_irq+0x3c/0x78
[  825.616384] hardirqs last disabled at (7634): [] 
__schedule+0x134/0x9f0
[  825.616411] softirqs last  enabled at (7630): [] 
local_bh_enable+0x4/0x30 [ipv6]
[  825.617019] softirqs last disabled at (7618): [] 
local_bh_disable+0x4/0x30 [ipv6]

[  825.617586] ---[ end trace  ]---

So, don't lock crtc->mutex inside the driver and let the right lock be
automatically acquired by drm_atomic_get_crtc_state().

Reported-by: Stefan Wahren 
Signed-off-by: Maíra Canal 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c 
b/drivers/gpu/drm/vc4/vc4_hdmi.c

index 0d3313c71f2f..b3b1958b5f4d 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -340,10 +340,6 @@ static int vc4_hdmi_reset_link(struct 
drm_connector *connector,

 if (!crtc)
 return 0;

-    ret = drm_modeset_lock(&crtc->mutex, ctx);
-    if (ret)
-    return ret;
-
 crtc_state = crtc->state;
 if (!crtc_state->active)
 return 0;

Re: WARNING: CPU: 2 PID: 42 at drivers/gpu/drm/drm_modeset_lock.c:276

[Stefan Wahren]

Hi,

Am 21.12.22 um 20:46 schrieb Stefan Wahren:

Hi,

if i enable PROVE_LOCKING on the Raspberry Pi 3 B+ 
(arm/multi_v7_defconfig) using v6.1 (didn't test older versions) i'm 
getting the following warning:


[  204.043396] WARNING: CPU: 2 PID: 42 at 
drivers/gpu/drm/drm_modeset_lock.c:276 drm_modeset_drop_locks+0x6c/0x70
[  204.043426] Modules linked in: aes_arm aes_generic cmac 
bcm2835_v4l2(C) bcm2835_mmal_vchiq(C) videobuf2_vmalloc 
videobuf2_memops videobuf2_v4l2 videobuf2_common videodev mc 
snd_bcm2835(C) crc32_arm_ce brcmfmac brcmutil vc4 snd_soc_hdmi_codec 
sha256_generic libsha256 snd_soc_core ac97_bus snd_pcm_dmaengine 
snd_pcm sha256_arm snd_timer cfg80211 onboard_usb_hub snd hci_uart 
raspberrypi_hwmon soundcore drm_dma_helper btbcm bluetooth 
ecdh_generic bcm2835_thermal ecc libaes vchiq(C) microchip lan78xx
[  204.043820] CPU: 2 PID: 42 Comm: kworker/2:1 Tainted: G C 
6.1.0-7-g22fada783b9f #31

[  204.043833] Hardware name: BCM2835
[  204.043842] Workqueue: events output_poll_execute
[  204.043866]  unwind_backtrace from show_stack+0x10/0x14
[  204.043886]  show_stack from dump_stack_lvl+0x58/0x70
[  204.043903]  dump_stack_lvl from __warn+0xc8/0x1e8
[  204.043920]  __warn from warn_slowpath_fmt+0x5c/0xb8
[  204.043936]  warn_slowpath_fmt from drm_modeset_drop_locks+0x6c/0x70
[  204.043952]  drm_modeset_drop_locks from 
drm_helper_probe_detect_ctx+0xd4/0x124
[  204.043969]  drm_helper_probe_detect_ctx from 
output_poll_execute+0x154/0x230

[  204.043986]  output_poll_execute from process_one_work+0x288/0x708
[  204.044004]  process_one_work from worker_thread+0x54/0x50c
[  204.044020]  worker_thread from kthread+0xe8/0x104
[  204.044034]  kthread from ret_from_fork+0x14/0x2c
[  204.044048] Exception stack(0xf0915fb0 to 0xf0915ff8)
[  204.044059] 5fa0:  
  
[  204.044070] 5fc0:      
  
[  204.044080] 5fe0:     0013 


[  204.044090] irq event stamp: 33189
[  204.044100] hardirqs last  enabled at (33195): [] 
__up_console_sem+0x50/0x60
[  204.044120] hardirqs last disabled at (33200): [] 
__up_console_sem+0x3c/0x60
[  204.044136] softirqs last  enabled at (32836): [] 
process_one_work+0x288/0x708
[  204.044152] softirqs last disabled at (32832): [] 
neigh_managed_work+0x18/0xa4

[  204.044168] ---[ end trace  ]---


here are my intermediate results. I'm stuck during bisecting. For the 
skipped steps either vc4 throw compile errors or X doesn't come up 
(display goes black before X and says no connection).


$ git bisect log
git bisect start
# good: [4fe89d07dcc2804c8b562f6c7896a45643d34b2f] Linux 6.0
git bisect good 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
# bad: [33e591dee915832c618cf68bb1058c8e7d296128] Merge tag 
'phy-for-6.1' of git://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy

git bisect bad 33e591dee915832c618cf68bb1058c8e7d296128
# good: [a47e60729d9624e931f988709ab76e043e2ee8b9] Merge tag 
'backlight-next-6.1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight

git bisect good a47e60729d9624e931f988709ab76e043e2ee8b9
# bad: [ff6862c23d2e83d12d1759bf4337d41248fb4dc8] Merge tag 
'arm-drivers-6.1' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc

git bisect bad ff6862c23d2e83d12d1759bf4337d41248fb4dc8
# good: [95d8c67187bcfaa519bafcdef9091cd906505454] Merge tag 
'drm-msm-next-2022-09-22' of https://gitlab.freedesktop.org/drm/msm into 
drm-next

git bisect good 95d8c67187bcfaa519bafcdef9091cd906505454
# good: [86a4d29e75540e20f991e72f17aa51d0e775a397] Merge tag 'asoc-v6.1' 
of https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into 
for-linus

git bisect good 86a4d29e75540e20f991e72f17aa51d0e775a397
# bad: [65898687cf7392c372ea8d04a88617e2cb794465] Merge tag 
'amd-drm-next-6.1-2022-09-30' of 
https://gitlab.freedesktop.org/agd5f/linux into drm-next

git bisect bad 65898687cf7392c372ea8d04a88617e2cb794465
# good: [2d89e2ddfd00ca569dd73883c7c70badbd57f4ac] drm/amdgpu: fix 
compiler warning for amdgpu_gfx_cp_init_microcode

git bisect good 2d89e2ddfd00ca569dd73883c7c70badbd57f4ac
# bad: [1637c315282e97efcb95cc7dfafbb15efa9fa27f] drm/mediatek: dp: Fix 
compiler warning in mtk_dp_video_mute()

git bisect bad 1637c315282e97efcb95cc7dfafbb15efa9fa27f
# skip: [b78e5d830f0db8e6d998cdc5a2b7b807cf463f99] drm/tests: Set also 
mock plane src_x, src_y, src_w and src_h

git bisect skip b78e5d830f0db8e6d998cdc5a2b7b807cf463f99
# skip: [213cb76ddc8b875e772f9f4d173feefa122716af] Merge tag 
'drm-intel-gt-next-2022-09-09' of 
git://anongit.freedesktop.org/drm/drm-intel into drm-next

git bisect skip 213cb76ddc8b875e772f9f4d173feefa122716af
# good: [4f96b1bc156e7076f6efedc2a76a8c7e897c7977] drm/todo: Add entry 
about dealing with brightness control 

WARNING: CPU: 2 PID: 42 at drivers/gpu/drm/drm_modeset_lock.c:276

[Stefan Wahren]

Hi,

if i enable PROVE_LOCKING on the Raspberry Pi 3 B+ 
(arm/multi_v7_defconfig) using v6.1 (didn't test older versions) i'm 
getting the following warning:


[  204.043396] WARNING: CPU: 2 PID: 42 at 
drivers/gpu/drm/drm_modeset_lock.c:276 drm_modeset_drop_locks+0x6c/0x70
[  204.043426] Modules linked in: aes_arm aes_generic cmac 
bcm2835_v4l2(C) bcm2835_mmal_vchiq(C) videobuf2_vmalloc videobuf2_memops 
videobuf2_v4l2 videobuf2_common videodev mc snd_bcm2835(C) crc32_arm_ce 
brcmfmac brcmutil vc4 snd_soc_hdmi_codec sha256_generic libsha256 
snd_soc_core ac97_bus snd_pcm_dmaengine snd_pcm sha256_arm snd_timer 
cfg80211 onboard_usb_hub snd hci_uart raspberrypi_hwmon soundcore 
drm_dma_helper btbcm bluetooth ecdh_generic bcm2835_thermal ecc libaes 
vchiq(C) microchip lan78xx
[  204.043820] CPU: 2 PID: 42 Comm: kworker/2:1 Tainted: G C 
6.1.0-7-g22fada783b9f #31

[  204.043833] Hardware name: BCM2835
[  204.043842] Workqueue: events output_poll_execute
[  204.043866]  unwind_backtrace from show_stack+0x10/0x14
[  204.043886]  show_stack from dump_stack_lvl+0x58/0x70
[  204.043903]  dump_stack_lvl from __warn+0xc8/0x1e8
[  204.043920]  __warn from warn_slowpath_fmt+0x5c/0xb8
[  204.043936]  warn_slowpath_fmt from drm_modeset_drop_locks+0x6c/0x70
[  204.043952]  drm_modeset_drop_locks from 
drm_helper_probe_detect_ctx+0xd4/0x124
[  204.043969]  drm_helper_probe_detect_ctx from 
output_poll_execute+0x154/0x230

[  204.043986]  output_poll_execute from process_one_work+0x288/0x708
[  204.044004]  process_one_work from worker_thread+0x54/0x50c
[  204.044020]  worker_thread from kthread+0xe8/0x104
[  204.044034]  kthread from ret_from_fork+0x14/0x2c
[  204.044048] Exception stack(0xf0915fb0 to 0xf0915ff8)
[  204.044059] 5fa0:  
  
[  204.044070] 5fc0:      
  

[  204.044080] 5fe0:     0013 
[  204.044090] irq event stamp: 33189
[  204.044100] hardirqs last  enabled at (33195): [] 
__up_console_sem+0x50/0x60
[  204.044120] hardirqs last disabled at (33200): [] 
__up_console_sem+0x3c/0x60
[  204.044136] softirqs last  enabled at (32836): [] 
process_one_work+0x288/0x708
[  204.044152] softirqs last disabled at (32832): [] 
neigh_managed_work+0x18/0xa4

[  204.044168] ---[ end trace  ]---

Best regards

Re: [PATCH 1/2] drm/vc4: hdmi: Enforce the minimum rate at runtime_resume

[Stefan Wahren]

Am 11.11.22 um 22:08 schrieb Stefan Wahren:

Hi Maxime,

Am 29.09.22 um 11:21 schrieb Maxime Ripard:

This is a revert of commit fd5894fa2413 ("drm/vc4: hdmi: Remove clock
rate initialization"), with the code slightly moved around.

It turns out that we can't downright remove that code from the driver,
since the Pi0-3 and Pi4 are in different cases, and it only works for
the Pi4.

Indeed, the commit mentioned above was relying on the RaspberryPi
firmware clocks driver to initialize the rate if it wasn't done by the
firmware. However, the Pi0-3 are using the clk-bcm2835 clock driver that
wasn't doing this initialization. We therefore end up with the clock not
being assigned a rate, and the CPU stalling when trying to access a
register.

We can't move that initialization in the clk-bcm2835 driver, since the
HSM clock we depend on is actually part of the HDMI power domain, so any
rate setup is only valid when the power domain is enabled. Thus, we
reinstated the minimum rate setup at runtime_suspend, which should
address both issues.

Link: 
https://lore.kernel.org/dri-devel/20220922145448.w3xfywkn5ecak...@pengutronix.de/

Fixes: fd5894fa2413 ("drm/vc4: hdmi: Remove clock rate initialization")
Reported-by: Marc Kleine-Budde 
Signed-off-by: Maxime Ripard 
---
  drivers/gpu/drm/vc4/vc4_hdmi.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c 
b/drivers/gpu/drm/vc4/vc4_hdmi.c

index 199bc398817f..2e28fe16ed5e 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2891,6 +2891,15 @@ static int vc4_hdmi_runtime_resume(struct 
device *dev)

  u32 __maybe_unused value;
  int ret;
  +    /*
+ * The HSM clock is in the HDMI power domain, so we need to set
+ * its frequency while the power domain is active so that it
+ * keeps its rate.
+ */
+    ret = clk_set_min_rate(vc4_hdmi->hsm_clock, HSM_MIN_CLOCK_FREQ);
+    if (ret)
+    return ret;
+


unfortunately this breaks X on Raspberry Pi 4 in Linux 6.0.5 
(multi_v7_defconfig + LPAE). Today i saw this report [1] and bisected 
the issue down to this patch. Shame on me that i only tested this 
patch with Rpi 3B+ :-(
Looks like "drm/vc4: hdmi: Fix HSM clock too low on Pi4" addresses this 
issue ...


Best regards

[1] - https://bugzilla.suse.com/show_bug.cgi?id=1205259


  ret = clk_prepare_enable(vc4_hdmi->hsm_clock);
  if (ret)
  return ret;

Re: [PATCH 1/2] drm/vc4: hdmi: Enforce the minimum rate at runtime_resume

[Stefan Wahren]

Hi Maxime,

Am 29.09.22 um 11:21 schrieb Maxime Ripard:

This is a revert of commit fd5894fa2413 ("drm/vc4: hdmi: Remove clock
rate initialization"), with the code slightly moved around.

It turns out that we can't downright remove that code from the driver,
since the Pi0-3 and Pi4 are in different cases, and it only works for
the Pi4.

Indeed, the commit mentioned above was relying on the RaspberryPi
firmware clocks driver to initialize the rate if it wasn't done by the
firmware. However, the Pi0-3 are using the clk-bcm2835 clock driver that
wasn't doing this initialization. We therefore end up with the clock not
being assigned a rate, and the CPU stalling when trying to access a
register.

We can't move that initialization in the clk-bcm2835 driver, since the
HSM clock we depend on is actually part of the HDMI power domain, so any
rate setup is only valid when the power domain is enabled. Thus, we
reinstated the minimum rate setup at runtime_suspend, which should
address both issues.

Link: 
https://lore.kernel.org/dri-devel/20220922145448.w3xfywkn5ecak...@pengutronix.de/
Fixes: fd5894fa2413 ("drm/vc4: hdmi: Remove clock rate initialization")
Reported-by: Marc Kleine-Budde 
Signed-off-by: Maxime Ripard 
---
  drivers/gpu/drm/vc4/vc4_hdmi.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 199bc398817f..2e28fe16ed5e 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2891,6 +2891,15 @@ static int vc4_hdmi_runtime_resume(struct device *dev)
u32 __maybe_unused value;
int ret;
  
+	/*

+* The HSM clock is in the HDMI power domain, so we need to set
+* its frequency while the power domain is active so that it
+* keeps its rate.
+*/
+   ret = clk_set_min_rate(vc4_hdmi->hsm_clock, HSM_MIN_CLOCK_FREQ);
+   if (ret)
+   return ret;
+


unfortunately this breaks X on Raspberry Pi 4 in Linux 6.0.5 
(multi_v7_defconfig + LPAE). Today i saw this report [1] and bisected 
the issue down to this patch. Shame on me that i only tested this patch 
with Rpi 3B+ :-(


Best regards

[1] - https://bugzilla.suse.com/show_bug.cgi?id=1205259


ret = clk_prepare_enable(vc4_hdmi->hsm_clock);
if (ret)
return ret;

Re: [PATCH v2 3/7] firmware: raspberrypi: Provide a helper to query a clock max rate

[Stefan Wahren]

Hi Maxime,

Am 20.09.22 um 14:50 schrieb Maxime Ripard:

The firmware allows to query for its clocks the operating range of a
given clock. We'll need this for some drivers (KMS, in particular) to
infer the state of some configuration options, so let's create a
function to do so.

Signed-off-by: Maxime Ripard 

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index b916e1e171f8..c4b9ea70f5a7 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -228,6 +228,21 @@ static void rpi_register_clk_driver(struct device *dev)
-1, NULL, 0);
  }
  
+unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw, unsigned int id)

+{
+   struct rpi_firmware_clk_rate_request msg =
+   RPI_FIRMWARE_CLK_RATE_REQUEST(id);
+   int ret;
+
+   ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
+   &msg, sizeof(msg));
+   if (ret)
+   return 0;
+
+   return le32_to_cpu(msg.rate);
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_clk_get_max_rate);
+
  static void rpi_firmware_delete(struct kref *kref)
  {
struct rpi_firmware *fw = container_of(kref, struct rpi_firmware,
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h 
b/include/soc/bcm2835/raspberrypi-firmware.h
index 74c7bcc1ac2a..10248c370229 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -154,12 +154,32 @@ enum rpi_firmware_clk_id {
RPI_FIRMWARE_NUM_CLK_ID,
  };
  
+/**

+ * struct rpi_firmware_clk_rate_request - Firmware Request for a rate
+ * @id:ID of the clock being queried
+ * @rate: Rate in Hertz. Set by the firmware.
+ *
+ * Used by @RPI_FIRMWARE_GET_CLOCK_RATE, @RPI_FIRMWARE_GET_CLOCK_MEASURED,
+ * @RPI_FIRMWARE_GET_MAX_CLOCK_RATE and @RPI_FIRMWARE_GET_MIN_CLOCK_RATE.
+ */
+struct rpi_firmware_clk_rate_request {
+   __le32 id;
+   __le32 rate;
+} __packed;
+
+#define RPI_FIRMWARE_CLK_RATE_REQUEST(_id) \
+   {   \
+   .id = _id,  \
+   }
+
  #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
  int rpi_firmware_property(struct rpi_firmware *fw,
  u32 tag, void *data, size_t len);
  int rpi_firmware_property_list(struct rpi_firmware *fw,
   void *data, size_t tag_size);
  void rpi_firmware_put(struct rpi_firmware *fw);
+unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw,
+  unsigned int id);
  struct device_node *rpi_firmware_find_node(void);
  struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
  struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
@@ -179,6 +199,12 @@ static inline int rpi_firmware_property_list(struct 
rpi_firmware *fw,
  
  static inline void rpi_firmware_put(struct rpi_firmware *fw) { }
  
+static inline unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw,

+unsigned int id)
+{
+   return UINT_MAX;
In case the driver is disabled the function return UINT_MAX, but in case 
the firmware doesn't support RPI_FIRMWARE_GET_MAX_CLOCK_RATE it returns 
0. This looks a little bit inconsistent to me.

+}
+
  static inline struct device_node *rpi_firmware_find_node(void)
  {
return NULL;

Re: [PATCH 0/2] drm/vc4: hdmi: Fixes for the RaspberryPi 0-3 stalls

[Stefan Wahren]

Am 10.10.22 um 10:50 schrieb Maxime Ripard:

Hi Mark, Stefan,

On Thu, Sep 29, 2022 at 11:21:16AM +0200, Maxime Ripard wrote:

Here's a series addressing the current bug that has been reported for the
RaspberryPi3, where booting without an HDMI monitor attached and with vc4
compiled as a module will lead to a stuck system when the module is loaded.

This is due to the HSM clock not being initialized by anyone, and thus not
being functional once we start accessing the HDMI registers.

The first patch will fix this, and the second will make sure we avoid that
situation entirely in the future. This has been tested with 5.19.12. Earlier
versions might need a backport of 88110a9f6209 ("clk: bcm2835: fix
bcm2835_clock_choose_div").

Could you test/review this?


Looks good to me:

Tested-by: Stefan Wahren 



Thanks
Maxime

Re: Raspberry Pi 3 Model B+ hangs in vc4_hdmi_runtime_resume()

[Stefan Wahren]

Am 29.09.22 um 11:35 schrieb Maxime Ripard:

On Tue, Sep 27, 2022 at 03:15:17PM +0200, Maxime Ripard wrote:

On Tue, Sep 27, 2022 at 02:25:12PM +0200, Maxime Ripard wrote:

On Tue, Sep 27, 2022 at 01:42:40PM +0200, Maxime Ripard wrote:

On Tue, Sep 27, 2022 at 01:12:35PM +0200, Stefan Wahren wrote:

Am 27.09.22 um 11:42 schrieb Maxime Ripard:

On Tue, Sep 27, 2022 at 09:25:54AM +0200, Maxime Ripard wrote:

Hi Stefan,

On Mon, Sep 26, 2022 at 08:50:12PM +0200, Stefan Wahren wrote:

Am 26.09.22 um 14:47 schrieb Maxime Ripard:

On Mon, Sep 26, 2022 at 02:40:48PM +0200, Marc Kleine-Budde wrote:

On 26.09.2022 14:08:04, Stefan Wahren wrote:

Hi Marc,

Am 26.09.22 um 12:21 schrieb Marc Kleine-Budde:

On 22.09.2022 17:06:00, Maxime Ripard wrote:

I'm on a Raspberry Pi 3 Model B+ running current Debian testing ARM64,
using Debian's v5.19 kernel (Debian's v5.18 was working flawless).

| [0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
| [0.00] Linux version 5.19.0-1-arm64 (debian-ker...@lists.debian.org) 
(gcc-11 (Debian 11.3.0-5) 11.3.0, GNU ld (GNU Binutils for Debian) 
2.38.90.20220713) #1 SMP Debian 5.19.6-1 (2022-0
9-01)
| [0.00] Machine model: Raspberry Pi 3 Model B+
| [3.747500] raspberrypi-firmware soc:firmware: Attached to firmware from 
2022-03-24T13:21:11

As soon a the vc4 module is loaded the following warnings hits 4
times, then the machine stops.

[...]


The warning itself is fixed, both upstream and in stable (5.19.7).

Ok. Debian is using 5.19.6


It shouldn't have any relation to the hang though. Can you share your
setup?

- config.txt:

>8>8>8>8
gpu_mem=16
disable_splash=1

arm_64bit=1
enable_uart=1
uart_2ndstage=1

os_prefix=/u-boot/

[pi3]
force_turbo=1
>8>8>8>8

- Raspberry Pi 3 Model B+
- no HDMI connected

Does it mean, the issue only occurs without HDMI connected?
If you didn't test with HDMI yet, could you please do?

The error occurs with HDMI not connected, as vc4 is the gfx driver I
thought this might be of interest. :)

I don't have a HDMI monitor here, but I'll come back to you as soon as I
get access to one (might take some time).

It's not the first time an issue like this one would occur. I'm trying
to make my Pi3 boot again, and will try to bisect the issue.

yes the issue is only triggered without HDMI connected. I was able to
reproduce with an older vc4 firmware from 2020 (don't want to upgrade yet).
Kernel was also an arm64 build with defconfig.

Here some rough starting point for bisection:

5.18.0 good
5.19.0 bad
5.19.6 bad

Sorry it took a bit of time, it looks like I found another bug while
trying to test this yesterday.

Your datapoints are interesting though. I have a custom configuration
and it does boot 5.19 without an HDMI connected.

So I guess it leaves us with either the firmware version being different
(I'm using a newer version, from March 2022), or the configuration. I'll
test with defconfig.

So it turns out compiling vc4 as a module is the culprit.

Do you mean regardless of the kernel version in your case?

No, I mean that, with vc4 as a module, 5.18 works but 5.19 doesn't, like
Marc said. But if vc4 is built in, both work.


In my test cases i build vc4 always as module.


It's not clear to me why at this point, but the first register write in
vc4_hdmi_reset stalls.

Sounds like timing issue or a missing dependency (clock or power domain)

It felt like a clock or power domain issue to me indeed, but adding
clk_ignore_unused and pd_ignore_unused isn't enough, so it's probably
something a bit more complicated than just the clock / PD being
disabled.

I found the offending patch:
https://lore.kernel.org/dri-devel/20220225143534.405820-13-max...@cerno.tech/

That code was removed because it was made irrelevant by that earlier patch:
https://lore.kernel.org/dri-devel/20220225143534.405820-10-max...@cerno.tech/

But it turns out that while it works when the driver is built-in, it
doesn't when it's a module. If we add a clk_hw_get_rate() call right
after that call to raspberrypi_fw_set_rate(), the rate returned is 0.

I'm not entirely sure why, but I wonder if it's related to:
https://github.com/raspberrypi/linux/issues/4962#issuecomment-1228593439

Turns out it's not, since the Pi3 is using the clk-bcm2835 driver.

However, even reverting that patch fails. clk_set_min_rate fails because
the rate is protected, but it doesn't look like it is anywhere for that
clock, so I'm a bit confused.

Even if we do remove the clock protection check in
clk_core_set_rate_nolock(), clk_calc_new_rates() will then fail because
the bcm2835 driver will round the clock rate below the minimum, which is
rejected.

I'm not entirely sure what to do at this point. I guess the proper fix
would be to:
   - Figure out why it'

Re: Raspberry Pi 3 Model B+ hangs in vc4_hdmi_runtime_resume()

[Stefan Wahren]

Hi Maxime,

Am 27.09.22 um 15:15 schrieb Maxime Ripard:

On Tue, Sep 27, 2022 at 02:25:12PM +0200, Maxime Ripard wrote:

On Tue, Sep 27, 2022 at 01:42:40PM +0200, Maxime Ripard wrote:

On Tue, Sep 27, 2022 at 01:12:35PM +0200, Stefan Wahren wrote:

Am 27.09.22 um 11:42 schrieb Maxime Ripard:

On Tue, Sep 27, 2022 at 09:25:54AM +0200, Maxime Ripard wrote:

Hi Stefan,

On Mon, Sep 26, 2022 at 08:50:12PM +0200, Stefan Wahren wrote:

Am 26.09.22 um 14:47 schrieb Maxime Ripard:

On Mon, Sep 26, 2022 at 02:40:48PM +0200, Marc Kleine-Budde wrote:

On 26.09.2022 14:08:04, Stefan Wahren wrote:

Hi Marc,

Am 26.09.22 um 12:21 schrieb Marc Kleine-Budde:

On 22.09.2022 17:06:00, Maxime Ripard wrote:

I'm on a Raspberry Pi 3 Model B+ running current Debian testing ARM64,
using Debian's v5.19 kernel (Debian's v5.18 was working flawless).

| [0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
| [0.00] Linux version 5.19.0-1-arm64 (debian-ker...@lists.debian.org) 
(gcc-11 (Debian 11.3.0-5) 11.3.0, GNU ld (GNU Binutils for Debian) 
2.38.90.20220713) #1 SMP Debian 5.19.6-1 (2022-0
9-01)
| [0.00] Machine model: Raspberry Pi 3 Model B+
| [3.747500] raspberrypi-firmware soc:firmware: Attached to firmware from 
2022-03-24T13:21:11

As soon a the vc4 module is loaded the following warnings hits 4
times, then the machine stops.

[...]


The warning itself is fixed, both upstream and in stable (5.19.7).

Ok. Debian is using 5.19.6


It shouldn't have any relation to the hang though. Can you share your
setup?

- config.txt:

>8>8>8>8
gpu_mem=16
disable_splash=1

arm_64bit=1
enable_uart=1
uart_2ndstage=1

os_prefix=/u-boot/

[pi3]
force_turbo=1
>8>8>8>8

- Raspberry Pi 3 Model B+
- no HDMI connected

Does it mean, the issue only occurs without HDMI connected?
If you didn't test with HDMI yet, could you please do?

The error occurs with HDMI not connected, as vc4 is the gfx driver I
thought this might be of interest. :)

I don't have a HDMI monitor here, but I'll come back to you as soon as I
get access to one (might take some time).

It's not the first time an issue like this one would occur. I'm trying
to make my Pi3 boot again, and will try to bisect the issue.

yes the issue is only triggered without HDMI connected. I was able to
reproduce with an older vc4 firmware from 2020 (don't want to upgrade yet).
Kernel was also an arm64 build with defconfig.

Here some rough starting point for bisection:

5.18.0 good
5.19.0 bad
5.19.6 bad

Sorry it took a bit of time, it looks like I found another bug while
trying to test this yesterday.

Your datapoints are interesting though. I have a custom configuration
and it does boot 5.19 without an HDMI connected.

So I guess it leaves us with either the firmware version being different
(I'm using a newer version, from March 2022), or the configuration. I'll
test with defconfig.

So it turns out compiling vc4 as a module is the culprit.

Do you mean regardless of the kernel version in your case?

No, I mean that, with vc4 as a module, 5.18 works but 5.19 doesn't, like
Marc said. But if vc4 is built in, both work.


In my test cases i build vc4 always as module.


It's not clear to me why at this point, but the first register write in
vc4_hdmi_reset stalls.

Sounds like timing issue or a missing dependency (clock or power domain)

It felt like a clock or power domain issue to me indeed, but adding
clk_ignore_unused and pd_ignore_unused isn't enough, so it's probably
something a bit more complicated than just the clock / PD being
disabled.

I found the offending patch:
https://lore.kernel.org/dri-devel/20220225143534.405820-13-max...@cerno.tech/

That code was removed because it was made irrelevant by that earlier patch:
https://lore.kernel.org/dri-devel/20220225143534.405820-10-max...@cerno.tech/

But it turns out that while it works when the driver is built-in, it
doesn't when it's a module. If we add a clk_hw_get_rate() call right
after that call to raspberrypi_fw_set_rate(), the rate returned is 0.

I'm not entirely sure why, but I wonder if it's related to:
https://github.com/raspberrypi/linux/issues/4962#issuecomment-1228593439

Turns out it's not, since the Pi3 is using the clk-bcm2835 driver.


FWIW i can confirm, that i see the same behavior:

fd5894fa2413cca3e6a3ea713b2bd57281af2e86 bad

5b6ef06ea6225570bc0b33325306c7b8c6bdf5eb good



However, even reverting that patch fails. clk_set_min_rate fails because
the rate is protected, but it doesn't look like it is anywhere for that
clock, so I'm a bit confused.

Even if we do remove the clock protection check in
clk_core_set_rate_nolock(), clk_calc_new_rates() will then fail because
the bcm2835 driver will round the clock rate below the minimum, which is
rejected.

I'm not entirely

Re: Raspberry Pi 3 Model B+ hangs in vc4_hdmi_runtime_resume()

[Stefan Wahren]

Am 27.09.22 um 11:42 schrieb Maxime Ripard:

On Tue, Sep 27, 2022 at 09:25:54AM +0200, Maxime Ripard wrote:

Hi Stefan,

On Mon, Sep 26, 2022 at 08:50:12PM +0200, Stefan Wahren wrote:

Am 26.09.22 um 14:47 schrieb Maxime Ripard:

On Mon, Sep 26, 2022 at 02:40:48PM +0200, Marc Kleine-Budde wrote:

On 26.09.2022 14:08:04, Stefan Wahren wrote:

Hi Marc,

Am 26.09.22 um 12:21 schrieb Marc Kleine-Budde:

On 22.09.2022 17:06:00, Maxime Ripard wrote:

I'm on a Raspberry Pi 3 Model B+ running current Debian testing ARM64,
using Debian's v5.19 kernel (Debian's v5.18 was working flawless).

| [0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
| [0.00] Linux version 5.19.0-1-arm64 (debian-ker...@lists.debian.org) 
(gcc-11 (Debian 11.3.0-5) 11.3.0, GNU ld (GNU Binutils for Debian) 
2.38.90.20220713) #1 SMP Debian 5.19.6-1 (2022-0
9-01)
| [0.00] Machine model: Raspberry Pi 3 Model B+
| [3.747500] raspberrypi-firmware soc:firmware: Attached to firmware from 
2022-03-24T13:21:11

As soon a the vc4 module is loaded the following warnings hits 4
times, then the machine stops.

[...]


The warning itself is fixed, both upstream and in stable (5.19.7).

Ok. Debian is using 5.19.6


It shouldn't have any relation to the hang though. Can you share your
setup?

- config.txt:

>8>8>8>8
gpu_mem=16
disable_splash=1

arm_64bit=1
enable_uart=1
uart_2ndstage=1

os_prefix=/u-boot/

[pi3]
force_turbo=1
>8>8>8>8

- Raspberry Pi 3 Model B+
- no HDMI connected

Does it mean, the issue only occurs without HDMI connected?
If you didn't test with HDMI yet, could you please do?

The error occurs with HDMI not connected, as vc4 is the gfx driver I
thought this might be of interest. :)

I don't have a HDMI monitor here, but I'll come back to you as soon as I
get access to one (might take some time).

It's not the first time an issue like this one would occur. I'm trying
to make my Pi3 boot again, and will try to bisect the issue.

yes the issue is only triggered without HDMI connected. I was able to
reproduce with an older vc4 firmware from 2020 (don't want to upgrade yet).
Kernel was also an arm64 build with defconfig.

Here some rough starting point for bisection:

5.18.0 good
5.19.0 bad
5.19.6 bad

Sorry it took a bit of time, it looks like I found another bug while
trying to test this yesterday.

Your datapoints are interesting though. I have a custom configuration
and it does boot 5.19 without an HDMI connected.

So I guess it leaves us with either the firmware version being different
(I'm using a newer version, from March 2022), or the configuration. I'll
test with defconfig.

So it turns out compiling vc4 as a module is the culprit.


Do you mean regardless of the kernel version in your case?

In my test cases i build vc4 always as module.


It's not clear to me why at this point, but the first register write in
vc4_hdmi_reset stalls.

Sounds like timing issue or a missing dependency (clock or power domain)


Maxime

Re: Raspberry Pi 3 Model B+ hangs in vc4_hdmi_runtime_resume()

[Stefan Wahren]

Hi Maxime,

Am 27.09.22 um 09:25 schrieb Maxime Ripard:

Hi Stefan,

On Mon, Sep 26, 2022 at 08:50:12PM +0200, Stefan Wahren wrote:

Am 26.09.22 um 14:47 schrieb Maxime Ripard:

On Mon, Sep 26, 2022 at 02:40:48PM +0200, Marc Kleine-Budde wrote:

On 26.09.2022 14:08:04, Stefan Wahren wrote:

Hi Marc,

Am 26.09.22 um 12:21 schrieb Marc Kleine-Budde:

On 22.09.2022 17:06:00, Maxime Ripard wrote:

I'm on a Raspberry Pi 3 Model B+ running current Debian testing ARM64,
using Debian's v5.19 kernel (Debian's v5.18 was working flawless).

| [0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
| [0.00] Linux version 5.19.0-1-arm64 (debian-ker...@lists.debian.org) 
(gcc-11 (Debian 11.3.0-5) 11.3.0, GNU ld (GNU Binutils for Debian) 
2.38.90.20220713) #1 SMP Debian 5.19.6-1 (2022-0
9-01)
| [0.00] Machine model: Raspberry Pi 3 Model B+
| [3.747500] raspberrypi-firmware soc:firmware: Attached to firmware from 
2022-03-24T13:21:11

As soon a the vc4 module is loaded the following warnings hits 4
times, then the machine stops.

[...]


The warning itself is fixed, both upstream and in stable (5.19.7).

Ok. Debian is using 5.19.6


It shouldn't have any relation to the hang though. Can you share your
setup?

- config.txt:

>8>8>8>8
gpu_mem=16
disable_splash=1

arm_64bit=1
enable_uart=1
uart_2ndstage=1

os_prefix=/u-boot/

[pi3]
force_turbo=1
>8>8>8>8

- Raspberry Pi 3 Model B+
- no HDMI connected

Does it mean, the issue only occurs without HDMI connected?
If you didn't test with HDMI yet, could you please do?

The error occurs with HDMI not connected, as vc4 is the gfx driver I
thought this might be of interest. :)

I don't have a HDMI monitor here, but I'll come back to you as soon as I
get access to one (might take some time).

It's not the first time an issue like this one would occur. I'm trying
to make my Pi3 boot again, and will try to bisect the issue.

yes the issue is only triggered without HDMI connected. I was able to
reproduce with an older vc4 firmware from 2020 (don't want to upgrade yet).
Kernel was also an arm64 build with defconfig.

Here some rough starting point for bisection:

5.18.0 good
5.19.0 bad
5.19.6 bad

Sorry it took a bit of time, it looks like I found another bug while
trying to test this yesterday.

Your datapoints are interesting though. I have a custom configuration
and it does boot 5.19 without an HDMI connected.

i will try to bisect this, too.


So I guess it leaves us with either the firmware version being different
(I'm using a newer version, from March 2022), or the configuration. I'll
test with defconfig.

Maxime

Re: Raspberry Pi 3 Model B+ hangs in vc4_hdmi_runtime_resume()

[Stefan Wahren]

Hi Maxime,

Am 26.09.22 um 14:47 schrieb Maxime Ripard:

On Mon, Sep 26, 2022 at 02:40:48PM +0200, Marc Kleine-Budde wrote:

On 26.09.2022 14:08:04, Stefan Wahren wrote:

Hi Marc,

Am 26.09.22 um 12:21 schrieb Marc Kleine-Budde:

On 22.09.2022 17:06:00, Maxime Ripard wrote:

I'm on a Raspberry Pi 3 Model B+ running current Debian testing ARM64,
using Debian's v5.19 kernel (Debian's v5.18 was working flawless).

| [0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
| [0.00] Linux version 5.19.0-1-arm64 (debian-ker...@lists.debian.org) 
(gcc-11 (Debian 11.3.0-5) 11.3.0, GNU ld (GNU Binutils for Debian) 
2.38.90.20220713) #1 SMP Debian 5.19.6-1 (2022-0
9-01)
| [0.00] Machine model: Raspberry Pi 3 Model B+
| [3.747500] raspberrypi-firmware soc:firmware: Attached to firmware from 
2022-03-24T13:21:11

As soon a the vc4 module is loaded the following warnings hits 4
times, then the machine stops.

[...]


The warning itself is fixed, both upstream and in stable (5.19.7).

Ok. Debian is using 5.19.6


It shouldn't have any relation to the hang though. Can you share your
setup?

- config.txt:

>8>8>8>8
gpu_mem=16
disable_splash=1

arm_64bit=1
enable_uart=1
uart_2ndstage=1

os_prefix=/u-boot/

[pi3]
force_turbo=1
>8>8>8>8

- Raspberry Pi 3 Model B+
- no HDMI connected

Does it mean, the issue only occurs without HDMI connected?
If you didn't test with HDMI yet, could you please do?

The error occurs with HDMI not connected, as vc4 is the gfx driver I
thought this might be of interest. :)

I don't have a HDMI monitor here, but I'll come back to you as soon as I
get access to one (might take some time).

It's not the first time an issue like this one would occur. I'm trying
to make my Pi3 boot again, and will try to bisect the issue.


yes the issue is only triggered without HDMI connected. I was able to 
reproduce with an older vc4 firmware from 2020 (don't want to upgrade 
yet). Kernel was also an arm64 build with defconfig.


Here some rough starting point for bisection:

5.18.0 good
5.19.0 bad
5.19.6 bad



Maxime

Re: Raspberry Pi 3 Model B+ hangs in vc4_hdmi_runtime_resume()

[Stefan Wahren]

Hi Marc,

Am 26.09.22 um 12:21 schrieb Marc Kleine-Budde:

On 22.09.2022 17:06:00, Maxime Ripard wrote:

I'm on a Raspberry Pi 3 Model B+ running current Debian testing ARM64,
using Debian's v5.19 kernel (Debian's v5.18 was working flawless).

| [0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
| [0.00] Linux version 5.19.0-1-arm64 (debian-ker...@lists.debian.org) 
(gcc-11 (Debian 11.3.0-5) 11.3.0, GNU ld (GNU Binutils for Debian) 
2.38.90.20220713) #1 SMP Debian 5.19.6-1 (2022-0
9-01)
| [0.00] Machine model: Raspberry Pi 3 Model B+
| [3.747500] raspberrypi-firmware soc:firmware: Attached to firmware from 
2022-03-24T13:21:11

As soon a the vc4 module is loaded the following warnings hits 4
times, then the machine stops.

[...]


The warning itself is fixed, both upstream and in stable (5.19.7).

Ok. Debian is using 5.19.6


It shouldn't have any relation to the hang though. Can you share your
setup?

- config.txt:

>8>8>8>8
gpu_mem=16
disable_splash=1

arm_64bit=1
enable_uart=1
uart_2ndstage=1

os_prefix=/u-boot/

[pi3]
force_turbo=1
>8>8>8>8

- Raspberry Pi 3 Model B+
- no HDMI connected


Thanks

Does it mean, the issue only occurs without HDMI connected?

If you didn't test with HDMI yet, could you please do?


- firmware 2022-03-24T13:21:11
   which boot u-boot:
- u-boot 2022.04+dfsg-2+b1 (Debian testing)
   I'm using the "/usr/lib/u-boot/rpi_3/u-boot.bin"
   u-boot start:
- Linux version 5.19.0-1-arm64 (debian-ker...@lists.debian.org)
   kernel and dtb are unmodified Debian's linux-image-5.19.0-1-arm64
   $ ls -l /boot/dtbs/5.19.0-1-arm64/broadcom/bcm2837-rpi-3-b-plus.dtb
   -rwxr-xr-x 1 root root 15351 Sep 22 09:52 
/boot/dtbs/5.19.0-1-arm64/broadcom/bcm2837-rpi-3-b-plus.dtb*
   $ md5sum /boot/dtbs/5.19.0-1-arm64/broadcom/bcm2837-rpi-3-b-plus.dtb
   4a9c76c3c4dcafac7d69872ee88ac5fc  
/boot/dtbs/5.19.0-1-arm64/broadcom/bcm2837-rpi-3-b-plus.dtb
- currently, I've blacklisted vc4, but systems hangs after modprobe vc4
   (4x backtrace, then hang)

regards,
Marc


___
linux-rpi-kernel mailing list
linux-rpi-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

Re: Raspberry Pi 3 Model B+ hangs in vc4_hdmi_runtime_resume()

[Stefan Wahren]

Hi Marc,

Am 22.09.22 um 16:54 schrieb Marc Kleine-Budde:

Hello,

I'm on a Raspberry Pi 3 Model B+ running current Debian testing ARM64,
using Debian's v5.19 kernel (Debian's v5.18 was working flawless).

| [0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
| [0.00] Linux version 5.19.0-1-arm64 (debian-ker...@lists.debian.org) 
(gcc-11 (Debian 11.3.0-5) 11.3.0, GNU ld (GNU Binutils for Debian) 
2.38.90.20220713) #1 SMP Debian 5.19.6-1 (2022-0
9-01)
| [0.00] Machine model: Raspberry Pi 3 Model B+
| [3.747500] raspberrypi-firmware soc:firmware: Attached to firmware from 
2022-03-24T13:21:11

As soon a the vc4 module is loaded the following warnings hits 4
times, then the machine stops.
additionally to Maxime's reply, could you also please provide md5sum and 
filesize of your bcm2837-rpi-3-b-plus.dtb

Re: [PATCH v2 2/7] firmware: raspberrypi: Move the clock IDs to the firmware header

[Stefan Wahren]

Hi Maxime,

Am 20.09.22 um 14:50 schrieb Maxime Ripard:

We'll need the clock IDs in more drivers than just the clock driver from
now on, so let's move them in the firmware header.


recently as i reviewed the clk-raspberrypi i noticed this, too. But from 
my point of view the clock ids should go to include/dt-bindings/clock 
(like bcm2835.h) because these clock ids are actually referenced in the 
DTS files and we need to make sure they are in sync. AFAIR this would 
also result in change from enum to defines.


Sorry, i didn't had the time to send a patch for this.



Signed-off-by: Maxime Ripard 

diff --git a/drivers/clk/bcm/clk-raspberrypi.c 
b/drivers/clk/bcm/clk-raspberrypi.c
index 876b37b8683c..1f5e6a1554e6 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -18,24 +18,6 @@
  
  #include 
  
-enum rpi_firmware_clk_id {

-   RPI_FIRMWARE_EMMC_CLK_ID = 1,
-   RPI_FIRMWARE_UART_CLK_ID,
-   RPI_FIRMWARE_ARM_CLK_ID,
-   RPI_FIRMWARE_CORE_CLK_ID,
-   RPI_FIRMWARE_V3D_CLK_ID,
-   RPI_FIRMWARE_H264_CLK_ID,
-   RPI_FIRMWARE_ISP_CLK_ID,
-   RPI_FIRMWARE_SDRAM_CLK_ID,
-   RPI_FIRMWARE_PIXEL_CLK_ID,
-   RPI_FIRMWARE_PWM_CLK_ID,
-   RPI_FIRMWARE_HEVC_CLK_ID,
-   RPI_FIRMWARE_EMMC2_CLK_ID,
-   RPI_FIRMWARE_M2MC_CLK_ID,
-   RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
-   RPI_FIRMWARE_NUM_CLK_ID,
-};
-
  static char *rpi_firmware_clk_names[] = {
[RPI_FIRMWARE_EMMC_CLK_ID]  = "emmc",
[RPI_FIRMWARE_UART_CLK_ID]  = "uart",
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h 
b/include/soc/bcm2835/raspberrypi-firmware.h
index 63426082bcb9..74c7bcc1ac2a 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -136,6 +136,24 @@ enum rpi_firmware_property_tag {
RPI_FIRMWARE_GET_DMA_CHANNELS =   0x00060001,
  };
  
+enum rpi_firmware_clk_id {

+   RPI_FIRMWARE_EMMC_CLK_ID = 1,
+   RPI_FIRMWARE_UART_CLK_ID,
+   RPI_FIRMWARE_ARM_CLK_ID,
+   RPI_FIRMWARE_CORE_CLK_ID,
+   RPI_FIRMWARE_V3D_CLK_ID,
+   RPI_FIRMWARE_H264_CLK_ID,
+   RPI_FIRMWARE_ISP_CLK_ID,
+   RPI_FIRMWARE_SDRAM_CLK_ID,
+   RPI_FIRMWARE_PIXEL_CLK_ID,
+   RPI_FIRMWARE_PWM_CLK_ID,
+   RPI_FIRMWARE_HEVC_CLK_ID,
+   RPI_FIRMWARE_EMMC2_CLK_ID,
+   RPI_FIRMWARE_M2MC_CLK_ID,
+   RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
+   RPI_FIRMWARE_NUM_CLK_ID,
+};
+
  #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
  int rpi_firmware_property(struct rpi_firmware *fw,
  u32 tag, void *data, size_t len);

Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum

[Stefan Wahren]

Hi Maxime,

Am 15.09.22 um 09:54 schrieb Maxime Ripard:

On Wed, Sep 14, 2022 at 08:26:55PM +0200, Stefan Wahren wrote:

Am 14.09.22 um 20:14 schrieb Stephen Boyd:

Quoting Stefan Wahren (2022-09-14 11:09:04)

Am 14.09.22 um 20:05 schrieb Stephen Boyd:

Quoting Stefan Wahren (2022-09-14 10:45:48)

Am 14.09.22 um 17:50 schrieb Stephen Boyd:

Furthermore, I wonder if even that part needs to be implemented.  Why
not make a direct call to rpi_firmware_property() and get the max rate?
All of that can live in the drm driver. Making it a generic API that
takes a 'struct clk' means that it looks like any clk can be passed,
when that isn't true. It would be better to restrict it to the one use
case so that the scope of the problem doesn't grow. I understand that it
duplicates a few lines of code, but that looks like a fair tradeoff vs.
exposing an API that can be used for other clks in the future.

it would be nice to keep all the Rpi specific stuff out of the DRM
driver, since there more users of it.

Instead of 'all' did you mean 'any'?

yes

Why?

This firmware is written specific for the Raspberry Pi and not stable from
interface point of view. So i'm afraid that the DRM driver is only usable
for the Raspberry Pi at the end with all these board specific dependencies.

I'm open for suggestions there, but is there any other bcm2711 device
that we support upstream?
I meant the driver as a whole. According to the vc4 binding there are 
three compatibles bcm2835-vc4, cygnus-vc4 and bcm2711-vc5. Unfortunately 
i don't have access to any of these Cygnus boards, so i cannot do any 
regression tests or provide more information to your question.

If not, I'm not sure what the big deal is at this point. Chances are the
DRM driver won't work as is on a different board.

Plus, such a board wouldn't be using config.txt at all, so this whole
dance to find what was enabled or not wouldn't be used at all.
My concern is that we reach some point that we need to say this kernel 
version requires this firmware version. In the Raspberry Pi OS world 
this is not a problem, but not all distributions has this specific 
knowledge.



Emma invested a lot of time to make this open source and now it looks that
like that more and more functionality moves back to firmware.

What functionality has been moved back to firmware?
This wasn't a offense against your great work. Just a slight warning 
that some functions of clock or power management moved back into 
firmware. We should watch out, but maybe i emote here.


Maxime

Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum

[Stefan Wahren]

Hi Stephen,

Am 14.09.22 um 20:20 schrieb Stephen Boyd:

Quoting Stefan Wahren (2022-09-14 11:09:04)

Am 14.09.22 um 20:05 schrieb Stephen Boyd:

Quoting Stefan Wahren (2022-09-14 10:45:48)

Am 14.09.22 um 17:50 schrieb Stephen Boyd:

Furthermore, I wonder if even that part needs to be implemented.  Why
not make a direct call to rpi_firmware_property() and get the max rate?
All of that can live in the drm driver. Making it a generic API that
takes a 'struct clk' means that it looks like any clk can be passed,
when that isn't true. It would be better to restrict it to the one use
case so that the scope of the problem doesn't grow. I understand that it
duplicates a few lines of code, but that looks like a fair tradeoff vs.
exposing an API that can be used for other clks in the future.

it would be nice to keep all the Rpi specific stuff out of the DRM
driver, since there more users of it.

Instead of 'all' did you mean 'any'?

yes

Another idea is to populate an OPP table in the rpi firmware driver for
this platform device with the adjusted max frequency. That would be an
SoC/firmware agnostic interface that expresses the constraints.

Do you mean in the source code of this driver or in the DT?

I'm
almost certain we talked about this before.
I'm not sure about the context. Do you mean the CPU frequency handling? 
I remember it was a hard decision. In the end it was little benefit but 
a lot of disadvantages (hard to maintain all theses OPP tables for all 
Raspberry Pi boards, doesn't work with already deployed DT). So yes, i'm 
part of the problem i mentioned before ;-)

Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum

[Stefan Wahren]

Am 14.09.22 um 20:14 schrieb Stephen Boyd:

Quoting Stefan Wahren (2022-09-14 11:09:04)

Am 14.09.22 um 20:05 schrieb Stephen Boyd:

Quoting Stefan Wahren (2022-09-14 10:45:48)

Am 14.09.22 um 17:50 schrieb Stephen Boyd:

Furthermore, I wonder if even that part needs to be implemented.  Why
not make a direct call to rpi_firmware_property() and get the max rate?
All of that can live in the drm driver. Making it a generic API that
takes a 'struct clk' means that it looks like any clk can be passed,
when that isn't true. It would be better to restrict it to the one use
case so that the scope of the problem doesn't grow. I understand that it
duplicates a few lines of code, but that looks like a fair tradeoff vs.
exposing an API that can be used for other clks in the future.

it would be nice to keep all the Rpi specific stuff out of the DRM
driver, since there more users of it.

Instead of 'all' did you mean 'any'?

yes

Why?
This firmware is written specific for the Raspberry Pi and not stable 
from interface point of view. So i'm afraid that the DRM driver is only 
usable for the Raspberry Pi at the end with all these board specific 
dependencies. Emma invested a lot of time to make this open source and 
now it looks that like that more and more functionality moves back to 
firmware.

Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum

[Stefan Wahren]

Am 14.09.22 um 20:05 schrieb Stephen Boyd:

Quoting Stefan Wahren (2022-09-14 10:45:48)

Am 14.09.22 um 17:50 schrieb Stephen Boyd:

Furthermore, I wonder if even that part needs to be implemented.  Why
not make a direct call to rpi_firmware_property() and get the max rate?
All of that can live in the drm driver. Making it a generic API that
takes a 'struct clk' means that it looks like any clk can be passed,
when that isn't true. It would be better to restrict it to the one use
case so that the scope of the problem doesn't grow. I understand that it
duplicates a few lines of code, but that looks like a fair tradeoff vs.
exposing an API that can be used for other clks in the future.

it would be nice to keep all the Rpi specific stuff out of the DRM
driver, since there more users of it.

Instead of 'all' did you mean 'any'?

yes