Hi Hans, On Fri, Apr 26, 2024 at 10:38:26AM +0200, Hans Verkuil wrote: > On 4/26/24 10:27, Sakari Ailus wrote: > > Hi Hans, > > > > On Fri, Apr 26, 2024 at 10:12:37AM +0200, Hans Verkuil wrote: > >> On 4/26/24 09:39, Sakari Ailus wrote: > >>> Hi Hans, > >>> > >>> On Fri, Apr 26, 2024 at 09:22:40AM +0200, Hans Verkuil wrote: > >>>> On 20/02/2024 14:03, Sakari Ailus wrote: > >>>>> When a device passes through the video data while still having its own > >>>>> receiver and transmitter, it can use the same frequency as the upstream > >>>>> link does. The Intel MEI CSI device is an example of this. An integer menu > >>>>> control isn't useful in conveying the actual frequency to the receiver in > >>>>> this case. > >>>>> > >>>>> Document that the LINK_FREQ control may also be a 64-bit integer control. > >>>>> > >>>>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > >>>>> --- > >>>>> .../userspace-api/media/v4l/ext-ctrls-image-process.rst | 2 +- > >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst > >>>>> index b1c2ab2854af..7a3ccb100e1d 100644 > >>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst > >>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst > >>>>> @@ -22,7 +22,7 @@ Image Process Control IDs > >>>>> > >>>>> .. _v4l2-cid-link-freq: > >>>>> > >>>>> -``V4L2_CID_LINK_FREQ (integer menu)`` > >>>>> +``V4L2_CID_LINK_FREQ (integer menu or 64-bit integer)`` > >>>>> The frequency of the data bus (e.g. parallel or CSI-2). > >>>> > >>>> I really think a new control should be created for this. > >>>> > >>>> As I understand it, V4L2_CID_LINK_FREQ gives a set of supported frequencies, > >>>> and the application has to pick one (I think?). Is it supposed to be a > >>>> read-only control? Some driver seem to set the READ_ONLY flag, and some do not. > >>>> The documentation isn't helpful in that respect. > >>> > >>> This is read-only effectively in current IVSC implementation. > >>> > >>>> > >>>> In the case of the Intel MEI CSI and similar devices a new control would be > >>>> better, I think. Do I understand it correctly that for these devices it would > >>>> always be a read-only control? I.e. it just reports the frequency, but applications > >>>> cannot change it? > >>> > >>> How would you call the new control? > >>> > >>> V4L2_CID_LINK_FREQ_READ_ONLY? > >>> > >>> Originally the reason for changing LINK_FREQ for sensors was be part of > >>> changing sensor's configuration to achieve a given frame interval. > >> > >> Will this new variant always be read-only? > > > > At least for this purpose I think so. Apart from the sensor configuration, > > I'm not aware of another use case for the user to change it. > > > >> > >> How about V4L2_CID_CUR_LINK_FREQ? > >> > >> I.e., it returns the current link frequency. That way it can also be > >> used by drivers that implement V4L2_CID_LINK_FREQ. > > > > It could, but the drivers already report this using V4L2_CID_LINK_FREQ. > > It'd be extra driver code for little if any gain. > > It's mainly for if we ever want to have consistent support for this > control for all drivers to make life easier for applications. > > In other words, supporting both controls (if we'd ever want to) wouldn't > cause problems API-wise. Apart from the sensor frame interval control, the users generally don't care. It's for the CSI-2 receiver drivers which use a framework function to obtain the value (it was amended in one of the three patches). > > > > >> > >> Better ideas are welcome :-) > > > > V4L2_CID_LINK_FREQ_VALUE? V4L2_CID_LINK_FREQ2?? :-) > > > > Hmm, I prefer CUR_LINK_FREQ, since that implies that it is a read-only > control. I'd be fine with that. -- Regards, Sakari Ailus
