Re: [PATCH 4/4] media: rkvdec: align bytesperline to 64 bytes in rkvdec_fill_decoded_pixfmt()

Hi

> This shouldn't be needed. As you set step_width to 64,
> v4l2_apply_frmsize_constraints() should already do the alignment.
> Do you have more context as to when width is not aligned ?

You're right that the width is aligned -- step_width = 64 handles that.
This isn't the width though, it's the stride, and it only bites on the
10-bit (Profile 2) capture format.

For 8-bit NV12 the luma stride equals the width, so once the width is
64-aligned the stride is too and this block is a no-op.

The 10-bit capture format is NV15, whose luma stride is width * 5 / 4
(bpp {5, 10}, bpp_div {4, 4} in v4l2-common.c). That isn't a multiple
of 64 even when the width is: a 1920-wide Profile 2 frame gives a
2400-byte stride, while the VDPU381 needs the luma stride 64-byte
aligned (2432). v4l2_apply_frmsize_constraints() only rounds the width
and height, and v4l2_fill_pixfmt_mp() fills the stride with alignment 1,
so NV15 comes out unaligned and the decoded frame is corrupted without
this. I can show the before/after on a 10-bit vector if that's useful.

If you'd rather express it through the framework than with a manual
ALIGN() here, v4l2_fill_pixfmt_mp_aligned(pixfmt, pixelformat, width,
height, 64) produces the same bytesperline and sizeimage -- happy to
switch to that in v2.

Igor


On Mon, Jul 27, 2026 at 5:58 PM Detlev Casanova
<[email protected]> wrote:
>
> Hi !
>
> On Sunday, 26 July 2026 16:25:37 EDT Venkata Atchuta Bheemeswara Sarma Darbha
> wrote:
> > From: Igor Paunovic <[email protected]>
> >
> > Ensure bytesperline is 64-byte aligned and scale sizeimage accordingly.
> >
> > Signed-off-by: Igor Paunovic <[email protected]>
> > Signed-off-by: Francesco Saverio Pavone <[email protected]>
> > Signed-off-by: Venkata Atchuta Bheemeswara Sarma Darbha
> > <[email protected]> ---
> >  drivers/media/platform/rockchip/rkvdec/rkvdec.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> > b/drivers/media/platform/rockchip/rkvdec/rkvdec.c index
> > 436bb460f04e..6ab6530b1cc9 100644
> > --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> > +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> > @@ -102,12 +102,20 @@ static void rkvdec_fill_decoded_pixfmt(struct
> > rkvdec_ctx *ctx, struct v4l2_pix_format_mplane *pix_mp)
> >  {
> >       const struct rkvdec_variant *variant = ctx->dev->variant;
> > +     struct v4l2_plane_pix_format *plane = &pix_mp->plane_fmt[0];
> > +     u32 aligned_bpl;
> >
> >       v4l2_fill_pixfmt_mp(pix_mp, pix_mp->pixelformat, pix_mp->width,
> > pix_mp->height);
> >
> > -     ctx->colmv_offset = pix_mp->plane_fmt[0].sizeimage;
> > +     /* ensure bpl is 64 byte aligned and scale sizeimage accordingly.
> */
> > +     aligned_bpl = ALIGN(plane->bytesperline, 64);
> > +     if (aligned_bpl != plane->bytesperline) {
> > +             plane->sizeimage = plane->sizeimage / plane-
> >bytesperline * aligned_bpl;
> > +             plane->bytesperline = aligned_bpl;
> > +     }
>
> This shouldn't be needed. As you set step_width to 64,
> v4l2_apply_frmsize_constraints() should already do the alignment.
>
> Do you have more context as to when width is not aligned ?
>
> >
> > -     pix_mp->plane_fmt[0].sizeimage += variant->ops->colmv_size(pix_mp-
> >width,
> > pix_mp->height); +    ctx->colmv_offset = plane->sizeimage;
> > +     plane->sizeimage += variant->ops->colmv_size(pix_mp->width,
> > pix_mp->height); }
> >
> >  static void rkvdec_reset_fmt(struct rkvdec_ctx *ctx, struct v4l2_format *f,
>
> Regards,
> Detlev.
>
>
>

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip