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

[Kieran Bingham]

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 at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at 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 at mobian:~/libcamera$ cam -c2 -I -srole=raw

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


With this patch:

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


Tested-by: Kieran Bingham <kieran.bingham at 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
>