drm/panfrost: Synchronize and disable interrupts before powering off
authorAngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Mon, 4 Dec 2023 11:42:15 +0000 (12:42 +0100)
committerBoris Brezillon <boris.brezillon@collabora.com>
Tue, 5 Dec 2023 10:39:59 +0000 (11:39 +0100)
To make sure that we don't unintentionally perform any unclocked and/or
unpowered R/W operation on GPU registers, before turning off clocks and
regulators we must make sure that no GPU, JOB or MMU ISR execution is
pending: doing that requires to add a mechanism to synchronize the
interrupts on suspend.

Add functions panfrost_{gpu,job,mmu}_suspend_irq() which will perform
interrupts masking and ISR execution synchronization, and then call
those in the panfrost_device_runtime_suspend() handler in the exact
sequence of job (may require mmu!) -> mmu -> gpu.

As a side note, JOB and MMU suspend_irq functions needed some special
treatment: as their interrupt handlers will unmask interrupts, it was
necessary to add an `is_suspended` bitmap which is used to address the
possible corner case of unintentional IRQ unmasking because of ISR
execution after a call to synchronize_irq().

At resume, clear each is_suspended bit in the reset path of JOB/MMU
to allow unmasking the interrupts.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231204114215.54575-4-angelogioacchino.delregno@collabora.com
drivers/gpu/drm/panfrost/panfrost_device.c
drivers/gpu/drm/panfrost/panfrost_device.h
drivers/gpu/drm/panfrost/panfrost_gpu.c
drivers/gpu/drm/panfrost/panfrost_gpu.h
drivers/gpu/drm/panfrost/panfrost_job.c
drivers/gpu/drm/panfrost/panfrost_job.h
drivers/gpu/drm/panfrost/panfrost_mmu.c
drivers/gpu/drm/panfrost/panfrost_mmu.h

index c90ad5e..a45e4ad 100644 (file)
@@ -421,6 +421,9 @@ static int panfrost_device_runtime_suspend(struct device *dev)
                return -EBUSY;
 
        panfrost_devfreq_suspend(pfdev);
+       panfrost_job_suspend_irq(pfdev);
+       panfrost_mmu_suspend_irq(pfdev);
+       panfrost_gpu_suspend_irq(pfdev);
        panfrost_gpu_power_off(pfdev);
 
        return 0;
index 54a8aad..62f7e35 100644 (file)
@@ -25,6 +25,13 @@ struct panfrost_perfcnt;
 #define NUM_JOB_SLOTS 3
 #define MAX_PM_DOMAINS 5
 
+enum panfrost_drv_comp_bits {
+       PANFROST_COMP_BIT_GPU,
+       PANFROST_COMP_BIT_JOB,
+       PANFROST_COMP_BIT_MMU,
+       PANFROST_COMP_BIT_MAX
+};
+
 /**
  * enum panfrost_gpu_pm - Supported kernel power management features
  * @GPU_PM_CLK_DIS:  Allow disabling clocks during system suspend
@@ -109,6 +116,7 @@ struct panfrost_device {
 
        struct panfrost_features features;
        const struct panfrost_compatible *comp;
+       DECLARE_BITMAP(is_suspended, PANFROST_COMP_BIT_MAX);
 
        spinlock_t as_lock;
        unsigned long as_in_use_mask;
index 7adc444..9063ce2 100644 (file)
 static irqreturn_t panfrost_gpu_irq_handler(int irq, void *data)
 {
        struct panfrost_device *pfdev = data;
-       u32 state = gpu_read(pfdev, GPU_INT_STAT);
-       u32 fault_status = gpu_read(pfdev, GPU_FAULT_STATUS);
+       u32 fault_status, state;
 
+       if (test_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended))
+               return IRQ_NONE;
+
+       fault_status = gpu_read(pfdev, GPU_FAULT_STATUS);
+       state = gpu_read(pfdev, GPU_INT_STAT);
        if (!state)
                return IRQ_NONE;
 
@@ -61,6 +65,8 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)
        gpu_write(pfdev, GPU_INT_MASK, 0);
        gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_RESET_COMPLETED);
 
+       clear_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended);
+
        gpu_write(pfdev, GPU_CMD, GPU_CMD_SOFT_RESET);
        ret = readl_relaxed_poll_timeout(pfdev->iomem + GPU_INT_RAWSTAT,
                val, val & GPU_IRQ_RESET_COMPLETED, 10, 10000);
@@ -452,6 +458,14 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev)
                dev_err(pfdev->dev, "l2 power transition timeout");
 }
 
+void panfrost_gpu_suspend_irq(struct panfrost_device *pfdev)
+{
+       set_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended);
+
+       gpu_write(pfdev, GPU_INT_MASK, 0);
+       synchronize_irq(pfdev->gpu_irq);
+}
+
 int panfrost_gpu_init(struct panfrost_device *pfdev)
 {
        int err;
index 876fdad..d841b86 100644 (file)
@@ -15,6 +15,7 @@ u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device *pfdev);
 int panfrost_gpu_soft_reset(struct panfrost_device *pfdev);
 void panfrost_gpu_power_on(struct panfrost_device *pfdev);
 void panfrost_gpu_power_off(struct panfrost_device *pfdev);
+void panfrost_gpu_suspend_irq(struct panfrost_device *pfdev);
 
 void panfrost_cycle_counter_get(struct panfrost_device *pfdev);
 void panfrost_cycle_counter_put(struct panfrost_device *pfdev);
index f9446e1..0c2dbf6 100644 (file)
@@ -405,6 +405,8 @@ void panfrost_job_enable_interrupts(struct panfrost_device *pfdev)
        int j;
        u32 irq_mask = 0;
 
+       clear_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended);
+
        for (j = 0; j < NUM_JOB_SLOTS; j++) {
                irq_mask |= MK_JS_MASK(j);
        }
@@ -413,6 +415,14 @@ void panfrost_job_enable_interrupts(struct panfrost_device *pfdev)
        job_write(pfdev, JOB_INT_MASK, irq_mask);
 }
 
+void panfrost_job_suspend_irq(struct panfrost_device *pfdev)
+{
+       set_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended);
+
+       job_write(pfdev, JOB_INT_MASK, 0);
+       synchronize_irq(pfdev->js->irq);
+}
+
 static void panfrost_job_handle_err(struct panfrost_device *pfdev,
                                    struct panfrost_job *job,
                                    unsigned int js)
@@ -792,17 +802,25 @@ static irqreturn_t panfrost_job_irq_handler_thread(int irq, void *data)
        struct panfrost_device *pfdev = data;
 
        panfrost_job_handle_irqs(pfdev);
-       job_write(pfdev, JOB_INT_MASK,
-                 GENMASK(16 + NUM_JOB_SLOTS - 1, 16) |
-                 GENMASK(NUM_JOB_SLOTS - 1, 0));
+
+       /* Enable interrupts only if we're not about to get suspended */
+       if (!test_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended))
+               job_write(pfdev, JOB_INT_MASK,
+                         GENMASK(16 + NUM_JOB_SLOTS - 1, 16) |
+                         GENMASK(NUM_JOB_SLOTS - 1, 0));
+
        return IRQ_HANDLED;
 }
 
 static irqreturn_t panfrost_job_irq_handler(int irq, void *data)
 {
        struct panfrost_device *pfdev = data;
-       u32 status = job_read(pfdev, JOB_INT_STAT);
+       u32 status;
+
+       if (test_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended))
+               return IRQ_NONE;
 
