Message ID | CALYq+qSVbQB7NXLLNqOWwFWqiTTfbdgCf+mRFQAXWkE=35mkOA@mail.gmail.com |
---|---|
State | New |
Headers | show |
Hi Abhinav, Could you provide more information about the testing scenario? Was it MFC + FIMC, or MFC + DRM, or MFC + FIMC + DRM, or other? According to the DMABUF spec map/unmap calls should be done just before a buffer is accessed by HW. In case of vb2, dma_buf_map_attachment is called just after passing the buffer to the vb2 queue. IMO, it is too early. The mapping call should be moved to __enqueue_in_driver function one day. As I understand the DMABUF assumptions, it is exporter code to be responsible for caching mappings for dmabuf attachments. I applied some caching policy in vb2-dma-contig exporter. The similar treat should be ported to the DRM code. I am waiting for opinions about this solution. Regards, Tomasz Stanislawski On 06/14/2012 06:48 PM, Abhinav Kochhar wrote: > Hi Tomasz,Sumit, > > We have been doing performance analysis for video playback scenarios. > Could get significant improvement in FPS (33% or 8~10 fps) for full > HD resolution when the map/unmap sequence is not called. > This is a considerable improvement in performance and would be helpful > to have some logical way using e.g. "dmabuf hint" to avoid what looks > like a bottleneck (map/unmap sequence). > > For your reference I have I have copied the changes below. > Please note this is a change for verification into the vb2 core to add > an additional condition check for avoiding the calls to map/unmap if > the buffer has been already mapped once. > > > diff --git a/drivers/media/video/videobuf2-core.c > b/drivers/media/video/videobuf2-core.c > index 1e0917d..e48a7fc 100644 > --- a/drivers/media/video/videobuf2-core.c > +++ b/drivers/media/video/videobuf2-core.c > @@ -1098,12 +1098,15 @@ static int __qbuf_dmabuf(struct vb2_buffer > *vb, const struct v4l2_buffer *b) > * really we want to do this just before the DMA, not while queueing > * the buffer(s).. > */ > + > for (plane = 0; plane < vb->num_planes; ++plane) { > - ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv); > - if (ret) { > - dprintk(1, "qbuf: failed mapping dmabuf " > - "memory for plane %d\n", plane); > - goto err; > + if( vb->planes[plane].dbuf_mapped != 1) { > + ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv); > + if (ret) { > + dprintk(1, "qbuf: failed mapping dmabuf " > + "memory for plane %d\n", plane); > + goto err; > + } > } > vb->planes[plane].dbuf_mapped = 1; > } > @@ -1525,14 +1528,6 @@ int vb2_dqbuf(struct vb2_queue *q, struct > v4l2_buffer *b, bool nonblocking) > * really we want to do this just after DMA, not when the > * buffer is dequeued.. > */ > - if (q->memory == V4L2_MEMORY_DMABUF) { > - unsigned int i; > - > - for (i = 0; i < vb->num_planes; ++i) { > - call_memop(q, unmap_dmabuf, vb->planes[i].mem_priv); > - vb->planes[i].dbuf_mapped = 0; > - } > - } > > switch (vb->state) { > case VB2_BUF_STATE_DONE:
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index 1e0917d..e48a7fc 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1098,12 +1098,15 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) * really we want to do this just before the DMA, not while queueing * the buffer(s).. */ + for (plane = 0; plane < vb->num_planes; ++plane) { - ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv); - if (ret) { - dprintk(1, "qbuf: failed mapping dmabuf " - "memory for plane %d\n", plane); - goto err; + if( vb->planes[plane].dbuf_mapped != 1) { + ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv); + if (ret) { + dprintk(1, "qbuf: failed mapping dmabuf " + "memory for plane %d\n", plane); + goto err; + } } vb->planes[plane].dbuf_mapped = 1;