On 14/10/2025 10:43, Karunika Choo wrote:
> Add helpers to issue reset commands through the PWR_CONTROL interface
> and wait for reset completion using IRQ signaling. This enables support
> for both RESET_SOFT and RESET_FAST operations with timeout handling and
> status verification.
>
> Signed-off-by: Karunika Choo <[email protected]>
> ---
> drivers/gpu/drm/panthor/panthor_pwr.c | 62 ++++++++++++++++++++++++++-
> drivers/gpu/drm/panthor/panthor_pwr.h | 2 +
> 2 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_pwr.c
> b/drivers/gpu/drm/panthor/panthor_pwr.c
> index 594181aba847..ecb278824d06 100644
> --- a/drivers/gpu/drm/panthor/panthor_pwr.c
> +++ b/drivers/gpu/drm/panthor/panthor_pwr.c
> @@ -3,6 +3,7 @@
>
> #include <linux/platform_device.h>
> #include <linux/interrupt.h>
> +#include <linux/cleanup.h>
> #include <linux/iopoll.h>
> #include <linux/wait.h>
>
> @@ -31,6 +32,8 @@
>
> #define PWR_RETRACT_TIMEOUT_US 2000
>
> +#define PWR_RESET_TIMEOUT_MS 500
> +
> /**
> * struct panthor_pwr - PWR_CONTROL block management data.
> */
> @@ -80,6 +83,42 @@ static void panthor_pwr_write_command(struct
> panthor_device *ptdev, u32 command,
> gpu_write(ptdev, PWR_COMMAND, command);
> }
>
> +static bool reset_irq_raised(struct panthor_device *ptdev)
> +{
> + return gpu_read(ptdev, PWR_INT_RAWSTAT) & PWR_IRQ_RESET_COMPLETED;
> +}
> +
> +static bool reset_completed(struct panthor_device *ptdev)
> +{
> + return (ptdev->pwr->pending_reqs & PWR_IRQ_RESET_COMPLETED);
> +}
> +
> +static int panthor_pwr_reset(struct panthor_device *ptdev, u32 reset_cmd)
> +{
> + scoped_guard(spinlock_irqsave, &ptdev->pwr->reqs_lock) {
> + if (!drm_WARN_ON(&ptdev->base, !reset_completed(ptdev))) {
> + ptdev->pwr->pending_reqs |= PWR_IRQ_RESET_COMPLETED;
> + gpu_write(ptdev, PWR_INT_CLEAR,
> PWR_IRQ_RESET_COMPLETED);
> + panthor_pwr_write_command(ptdev, reset_cmd, 0);
> + }
This would be easier to read as:
if (reset_completed(ptdev)) {
....
} else {
drm_WARN(&ptdev->base, 1, "Hey, we're already resetting?");
}
[Message modified to taste ;) ]
I'm also wondering if things would be easier to read if you switched
from reset_completed() to reset_pending(). Certainly here it's the
'pending' test you are trying to do.
> + }
> +
> + if (!wait_event_timeout(ptdev->pwr->reqs_acked, reset_completed(ptdev),
> + msecs_to_jiffies(PWR_RESET_TIMEOUT_MS))) {
> + guard(spinlock_irqsave)(&ptdev->pwr->reqs_lock);
> +
> + if (!reset_completed(ptdev) && !reset_irq_raised(ptdev)) {
> + drm_err(&ptdev->base, "RESET_%s timed out",
> + reset_cmd == PWR_COMMAND_RESET_SOFT ? "SOFT" :
> "FAST");
> + return -ETIMEDOUT;
> + }
> +
> + ptdev->pwr->pending_reqs &= ~PWR_IRQ_RESET_COMPLETED;
> + }
> +
> + return 0;
> +}
> +
> static const char *get_domain_name(u8 domain)
> {
> switch (domain) {
> @@ -407,9 +446,30 @@ int panthor_pwr_init(struct panthor_device *ptdev)
> return 0;
> }
>
> +int panthor_pwr_reset_fast(struct panthor_device *ptdev)
> +{
> + if (!ptdev->pwr)
> + return 0;
> +
> + if (!(panthor_pwr_read_status(ptdev) & PWR_STATUS_ALLOW_FAST_RESET)) {
> + drm_err(&ptdev->base, "RESET_SOFT not allowed");
Copy/paste mistake on the error message.
> + return -EOPNOTSUPP;
> + }
> +
> + return panthor_pwr_reset(ptdev, PWR_COMMAND_RESET_FAST);
> +}
I can't actually find a caller of this function within the series.
> +
> int panthor_pwr_reset_soft(struct panthor_device *ptdev)
> {
> - return 0;
> + if (!ptdev->pwr)
> + return 0;
When would this happen? Is this not a programming error?
Thanks,
Steve
> +
> + if (!(panthor_pwr_read_status(ptdev) & PWR_STATUS_ALLOW_SOFT_RESET)) {
> + drm_err(&ptdev->base, "RESET_SOFT not allowed");
> + return -EOPNOTSUPP;
> + }
> +
> + return panthor_pwr_reset(ptdev, PWR_COMMAND_RESET_SOFT);
> }
>
> int panthor_pwr_l2_power_off(struct panthor_device *ptdev)
> diff --git a/drivers/gpu/drm/panthor/panthor_pwr.h
> b/drivers/gpu/drm/panthor/panthor_pwr.h
> index a4042c125448..2301c26dab86 100644
> --- a/drivers/gpu/drm/panthor/panthor_pwr.h
> +++ b/drivers/gpu/drm/panthor/panthor_pwr.h
> @@ -10,6 +10,8 @@ void panthor_pwr_unplug(struct panthor_device *ptdev);
>
> int panthor_pwr_init(struct panthor_device *ptdev);
>
> +int panthor_pwr_reset_fast(struct panthor_device *ptdev);
> +
> int panthor_pwr_reset_soft(struct panthor_device *ptdev);
>
> int panthor_pwr_l2_power_on(struct panthor_device *ptdev);
