diff mbox series

[9/9] drm/panfrost: Explicitly clean up panfrost fence

Message ID 20241014233758.994861-9-adrian.larumbe@collabora.com (mailing list archive)
State New
Headers show
Series None | expand

Commit Message

Adrián Larumbe Oct. 14, 2024, 11:31 p.m. UTC
Doesn't make any functional difference because generic dma_fence is the
first panfrost_fence structure member, but I guess it doesn't hurt either.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_job.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Christian König Oct. 16, 2024, 1:12 p.m. UTC | #1
Am 15.10.24 um 01:31 schrieb Adrián Larumbe:
> Doesn't make any functional difference because generic dma_fence is the
> first panfrost_fence structure member, but I guess it doesn't hurt either.

As discussed with Sima we want to push into the exactly opposite 
direction because that requires that the panfrost module stays loaded as 
long as fences are around.

So clearly a NAK to this one here. Rather document on the structure that 
the dma_fence structure must be the first member.

Regards,
Christian.

> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>   drivers/gpu/drm/panfrost/panfrost_job.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> index 5d83c6a148ec..fa219f719bdc 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> @@ -85,9 +85,15 @@ static const char *panfrost_fence_get_timeline_name(struct dma_fence *fence)
>   	}
>   }
>   
> +static void panfrost_fence_release(struct dma_fence *fence)
> +{
> +	kfree(to_panfrost_fence(fence));
> +}
> +
>   static const struct dma_fence_ops panfrost_fence_ops = {
>   	.get_driver_name = panfrost_fence_get_driver_name,
>   	.get_timeline_name = panfrost_fence_get_timeline_name,
> +	.release = panfrost_fence_release,
>   };
>   
>   static struct dma_fence *panfrost_fence_create(struct panfrost_device *pfdev, int js_num)
Adrián Larumbe Oct. 16, 2024, 4:43 p.m. UTC | #2
On 16.10.2024 15:12, Christian König wrote:
> Am 15.10.24 um 01:31 schrieb Adrián Larumbe:
> > Doesn't make any functional difference because generic dma_fence is the
> > first panfrost_fence structure member, but I guess it doesn't hurt either.
> 
> As discussed with Sima we want to push into the exactly opposite direction
> because that requires that the panfrost module stays loaded as long as fences
> are around.

Does that mean in future commits the struct dma_fence_ops' .release pointer will be
done with altogether?

> So clearly a NAK to this one here. Rather document on the structure that the
> dma_fence structure must be the first member.
> 
> Regards,
> Christian.
> 
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> > ---
> >   drivers/gpu/drm/panfrost/panfrost_job.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> > index 5d83c6a148ec..fa219f719bdc 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> > @@ -85,9 +85,15 @@ static const char *panfrost_fence_get_timeline_name(struct dma_fence *fence)
> >   	}
> >   }
> > +static void panfrost_fence_release(struct dma_fence *fence)
> > +{
> > +	kfree(to_panfrost_fence(fence));
> > +}
> > +
> >   static const struct dma_fence_ops panfrost_fence_ops = {
> >   	.get_driver_name = panfrost_fence_get_driver_name,
> >   	.get_timeline_name = panfrost_fence_get_timeline_name,
> > +	.release = panfrost_fence_release,
> >   };
> >   static struct dma_fence *panfrost_fence_create(struct panfrost_device *pfdev, int js_num)
Christian König Oct. 17, 2024, 9:27 a.m. UTC | #3
Am 16.10.24 um 18:43 schrieb Adrián Larumbe:
> On 16.10.2024 15:12, Christian König wrote:
>> Am 15.10.24 um 01:31 schrieb Adrián Larumbe:
>>> Doesn't make any functional difference because generic dma_fence is the
>>> first panfrost_fence structure member, but I guess it doesn't hurt either.
>> As discussed with Sima we want to push into the exactly opposite direction
>> because that requires that the panfrost module stays loaded as long as fences
>> are around.
> Does that mean in future commits the struct dma_fence_ops' .release pointer will be
> done with altogether?

Yes, exactly that's the idea.

As a first step I'm preparing patches right now to enforce using kmalloc 
instead of driver brewed approaches for dma_fence handling.

Regards,
Christian.

>
>> So clearly a NAK to this one here. Rather document on the structure that the
>> dma_fence structure must be the first member.
>>
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
>>> ---
>>>    drivers/gpu/drm/panfrost/panfrost_job.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
>>> index 5d83c6a148ec..fa219f719bdc 100644
>>> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
>>> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
>>> @@ -85,9 +85,15 @@ static const char *panfrost_fence_get_timeline_name(struct dma_fence *fence)
>>>    	}
>>>    }
>>> +static void panfrost_fence_release(struct dma_fence *fence)
>>> +{
>>> +	kfree(to_panfrost_fence(fence));
>>> +}
>>> +
>>>    static const struct dma_fence_ops panfrost_fence_ops = {
>>>    	.get_driver_name = panfrost_fence_get_driver_name,
>>>    	.get_timeline_name = panfrost_fence_get_timeline_name,
>>> +	.release = panfrost_fence_release,
>>>    };
>>>    static struct dma_fence *panfrost_fence_create(struct panfrost_device *pfdev, int js_num)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
index 5d83c6a148ec..fa219f719bdc 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -85,9 +85,15 @@  static const char *panfrost_fence_get_timeline_name(struct dma_fence *fence)
 	}
 }
 
+static void panfrost_fence_release(struct dma_fence *fence)
+{
+	kfree(to_panfrost_fence(fence));
+}
+
 static const struct dma_fence_ops panfrost_fence_ops = {
 	.get_driver_name = panfrost_fence_get_driver_name,
 	.get_timeline_name = panfrost_fence_get_timeline_name,
+	.release = panfrost_fence_release,
 };
 
 static struct dma_fence *panfrost_fence_create(struct panfrost_device *pfdev, int js_num)