| Message ID | 20240220130339.543749-1-sakari.ailus@linux.intel.com |
|---|---|
| Headers | show |
| Series | Use INTEGER64 type for MEI CSI LINK_FREQ control | expand |
Hi Sakari, On 20/02/2024 14:03, Sakari Ailus wrote: > Hi folks, > > An integer menu isn't a useful control type for conveying the frequency > depending on an external device to the receiver. Instead, in the MEI CSI > driver, just obtain the link frequency from the upsteam sub-device and > pass it on to the receiver. > > The v4l2_get_link_freq() is changed to add support for this and it's > documented as well. > > Sakari Ailus (3): > media: v4l: Support obtaining link frequency from INTEGER64 controls > media: Documentation: v4l: LINK_FREQ can be an INTEGER64 control I'm not sure if you can do this. This means userspace has to support two different types as well and query what to use. That's an ABI change. > media: ivsc: csi: Fix link frequency control behaviour And this third patch marks the control as VOLATILE, which is wrong. It is a normal read-only control and you can just update the control's value in the driver whenever it needs changing. I would comment in the patch itself, but for some reason I never got it. I am dropping these three patches from the PR you sent. Regards, Hans > > .../media/v4l/ext-ctrls-image-process.rst | 2 +- > drivers/media/pci/intel/ivsc/mei_csi.c | 30 +++++++++---------- > drivers/media/v4l2-core/v4l2-common.c | 3 ++ > 3 files changed, 18 insertions(+), 17 deletions(-) >
Hi Hans, On Fri, Apr 26, 2024 at 09:12:32AM +0200, Hans Verkuil wrote: > Hi Sakari, > > On 20/02/2024 14:03, Sakari Ailus wrote: > > Hi folks, > > > > An integer menu isn't a useful control type for conveying the frequency > > depending on an external device to the receiver. Instead, in the MEI CSI > > driver, just obtain the link frequency from the upsteam sub-device and > > pass it on to the receiver. > > > > The v4l2_get_link_freq() is changed to add support for this and it's > > documented as well. > > > > Sakari Ailus (3): > > media: v4l: Support obtaining link frequency from INTEGER64 controls > > media: Documentation: v4l: LINK_FREQ can be an INTEGER64 control > > I'm not sure if you can do this. This means userspace has to support two > different types as well and query what to use. That's an ABI change. > > > media: ivsc: csi: Fix link frequency control behaviour > > And this third patch marks the control as VOLATILE, which is wrong. It is a > normal read-only control and you can just update the control's value in > the driver whenever it needs changing. > > I would comment in the patch itself, but for some reason I never got it. > > I am dropping these three patches from the PR you sent. Fair enough. The IVSC takes the frequency from the upstream sub-device so it doesn't have a list of frequencies on its own. One alternative might be to use always the highest frequency, assuming this can be found from the upstream sub-device but I don't know whether this would actually work. Do you know, Wentong? The current state of using a static value is not an option.