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. > >> >> 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. Regards, Hans