[v1,1/2] media: camss: Increase the maximum frame size
Commit Message
Commit 35493d653a2d
("media: camss: add support for vidioc_enum_framesizes ioctl") added a
maximum frame width and height but the values selected seemed to have
been arbitrary. In reality the cam hardware doesn't seem to have a maximum
size restriction so double up the maximum reported width and height to
allow for larger frames.
Also increase the maximum size checks at each point in the pipeline so
the increased sizes are allowed all the way down to the sensor.
Signed-off-by: Jordan Crouse <jorcrous@amazon.com>
---
drivers/media/platform/qcom/camss/camss-csid.c | 8 ++++----
drivers/media/platform/qcom/camss/camss-csiphy.c | 4 ++--
drivers/media/platform/qcom/camss/camss-ispif.c | 4 ++--
drivers/media/platform/qcom/camss/camss-vfe.c | 4 ++--
drivers/media/platform/qcom/camss/camss-video.c | 6 +++---
5 files changed, 13 insertions(+), 13 deletions(-)
Comments
On 02/08/2024 16:24, Jordan Crouse wrote:
> Commit 35493d653a2d
> ("media: camss: add support for vidioc_enum_framesizes ioctl") added a
> maximum frame width and height but the values selected seemed to have
> been arbitrary. In reality the cam hardware doesn't seem to have a maximum
> size restriction so double up the maximum reported width and height to
> allow for larger frames.
>
> Also increase the maximum size checks at each point in the pipeline so
> the increased sizes are allowed all the way down to the sensor.
So, I think this should be a Fixes: also.
>
> Signed-off-by: Jordan Crouse <jorcrous@amazon.com>
> ---
>
> drivers/media/platform/qcom/camss/camss-csid.c | 8 ++++----
> drivers/media/platform/qcom/camss/camss-csiphy.c | 4 ++--
> drivers/media/platform/qcom/camss/camss-ispif.c | 4 ++--
> drivers/media/platform/qcom/camss/camss-vfe.c | 4 ++--
> drivers/media/platform/qcom/camss/camss-video.c | 6 +++---
> 5 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 858db5d4ca75..886c42c82612 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -752,8 +752,8 @@ static void csid_try_format(struct csid_device *csid,
> if (i >= csid->res->formats->nformats)
> fmt->code = MEDIA_BUS_FMT_UYVY8_1X16;
>
> - fmt->width = clamp_t(u32, fmt->width, 1, 8191);
> - fmt->height = clamp_t(u32, fmt->height, 1, 8191);
> + fmt->width = clamp_t(u32, fmt->width, 1, 16383);
> + fmt->height = clamp_t(u32, fmt->height, 1, 16383);
Feels like we should have a define instead of hard coded values repeated
constantly.
---
bod
On 07/08/2024 00:09, Bryan O'Donoghue wrote:
>>
>> Also increase the maximum size checks at each point in the pipeline so
>> the increased sizes are allowed all the way down to the sensor.
>
> So, I think this should be a Fixes: also.
TL;DR we don't need a Fixes: for your v2.
Pardon me, not a Fixes: the sizeof(sensor) that can be supported is SoC
specific - camera IP specific really - so 8192 is a blunt descriptor,
raising to 16384 makes some kind of sense.
A long term fix for this would be for each camss descriptor to declare
the maximum sizeof(sensor) it supports. For sm8250, sc8280xp that's 64
megapixels - for older SoCs its going to be smaller.
---
bod
@@ -752,8 +752,8 @@ static void csid_try_format(struct csid_device *csid,
if (i >= csid->res->formats->nformats)
fmt->code = MEDIA_BUS_FMT_UYVY8_1X16;
- fmt->width = clamp_t(u32, fmt->width, 1, 8191);
- fmt->height = clamp_t(u32, fmt->height, 1, 8191);
+ fmt->width = clamp_t(u32, fmt->width, 1, 16383);
+ fmt->height = clamp_t(u32, fmt->height, 1, 16383);
fmt->field = V4L2_FIELD_NONE;
fmt->colorspace = V4L2_COLORSPACE_SRGB;
@@ -781,8 +781,8 @@ static void csid_try_format(struct csid_device *csid,
if (i >= csid->res->formats->nformats)
fmt->code = MEDIA_BUS_FMT_UYVY8_1X16;
- fmt->width = clamp_t(u32, fmt->width, 1, 8191);
- fmt->height = clamp_t(u32, fmt->height, 1, 8191);
+ fmt->width = clamp_t(u32, fmt->width, 1, 16383);
+ fmt->height = clamp_t(u32, fmt->height, 1, 16383);
fmt->field = V4L2_FIELD_NONE;
}
@@ -368,8 +368,8 @@ static void csiphy_try_format(struct csiphy_device *csiphy,
if (i >= csiphy->res->formats->nformats)
fmt->code = MEDIA_BUS_FMT_UYVY8_1X16;
- fmt->width = clamp_t(u32, fmt->width, 1, 8191);
- fmt->height = clamp_t(u32, fmt->height, 1, 8191);
+ fmt->width = clamp_t(u32, fmt->width, 1, 16383);
+ fmt->height = clamp_t(u32, fmt->height, 1, 16383);
fmt->field = V4L2_FIELD_NONE;
fmt->colorspace = V4L2_COLORSPACE_SRGB;
@@ -912,8 +912,8 @@ static void ispif_try_format(struct ispif_line *line,
if (i >= line->nformats)
fmt->code = MEDIA_BUS_FMT_UYVY8_1X16;
- fmt->width = clamp_t(u32, fmt->width, 1, 8191);
- fmt->height = clamp_t(u32, fmt->height, 1, 8191);
+ fmt->width = clamp_t(u32, fmt->width, 1, 16383);
+ fmt->height = clamp_t(u32, fmt->height, 1, 16383);
fmt->field = V4L2_FIELD_NONE;
fmt->colorspace = V4L2_COLORSPACE_SRGB;
@@ -1049,8 +1049,8 @@ static void vfe_try_format(struct vfe_line *line,
if (i >= line->nformats)
fmt->code = MEDIA_BUS_FMT_UYVY8_1X16;
- fmt->width = clamp_t(u32, fmt->width, 1, 8191);
- fmt->height = clamp_t(u32, fmt->height, 1, 8191);
+ fmt->width = clamp_t(u32, fmt->width, 1, 16383);
+ fmt->height = clamp_t(u32, fmt->height, 1, 16383);
fmt->field = V4L2_FIELD_NONE;
fmt->colorspace = V4L2_COLORSPACE_SRGB;
@@ -19,10 +19,10 @@
#include "camss.h"
#define CAMSS_FRAME_MIN_WIDTH 1
-#define CAMSS_FRAME_MAX_WIDTH 8191
+#define CAMSS_FRAME_MAX_WIDTH 16833
#define CAMSS_FRAME_MIN_HEIGHT 1
-#define CAMSS_FRAME_MAX_HEIGHT_RDI 8191
-#define CAMSS_FRAME_MAX_HEIGHT_PIX 4096
+#define CAMSS_FRAME_MAX_HEIGHT_RDI 16833
+#define CAMSS_FRAME_MAX_HEIGHT_PIX 8192
/* -----------------------------------------------------------------------------
* Helper functions