[libcamera-devel] [PATCH] libcamera: Add Samsung S5K3L6XX sensor support

[Kieran Bingham]

Hi Dorota,

It's great to see this progress on the Purism phone camera support.

Quoting Dorota Czaplejewicz (2021-10-12 12:39:46)
> The sensor is called s5k3l6xx in the kernel.
> 
> The driver is currently out of tree and maintained by Purism.

We prefer kernel drivers to be either upstream, or at least on their way
upstream. Ideally, a minimum of having been posted to the linux-media
mailinglist, however we know that the review process can take time, so
we can integrate support to libcamera before it's fully upstream in the
kernel to support the upstreaming process.

Is this sensor driver something that you are intending to post to the
linux-media mailinglist in the near future, or has it been posted
already?

> 
> It's the main camera sensor on the Librem 5.
> 
> Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz at puri.sm>
> ---
>  src/libcamera/camera_sensor_properties.cpp | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> index 48305ac4..0625a0d8 100644
> --- a/src/libcamera/camera_sensor_properties.cpp
> +++ b/src/libcamera/camera_sensor_properties.cpp
> @@ -134,6 +134,19 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeColorBars, 1 },
>                         },
>                 } },
> +               { "s5k3l6xx", {
> +                         .unitCellSize = { 1120, 1120 },
> +                         .testPatternModes = {
> +                                 { 0, controls::draft::TestPatternModeOff },
> +                                 { 1, controls::draft::TestPatternModeSolidColor },
> +                                 { 2, controls::draft::TestPatternModeColorBars },
> +                                 { 3, controls::draft::TestPatternModeColorBarsFadeToGray },
> +                                 { 4, controls::draft::TestPatternModeCustom1 }, /* White */
> +                                 { 5, controls::draft::TestPatternModePn9 },
> +                                 { 6, controls::draft::TestPatternModeCustom1 + 1 }, /* LFSR32 */
> +                                 { 7, controls::draft::TestPatternModeCustom1 + 2 }, /* Address */

That's an interesting way to define the extra custom modes...
Interstingly, I don't see any users of TestPatternModeCustom1 yet.

What is LFSR32 and Address?

Is 'White' just a solid colour of 'white'?

I suspect these modes should be defined within
src/libcamera/control_ids.yaml where possible.  An all white could be
TestPatternModeSolidColorWhite or TestPatternModeSolidWhite for intance.

--
Kieran


> +                       },
> +               } },
>         };
>  
>         const auto it = sensorProps.find(sensor);
> -- 
> 2.31.1
>