Hi Laurent,
On Wed, 2019-09-25 at 16:41 +0300, Laurent Pinchart wrote:
> Hi Philipp,
>
> (CC'ing Sakari, Jacopo, Kieran and Niklas)
>
> Thank you for the patch.
>
> On Tue, Sep 24, 2019 at 01:49:53PM +0200, Philipp Zabel wrote:
> > Add a subdevice video operation that allows to query the number
> > of data lanes a MIPI CSI-2 TX is actively transmitting on.
> >
> > Suggested-by: Hans Verkuil <hverk...@xs4all.nl>
> > Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
> > ---
> > New in v4.
> > ---
> > include/media/v4l2-subdev.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index 71f1f2f0da53..bb71eedc38f6 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -411,6 +411,8 @@ struct v4l2_mbus_frame_desc {
> > * @s_rx_buffer: set a host allocated memory buffer for the subdev. The
> > subdev
> > * can adjust @size to a lower value and must not write more data to the
> > * buffer starting at @data than the original value of @size.
> > + *
> > + * @g_csi_active_lanes: Get number of currently active MIPI CSI-2 data
> > lanes.
> > */
> > struct v4l2_subdev_video_ops {
> > int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
> > config);
> > @@ -441,6 +443,7 @@ struct v4l2_subdev_video_ops {
> > const struct v4l2_mbus_config *cfg);
> > int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> > unsigned int *size);
> > + int (*g_csi_active_lanes)(struct v4l2_subdev *sd, u32 *lanes);
>
> This shouldn't be a video operation but a pad operation, as a subdev
> could have multiple CSI-2 pads.
Right this should be pad specific.
> Furthermore, you need to define the semantics of this operation more
> precisely. When can it be called,
The downstream subdevice connected to this pad is expected to call this
in its s_stream(enable=1) op, right before enabling the MIPI CSI-2 RX,
and then calling s_stream(enable=1) on the same upstream subdevice.
The returned value is a decision by the upstream subdevice driver based
on external factors such as available link-frequencies and mbus frame
format, so it can change whenever those are changed, but not by itself.
> when is the information valid ?
It is valid until the next time the pad's mbus frame format or link
frequency are changed. Since the caller
> Can the subdev change the number of lanes it supports at runtime ?
At least for MIPI CSI-2, no. Are there any buses that can dynamically
change bus width while active?
> If so, how are race conditions avoided ? All this needs to be documented.
I think no, so the only possible race conditions would be with
reconfiguration, which should already be avoided by requiring this to be
called from s_stream(),as the media pipeline is already started and
all configuration locked in place at this point.
> Finally, the number of lanes is far from being the only information
> about a physical bus that could be interesting for a remote subdev. I
> would much prefer a more generic operation to retrieve bus
> information/configuration, with a data structure that we will be able to
> extend later.
This is specifically about configuration chosen by the transmitter, not
physical bus properties, which can be specified in the device tree. The
chosen link frequency (if more than one is specified in DT) could be one
of those values.
regards
Philipp
