Received: by 2002:a25:ca44:0:0:0:0:0 with SMTP id a65csp1116526ybg; Wed, 29 Jul 2020 06:19:47 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxeSAuitmUnOcntp1oZz6mdbN7425lorwAQxp1gadSDTqRtD5ZKTsb/Qf0vAY9DAMiLY4su X-Received: by 2002:a17:906:c259:: with SMTP id bl25mr29965948ejb.303.1596028787482; Wed, 29 Jul 2020 06:19:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1596028787; cv=none; d=google.com; s=arc-20160816; b=mwTsRR583zvu45PV01VhCIvoFx/udqz60vdn/oKVzwAsWSSMwwGb+xZitEQgCFwb8F Ahh2ljoi+5Z8Gsy3gV5z0xEbDHferp3tlyflKX5jQXF1hT/eo6hh8wnRg+1knL0cCGzj 28gBe3rx2bW5RZZ5fObCgAWYjuCtOeJZlJQ3CJUyqNjfwZqdfbDuFWiXF9QncnbkOExe Xz0EBpn6gLH5SeImG8niCDyJl3OenjvNgXfJxHAsI6uAVAglfFqFxuB+v/OhipKWs5Yk HfkYLjIvp8rgcVeMd4i3ZloJI9WD3aCw4EuiXgaOwvffS05JlR+smpShsxlgY/xlNKc/ 3daw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=U9KASq2nKNndSi9IUcOP8Cs3c02jt3uDwiSsAqYTNuQ=; b=aGmPJAlyXBQ0UnuLGwpM/Cg3PhhdtlDVCVBOTwh9ELn/mNAV4SV44ObxNSie0R9BfI Fnrk7EziiHLh5/Nk0hyPHV3wOZpYhHkr86wRSeNTSeNN30NcWQK+mFQ4rhUd85ID2WOr hV8Ilbfx+vrzAec1fM85eJwFjhJuDDb/7k85ZFvt3hhSEzhvQ93uyehrbCkKAharADKn GOI256xNACBTIh/T71/VHehOugO6M/EPwY9iqwGArg0PneSLrWAZvkUH5M24ByRm1euA ba/Rt7KueqkD46MCfIdYGMQnUzZkbgrfT2A1kV0JSXx2XPE7ekPYndlxaqLKFTSDYsZ6 7XZw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=collabora.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id a3si926167ejc.288.2020.07.29.06.19.24; Wed, 29 Jul 2020 06:19:47 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=collabora.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727051AbgG2NQy (ORCPT + 99 others); Wed, 29 Jul 2020 09:16:54 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:39254 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726353AbgG2NQy (ORCPT ); Wed, 29 Jul 2020 09:16:54 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: dafna) with ESMTPSA id DAB6B297C0B Subject: Re: [PATCH v2 0/3] media: vimc: Allow multiple capture devices to use the same sensor To: kieran.bingham@ideasonboard.com, Kaaira Gupta , Helen Koike Cc: =?UTF-8?Q?Niklas_S=c3=b6derlund?= , Shuah Khan , Mauro Carvalho Chehab , linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Hans Verkuil , Laurent Pinchart , Ezequiel Garcia References: <20200724120213.17119-1-kgupta@es.iitr.ac.in> <20200724121521.GA2705690@oden.dyn.berto.se> <20200724122104.GA18482@kaaira-HP-Pavilion-Notebook> <2a6cb067-283d-ca65-2698-1fae66a17d02@collabora.com> <20200728113959.GA6350@kaaira-HP-Pavilion-Notebook> <3a9ac970-77b8-1bc5-536a-5b4f2bd60745@collabora.com> From: Dafna Hirschfeld Message-ID: Date: Wed, 29 Jul 2020 15:16:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29.07.20 15:05, Kieran Bingham wrote: > Hi Dafna, > > On 28/07/2020 15:00, Dafna Hirschfeld wrote: >> >> >> On 28.07.20 14:07, Dafna Hirschfeld wrote: >>> Hi >>> >>> On 28.07.20 13:39, Kaaira Gupta wrote: >>>> On Mon, Jul 27, 2020 at 02:54:30PM -0300, Helen Koike wrote: >>>>> Hi, >>>>> >>>>> On 7/27/20 11:31 AM, Kieran Bingham wrote: >>>>>> Hi all, >>>>>> >>>>>> +Dafna for the thread discussion, as she's missed from the to/cc list. >>>>>> >>>>>> >>>>>> On 24/07/2020 13:21, Kaaira Gupta wrote: >>>>>>> On Fri, Jul 24, 2020 at 02:15:21PM +0200, Niklas Söderlund wrote: >>>>>>> Hi, >>>>>>> >>>>>>>> Hi Kaaira, >>>>>>>> >>>>>>>> Thanks for your work. >>>>>>> >>>>>>> Thanks for yours :D >>>>>>> >>>>>>>> >>>>>>>> On 2020-07-24 17:32:10 +0530, Kaaira Gupta wrote: >>>>>>>>> This is version 2 of the patch series posted by Niklas for allowing >>>>>>>>> multiple streams in VIMC. >>>>>>>>> The original series can be found here: >>>>>>>>> https://patchwork.kernel.org/cover/10948831/ >>>>>>>>> >>>>>>>>> This series adds support for two (or more) capture devices to be >>>>>>>>> connected to the same sensors and run simultaneously. Each >>>>>>>>> capture device >>>>>>>>> can be started and stopped independent of each other. >>>>>>>>> >>>>>>>>> Patch 1/3 and 2/3 deals with solving the issues that arises once >>>>>>>>> two >>>>>>>>> capture devices can be part of the same pipeline. While 3/3 >>>>>>>>> allows for >>>>>>>>> two capture devices to be part of the same pipeline and thus >>>>>>>>> allows for >>>>>>>>> simultaneously use. >>> >>> I wonder if these two patches are enough, since each vimc entity also >>> have >>> a 'process_frame' callback, but only one allocated frame. That means >>> that the 'process_frame' can be called concurrently by two different >>> streams >>> on the same frame and cause corruption. >>> >> >> I think we should somehow change the vimc-stream.c code so that we have >> only >> one stream process per pipe. So if one capture is already streaming, >> then the new >> capture that wants to stream uses the same thread so we don't have two >> threads >> both calling 'process_frame'. > > > Yes, I think it looks and sounds like there are two threads running when > there are two streams. > > so in effect, although they 'share a pipe', aren't they in effect just > sending two separate buffers through their stream-path? > > If that's the case, then I don't think there's any frame corruption, > because they would both have grabbed their own frame separately. But each entity allocates just one buffer. So the same buffer is used for both stream. What for example can happen is that the debayer of one stream can read the sensor's buffer while the sensor itself writes to the buffer for the other stream. Thanks, Dafna > > > I don't think that's a good example of the hardware though, as that > doesn't reflect what 'should' happen where the TPG runs once to generate > a frame at the sensor, which is then read by both the debayer entity and > the RAW capture device when there are two streams... > > > So I suspect trying to move to a single thread is desirable, but that > might be a fair bit of work also. > > -- > Kieran > > > >> The second capture that wants to stream should iterate the topology >> downwards until >> reaching an entity that already belong to the stream path of the other >> streaming capture >> and tell the streamer it wants to read the frames this entity >> produces. >> >> Thanks, >> Dafna >> >>> Thanks, >>> Dafna >>> >>>>>>>> >>>>>>>> I'm just curious if you are aware of this series? It would >>>>>>>> replace the >>>>>>>> need for 1/3 and 2/3 of this series right? >>>>>>> >>>>>>> v3 of this series replaces the need for 1/3, but not the current >>>>>>> version >>>>>>> (ie v4). v4 of patch 2/5 removes the stream_counter that is needed to >>>>>>> keep count of the calls to s_stream. Hence 1/3 becomes relevant >>>>>>> again. >>>>>> >>>>>> So the question really is, how do we best make use of the two current >>>>>> series, to achieve our goal of supporting multiple streams. >>>>>> >>>>>> Having not parsed Dafna's series yet, do we need to combine >>>>>> elements of >>>>>> both ? Or should we work towards starting with this series and get >>>>>> dafna's patches built on top ? >>>>>> >>>>>> Or should patch 1/3 and 3/3 of this series be on top of Dafna's v4 ? >>>>>> >>>>>> (It might be noteworthy to say that Kaaira has reported successful >>>>>> multiple stream operation from /this/ series and her development >>>>>> branch >>>>>> on libcamera). >>>>> >>>>> Dafna's patch seems still under discussion, but I don't want to >>>>> block progress in Vimc either. >>>>> >>>>> So I was wondering if we can move forward with Vimc support for >>>>> multistreaming, >>>>> without considering Dafna's patchset, and we can do the clean up >>>>> later once we solve that. >>>>> >>>>> What do you think? >>>> >>>> I agree with supporting multiple streams with VIMC with this patchset, >>>> and then we can refactor the counters for s_stream in VIMC later (over >>>> this series) if dafna includes them in subsequent version of her >>>> patchset. >>>> >>> >>> I also think that adding support in the code will take much longer and >>> should not >>> stop us from supporting vimc independently. >>> >>> Thanks, >>> Dafna >>> >>>>> >>>>> Regards, >>>>> Helen >>>>> >>>>>> >>>>>> >>>>>>>> 1. >>>>>>>> https://lore.kernel.org/linux-media/20200522075522.6190-1-dafna.hirschfeld@collabora.com/ >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Changes since v1: >>>>>>>>>     - All three patches rebased on latest media-tree. >>>>>>>>>     Patch 3: >>>>>>>>>     - Search for an entity with a non-NULL pipe instead of >>>>>>>>> searching >>>>>>>>>       for sensor. This terminates the search at output itself. >>>>>>>>> >>>>>>>>> Kaaira Gupta (3): >>>>>>>>>    media: vimc: Add usage count to subdevices >>>>>>>>>    media: vimc: Serialize vimc_streamer_s_stream() >>>>>>>>>    media: vimc: Join pipeline if one already exists >>>>>>>>> >>>>>>>>>   .../media/test-drivers/vimc/vimc-capture.c    | 35 >>>>>>>>> ++++++++++++++++++- >>>>>>>>>   .../media/test-drivers/vimc/vimc-debayer.c    |  8 +++++ >>>>>>>>>   drivers/media/test-drivers/vimc/vimc-scaler.c |  8 +++++ >>>>>>>>>   drivers/media/test-drivers/vimc/vimc-sensor.c |  9 ++++- >>>>>>>>>   .../media/test-drivers/vimc/vimc-streamer.c   | 23 +++++++----- >>>>>>>>>   5 files changed, 73 insertions(+), 10 deletions(-) >>>>>>>>> >>>>>>>>> -- >>>>>>>>> 2.17.1 >>>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Regards, >>>>>>>> Niklas Söderlund >>>>>> >