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 CA0E3C83F01 for ; Wed, 30 Aug 2023 08:21:29 +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:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=OenidJ0F4ZripqGkAf7ytw+9FqPPeD3YmHP9jL3suZY=; b=efBELrKwJAvKdW Y+SvaoAOVuHvABbSPLZZg8ouC1FVTgkApzFtbP3fOV36UIX++XwNdRdOcKGPHPXEFQisfMRqUtdlC MvHmPQqUxNJitCDxTONSfwDGq1llSvFNgeyPpHyUhRffvwfW3DaY6S3axi/8/f/88vE1eRMlo3V4r FkaJs6k1R2OZzGrX673hZSIvgfUh95Wijz/cgvt5FbjViSfbWVj/icWRcyjdU1+Ba42Bv9evICQWR CUlqkVmAdUxJxyZSzF3g8DCO7DgMFjT6h+sf4kCRtzyjvDfhrV71CB3ZOziAhFhLxRglw74m0myOT UbhbL/VEwYdZQfuFpJqg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qbGRc-00D4n5-2T; Wed, 30 Aug 2023 08:20:56 +0000 Received: from mgamail.intel.com ([192.55.52.120]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qbGRa-00D4ly-1T for linux-arm-kernel@lists.infradead.org; Wed, 30 Aug 2023 08:20:55 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693383654; x=1724919654; h=date:from:to:subject:message-id:references:mime-version: in-reply-to; bh=qCGMFtQfgwR1F9QTDNMJxmJP2VuKJ61eRyS0eGjsPSY=; b=Jt0rTajMwlMmJH8d4HWDdR/d1zwoT2NqXmJqgpj8sCsAGvkdcgc+TGVM 5HEcRnPd9JtwPlAzNzOTXQch24dAWLl75Swh3eumjqqpkYKGaV0zvLWLp KjRtYWKz2ttVlChh8mA05Ip44aC78ze8Rn895sha6jQZkPM31Ect3WfNG CtD3vUdULV55jQ6dO9YTLgR75hJ8x+gMtXmCaLxTqwcEqtNDH/2Ztk98D 3zPwyRzIt9ghDelUhTho6syKSaVS0rNY0emfh8UVbrexC+YO1u0C9Olq7 AHVeyWtx+dM6718jljVRDVk2+ewLM2DbjRUE7vvYG2fw8bbCBVDTfVmkj A==; X-IronPort-AV: E=McAfee;i="6600,9927,10817"; a="374488118" X-IronPort-AV: E=Sophos;i="6.02,213,1688454000"; d="scan'208";a="374488118" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Aug 2023 01:20:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10817"; a="853643153" X-IronPort-AV: E=Sophos;i="6.02,213,1688454000"; d="scan'208";a="853643153" Received: from turnipsi.fi.intel.com (HELO kekkonen.fi.intel.com) ([10.237.72.44]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Aug 2023 01:20:40 -0700 Received: from kekkonen.localdomain (localhost [127.0.0.1]) by kekkonen.fi.intel.com (Postfix) with SMTP id 4CA1111FAB1; Wed, 30 Aug 2023 11:20:37 +0300 (EEST) Date: Wed, 30 Aug 2023 08:20:37 +0000 From: Sakari Ailus To: Hugues Fruchet , Alexandre Torgue , Mauro Carvalho Chehab , Hans Verkuil , Rob Herring , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, Philippe CORNU Subject: Re: [PATCH v1 3/5] media: stm32-dcmipp: STM32 DCMIPP camera interface driver Message-ID: References: <20220910144010.34272-1-hugues.fruchet@foss.st.com> <20220910144010.34272-4-hugues.fruchet@foss.st.com> <20230825110903.GA30381@gnbcxd0016.gnb.st.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230825110903.GA30381@gnbcxd0016.gnb.st.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230830_012054_532395_9A5F1D8C X-CRM114-Status: GOOD ( 37.78 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Alain, On Fri, Aug 25, 2023 at 01:09:03PM +0200, Alain Volmat wrote: ... > > > +static int dcmipp_pipeline_s_stream(struct dcmipp_bytecap_device *vcap, > > > + int state) > > > +{ > > > + struct media_entity *entity = &vcap->vdev.entity; > > > + struct v4l2_subdev *subdev; > > > + struct media_pad *pad; > > > + int ret; > > > + > > > + /* Start/stop all entities within pipeline */ > > > + while (1) { > > > + pad = &entity->pads[0]; > > > + if (!(pad->flags & MEDIA_PAD_FL_SINK)) > > > + break; > > > + > > > + pad = media_pad_remote_pad_first(pad); > > > + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) > > > + break; > > > + > > > + entity = pad->entity; > > > + subdev = media_entity_to_v4l2_subdev(entity); > > > + > > > + ret = v4l2_subdev_call(subdev, video, s_stream, state); > > > > Does this driver handle multiple sub-devices in the same pipeline? > > > > If not, then you don't need a loop here. > > The idea was to enable one after the other each subdevs part of the > pipeline (aka: sensor -> bridge -> parallel -> byteproc -> bytecap) > however following a discussion with Laurent in Prague I changed that > so that each subdev call each other in cascade, quite like I already did > the following patch for the dcmi driver: Ack! > > commit 525011d84a3f547d0643c10bbfc01d32e26a9963 > Author: Alain Volmat > Date: Fri Jul 21 14:03:15 2023 +0200 > > media: stm32: dcmi: only call s_stream on the source subdev > > Avoid calling s_stream on each subdev until reaching the sensor and > instead call s_stream on the source subdev only (which will in turn > do whatever needed to start the stream). > > Signed-off-by: Alain Volmat > Reviewed-by: Hugues FRUCHET > Signed-off-by: Hans Verkuil ... > > > +#define STOP_TIMEOUT_US 1000 > > > +#define POLL_INTERVAL_US 50 > > > +static int dcmipp_byteproc_s_stream(struct v4l2_subdev *sd, int enable) > > > +{ > > > + struct dcmipp_byteproc_device *byteproc = v4l2_get_subdevdata(sd); > > > + int ret = 0; > > > + > > > + mutex_lock(&byteproc->lock); > > > + if (enable) { > > > + dcmipp_byteproc_configure_framerate(byteproc); > > > + > > > + ret = dcmipp_byteproc_configure_scale_crop(byteproc); > > > + if (ret) > > > + goto err; > > > > This does nothing. > > Not sure to understand your point here. The s_stream callback of this > subdev is used to configure the registers (here the ones controlling > decimation and cropping) of the byteproc subdev. I was referring to the last two lines --- you're jumping to essentially the same location here. > > > > > > + } > > > + > > > +err: > > > + mutex_unlock(&byteproc->lock); > > > + > > > + return ret; > > > +} ... > > > diff --git a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c > > > new file mode 100644 > > > index 000000000000..aa7ae9a5b1a8 > > > --- /dev/null > > > +++ b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c > > > @@ -0,0 +1,682 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Driver for STM32 Digital Camera Memory Interface Pixel Processor > > > + * > > > + * Copyright (C) STMicroelectronics SA 2022 > > > + * Authors: Hugues Fruchet > > > + * Alain Volmat > > > + * for STMicroelectronics. > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > > #include instead of these three. > > Added linux/property.h however kept of_graph.h which is still necessary. > You should switch to fwnode graph API as you're already using fwnodes in the driver --- due to V4L2 fwnode. ... > > > +static int dcmipp_graph_notify_bound(struct v4l2_async_notifier *notifier, > > > + struct v4l2_subdev *subdev, > > > + struct v4l2_async_subdev *asd) > > > +{ > > > + struct dcmipp_device *dcmipp = notifier_to_dcmipp(notifier); > > > + unsigned int ret; > > > + int src_pad; > > > + struct dcmipp_ent_device *sink; > > > + struct device_node *np = dcmipp->dev->of_node; > > > + struct v4l2_fwnode_endpoint ep = { .bus_type = 0 }; > > > > Please set bus_type explicitly (DPHY)? > > My understanding is that I cannot set the bus_type here to have the > framework check for me since we support both V4L2_MBUS_PARALLEL and > V4L2_MBUS_BT656. Ah, I missed this was using a parallel bus. As you have a default in bindings, then you'll need to parse this assuming that bus-type first. I.e. set the bus type to the default and if parsing fails, try the other one. -- Kind regards, Sakari Ailus _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel