| Message ID | 20241120-qcom-video-iris-v6-10-a8cf6704e992@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Qualcomm iris video decoder driver | expand |
Hi Dikshita, Some comments below... On 20/11/2024 15:46, Dikshita Agarwal wrote: > From: Vedang Nagar <quic_vnagar@quicinc.com> > > Implement s_fmt, g_fmt and try_fmt ioctl ops with necessary hooks. > > Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com> > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > drivers/media/platform/qcom/iris/iris_vdec.c | 122 +++++++++++++++++++++++++++ > drivers/media/platform/qcom/iris/iris_vdec.h | 2 + > drivers/media/platform/qcom/iris/iris_vidc.c | 48 +++++++++++ > 3 files changed, 172 insertions(+) > > diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c > index 2ed50ad5d58b..11a2507fc35f 100644 > --- a/drivers/media/platform/qcom/iris/iris_vdec.c > +++ b/drivers/media/platform/qcom/iris/iris_vdec.c > @@ -3,6 +3,8 @@ > * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. > */ > > +#include <media/v4l2-mem2mem.h> > + > #include "iris_buffer.h" > #include "iris_instance.h" > #include "iris_vdec.h" > @@ -10,6 +12,7 @@ > > #define DEFAULT_WIDTH 320 > #define DEFAULT_HEIGHT 240 > +#define DEFAULT_CODEC_ALIGNMENT 16 > > void iris_vdec_inst_init(struct iris_inst *inst) > { > @@ -54,3 +57,122 @@ void iris_vdec_inst_deinit(struct iris_inst *inst) > kfree(inst->fmt_dst); > kfree(inst->fmt_src); > } > + > +int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f) > +{ > + struct v4l2_pix_format_mplane *pixmp = &f->fmt.pix_mp; > + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx; > + struct v4l2_format *f_inst; > + struct vb2_queue *src_q; > + > + memset(pixmp->reserved, 0, sizeof(pixmp->reserved)); > + switch (f->type) { > + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: > + if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_H264) { > + f_inst = inst->fmt_src; > + f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; > + f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; > + f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat; > + } > + break; > + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: > + if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) { > + f_inst = inst->fmt_dst; > + f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat; > + f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; > + f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; > + } > + > + src_q = v4l2_m2m_get_src_vq(m2m_ctx); > + if (vb2_is_streaming(src_q)) { > + f_inst = inst->fmt_src; > + f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; > + f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; > + } > + break; > + default: > + return -EINVAL; > + } > + > + if (pixmp->field == V4L2_FIELD_ANY) > + pixmp->field = V4L2_FIELD_NONE; > + > + pixmp->num_planes = 1; > + > + return 0; > +} > + > +int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f) > +{ > + struct v4l2_format *fmt, *output_fmt; > + struct vb2_queue *q; > + u32 codec_align; > + > + iris_vdec_try_fmt(inst, f); I'm missing a vb2_is_busy() check here (and a return -EBUSY). Changing the format while busy (i.e. while buffers are allocated) is generally a no-go since the format and the buffer sizes are linked. > + > + switch (f->type) { > + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: > + if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_H264) > + return -EINVAL; > + > + fmt = inst->fmt_src; > + fmt->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; > + > + codec_align = DEFAULT_CODEC_ALIGNMENT; > + fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, codec_align); > + 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 = 0; > + fmt->fmt.pix_mp.plane_fmt[0].sizeimage = iris_get_buffer_size(inst, BUF_INPUT); > + inst->buffers[BUF_INPUT].min_count = iris_vpu_buf_count(inst, BUF_INPUT); > + inst->buffers[BUF_INPUT].size = fmt->fmt.pix_mp.plane_fmt[0].sizeimage; > + > + fmt->fmt.pix_mp.colorspace = f->fmt.pix_mp.colorspace; > + fmt->fmt.pix_mp.xfer_func = f->fmt.pix_mp.xfer_func; > + fmt->fmt.pix_mp.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc; > + fmt->fmt.pix_mp.quantization = f->fmt.pix_mp.quantization; > + > + output_fmt = inst->fmt_dst; > + output_fmt->fmt.pix_mp.colorspace = f->fmt.pix_mp.colorspace; > + output_fmt->fmt.pix_mp.xfer_func = f->fmt.pix_mp.xfer_func; > + output_fmt->fmt.pix_mp.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc; > + output_fmt->fmt.pix_mp.quantization = f->fmt.pix_mp.quantization; > + > + inst->crop.left = 0; > + inst->crop.top = 0; > + inst->crop.width = f->fmt.pix_mp.width; > + inst->crop.height = f->fmt.pix_mp.height; > + break; > + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: > + fmt = inst->fmt_dst; > + fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > + q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type); > + if (q->streaming) { > + f->fmt.pix_mp.height = inst->fmt_src->fmt.pix_mp.height; > + f->fmt.pix_mp.width = inst->fmt_src->fmt.pix_mp.width; > + } > + if (fmt->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) > + return -EINVAL; > + 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); > + fmt->fmt.pix_mp.num_planes = 1; > + fmt->fmt.pix_mp.plane_fmt[0].bytesperline = ALIGN(f->fmt.pix_mp.width, 128); > + 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; > + > + if (!q->streaming) { Use vb2_is_streaming(), don't refer to q->streaming directly. Please check if this is done anywhere else as well in the driver and use the helper function instead. > + inst->crop.top = 0; > + inst->crop.left = 0; > + inst->crop.width = f->fmt.pix_mp.width; > + inst->crop.height = f->fmt.pix_mp.height; A bigger question is why you do this? See the question about vb2_is_busy above: you shouldn't be able to get here at all if you are streaming since the vb2_is_busy check will catch that. > + } > + break; > + default: > + return -EINVAL; > + } > + memcpy(f, fmt, sizeof(*fmt)); > + > + return 0; > +} > diff --git a/drivers/media/platform/qcom/iris/iris_vdec.h b/drivers/media/platform/qcom/iris/iris_vdec.h > index 353b73b76230..85e93f33e9e7 100644 > --- a/drivers/media/platform/qcom/iris/iris_vdec.h > +++ b/drivers/media/platform/qcom/iris/iris_vdec.h > @@ -10,5 +10,7 @@ struct iris_inst; > > void iris_vdec_inst_init(struct iris_inst *inst); > void iris_vdec_inst_deinit(struct iris_inst *inst); > +int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f); > +int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f); > > #endif > diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c > index ab3b63171c1d..6707eb9917fe 100644 > --- a/drivers/media/platform/qcom/iris/iris_vidc.c > +++ b/drivers/media/platform/qcom/iris/iris_vidc.c > @@ -217,6 +217,48 @@ int iris_close(struct file *filp) > return 0; > } > > +static int iris_try_fmt_vid_mplane(struct file *filp, void *fh, struct v4l2_format *f) > +{ > + struct iris_inst *inst = iris_get_inst(filp, NULL); > + int ret; > + > + mutex_lock(&inst->lock); > + ret = iris_vdec_try_fmt(inst, f); > + mutex_unlock(&inst->lock); > + > + return ret; > +} > + > +static int iris_s_fmt_vid_mplane(struct file *filp, void *fh, struct v4l2_format *f) > +{ > + struct iris_inst *inst = iris_get_inst(filp, NULL); > + int ret; > + > + mutex_lock(&inst->lock); > + ret = iris_vdec_s_fmt(inst, f); > + mutex_unlock(&inst->lock); > + > + return ret; > +} > + > +static int iris_g_fmt_vid_mplane(struct file *filp, void *fh, struct v4l2_format *f) > +{ > + struct iris_inst *inst = iris_get_inst(filp, NULL); > + int ret = 0; > + > + mutex_lock(&inst->lock); > + if (V4L2_TYPE_IS_OUTPUT(f->type)) > + memcpy(f, inst->fmt_src, sizeof(*f)); Use: *f = *inst->fmt_src > + else if (V4L2_TYPE_IS_CAPTURE(f->type)) > + memcpy(f, inst->fmt_dst, sizeof(*f)); Ditto for fmt_dst > + else > + ret = -EINVAL; > + > + mutex_unlock(&inst->lock); > + > + return ret; > +} > + > static struct v4l2_file_operations iris_v4l2_file_ops = { > .owner = THIS_MODULE, > .open = iris_open, > @@ -231,6 +273,12 @@ static const struct vb2_ops iris_vb2_ops = { > }; > > static const struct v4l2_ioctl_ops iris_v4l2_ioctl_ops = { > + .vidioc_try_fmt_vid_cap_mplane = iris_try_fmt_vid_mplane, > + .vidioc_try_fmt_vid_out_mplane = iris_try_fmt_vid_mplane, > + .vidioc_s_fmt_vid_cap_mplane = iris_s_fmt_vid_mplane, > + .vidioc_s_fmt_vid_out_mplane = iris_s_fmt_vid_mplane, > + .vidioc_g_fmt_vid_cap_mplane = iris_g_fmt_vid_mplane, > + .vidioc_g_fmt_vid_out_mplane = iris_g_fmt_vid_mplane, > .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, > }; > > Regards, Hans
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 2ed50ad5d58b..11a2507fc35f 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -3,6 +3,8 @@ * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. */ +#include <media/v4l2-mem2mem.h> + #include "iris_buffer.h" #include "iris_instance.h" #include "iris_vdec.h" @@ -10,6 +12,7 @@ #define DEFAULT_WIDTH 320 #define DEFAULT_HEIGHT 240 +#define DEFAULT_CODEC_ALIGNMENT 16 void iris_vdec_inst_init(struct iris_inst *inst) { @@ -54,3 +57,122 @@ void iris_vdec_inst_deinit(struct iris_inst *inst) kfree(inst->fmt_dst); kfree(inst->fmt_src); } + +int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f) +{ + struct v4l2_pix_format_mplane *pixmp = &f->fmt.pix_mp; + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx; + struct v4l2_format *f_inst; + struct vb2_queue *src_q; + + memset(pixmp->reserved, 0, sizeof(pixmp->reserved)); + switch (f->type) { + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_H264) { + f_inst = inst->fmt_src; + f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; + f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; + f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat; + } + break; + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) { + f_inst = inst->fmt_dst; + f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat; + f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; + f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; + } + + src_q = v4l2_m2m_get_src_vq(m2m_ctx); + if (vb2_is_streaming(src_q)) { + f_inst = inst->fmt_src; + f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height; + f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width; + } + break; + default: + return -EINVAL; + } + + if (pixmp->field == V4L2_FIELD_ANY) + pixmp->field = V4L2_FIELD_NONE; + + pixmp->num_planes = 1; + + return 0; +} + +int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f) +{ + struct v4l2_format *fmt, *output_fmt; + struct vb2_queue *q; + u32 codec_align; + + iris_vdec_try_fmt(inst, f); + + switch (f->type) { + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_H264) + return -EINVAL; + + fmt = inst->fmt_src; + fmt->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; + + codec_align = DEFAULT_CODEC_ALIGNMENT; + fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, codec_align); + 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 = 0; + fmt->fmt.pix_mp.plane_fmt[0].sizeimage = iris_get_buffer_size(inst, BUF_INPUT); + inst->buffers[BUF_INPUT].min_count = iris_vpu_buf_count(inst, BUF_INPUT); + inst->buffers[BUF_INPUT].size = fmt->fmt.pix_mp.plane_fmt[0].sizeimage; + + fmt->fmt.pix_mp.colorspace = f->fmt.pix_mp.colorspace; + fmt->fmt.pix_mp.xfer_func = f->fmt.pix_mp.xfer_func; + fmt->fmt.pix_mp.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc; + fmt->fmt.pix_mp.quantization = f->fmt.pix_mp.quantization; + + output_fmt = inst->fmt_dst; + output_fmt->fmt.pix_mp.colorspace = f->fmt.pix_mp.colorspace; + output_fmt->fmt.pix_mp.xfer_func = f->fmt.pix_mp.xfer_func; + output_fmt->fmt.pix_mp.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc; + output_fmt->fmt.pix_mp.quantization = f->fmt.pix_mp.quantization; + + inst->crop.left = 0; + inst->crop.top = 0; + inst->crop.width = f->fmt.pix_mp.width; + inst->crop.height = f->fmt.pix_mp.height; + break; + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + fmt = inst->fmt_dst; + fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; + q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type); + if (q->streaming) { + f->fmt.pix_mp.height = inst->fmt_src->fmt.pix_mp.height; + f->fmt.pix_mp.width = inst->fmt_src->fmt.pix_mp.width; + } + if (fmt->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) + return -EINVAL; + 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); + fmt->fmt.pix_mp.num_planes = 1; + fmt->fmt.pix_mp.plane_fmt[0].bytesperline = ALIGN(f->fmt.pix_mp.width, 128); + 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; + + if (!q->streaming) { + inst->crop.top = 0; + inst->crop.left = 0; + inst->crop.width = f->fmt.pix_mp.width; + inst->crop.height = f->fmt.pix_mp.height; + } + break; + default: + return -EINVAL; + } + memcpy(f, fmt, sizeof(*fmt)); + + return 0; +} diff --git a/drivers/media/platform/qcom/iris/iris_vdec.h b/drivers/media/platform/qcom/iris/iris_vdec.h index 353b73b76230..85e93f33e9e7 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.h +++ b/drivers/media/platform/qcom/iris/iris_vdec.h @@ -10,5 +10,7 @@ struct iris_inst; void iris_vdec_inst_init(struct iris_inst *inst); void iris_vdec_inst_deinit(struct iris_inst *inst); +int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f); +int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f); #endif diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c index ab3b63171c1d..6707eb9917fe 100644 --- a/drivers/media/platform/qcom/iris/iris_vidc.c +++ b/drivers/media/platform/qcom/iris/iris_vidc.c @@ -217,6 +217,48 @@ int iris_close(struct file *filp) return 0; } +static int iris_try_fmt_vid_mplane(struct file *filp, void *fh, struct v4l2_format *f) +{ + struct iris_inst *inst = iris_get_inst(filp, NULL); + int ret; + + mutex_lock(&inst->lock); + ret = iris_vdec_try_fmt(inst, f); + mutex_unlock(&inst->lock); + + return ret; +} + +static int iris_s_fmt_vid_mplane(struct file *filp, void *fh, struct v4l2_format *f) +{ + struct iris_inst *inst = iris_get_inst(filp, NULL); + int ret; + + mutex_lock(&inst->lock); + ret = iris_vdec_s_fmt(inst, f); + mutex_unlock(&inst->lock); + + return ret; +} + +static int iris_g_fmt_vid_mplane(struct file *filp, void *fh, struct v4l2_format *f) +{ + struct iris_inst *inst = iris_get_inst(filp, NULL); + int ret = 0; + + mutex_lock(&inst->lock); + if (V4L2_TYPE_IS_OUTPUT(f->type)) + memcpy(f, inst->fmt_src, sizeof(*f)); + else if (V4L2_TYPE_IS_CAPTURE(f->type)) + memcpy(f, inst->fmt_dst, sizeof(*f)); + else + ret = -EINVAL; + + mutex_unlock(&inst->lock); + + return ret; +} + static struct v4l2_file_operations iris_v4l2_file_ops = { .owner = THIS_MODULE, .open = iris_open, @@ -231,6 +273,12 @@ static const struct vb2_ops iris_vb2_ops = { }; static const struct v4l2_ioctl_ops iris_v4l2_ioctl_ops = { + .vidioc_try_fmt_vid_cap_mplane = iris_try_fmt_vid_mplane, + .vidioc_try_fmt_vid_out_mplane = iris_try_fmt_vid_mplane, + .vidioc_s_fmt_vid_cap_mplane = iris_s_fmt_vid_mplane, + .vidioc_s_fmt_vid_out_mplane = iris_s_fmt_vid_mplane, + .vidioc_g_fmt_vid_cap_mplane = iris_g_fmt_vid_mplane, + .vidioc_g_fmt_vid_out_mplane = iris_g_fmt_vid_mplane, .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, };