Re: [PATCH 3/9] dma-heap: Provide accessors so that in-kernel drivers can allocate dmabufs from specific heaps

[Nicolas Dufresne <nicolas@ndufresne.ca>]

Le mardi 12 septembre 2023 à 16:46 +0200, Christian König a écrit :
> Am 12.09.23 um 10:52 schrieb Yong Wu (吴勇):
> > [SNIP]
> > > But what we should try to avoid is that newly merged drivers provide
> > > both a driver specific UAPI and DMA-heaps. The justification that
> > > this
> > > makes it easier to transit userspace to the new UAPI doesn't really
> > > count.
> > > 
> > > That would be adding UAPI already with a plan to deprecate it and
> > > that
> > > is most likely not helpful considering that UAPI must be supported
> > > forever as soon as it is upstream.
> > Sorry, I didn't understand this. I think we have not change the UAPI.
> > Which code are you referring to?
> 
> Well, what do you need this for if not a new UAPI?
> 
> My assumption here is that you need to export the DMA-heap allocation 
> function so that you can server an UAPI in your new driver. Or what else 
> is that good for?
> 
> As far as I understand you try to upstream your new vcodec driver. So 
> while this change here seems to be a good idea to clean up existing 
> drivers it doesn't look like a good idea for a newly created driver.

MTK VCODEC has been upstream for quite some time now. The other patchset is
trying to add secure decoding/encoding support to that existing upstream driver.

Regarding the uAPI, it seems that this addition to dmabuf heap internal API is
exactly the opposite. By making heaps available to drivers, modification to the
V4L2 uAPI is being reduced to adding "SECURE_MODE" + "SECURE_HEAP_ID" controls
(this is not debated yet has an approach). The heaps is being used internally in
replacement to every allocation, user visible or not.

Nicolas

> 
> Regards,
> Christian.
> 
> > > > So I think this patch is a little confusing in this series, as I
> > > don't
> > > > see much of it actually being used here (though forgive me if I'm
> > > > missing it).
> > > > 
> > > > Instead, It seems it get used in a separate patch series here:
> > > >     
> > > https://lore.kernel.org/all/20230911125936.10648-1-yunfei.dong@mediatek.com/
> > > 
> > > Please try to avoid stuff like that it is really confusing and eats
> > > reviewers time.
> > My fault, I thought dma-buf and media belonged to the different tree,
> > so I send them separately. The cover letter just said "The consumers of
> > the new heap and new interface are our codecs and DRM, which will be
> > sent upstream soon", and there was no vcodec link at that time.
> > 
> > In the next version, we will put the first three patches into the
> > vcodec patchset.
> > 
> > Thanks.
> > 
>