[libcamera-devel] libcamera: camera_sensor: Cache pixel rate
diff mbox series

Message ID 20240119110708.377399-1-paul.elder@ideasonboard.com
State New
Headers show
Series
  • [libcamera-devel] libcamera: camera_sensor: Cache pixel rate
Related show

Commit Message

Paul Elder Jan. 19, 2024, 11:07 a.m. UTC 
Cache the pixel rate at initialization time instead of fetching it from
the v4l2 subdev every time it's needed.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/internal/camera_sensor.h | 2 ++
 src/libcamera/camera_sensor.cpp            | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Andrei Konovalov Jan. 19, 2024, 11:36 a.m. UTC | #1 
Hi Paul,

It doesn't look like it will always work, as there is at least one sensor/driver
for which the pixel rate depends on the mode:

https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L46

https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L692
https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L715
https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L738
https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L738


Thanks,
Andrei

On 19.01.2024 14:07, Paul Elder via libcamera-devel wrote:
> Cache the pixel rate at initialization time instead of fetching it from
> the v4l2 subdev every time it's needed.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>   include/libcamera/internal/camera_sensor.h | 2 ++
>   src/libcamera/camera_sensor.cpp            | 5 ++++-
>   2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 60a8b106d..da3bf12b3 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -105,6 +105,8 @@ private:
>   	std::string model_;
>   	std::string id_;
>   
> +	uint64_t pixelRate_;
> +
>   	V4L2Subdevice::Formats formats_;
>   	std::vector<unsigned int> mbusCodes_;
>   	std::vector<Size> sizes_;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 0ef78d9c8..33fdb4c8e 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -515,6 +515,9 @@ int CameraSensor::initProperties()
>   		properties_.set(properties::draft::ColorFilterArrangement, cfa);
>   	}
>   
> +	ControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });
> +	pixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
> +
>   	return 0;
>   }
>   
> @@ -1080,7 +1083,7 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const
>   		return -EINVAL;
>   	}
>   
> -	info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
> +	info->pixelRate = pixelRate_;
>   
>   	const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);
>   	info->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();
Paul Elder Jan. 22, 2024, 10:51 a.m. UTC | #2 
Hi Andrei,

On Fri, Jan 19, 2024 at 02:36:27PM +0300, Andrei Konovalov wrote:
> Hi Paul,
> 
> It doesn't look like it will always work, as there is at least one sensor/driver
> for which the pixel rate depends on the mode:

Ah indeed that does seem to be the case.

Thanks for catching it.


Paul

> 
> https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L46
> 
> https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L692
> https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L715
> https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L738
> https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/i2c/imx708.c#L738
> 
> 
> Thanks,
> Andrei
> 
> On 19.01.2024 14:07, Paul Elder via libcamera-devel wrote:
> > Cache the pixel rate at initialization time instead of fetching it from
> > the v4l2 subdev every time it's needed.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >   include/libcamera/internal/camera_sensor.h | 2 ++
> >   src/libcamera/camera_sensor.cpp            | 5 ++++-
> >   2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > index 60a8b106d..da3bf12b3 100644
> > --- a/include/libcamera/internal/camera_sensor.h
> > +++ b/include/libcamera/internal/camera_sensor.h
> > @@ -105,6 +105,8 @@ private:
> >   	std::string model_;
> >   	std::string id_;
> > +	uint64_t pixelRate_;
> > +
> >   	V4L2Subdevice::Formats formats_;
> >   	std::vector<unsigned int> mbusCodes_;
> >   	std::vector<Size> sizes_;
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index 0ef78d9c8..33fdb4c8e 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -515,6 +515,9 @@ int CameraSensor::initProperties()
> >   		properties_.set(properties::draft::ColorFilterArrangement, cfa);
> >   	}
> > +	ControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });
> > +	pixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
> > +
> >   	return 0;
> >   }
> > @@ -1080,7 +1083,7 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const
> >   		return -EINVAL;
> >   	}
> > -	info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
> > +	info->pixelRate = pixelRate_;
> >   	const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);
> >   	info->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();

Patch
diff mbox series 

diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
index 60a8b106d..da3bf12b3 100644
--- a/include/libcamera/internal/camera_sensor.h
+++ b/include/libcamera/internal/camera_sensor.h
@@ -105,6 +105,8 @@  private:
 	std::string model_;
 	std::string id_;
 
+	uint64_t pixelRate_;
+
 	V4L2Subdevice::Formats formats_;
 	std::vector<unsigned int> mbusCodes_;
 	std::vector<Size> sizes_;
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index 0ef78d9c8..33fdb4c8e 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -515,6 +515,9 @@  int CameraSensor::initProperties()
 		properties_.set(properties::draft::ColorFilterArrangement, cfa);
 	}
 
+	ControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });
+	pixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
+
 	return 0;
 }
 
@@ -1080,7 +1083,7 @@  int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const
 		return -EINVAL;
 	}
 
-	info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
+	info->pixelRate = pixelRate_;
 
 	const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);
 	info->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();