[PATCH 1/2] drm/v3d: Ensure job pointer is set to NULL after job completion

[Maíra Canal]

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 at 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 at igalia.com>
> 
> Thanks for fixing this so fast.
> 
> Regards,
> 
> Chema
>