| Message ID | 20250113154741.67520-1-mcanal@igalia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] drm/v3d: Ensure job pointer is set to NULL after job completion | expand |
El 13/1/25 a las 16:47, Maíra Canal escribió: > After a job completes, the corresponding pointer in the device must > be set to NULL. Failing to do so triggers a warning when unloading > the driver, as it appears the job is still active. To prevent this, > assign the job pointer to NULL after completing the job and signaling > the fence, indicating the job has finished. > > Fixes: 14d1d1908696 ("drm/v3d: Remove the bad signaled() implementation") Just a question, should we add next commit to the Fixes tag: Fixes: 79d94360d50f ("drm/v3d: wait for all jobs to finish before unregistering") > Signed-off-by: Maíra Canal <mcanal@igalia.com> > --- > drivers/gpu/drm/v3d/v3d_irq.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c > index 20bf33702c3c..da203045df9b 100644 > --- a/drivers/gpu/drm/v3d/v3d_irq.c > +++ b/drivers/gpu/drm/v3d/v3d_irq.c > @@ -108,6 +108,7 @@ v3d_irq(int irq, void *arg) > v3d_job_update_stats(&v3d->bin_job->base, V3D_BIN); > trace_v3d_bcl_irq(&v3d->drm, fence->seqno); > dma_fence_signal(&fence->base); > + v3d->bin_job = NULL; > status = IRQ_HANDLED; > } > > @@ -118,6 +119,7 @@ v3d_irq(int irq, void *arg) > v3d_job_update_stats(&v3d->render_job->base, V3D_RENDER); > trace_v3d_rcl_irq(&v3d->drm, fence->seqno); > dma_fence_signal(&fence->base); > + v3d->render_job = NULL; > status = IRQ_HANDLED; > } > > @@ -128,6 +130,7 @@ v3d_irq(int irq, void *arg) > v3d_job_update_stats(&v3d->csd_job->base, V3D_CSD); > trace_v3d_csd_irq(&v3d->drm, fence->seqno); > dma_fence_signal(&fence->base); > + v3d->csd_job = NULL; > status = IRQ_HANDLED; > } > > @@ -165,6 +168,7 @@ v3d_hub_irq(int irq, void *arg) > v3d_job_update_stats(&v3d->tfu_job->base, V3D_TFU); > trace_v3d_tfu_irq(&v3d->drm, fence->seqno); > dma_fence_signal(&fence->base); > + v3d->tfu_job = NULL; > status = IRQ_HANDLED; > } > With or without my previous comment this is: Reviewed-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com> Thanks for fixing this so fast. Regards, Chema
Hi Chema, Thanks for the review! On 13/01/25 16:26, Chema Casanova wrote: > El 13/1/25 a las 16:47, Maíra Canal escribió: >> After a job completes, the corresponding pointer in the device must >> be set to NULL. Failing to do so triggers a warning when unloading >> the driver, as it appears the job is still active. To prevent this, >> assign the job pointer to NULL after completing the job and signaling >> the fence, indicating the job has finished. >> >> Fixes: 14d1d1908696 ("drm/v3d: Remove the bad signaled() implementation") > > Just a question, should we add next commit to the Fixes tag: > > Fixes: 79d94360d50f ("drm/v3d: wait for all jobs to finish before > unregistering") I believe it is better to have the older reference, as it will ease the backport to kernels older than 79d94360d50f. I just applied the patch to misc/kernel.git (drm-misc-fixes). Best Regards, - Maíra > >> Signed-off-by: Maíra Canal <mcanal@igalia.com> >> --- >> drivers/gpu/drm/v3d/v3d_irq.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/ >> v3d_irq.c >> index 20bf33702c3c..da203045df9b 100644 >> --- a/drivers/gpu/drm/v3d/v3d_irq.c >> +++ b/drivers/gpu/drm/v3d/v3d_irq.c >> @@ -108,6 +108,7 @@ v3d_irq(int irq, void *arg) >> v3d_job_update_stats(&v3d->bin_job->base, V3D_BIN); >> trace_v3d_bcl_irq(&v3d->drm, fence->seqno); >> dma_fence_signal(&fence->base); >> + v3d->bin_job = NULL; >> status = IRQ_HANDLED; >> } >> @@ -118,6 +119,7 @@ v3d_irq(int irq, void *arg) >> v3d_job_update_stats(&v3d->render_job->base, V3D_RENDER); >> trace_v3d_rcl_irq(&v3d->drm, fence->seqno); >> dma_fence_signal(&fence->base); >> + v3d->render_job = NULL; >> status = IRQ_HANDLED; >> } >> @@ -128,6 +130,7 @@ v3d_irq(int irq, void *arg) >> v3d_job_update_stats(&v3d->csd_job->base, V3D_CSD); >> trace_v3d_csd_irq(&v3d->drm, fence->seqno); >> dma_fence_signal(&fence->base); >> + v3d->csd_job = NULL; >> status = IRQ_HANDLED; >> } >> @@ -165,6 +168,7 @@ v3d_hub_irq(int irq, void *arg) >> v3d_job_update_stats(&v3d->tfu_job->base, V3D_TFU); >> trace_v3d_tfu_irq(&v3d->drm, fence->seqno); >> dma_fence_signal(&fence->base); >> + v3d->tfu_job = NULL; >> status = IRQ_HANDLED; >> } > > With or without my previous comment this is: > > Reviewed-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com> > > Thanks for fixing this so fast. > > Regards, > > Chema >
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c index 20bf33702c3c..da203045df9b 100644 --- a/drivers/gpu/drm/v3d/v3d_irq.c +++ b/drivers/gpu/drm/v3d/v3d_irq.c @@ -108,6 +108,7 @@ v3d_irq(int irq, void *arg) v3d_job_update_stats(&v3d->bin_job->base, V3D_BIN); trace_v3d_bcl_irq(&v3d->drm, fence->seqno); dma_fence_signal(&fence->base); + v3d->bin_job = NULL; status = IRQ_HANDLED; } @@ -118,6 +119,7 @@ v3d_irq(int irq, void *arg) v3d_job_update_stats(&v3d->render_job->base, V3D_RENDER); trace_v3d_rcl_irq(&v3d->drm, fence->seqno); dma_fence_signal(&fence->base); + v3d->render_job = NULL; status = IRQ_HANDLED; } @@ -128,6 +130,7 @@ v3d_irq(int irq, void *arg) v3d_job_update_stats(&v3d->csd_job->base, V3D_CSD); trace_v3d_csd_irq(&v3d->drm, fence->seqno); dma_fence_signal(&fence->base); + v3d->csd_job = NULL; status = IRQ_HANDLED; } @@ -165,6 +168,7 @@ v3d_hub_irq(int irq, void *arg) v3d_job_update_stats(&v3d->tfu_job->base, V3D_TFU); trace_v3d_tfu_irq(&v3d->drm, fence->seqno); dma_fence_signal(&fence->base); + v3d->tfu_job = NULL; status = IRQ_HANDLED; }
After a job completes, the corresponding pointer in the device must be set to NULL. Failing to do so triggers a warning when unloading the driver, as it appears the job is still active. To prevent this, assign the job pointer to NULL after completing the job and signaling the fence, indicating the job has finished. Fixes: 14d1d1908696 ("drm/v3d: Remove the bad signaled() implementation") Signed-off-by: Maíra Canal <mcanal@igalia.com> --- drivers/gpu/drm/v3d/v3d_irq.c | 4 ++++ 1 file changed, 4 insertions(+)