Re: [PATCH v4 3/7] accel/ivpu: Add GEM buffer object management

[Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>]

Hi,

On 06.01.2023 19:25, Daniel Vetter wrote:
> On Fri, 6 Jan 2023 at 14:23, Stanislaw Gruszka
> <stanislaw.gruszka@linux.intel.com> wrote:
>>
>> On Fri, Jan 06, 2023 at 11:50:05AM +0100, Daniel Vetter wrote:
>>> On Thu, Dec 08, 2022 at 12:07:29PM +0100, Jacek Lawrynowicz wrote:
>>>> Adds four types of GEM-based BOs for the VPU:
>>>>   - shmem
>>>>   - userptr
>>>>   - internal
>>>
>>> Uh what do you need this for? Usually the way we do these is just alloce a
>>> normal bo, and then pin them.
>>
>> I think we do alloc/pin this way, but all our bo's are GEM based.
>> For those bo's we use internally and other non-shmem we create them
>> with drm_gem_private_object_init(). I think this way is simpler than
>> have separate code for non-GEM and GEM bo's ...
> 
> They should be all gem bo, I guess you mean shmem vs non-shmem? And
> the allocate+pin is the standard approach for drivers that have
> somewhat dynamic bo (i.e. not using dma_alloc) and need some of them
> (hopefully only for driver internal objects, not for userspace) pinned
> in place. So you handrolling a perma-pinned gem bo for internal
> objects is rather strange by drm driver standards.
> 
>>> Also, gem shmem helpers should be able to mostly cover you here, why not
>>> use those? Might need some work to push basic userptr to them, but we have
>>> enough drivers reinventing that wheel to justify that work.
>>>
>>> Can I guess also be done after merging.
>>
>> ... but if not, we can add this to TODO.
> 
> Yeah I'm fine with todo to cut these over to shmem helpers, this
> driver has been stuck in limbo for way too long anyway.

Yeah, I think it would be easier for everyone if this driver was merged.
Especially for me :)
I feel like I'm shifting tons of coal every time I need to update it.
But also from the reviewer perspective it would be easier to track changes
between driver revisions if only delta was posted instead of the whole 8K lines of code.

Guys, please make my day and merge it to 6.3.

Regards,
Jacek