[PATCH v2] libcamera: camera_sensor: Cache pixel rate

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Mon, Jan 22, 2024 at 09:13:58PM +0900, Paul Elder wrote:
> Cache the pixel rate at set format time instead of fetching it from the
> v4l2 subdev every time it's needed.

Could you please explain in the commit message why this is needed ? Is
it "just" a small optimization, or does it fix an issue ?

> It needs to be cached at set format time as opposed to initialization
> time as some sensor drives (eg. imx708) change the pixel rate depending

s/drives/drivers/

> on the mode.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v2:
> - Cache the pixel rate at set format time instead of at initialization
>   time
> ---
>  include/libcamera/internal/camera_sensor.h | 2 ++
>  src/libcamera/camera_sensor.cpp            | 8 +++++++-
>  2 files changed, 9 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..127610321 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;
>  }
>  
> @@ -814,6 +817,9 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format, Transform transform)
>  	if (ret)
>  		return ret;
>  
> +	ControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });
> +	pixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
> +
>  	updateControlInfo();
>  	return 0;
>  }
> @@ -1080,7 +1086,7 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const
>  		return -EINVAL;
>  	}
>  
> -	info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();

This avoids fetching the pixel rate from the ControlList, but it's still
fetched from the driver when populating the control list. I'm thus
curious to know the reason for this patch.

> +	info->pixelRate = pixelRate_;
>  
>  	const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);
>  	info->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();

-- 
Regards,

Laurent Pinchart