Hello,
sorry for having missed this
On Wed, Sep 25, 2019 at 04:41:13PM +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 <hverkuil@xxxxxxxxx>
> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > ---
> > 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.
>
> Furthermore, you need to define the semantics of this operation more
> precisely. When can it be called, when is the information valid ? Can
> the subdev change the number of lanes it supports at runtime ? If so,
> how are race conditions avoided ? All this needs to be documented.
>
> 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.
>
For the record we tried to get those information from the frame
descriptor (the number of used data lanes and the clock
continous/non-continous information to be precise)
This is the RFC series I sent
https://patchwork.kernel.org/project/linux-media/list/?series=92501
Which depends on Sakari's patches:
https://patchwork.kernel.org/patch/10875871/
https://patchwork.kernel.org/patch/10875873/
The latest two were part of a much bigger series that tried to add
support for multiplexed streams. Honestly, it now seems to be that
part could be breakout without involving streams, and re-use that
portion to negotiate the csi-2 bus configuration. I might be wrong
though, and the two parts could not be separate easily (from a very
quick look, after months, it does not seem so).
In the past I spoke against this solution as I would have preferred
leaving frame_desc alone and introduce a bus configuration operation.
I tried a few times and I ended up implementing g_mbus_format but on
pads and not video. Right now with Sakari's and Laurent's ack I would
say re-using frame_desc might actually work and get use a feature
which is needed by many (cc also Dave, as he had a similar issue with
TC358743 iirc)
Thanks
j
> > };
> >
> > /**
>
> --
> Regards,
>
> Laurent Pinchart
Attachment:
signature.asc
Description: PGP signature
