[{"id":10955,"web_url":"https://patchwork.libcamera.org/comment/10955/","msgid":"","date":"2020-06-29T09:55:47","subject":"Re: [libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring\n\tof ImgU","submitter":{"id":9,"url":"https://patchwork.libcamera.org/api/people/9/","name":"Tomasz Figa","email":"tfiga@chromium.org"},"content":"Hi Niklas and everyone,\n\nOn Sun, Jun 28, 2020 at 2:15 AM Niklas Söderlund\n wrote:\n>\n> Hi,\n>\n> This series do some cleanup of the IPU3 pipeline. It removes dead or\n> almost dead code, replacing it with something more coherent to what is\n> done elsewhere in the pipeline. It breaks out the ImgU to a separate cpp\n> and h file.\n>\n> It reworks a bit how the streams are identified and handled inside the\n> pipeline. When the pipeline was first developed only the output and\n> viewfinder streams where available, now we have a RAW stream in addition\n> to this. Some of the abstraction around streams and which hardware\n> resource they are backed by made sens in the earlier context but not so\n> much once RAW is added to the mix. The biggest difference in this rework\n> is that the ImgUDevice gets explicit functions to configure each of its\n> sink instead of having a generic one which depends which pointer is\n> passed to it. This makes reading code where RAW and ImgU streams are\n> mixed much nicer IMHO.\n\nCIO2 is quite a generic capture interface, which could be handled by\nsome platform-independent code. Have we considered splitting the\npipeline into a generic capture part and platform-specific processing\npart? I think it could be useful on its own, as the capture part of\nall the SoCs could be handled by generic code, but I also suspect that\nthe ability to split the pipeline into multiple smaller pipelines and\nthen join them together could be very useful when dealing with many\nSoC generations, since vendors quite often replace some parts of the\npipeline with new hardware, while keeping the others unchanged.\n\nWhat do you think?\n\nBest regards,\nTomasz\n\n>\n> Lastly some assumptions that buffers must be allocated at video nodes\n> that are not involved in the capture session are being challenged. This\n> was true a year ago but not any more it seems. Chancing this simplifies\n> the driver enormously and saves on memory that otherwise would be\n> wasted. I have really tried to force the end result to failed by\n> resetting the hardware between each test so that no video node\n> configuration from a previous sessions saves the day.\n>\n> I have for both sensors reset the hardware and then tested the following\n> capture combinations successfully. After the first capture session for\n> each one in the list that was done after a power cycle all the other\n> captures where tried in a semi random order and always succeeded.\n>\n> cam -c 1 -s role=viewfinder -C\n> cam -c 1 -s role=still -C\n> cam -c 1 -s role=still -s role=viewfinder -C\n> cam -c 1 -s role=still -s role=viewfinder -s role=stillraw -C\n> cam -c 1 -s role=stillraw -C\n>\n> Niklas Söderlund (13):\n> libcamera: ipu3: Remove unused name_ filed from IPU3Stream\n> libcamera: ipu3: Import instead of allocate statistic buffers\n> libcamera: ipu3: Always import buffers for ImgU sinks\n> libcamera: ipu3: Remove usage of IPU3CameraData from ImgUDevice\n> libcamera: ipu3: imgu: Move the ImgUDevice class to separate files\n> libcamera: ipu3: imgu: Do not cache index\n> libcamera: ipu3: imgu: Mark things that are internal as private\n> libcamera: ipu3: imgu: Use specific functions to configure each sink\n> libcamera: ipu3: Do not duplicate data in IPU3Stream\n> libcamera: ipu3: Remove the active flag from IPU3Stream\n> libcamera: ipu3: Remove IPU3Stream\n> libcamera: ipu3: imgu: Remove ImgUOutput\n> libcamera: ipu3: imgu: Use unique_ptr for video and subdevices\n>\n> src/libcamera/pipeline/ipu3/imgu.cpp | 353 +++++++++++++++\n> src/libcamera/pipeline/ipu3/imgu.h | 87 ++++\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 548 +++---------------------\n> src/libcamera/pipeline/ipu3/meson.build | 1 +\n> 4 files changed, 505 insertions(+), 484 deletions(-)\n> create mode 100644 src/libcamera/pipeline/ipu3/imgu.cpp\n> create mode 100644 src/libcamera/pipeline/ipu3/imgu.h\n>\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F1ADDC2E6B\n\tfor ;\n\tMon, 29 Jun 2020 09:56:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DC78609C9;\n\tMon, 29 Jun 2020 11:56:02 +0200 (CEST)","from mail-ed1-x544.google.com (mail-ed1-x544.google.com\n\t[IPv6:2a00:1450:4864:20::544])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0730E609C2\n\tfor ;\n\tMon, 29 Jun 2020 11:56:01 +0200 (CEST)","by mail-ed1-x544.google.com with SMTP id h28so12310024edz.0\n\tfor ;\n\tMon, 29 Jun 2020 02:56:00 -0700 (PDT)","from mail-wr1-f48.google.com (mail-wr1-f48.google.com.\n\t[209.85.221.48]) by smtp.gmail.com with ESMTPSA id\n\tq24sm23191772edg.51.2020.06.29.02.55.59\n\tfor \n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 29 Jun 2020 02:55:59 -0700 (PDT)","by mail-wr1-f48.google.com with SMTP id f7so12888955wrw.1\n\tfor ;\n\tMon, 29 Jun 2020 02:55:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"OGFEDviv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=EXb038DUqJAAtUOg8ZChW3OQo1d5xy2B5Sb79k1ELaQ=;\n\tb=OGFEDvivXD1CpF3i+9MhyLQBYlDAhZBWnzloCpuTr9B6R6YqpT8p9L+nfSeylDVsm7\n\tepN7ta1dgnlT8L9MvhbaPPO0w+ugNKt5ULWmcu1VAvm+Z+Md8mXvj2bD00O4S6Ykom2+\n\tPBg9GIXZIeEaKmGIlffOZhcONQ3S7Pnngpzf4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=EXb038DUqJAAtUOg8ZChW3OQo1d5xy2B5Sb79k1ELaQ=;\n\tb=qNEQZgkI2DkdVqXzbvFZx+dy3aw/TVh+P1qL0SanZV2CgpQ0tnxvKBT3GPbb8IC6Ek\n\tqqzx9xtG+xbHr+KZkpWKa/1sDLa9h1sWqPmOUHQIffdwJJDxkWfkddpuVe1v5nd8UQrP\n\trywbWZWqx8Me9yobSchtokwLKsM83Y+5F2XrXtra5LJhlk7rfxZOSEFfY+SQgJ6e41Tr\n\tg+XsLdvyTztF44peyWGLivHss9y85ahAJshLfvS8mGCff74tDJBz25F2HIluCknPSM/l\n\tMMpQ89HUDCYwvwiOyWLEpATH3cJhFq0EN8X9v964Xkvtp55MDsN5m1yHM+tCz4JwNJYc\n\tbi6Q==","X-Gm-Message-State":"AOAM531YYhtc05po/tvh0UD311watI11nScJTkt0ayHlKg9BsXYcbAbh\n\trYGMvnDxUqPcjR0d5Il8Vtqag3drYp1g0w==","X-Google-Smtp-Source":"ABdhPJzPI5km0L/pS1D6xH4MPpEVedZvrBnXEcTUlxByeBEb4Fco/YGy3/NnKmsxID2D/KS2MfwBpA==","X-Received":["by 2002:aa7:df08:: with SMTP id\n\tc8mr16427971edy.372.1593424560226; \n\tMon, 29 Jun 2020 02:56:00 -0700 (PDT)","by 2002:adf:80e6:: with SMTP id 93mr15766819wrl.17.1593424558899;\n\tMon, 29 Jun 2020 02:55:58 -0700 (PDT)"],"MIME-Version":"1.0","References":"<20200628001532.2685967-1-niklas.soderlund@ragnatech.se>","In-Reply-To":"<20200628001532.2685967-1-niklas.soderlund@ragnatech.se>","From":"Tomasz Figa ","Date":"Mon, 29 Jun 2020 11:55:47 +0200","X-Gmail-Original-Message-ID":"","Message-ID":"","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","Subject":"Re: [libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring\n\tof ImgU","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10957,"web_url":"https://patchwork.libcamera.org/comment/10957/","msgid":"<20200629102155.GS1105424@oden.dyn.berto.se>","date":"2020-06-29T10:21:55","subject":"Re: [libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring\n\tof ImgU","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Tomasz,\n\nOn 2020-06-29 11:55:47 +0200, Tomasz Figa wrote:\n> Hi Niklas and everyone,\n> \n> On Sun, Jun 28, 2020 at 2:15 AM Niklas Söderlund\n> wrote:\n> >\n> > Hi,\n> >\n> > This series do some cleanup of the IPU3 pipeline. It removes dead or\n> > almost dead code, replacing it with something more coherent to what is\n> > done elsewhere in the pipeline. It breaks out the ImgU to a separate cpp\n> > and h file.\n> >\n> > It reworks a bit how the streams are identified and handled inside the\n> > pipeline. When the pipeline was first developed only the output and\n> > viewfinder streams where available, now we have a RAW stream in addition\n> > to this. Some of the abstraction around streams and which hardware\n> > resource they are backed by made sens in the earlier context but not so\n> > much once RAW is added to the mix. The biggest difference in this rework\n> > is that the ImgUDevice gets explicit functions to configure each of its\n> > sink instead of having a generic one which depends which pointer is\n> > passed to it. This makes reading code where RAW and ImgU streams are\n> > mixed much nicer IMHO.\n> \n> CIO2 is quite a generic capture interface, which could be handled by\n> some platform-independent code. Have we considered splitting the\n> pipeline into a generic capture part and platform-specific processing\n> part? I think it could be useful on its own, as the capture part of\n> all the SoCs could be handled by generic code, but I also suspect that\n> the ability to split the pipeline into multiple smaller pipelines and\n> then join them together could be very useful when dealing with many\n> SoC generations, since vendors quite often replace some parts of the\n> pipeline with new hardware, while keeping the others unchanged.\n> \n> What do you think?\n\nI think it is a good idea and something that have been lurking in the \nback of my head for some time. I'm even entertaining the thought if we \ncould levy the simple-pipeline handler here somehow. I still have to dig \nin the code to see what is possible and what is not, and what is a good \nidea and what is not :-)\n\nRight now tho I think the most important area to try and help out \npipeline handlers is around the Request object and how internal buffers \n(RAW, Statistics and Parameters) are attached to it when for example the \nRAW buffer could come from the application or from an internal pool. \nThis will be a common problem for all pipelines and it would be nice if \nthe core could help out here.\n\n> \n> Best regards,\n> Tomasz\n> \n> >\n> > Lastly some assumptions that buffers must be allocated at video nodes\n> > that are not involved in the capture session are being challenged. This\n> > was true a year ago but not any more it seems. Chancing this simplifies\n> > the driver enormously and saves on memory that otherwise would be\n> > wasted. I have really tried to force the end result to failed by\n> > resetting the hardware between each test so that no video node\n> > configuration from a previous sessions saves the day.\n> >\n> > I have for both sensors reset the hardware and then tested the following\n> > capture combinations successfully. After the first capture session for\n> > each one in the list that was done after a power cycle all the other\n> > captures where tried in a semi random order and always succeeded.\n> >\n> > cam -c 1 -s role=viewfinder -C\n> > cam -c 1 -s role=still -C\n> > cam -c 1 -s role=still -s role=viewfinder -C\n> > cam -c 1 -s role=still -s role=viewfinder -s role=stillraw -C\n> > cam -c 1 -s role=stillraw -C\n> >\n> > Niklas Söderlund (13):\n> > libcamera: ipu3: Remove unused name_ filed from IPU3Stream\n> > libcamera: ipu3: Import instead of allocate statistic buffers\n> > libcamera: ipu3: Always import buffers for ImgU sinks\n> > libcamera: ipu3: Remove usage of IPU3CameraData from ImgUDevice\n> > libcamera: ipu3: imgu: Move the ImgUDevice class to separate files\n> > libcamera: ipu3: imgu: Do not cache index\n> > libcamera: ipu3: imgu: Mark things that are internal as private\n> > libcamera: ipu3: imgu: Use specific functions to configure each sink\n> > libcamera: ipu3: Do not duplicate data in IPU3Stream\n> > libcamera: ipu3: Remove the active flag from IPU3Stream\n> > libcamera: ipu3: Remove IPU3Stream\n> > libcamera: ipu3: imgu: Remove ImgUOutput\n> > libcamera: ipu3: imgu: Use unique_ptr for video and subdevices\n> >\n> > src/libcamera/pipeline/ipu3/imgu.cpp | 353 +++++++++++++++\n> > src/libcamera/pipeline/ipu3/imgu.h | 87 ++++\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 548 +++---------------------\n> > src/libcamera/pipeline/ipu3/meson.build | 1 +\n> > 4 files changed, 505 insertions(+), 484 deletions(-)\n> > create mode 100644 src/libcamera/pipeline/ipu3/imgu.cpp\n> > create mode 100644 src/libcamera/pipeline/ipu3/imgu.h\n> >\n> > --\n> > 2.27.0\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 920D6BF415\n\tfor ;\n\tMon, 29 Jun 2020 10:21:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C3EC609DB;\n\tMon, 29 Jun 2020 12:21:59 +0200 (CEST)","from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B005F609C8\n\tfor ;\n\tMon, 29 Jun 2020 12:21:57 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id b25so13787560ljp.6\n\tfor ;\n\tMon, 29 Jun 2020 03:21:57 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\th189sm9019230lfd.47.2020.06.29.03.21.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 29 Jun 2020 03:21:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"V/ncOBsw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=2d/zkwh2CGUWdWgHZDsME+LoV+PNyBUqIDO0FdmLhY4=;\n\tb=V/ncOBswhbU3LNfdYLNma8IT/jDkmWECVPuIh5iaTZsM4y0GA3spfTRsdPHFOqQsp+\n\tufrRhArK+4N2nREtwstPiNhpB2ggvV2Dancp0JzxRLSqajsK/g1SKmV46ESV0d7nmGO+\n\tIYZzsT4x4+eclO0jJ4Yalk+06aHhBd7Qh8LQq06QhcTldQLXazXcSszpITDvOKEaPwjJ\n\tz+I7wBoT0mNHVZczZfL1xYRKDgIQBn+7MgmBXYgCYSjAJbzG2WS7bDnY1yP4U6asopcT\n\tlMp+L8+oPI2n+QDZUwur9JDi0QcP3f3eXFguMb0PhiKBGr9eDSQklCKwE+Zzy4rvTjMF\n\teMgA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=2d/zkwh2CGUWdWgHZDsME+LoV+PNyBUqIDO0FdmLhY4=;\n\tb=jA6XRmEmMtO73YRE/a+RqM532EWFMBmjCEVmuetEY8647CZZWncEc+9r+tH9Zm+Aob\n\tW5oJl3ncB0SJ3BxNgP41j77DDuy47JUqxklcndlLVs53e3owaUoleZ8hMuFUrIPRFzWN\n\toL9g6ZLaeOf+f16oALfCqOICFC0rNQ3dTshJr2Do6yhQ1wtXoBaj4CHBx20vYOiwLSrd\n\tpUjcLZgogD70DdC5iCCU5DO0SJ+1DkH4nO1zqLri/BkkLf7+ZVpTkOkYfmuvI0pnvfsn\n\tE6DY9xkuC9nip/pt3pFugTG4VFSqRommzw4L9py2avWSmH6gjwg5LvU7LcEVUOOrlnsj\n\tNXxw==","X-Gm-Message-State":"AOAM531iCc7IUZowyz7VTOk7HBa3ZppuDGDtKproUeFFwDzZIou8hT67\n\tL6Yc7jJ5dF3O5xLZR7aRgZ/n3Q==","X-Google-Smtp-Source":"ABdhPJxjtf+EhCyf7rpGxB8icq0+mmCxZu0PcaCcCMGuP13ixin2C64REpB6q+2ob6bG/xm1LmkGjQ==","X-Received":"by 2002:a2e:96c2:: with SMTP id d2mr7688340ljj.439.1593426116975;\n\tMon, 29 Jun 2020 03:21:56 -0700 (PDT)","Date":"Mon, 29 Jun 2020 12:21:55 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Tomasz Figa ","Message-ID":"<20200629102155.GS1105424@oden.dyn.berto.se>","References":"<20200628001532.2685967-1-niklas.soderlund@ragnatech.se>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring\n\tof ImgU","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10959,"web_url":"https://patchwork.libcamera.org/comment/10959/","msgid":"","date":"2020-06-29T10:29:40","subject":"Re: [libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring\n\tof ImgU","submitter":{"id":9,"url":"https://patchwork.libcamera.org/api/people/9/","name":"Tomasz Figa","email":"tfiga@chromium.org"},"content":"On Mon, Jun 29, 2020 at 12:22 PM Niklas Söderlund\n wrote:\n>\n> Hi Tomasz,\n>\n> On 2020-06-29 11:55:47 +0200, Tomasz Figa wrote:\n> > Hi Niklas and everyone,\n> >\n> > On Sun, Jun 28, 2020 at 2:15 AM Niklas Söderlund\n> > wrote:\n> > >\n> > > Hi,\n> > >\n> > > This series do some cleanup of the IPU3 pipeline. It removes dead or\n> > > almost dead code, replacing it with something more coherent to what is\n> > > done elsewhere in the pipeline. It breaks out the ImgU to a separate cpp\n> > > and h file.\n> > >\n> > > It reworks a bit how the streams are identified and handled inside the\n> > > pipeline. When the pipeline was first developed only the output and\n> > > viewfinder streams where available, now we have a RAW stream in addition\n> > > to this. Some of the abstraction around streams and which hardware\n> > > resource they are backed by made sens in the earlier context but not so\n> > > much once RAW is added to the mix. The biggest difference in this rework\n> > > is that the ImgUDevice gets explicit functions to configure each of its\n> > > sink instead of having a generic one which depends which pointer is\n> > > passed to it. This makes reading code where RAW and ImgU streams are\n> > > mixed much nicer IMHO.\n> >\n> > CIO2 is quite a generic capture interface, which could be handled by\n> > some platform-independent code. Have we considered splitting the\n> > pipeline into a generic capture part and platform-specific processing\n> > part? I think it could be useful on its own, as the capture part of\n> > all the SoCs could be handled by generic code, but I also suspect that\n> > the ability to split the pipeline into multiple smaller pipelines and\n> > then join them together could be very useful when dealing with many\n> > SoC generations, since vendors quite often replace some parts of the\n> > pipeline with new hardware, while keeping the others unchanged.\n> >\n> > What do you think?\n>\n> I think it is a good idea and something that have been lurking in the\n> back of my head for some time. I'm even entertaining the thought if we\n> could levy the simple-pipeline handler here somehow. I still have to dig\n> in the code to see what is possible and what is not, and what is a good\n> idea and what is not :-)\n>\n\nCool. Please let me know if I could be useful in brainstorming various ideas.\n\n> Right now tho I think the most important area to try and help out\n> pipeline handlers is around the Request object and how internal buffers\n> (RAW, Statistics and Parameters) are attached to it when for example the\n> RAW buffer could come from the application or from an internal pool.\n> This will be a common problem for all pipelines and it would be nice if\n> the core could help out here.\n\nAgreed. It was more of just a long term note. I agree that this series\nmakes for a good overall improvement on its own.\n\n>\n> >\n> > Best regards,\n> > Tomasz\n> >\n> > >\n> > > Lastly some assumptions that buffers must be allocated at video nodes\n> > > that are not involved in the capture session are being challenged. This\n> > > was true a year ago but not any more it seems. Chancing this simplifies\n> > > the driver enormously and saves on memory that otherwise would be\n> > > wasted. I have really tried to force the end result to failed by\n> > > resetting the hardware between each test so that no video node\n> > > configuration from a previous sessions saves the day.\n> > >\n> > > I have for both sensors reset the hardware and then tested the following\n> > > capture combinations successfully. After the first capture session for\n> > > each one in the list that was done after a power cycle all the other\n> > > captures where tried in a semi random order and always succeeded.\n> > >\n> > > cam -c 1 -s role=viewfinder -C\n> > > cam -c 1 -s role=still -C\n> > > cam -c 1 -s role=still -s role=viewfinder -C\n> > > cam -c 1 -s role=still -s role=viewfinder -s role=stillraw -C\n> > > cam -c 1 -s role=stillraw -C\n> > >\n> > > Niklas Söderlund (13):\n> > > libcamera: ipu3: Remove unused name_ filed from IPU3Stream\n> > > libcamera: ipu3: Import instead of allocate statistic buffers\n> > > libcamera: ipu3: Always import buffers for ImgU sinks\n> > > libcamera: ipu3: Remove usage of IPU3CameraData from ImgUDevice\n> > > libcamera: ipu3: imgu: Move the ImgUDevice class to separate files\n> > > libcamera: ipu3: imgu: Do not cache index\n> > > libcamera: ipu3: imgu: Mark things that are internal as private\n> > > libcamera: ipu3: imgu: Use specific functions to configure each sink\n> > > libcamera: ipu3: Do not duplicate data in IPU3Stream\n> > > libcamera: ipu3: Remove the active flag from IPU3Stream\n> > > libcamera: ipu3: Remove IPU3Stream\n> > > libcamera: ipu3: imgu: Remove ImgUOutput\n> > > libcamera: ipu3: imgu: Use unique_ptr for video and subdevices\n> > >\n> > > src/libcamera/pipeline/ipu3/imgu.cpp | 353 +++++++++++++++\n> > > src/libcamera/pipeline/ipu3/imgu.h | 87 ++++\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 548 +++---------------------\n> > > src/libcamera/pipeline/ipu3/meson.build | 1 +\n> > > 4 files changed, 505 insertions(+), 484 deletions(-)\n> > > create mode 100644 src/libcamera/pipeline/ipu3/imgu.cpp\n> > > create mode 100644 src/libcamera/pipeline/ipu3/imgu.h\n> > >\n> > > --\n> > > 2.27.0\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4C029BFFE2\n\tfor ;\n\tMon, 29 Jun 2020 10:29:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 12C8F609DD;\n\tMon, 29 Jun 2020 12:29:54 +0200 (CEST)","from mail-ej1-x642.google.com (mail-ej1-x642.google.com\n\t[IPv6:2a00:1450:4864:20::642])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CC4EA603B4\n\tfor ;\n\tMon, 29 Jun 2020 12:29:52 +0200 (CEST)","by mail-ej1-x642.google.com with SMTP id w16so16039758ejj.5\n\tfor ;\n\tMon, 29 Jun 2020 03:29:52 -0700 (PDT)","from mail-wm1-f50.google.com (mail-wm1-f50.google.com.\n\t[209.85.128.50]) by smtp.gmail.com with ESMTPSA id\n\td20sm2593210edy.9.2020.06.29.03.29.51\n\tfor \n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 29 Jun 2020 03:29:51 -0700 (PDT)","by mail-wm1-f50.google.com with SMTP id g75so14846645wme.5\n\tfor ;\n\tMon, 29 Jun 2020 03:29:51 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"QxE1MF1R\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=m/93g23n6Nn09ooFeFAW/rmLSmhFKn1WTvKj7Aer390=;\n\tb=QxE1MF1RZE2LO+k4uPMHOosqPNdhl+VRlkGxGJKOTQXC166WTo/sh1Sm5y+6AJrXuk\n\tB3CZ92WCqCdKOQlsq0pxA7fWCXcZUCRSS+JvOXijw8wuT3vLQx2jGOo6LTiFMr4nnpIX\n\tezSogFiX/dZvQnzo9lkdjeyUfyo1KiUt2AS1w=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=m/93g23n6Nn09ooFeFAW/rmLSmhFKn1WTvKj7Aer390=;\n\tb=Jmxv5YVYnzdJJXCdSP541y3wy7YRYSp0Zk8mR2RpeAPsJFpmQdLV3FFQF1P8fVadh5\n\tJ+5ZX2E5BdEheNcZ3QmH4uKjSUeAXP63P55MvLQPcAXTDlKzjmy/iKztGJlcbj9ON74V\n\thH248jfPgiex/IVz+iHyTNcMKOs5MSfXEad5dEtLwkQmBcSpj4Q+qSb1mgZLjfK1UOnK\n\tX/BPOCvkSCjwSSyyLTOEaGiNd9GmbsoeKDIUN+fC9S4wknruYp5LLp9suDHMh1WnaEwl\n\tKfkBibGzEKhqg5aCiCGp8qausbBbdre12yCJMkeR0IC7pp3XLisTXBjhN0dA8pZoxL+V\n\tINcQ==","X-Gm-Message-State":"AOAM531nAb+orftsQ876up2twdFCrkTRxfiWzKkaryGQah8z36Q+I9CB\n\tok2RSmZiSn2/nuEehqbeWWdzw8/4FWQzGQ==","X-Google-Smtp-Source":"ABdhPJwYWt/xxNDZKE0j0eqvrkiFQqYX3izbeirWhnN8rh/+gv6wLMstMTAAEhzDdAxj1Pu+YHVvhQ==","X-Received":["by 2002:a17:906:4b59:: with SMTP id\n\tj25mr8963294ejv.301.1593426592169; \n\tMon, 29 Jun 2020 03:29:52 -0700 (PDT)","by 2002:a7b:c2a9:: with SMTP id c9mr16416307wmk.11.1593426591128;\n\tMon, 29 Jun 2020 03:29:51 -0700 (PDT)"],"MIME-Version":"1.0","References":"<20200628001532.2685967-1-niklas.soderlund@ragnatech.se>\n\t\n\t<20200629102155.GS1105424@oden.dyn.berto.se>","In-Reply-To":"<20200629102155.GS1105424@oden.dyn.berto.se>","From":"Tomasz Figa ","Date":"Mon, 29 Jun 2020 12:29:40 +0200","X-Gmail-Original-Message-ID":"","Message-ID":"","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","Subject":"Re: [libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring\n\tof ImgU","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10961,"web_url":"https://patchwork.libcamera.org/comment/10961/","msgid":"<20200629120830.GB10681@pendragon.ideasonboard.com>","date":"2020-06-29T12:08:30","subject":"Re: [libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring\n\tof ImgU","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, Jun 29, 2020 at 12:29:40PM +0200, Tomasz Figa wrote:\n> On Mon, Jun 29, 2020 at 12:22 PM Niklas Söderlund wrote:\n> > On 2020-06-29 11:55:47 +0200, Tomasz Figa wrote:\n> > > On Sun, Jun 28, 2020 at 2:15 AM Niklas Söderlund wrote:\n> > > >\n> > > > Hi,\n> > > >\n> > > > This series do some cleanup of the IPU3 pipeline. It removes dead or\n> > > > almost dead code, replacing it with something more coherent to what is\n> > > > done elsewhere in the pipeline. It breaks out the ImgU to a separate cpp\n> > > > and h file.\n> > > >\n> > > > It reworks a bit how the streams are identified and handled inside the\n> > > > pipeline. When the pipeline was first developed only the output and\n> > > > viewfinder streams where available, now we have a RAW stream in addition\n> > > > to this. Some of the abstraction around streams and which hardware\n> > > > resource they are backed by made sens in the earlier context but not so\n> > > > much once RAW is added to the mix. The biggest difference in this rework\n> > > > is that the ImgUDevice gets explicit functions to configure each of its\n> > > > sink instead of having a generic one which depends which pointer is\n> > > > passed to it. This makes reading code where RAW and ImgU streams are\n> > > > mixed much nicer IMHO.\n> > >\n> > > CIO2 is quite a generic capture interface, which could be handled by\n> > > some platform-independent code. Have we considered splitting the\n> > > pipeline into a generic capture part and platform-specific processing\n> > > part? I think it could be useful on its own, as the capture part of\n> > > all the SoCs could be handled by generic code, but I also suspect that\n> > > the ability to split the pipeline into multiple smaller pipelines and\n> > > then join them together could be very useful when dealing with many\n> > > SoC generations, since vendors quite often replace some parts of the\n> > > pipeline with new hardware, while keeping the others unchanged.\n> > >\n> > > What do you think?\n> >\n> > I think it is a good idea and something that have been lurking in the\n> > back of my head for some time. I'm even entertaining the thought if we\n> > could levy the simple-pipeline handler here somehow. I still have to dig\n> > in the code to see what is possible and what is not, and what is a good\n> > idea and what is not :-)\n\nLikewise, I've been thinking we should handle it in a more generic way.\nI thought there would be a chance to share code with the RPi pipeline\nhandler as it also handles a CSI-2 receiver and a mem-to-mem ISP, but\nthe driver for the CSI-2 receiver is video-node-centric there, while\nCIO2 is MC-centric. I've thus postponed working on this until we get a\nsecond pipeline handler that could share code with IPU3.\n\n> Cool. Please let me know if I could be useful in brainstorming various\n> ideas.\n> \n> > Right now tho I think the most important area to try and help out\n> > pipeline handlers is around the Request object and how internal buffers\n> > (RAW, Statistics and Parameters) are attached to it when for example the\n> > RAW buffer could come from the application or from an internal pool.\n> > This will be a common problem for all pipelines and it would be nice if\n> > the core could help out here.\n> \n> Agreed. It was more of just a long term note. I agree that this series\n> makes for a good overall improvement on its own.\n> \n> > > > Lastly some assumptions that buffers must be allocated at video nodes\n> > > > that are not involved in the capture session are being challenged. This\n> > > > was true a year ago but not any more it seems. Chancing this simplifies\n> > > > the driver enormously and saves on memory that otherwise would be\n> > > > wasted. I have really tried to force the end result to failed by\n> > > > resetting the hardware between each test so that no video node\n> > > > configuration from a previous sessions saves the day.\n> > > >\n> > > > I have for both sensors reset the hardware and then tested the following\n> > > > capture combinations successfully. After the first capture session for\n> > > > each one in the list that was done after a power cycle all the other\n> > > > captures where tried in a semi random order and always succeeded.\n> > > >\n> > > > cam -c 1 -s role=viewfinder -C\n> > > > cam -c 1 -s role=still -C\n> > > > cam -c 1 -s role=still -s role=viewfinder -C\n> > > > cam -c 1 -s role=still -s role=viewfinder -s role=stillraw -C\n> > > > cam -c 1 -s role=stillraw -C\n> > > >\n> > > > Niklas Söderlund (13):\n> > > > libcamera: ipu3: Remove unused name_ filed from IPU3Stream\n> > > > libcamera: ipu3: Import instead of allocate statistic buffers\n> > > > libcamera: ipu3: Always import buffers for ImgU sinks\n> > > > libcamera: ipu3: Remove usage of IPU3CameraData from ImgUDevice\n> > > > libcamera: ipu3: imgu: Move the ImgUDevice class to separate files\n> > > > libcamera: ipu3: imgu: Do not cache index\n> > > > libcamera: ipu3: imgu: Mark things that are internal as private\n> > > > libcamera: ipu3: imgu: Use specific functions to configure each sink\n> > > > libcamera: ipu3: Do not duplicate data in IPU3Stream\n> > > > libcamera: ipu3: Remove the active flag from IPU3Stream\n> > > > libcamera: ipu3: Remove IPU3Stream\n> > > > libcamera: ipu3: imgu: Remove ImgUOutput\n> > > > libcamera: ipu3: imgu: Use unique_ptr for video and subdevices\n> > > >\n> > > > src/libcamera/pipeline/ipu3/imgu.cpp | 353 +++++++++++++++\n> > > > src/libcamera/pipeline/ipu3/imgu.h | 87 ++++\n> > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 548 +++---------------------\n> > > > src/libcamera/pipeline/ipu3/meson.build | 1 +\n> > > > 4 files changed, 505 insertions(+), 484 deletions(-)\n> > > > create mode 100644 src/libcamera/pipeline/ipu3/imgu.cpp\n> > > > create mode 100644 src/libcamera/pipeline/ipu3/imgu.h\n> > > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1CB39BFFE2\n\tfor ;\n\tMon, 29 Jun 2020 12:08:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 98977609C9;\n\tMon, 29 Jun 2020 14:08:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 99793603B4\n\tfor ;\n\tMon, 29 Jun 2020 14:08:33 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0991D299;\n\tMon, 29 Jun 2020 14:08:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vkwOTRW2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593432513;\n\tbh=+r5CHZ4y/hqqJRr+zi3+THtGNKwYHodwMWNYSCTTffs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vkwOTRW2cLP/6pezuVpI6G+fzZvNrnF8OlqM8605OeFitzxI/8j5IX9NKUdjtB2Dt\n\ttr+md7B4miafVa8M2AOtlv5utBdmko1R/utg+45e70o3BhhI+VkizIfWo3dtZW32hj\n\tqVk9RRS8VmfY9BQJZ6gzPA2IQMsjpV5Q3dVQaMpE=","Date":"Mon, 29 Jun 2020 15:08:30 +0300","From":"Laurent Pinchart ","To":"Tomasz Figa ","Message-ID":"<20200629120830.GB10681@pendragon.ideasonboard.com>","References":"<20200628001532.2685967-1-niklas.soderlund@ragnatech.se>\n\t\n\t<20200629102155.GS1105424@oden.dyn.berto.se>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring\n\tof ImgU","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]