+       status = job_read(pfdev, JOB_INT_STAT);
        if (!status)
                return IRQ_NONE;
 
index 17ff808..ec581b9 100644 (file)
@@ -47,6 +47,7 @@ int panfrost_job_get_slot(struct panfrost_job *job);
 int panfrost_job_push(struct panfrost_job *job);
 void panfrost_job_put(struct panfrost_job *job);
 void panfrost_job_enable_interrupts(struct panfrost_device *pfdev);
+void panfrost_job_suspend_irq(struct panfrost_device *pfdev);
 int panfrost_job_is_idle(struct panfrost_device *pfdev);
 
 #endif
index ac4296c..f38385f 100644 (file)
@@ -231,6 +231,8 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
 {
        struct panfrost_mmu *mmu, *mmu_tmp;
 
+       clear_bit(PANFROST_COMP_BIT_MMU, pfdev->is_suspended);
+
        spin_lock(&pfdev->as_lock);
 
        pfdev->as_alloc_mask = 0;
@@ -670,6 +672,9 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data)
 {
        struct panfrost_device *pfdev = data;
 
+       if (test_bit(PANFROST_COMP_BIT_MMU, pfdev->is_suspended))
+               return IRQ_NONE;
+
        if (!mmu_read(pfdev, MMU_INT_STAT))
                return IRQ_NONE;
 
@@ -744,9 +749,12 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data)
                        status = mmu_read(pfdev, MMU_INT_RAWSTAT) & ~pfdev->as_faulty_mask;
        }
 
-       spin_lock(&pfdev->as_lock);
-       mmu_write(pfdev, MMU_INT_MASK, ~pfdev->as_faulty_mask);
-       spin_unlock(&pfdev->as_lock);
+       /* Enable interrupts only if we're not about to get suspended */
+       if (!test_bit(PANFROST_COMP_BIT_MMU, pfdev->is_suspended)) {
+               spin_lock(&pfdev->as_lock);
+               mmu_write(pfdev, MMU_INT_MASK, ~pfdev->as_faulty_mask);
+               spin_unlock(&pfdev->as_lock);
+       }
 
        return IRQ_HANDLED;
 };
@@ -777,3 +785,11 @@ void panfrost_mmu_fini(struct panfrost_device *pfdev)
 {
        mmu_write(pfdev, MMU_INT_MASK, 0);
 }
+
+void panfrost_mmu_suspend_irq(struct panfrost_device *pfdev)
+{
+       set_bit(PANFROST_COMP_BIT_MMU, pfdev->is_suspended);
+
+       mmu_write(pfdev, MMU_INT_MASK, 0);
+       synchronize_irq(pfdev->mmu_irq);
+}
index cc2a0d3..022a9a7 100644 (file)
@@ -14,6 +14,7 @@ void panfrost_mmu_unmap(struct panfrost_gem_mapping *mapping);
 int panfrost_mmu_init(struct panfrost_device *pfdev);
 void panfrost_mmu_fini(struct panfrost_device *pfdev);
 void panfrost_mmu_reset(struct panfrost_device *pfdev);
+void panfrost_mmu_suspend_irq(struct panfrost_device *pfdev);
 
 u32 panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu *mmu);
 void panfrost_mmu_as_put(struct panfrost_device *pfdev, struct panfrost_mmu *mmu);