> Hi Guennadi, > > > On Mon, 30 Jul 2012, Alex Gershgorin wrote: > > > This patch check the state of the buffer when calling buf_init() method. > > The thread http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/48587 > > also includes report witch can show the problem. This patch solved the problem. > > Both MMAP and USERPTR methods was successfully tested. > > > > Signed-off-by: Alex Gershgorin <alexg@xxxxxxxxxxxxxx> > > --- > > drivers/media/video/mx3_camera.c | 12 +++++++----- > > 1 files changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c > > index f13643d..950a8fe 100644 > > --- a/drivers/media/video/mx3_camera.c > > +++ b/drivers/media/video/mx3_camera.c > > @@ -405,13 +405,15 @@ static int mx3_videobuf_init(struct vb2_buffer *vb) > > >> Sorry, don't understand. As we see in the thread, mentioned above, the > >> Oops happened in mx3_videobuf_release(). If my analysis was correct in > >> that thread, that Oops happened, when .buf_cleanup() was called without > >> .buf_init() being called. This your patch modifies mx3_videobuf_init(). > >> which isn't even called in the problem case. How does this help? > > Sorry for not quite a clear explanation, I will explain in more details. > if you divide the report into two parts: > 1) USERPTR method Oops happened as a result discontiguous memory allocation > 2) USERPTR method use framebuffer memory allocation video starting, but after a few seconds the video freezes. > if we consider the first part of the report, your analysis is absolutely > correct and unfortunately this patch does not solve the problems mentioned in the thread. > This patch solves a problem that is described in the second part of the report. > > Ok, I understand now what this patch fixes, but I still don't understand > > what the problem is and how this patch fixes it. AFAICS, there should be > > nothing wrong with calling mx3_videobuf_init() each time a buffer gets > > queued. I only see the problem when the user implements USERPTR method :-) > > I suspect, it's just one of those 4 lines of code, that you put > > under "if (buf->state != CSI_BUF_PREPARED)", that breaks it. Could you I think the best way to make sure this solves the problem is to test it, and that is what I did :-) > > please try to find out which exactly line it is? Just try to use your "if > > (...)" with each of them separately. My guess goes for > > buf->state = CSI_BUF_NEEDS_INIT; > > but it would help if you could verify it. mx3_camera uses slave dma API of the dmaengine. In MMAP and USERPTR methods, the buffer which had been prepared already have a descriptor for transaction and does not need additional descriptor. If user implement MMAP method, there is no problem because mx3_videobuf_init() happens at REQBUFS time and we have only one descriptor for transaction per video buffer. As you mentioned, In case of USERPTR method mx3_videobuf_init() happens at QBUF time. In my opinion there are two problems: 1) During QBUF there is no need to get a new descriptor for the transaction if it has been already obtained. 2) The second problem derives from the first as the number of descriptors for each channel is limited and very quickly we reach the maximum permitted number of descriptors. As a results of that, the video freezes. This patch solves the two problems. Regards, Alex > > struct mx3_camera_dev *mx3_cam = ici->priv; > > struct mx3_camera_buffer *buf = to_mx3_vb(vb); > > > > - /* This is for locking debugging only */ > > - INIT_LIST_HEAD(&buf->queue); > > - sg_init_table(&buf->sg, 1); > > + if (buf->state != CSI_BUF_PREPARED) { > > + /* This is for locking debugging only */ > > + INIT_LIST_HEAD(&buf->queue); > > + sg_init_table(&buf->sg, 1); > > > > - buf->state = CSI_BUF_NEEDS_INIT; > > + buf->state = CSI_BUF_NEEDS_INIT; > > > > - mx3_cam->buf_total += vb2_plane_size(vb, 0); > > + mx3_cam->buf_total += vb2_plane_size(vb, 0); > > + } > > > > return 0; > > } > > -- > > 1.7.0.4 > > > > Regards, > Alex > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/-- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
