From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6D7F3C43334 for ; Wed, 8 Jun 2022 20:43:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=N4sogmLB2K6clOfUuUK5k3Tozmojv07KUFMwaf1ilnE=; b=r4G0C/KqWqMoEx 7KSHWP7eMmi8xZDfjwNN0fYHpOjvOEbbkSnFllCwKbDuVRr4XHV02r4qflc1q0l03JwRVxsOXtE5M sino/RDpzuW/eQswx9S9jTP+dm2B4ABgHDZEO92Xt0cCR0dx+oUPnHE/21ediD3GH+NW9PwRFdQag ajeGBMQNbh3ljSjDrzBCjrspjZsWJf3kd/dGWIvtkr2O0aherspV0HTyo3xgudV4LelvNfWZB2mca jUWH7zhzIA3RB38/BUrKW9s4vV4YSqEPLVmaJ8aDdcP0dMOGmx1yv3rbS4puPtsoIDAdUSJH1sPsW qfoI2UIUXlj8MAfBENcg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nz2WN-00Euly-F6; Wed, 08 Jun 2022 20:43:19 +0000 Received: from mail-wm1-x336.google.com ([2a00:1450:4864:20::336]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nz2WL-00EulO-20 for linux-rockchip@lists.infradead.org; Wed, 08 Jun 2022 20:43:18 +0000 Received: by mail-wm1-x336.google.com with SMTP id h62-20020a1c2141000000b0039aa4d054e2so13995352wmh.1 for ; Wed, 08 Jun 2022 13:43:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=FjhE5PPiSPANLbERBUoaLIjjd9QBlbuIaGPi78yCQPY=; b=X7eL2PC8+p8RlO9EuZXV7hUADUmUGDLPB21/oiOrCQ1TJgUzf0S6Jh4eS5UJmteeHC 9wE88WK5AAOjs10hdvEURZX3KTFe9yR4phHAqfYssU05BHAoiRAKC/Xpx6OF71DqlqQw vfLepy8+F7dxXW3ENVNR5RHySeM9VautEk6JnEPH+zxcPtEPK1JX0jmvVvMqxuSzRLy2 UQ6EudG1mp2COlORH5JdlA9tQxlFk5kpVyw6VQRtMA1Xl+H2MR7bmfyDd/bzV+bxrt3r Y3II48yLr/kCUsC0iQj0/DbjwJXXcd0jjvLFpGBtgj95YOSMDHWr8dmWMDr50tlHOnXK q4QQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=FjhE5PPiSPANLbERBUoaLIjjd9QBlbuIaGPi78yCQPY=; b=WfzDEaCzLhpo7qUTyaLIO0cs6V4zCvtApGERy5XPc2gJ8m+LlhKKdBaPQ4k0mbhK+i L2KKetust7sv/8YtSDTxmgkVhKP74RllipXqsafB7m0DbbqV7lM3sAWOHNI0u34hRADW 7Ssep2Pr4ic5k8nEZMT74D3tttsvdxap+QetF4m1T0FYPoCKQhQDUVez21RBn4xmJHrG UTjmZ5YTlHUWl5EZsV//U1JJwqhnmToNEHqVAaccoQ/BwdkarvzZvuQvnkmnAF8GOFxf WI+b0IdODG21LdQ65c5+9C81QgEwQvjefUuyh//llFmUTnTMJqD1AmnL51RJIXkTebrJ la0g== X-Gm-Message-State: AOAM531Ai4eYeVpqFWBlDMnflMvjX1eQPXWuaAOG1zA5gX/BmVOwYnh+ m5c9geiuUAKf5bdSa5lO3QY= X-Google-Smtp-Source: ABdhPJy1raBmoIyJjdtXT7dQKKRodaNcG8RPUZurnfws3uUCnWCKRMZH3/kVeq8lMAqSXvlIC1A/nQ== X-Received: by 2002:a05:600c:1910:b0:394:8517:496e with SMTP id j16-20020a05600c191000b003948517496emr1070904wmq.24.1654720994460; Wed, 08 Jun 2022 13:43:14 -0700 (PDT) Received: from [192.168.0.14] (cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net. [86.13.91.161]) by smtp.gmail.com with ESMTPSA id h24-20020a05600c145800b0039c54bb28f2sm8887353wmi.36.2022.06.08.13.43.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 08 Jun 2022 13:43:13 -0700 (PDT) Message-ID: Date: Wed, 8 Jun 2022 21:43:12 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH] media: rkisp1: Don't create data links for non-sensor subdevs Content-Language: en-US To: Jacopo Mondi Cc: linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org, dafna@fastmail.com, heiko@sntech.de, foss+kernel@0leil.net References: <20220606225149.2941160-1-djrscally@gmail.com> <20220607164125.oabtqz6kh5bo6k6v@uno.localdomain> <20220608153416.ciwiwg4tbfyetprh@uno.localdomain> From: Daniel Scally In-Reply-To: <20220608153416.ciwiwg4tbfyetprh@uno.localdomain> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220608_134317_167256_A45FDADC X-CRM114-Status: GOOD ( 32.94 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org Hi Jacopo On 08/06/2022 16:34, Jacopo Mondi wrote: > Hi Dan, > > On Wed, Jun 08, 2022 at 03:25:36PM +0100, Daniel Scally wrote: >> Hi Jacopo >> >> On 07/06/2022 17:41, Jacopo Mondi wrote: >>> Hi Dan >>> >>> On Mon, Jun 06, 2022 at 11:51:49PM +0100, Daniel Scally wrote: >>>> With the introduction of ancillary links, not all subdevs linked to >>>> the ISP's v4l2_dev necessarily represent sensors / bridges. Check the >>>> function for the subdevs and skip any that represent lens or flash >>>> controllers before creating data links. >>>> >>>> Signed-off-by: Daniel Scally >>>> --- >>>> >>>> This should fix the issues that have been noticed, but perhaps a new flag like >>>> MEDIA_ENT_FL_HAS_SOURCE or something would be a better way to denote subdevs >>>> that need data links? >>>> >>> I agree this a bit fragile... >>> >>> I noticed ancillary links are only created for subdev notifiers, >>> which have a populated 'sd' and consequentially an entity. Could an >>> helper that walks the links of the notifier's subdev links and checks >>> if the subdev at hand is already linked, help ? Maybe with an optional >>> set of link flags to match on ? > This is actually a mess, as the list of links to be walked is the list > of the sensor's notifier, not the one of the rkisp1. Bad advice, > sorry.. No problem! >> >> Or maybe just check if the subdev's notifier is the same as the rkisp1's >> notifier? Like: >> >> >> if(sd->notifier!= &rkisp1->notifier) > Not all subdevs will have a notifier, won't they ? In facts only > sensor that registers a notifier for their connected lenses/flashes > will have one. sd->notifier is the one the subdev binds to, so the rkisp1 in this case. The notifier that's registered by the sensor would be sd->subdev_notifier...so I think this will work - any subdev that gets to this point should have a notifier, which will be the rkisp1 for the sensors and the sensor for the VCM. > > Anyway, I think the issue here is that we walk the list of all subdevs > registered to the root notifier's v4l2_dev. > > All async subdevices matched in the notifiers chain will end up being > registered to the root notifier's v4l2_dev, hence also lenses and > flashes will appear in this list. > > list_for_each_entry(sd, &rkisp1->v4l2_dev.subdevs, list) { > > } > > Can't we do like the CIO2 does, by walking the list of async subdevs > registered to the root notifier only ? This list should not include > lenses and flashes if I'm not mistaken. > > list_for_each_entry(asd, &rkisp1->notifier.asd_list, asd_list) { > > } > > You can cast the struct v4l2_async_subdev back to the wrapping struct > rkisp1_sensor_async and from there get the sd to create the links on. > Could this work in your opinion ? I'm sorry I can't test it right > away... Yeah I think this should work fine too (it's why the ipu3-cio2 driver doesn't experience the same problem) - I can see how easy it is to switch it over > > > >> continue >> That's a bit less clunky than both other solutions I think >>> >>>> drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >>>> index 3f5cfa7eb937..e90f0216cb06 100644 >>>> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >>>> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c >>>> @@ -134,6 +134,10 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1) >>>> sd == &rkisp1->resizer_devs[RKISP1_SELFPATH].sd) >>>> continue; >>>> >>>> + if (sd->entity.function == MEDIA_ENT_F_LENS || >>>> + sd->entity.function == MEDIA_ENT_F_FLASH) >>>> + continue; >>>> + >>>> ret = media_entity_get_fwnode_pad(&sd->entity, sd->fwnode, >>>> MEDIA_PAD_FL_SOURCE); >>>> if (ret < 0) { >>>> -- >>>> 2.25.1 >>>> _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip