| Message ID | 20210816043138.957984-6-hiroh@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Mon, Aug 16, 2021 at 01:31:33PM +0900, Hirokazu Honda wrote: > IPABuffer is represented by FrameBuffer. FrameBuffer::Plane has > now an offset. This uses the offset variable to map the IPABuffer. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > --- > src/ipa/rkisp1/rkisp1.cpp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 06fb9640..eb7a2705 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -165,7 +165,7 @@ void IPARkISP1::mapBuffers(const std::vector<IPABuffer> &buffers) > * to applications). > */ > buffersMemory_[buffer.id] = mmap(NULL, > - fb.planes()[0].length, > + fb.planes()[0].offset + fb.planes()[0].length, Is this right ? You're mapping offset + length bytes at offset 0 from the beginning of the dmabuf. The munmap() call is updated accordingly, but the code that uses the mapped memory isn't, and will look at offset 0. Now, as those buffers, are used for statistics and ISP parameters, the offset will always be zero, but the code isn't really correct. How about updating the RkISP1 IPA to use MappedFrameBuffer instead, like the other IPA modules ? > PROT_READ | PROT_WRITE, > MAP_SHARED, > fb.planes()[0].fd.fd(), > @@ -186,7 +186,7 @@ void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids) > if (fb == buffers_.end()) > continue; > > - munmap(buffersMemory_[id], fb->second.planes()[0].length); > + munmap(buffersMemory_[id], fb->second.planes()[0].offset + fb->second.planes()[0].length); > buffersMemory_.erase(id); > buffers_.erase(id); > }
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 06fb9640..eb7a2705 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -165,7 +165,7 @@ void IPARkISP1::mapBuffers(const std::vector<IPABuffer> &buffers) * to applications). */ buffersMemory_[buffer.id] = mmap(NULL, - fb.planes()[0].length, + fb.planes()[0].offset + fb.planes()[0].length, PROT_READ | PROT_WRITE, MAP_SHARED, fb.planes()[0].fd.fd(), @@ -186,7 +186,7 @@ void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids) if (fb == buffers_.end()) continue; - munmap(buffersMemory_[id], fb->second.planes()[0].length); + munmap(buffersMemory_[id], fb->second.planes()[0].offset + fb->second.planes()[0].length); buffersMemory_.erase(id); buffers_.erase(id); }
IPABuffer is represented by FrameBuffer. FrameBuffer::Plane has now an offset. This uses the offset variable to map the IPABuffer. Signed-off-by: Hirokazu Honda <hiroh@chromium.org> --- src/ipa/rkisp1/rkisp1.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)