| Message ID | 20240426145538.654212-1-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Use INTEGER64 type for MEI CSI LINK_FREQ control | expand |
Hi Sakari, Thank you for the patches. On Fri, Apr 26, 2024 at 05:55:35PM +0300, 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. Using a control for this seems to be a bit of a hack :-S Wouldn't it fit more nicely in the .get_mbus_config() operation ? That would avoid exposing this to userspace, and would also have the nice advantage that the value could be queried per pad. > since v1: > > - Add a new 64-bit integer control V4L2_CID_CUR_LINK_FREQ instead of > re-using V4L2_CID_LINK_FREQ. > > Sakari Ailus (3): > media: Documentation: v4l: Add CUR_LINK_FREQ control > media: v4l: Support obtaining link frequency from CUR_LINK_FREQ > control > media: ivsc: csi: Fix link frequency control behaviour > > .../media/v4l/ext-ctrls-image-process.rst | 4 ++++ > drivers/media/pci/intel/ivsc/mei_csi.c | 24 ++++++------------- > drivers/media/v4l2-core/v4l2-common.c | 6 ++++- > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 5 ++++ > include/uapi/linux/v4l2-controls.h | 1 + > 5 files changed, 22 insertions(+), 18 deletions(-) > > > base-commit: 6612d1ae37b52d37d1d515d7c5110b9f3afe94d2
Hi Laurent, On Mon, Apr 29, 2024 at 02:20:28AM +0300, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patches. > > On Fri, Apr 26, 2024 at 05:55:35PM +0300, 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. > > Using a control for this seems to be a bit of a hack :-S Wouldn't it fit > more nicely in the .get_mbus_config() operation ? That would avoid > exposing this to userspace, and would also have the nice advantage that > the value could be queried per pad. Works for me. I also think it's better than using a new control for this actually. > > > since v1: > > > > - Add a new 64-bit integer control V4L2_CID_CUR_LINK_FREQ instead of > > re-using V4L2_CID_LINK_FREQ. > > > > Sakari Ailus (3): > > media: Documentation: v4l: Add CUR_LINK_FREQ control > > media: v4l: Support obtaining link frequency from CUR_LINK_FREQ > > control > > media: ivsc: csi: Fix link frequency control behaviour > > > > .../media/v4l/ext-ctrls-image-process.rst | 4 ++++ > > drivers/media/pci/intel/ivsc/mei_csi.c | 24 ++++++------------- > > drivers/media/v4l2-core/v4l2-common.c | 6 ++++- > > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 5 ++++ > > include/uapi/linux/v4l2-controls.h | 1 + > > 5 files changed, 22 insertions(+), 18 deletions(-) > > > > > > base-commit: 6612d1ae37b52d37d1d515d7c5110b9f3afe94d2 >
On 29/04/2024 09:28, Sakari Ailus wrote: > Hi Laurent, > > On Mon, Apr 29, 2024 at 02:20:28AM +0300, Laurent Pinchart wrote: >> Hi Sakari, >> >> Thank you for the patches. >> >> On Fri, Apr 26, 2024 at 05:55:35PM +0300, 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. >> >> Using a control for this seems to be a bit of a hack :-S Wouldn't it fit >> more nicely in the .get_mbus_config() operation ? That would avoid >> exposing this to userspace, and would also have the nice advantage that >> the value could be queried per pad. > > Works for me. I also think it's better than using a new control for this > actually. Same for me. As you gathered, I was not very enthusiastic about using a control for this either. Regards, Hans > >> >>> since v1: >>> >>> - Add a new 64-bit integer control V4L2_CID_CUR_LINK_FREQ instead of >>> re-using V4L2_CID_LINK_FREQ. >>> >>> Sakari Ailus (3): >>> media: Documentation: v4l: Add CUR_LINK_FREQ control >>> media: v4l: Support obtaining link frequency from CUR_LINK_FREQ >>> control >>> media: ivsc: csi: Fix link frequency control behaviour >>> >>> .../media/v4l/ext-ctrls-image-process.rst | 4 ++++ >>> drivers/media/pci/intel/ivsc/mei_csi.c | 24 ++++++------------- >>> drivers/media/v4l2-core/v4l2-common.c | 6 ++++- >>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 5 ++++ >>> include/uapi/linux/v4l2-controls.h | 1 + >>> 5 files changed, 22 insertions(+), 18 deletions(-) >>> >>> >>> base-commit: 6612d1ae37b52d37d1d515d7c5110b9f3afe94d2 >> >
Hi Hans, On Mon, Apr 29, 2024 at 09:30:37AM +0200, Hans Verkuil wrote: > On 29/04/2024 09:28, Sakari Ailus wrote: > > Hi Laurent, > > > > On Mon, Apr 29, 2024 at 02:20:28AM +0300, Laurent Pinchart wrote: > >> Hi Sakari, > >> > >> Thank you for the patches. > >> > >> On Fri, Apr 26, 2024 at 05:55:35PM +0300, 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. > >> > >> Using a control for this seems to be a bit of a hack :-S Wouldn't it fit > >> more nicely in the .get_mbus_config() operation ? That would avoid > >> exposing this to userspace, and would also have the nice advantage that > >> the value could be queried per pad. > > > > Works for me. I also think it's better than using a new control for this > > actually. > > Same for me. As you gathered, I was not very enthusiastic about using a > control for this either. Ack. I guess these are fine to be merged for 6.11 but still backporting to 6.10 should be done. Nothing breaks now as there's a single frequency used right now and that's 400 MHz.
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. since v1: - Add a new 64-bit integer control V4L2_CID_CUR_LINK_FREQ instead of re-using V4L2_CID_LINK_FREQ. Sakari Ailus (3): media: Documentation: v4l: Add CUR_LINK_FREQ control media: v4l: Support obtaining link frequency from CUR_LINK_FREQ control media: ivsc: csi: Fix link frequency control behaviour .../media/v4l/ext-ctrls-image-process.rst | 4 ++++ drivers/media/pci/intel/ivsc/mei_csi.c | 24 ++++++------------- drivers/media/v4l2-core/v4l2-common.c | 6 ++++- drivers/media/v4l2-core/v4l2-ctrls-defs.c | 5 ++++ include/uapi/linux/v4l2-controls.h | 1 + 5 files changed, 22 insertions(+), 18 deletions(-) base-commit: 6612d1ae37b52d37d1d515d7c5110b9f3afe94d2