From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Pratyush Yadav <p.yadav@ti.com>
Cc: "Mauro Carvalho Chehab" <mchehab@kernel.org>,
"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
"Nikhil Devshatwar" <nikhil.nd@ti.com>,
"Tomi Valkeinen" <tomi.valkeinen@ideasonboard.com>,
"Vignesh Raghavendra" <vigneshr@ti.com>,
"Benoit Parrot" <bparrot@ti.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Rob Herring" <robh+dt@kernel.org>,
"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-media@vger.kernel.org
Subject: Re: [PATCH v4 08/11] media: ti: Add CSI2RX support for J721E
Date: Thu, 7 Oct 2021 00:08:33 +0300 [thread overview]
Message-ID: <YV4QUU1NxcgEN81V@paasikivi.fi.intel.com> (raw)
In-Reply-To: <20211006210141.3mi2popzfsalskm3@ti.com>
On Thu, Oct 07, 2021 at 02:31:43AM +0530, Pratyush Yadav wrote:
> On 06/10/21 11:28PM, Sakari Ailus wrote:
> > Hi Pratyush,
> >
> > On Wed, Sep 15, 2021 at 05:32:37PM +0530, Pratyush Yadav wrote:
> > ...
> > > +/*
> > > + * Find the input format. This is done by finding the first device in the
> > > + * pipeline which can tell us the current format. This could be the sensor, or
> > > + * this could be another device in the middle which is capable of format
> > > + * conversions.
> > > + */
> > > +static int ti_csi2rx_validate_pipeline(struct ti_csi2rx_dev *csi)
> > > +{
> > > + struct media_pipeline *pipe = &csi->pipe;
> > > + struct media_entity *entity;
> > > + struct v4l2_subdev *sd;
> > > + struct v4l2_subdev_format fmt;
> > > + struct v4l2_pix_format *pix = &csi->v_fmt.fmt.pix;
> > > + struct media_device *mdev = &csi->mdev;
> > > + const struct ti_csi2rx_fmt *ti_fmt;
> > > + int ret;
> > > +
> > > + mutex_lock(&mdev->graph_mutex);
> > > + ret = media_graph_walk_init(&pipe->graph, mdev);
> > > + if (ret) {
> > > + mutex_unlock(&mdev->graph_mutex);
> > > + return ret;
> > > + }
> > > +
> > > + media_graph_walk_start(&pipe->graph, &csi->vdev.entity);
> > > +
> > > + while ((entity = media_graph_walk_next(&pipe->graph))) {
> > > + if (!is_media_entity_v4l2_subdev(entity))
> > > + continue;
> >
> > You shouldn't rely on media_graph_walk_next() to return entities in a
> > particular order.
>
> Ah, right. Need to drop this.
>
> >
> > I'd suggest approach taken in isp_video_check_external_subdevs() (in
> > drivers/media/platform/omap3isp/ispvideo.c).
> >
> > > +
> > > + sd = media_entity_to_v4l2_subdev(entity);
> > > +
> > > + fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > > + fmt.pad = media_get_pad_index(entity, 0, PAD_SIGNAL_DEFAULT);
> > > +
> > > + ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> > > + if (ret && ret != -ENOIOCTLCMD) {
> > > + media_graph_walk_cleanup(&pipe->graph);
> > > + mutex_unlock(&mdev->graph_mutex);
> > > + return ret;
> > > + }
> > > +
> > > + if (!ret)
> > > + break;
> > > + }
> > > +
> > > + media_graph_walk_cleanup(&pipe->graph);
> > > + mutex_unlock(&mdev->graph_mutex);
> > > +
> > > + /* Could not find input format. */
> > > + if (!entity)
> > > + return -EPIPE;
> > > +
> > > + if (fmt.format.width != pix->width)
> > > + return -EPIPE;
> > > + if (fmt.format.height != pix->height)
> > > + return -EPIPE;
> >
> > Pipeline validation should take place during media_pipeline_start(). Why
> > are you doing it here?
>
> How would be do that? Via the link_validate callback?
Yes, please. See other drivers for examples --- such as omap3isp.
--
Sakari Ailus
next prev parent reply other threads:[~2021-10-06 21:09 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-15 12:02 [PATCH v4 00/11] CSI2RX support on J721E Pratyush Yadav
2021-09-15 12:02 ` [PATCH v4 01/11] media: cadence: csi2rx: Unregister v4l2 async notifier Pratyush Yadav
2021-10-06 23:31 ` Laurent Pinchart
2021-10-07 12:19 ` Pratyush Yadav
2021-09-15 12:02 ` [PATCH v4 02/11] media: cadence: csi2rx: Add external DPHY support Pratyush Yadav
2021-10-06 20:19 ` Sakari Ailus
2021-10-06 21:01 ` Pratyush Yadav
2021-10-06 23:40 ` Laurent Pinchart
2021-09-15 12:02 ` [PATCH v4 03/11] media: cadence: csi2rx: Soft reset the streams before starting capture Pratyush Yadav
2021-10-06 23:41 ` Laurent Pinchart
2021-09-15 12:02 ` [PATCH v4 04/11] media: cadence: csi2rx: Set the STOP bit when stopping a stream Pratyush Yadav
2021-10-06 23:42 ` Laurent Pinchart
2021-09-15 12:02 ` [PATCH v4 05/11] media: cadence: csi2rx: Fix stream data configuration Pratyush Yadav
2021-10-06 23:44 ` Laurent Pinchart
2021-09-15 12:02 ` [PATCH v4 06/11] media: cadence: csi2rx: Populate subdev devnode Pratyush Yadav
2021-10-06 23:45 ` Laurent Pinchart
2021-09-15 12:02 ` [PATCH v4 07/11] media: Re-structure TI platform drivers Pratyush Yadav
2021-10-06 23:46 ` Laurent Pinchart
2021-09-15 12:02 ` [PATCH v4 08/11] media: ti: Add CSI2RX support for J721E Pratyush Yadav
2021-10-06 20:28 ` Sakari Ailus
2021-10-06 21:01 ` Pratyush Yadav
2021-10-06 21:08 ` Sakari Ailus [this message]
2021-09-15 12:02 ` [PATCH v4 09/11] media: dt-bindings: Make sure items in data-lanes are unique Pratyush Yadav
2021-09-21 21:31 ` Rob Herring
2021-10-06 23:47 ` Laurent Pinchart
2021-09-15 12:02 ` [PATCH v4 10/11] media: dt-bindings: Add DT bindings for TI J721E CSI2RX driver Pratyush Yadav
2021-09-21 21:32 ` Rob Herring
2021-10-06 23:49 ` Laurent Pinchart
2021-09-15 12:02 ` [PATCH v4 11/11] media: dt-bindings: Convert Cadence CSI2RX binding to YAML Pratyush Yadav
2021-09-21 21:35 ` Rob Herring
2021-10-06 23:54 ` Laurent Pinchart
2021-10-07 12:16 ` Pratyush Yadav
2021-10-06 8:21 ` [PATCH v4 00/11] CSI2RX support on J721E Pratyush Yadav
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YV4QUU1NxcgEN81V@paasikivi.fi.intel.com \
--to=sakari.ailus@linux.intel.com \
--cc=bparrot@ti.com \
--cc=devicetree@vger.kernel.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=mripard@kernel.org \
--cc=nikhil.nd@ti.com \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=p.yadav@ti.com \
--cc=robh+dt@kernel.org \
--cc=tomi.valkeinen@ideasonboard.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).