public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] media: camss: Fix PIX BPL alignment handling in CAMSS
@ 2026-03-06 16:00 Loic Poulain
  2026-03-06 16:00 ` [PATCH 1/3] media: camss: Add per-format BPL alignment helper Loic Poulain
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Loic Poulain @ 2026-03-06 16:00 UTC (permalink / raw)
  To: bryan.odonoghue, vladimir.zapolskiy
  Cc: linux-media, linux-arm-msm, mchehab, Loic Poulain

This series refines the bytes-per-line (BPL) alignment logic across the
CAMSS pipeline. The current implementation relies on ALIGN(), which
only works for power-of-two alignment values. This is fine for RDI
interfaces, which typically use 64-bit or 128-bit alignment, but it
fails for PIX interfaces with Bayer formats such as RAW10 or RAW12
that require non-power-of-two byte alignment, causing hardware config
violations.

The series introduces a per-format alignment helper, updates the VFE
code to use proper rounding for non-power-of-two cases, and makes the
PIX BPL alignment logic format-based on CAMSS_2290.

Loic Poulain (3):
  media: camss: Add per-format BPL alignment helper
  media: camss: Use proper BPL alignment helper and non-power-of-two
    rounding
  media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290

 .../media/platform/qcom/camss/camss-format.c  | 28 +++++++++++++++++++
 .../media/platform/qcom/camss/camss-format.h  |  1 +
 drivers/media/platform/qcom/camss/camss-vfe.c | 28 ++++++++++++++++---
 .../media/platform/qcom/camss/camss-video.c   | 13 +++++++--
 4 files changed, 63 insertions(+), 7 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] media: camss: Add per-format BPL alignment helper
  2026-03-06 16:00 [PATCH 0/3] media: camss: Fix PIX BPL alignment handling in CAMSS Loic Poulain
@ 2026-03-06 16:00 ` Loic Poulain
  2026-03-06 18:00   ` Bryan O'Donoghue
  2026-03-09 11:10   ` Konrad Dybcio
  2026-03-06 16:00 ` [PATCH 2/3] media: camss: Use proper BPL alignment helper and non-power-of-two rounding Loic Poulain
  2026-03-06 16:00 ` [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290 Loic Poulain
  2 siblings, 2 replies; 12+ messages in thread
From: Loic Poulain @ 2026-03-06 16:00 UTC (permalink / raw)
  To: bryan.odonoghue, vladimir.zapolskiy
  Cc: linux-media, linux-arm-msm, mchehab, Loic Poulain

Add camss_format_get_bpl_alignment(), a helper that returns the
bytes-per-line (BPL) alignment requirement for a given CAMSS format.

Different RAW Bayer packing schemes impose different BPL alignment
constraints (e.g. RAW10 requires multiples of 5 bytes, RAW12 multiples of
3 bytes, RAW14 multiples of 7 bytes, etc.). Centralizing this logic
makes the alignment rules explicit and avoids duplicating them across
the pipeline.

This will allow PIX paths and buffer preparation code to correctly
round up BPL values to hardware-required boundaries.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-format.c  | 28 +++++++++++++++++++
 .../media/platform/qcom/camss/camss-format.h  |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/media/platform/qcom/camss/camss-format.c b/drivers/media/platform/qcom/camss/camss-format.c
index 4a3d5549615c..2cd0f3a0bfac 100644
--- a/drivers/media/platform/qcom/camss/camss-format.c
+++ b/drivers/media/platform/qcom/camss/camss-format.c
@@ -33,6 +33,34 @@ u8 camss_format_get_bpp(const struct camss_format_info *formats, unsigned int nf
 	return formats[0].mbus_bpp;
 }
 
+
+/*
+ * camss_format_get_bpl_alignment - Retrieve required BPL alignment for a given format.
+ * @format: a pointer to the format
+ *
+ * Return the required alignment, in bytes.
+ */
+unsigned int camss_format_get_bpl_alignment(const struct camss_format_info *format)
+{
+	switch (format->mbus_bpp) {
+	case 8: /* Plain 8-bit -> output must be a multiple of 1 pixel (1 byte) */
+		return 1;
+	case 10: /* Packed 10-bit -> output must be a multiple of 4 pixels (5 bytes) */
+		return 5;
+	case 12: /* Packed 12-bit -> output must be a multiple of 2 pixels (3 bytes) */
+		return 3;
+	case 14: /* Packed 14-bit -> output must be a multiple of 4 pixels (7 bytes) */
+		return 7;
+	case 16: /* 16-bit -> output must be a multiple of 1 pixel (2 bytes) */
+		return 2;
+	default:
+		WARN(1, "Unsupported format/bpp (%u)", format->mbus_bpp);
+	}
+
+	return 1;
+}
+
+
 /*
  * camss_format_find_code - Find a format code in an array
  * @code: a pointer to media bus format codes array
diff --git a/drivers/media/platform/qcom/camss/camss-format.h b/drivers/media/platform/qcom/camss/camss-format.h
index 923a48c9c3fb..4f87ac8c4975 100644
--- a/drivers/media/platform/qcom/camss/camss-format.h
+++ b/drivers/media/platform/qcom/camss/camss-format.h
@@ -55,6 +55,7 @@ struct camss_formats {
 };
 
 u8 camss_format_get_bpp(const struct camss_format_info *formats, unsigned int nformats, u32 code);
+unsigned int camss_format_get_bpl_alignment(const struct camss_format_info *f);
 u32 camss_format_find_code(u32 *code, unsigned int n_code, unsigned int index, u32 req_code);
 int camss_format_find_format(u32 code, u32 pixelformat, const struct camss_format_info *formats,
 			     unsigned int nformats);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] media: camss: Use proper BPL alignment helper and non-power-of-two rounding
  2026-03-06 16:00 [PATCH 0/3] media: camss: Fix PIX BPL alignment handling in CAMSS Loic Poulain
  2026-03-06 16:00 ` [PATCH 1/3] media: camss: Add per-format BPL alignment helper Loic Poulain
@ 2026-03-06 16:00 ` Loic Poulain
  2026-03-06 18:07   ` Bryan O'Donoghue
  2026-03-06 16:00 ` [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290 Loic Poulain
  2 siblings, 1 reply; 12+ messages in thread
From: Loic Poulain @ 2026-03-06 16:00 UTC (permalink / raw)
  To: bryan.odonoghue, vladimir.zapolskiy
  Cc: linux-media, linux-arm-msm, mchehab, Loic Poulain

Bytes-per-line (BPL) alignment in CAMSS currently uses ALIGN(), which
only works correctly for power-of-two values. Some RAW Bayer packing
formats (e.g. RAW10/12/14) require non-power-of-two alignment such as
3, 5, or 7-byte multiples, so ALIGN() produces incorrect results.

Introduce the use of roundup() with the per-format alignment returned by
camss_format_get_bpl_alignment() when no hardware alignment is enforced
(video->bpl_alignment).

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/media/platform/qcom/camss/camss-video.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index f52d8e84f970..0852eb6f1315 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -47,6 +47,9 @@ static int video_mbus_to_pix_mp(const struct v4l2_mbus_framefmt *mbus,
 	unsigned int i;
 	u32 bytesperline;
 
+	if (!alignment)
+		alignment = camss_format_get_bpl_alignment(f);
+
 	memset(pix, 0, sizeof(*pix));
 	v4l2_fill_pix_format_mplane(pix, mbus);
 	pix->pixelformat = f->pixelformat;
@@ -54,7 +57,7 @@ static int video_mbus_to_pix_mp(const struct v4l2_mbus_framefmt *mbus,
 	for (i = 0; i < pix->num_planes; i++) {
 		bytesperline = pix->width / f->hsub[i].numerator *
 			f->hsub[i].denominator * f->bpp[i] / 8;
-		bytesperline = ALIGN(bytesperline, alignment);
+		bytesperline = roundup(bytesperline, alignment);
 		pix->plane_fmt[i].bytesperline = bytesperline;
 		pix->plane_fmt[i].sizeimage = pix->height /
 				f->vsub[i].numerator * f->vsub[i].denominator *
@@ -459,6 +462,7 @@ static int video_g_fmt(struct file *file, void *fh, struct v4l2_format *f)
 
 static int __video_try_fmt(struct camss_video *video, struct v4l2_format *f)
 {
+	unsigned int alignment = video->bpl_alignment;
 	struct v4l2_pix_format_mplane *pix_mp;
 	const struct camss_format_info *fi;
 	struct v4l2_plane_pix_format *p;
@@ -491,6 +495,9 @@ static int __video_try_fmt(struct camss_video *video, struct v4l2_format *f)
 	width = pix_mp->width;
 	height = pix_mp->height;
 
+	if (!alignment)
+		alignment = camss_format_get_bpl_alignment(fi);
+
 	memset(pix_mp, 0, sizeof(*pix_mp));
 
 	pix_mp->pixelformat = fi->pixelformat;
@@ -500,7 +507,7 @@ static int __video_try_fmt(struct camss_video *video, struct v4l2_format *f)
 	for (i = 0; i < pix_mp->num_planes; i++) {
 		bpl = pix_mp->width / fi->hsub[i].numerator *
 			fi->hsub[i].denominator * fi->bpp[i] / 8;
-		bpl = ALIGN(bpl, video->bpl_alignment);
+		bpl = roundup(bpl, alignment);
 		pix_mp->plane_fmt[i].bytesperline = bpl;
 		pix_mp->plane_fmt[i].sizeimage = pix_mp->height /
 			fi->vsub[i].numerator * fi->vsub[i].denominator * bpl;
@@ -525,7 +532,7 @@ static int __video_try_fmt(struct camss_video *video, struct v4l2_format *f)
 			lines = p->sizeimage / p->bytesperline;
 
 			if (p->bytesperline < bytesperline[i])
-				p->bytesperline = ALIGN(bytesperline[i], 8);
+				p->bytesperline = roundup(bytesperline[i], alignment);
 
 			if (p->sizeimage < p->bytesperline * lines)
 				p->sizeimage = p->bytesperline * lines;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290
  2026-03-06 16:00 [PATCH 0/3] media: camss: Fix PIX BPL alignment handling in CAMSS Loic Poulain
  2026-03-06 16:00 ` [PATCH 1/3] media: camss: Add per-format BPL alignment helper Loic Poulain
  2026-03-06 16:00 ` [PATCH 2/3] media: camss: Use proper BPL alignment helper and non-power-of-two rounding Loic Poulain
@ 2026-03-06 16:00 ` Loic Poulain
  2026-03-06 18:04   ` Bryan O'Donoghue
  2026-03-09 11:12   ` Konrad Dybcio
  2 siblings, 2 replies; 12+ messages in thread
From: Loic Poulain @ 2026-03-06 16:00 UTC (permalink / raw)
  To: bryan.odonoghue, vladimir.zapolskiy
  Cc: linux-media, linux-arm-msm, mchehab, Loic Poulain

Split the VFE bytes-per-line (BPL) alignment logic into separate
helpers for RDI and PIX paths. RDI is usually aligned on RDI write
engine bus constraint such as 64-bit or 128-bit. But PIX engine
is usually (at least on platform I looked at) based on pixel format.

On CAMSS_2290, PIX BPL alignment is set to 0 to indicate that the
alignment must be derived from the pixel format. This allows the
pipeline to use camss_format_get_bpl_alignment().

For other platforms, retain the legacy PIX default (16 bytes), until
PIX is properly tested/enabled.

A future improvement would be to remove platform-specific conditionals
from the VFE code and move the alignment requirements into the
per-platform VFE resource data.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/media/platform/qcom/camss/camss-vfe.c | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 9c7ad8aa4058..c174c7d706e2 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -1996,7 +1996,7 @@ static const struct media_entity_operations vfe_media_ops = {
 	.link_validate = v4l2_subdev_link_validate,
 };
 
-static int vfe_bpl_align(struct vfe_device *vfe)
+static int vfe_bpl_align_rdi(struct vfe_device *vfe)
 {
 	int ret = 8;
 
@@ -2019,6 +2019,25 @@ static int vfe_bpl_align(struct vfe_device *vfe)
 	return ret;
 }
 
+static int vfe_bpl_align_pix(struct vfe_device *vfe)
+{
+	int ret = 16;
+
+	switch (vfe->camss->res->version) {
+	case CAMSS_2290:
+		/* The alignment/bpl depends solely on the pixel format and is
+		 * computed dynamically in camss_format_get_bpl_alignment().
+		 */
+		ret = 0;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+
 /*
  * msm_vfe_register_entities - Register subdev node for VFE module
  * @vfe: VFE device
@@ -2085,11 +2104,12 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
 		}
 
 		video_out->ops = &vfe->video_ops;
-		video_out->bpl_alignment = vfe_bpl_align(vfe);
-		video_out->line_based = 0;
 		if (i == VFE_LINE_PIX) {
-			video_out->bpl_alignment = 16;
+			video_out->bpl_alignment = vfe_bpl_align_pix(vfe);
 			video_out->line_based = 1;
+		} else {
+			video_out->bpl_alignment = vfe_bpl_align_rdi(vfe);
+			video_out->line_based = 0;
 		}
 
 		video_out->nformats = vfe->line[i].nformats;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] media: camss: Add per-format BPL alignment helper
  2026-03-06 16:00 ` [PATCH 1/3] media: camss: Add per-format BPL alignment helper Loic Poulain
@ 2026-03-06 18:00   ` Bryan O'Donoghue
  2026-03-06 19:56     ` Loic Poulain
  2026-03-09 11:10   ` Konrad Dybcio
  1 sibling, 1 reply; 12+ messages in thread
From: Bryan O'Donoghue @ 2026-03-06 18:00 UTC (permalink / raw)
  To: Loic Poulain, vladimir.zapolskiy; +Cc: linux-media, linux-arm-msm, mchehab

On 06/03/2026 16:00, Loic Poulain wrote:
> +	default:
> +		WARN(1, "Unsupported format/bpp (%u)", format->mbus_bpp);
> +	}
> +
> +	return 1;

An error should return an error not a default.

-ENONOTSUPP, -EINVAL, -ENOWAYDUDE something to indicate failure.

---
bod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290
  2026-03-06 16:00 ` [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290 Loic Poulain
@ 2026-03-06 18:04   ` Bryan O'Donoghue
  2026-03-09 11:12   ` Konrad Dybcio
  1 sibling, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2026-03-06 18:04 UTC (permalink / raw)
  To: Loic Poulain, vladimir.zapolskiy; +Cc: linux-media, linux-arm-msm, mchehab

On 06/03/2026 16:00, Loic Poulain wrote:
> Split the VFE bytes-per-line (BPL) alignment logic into separate
> helpers for RDI and PIX paths. RDI is usually aligned on RDI write
> engine bus constraint such as 64-bit or 128-bit. But PIX engine
> is usually (at least on platform I looked at) based on pixel format.
> 
> On CAMSS_2290, PIX BPL alignment is set to 0 to indicate that the
> alignment must be derived from the pixel format. This allows the
> pipeline to use camss_format_get_bpl_alignment().
> 
> For other platforms, retain the legacy PIX default (16 bytes), until
> PIX is properly tested/enabled.
> 
> A future improvement would be to remove platform-specific conditionals
> from the VFE code and move the alignment requirements into the
> per-platform VFE resource data.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/camss/camss-vfe.c | 28 ++++++++++++++++---
>   1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index 9c7ad8aa4058..c174c7d706e2 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -1996,7 +1996,7 @@ static const struct media_entity_operations vfe_media_ops = {
>   	.link_validate = v4l2_subdev_link_validate,
>   };
>   
> -static int vfe_bpl_align(struct vfe_device *vfe)
> +static int vfe_bpl_align_rdi(struct vfe_device *vfe)
>   {
>   	int ret = 8;
>   
> @@ -2019,6 +2019,25 @@ static int vfe_bpl_align(struct vfe_device *vfe)
>   	return ret;
>   }
>   
> +static int vfe_bpl_align_pix(struct vfe_device *vfe)
> +{
> +	int ret = 16;
> +
> +	switch (vfe->camss->res->version) {
> +	case CAMSS_2290:
> +		/* The alignment/bpl depends solely on the pixel format and is
> +		 * computed dynamically in camss_format_get_bpl_alignment().
> +		 */
> +		ret = 0;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +
>   /*
>    * msm_vfe_register_entities - Register subdev node for VFE module
>    * @vfe: VFE device
> @@ -2085,11 +2104,12 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
>   		}
>   
>   		video_out->ops = &vfe->video_ops;
> -		video_out->bpl_alignment = vfe_bpl_align(vfe);
> -		video_out->line_based = 0;
>   		if (i == VFE_LINE_PIX) {
> -			video_out->bpl_alignment = 16;
> +			video_out->bpl_alignment = vfe_bpl_align_pix(vfe);
>   			video_out->line_based = 1;
> +		} else {
> +			video_out->bpl_alignment = vfe_bpl_align_rdi(vfe);
> +			video_out->line_based = 0;
>   		}
>   
>   		video_out->nformats = vfe->line[i].nformats;

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] media: camss: Use proper BPL alignment helper and non-power-of-two rounding
  2026-03-06 16:00 ` [PATCH 2/3] media: camss: Use proper BPL alignment helper and non-power-of-two rounding Loic Poulain
@ 2026-03-06 18:07   ` Bryan O'Donoghue
  0 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2026-03-06 18:07 UTC (permalink / raw)
  To: Loic Poulain, vladimir.zapolskiy; +Cc: linux-media, linux-arm-msm, mchehab

On 06/03/2026 16:00, Loic Poulain wrote:
> Bytes-per-line (BPL) alignment in CAMSS currently uses ALIGN(), which
> only works correctly for power-of-two values. Some RAW Bayer packing
> formats (e.g. RAW10/12/14) require non-power-of-two alignment such as
> 3, 5, or 7-byte multiples, so ALIGN() produces incorrect results.
> 
> Introduce the use of roundup() with the per-format alignment returned by
> camss_format_get_bpl_alignment() when no hardware alignment is enforced
> (video->bpl_alignment).
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/camss/camss-video.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index f52d8e84f970..0852eb6f1315 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -47,6 +47,9 @@ static int video_mbus_to_pix_mp(const struct v4l2_mbus_framefmt *mbus,
>   	unsigned int i;
>   	u32 bytesperline;
>   
> +	if (!alignment)
> +		alignment = camss_format_get_bpl_alignment(f);
> +
>   	memset(pix, 0, sizeof(*pix));
>   	v4l2_fill_pix_format_mplane(pix, mbus);
>   	pix->pixelformat = f->pixelformat;
> @@ -54,7 +57,7 @@ static int video_mbus_to_pix_mp(const struct v4l2_mbus_framefmt *mbus,
>   	for (i = 0; i < pix->num_planes; i++) {
>   		bytesperline = pix->width / f->hsub[i].numerator *
>   			f->hsub[i].denominator * f->bpp[i] / 8;
> -		bytesperline = ALIGN(bytesperline, alignment);
> +		bytesperline = roundup(bytesperline, alignment);
>   		pix->plane_fmt[i].bytesperline = bytesperline;
>   		pix->plane_fmt[i].sizeimage = pix->height /
>   				f->vsub[i].numerator * f->vsub[i].denominator *
> @@ -459,6 +462,7 @@ static int video_g_fmt(struct file *file, void *fh, struct v4l2_format *f)
>   
>   static int __video_try_fmt(struct camss_video *video, struct v4l2_format *f)
>   {
> +	unsigned int alignment = video->bpl_alignment;
>   	struct v4l2_pix_format_mplane *pix_mp;
>   	const struct camss_format_info *fi;
>   	struct v4l2_plane_pix_format *p;
> @@ -491,6 +495,9 @@ static int __video_try_fmt(struct camss_video *video, struct v4l2_format *f)
>   	width = pix_mp->width;
>   	height = pix_mp->height;
>   
> +	if (!alignment)
> +		alignment = camss_format_get_bpl_alignment(fi);
> +
>   	memset(pix_mp, 0, sizeof(*pix_mp));
>   
>   	pix_mp->pixelformat = fi->pixelformat;
> @@ -500,7 +507,7 @@ static int __video_try_fmt(struct camss_video *video, struct v4l2_format *f)
>   	for (i = 0; i < pix_mp->num_planes; i++) {
>   		bpl = pix_mp->width / fi->hsub[i].numerator *
>   			fi->hsub[i].denominator * fi->bpp[i] / 8;
> -		bpl = ALIGN(bpl, video->bpl_alignment);
> +		bpl = roundup(bpl, alignment);
>   		pix_mp->plane_fmt[i].bytesperline = bpl;
>   		pix_mp->plane_fmt[i].sizeimage = pix_mp->height /
>   			fi->vsub[i].numerator * fi->vsub[i].denominator * bpl;
> @@ -525,7 +532,7 @@ static int __video_try_fmt(struct camss_video *video, struct v4l2_format *f)
>   			lines = p->sizeimage / p->bytesperline;
>   
>   			if (p->bytesperline < bytesperline[i])
> -				p->bytesperline = ALIGN(bytesperline[i], 8);
> +				p->bytesperline = roundup(bytesperline[i], alignment);
>   
>   			if (p->sizeimage < p->bytesperline * lines)
>   				p->sizeimage = p->bytesperline * lines;

This seems fine.

I still don't think the error handling in the dependent function is 
right/warranted so I'll hold off on RB until you updated that.

---
bod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] media: camss: Add per-format BPL alignment helper
  2026-03-06 18:00   ` Bryan O'Donoghue
@ 2026-03-06 19:56     ` Loic Poulain
  0 siblings, 0 replies; 12+ messages in thread
From: Loic Poulain @ 2026-03-06 19:56 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: vladimir.zapolskiy, linux-media, linux-arm-msm, mchehab

On Fri, Mar 6, 2026 at 7:00 PM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 06/03/2026 16:00, Loic Poulain wrote:
> > +     default:
> > +             WARN(1, "Unsupported format/bpp (%u)", format->mbus_bpp);
> > +     }
> > +
> > +     return 1;
>
> An error should return an error not a default.
>
> -ENONOTSUPP, -EINVAL, -ENOWAYDUDE something to indicate failure.

Yes, here we return a 1‑byte default alignment with a warning as a
best‑effort guess. However, it would be indeed safer to return an
error and let the caller handle it.

Regards,
Loic

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] media: camss: Add per-format BPL alignment helper
  2026-03-06 16:00 ` [PATCH 1/3] media: camss: Add per-format BPL alignment helper Loic Poulain
  2026-03-06 18:00   ` Bryan O'Donoghue
@ 2026-03-09 11:10   ` Konrad Dybcio
  2026-03-13 14:11     ` Loic Poulain
  1 sibling, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2026-03-09 11:10 UTC (permalink / raw)
  To: Loic Poulain, bryan.odonoghue, vladimir.zapolskiy
  Cc: linux-media, linux-arm-msm, mchehab

On 3/6/26 5:00 PM, Loic Poulain wrote:
> Add camss_format_get_bpl_alignment(), a helper that returns the
> bytes-per-line (BPL) alignment requirement for a given CAMSS format.
> 
> Different RAW Bayer packing schemes impose different BPL alignment
> constraints (e.g. RAW10 requires multiples of 5 bytes, RAW12 multiples of
> 3 bytes, RAW14 multiples of 7 bytes, etc.). Centralizing this logic
> makes the alignment rules explicit and avoids duplicating them across
> the pipeline.
> 
> This will allow PIX paths and buffer preparation code to correctly
> round up BPL values to hardware-required boundaries.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  .../media/platform/qcom/camss/camss-format.c  | 28 +++++++++++++++++++
>  .../media/platform/qcom/camss/camss-format.h  |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-format.c b/drivers/media/platform/qcom/camss/camss-format.c
> index 4a3d5549615c..2cd0f3a0bfac 100644
> --- a/drivers/media/platform/qcom/camss/camss-format.c
> +++ b/drivers/media/platform/qcom/camss/camss-format.c
> @@ -33,6 +33,34 @@ u8 camss_format_get_bpp(const struct camss_format_info *formats, unsigned int nf
>  	return formats[0].mbus_bpp;
>  }
>  
> +
> +/*
> + * camss_format_get_bpl_alignment - Retrieve required BPL alignment for a given format.
> + * @format: a pointer to the format
> + *
> + * Return the required alignment, in bytes.
> + */
> +unsigned int camss_format_get_bpl_alignment(const struct camss_format_info *format)
> +{
> +	switch (format->mbus_bpp) {
> +	case 8: /* Plain 8-bit -> output must be a multiple of 1 pixel (1 byte) */
> +		return 1;
> +	case 10: /* Packed 10-bit -> output must be a multiple of 4 pixels (5 bytes) */
> +		return 5;
> +	case 12: /* Packed 12-bit -> output must be a multiple of 2 pixels (3 bytes) */
> +		return 3;
> +	case 14: /* Packed 14-bit -> output must be a multiple of 4 pixels (7 bytes) */
> +		return 7;
> +	case 16: /* 16-bit -> output must be a multiple of 1 pixel (2 bytes) */
> +		return 2;
> +	default:
> +		WARN(1, "Unsupported format/bpp (%u)", format->mbus_bpp);
> +	}

The intention behind this could be better expressed with:

#include <linux/lcm.h>

return lcm(BITS_PER_BYTE, bpp)/BITS_PER_BYTE

"take as many bytes as we need so that we get k full pixels"

There's probably an even smarter way to write this that doesn't require
BITS_PER_BYTE to be present twice

Konrad

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290
  2026-03-06 16:00 ` [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290 Loic Poulain
  2026-03-06 18:04   ` Bryan O'Donoghue
@ 2026-03-09 11:12   ` Konrad Dybcio
  2026-03-13 13:46     ` Loic Poulain
  1 sibling, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2026-03-09 11:12 UTC (permalink / raw)
  To: Loic Poulain, bryan.odonoghue, vladimir.zapolskiy
  Cc: linux-media, linux-arm-msm, mchehab

On 3/6/26 5:00 PM, Loic Poulain wrote:
> Split the VFE bytes-per-line (BPL) alignment logic into separate
> helpers for RDI and PIX paths. RDI is usually aligned on RDI write
> engine bus constraint such as 64-bit or 128-bit. But PIX engine
> is usually (at least on platform I looked at) based on pixel format.
> 
> On CAMSS_2290, PIX BPL alignment is set to 0 to indicate that the
> alignment must be derived from the pixel format. This allows the
> pipeline to use camss_format_get_bpl_alignment().
> 
> For other platforms, retain the legacy PIX default (16 bytes), until
> PIX is properly tested/enabled.
> 
> A future improvement would be to remove platform-specific conditionals
> from the VFE code and move the alignment requirements into the
> per-platform VFE resource data.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/media/platform/qcom/camss/camss-vfe.c | 28 ++++++++++++++++---
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index 9c7ad8aa4058..c174c7d706e2 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -1996,7 +1996,7 @@ static const struct media_entity_operations vfe_media_ops = {
>  	.link_validate = v4l2_subdev_link_validate,
>  };
>  
> -static int vfe_bpl_align(struct vfe_device *vfe)
> +static int vfe_bpl_align_rdi(struct vfe_device *vfe)
>  {
>  	int ret = 8;
>  
> @@ -2019,6 +2019,25 @@ static int vfe_bpl_align(struct vfe_device *vfe)
>  	return ret;
>  }
>  
> +static int vfe_bpl_align_pix(struct vfe_device *vfe)
> +{
> +	int ret = 16;
> +
> +	switch (vfe->camss->res->version) {
> +	case CAMSS_2290:
> +		/* The alignment/bpl depends solely on the pixel format and is
> +		 * computed dynamically in camss_format_get_bpl_alignment().

The immediate question to ask is whether this will be the case for all
other platforms, i.e. whether video->bpl_alignment will ever be nonzero
in patch 3

Konrad

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290
  2026-03-09 11:12   ` Konrad Dybcio
@ 2026-03-13 13:46     ` Loic Poulain
  0 siblings, 0 replies; 12+ messages in thread
From: Loic Poulain @ 2026-03-13 13:46 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: bryan.odonoghue, vladimir.zapolskiy, linux-media, linux-arm-msm,
	mchehab

On Mon, Mar 9, 2026 at 12:12 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 3/6/26 5:00 PM, Loic Poulain wrote:
> > Split the VFE bytes-per-line (BPL) alignment logic into separate
> > helpers for RDI and PIX paths. RDI is usually aligned on RDI write
> > engine bus constraint such as 64-bit or 128-bit. But PIX engine
> > is usually (at least on platform I looked at) based on pixel format.
> >
> > On CAMSS_2290, PIX BPL alignment is set to 0 to indicate that the
> > alignment must be derived from the pixel format. This allows the
> > pipeline to use camss_format_get_bpl_alignment().
> >
> > For other platforms, retain the legacy PIX default (16 bytes), until
> > PIX is properly tested/enabled.
> >
> > A future improvement would be to remove platform-specific conditionals
> > from the VFE code and move the alignment requirements into the
> > per-platform VFE resource data.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  drivers/media/platform/qcom/camss/camss-vfe.c | 28 ++++++++++++++++---
> >  1 file changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> > index 9c7ad8aa4058..c174c7d706e2 100644
> > --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> > +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> > @@ -1996,7 +1996,7 @@ static const struct media_entity_operations vfe_media_ops = {
> >       .link_validate = v4l2_subdev_link_validate,
> >  };
> >
> > -static int vfe_bpl_align(struct vfe_device *vfe)
> > +static int vfe_bpl_align_rdi(struct vfe_device *vfe)
> >  {
> >       int ret = 8;
> >
> > @@ -2019,6 +2019,25 @@ static int vfe_bpl_align(struct vfe_device *vfe)
> >       return ret;
> >  }
> >
> > +static int vfe_bpl_align_pix(struct vfe_device *vfe)
> > +{
> > +     int ret = 16;
> > +
> > +     switch (vfe->camss->res->version) {
> > +     case CAMSS_2290:
> > +             /* The alignment/bpl depends solely on the pixel format and is
> > +              * computed dynamically in camss_format_get_bpl_alignment().
>
> The immediate question to ask is whether this will be the case for all
> other platforms, i.e. whether video->bpl_alignment will ever be nonzero
> in patch 3

I expect most modern platforms to rely on pixel‑format based
constraints, so they would eventually also use bpl_alignment = 0.
However, for this first enabling, I prefer to stay conservative and
keep the legacy fixed value on platforms that haven’t been validated
yet.

I would like to review the other platforms in the coming months to get
a clearer picture. Once that’s done, I’ll follow up with a change that
moves the alignment requirements into the per‑platform resource data,
so we can eliminate the platform‑specific conditionals from camss-vfe.

Regards,
Loic

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] media: camss: Add per-format BPL alignment helper
  2026-03-09 11:10   ` Konrad Dybcio
@ 2026-03-13 14:11     ` Loic Poulain
  0 siblings, 0 replies; 12+ messages in thread
From: Loic Poulain @ 2026-03-13 14:11 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: bryan.odonoghue, vladimir.zapolskiy, linux-media, linux-arm-msm,
	mchehab

On Mon, Mar 9, 2026 at 12:10 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 3/6/26 5:00 PM, Loic Poulain wrote:
> > Add camss_format_get_bpl_alignment(), a helper that returns the
> > bytes-per-line (BPL) alignment requirement for a given CAMSS format.
> >
> > Different RAW Bayer packing schemes impose different BPL alignment
> > constraints (e.g. RAW10 requires multiples of 5 bytes, RAW12 multiples of
> > 3 bytes, RAW14 multiples of 7 bytes, etc.). Centralizing this logic
> > makes the alignment rules explicit and avoids duplicating them across
> > the pipeline.
> >
> > This will allow PIX paths and buffer preparation code to correctly
> > round up BPL values to hardware-required boundaries.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  .../media/platform/qcom/camss/camss-format.c  | 28 +++++++++++++++++++
> >  .../media/platform/qcom/camss/camss-format.h  |  1 +
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/media/platform/qcom/camss/camss-format.c b/drivers/media/platform/qcom/camss/camss-format.c
> > index 4a3d5549615c..2cd0f3a0bfac 100644
> > --- a/drivers/media/platform/qcom/camss/camss-format.c
> > +++ b/drivers/media/platform/qcom/camss/camss-format.c
> > @@ -33,6 +33,34 @@ u8 camss_format_get_bpp(const struct camss_format_info *formats, unsigned int nf
> >       return formats[0].mbus_bpp;
> >  }
> >
> > +
> > +/*
> > + * camss_format_get_bpl_alignment - Retrieve required BPL alignment for a given format.
> > + * @format: a pointer to the format
> > + *
> > + * Return the required alignment, in bytes.
> > + */
> > +unsigned int camss_format_get_bpl_alignment(const struct camss_format_info *format)
> > +{
> > +     switch (format->mbus_bpp) {
> > +     case 8: /* Plain 8-bit -> output must be a multiple of 1 pixel (1 byte) */
> > +             return 1;
> > +     case 10: /* Packed 10-bit -> output must be a multiple of 4 pixels (5 bytes) */
> > +             return 5;
> > +     case 12: /* Packed 12-bit -> output must be a multiple of 2 pixels (3 bytes) */
> > +             return 3;
> > +     case 14: /* Packed 14-bit -> output must be a multiple of 4 pixels (7 bytes) */
> > +             return 7;
> > +     case 16: /* 16-bit -> output must be a multiple of 1 pixel (2 bytes) */
> > +             return 2;
> > +     default:
> > +             WARN(1, "Unsupported format/bpp (%u)", format->mbus_bpp);
> > +     }
>
> The intention behind this could be better expressed with:
>
> #include <linux/lcm.h>
>
> return lcm(BITS_PER_BYTE, bpp)/BITS_PER_BYTE
>
> "take as many bytes as we need so that we get k full pixels"

Yes, that works, and it should correctly reflect the hardware
constraints for all bpp formats we support today.

> There's probably an even smarter way to write this that doesn't require
> BITS_PER_BYTE to be present twice

A lcm() or gcd() based expression looks smart enough :-) I’m not aware
of any simpler custom helper for this.

Thanks,
Loic

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-03-13 14:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-06 16:00 [PATCH 0/3] media: camss: Fix PIX BPL alignment handling in CAMSS Loic Poulain
2026-03-06 16:00 ` [PATCH 1/3] media: camss: Add per-format BPL alignment helper Loic Poulain
2026-03-06 18:00   ` Bryan O'Donoghue
2026-03-06 19:56     ` Loic Poulain
2026-03-09 11:10   ` Konrad Dybcio
2026-03-13 14:11     ` Loic Poulain
2026-03-06 16:00 ` [PATCH 2/3] media: camss: Use proper BPL alignment helper and non-power-of-two rounding Loic Poulain
2026-03-06 18:07   ` Bryan O'Donoghue
2026-03-06 16:00 ` [PATCH 3/3] media: camss: vfe: Make PIX BPL alignment format-based on CAMSS_2290 Loic Poulain
2026-03-06 18:04   ` Bryan O'Donoghue
2026-03-09 11:12   ` Konrad Dybcio
2026-03-13 13:46     ` Loic Poulain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox