Hi Nicholas
Sorry for the slight delay in replying.
On Mon, 11 May 2020 at 20:19, Nicolas Saenz Julienne
<nsaenzjulienne@xxxxxxx> wrote:
>
> Hi Naushir,
> a small comment.
>
> On Mon, 2020-05-04 at 12:26 +0300, Laurent Pinchart wrote:
> > From: Naushir Patuck <naush@xxxxxxxxxxxxxxx>
> >
> > Port the V4L2 compatible driver for the ISP unit found on Broadcom BCM2835
> > chips.
> >
> > The driver interfaces though the VideoCore unit using the VCHIQ MMAL
> > interface.
> >
> > ISP driver upported from from RaspberryPi BSP at revision:
> > 6c3505be6c3e ("staging: vc04_services: isp: Make all references to
> > bcm2835_isp_fmt const")
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > Signed-off-by: Naushir Patuck <naush@xxxxxxxxxxxxxxx>
> > [Adapt to staging by moving all modifications that in the BSP are scattered
> > in core components inside this directory]
> > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> > ---
>
> [...]
>
> > +static int bcm2835_isp_mmal_buf_cleanup(struct mmal_buffer *mmal_buf)
> > +{
> > + mmal_vchi_buffer_cleanup(mmal_buf);
> > +
> > + if (mmal_buf->dma_buf) {
> > + dma_buf_put(mmal_buf->dma_buf);
> > + mmal_buf->dma_buf = NULL;
>
> Why is this needed here, shouldn't this be mmal-vchi's responsibility? IIUC the
> original dma_buf_get() happens there too.
The original dma_buf_get is in bcm2835_isp_buf_prepare as it either
comes from a dma_buf_get(vb->planes[0].m.fd) for VB2_MEMORY_DMABUF
operations, or a vb2_core_expbuf_dmabuf for VB2_MEMORY_MMAP.
Seeing as the original "get" is in this calling code, it seemed
reasonable that the put is as well.
There are no get/put operations on dma-bufs (other than indirectly
through vc_sm_cma) within mmal-vchiq. You have the call
vc_sm_cma_import_dmabuf to take the external dma_buf and pull it into
vc_sm_cma, but that is the limit of it.
Dave
