| Message ID | 20241014233758.994861-6-adrian.larumbe@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] drm/panfrost: Replace DRM driver allocation method with newer one | expand |
On Tue, 15 Oct 2024 00:31:41 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Rather than remasking interrupts after a device reset in the main reset > path, allow selecting whether to do this with an additional bool parameter. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 6 +++--- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > drivers/gpu/drm/panfrost/panfrost_job.c | 14 +++++--------- > drivers/gpu/drm/panfrost/panfrost_job.h | 2 +- > 4 files changed, 10 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c > index 4fe29286bbe3..a2a58501cbd9 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -395,20 +395,20 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev, > return false; > } > > -void panfrost_device_reset(struct panfrost_device *pfdev) > +void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int) > { > panfrost_gpu_soft_reset(pfdev); > > panfrost_gpu_power_on(pfdev); > panfrost_mmu_reset(pfdev); > - panfrost_job_enable_interrupts(pfdev); > + panfrost_job_enable_interrupts(pfdev, enable_job_int); > } > > static int panfrost_device_runtime_resume(struct device *dev) > { > struct panfrost_device *pfdev = dev_get_drvdata(dev); > > - panfrost_device_reset(pfdev); > + panfrost_device_reset(pfdev, true); > panfrost_devfreq_resume(pfdev); > > return 0; > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index d9aea2c2cbe5..fc83d5e104a3 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -205,7 +205,7 @@ int panfrost_unstable_ioctl_check(void); > > int panfrost_device_init(struct panfrost_device *pfdev); > void panfrost_device_fini(struct panfrost_device *pfdev); > -void panfrost_device_reset(struct panfrost_device *pfdev); > +void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int); > > extern const struct dev_pm_ops panfrost_pm_ops; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index 52ec9dc2397c..68be555c6c52 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -414,7 +414,7 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) > return fence; > } > > -void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) > +void panfrost_job_enable_interrupts(struct panfrost_device *pfdev, bool enable_job_int) > { > int j; > u32 irq_mask = 0; > @@ -426,7 +426,8 @@ void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) > } > > job_write(pfdev, JOB_INT_CLEAR, irq_mask); > - job_write(pfdev, JOB_INT_MASK, irq_mask); > + if (enable_job_int) > + job_write(pfdev, JOB_INT_MASK, irq_mask); > } This makes things utterly confusing. Now you have a function named panfrost_job_enable_interrupts() with a boolean encoding whether you want to enable interrupts or not. If we really want to dissociate the two things done in panfrost_job_enable_interrupts(), I'd rather go for a reset_interrupts()/enable_interrupts() split, with reset_interrupts clearing the IRQs, and enable_interrupts() unmasking the interrupts and clearing the COMP_BIT_JOB bit. > > void panfrost_job_suspend_irq(struct panfrost_device *pfdev) > @@ -718,12 +719,7 @@ panfrost_reset(struct panfrost_device *pfdev, > spin_unlock(&pfdev->js->job_lock); > > /* Proceed with reset now. */ > - panfrost_device_reset(pfdev); > - > - /* panfrost_device_reset() unmasks job interrupts, but we want to > - * keep them masked a bit longer. > - */ > - job_write(pfdev, JOB_INT_MASK, 0); > + panfrost_device_reset(pfdev, false); > > /* GPU has been reset, we can clear the reset pending bit. */ > atomic_set(&pfdev->reset.pending, 0); > @@ -898,7 +894,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) > } > } > > - panfrost_job_enable_interrupts(pfdev); > + panfrost_job_enable_interrupts(pfdev, true); > > return 0; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h b/drivers/gpu/drm/panfrost/panfrost_job.h > index ec581b97852b..3b8b712c1722 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.h > +++ b/drivers/gpu/drm/panfrost/panfrost_job.h > @@ -46,7 +46,7 @@ void panfrost_job_close(struct panfrost_file_priv *panfrost_priv); > 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_enable_interrupts(struct panfrost_device *pfdev, bool enable_job_int); > void panfrost_job_suspend_irq(struct panfrost_device *pfdev); > int panfrost_job_is_idle(struct panfrost_device *pfdev); >
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 4fe29286bbe3..a2a58501cbd9 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -395,20 +395,20 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev, return false; } -void panfrost_device_reset(struct panfrost_device *pfdev) +void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int) { panfrost_gpu_soft_reset(pfdev); panfrost_gpu_power_on(pfdev); panfrost_mmu_reset(pfdev); - panfrost_job_enable_interrupts(pfdev); + panfrost_job_enable_interrupts(pfdev, enable_job_int); } static int panfrost_device_runtime_resume(struct device *dev) { struct panfrost_device *pfdev = dev_get_drvdata(dev); - panfrost_device_reset(pfdev); + panfrost_device_reset(pfdev, true); panfrost_devfreq_resume(pfdev); return 0; diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index d9aea2c2cbe5..fc83d5e104a3 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -205,7 +205,7 @@ int panfrost_unstable_ioctl_check(void); int panfrost_device_init(struct panfrost_device *pfdev); void panfrost_device_fini(struct panfrost_device *pfdev); -void panfrost_device_reset(struct panfrost_device *pfdev); +void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int); extern const struct dev_pm_ops panfrost_pm_ops; diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 52ec9dc2397c..68be555c6c52 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -414,7 +414,7 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) return fence; } -void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) +void panfrost_job_enable_interrupts(struct panfrost_device *pfdev, bool enable_job_int) { int j; u32 irq_mask = 0; @@ -426,7 +426,8 @@ void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) } job_write(pfdev, JOB_INT_CLEAR, irq_mask); - job_write(pfdev, JOB_INT_MASK, irq_mask); + if (enable_job_int) + job_write(pfdev, JOB_INT_MASK, irq_mask); } void panfrost_job_suspend_irq(struct panfrost_device *pfdev) @@ -718,12 +719,7 @@ panfrost_reset(struct panfrost_device *pfdev, spin_unlock(&pfdev->js->job_lock); /* Proceed with reset now. */ - panfrost_device_reset(pfdev); - - /* panfrost_device_reset() unmasks job interrupts, but we want to - * keep them masked a bit longer. - */ - job_write(pfdev, JOB_INT_MASK, 0); + panfrost_device_reset(pfdev, false); /* GPU has been reset, we can clear the reset pending bit. */ atomic_set(&pfdev->reset.pending, 0); @@ -898,7 +894,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) } } - panfrost_job_enable_interrupts(pfdev); + panfrost_job_enable_interrupts(pfdev, true); return 0; diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h b/drivers/gpu/drm/panfrost/panfrost_job.h index ec581b97852b..3b8b712c1722 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.h +++ b/drivers/gpu/drm/panfrost/panfrost_job.h @@ -46,7 +46,7 @@ void panfrost_job_close(struct panfrost_file_priv *panfrost_priv); 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_enable_interrupts(struct panfrost_device *pfdev, bool enable_job_int); void panfrost_job_suspend_irq(struct panfrost_device *pfdev); int panfrost_job_is_idle(struct panfrost_device *pfdev);
Rather than remasking interrupts after a device reset in the main reset path, allow selecting whether to do this with an additional bool parameter. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_device.c | 6 +++--- drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- drivers/gpu/drm/panfrost/panfrost_job.c | 14 +++++--------- drivers/gpu/drm/panfrost/panfrost_job.h | 2 +- 4 files changed, 10 insertions(+), 14 deletions(-)