[v2,2/2] pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline

21261 diff mbox series

It is possible that the maximum sensor size (returned by
CameraSensor::resolution()) is not supported by the pipeline. In such
cases, a filter function is required to filter out resolutions of the
camera sensor, which cannot be supported by the pipeline.

Introduce the filter function filterSensorResolution() in RkISP1Path
class and use it for validate() and generateConfiguration(). The
maximum sensor resolution is picked from that filtered set of
resolutions instead of CameraSensor::resolution().

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 51 ++++++++++++++++++-
 src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  8 +++
 2 files changed, 57 insertions(+), 2 deletions(-)

Quoting Umang Jain (2024-09-13 11:37:59)
> It is possible that the maximum sensor size (returned by
> CameraSensor::resolution()) is not supported by the pipeline. In such
> cases, a filter function is required to filter out resolutions of the
> camera sensor, which cannot be supported by the pipeline.
> 
> Introduce the filter function filterSensorResolution() in RkISP1Path
> class and use it for validate() and generateConfiguration(). The
> maximum sensor resolution is picked from that filtered set of
> resolutions instead of CameraSensor::resolution().
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Aha, and at last I've found a way to validate/test this distinctly.

Running a PPP with an IMX258 which fails to configure the pipe on the
largest frame size, now excludes that frame size with this patch series:

Without this patch:

mobian@mobian:~/libcamera$ cam -c2 -I -srole=raw

Using camera /base/i2c@ff110000/camera@1a as cam0
0: 4208x3120-SRGGB10
 * Pixelformat: SRGGB10 (1048x780)-(4208x3120)/(+0,+0)
  - 1048x780
  - 2104x1560
  - 4208x3120


With this patch:

mobian@mobian:~/libcamera$ ./build/src/apps/cam/cam -c2 -I -srole=raw
Using camera /base/i2c@ff110000/camera@1a as cam0
0: 2104x1560-SRGGB10
 * Pixelformat: SRGGB10 (1048x780)-(2104x1560)/(+0,+0)
  - 1048x780
  - 2104x1560


Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


Now ... technically - I think
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240220235720.3010608-1-megi@xff.cz/
tells me that the RK3399 might actually support the highest mode (-
RK3399 has input/output limit of main path 4416 x 3312) but I don't
think the support for different platform limits has made it into 6.11
yet.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h?h=v6.11
still shows

#define RKISP1_ISP_MAX_WIDTH			4032
#define RKISP1_ISP_MAX_HEIGHT			3024

Which means, the PPP could be 'fixed' to get full output from the
IMX258, but for now - it's really useful for testing this patch series
;-)



> ---
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 51 ++++++++++++++++++-
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  8 +++
>  2 files changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 08b39e1e..1d818b32 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -137,12 +137,59 @@ void RkISP1Path::populateFormats()
>         }
>  }
>  
> +/**
> + * \brief Filter the sensor resolutions that can be supported
> + * \param[in] sensor The camera sensor
> + *
> + * This function retrieves all the sizes supported by the sensor and
> + * filters all the resolutions that can be supported on the pipeline.
> + * It is possible that the sensor's maximum output resolution is higher
> + * than the ISP maximum input. In that case, this function filters out all
> + * the resolution incapable of being supported and returns the maximum
> + * sensor resolution that can be supported by the pipeline.
> + *
> + * \return Maximum sensor size supported on the pipeline
> + */
> +Size RkISP1Path::filterSensorResolution(const CameraSensor *sensor)
> +{
> +       auto iter = sensorSizesMap_.find(sensor);
> +       if (iter != sensorSizesMap_.end() && !iter->second.empty())
> +               return iter->second.back();
> +
> +       sensorSizesMap_.emplace(sensor, std::vector<Size>());
> +
> +       std::vector<Size> sensorSizes;
> +       const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> +       for (const auto it : mbusCodes) {
> +               std::vector<Size> sizes = sensor->sizes(it);
> +               for (Size sz : sizes)
> +                       sensorSizes.push_back(sz);
> +       }
> +
> +       std::sort(sensorSizes.begin(), sensorSizes.end());
> +
> +       /* Remove duplicates. */
> +       auto last = std::unique(sensorSizes.begin(), sensorSizes.end());
> +       sensorSizes.erase(last, sensorSizes.end());
> +
> +       /* Discard any sizes that the pipeline is unable to support. */
> +       for (auto sz : sensorSizes) {
> +               if (sz.width > maxResolution_.width ||
> +                   sz.height > maxResolution_.height)
> +                       continue;
> +
> +               sensorSizesMap_[sensor].push_back(sz);
> +       }
> +
> +       return sensorSizesMap_[sensor].back();
> +}
> +
>  StreamConfiguration
>  RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size,
>                                   StreamRole role)
>  {
>         const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> -       const Size &resolution = sensor->resolution();
> +       Size resolution = filterSensorResolution(sensor);
>  
>         /* Min and max resolutions to populate the available stream formats. */
>         Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
> @@ -231,7 +278,7 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,
>                                                  StreamConfiguration *cfg)
>  {
>         const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> -       const Size &resolution = sensor->resolution();
> +       Size resolution = filterSensorResolution(sensor);
>  
>         const StreamConfiguration reqCfg = *cfg;
>         CameraConfiguration::Status status = CameraConfiguration::Valid;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index 13ba4b62..457c9ac9 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -7,6 +7,7 @@
>  
>  #pragma once
>  
> +#include <map>
>  #include <memory>
>  #include <set>
>  #include <vector>
> @@ -63,6 +64,7 @@ public:
>  
>  private:
>         void populateFormats();
> +       Size filterSensorResolution(const CameraSensor *sensor);
>  
>         static constexpr unsigned int RKISP1_BUFFER_COUNT = 4;
>  
> @@ -77,6 +79,12 @@ private:
>         std::unique_ptr<V4L2Subdevice> resizer_;
>         std::unique_ptr<V4L2VideoDevice> video_;
>         MediaLink *link_;
> +
> +       /*
> +        * Map from camera sensors to the sizes (in increasing order),
> +        * which are guaranteed to be supported by the pipeline.
> +        */
> +       std::map<const CameraSensor *, std::vector<Size>> sensorSizesMap_;
>  };
>  
>  class RkISP1MainPath : public RkISP1Path
> -- 
> 2.45.0
>