From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [RFC v2 03/11] vb2: Move cache synchronisation from buffer done to dqbuf handler Date: Tue, 28 Mar 2017 15:31:59 +0300 Message-ID: <3080874.EIbUBugxIQ@avalon> References: <20161216012425.11179-1-laurent.pinchart+renesas@ideasonboard.com> <20161216012425.11179-4-laurent.pinchart+renesas@ideasonboard.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [IPv6:2001:4b98:dc2:45:216:3eff:febb:480d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2F4B389804 for ; Tue, 28 Mar 2017 12:31:21 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Shuah Khan Cc: Laurent Pinchart , Daniel Vetter , dri-devel@lists.freedesktop.org, Hans Verkuil , Kyungmin Park , Sakari Ailus , linux-media@vger.kernel.org, Pawel Osciak , Marek Szyprowski List-Id: dri-devel@lists.freedesktop.org SGkgU2h1YWgsCgpPbiBNb25kYXkgMjcgTWFyIDIwMTcgMTU6Mjc6MzYgU2h1YWggS2hhbiB3cm90 ZToKPiBPbiBUaHUsIERlYyAxNSwgMjAxNiBhdCA2OjI0IFBNLCBMYXVyZW50IFBpbmNoYXJ0IHdy b3RlOgo+ID4gRnJvbTogU2FrYXJpIEFpbHVzIDxzYWthcmkuYWlsdXNAbGludXguaW50ZWwuY29t Pgo+ID4gCj4gPiBUaGUgY2FjaGUgc3luY2hyb25pc2F0aW9uIG1heSBiZSBhIHRpbWUgY29uc3Vt aW5nIG9wZXJhdGlvbiBhbmQgdGh1cyBub3QKPiA+IGJlc3QgcGVyZm9ybWVkIGluIGFuIGludGVy cnVwdCB3aGljaCBpcyBhIHR5cGljYWwgY29udGV4dCBmb3IKPiA+IHZiMl9idWZmZXJfZG9uZSgp IGNhbGxzLiBUaGlzIG1heSBjb25zdW1lIHVwIHRvIHRlbnMgb2YgbXMgb24gc29tZQo+ID4gbWFj aGluZXMsIGRlcGVuZGluZyBvbiB0aGUgYnVmZmVyIHNpemUuCj4gPiAKPiA+IFNpZ25lZC1vZmYt Ynk6IFNha2FyaSBBaWx1cyA8c2FrYXJpLmFpbHVzQGxpbnV4LmludGVsLmNvbT4KPiA+IC0tLQo+ ID4gQ2hhbmdlcyBzaW5jZSB2MToKPiA+IAo+ID4gLSBEb24ndCByZW5hbWUgdGhlICdpJyBsb29w IGNvdW50ZXIgdG8gJ3BsYW5lJwo+ID4gLS0tCj4gPiAKPiA+ICBkcml2ZXJzL21lZGlhL3Y0bDIt Y29yZS92aWRlb2J1ZjItY29yZS5jIHwgOSArKysrLS0tLS0KPiA+ICAxIGZpbGUgY2hhbmdlZCwg NCBpbnNlcnRpb25zKCspLCA1IGRlbGV0aW9ucygtKQo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy9tZWRpYS92NGwyLWNvcmUvdmlkZW9idWYyLWNvcmUuYwo+ID4gYi9kcml2ZXJzL21lZGlh L3Y0bDItY29yZS92aWRlb2J1ZjItY29yZS5jIGluZGV4Cj4gPiA4YmE0ODcwM2IxODkuLjE1YTgz ZjMzODA3MiAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvbWVkaWEvdjRsMi1jb3JlL3ZpZGVvYnVm Mi1jb3JlLmMKPiA+ICsrKyBiL2RyaXZlcnMvbWVkaWEvdjRsMi1jb3JlL3ZpZGVvYnVmMi1jb3Jl LmMKPiA+IEBAIC04ODksNyArODg5LDYgQEAgdm9pZCB2YjJfYnVmZmVyX2RvbmUoc3RydWN0IHZi Ml9idWZmZXIgKnZiLCBlbnVtCj4gPiB2YjJfYnVmZmVyX3N0YXRlIHN0YXRlKT4gCj4gPiAgewo+ ID4gICAgICAgICBzdHJ1Y3QgdmIyX3F1ZXVlICpxID0gdmItPnZiMl9xdWV1ZTsKPiA+ICAgICAg ICAgdW5zaWduZWQgbG9uZyBmbGFnczsKPiA+IC0gICAgICAgdW5zaWduZWQgaW50IHBsYW5lOwo+ ID4gCj4gPiAgICAgICAgIGlmIChXQVJOX09OKHZiLT5zdGF0ZSAhPSBWQjJfQlVGX1NUQVRFX0FD VElWRSkpCj4gPiAgICAgICAgICAgICAgICAgcmV0dXJuOwo+ID4gQEAgLTkxMCwxMCArOTA5LDYg QEAgdm9pZCB2YjJfYnVmZmVyX2RvbmUoc3RydWN0IHZiMl9idWZmZXIgKnZiLCBlbnVtCj4gPiB2 YjJfYnVmZmVyX3N0YXRlIHN0YXRlKQo+ID4gICAgICAgICBkcHJpbnRrKDQsICJkb25lIHByb2Nl c3Npbmcgb24gYnVmZmVyICVkLCBzdGF0ZTogJWRcbiIsCj4gPiAgICAgICAgICAgICAgICAgICAg ICAgICB2Yi0+aW5kZXgsIHN0YXRlKTsKPiA+IAo+ID4gLSAgICAgICAvKiBzeW5jIGJ1ZmZlcnMg Ki8KPiA+IC0gICAgICAgZm9yIChwbGFuZSA9IDA7IHBsYW5lIDwgdmItPm51bV9wbGFuZXM7ICsr cGxhbmUpCj4gPiAtICAgICAgICAgICAgICAgY2FsbF92b2lkX21lbW9wKHZiLCBmaW5pc2gsIHZi LT5wbGFuZXNbcGxhbmVdLm1lbV9wcml2KTsKPiA+IC0KPiA+ICAgICAgICAgc3Bpbl9sb2NrX2ly cXNhdmUoJnEtPmRvbmVfbG9jaywgZmxhZ3MpOwo+ID4gICAgICAgICBpZiAoc3RhdGUgPT0gVkIy X0JVRl9TVEFURV9RVUVVRUQgfHwKPiA+ICAgICAgICAgICAgIHN0YXRlID09IFZCMl9CVUZfU1RB VEVfUkVRVUVVRUlORykgewo+ID4gQEAgLTE1NzEsNiArMTU2NiwxMCBAQCBzdGF0aWMgdm9pZCBf X3ZiMl9kcWJ1ZihzdHJ1Y3QgdmIyX2J1ZmZlciAqdmIpCj4gPiAKPiA+ICAgICAgICAgdmItPnN0 YXRlID0gVkIyX0JVRl9TVEFURV9ERVFVRVVFRDsKPiA+IAo+ID4gKyAgICAgICAvKiBzeW5jIGJ1 ZmZlcnMgKi8KPiA+ICsgICAgICAgZm9yIChpID0gMDsgaSA8IHZiLT5udW1fcGxhbmVzOyArK2kp Cj4gPiArICAgICAgICAgICAgICAgY2FsbF92b2lkX21lbW9wKHZiLCBmaW5pc2gsIHZiLT5wbGFu ZXNbaV0ubWVtX3ByaXYpOwo+ID4gKwo+IAo+IERvZXMgdGhpcyBjb21waWxlPz8gV2hlcmUgaXMg ImkiIGRlZmluZWQ/IExvb2tzIGxpa2UgaXQgbmVlZHMgdG8gYmUgYWRkZWQKPiBiYWNrIGluLgoK SXQncyBhbHJlYWR5IGRlZmluZWQgYXQgdGhlIGJlZ2lubmluZyBvZiB0aGUgX192YjJfZHFidWYo KSBmdW5jdGlvbi4KCj4gPiAgICAgICAgIC8qIHVubWFwIERNQUJVRiBidWZmZXIgKi8KPiA+ICAg ICAgICAgaWYgKHEtPm1lbW9yeSA9PSBWQjJfTUVNT1JZX0RNQUJVRikKPiA+ICAgICAgICAgICAg ICAgICBmb3IgKGkgPSAwOyBpIDwgdmItPm51bV9wbGFuZXM7ICsraSkgewoKLS0gClJlZ2FyZHMs CgpMYXVyZW50IFBpbmNoYXJ0CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVz a3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9k cmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from galahad.ideasonboard.com ([185.26.127.97]:35961 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932717AbdC1MyM (ORCPT ); Tue, 28 Mar 2017 08:54:12 -0400 From: Laurent Pinchart To: Shuah Khan Cc: Laurent Pinchart , linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, Pawel Osciak , Marek Szyprowski , Kyungmin Park , Hans Verkuil , Sumit Semwal , Rob Clark , Daniel Vetter , Laura Abbott , Sakari Ailus Subject: Re: [RFC v2 03/11] vb2: Move cache synchronisation from buffer done to dqbuf handler Date: Tue, 28 Mar 2017 15:31:59 +0300 Message-ID: <3080874.EIbUBugxIQ@avalon> In-Reply-To: References: <20161216012425.11179-1-laurent.pinchart+renesas@ideasonboard.com> <20161216012425.11179-4-laurent.pinchart+renesas@ideasonboard.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-media-owner@vger.kernel.org List-ID: Hi Shuah, On Monday 27 Mar 2017 15:27:36 Shuah Khan wrote: > On Thu, Dec 15, 2016 at 6:24 PM, Laurent Pinchart wrote: > > From: Sakari Ailus > > > > The cache synchronisation may be a time consuming operation and thus not > > best performed in an interrupt which is a typical context for > > vb2_buffer_done() calls. This may consume up to tens of ms on some > > machines, depending on the buffer size. > > > > Signed-off-by: Sakari Ailus > > --- > > Changes since v1: > > > > - Don't rename the 'i' loop counter to 'plane' > > --- > > > > drivers/media/v4l2-core/videobuf2-core.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > > b/drivers/media/v4l2-core/videobuf2-core.c index > > 8ba48703b189..15a83f338072 100644 > > --- a/drivers/media/v4l2-core/videobuf2-core.c > > +++ b/drivers/media/v4l2-core/videobuf2-core.c > > @@ -889,7 +889,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum > > vb2_buffer_state state)> > > { > > struct vb2_queue *q = vb->vb2_queue; > > unsigned long flags; > > - unsigned int plane; > > > > if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE)) > > return; > > @@ -910,10 +909,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum > > vb2_buffer_state state) > > dprintk(4, "done processing on buffer %d, state: %d\n", > > vb->index, state); > > > > - /* sync buffers */ > > - for (plane = 0; plane < vb->num_planes; ++plane) > > - call_void_memop(vb, finish, vb->planes[plane].mem_priv); > > - > > spin_lock_irqsave(&q->done_lock, flags); > > if (state == VB2_BUF_STATE_QUEUED || > > state == VB2_BUF_STATE_REQUEUEING) { > > @@ -1571,6 +1566,10 @@ static void __vb2_dqbuf(struct vb2_buffer *vb) > > > > vb->state = VB2_BUF_STATE_DEQUEUED; > > > > + /* sync buffers */ > > + for (i = 0; i < vb->num_planes; ++i) > > + call_void_memop(vb, finish, vb->planes[i].mem_priv); > > + > > Does this compile?? Where is "i" defined? Looks like it needs to be added > back in. It's already defined at the beginning of the __vb2_dqbuf() function. > > /* unmap DMABUF buffer */ > > if (q->memory == VB2_MEMORY_DMABUF) > > for (i = 0; i < vb->num_planes; ++i) { -- Regards, Laurent Pinchart