Re: [PATCH RFC 4/7] media: qcom: iris: vdec: update size and stride calculations for 10bit formats

[Nicolas Dufresne]

Le vendredi 10 avril 2026 à 13:59 +0200, Neil Armstrong a écrit :
> On 4/10/26 12:10, Vishnu Reddy wrote:
> > 
> > On 4/8/2026 10:13 PM, Neil Armstrong wrote:
> > > Update the gen2 response and vdec s_fmt code to take in account
> > > the P010 and QC010 when calculating the width, height and stride.
> > > 
> > > Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> > > ---
> > >   .../platform/qcom/iris/iris_hfi_gen2_response.c     | 19
> > > ++++++++++++++++---
> > >   drivers/media/platform/qcom/iris/iris_vdec.c        | 21
> > > ++++++++++++++++++---
> > >   2 files changed, 34 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> > > b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> > > index 8e19f61bbbf9..d268149191ea 100644
> > > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> > > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> > > @@ -542,9 +542,22 @@ static void
> > > iris_hfi_gen2_read_input_subcr_params(struct iris_inst *inst)
> > >       pixmp_ip->width = width;
> > >       pixmp_ip->height = height;
> > > -    pixmp_op->width = ALIGN(width, 128);
> > > -    pixmp_op->height = ALIGN(height, 32);
> > > -    pixmp_op->plane_fmt[0].bytesperline = ALIGN(width, 128);
> > > +    pixmp_op->width = pixmp_op->pixelformat == V4L2_PIX_FMT_QC10C ?
> > > +        ALIGN(width, 192) : ALIGN(width, 128);
> > > +    pixmp_op->height = pixmp_op->pixelformat == V4L2_PIX_FMT_QC10C ?
> > > +        ALIGN(height, 16) : ALIGN(height, 32);
> > > +    switch (pixmp_op->pixelformat) {
> > > +    case V4L2_PIX_FMT_P010:
> > > +        pixmp_op->plane_fmt[0].bytesperline = ALIGN(width * 2, 256);
> > > +        break;
> > > +    case V4L2_PIX_FMT_QC10C:
> > > +        pixmp_op->plane_fmt[0].bytesperline = ALIGN(ALIGN(width, 192) * 4
> > > / 3, 256);
> > > +        break;
> > > +    case V4L2_PIX_FMT_NV12:
> > > +    case V4L2_PIX_FMT_QC08C:
> > > +        pixmp_op->plane_fmt[0].bytesperline = ALIGN(width, 128);
> > > +        break;
> > > +    }
> > >       pixmp_op->plane_fmt[0].sizeimage = iris_get_buffer_size(inst,
> > > BUF_OUTPUT);
> > >       matrix_coeff = subsc_params.color_info & 0xFF;
> > > diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c
> > > b/drivers/media/platform/qcom/iris/iris_vdec.c
> > > index 719217399a30..ca0518c27834 100644
> > > --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> > > +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> > > @@ -272,10 +272,25 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct
> > > v4l2_format *f)
> > >           fmt = inst->fmt_dst;
> > >           fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > >           fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
> > > -        fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
> > > -        fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
> > > +        codec_align = f->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C ?
> > > 192 : 128;
> > > +        fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, codec_align);
> > > +        codec_align = f->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C ?
> > > 16 : 32;
> > > +        fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height,
> > > codec_align);
> > >           fmt->fmt.pix_mp.num_planes = 1;
> > > -        fmt->fmt.pix_mp.plane_fmt[0].bytesperline = ALIGN(f-
> > > >fmt.pix_mp.width, 128);
> > > +        switch (f->fmt.pix_mp.pixelformat) {
> > > +        case V4L2_PIX_FMT_P010:
> > > +            fmt->fmt.pix_mp.plane_fmt[0].bytesperline =
> > > +                ALIGN(f->fmt.pix_mp.width * 2, 256);
> > > +            break;
> > > +        case V4L2_PIX_FMT_QC10C:
> > > +            fmt->fmt.pix_mp.plane_fmt[0].bytesperline =
> > > +                ALIGN(f->fmt.pix_mp.width * 4 / 3, 256);
> > > +            break;
> > > +        case V4L2_PIX_FMT_NV12:
> > > +        case V4L2_PIX_FMT_QC08C:
> > > +            fmt->fmt.pix_mp.plane_fmt[0].bytesperline = f-
> > > >fmt.pix_mp.width;
> > In the removed code, bytesperline for NV12 and QC08C was aligned to 128
> > bytes.
> > In the new code, Is that alignment missed or not required?
> 
> The alignment is done right before:
> 	codec_align = f->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C ? 192 :
> 128;
> 	fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, codec_align);
> 
> calling ALIGN(f->fmt.pix_mp.width, 128) again is a no-op.

What typically other drivers do is implement a static table that associate a
v4l2_frmsize_stepwise to each format, and possibly per HW generation. And then
uses v4l2_apply_frmsize_constraints() to apply them generically in their
try/s_fmt functions. This make lot cleaner and clearer code then the sate of
this driver.

Nicolas

> 
> Thanks,
> Neil
> 
> > > +            break;
> > > +        }
> > >           fmt->fmt.pix_mp.plane_fmt[0].sizeimage =
> > > iris_get_buffer_size(inst, BUF_OUTPUT);
> > >           inst->buffers[BUF_OUTPUT].min_count = iris_vpu_buf_count(inst,
> > > BUF_OUTPUT);
> > >           inst->buffers[BUF_OUTPUT].size = fmt-
> > > >fmt.pix_mp.plane_fmt[0].sizeimage;
> > > 
>

Attachment: signature.asc
Description: This is a digitally signed message part