Message ID | 20220706075454.15569-1-moudy.ho@mediatek.com (mailing list archive) |
---|---|
Headers | show |
Series | media: mediatek: support mdp3 on mt8183 platform | expand |
Hi Moudy, Some comments below: On 7/6/22 09:54, Moudy Ho wrote: > This patch adds driver for Mediatek's Media Data Path ver.3 (MDP3). > It provides the following functions: > color transform, format conversion, resize, crop, rotate, flip > and additional image quality enhancement. > > The MDP3 driver is mainly used for Google Chromebook products to > import the new architecture to set the HW settings as shown below: > User -> V4L2 framework > -> MDP3 driver -> SCP (setting calculations) > -> MDP3 driver -> CMDQ (GCE driver) -> HW > > Each modules' related operation control is sited in mtk-mdp3-comp.c > Each modules' register table is defined in file with "mdp_reg_" prefix > GCE related API, operation control sited in mtk-mdp3-cmdq.c > V4L2 m2m device functions are implemented in mtk-mdp3-m2m.c > Probe, power, suspend/resume, system level functions are defined in > mtk-mdp3-core.c > > v4l2-compliance 1.22.1, 32 bits, 32-bit time_t > > Compliance test for mtk-mdp3 device /dev/video2: > > Driver Info: > Driver name : mtk-mdp3 > Card type : 14001000.mdp3-rdma0 Card type is expected to be a human readable name, and that's not the case here. > Bus info : platform:mtk-mdp3 <snip> > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c > new file mode 100644 > index 000000000000..9d2b8acc303f > --- /dev/null > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c > @@ -0,0 +1,769 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022 MediaTek Inc. > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com> > + */ > + > +#include <linux/platform_device.h> > +#include <media/v4l2-ioctl.h> > +#include <media/v4l2-event.h> > +#include <media/videobuf2-dma-contig.h> > +#include "mtk-mdp3-m2m.h" > + > +static inline struct mdp_m2m_ctx *fh_to_ctx(struct v4l2_fh *fh) > +{ > + return container_of(fh, struct mdp_m2m_ctx, fh); > +} > + > +static inline struct mdp_m2m_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl) > +{ > + return container_of(ctrl->handler, struct mdp_m2m_ctx, ctrl_handler); > +} > + > +static inline struct mdp_frame *ctx_get_frame(struct mdp_m2m_ctx *ctx, > + enum v4l2_buf_type type) > +{ > + if (V4L2_TYPE_IS_OUTPUT(type)) > + return &ctx->curr_param.output; > + else > + return &ctx->curr_param.captures[0]; > +} > + > +static void mdp_m2m_ctx_set_state(struct mdp_m2m_ctx *ctx, u32 state) > +{ > + atomic_or(state, &ctx->curr_param.state); > +} > + > +static bool mdp_m2m_ctx_is_state_set(struct mdp_m2m_ctx *ctx, u32 mask) > +{ > + bool ret; > + > + ret = ((atomic_read(&ctx->curr_param.state) & mask) == mask); This can just do 'return' so you can drop the 'ret' variable. > + > + return ret; > +} > + > +static void mdp_m2m_process_done(void *priv, int vb_state) > +{ > + struct mdp_m2m_ctx *ctx = priv; > + struct vb2_v4l2_buffer *src_vbuf, *dst_vbuf; > + > + src_vbuf = (struct vb2_v4l2_buffer *) > + v4l2_m2m_src_buf_remove(ctx->m2m_ctx); > + dst_vbuf = (struct vb2_v4l2_buffer *) > + v4l2_m2m_dst_buf_remove(ctx->m2m_ctx); > + ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC]; > + src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++; > + dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++; > + v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true); > + > + v4l2_m2m_buf_done(src_vbuf, vb_state); > + v4l2_m2m_buf_done(dst_vbuf, vb_state); > + v4l2_m2m_job_finish(ctx->mdp_dev->m2m_dev, ctx->m2m_ctx); > +} > + > +static void mdp_m2m_device_run(void *priv) > +{ > + struct mdp_m2m_ctx *ctx = priv; > + struct mdp_frame *frame; > + struct vb2_v4l2_buffer *src_vb, *dst_vb; > + struct img_ipi_frameparam param = {0}; > + struct mdp_cmdq_param task = {0}; Just use '= {}', it's cleaner. > + enum vb2_buffer_state vb_state = VB2_BUF_STATE_ERROR; > + int ret; > + > + if (mdp_m2m_ctx_is_state_set(ctx, MDP_M2M_CTX_ERROR)) { > + dev_err(&ctx->mdp_dev->pdev->dev, > + "mdp_m2m_ctx is in error state\n"); > + goto worker_end; > + } > + > + param.frame_no = ctx->curr_param.frame_no; > + param.type = ctx->curr_param.type; > + param.num_inputs = 1; > + param.num_outputs = 1; > + > + frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); > + src_vb = v4l2_m2m_next_src_buf(ctx->m2m_ctx); > + mdp_set_src_config(¶m.inputs[0], frame, &src_vb->vb2_buf); > + > + frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + dst_vb = v4l2_m2m_next_dst_buf(ctx->m2m_ctx); > + mdp_set_dst_config(¶m.outputs[0], frame, &dst_vb->vb2_buf); > + > + ret = mdp_vpu_process(&ctx->vpu, ¶m); > + if (ret) { > + dev_err(&ctx->mdp_dev->pdev->dev, > + "VPU MDP process failed: %d\n", ret); > + goto worker_end; > + } > + > + task.config = ctx->vpu.config; > + task.param = ¶m; > + task.composes[0] = &frame->compose; > + task.cmdq_cb = NULL; > + task.cb_data = NULL; > + task.mdp_ctx = ctx; > + > + ret = mdp_cmdq_send(ctx->mdp_dev, &task); > + if (ret) { > + dev_err(&ctx->mdp_dev->pdev->dev, > + "CMDQ sendtask failed: %d\n", ret); > + goto worker_end; > + } > + > + return; > + > +worker_end: > + mdp_m2m_process_done(ctx, vb_state); > +} > + > +static int mdp_m2m_start_streaming(struct vb2_queue *q, unsigned int count) > +{ > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q); > + > + ctx->frame_count[MDP_M2M_SRC] = 0; > + ctx->frame_count[MDP_M2M_DST] = 0; Don't set both, check which queue it is first. > + > + return 0; > +} > + > +static struct vb2_v4l2_buffer *mdp_m2m_buf_remove(struct mdp_m2m_ctx *ctx, > + unsigned int type) > +{ > + if (V4L2_TYPE_IS_OUTPUT(type)) > + return (struct vb2_v4l2_buffer *) > + v4l2_m2m_src_buf_remove(ctx->m2m_ctx); > + else > + return (struct vb2_v4l2_buffer *) > + v4l2_m2m_dst_buf_remove(ctx->m2m_ctx); > +} > + > +static void mdp_m2m_stop_streaming(struct vb2_queue *q) > +{ > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q); > + struct vb2_v4l2_buffer *vb; > + > + vb = mdp_m2m_buf_remove(ctx, q->type); > + while (vb) { > + v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR); > + vb = mdp_m2m_buf_remove(ctx, q->type); > + } > +} > + > +static int mdp_m2m_queue_setup(struct vb2_queue *q, > + unsigned int *num_buffers, > + unsigned int *num_planes, unsigned int sizes[], > + struct device *alloc_devs[]) > +{ > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q); > + struct v4l2_pix_format_mplane *pix_mp; > + struct device *dev = &ctx->mdp_dev->pdev->dev; > + u32 i; > + > + pix_mp = &ctx_get_frame(ctx, q->type)->format.fmt.pix_mp; > + > + /* from VIDIOC_CREATE_BUFS */ > + if (*num_planes) { > + if (*num_planes != pix_mp->num_planes) > + return -EINVAL; > + for (i = 0; i < pix_mp->num_planes; ++i) > + if (sizes[i] < pix_mp->plane_fmt[i].sizeimage) > + return -EINVAL; > + } else {/* from VIDIOC_REQBUFS */ > + *num_planes = pix_mp->num_planes; > + for (i = 0; i < pix_mp->num_planes; ++i) > + sizes[i] = pix_mp->plane_fmt[i].sizeimage; > + } > + > + dev_info(dev, "[%d] type:%d, planes:%u, buffers:%u, size:%u,%u,%u", This should be dropped or changed to dev_dbg. You don't want logging when doing normal things, that will just spam the kernel log. > + ctx->id, q->type, *num_planes, *num_buffers, > + sizes[0], sizes[1], sizes[2]); > + return 0; > +} > + > +static int mdp_m2m_buf_prepare(struct vb2_buffer *vb) > +{ > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > + struct v4l2_pix_format_mplane *pix_mp; > + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb); > + u32 i; > + > + v4l2_buf->field = V4L2_FIELD_NONE; > + > + if (!V4L2_TYPE_IS_OUTPUT(vb->type)) { > + pix_mp = &ctx_get_frame(ctx, vb->type)->format.fmt.pix_mp; > + for (i = 0; i < pix_mp->num_planes; ++i) { > + vb2_set_plane_payload(vb, i, > + pix_mp->plane_fmt[i].sizeimage); > + } > + } > + return 0; > +} > + > +static int mdp_m2m_buf_out_validate(struct vb2_buffer *vb) > +{ > + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb); > + > + v4l2_buf->field = V4L2_FIELD_NONE; > + > + return 0; > +} > + > +static void mdp_m2m_buf_queue(struct vb2_buffer *vb) > +{ > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb); > + > + v4l2_buf->field = V4L2_FIELD_NONE; > + > + v4l2_m2m_buf_queue(ctx->m2m_ctx, to_vb2_v4l2_buffer(vb)); > +} > + > +static const struct vb2_ops mdp_m2m_qops = { > + .queue_setup = mdp_m2m_queue_setup, > + .wait_prepare = vb2_ops_wait_prepare, > + .wait_finish = vb2_ops_wait_finish, > + .buf_prepare = mdp_m2m_buf_prepare, > + .start_streaming = mdp_m2m_start_streaming, > + .stop_streaming = mdp_m2m_stop_streaming, > + .buf_queue = mdp_m2m_buf_queue, > + .buf_out_validate = mdp_m2m_buf_out_validate, > +}; > + > +static int mdp_m2m_querycap(struct file *file, void *fh, > + struct v4l2_capability *cap) > +{ > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > + > + strscpy(cap->driver, MDP_MODULE_NAME, sizeof(cap->driver)); > + strscpy(cap->card, ctx->mdp_dev->pdev->name, sizeof(cap->card)); As mentioned at the top, this is not a suitable name for cap->card. > + strscpy(cap->bus_info, "platform:mtk-mdp3", sizeof(cap->bus_info)); > + > + return 0; > +} > + > +static int mdp_m2m_enum_fmt_mplane(struct file *file, void *fh, > + struct v4l2_fmtdesc *f) > +{ > + return mdp_enum_fmt_mplane(f); > +} > + > +static int mdp_m2m_g_fmt_mplane(struct file *file, void *fh, > + struct v4l2_format *f) > +{ > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > + struct mdp_frame *frame; > + struct v4l2_pix_format_mplane *pix_mp; > + struct device *dev = &ctx->mdp_dev->pdev->dev; > + > + frame = ctx_get_frame(ctx, f->type); > + *f = frame->format; > + pix_mp = &f->fmt.pix_mp; > + pix_mp->colorspace = ctx->curr_param.colorspace; > + pix_mp->xfer_func = ctx->curr_param.xfer_func; > + pix_mp->ycbcr_enc = ctx->curr_param.ycbcr_enc; > + pix_mp->quantization = ctx->curr_param.quant; > + > + dev_info(dev, "[%d] type:%d, frame:%ux%u colorspace=%d", ctx->id, f->type, > + f->fmt.pix_mp.width, f->fmt.pix_mp.height, > + f->fmt.pix_mp.colorspace); Drop this, you're returning this info to userspace anyway, so why spam the kernel log? > + return 0; > +} > + > +static int mdp_m2m_s_fmt_mplane(struct file *file, void *fh, > + struct v4l2_format *f) > +{ > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > + struct mdp_frame *frame = ctx_get_frame(ctx, f->type); > + struct mdp_frame *capture; > + const struct mdp_format *fmt; > + struct vb2_queue *vq; > + struct device *dev = &ctx->mdp_dev->pdev->dev; > + > + dev_info(dev, "[%d] type:%d", ctx->id, f->type); dev_dbg. If dev_info is used elsewhere in similar situations, then replace that with dev_dbg as well. Regular usage of this driver must not produce kernel logging. > + > + fmt = mdp_try_fmt_mplane(f, &ctx->curr_param, ctx->id); > + if (!fmt) { > + dev_info(dev, "[%d] try_fmt failed, type:%d", ctx->id, f->type); Drop this, the error code says enough. > + return -EINVAL; > + } > + > + vq = v4l2_m2m_get_vq(ctx->m2m_ctx, f->type); > + if (vb2_is_streaming(vq)) { This must be vb2_is_busy(): changing formats is prohibited once buffers are allocated (vb2_is_busy() is true in that case). > + dev_info(&ctx->mdp_dev->pdev->dev, "Queue %d busy\n", f->type); Drop this, the error code says enough. > + return -EBUSY; > + } > + > + frame->format = *f; > + frame->mdp_fmt = fmt; > + frame->ycbcr_prof = mdp_map_ycbcr_prof_mplane(f, fmt->mdp_color); > + frame->usage = V4L2_TYPE_IS_OUTPUT(f->type) ? > + MDP_BUFFER_USAGE_HW_READ : MDP_BUFFER_USAGE_MDP; > + > + capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + if (V4L2_TYPE_IS_OUTPUT(f->type)) { > + capture->crop.c.left = 0; > + capture->crop.c.top = 0; > + capture->crop.c.width = f->fmt.pix_mp.width; > + capture->crop.c.height = f->fmt.pix_mp.height; > + ctx->curr_param.colorspace = f->fmt.pix_mp.colorspace; > + ctx->curr_param.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc; > + ctx->curr_param.quant = f->fmt.pix_mp.quantization; > + ctx->curr_param.xfer_func = f->fmt.pix_mp.xfer_func; > + } else { > + capture->compose.left = 0; > + capture->compose.top = 0; > + capture->compose.width = f->fmt.pix_mp.width; > + capture->compose.height = f->fmt.pix_mp.height; > + } > + > + ctx->frame_count[MDP_M2M_SRC] = 0; > + ctx->frame_count[MDP_M2M_DST] = 0; Changing the format doesn't mean you need to zero this. Just drop these two line. > + > + dev_info(dev, "[%d] type:%d, frame:%ux%u", ctx->id, f->type, > + f->fmt.pix_mp.width, f->fmt.pix_mp.height); Drop this. > + return 0; > +} > + > +static int mdp_m2m_try_fmt_mplane(struct file *file, void *fh, > + struct v4l2_format *f) > +{ > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > + > + if (!mdp_try_fmt_mplane(f, &ctx->curr_param, ctx->id)) > + return -EINVAL; > + > + return 0; > +} > + > +static int mdp_m2m_streamon(struct file *file, void *fh, > + enum v4l2_buf_type type) > +{ > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > + struct mdp_frame *capture; > + int ret; > + bool out_streaming, cap_streaming; > + > + capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + out_streaming = ctx->m2m_ctx->out_q_ctx.q.streaming; > + cap_streaming = ctx->m2m_ctx->cap_q_ctx.q.streaming; Use vb2_is_streaming() rather than accessing q.streaming directly. > + > + /* Check to see if scaling ratio is within supported range */ > + if ((V4L2_TYPE_IS_OUTPUT(type) && cap_streaming) || > + (!V4L2_TYPE_IS_OUTPUT(type) && out_streaming)) { > + ret = mdp_check_scaling_ratio(&capture->crop.c, > + &capture->compose, > + capture->rotation, > + ctx->curr_param.limit); > + if (ret) { > + dev_info(&ctx->mdp_dev->pdev->dev, > + "Out of scaling range\n"); > + return ret; > + } > + } > + > + if (!mdp_m2m_ctx_is_state_set(ctx, MDP_VPU_INIT)) { > + ret = mdp_vpu_get_locked(ctx->mdp_dev); > + if (ret) > + return ret; > + > + ret = mdp_vpu_ctx_init(&ctx->vpu, &ctx->mdp_dev->vpu, > + MDP_DEV_M2M); > + if (ret) { > + dev_err(&ctx->mdp_dev->pdev->dev, > + "VPU init failed %d\n", ret); > + return -EINVAL; > + } > + mdp_m2m_ctx_set_state(ctx, MDP_VPU_INIT); > + } > + > + return v4l2_m2m_streamon(file, ctx->m2m_ctx, type); > +} There really is no reason to override streamon. You can also do this in start_streaming(). I recommend moving these tests there. Internally streamon() will call start_streaming(), so any error you return in that callback function will be the error return of the streamon as well. > + > +static int mdp_m2m_g_selection(struct file *file, void *fh, > + struct v4l2_selection *s) > +{ > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > + struct mdp_frame *frame; > + struct device *dev = &ctx->mdp_dev->pdev->dev; > + bool valid = false; > + > + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) > + valid = mdp_target_is_crop(s->target); > + else if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > + valid = mdp_target_is_compose(s->target); > + > + if (!valid) { > + dev_err(dev, "[%s:%d] invalid type:%u target:%u", __func__, ctx->id, > + s->type, s->target); Drop this. User errors should never cause spamming in the kernel log. Please check your code: you can use dev_dbg to help debugging, but dev_info should be used very sparingly (typically only in probe() and remove()), and dev_warn/err only for hardware/driver warnings/errors, and never due to userspace inputs. > + return -EINVAL; > + } > + > + switch (s->target) { > + case V4L2_SEL_TGT_CROP: > + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { > + frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + s->r = frame->crop.c; > + return 0; > + } It's easier to read if you swap the tests: if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) return -EINVAL; frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); s->r = frame->crop.c; return 0; Same below. > + break; > + case V4L2_SEL_TGT_COMPOSE: > + if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { > + frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + s->r = frame->compose; > + return 0; > + } > + break; > + case V4L2_SEL_TGT_CROP_DEFAULT: > + case V4L2_SEL_TGT_CROP_BOUNDS: > + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { > + frame = ctx_get_frame(ctx, s->type); > + s->r.left = 0; > + s->r.top = 0; > + s->r.width = frame->format.fmt.pix_mp.width; > + s->r.height = frame->format.fmt.pix_mp.height; > + return 0; > + } > + break; > + case V4L2_SEL_TGT_COMPOSE_DEFAULT: > + case V4L2_SEL_TGT_COMPOSE_BOUNDS: > + if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { > + frame = ctx_get_frame(ctx, s->type); > + s->r.left = 0; > + s->r.top = 0; > + s->r.width = frame->format.fmt.pix_mp.width; > + s->r.height = frame->format.fmt.pix_mp.height; > + return 0; > + } > + break; > + } > + return -EINVAL; > +} > + > +static int mdp_m2m_s_selection(struct file *file, void *fh, > + struct v4l2_selection *s) > +{ > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > + struct mdp_frame *frame = ctx_get_frame(ctx, s->type); > + struct mdp_frame *capture; > + struct v4l2_rect r; > + struct device *dev = &ctx->mdp_dev->pdev->dev; > + bool valid = false; > + int ret; > + > + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) > + valid = (s->target == V4L2_SEL_TGT_CROP); > + else if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > + valid = (s->target == V4L2_SEL_TGT_COMPOSE); > + > + if (!valid) { > + dev_err(dev, "[%s:%d] invalid type:%u target:%u", __func__, ctx->id, > + s->type, s->target); > + return -EINVAL; > + } > + > + ret = mdp_try_crop(ctx, &r, s, frame); > + if (ret) > + return ret; > + capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + > + if (mdp_target_is_crop(s->target)) > + capture->crop.c = r; > + else > + capture->compose = r; > + > + s->r = r; > + memset(s->reserved, 0, sizeof(s->reserved)); No need for this memset, the v4l2 core will clear this for you. > + > + ctx->frame_count[MDP_M2M_SRC] = 0; > + ctx->frame_count[MDP_M2M_DST] = 0; Drop this! > + return 0; > +} > + > +static const struct v4l2_ioctl_ops mdp_m2m_ioctl_ops = { > + .vidioc_querycap = mdp_m2m_querycap, > + .vidioc_enum_fmt_vid_cap = mdp_m2m_enum_fmt_mplane, > + .vidioc_enum_fmt_vid_out = mdp_m2m_enum_fmt_mplane, > + .vidioc_g_fmt_vid_cap_mplane = mdp_m2m_g_fmt_mplane, > + .vidioc_g_fmt_vid_out_mplane = mdp_m2m_g_fmt_mplane, > + .vidioc_s_fmt_vid_cap_mplane = mdp_m2m_s_fmt_mplane, > + .vidioc_s_fmt_vid_out_mplane = mdp_m2m_s_fmt_mplane, > + .vidioc_try_fmt_vid_cap_mplane = mdp_m2m_try_fmt_mplane, > + .vidioc_try_fmt_vid_out_mplane = mdp_m2m_try_fmt_mplane, > + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, > + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf, > + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf, > + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf, > + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf, > + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs, > + .vidioc_streamon = mdp_m2m_streamon, > + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff, > + .vidioc_g_selection = mdp_m2m_g_selection, > + .vidioc_s_selection = mdp_m2m_s_selection, > + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, > + .vidioc_unsubscribe_event = v4l2_event_unsubscribe, > +}; > + > +static int mdp_m2m_queue_init(void *priv, > + struct vb2_queue *src_vq, > + struct vb2_queue *dst_vq) > +{ > + struct mdp_m2m_ctx *ctx = priv; > + int ret; > + > + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; > + src_vq->io_modes = VB2_MMAP | VB2_DMABUF; > + src_vq->ops = &mdp_m2m_qops; > + src_vq->mem_ops = &vb2_dma_contig_memops; > + src_vq->drv_priv = ctx; > + src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); > + src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; > + src_vq->dev = &ctx->mdp_dev->pdev->dev; > + src_vq->lock = &ctx->ctx_lock; > + > + ret = vb2_queue_init(src_vq); > + if (ret) > + return ret; > + > + dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF; > + dst_vq->ops = &mdp_m2m_qops; > + dst_vq->mem_ops = &vb2_dma_contig_memops; > + dst_vq->drv_priv = ctx; > + dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); > + dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; > + dst_vq->dev = &ctx->mdp_dev->pdev->dev; > + dst_vq->lock = &ctx->ctx_lock; > + > + return vb2_queue_init(dst_vq); > +} > + > +static int mdp_m2m_s_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct mdp_m2m_ctx *ctx = ctrl_to_ctx(ctrl); > + struct mdp_frame *capture; > + > + if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE) > + return 0; Why this test? As far as I can tell this flag is never set, so there is no need to check against it. > + > + capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + switch (ctrl->id) { > + case V4L2_CID_HFLIP: > + capture->hflip = ctrl->val; > + break; > + case V4L2_CID_VFLIP: > + capture->vflip = ctrl->val; > + break; > + case V4L2_CID_ROTATE: > + capture->rotation = ctrl->val; > + break; > + } > + > + return 0; > +} > + > +static const struct v4l2_ctrl_ops mdp_m2m_ctrl_ops = { > + .s_ctrl = mdp_m2m_s_ctrl, > +}; > + > +static int mdp_m2m_ctrls_create(struct mdp_m2m_ctx *ctx) > +{ > + v4l2_ctrl_handler_init(&ctx->ctrl_handler, MDP_MAX_CTRLS); > + ctx->ctrls.hflip = v4l2_ctrl_new_std(&ctx->ctrl_handler, > + &mdp_m2m_ctrl_ops, V4L2_CID_HFLIP, > + 0, 1, 1, 0); > + ctx->ctrls.vflip = v4l2_ctrl_new_std(&ctx->ctrl_handler, > + &mdp_m2m_ctrl_ops, V4L2_CID_VFLIP, > + 0, 1, 1, 0); > + ctx->ctrls.rotate = v4l2_ctrl_new_std(&ctx->ctrl_handler, > + &mdp_m2m_ctrl_ops, > + V4L2_CID_ROTATE, 0, 270, 90, 0); > + > + if (ctx->ctrl_handler.error) { > + int err = ctx->ctrl_handler.error; > + > + v4l2_ctrl_handler_free(&ctx->ctrl_handler); > + dev_err(&ctx->mdp_dev->pdev->dev, > + "Failed to register controls\n"); > + return err; > + } > + return 0; > +} > + > +static int mdp_m2m_open(struct file *file) > +{ > + struct video_device *vdev = video_devdata(file); > + struct mdp_dev *mdp = video_get_drvdata(vdev); > + struct mdp_m2m_ctx *ctx; > + struct device *dev = &mdp->pdev->dev; > + int ret; > + struct v4l2_format default_format; Just add '= {}' to avoid the memset later on. > + > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + if (mutex_lock_interruptible(&mdp->m2m_lock)) { > + ret = -ERESTARTSYS; > + goto err_free_ctx; > + } > + > + ctx->id = ida_alloc(&mdp->mdp_ida, GFP_KERNEL); > + ctx->mdp_dev = mdp; > + > + v4l2_fh_init(&ctx->fh, vdev); > + vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; This doesn't belong in open(). It is already done when the video device is registered. > + file->private_data = &ctx->fh; > + ret = mdp_m2m_ctrls_create(ctx); > + if (ret) > + goto err_exit_fh; > + > + /* Use separate control handler per file handle */ > + ctx->fh.ctrl_handler = &ctx->ctrl_handler; > + v4l2_fh_add(&ctx->fh); > + > + mutex_init(&ctx->ctx_lock); > + ctx->m2m_ctx = v4l2_m2m_ctx_init(mdp->m2m_dev, ctx, mdp_m2m_queue_init); > + if (IS_ERR(ctx->m2m_ctx)) { > + dev_err(dev, "Failed to initialize m2m context\n"); > + ret = PTR_ERR(ctx->m2m_ctx); > + goto err_release_handler; > + } > + ctx->fh.m2m_ctx = ctx->m2m_ctx; > + > + ctx->curr_param.ctx = ctx; > + ret = mdp_frameparam_init(&ctx->curr_param); > + if (ret) { > + dev_err(dev, "Failed to initialize mdp parameter\n"); > + goto err_release_m2m_ctx; > + } > + > + mutex_unlock(&mdp->m2m_lock); > + > + /* Default format */ > + memset(&default_format, 0, sizeof(default_format)); This can be dropped if default_format is inited with = {}. > + default_format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; > + default_format.fmt.pix_mp.width = 32; > + default_format.fmt.pix_mp.height = 32; > + default_format.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_YUV420M; > + mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format); > + default_format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > + mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format); > + > + dev_dbg(dev, "%s:[%d]", __func__, ctx->id); > + > + return 0; > + > +err_release_m2m_ctx: > + v4l2_m2m_ctx_release(ctx->m2m_ctx); > +err_release_handler: > + v4l2_ctrl_handler_free(&ctx->ctrl_handler); > + v4l2_fh_del(&ctx->fh); > +err_exit_fh: > + v4l2_fh_exit(&ctx->fh); > + mutex_unlock(&mdp->m2m_lock); > +err_free_ctx: > + kfree(ctx); > + > + return ret; > +} > + > +static int mdp_m2m_release(struct file *file) > +{ > + struct mdp_m2m_ctx *ctx = fh_to_ctx(file->private_data); > + struct mdp_dev *mdp = video_drvdata(file); > + struct device *dev = &mdp->pdev->dev; > + > + mutex_lock(&mdp->m2m_lock); > + v4l2_m2m_ctx_release(ctx->m2m_ctx); > + if (mdp_m2m_ctx_is_state_set(ctx, MDP_VPU_INIT)) { > + mdp_vpu_ctx_deinit(&ctx->vpu); > + mdp_vpu_put_locked(mdp); > + } > + > + v4l2_ctrl_handler_free(&ctx->ctrl_handler); > + v4l2_fh_del(&ctx->fh); > + v4l2_fh_exit(&ctx->fh); > + ida_free(&mdp->mdp_ida, ctx->id); > + mutex_unlock(&mdp->m2m_lock); > + > + dev_dbg(dev, "%s:[%d]", __func__, ctx->id); > + kfree(ctx); > + > + return 0; > +} > + > +static const struct v4l2_file_operations mdp_m2m_fops = { > + .owner = THIS_MODULE, > + .poll = v4l2_m2m_fop_poll, > + .unlocked_ioctl = video_ioctl2, > + .mmap = v4l2_m2m_fop_mmap, > + .open = mdp_m2m_open, > + .release = mdp_m2m_release, > +}; > + > +static const struct v4l2_m2m_ops mdp_m2m_ops = { > + .device_run = mdp_m2m_device_run, > +}; > + > +int mdp_m2m_device_register(struct mdp_dev *mdp) > +{ > + struct device *dev = &mdp->pdev->dev; > + int ret = 0; > + > + mdp->m2m_vdev = video_device_alloc(); > + if (!mdp->m2m_vdev) { > + dev_err(dev, "Failed to allocate video device\n"); > + ret = -ENOMEM; > + goto err_video_alloc; > + } > + mdp->m2m_vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | > + V4L2_CAP_STREAMING; > + mdp->m2m_vdev->fops = &mdp_m2m_fops; > + mdp->m2m_vdev->ioctl_ops = &mdp_m2m_ioctl_ops; > + mdp->m2m_vdev->release = mdp_video_device_release; > + mdp->m2m_vdev->lock = &mdp->m2m_lock; > + mdp->m2m_vdev->vfl_dir = VFL_DIR_M2M; > + mdp->m2m_vdev->v4l2_dev = &mdp->v4l2_dev; > + snprintf(mdp->m2m_vdev->name, sizeof(mdp->m2m_vdev->name), "%s:m2m", > + MDP_MODULE_NAME); > + video_set_drvdata(mdp->m2m_vdev, mdp); > + > + mdp->m2m_dev = v4l2_m2m_init(&mdp_m2m_ops); > + if (IS_ERR(mdp->m2m_dev)) { > + dev_err(dev, "Failed to initialize v4l2-m2m device\n"); > + ret = PTR_ERR(mdp->m2m_dev); > + goto err_m2m_init; > + } > + > + ret = video_register_device(mdp->m2m_vdev, VFL_TYPE_VIDEO, 2); Why 2? Just use -1 to pick the first free device number. > + if (ret) { > + dev_err(dev, "Failed to register video device\n"); > + goto err_video_register; > + } > + > + v4l2_info(&mdp->v4l2_dev, "Driver registered as /dev/video%d", > + mdp->m2m_vdev->num); > + return 0; > + > +err_video_register: > + v4l2_m2m_release(mdp->m2m_dev); > +err_m2m_init: > + video_device_release(mdp->m2m_vdev); > +err_video_alloc: > + > + return ret; > +} > + > +void mdp_m2m_device_unregister(struct mdp_dev *mdp) > +{ > + video_unregister_device(mdp->m2m_vdev); > +} > + > +void mdp_m2m_job_finish(struct mdp_m2m_ctx *ctx) > +{ > + enum vb2_buffer_state vb_state = VB2_BUF_STATE_DONE; > + > + mdp_m2m_process_done(ctx, vb_state); > +} > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h > new file mode 100644 > index 000000000000..61ddbaf1bf13 > --- /dev/null > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h > @@ -0,0 +1,48 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2022 MediaTek Inc. > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com> > + */ > + > +#ifndef __MTK_MDP3_M2M_H__ > +#define __MTK_MDP3_M2M_H__ > + > +#include <media/v4l2-ctrls.h> > +#include "mtk-mdp3-core.h" > +#include "mtk-mdp3-vpu.h" > +#include "mtk-mdp3-regs.h" > + > +#define MDP_MAX_CTRLS 10 > + > +enum { > + MDP_M2M_SRC = 0, > + MDP_M2M_DST = 1, > + MDP_M2M_MAX, > +}; > + > +struct mdp_m2m_ctrls { > + struct v4l2_ctrl *hflip; > + struct v4l2_ctrl *vflip; > + struct v4l2_ctrl *rotate; > +}; > + > +struct mdp_m2m_ctx { > + u32 id; > + struct mdp_dev *mdp_dev; > + struct v4l2_fh fh; > + struct v4l2_ctrl_handler ctrl_handler; > + struct mdp_m2m_ctrls ctrls; > + struct v4l2_m2m_ctx *m2m_ctx; > + struct mdp_vpu_ctx vpu; > + u32 frame_count[MDP_M2M_MAX]; > + > + struct mdp_frameparam curr_param; > + /* synchronization protect for mdp m2m context */ > + struct mutex ctx_lock; > +}; > + > +int mdp_m2m_device_register(struct mdp_dev *mdp); > +void mdp_m2m_device_unregister(struct mdp_dev *mdp); > +void mdp_m2m_job_finish(struct mdp_m2m_ctx *ctx); > + > +#endif /* __MTK_MDP3_M2M_H__ */ > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c > new file mode 100644 > index 000000000000..18874eb7851c > --- /dev/null > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c > @@ -0,0 +1,742 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022 MediaTek Inc. > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com> > + */ > + > +#include <media/v4l2-common.h> > +#include <media/videobuf2-v4l2.h> > +#include <media/videobuf2-dma-contig.h> > +#include "mtk-mdp3-core.h" > +#include "mtk-mdp3-regs.h" > +#include "mtk-mdp3-m2m.h" > + > +/* > + * All 10-bit related formats are not added in the basic format list, > + * please add the corresponding format settings before use. > + */ > +static const struct mdp_format mdp_formats[] = { > + { > + .pixelformat = V4L2_PIX_FMT_GREY, > + .mdp_color = MDP_COLOR_GREY, > + .depth = { 8 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_RGB565X, > + .mdp_color = MDP_COLOR_BGR565, > + .depth = { 16 }, > + .row_depth = { 16 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_RGB565, > + .mdp_color = MDP_COLOR_RGB565, > + .depth = { 16 }, > + .row_depth = { 16 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_RGB24, > + .mdp_color = MDP_COLOR_RGB888, > + .depth = { 24 }, > + .row_depth = { 24 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_BGR24, > + .mdp_color = MDP_COLOR_BGR888, > + .depth = { 24 }, > + .row_depth = { 24 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_ABGR32, > + .mdp_color = MDP_COLOR_BGRA8888, > + .depth = { 32 }, > + .row_depth = { 32 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_ARGB32, > + .mdp_color = MDP_COLOR_ARGB8888, > + .depth = { 32 }, > + .row_depth = { 32 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_UYVY, > + .mdp_color = MDP_COLOR_UYVY, > + .depth = { 16 }, > + .row_depth = { 16 }, > + .num_planes = 1, > + .walign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_VYUY, > + .mdp_color = MDP_COLOR_VYUY, > + .depth = { 16 }, > + .row_depth = { 16 }, > + .num_planes = 1, > + .walign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_YUYV, > + .mdp_color = MDP_COLOR_YUYV, > + .depth = { 16 }, > + .row_depth = { 16 }, > + .num_planes = 1, > + .walign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_YVYU, > + .mdp_color = MDP_COLOR_YVYU, > + .depth = { 16 }, > + .row_depth = { 16 }, > + .num_planes = 1, > + .walign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_YUV420, > + .mdp_color = MDP_COLOR_I420, > + .depth = { 12 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .walign = 1, > + .halign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_YVU420, > + .mdp_color = MDP_COLOR_YV12, > + .depth = { 12 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .walign = 1, > + .halign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV12, > + .mdp_color = MDP_COLOR_NV12, > + .depth = { 12 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .walign = 1, > + .halign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV21, > + .mdp_color = MDP_COLOR_NV21, > + .depth = { 12 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .walign = 1, > + .halign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV16, > + .mdp_color = MDP_COLOR_NV16, > + .depth = { 16 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .walign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV61, > + .mdp_color = MDP_COLOR_NV61, > + .depth = { 16 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .walign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV24, > + .mdp_color = MDP_COLOR_NV24, > + .depth = { 24 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV42, > + .mdp_color = MDP_COLOR_NV42, > + .depth = { 24 }, > + .row_depth = { 8 }, > + .num_planes = 1, > + .flags = MDP_FMT_FLAG_OUTPUT, > + }, { > + .pixelformat = V4L2_PIX_FMT_MT21C, > + .mdp_color = MDP_COLOR_420_BLK_UFO, > + .depth = { 8, 4 }, > + .row_depth = { 8, 8 }, > + .num_planes = 2, > + .walign = 4, > + .halign = 5, > + .flags = MDP_FMT_FLAG_OUTPUT, > + }, { > + .pixelformat = V4L2_PIX_FMT_MM21, > + .mdp_color = MDP_COLOR_420_BLK, > + .depth = { 8, 4 }, > + .row_depth = { 8, 8 }, > + .num_planes = 2, > + .walign = 4, > + .halign = 5, > + .flags = MDP_FMT_FLAG_OUTPUT, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV12M, > + .mdp_color = MDP_COLOR_NV12, > + .depth = { 8, 4 }, > + .row_depth = { 8, 8 }, > + .num_planes = 2, > + .walign = 1, > + .halign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV21M, > + .mdp_color = MDP_COLOR_NV21, > + .depth = { 8, 4 }, > + .row_depth = { 8, 8 }, > + .num_planes = 2, > + .walign = 1, > + .halign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV16M, > + .mdp_color = MDP_COLOR_NV16, > + .depth = { 8, 8 }, > + .row_depth = { 8, 8 }, > + .num_planes = 2, > + .walign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT, > + }, { > + .pixelformat = V4L2_PIX_FMT_NV61M, > + .mdp_color = MDP_COLOR_NV61, > + .depth = { 8, 8 }, > + .row_depth = { 8, 8 }, > + .num_planes = 2, > + .walign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT, > + }, { > + .pixelformat = V4L2_PIX_FMT_YUV420M, > + .mdp_color = MDP_COLOR_I420, > + .depth = { 8, 2, 2 }, > + .row_depth = { 8, 4, 4 }, > + .num_planes = 3, > + .walign = 1, > + .halign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + }, { > + .pixelformat = V4L2_PIX_FMT_YVU420M, > + .mdp_color = MDP_COLOR_YV12, > + .depth = { 8, 2, 2 }, > + .row_depth = { 8, 4, 4 }, > + .num_planes = 3, > + .walign = 1, > + .halign = 1, > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > + } > +}; > + > +static const struct mdp_limit mdp_def_limit = { > + .out_limit = { > + .wmin = 16, > + .hmin = 16, > + .wmax = 8176, > + .hmax = 8176, > + }, > + .cap_limit = { > + .wmin = 2, > + .hmin = 2, > + .wmax = 8176, > + .hmax = 8176, > + }, > + .h_scale_up_max = 32, > + .v_scale_up_max = 32, > + .h_scale_down_max = 20, > + .v_scale_down_max = 128, > +}; > + > +static const struct mdp_format *mdp_find_fmt(u32 pixelformat, u32 type) > +{ > + u32 i, flag; > + > + flag = V4L2_TYPE_IS_OUTPUT(type) ? MDP_FMT_FLAG_OUTPUT : > + MDP_FMT_FLAG_CAPTURE; > + for (i = 0; i < ARRAY_SIZE(mdp_formats); ++i) { > + if (!(mdp_formats[i].flags & flag)) > + continue; > + if (mdp_formats[i].pixelformat == pixelformat) > + return &mdp_formats[i]; > + } > + return NULL; > +} > + > +static const struct mdp_format *mdp_find_fmt_by_index(u32 index, u32 type) > +{ > + u32 i, flag, num = 0; > + > + flag = V4L2_TYPE_IS_OUTPUT(type) ? MDP_FMT_FLAG_OUTPUT : > + MDP_FMT_FLAG_CAPTURE; > + for (i = 0; i < ARRAY_SIZE(mdp_formats); ++i) { > + if (!(mdp_formats[i].flags & flag)) > + continue; > + if (index == num) > + return &mdp_formats[i]; > + num++; > + } > + return NULL; > +} > + > +enum mdp_ycbcr_profile mdp_map_ycbcr_prof_mplane(struct v4l2_format *f, > + u32 mdp_color) > +{ > + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; > + > + if (MDP_COLOR_IS_RGB(mdp_color)) > + return MDP_YCBCR_PROFILE_FULL_BT601; > + > + switch (pix_mp->colorspace) { > + case V4L2_COLORSPACE_JPEG: > + return MDP_YCBCR_PROFILE_JPEG; > + case V4L2_COLORSPACE_REC709: > + case V4L2_COLORSPACE_DCI_P3: > + if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE) > + return MDP_YCBCR_PROFILE_FULL_BT709; > + return MDP_YCBCR_PROFILE_BT709; > + case V4L2_COLORSPACE_BT2020: > + if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE) > + return MDP_YCBCR_PROFILE_FULL_BT2020; > + return MDP_YCBCR_PROFILE_BT2020; > + default: > + if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE) > + return MDP_YCBCR_PROFILE_FULL_BT601; > + return MDP_YCBCR_PROFILE_BT601; > + } > +} > + > +static void mdp_bound_align_image(u32 *w, u32 *h, > + struct v4l2_frmsize_stepwise *s, > + unsigned int salign) > +{ > + unsigned int org_w, org_h; > + > + org_w = *w; > + org_h = *h; > + v4l_bound_align_image(w, s->min_width, s->max_width, s->step_width, > + h, s->min_height, s->max_height, s->step_height, > + salign); > + > + s->min_width = org_w; > + s->min_height = org_h; > + v4l2_apply_frmsize_constraints(w, h, s); > +} > + > +static int mdp_clamp_align(s32 *x, int min, int max, unsigned int align) > +{ > + unsigned int mask; > + > + if (min < 0 || max < 0) > + return -ERANGE; > + > + /* Bits that must be zero to be aligned */ > + mask = ~((1 << align) - 1); > + > + min = 0 ? 0 : ((min + ~mask) & mask); > + max = max & mask; > + if ((unsigned int)min > (unsigned int)max) > + return -ERANGE; > + > + /* Clamp to aligned min and max */ > + *x = clamp(*x, min, max); > + > + /* Round to nearest aligned value */ > + if (align) > + *x = (*x + (1 << (align - 1))) & mask; > + return 0; > +} > + > +int mdp_enum_fmt_mplane(struct v4l2_fmtdesc *f) > +{ > + const struct mdp_format *fmt; > + > + fmt = mdp_find_fmt_by_index(f->index, f->type); > + if (!fmt) > + return -EINVAL; > + > + f->pixelformat = fmt->pixelformat; > + return 0; > +} > + > +const struct mdp_format *mdp_try_fmt_mplane(struct v4l2_format *f, > + struct mdp_frameparam *param, > + u32 ctx_id) > +{ > + struct device *dev = ¶m->ctx->mdp_dev->pdev->dev; > + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; > + const struct mdp_format *fmt; > + const struct mdp_pix_limit *pix_limit; > + struct v4l2_frmsize_stepwise s; > + u32 org_w, org_h; > + unsigned int i; > + > + fmt = mdp_find_fmt(pix_mp->pixelformat, f->type); > + if (!fmt) { > + fmt = mdp_find_fmt_by_index(0, f->type); > + if (!fmt) { > + dev_dbg(dev, "%d: pixelformat %c%c%c%c invalid", ctx_id, > + (pix_mp->pixelformat & 0xff), > + (pix_mp->pixelformat >> 8) & 0xff, > + (pix_mp->pixelformat >> 16) & 0xff, > + (pix_mp->pixelformat >> 24) & 0xff); > + return NULL; > + } > + } > + > + pix_mp->field = V4L2_FIELD_NONE; > + pix_mp->flags = 0; > + pix_mp->pixelformat = fmt->pixelformat; > + if (!V4L2_TYPE_IS_OUTPUT(f->type)) { > + pix_mp->colorspace = param->colorspace; > + pix_mp->xfer_func = param->xfer_func; > + pix_mp->ycbcr_enc = param->ycbcr_enc; > + pix_mp->quantization = param->quant; > + } > + > + pix_limit = V4L2_TYPE_IS_OUTPUT(f->type) ? ¶m->limit->out_limit : > + ¶m->limit->cap_limit; > + s.min_width = pix_limit->wmin; > + s.max_width = pix_limit->wmax; > + s.step_width = fmt->walign; > + s.min_height = pix_limit->hmin; > + s.max_height = pix_limit->hmax; > + s.step_height = fmt->halign; > + org_w = pix_mp->width; > + org_h = pix_mp->height; > + > + mdp_bound_align_image(&pix_mp->width, &pix_mp->height, &s, fmt->salign); > + if (org_w != pix_mp->width || org_h != pix_mp->height) > + dev_dbg(dev, "%d: size change: %ux%u to %ux%u", ctx_id, > + org_w, org_h, pix_mp->width, pix_mp->height); > + > + if (pix_mp->num_planes && pix_mp->num_planes != fmt->num_planes) > + dev_dbg(dev, "%d num of planes change: %u to %u", ctx_id, > + pix_mp->num_planes, fmt->num_planes); > + pix_mp->num_planes = fmt->num_planes; > + > + for (i = 0; i < pix_mp->num_planes; ++i) { > + u32 min_bpl = (pix_mp->width * fmt->row_depth[i]) >> 3; > + u32 max_bpl = (pix_limit->wmax * fmt->row_depth[i]) >> 3; > + u32 bpl = pix_mp->plane_fmt[i].bytesperline; > + u32 min_si, max_si; > + u32 si = pix_mp->plane_fmt[i].sizeimage; > + > + bpl = clamp(bpl, min_bpl, max_bpl); > + pix_mp->plane_fmt[i].bytesperline = bpl; > + > + min_si = (bpl * pix_mp->height * fmt->depth[i]) / fmt->row_depth[i]; > + max_si = (bpl * s.max_height * fmt->depth[i]) / fmt->row_depth[i]; > + > + si = clamp(si, min_si, max_si); > + pix_mp->plane_fmt[i].sizeimage = si; > + > + dev_dbg(dev, "%d: p%u, bpl:%u [%u, %u], sizeimage:%u [%u, %u]", ctx_id, > + i, bpl, min_bpl, max_bpl, si, min_si, max_si); > + } > + > + return fmt; > +} > + > +static int mdp_clamp_start(s32 *x, int min, int max, unsigned int align, > + u32 flags) > +{ > + if (flags & V4L2_SEL_FLAG_GE) > + max = *x; > + if (flags & V4L2_SEL_FLAG_LE) > + min = *x; > + return mdp_clamp_align(x, min, max, align); > +} > + > +static int mdp_clamp_end(s32 *x, int min, int max, unsigned int align, > + u32 flags) > +{ > + if (flags & V4L2_SEL_FLAG_GE) > + min = *x; > + if (flags & V4L2_SEL_FLAG_LE) > + max = *x; > + return mdp_clamp_align(x, min, max, align); > +} > + > +int mdp_try_crop(struct mdp_m2m_ctx *ctx, struct v4l2_rect *r, > + const struct v4l2_selection *s, struct mdp_frame *frame) > +{ > + struct device *dev = &ctx->mdp_dev->pdev->dev; > + s32 left, top, right, bottom; > + u32 framew, frameh, walign, halign; > + int ret; > + > + dev_dbg(dev, "%d target:%d, set:(%d,%d) %ux%u", ctx->id, > + s->target, s->r.left, s->r.top, s->r.width, s->r.height); > + > + left = s->r.left; > + top = s->r.top; > + right = s->r.left + s->r.width; > + bottom = s->r.top + s->r.height; > + framew = frame->format.fmt.pix_mp.width; > + frameh = frame->format.fmt.pix_mp.height; > + > + if (mdp_target_is_crop(s->target)) { > + walign = 1; > + halign = 1; > + } else { > + walign = frame->mdp_fmt->walign; > + halign = frame->mdp_fmt->halign; > + } > + > + dev_dbg(dev, "%d align:%u,%u, bound:%ux%u", ctx->id, > + walign, halign, framew, frameh); > + > + ret = mdp_clamp_start(&left, 0, right, walign, s->flags); > + if (ret) > + return ret; > + ret = mdp_clamp_start(&top, 0, bottom, halign, s->flags); > + if (ret) > + return ret; > + ret = mdp_clamp_end(&right, left, framew, walign, s->flags); > + if (ret) > + return ret; > + ret = mdp_clamp_end(&bottom, top, frameh, halign, s->flags); > + if (ret) > + return ret; > + > + r->left = left; > + r->top = top; > + r->width = right - left; > + r->height = bottom - top; > + > + dev_dbg(dev, "%d crop:(%d,%d) %ux%u", ctx->id, > + r->left, r->top, r->width, r->height); > + return 0; > +} > + > +int mdp_check_scaling_ratio(const struct v4l2_rect *crop, > + const struct v4l2_rect *compose, s32 rotation, > + const struct mdp_limit *limit) > +{ > + u32 crop_w, crop_h, comp_w, comp_h; > + > + crop_w = crop->width; > + crop_h = crop->height; > + if (90 == rotation || 270 == rotation) { > + comp_w = compose->height; > + comp_h = compose->width; > + } else { > + comp_w = compose->width; > + comp_h = compose->height; > + } > + > + if ((crop_w / comp_w) > limit->h_scale_down_max || > + (crop_h / comp_h) > limit->v_scale_down_max || > + (comp_w / crop_w) > limit->h_scale_up_max || > + (comp_h / crop_h) > limit->v_scale_up_max) > + return -ERANGE; > + return 0; > +} > + > +/* Stride that is accepted by MDP HW */ > +static u32 mdp_fmt_get_stride(const struct mdp_format *fmt, > + u32 bytesperline, unsigned int plane) > +{ > + enum mdp_color c = fmt->mdp_color; > + u32 stride; > + > + stride = (bytesperline * MDP_COLOR_BITS_PER_PIXEL(c)) > + / fmt->row_depth[0]; > + if (plane == 0) > + return stride; > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) { > + if (MDP_COLOR_IS_BLOCK_MODE(c)) > + stride = stride / 2; > + return stride; > + } > + return 0; > +} > + > +/* Stride that is accepted by MDP HW of format with contiguous planes */ > +static u32 mdp_fmt_get_stride_contig(const struct mdp_format *fmt, > + u32 pix_stride, unsigned int plane) > +{ > + enum mdp_color c = fmt->mdp_color; > + u32 stride = pix_stride; > + > + if (plane == 0) > + return stride; > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) { > + stride = stride >> MDP_COLOR_GET_H_SUBSAMPLE(c); > + if (MDP_COLOR_IS_UV_COPLANE(c) && !MDP_COLOR_IS_BLOCK_MODE(c)) > + stride = stride * 2; > + return stride; > + } > + return 0; > +} > + > +/* Plane size that is accepted by MDP HW */ > +static u32 mdp_fmt_get_plane_size(const struct mdp_format *fmt, > + u32 stride, u32 height, unsigned int plane) > +{ > + enum mdp_color c = fmt->mdp_color; > + u32 bytesperline; > + > + bytesperline = (stride * fmt->row_depth[0]) > + / MDP_COLOR_BITS_PER_PIXEL(c); > + if (plane == 0) > + return bytesperline * height; > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) { > + height = height >> MDP_COLOR_GET_V_SUBSAMPLE(c); > + if (MDP_COLOR_IS_BLOCK_MODE(c)) > + bytesperline = bytesperline * 2; > + return bytesperline * height; > + } > + return 0; > +} > + > +static void mdp_prepare_buffer(struct img_image_buffer *b, > + struct mdp_frame *frame, struct vb2_buffer *vb) > +{ > + struct v4l2_pix_format_mplane *pix_mp = &frame->format.fmt.pix_mp; > + unsigned int i; > + > + b->format.colorformat = frame->mdp_fmt->mdp_color; > + b->format.ycbcr_prof = frame->ycbcr_prof; > + for (i = 0; i < pix_mp->num_planes; ++i) { > + u32 stride = mdp_fmt_get_stride(frame->mdp_fmt, > + pix_mp->plane_fmt[i].bytesperline, i); > + > + b->format.plane_fmt[i].stride = stride; > + /* > + * TODO : The way to pass an offset within a DMA-buf > + * is not defined in V4L2 specification, so we abuse > + * data_offset for now. Fix it when we have the right interface, > + * including any necessary validation and potential alignment > + * issues. > + */ > + b->format.plane_fmt[i].size = > + mdp_fmt_get_plane_size(frame->mdp_fmt, stride, > + pix_mp->height, i) - > + vb->planes[i].data_offset; > + b->iova[i] = vb2_dma_contig_plane_dma_addr(vb, i) + > + vb->planes[i].data_offset; Why do you need data_offset? What is the use case? Obviously I can't merge a driver that abuses an API. > + } > + for (; i < MDP_COLOR_GET_PLANE_COUNT(b->format.colorformat); ++i) { > + u32 stride = mdp_fmt_get_stride_contig(frame->mdp_fmt, > + b->format.plane_fmt[0].stride, i); > + > + b->format.plane_fmt[i].stride = stride; > + b->format.plane_fmt[i].size = > + mdp_fmt_get_plane_size(frame->mdp_fmt, stride, > + pix_mp->height, i); > + b->iova[i] = b->iova[i - 1] + b->format.plane_fmt[i - 1].size; > + } > + b->usage = frame->usage; > +} > + > +void mdp_set_src_config(struct img_input *in, > + struct mdp_frame *frame, struct vb2_buffer *vb) > +{ > + in->buffer.format.width = frame->format.fmt.pix_mp.width; > + in->buffer.format.height = frame->format.fmt.pix_mp.height; > + mdp_prepare_buffer(&in->buffer, frame, vb); > +} > + > +static u32 mdp_to_fixed(u32 *r, struct v4l2_fract *f) > +{ > + u32 q; > + > + if (f->denominator == 0) { > + *r = 0; > + return 0; > + } > + > + q = f->numerator / f->denominator; > + *r = div_u64(((u64)f->numerator - q * f->denominator) << > + IMG_SUBPIXEL_SHIFT, f->denominator); > + return q; > +} > + > +static void mdp_set_src_crop(struct img_crop *c, struct mdp_crop *crop) > +{ > + c->left = crop->c.left > + + mdp_to_fixed(&c->left_subpix, &crop->left_subpix); > + c->top = crop->c.top > + + mdp_to_fixed(&c->top_subpix, &crop->top_subpix); > + c->width = crop->c.width > + + mdp_to_fixed(&c->width_subpix, &crop->width_subpix); > + c->height = crop->c.height > + + mdp_to_fixed(&c->height_subpix, &crop->height_subpix); > +} > + > +static void mdp_set_orientation(struct img_output *out, > + s32 rotation, bool hflip, bool vflip) > +{ > + u8 flip = 0; > + > + if (hflip) > + flip ^= 1; > + if (vflip) { > + /* > + * A vertical flip is equivalent to > + * a 180-degree rotation with a horizontal flip > + */ > + rotation += 180; > + flip ^= 1; > + } > + > + out->rotation = rotation % 360; > + if (flip != 0) > + out->flags |= IMG_CTRL_FLAG_HFLIP; > + else > + out->flags &= ~IMG_CTRL_FLAG_HFLIP; > +} > + > +void mdp_set_dst_config(struct img_output *out, > + struct mdp_frame *frame, struct vb2_buffer *vb) > +{ > + out->buffer.format.width = frame->compose.width; > + out->buffer.format.height = frame->compose.height; > + mdp_prepare_buffer(&out->buffer, frame, vb); > + mdp_set_src_crop(&out->crop, &frame->crop); > + mdp_set_orientation(out, frame->rotation, frame->hflip, frame->vflip); > +} > + > +int mdp_frameparam_init(struct mdp_frameparam *param) > +{ > + struct mdp_frame *frame; > + > + if (!param) > + return -EINVAL; > + > + INIT_LIST_HEAD(¶m->list); > + param->limit = &mdp_def_limit; > + param->type = MDP_STREAM_TYPE_BITBLT; > + > + frame = ¶m->output; > + frame->format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; > + frame->mdp_fmt = mdp_try_fmt_mplane(&frame->format, param, 0); > + frame->ycbcr_prof = > + mdp_map_ycbcr_prof_mplane(&frame->format, > + frame->mdp_fmt->mdp_color); > + frame->usage = MDP_BUFFER_USAGE_HW_READ; > + > + param->num_captures = 1; > + frame = ¶m->captures[0]; > + frame->format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > + frame->mdp_fmt = mdp_try_fmt_mplane(&frame->format, param, 0); > + frame->ycbcr_prof = > + mdp_map_ycbcr_prof_mplane(&frame->format, > + frame->mdp_fmt->mdp_color); > + frame->usage = MDP_BUFFER_USAGE_MDP; > + frame->crop.c.width = param->output.format.fmt.pix_mp.width; > + frame->crop.c.height = param->output.format.fmt.pix_mp.height; > + frame->compose.width = frame->format.fmt.pix_mp.width; > + frame->compose.height = frame->format.fmt.pix_mp.height; > + > + return 0; > +} Regards, Hans
On Fri, Jul 08, 2022 at 10:08:44AM +0200, Hans Verkuil wrote: > Hi Moudy, > > Some comments below: And one more > On 7/6/22 09:54, Moudy Ho wrote: > > This patch adds driver for Mediatek's Media Data Path ver.3 (MDP3). > > It provides the following functions: > > color transform, format conversion, resize, crop, rotate, flip > > and additional image quality enhancement. > > > > The MDP3 driver is mainly used for Google Chromebook products to > > import the new architecture to set the HW settings as shown below: > > User -> V4L2 framework > > -> MDP3 driver -> SCP (setting calculations) > > -> MDP3 driver -> CMDQ (GCE driver) -> HW > > > > Each modules' related operation control is sited in mtk-mdp3-comp.c > > Each modules' register table is defined in file with "mdp_reg_" prefix > > GCE related API, operation control sited in mtk-mdp3-cmdq.c > > V4L2 m2m device functions are implemented in mtk-mdp3-m2m.c > > Probe, power, suspend/resume, system level functions are defined in > > mtk-mdp3-core.c > > > > v4l2-compliance 1.22.1, 32 bits, 32-bit time_t > > > > Compliance test for mtk-mdp3 device /dev/video2: > > > > Driver Info: > > Driver name : mtk-mdp3 > > Card type : 14001000.mdp3-rdma0 > > Card type is expected to be a human readable name, and that's not the case > here. > > > Bus info : platform:mtk-mdp3 > > <snip> > > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c > > new file mode 100644 > > index 000000000000..9d2b8acc303f > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c > > @@ -0,0 +1,769 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2022 MediaTek Inc. > > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com> > > + */ > > + > > +#include <linux/platform_device.h> > > +#include <media/v4l2-ioctl.h> > > +#include <media/v4l2-event.h> > > +#include <media/videobuf2-dma-contig.h> > > +#include "mtk-mdp3-m2m.h" > > + > > +static inline struct mdp_m2m_ctx *fh_to_ctx(struct v4l2_fh *fh) > > +{ > > + return container_of(fh, struct mdp_m2m_ctx, fh); > > +} > > + > > +static inline struct mdp_m2m_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl) > > +{ > > + return container_of(ctrl->handler, struct mdp_m2m_ctx, ctrl_handler); > > +} > > + > > +static inline struct mdp_frame *ctx_get_frame(struct mdp_m2m_ctx *ctx, > > + enum v4l2_buf_type type) > > +{ > > + if (V4L2_TYPE_IS_OUTPUT(type)) > > + return &ctx->curr_param.output; > > + else > > + return &ctx->curr_param.captures[0]; > > +} > > + > > +static void mdp_m2m_ctx_set_state(struct mdp_m2m_ctx *ctx, u32 state) > > +{ > > + atomic_or(state, &ctx->curr_param.state); > > +} > > + > > +static bool mdp_m2m_ctx_is_state_set(struct mdp_m2m_ctx *ctx, u32 mask) > > +{ > > + bool ret; > > + > > + ret = ((atomic_read(&ctx->curr_param.state) & mask) == mask); > > This can just do 'return' so you can drop the 'ret' variable. > > > + > > + return ret; > > +} > > + > > +static void mdp_m2m_process_done(void *priv, int vb_state) > > +{ > > + struct mdp_m2m_ctx *ctx = priv; > > + struct vb2_v4l2_buffer *src_vbuf, *dst_vbuf; > > + > > + src_vbuf = (struct vb2_v4l2_buffer *) > > + v4l2_m2m_src_buf_remove(ctx->m2m_ctx); > > + dst_vbuf = (struct vb2_v4l2_buffer *) > > + v4l2_m2m_dst_buf_remove(ctx->m2m_ctx); > > + ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC]; > > + src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++; > > + dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++; > > + v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true); > > + > > + v4l2_m2m_buf_done(src_vbuf, vb_state); > > + v4l2_m2m_buf_done(dst_vbuf, vb_state); > > + v4l2_m2m_job_finish(ctx->mdp_dev->m2m_dev, ctx->m2m_ctx); > > +} > > + > > +static void mdp_m2m_device_run(void *priv) > > +{ > > + struct mdp_m2m_ctx *ctx = priv; > > + struct mdp_frame *frame; > > + struct vb2_v4l2_buffer *src_vb, *dst_vb; > > + struct img_ipi_frameparam param = {0}; > > + struct mdp_cmdq_param task = {0}; > > Just use '= {}', it's cleaner. > > > + enum vb2_buffer_state vb_state = VB2_BUF_STATE_ERROR; > > + int ret; > > + > > + if (mdp_m2m_ctx_is_state_set(ctx, MDP_M2M_CTX_ERROR)) { > > + dev_err(&ctx->mdp_dev->pdev->dev, > > + "mdp_m2m_ctx is in error state\n"); > > + goto worker_end; > > + } > > + > > + param.frame_no = ctx->curr_param.frame_no; > > + param.type = ctx->curr_param.type; > > + param.num_inputs = 1; > > + param.num_outputs = 1; > > + > > + frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); > > + src_vb = v4l2_m2m_next_src_buf(ctx->m2m_ctx); > > + mdp_set_src_config(¶m.inputs[0], frame, &src_vb->vb2_buf); > > + > > + frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > + dst_vb = v4l2_m2m_next_dst_buf(ctx->m2m_ctx); > > + mdp_set_dst_config(¶m.outputs[0], frame, &dst_vb->vb2_buf); > > + > > + ret = mdp_vpu_process(&ctx->vpu, ¶m); > > + if (ret) { > > + dev_err(&ctx->mdp_dev->pdev->dev, > > + "VPU MDP process failed: %d\n", ret); > > + goto worker_end; > > + } > > + > > + task.config = ctx->vpu.config; > > + task.param = ¶m; > > + task.composes[0] = &frame->compose; > > + task.cmdq_cb = NULL; > > + task.cb_data = NULL; > > + task.mdp_ctx = ctx; > > + > > + ret = mdp_cmdq_send(ctx->mdp_dev, &task); > > + if (ret) { > > + dev_err(&ctx->mdp_dev->pdev->dev, > > + "CMDQ sendtask failed: %d\n", ret); > > + goto worker_end; > > + } > > + > > + return; > > + > > +worker_end: > > + mdp_m2m_process_done(ctx, vb_state); > > +} > > + > > +static int mdp_m2m_start_streaming(struct vb2_queue *q, unsigned int count) > > +{ > > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q); > > + > > + ctx->frame_count[MDP_M2M_SRC] = 0; > > + ctx->frame_count[MDP_M2M_DST] = 0; > > Don't set both, check which queue it is first. > > > + > > + return 0; > > +} > > + > > +static struct vb2_v4l2_buffer *mdp_m2m_buf_remove(struct mdp_m2m_ctx *ctx, > > + unsigned int type) > > +{ > > + if (V4L2_TYPE_IS_OUTPUT(type)) > > + return (struct vb2_v4l2_buffer *) > > + v4l2_m2m_src_buf_remove(ctx->m2m_ctx); > > + else > > + return (struct vb2_v4l2_buffer *) > > + v4l2_m2m_dst_buf_remove(ctx->m2m_ctx); > > +} > > + > > +static void mdp_m2m_stop_streaming(struct vb2_queue *q) > > +{ > > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q); > > + struct vb2_v4l2_buffer *vb; > > + > > + vb = mdp_m2m_buf_remove(ctx, q->type); > > + while (vb) { > > + v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR); > > + vb = mdp_m2m_buf_remove(ctx, q->type); > > + } > > +} > > + > > +static int mdp_m2m_queue_setup(struct vb2_queue *q, > > + unsigned int *num_buffers, > > + unsigned int *num_planes, unsigned int sizes[], > > + struct device *alloc_devs[]) > > +{ > > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q); > > + struct v4l2_pix_format_mplane *pix_mp; > > + struct device *dev = &ctx->mdp_dev->pdev->dev; > > + u32 i; > > + > > + pix_mp = &ctx_get_frame(ctx, q->type)->format.fmt.pix_mp; > > + > > + /* from VIDIOC_CREATE_BUFS */ > > + if (*num_planes) { > > + if (*num_planes != pix_mp->num_planes) > > + return -EINVAL; > > + for (i = 0; i < pix_mp->num_planes; ++i) > > + if (sizes[i] < pix_mp->plane_fmt[i].sizeimage) > > + return -EINVAL; > > + } else {/* from VIDIOC_REQBUFS */ > > + *num_planes = pix_mp->num_planes; > > + for (i = 0; i < pix_mp->num_planes; ++i) > > + sizes[i] = pix_mp->plane_fmt[i].sizeimage; > > + } > > + > > + dev_info(dev, "[%d] type:%d, planes:%u, buffers:%u, size:%u,%u,%u", > > This should be dropped or changed to dev_dbg. You don't want logging > when doing normal things, that will just spam the kernel log. > > > + ctx->id, q->type, *num_planes, *num_buffers, > > + sizes[0], sizes[1], sizes[2]); > > + return 0; > > +} > > + > > +static int mdp_m2m_buf_prepare(struct vb2_buffer *vb) > > +{ > > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > > + struct v4l2_pix_format_mplane *pix_mp; > > + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb); > > + u32 i; > > + > > + v4l2_buf->field = V4L2_FIELD_NONE; > > + > > + if (!V4L2_TYPE_IS_OUTPUT(vb->type)) { > > + pix_mp = &ctx_get_frame(ctx, vb->type)->format.fmt.pix_mp; > > + for (i = 0; i < pix_mp->num_planes; ++i) { > > + vb2_set_plane_payload(vb, i, > > + pix_mp->plane_fmt[i].sizeimage); > > + } > > + } > > + return 0; > > +} > > + > > +static int mdp_m2m_buf_out_validate(struct vb2_buffer *vb) > > +{ > > + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb); > > + > > + v4l2_buf->field = V4L2_FIELD_NONE; > > + > > + return 0; > > +} > > + > > +static void mdp_m2m_buf_queue(struct vb2_buffer *vb) > > +{ > > + struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > > + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb); > > + > > + v4l2_buf->field = V4L2_FIELD_NONE; > > + > > + v4l2_m2m_buf_queue(ctx->m2m_ctx, to_vb2_v4l2_buffer(vb)); > > +} > > + > > +static const struct vb2_ops mdp_m2m_qops = { > > + .queue_setup = mdp_m2m_queue_setup, > > + .wait_prepare = vb2_ops_wait_prepare, > > + .wait_finish = vb2_ops_wait_finish, > > + .buf_prepare = mdp_m2m_buf_prepare, > > + .start_streaming = mdp_m2m_start_streaming, > > + .stop_streaming = mdp_m2m_stop_streaming, > > + .buf_queue = mdp_m2m_buf_queue, > > + .buf_out_validate = mdp_m2m_buf_out_validate, > > +}; > > + > > +static int mdp_m2m_querycap(struct file *file, void *fh, > > + struct v4l2_capability *cap) > > +{ > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > > + > > + strscpy(cap->driver, MDP_MODULE_NAME, sizeof(cap->driver)); > > + strscpy(cap->card, ctx->mdp_dev->pdev->name, sizeof(cap->card)); > > As mentioned at the top, this is not a suitable name for cap->card. > > > + strscpy(cap->bus_info, "platform:mtk-mdp3", sizeof(cap->bus_info)); And don't override bus_info, the value isn't right. The V4L2 core should do the right thing for platform devices now. > > + return 0; > > +} > > + > > +static int mdp_m2m_enum_fmt_mplane(struct file *file, void *fh, > > + struct v4l2_fmtdesc *f) > > +{ > > + return mdp_enum_fmt_mplane(f); > > +} > > + > > +static int mdp_m2m_g_fmt_mplane(struct file *file, void *fh, > > + struct v4l2_format *f) > > +{ > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > > + struct mdp_frame *frame; > > + struct v4l2_pix_format_mplane *pix_mp; > > + struct device *dev = &ctx->mdp_dev->pdev->dev; > > + > > + frame = ctx_get_frame(ctx, f->type); > > + *f = frame->format; > > + pix_mp = &f->fmt.pix_mp; > > + pix_mp->colorspace = ctx->curr_param.colorspace; > > + pix_mp->xfer_func = ctx->curr_param.xfer_func; > > + pix_mp->ycbcr_enc = ctx->curr_param.ycbcr_enc; > > + pix_mp->quantization = ctx->curr_param.quant; > > + > > + dev_info(dev, "[%d] type:%d, frame:%ux%u colorspace=%d", ctx->id, f->type, > > + f->fmt.pix_mp.width, f->fmt.pix_mp.height, > > + f->fmt.pix_mp.colorspace); > > Drop this, you're returning this info to userspace anyway, so why spam the > kernel log? > > > + return 0; > > +} > > + > > +static int mdp_m2m_s_fmt_mplane(struct file *file, void *fh, > > + struct v4l2_format *f) > > +{ > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > > + struct mdp_frame *frame = ctx_get_frame(ctx, f->type); > > + struct mdp_frame *capture; > > + const struct mdp_format *fmt; > > + struct vb2_queue *vq; > > + struct device *dev = &ctx->mdp_dev->pdev->dev; > > + > > + dev_info(dev, "[%d] type:%d", ctx->id, f->type); > > dev_dbg. If dev_info is used elsewhere in similar situations, then replace that > with dev_dbg as well. Regular usage of this driver must not produce kernel logging. > > > + > > + fmt = mdp_try_fmt_mplane(f, &ctx->curr_param, ctx->id); > > + if (!fmt) { > > + dev_info(dev, "[%d] try_fmt failed, type:%d", ctx->id, f->type); > > Drop this, the error code says enough. > > > + return -EINVAL; > > + } > > + > > + vq = v4l2_m2m_get_vq(ctx->m2m_ctx, f->type); > > + if (vb2_is_streaming(vq)) { > > This must be vb2_is_busy(): changing formats is prohibited once buffers are > allocated (vb2_is_busy() is true in that case). > > > + dev_info(&ctx->mdp_dev->pdev->dev, "Queue %d busy\n", f->type); > > Drop this, the error code says enough. > > > + return -EBUSY; > > + } > > + > > + frame->format = *f; > > + frame->mdp_fmt = fmt; > > + frame->ycbcr_prof = mdp_map_ycbcr_prof_mplane(f, fmt->mdp_color); > > + frame->usage = V4L2_TYPE_IS_OUTPUT(f->type) ? > > + MDP_BUFFER_USAGE_HW_READ : MDP_BUFFER_USAGE_MDP; > > + > > + capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > + if (V4L2_TYPE_IS_OUTPUT(f->type)) { > > + capture->crop.c.left = 0; > > + capture->crop.c.top = 0; > > + capture->crop.c.width = f->fmt.pix_mp.width; > > + capture->crop.c.height = f->fmt.pix_mp.height; > > + ctx->curr_param.colorspace = f->fmt.pix_mp.colorspace; > > + ctx->curr_param.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc; > > + ctx->curr_param.quant = f->fmt.pix_mp.quantization; > > + ctx->curr_param.xfer_func = f->fmt.pix_mp.xfer_func; > > + } else { > > + capture->compose.left = 0; > > + capture->compose.top = 0; > > + capture->compose.width = f->fmt.pix_mp.width; > > + capture->compose.height = f->fmt.pix_mp.height; > > + } > > + > > + ctx->frame_count[MDP_M2M_SRC] = 0; > > + ctx->frame_count[MDP_M2M_DST] = 0; > > Changing the format doesn't mean you need to zero this. Just drop these two > line. > > > + > > + dev_info(dev, "[%d] type:%d, frame:%ux%u", ctx->id, f->type, > > + f->fmt.pix_mp.width, f->fmt.pix_mp.height); > > Drop this. > > > + return 0; > > +} > > + > > +static int mdp_m2m_try_fmt_mplane(struct file *file, void *fh, > > + struct v4l2_format *f) > > +{ > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > > + > > + if (!mdp_try_fmt_mplane(f, &ctx->curr_param, ctx->id)) > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > +static int mdp_m2m_streamon(struct file *file, void *fh, > > + enum v4l2_buf_type type) > > +{ > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > > + struct mdp_frame *capture; > > + int ret; > > + bool out_streaming, cap_streaming; > > + > > + capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > + out_streaming = ctx->m2m_ctx->out_q_ctx.q.streaming; > > + cap_streaming = ctx->m2m_ctx->cap_q_ctx.q.streaming; > > Use vb2_is_streaming() rather than accessing q.streaming directly. > > > + > > + /* Check to see if scaling ratio is within supported range */ > > + if ((V4L2_TYPE_IS_OUTPUT(type) && cap_streaming) || > > + (!V4L2_TYPE_IS_OUTPUT(type) && out_streaming)) { > > + ret = mdp_check_scaling_ratio(&capture->crop.c, > > + &capture->compose, > > + capture->rotation, > > + ctx->curr_param.limit); > > + if (ret) { > > + dev_info(&ctx->mdp_dev->pdev->dev, > > + "Out of scaling range\n"); > > + return ret; > > + } > > + } > > + > > + if (!mdp_m2m_ctx_is_state_set(ctx, MDP_VPU_INIT)) { > > + ret = mdp_vpu_get_locked(ctx->mdp_dev); > > + if (ret) > > + return ret; > > + > > + ret = mdp_vpu_ctx_init(&ctx->vpu, &ctx->mdp_dev->vpu, > > + MDP_DEV_M2M); > > + if (ret) { > > + dev_err(&ctx->mdp_dev->pdev->dev, > > + "VPU init failed %d\n", ret); > > + return -EINVAL; > > + } > > + mdp_m2m_ctx_set_state(ctx, MDP_VPU_INIT); > > + } > > + > > + return v4l2_m2m_streamon(file, ctx->m2m_ctx, type); > > +} > > There really is no reason to override streamon. You can also do this in > start_streaming(). I recommend moving these tests there. Internally > streamon() will call start_streaming(), so any error you return in that > callback function will be the error return of the streamon as well. > > > + > > +static int mdp_m2m_g_selection(struct file *file, void *fh, > > + struct v4l2_selection *s) > > +{ > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > > + struct mdp_frame *frame; > > + struct device *dev = &ctx->mdp_dev->pdev->dev; > > + bool valid = false; > > + > > + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) > > + valid = mdp_target_is_crop(s->target); > > + else if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > > + valid = mdp_target_is_compose(s->target); > > + > > + if (!valid) { > > + dev_err(dev, "[%s:%d] invalid type:%u target:%u", __func__, ctx->id, > > + s->type, s->target); > > Drop this. User errors should never cause spamming in the kernel log. > > Please check your code: you can use dev_dbg to help debugging, but dev_info > should be used very sparingly (typically only in probe() and remove()), and > dev_warn/err only for hardware/driver warnings/errors, and never due to > userspace inputs. > > > + return -EINVAL; > > + } > > + > > + switch (s->target) { > > + case V4L2_SEL_TGT_CROP: > > + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { > > + frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > + s->r = frame->crop.c; > > + return 0; > > + } > > It's easier to read if you swap the tests: > > if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) > return -EINVAL; > frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > s->r = frame->crop.c; > return 0; > > Same below. > > > + break; > > + case V4L2_SEL_TGT_COMPOSE: > > + if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { > > + frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > + s->r = frame->compose; > > + return 0; > > + } > > + break; > > + case V4L2_SEL_TGT_CROP_DEFAULT: > > + case V4L2_SEL_TGT_CROP_BOUNDS: > > + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { > > + frame = ctx_get_frame(ctx, s->type); > > + s->r.left = 0; > > + s->r.top = 0; > > + s->r.width = frame->format.fmt.pix_mp.width; > > + s->r.height = frame->format.fmt.pix_mp.height; > > + return 0; > > + } > > + break; > > + case V4L2_SEL_TGT_COMPOSE_DEFAULT: > > + case V4L2_SEL_TGT_COMPOSE_BOUNDS: > > + if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { > > + frame = ctx_get_frame(ctx, s->type); > > + s->r.left = 0; > > + s->r.top = 0; > > + s->r.width = frame->format.fmt.pix_mp.width; > > + s->r.height = frame->format.fmt.pix_mp.height; > > + return 0; > > + } > > + break; > > + } > > + return -EINVAL; > > +} > > + > > +static int mdp_m2m_s_selection(struct file *file, void *fh, > > + struct v4l2_selection *s) > > +{ > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > > + struct mdp_frame *frame = ctx_get_frame(ctx, s->type); > > + struct mdp_frame *capture; > > + struct v4l2_rect r; > > + struct device *dev = &ctx->mdp_dev->pdev->dev; > > + bool valid = false; > > + int ret; > > + > > + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) > > + valid = (s->target == V4L2_SEL_TGT_CROP); > > + else if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > > + valid = (s->target == V4L2_SEL_TGT_COMPOSE); > > + > > + if (!valid) { > > + dev_err(dev, "[%s:%d] invalid type:%u target:%u", __func__, ctx->id, > > + s->type, s->target); > > + return -EINVAL; > > + } > > + > > + ret = mdp_try_crop(ctx, &r, s, frame); > > + if (ret) > > + return ret; > > + capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > + > > + if (mdp_target_is_crop(s->target)) > > + capture->crop.c = r; > > + else > > + capture->compose = r; > > + > > + s->r = r; > > + memset(s->reserved, 0, sizeof(s->reserved)); > > No need for this memset, the v4l2 core will clear this for you. > > > + > > + ctx->frame_count[MDP_M2M_SRC] = 0; > > + ctx->frame_count[MDP_M2M_DST] = 0; > > Drop this! > > > + return 0; > > +} > > + > > +static const struct v4l2_ioctl_ops mdp_m2m_ioctl_ops = { > > + .vidioc_querycap = mdp_m2m_querycap, > > + .vidioc_enum_fmt_vid_cap = mdp_m2m_enum_fmt_mplane, > > + .vidioc_enum_fmt_vid_out = mdp_m2m_enum_fmt_mplane, > > + .vidioc_g_fmt_vid_cap_mplane = mdp_m2m_g_fmt_mplane, > > + .vidioc_g_fmt_vid_out_mplane = mdp_m2m_g_fmt_mplane, > > + .vidioc_s_fmt_vid_cap_mplane = mdp_m2m_s_fmt_mplane, > > + .vidioc_s_fmt_vid_out_mplane = mdp_m2m_s_fmt_mplane, > > + .vidioc_try_fmt_vid_cap_mplane = mdp_m2m_try_fmt_mplane, > > + .vidioc_try_fmt_vid_out_mplane = mdp_m2m_try_fmt_mplane, > > + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, > > + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf, > > + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf, > > + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf, > > + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf, > > + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs, > > + .vidioc_streamon = mdp_m2m_streamon, > > + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff, > > + .vidioc_g_selection = mdp_m2m_g_selection, > > + .vidioc_s_selection = mdp_m2m_s_selection, > > + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, > > + .vidioc_unsubscribe_event = v4l2_event_unsubscribe, > > +}; > > + > > +static int mdp_m2m_queue_init(void *priv, > > + struct vb2_queue *src_vq, > > + struct vb2_queue *dst_vq) > > +{ > > + struct mdp_m2m_ctx *ctx = priv; > > + int ret; > > + > > + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; > > + src_vq->io_modes = VB2_MMAP | VB2_DMABUF; > > + src_vq->ops = &mdp_m2m_qops; > > + src_vq->mem_ops = &vb2_dma_contig_memops; > > + src_vq->drv_priv = ctx; > > + src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); > > + src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; > > + src_vq->dev = &ctx->mdp_dev->pdev->dev; > > + src_vq->lock = &ctx->ctx_lock; > > + > > + ret = vb2_queue_init(src_vq); > > + if (ret) > > + return ret; > > + > > + dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > > + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF; > > + dst_vq->ops = &mdp_m2m_qops; > > + dst_vq->mem_ops = &vb2_dma_contig_memops; > > + dst_vq->drv_priv = ctx; > > + dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); > > + dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; > > + dst_vq->dev = &ctx->mdp_dev->pdev->dev; > > + dst_vq->lock = &ctx->ctx_lock; > > + > > + return vb2_queue_init(dst_vq); > > +} > > + > > +static int mdp_m2m_s_ctrl(struct v4l2_ctrl *ctrl) > > +{ > > + struct mdp_m2m_ctx *ctx = ctrl_to_ctx(ctrl); > > + struct mdp_frame *capture; > > + > > + if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE) > > + return 0; > > Why this test? As far as I can tell this flag is never set, so there > is no need to check against it. > > > + > > + capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > + switch (ctrl->id) { > > + case V4L2_CID_HFLIP: > > + capture->hflip = ctrl->val; > > + break; > > + case V4L2_CID_VFLIP: > > + capture->vflip = ctrl->val; > > + break; > > + case V4L2_CID_ROTATE: > > + capture->rotation = ctrl->val; > > + break; > > + } > > + > > + return 0; > > +} > > + > > +static const struct v4l2_ctrl_ops mdp_m2m_ctrl_ops = { > > + .s_ctrl = mdp_m2m_s_ctrl, > > +}; > > + > > +static int mdp_m2m_ctrls_create(struct mdp_m2m_ctx *ctx) > > +{ > > + v4l2_ctrl_handler_init(&ctx->ctrl_handler, MDP_MAX_CTRLS); > > + ctx->ctrls.hflip = v4l2_ctrl_new_std(&ctx->ctrl_handler, > > + &mdp_m2m_ctrl_ops, V4L2_CID_HFLIP, > > + 0, 1, 1, 0); > > + ctx->ctrls.vflip = v4l2_ctrl_new_std(&ctx->ctrl_handler, > > + &mdp_m2m_ctrl_ops, V4L2_CID_VFLIP, > > + 0, 1, 1, 0); > > + ctx->ctrls.rotate = v4l2_ctrl_new_std(&ctx->ctrl_handler, > > + &mdp_m2m_ctrl_ops, > > + V4L2_CID_ROTATE, 0, 270, 90, 0); > > + > > + if (ctx->ctrl_handler.error) { > > + int err = ctx->ctrl_handler.error; > > + > > + v4l2_ctrl_handler_free(&ctx->ctrl_handler); > > + dev_err(&ctx->mdp_dev->pdev->dev, > > + "Failed to register controls\n"); > > + return err; > > + } > > + return 0; > > +} > > + > > +static int mdp_m2m_open(struct file *file) > > +{ > > + struct video_device *vdev = video_devdata(file); > > + struct mdp_dev *mdp = video_get_drvdata(vdev); > > + struct mdp_m2m_ctx *ctx; > > + struct device *dev = &mdp->pdev->dev; > > + int ret; > > + struct v4l2_format default_format; > > Just add '= {}' to avoid the memset later on. > > > + > > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > > + if (!ctx) > > + return -ENOMEM; > > + > > + if (mutex_lock_interruptible(&mdp->m2m_lock)) { > > + ret = -ERESTARTSYS; > > + goto err_free_ctx; > > + } > > + > > + ctx->id = ida_alloc(&mdp->mdp_ida, GFP_KERNEL); > > + ctx->mdp_dev = mdp; > > + > > + v4l2_fh_init(&ctx->fh, vdev); > > + vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; > > This doesn't belong in open(). It is already done when the video device is registered. > > > + file->private_data = &ctx->fh; > > + ret = mdp_m2m_ctrls_create(ctx); > > + if (ret) > > + goto err_exit_fh; > > + > > + /* Use separate control handler per file handle */ > > + ctx->fh.ctrl_handler = &ctx->ctrl_handler; > > + v4l2_fh_add(&ctx->fh); > > + > > + mutex_init(&ctx->ctx_lock); > > + ctx->m2m_ctx = v4l2_m2m_ctx_init(mdp->m2m_dev, ctx, mdp_m2m_queue_init); > > + if (IS_ERR(ctx->m2m_ctx)) { > > + dev_err(dev, "Failed to initialize m2m context\n"); > > + ret = PTR_ERR(ctx->m2m_ctx); > > + goto err_release_handler; > > + } > > + ctx->fh.m2m_ctx = ctx->m2m_ctx; > > + > > + ctx->curr_param.ctx = ctx; > > + ret = mdp_frameparam_init(&ctx->curr_param); > > + if (ret) { > > + dev_err(dev, "Failed to initialize mdp parameter\n"); > > + goto err_release_m2m_ctx; > > + } > > + > > + mutex_unlock(&mdp->m2m_lock); > > + > > + /* Default format */ > > + memset(&default_format, 0, sizeof(default_format)); > > This can be dropped if default_format is inited with = {}. > > > + default_format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; > > + default_format.fmt.pix_mp.width = 32; > > + default_format.fmt.pix_mp.height = 32; > > + default_format.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_YUV420M; > > + mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format); > > + default_format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > > + mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format); > > + > > + dev_dbg(dev, "%s:[%d]", __func__, ctx->id); > > + > > + return 0; > > + > > +err_release_m2m_ctx: > > + v4l2_m2m_ctx_release(ctx->m2m_ctx); > > +err_release_handler: > > + v4l2_ctrl_handler_free(&ctx->ctrl_handler); > > + v4l2_fh_del(&ctx->fh); > > +err_exit_fh: > > + v4l2_fh_exit(&ctx->fh); > > + mutex_unlock(&mdp->m2m_lock); > > +err_free_ctx: > > + kfree(ctx); > > + > > + return ret; > > +} > > + > > +static int mdp_m2m_release(struct file *file) > > +{ > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(file->private_data); > > + struct mdp_dev *mdp = video_drvdata(file); > > + struct device *dev = &mdp->pdev->dev; > > + > > + mutex_lock(&mdp->m2m_lock); > > + v4l2_m2m_ctx_release(ctx->m2m_ctx); > > + if (mdp_m2m_ctx_is_state_set(ctx, MDP_VPU_INIT)) { > > + mdp_vpu_ctx_deinit(&ctx->vpu); > > + mdp_vpu_put_locked(mdp); > > + } > > + > > + v4l2_ctrl_handler_free(&ctx->ctrl_handler); > > + v4l2_fh_del(&ctx->fh); > > + v4l2_fh_exit(&ctx->fh); > > + ida_free(&mdp->mdp_ida, ctx->id); > > + mutex_unlock(&mdp->m2m_lock); > > + > > + dev_dbg(dev, "%s:[%d]", __func__, ctx->id); > > + kfree(ctx); > > + > > + return 0; > > +} > > + > > +static const struct v4l2_file_operations mdp_m2m_fops = { > > + .owner = THIS_MODULE, > > + .poll = v4l2_m2m_fop_poll, > > + .unlocked_ioctl = video_ioctl2, > > + .mmap = v4l2_m2m_fop_mmap, > > + .open = mdp_m2m_open, > > + .release = mdp_m2m_release, > > +}; > > + > > +static const struct v4l2_m2m_ops mdp_m2m_ops = { > > + .device_run = mdp_m2m_device_run, > > +}; > > + > > +int mdp_m2m_device_register(struct mdp_dev *mdp) > > +{ > > + struct device *dev = &mdp->pdev->dev; > > + int ret = 0; > > + > > + mdp->m2m_vdev = video_device_alloc(); > > + if (!mdp->m2m_vdev) { > > + dev_err(dev, "Failed to allocate video device\n"); > > + ret = -ENOMEM; > > + goto err_video_alloc; > > + } > > + mdp->m2m_vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | > > + V4L2_CAP_STREAMING; > > + mdp->m2m_vdev->fops = &mdp_m2m_fops; > > + mdp->m2m_vdev->ioctl_ops = &mdp_m2m_ioctl_ops; > > + mdp->m2m_vdev->release = mdp_video_device_release; > > + mdp->m2m_vdev->lock = &mdp->m2m_lock; > > + mdp->m2m_vdev->vfl_dir = VFL_DIR_M2M; > > + mdp->m2m_vdev->v4l2_dev = &mdp->v4l2_dev; > > + snprintf(mdp->m2m_vdev->name, sizeof(mdp->m2m_vdev->name), "%s:m2m", > > + MDP_MODULE_NAME); > > + video_set_drvdata(mdp->m2m_vdev, mdp); > > + > > + mdp->m2m_dev = v4l2_m2m_init(&mdp_m2m_ops); > > + if (IS_ERR(mdp->m2m_dev)) { > > + dev_err(dev, "Failed to initialize v4l2-m2m device\n"); > > + ret = PTR_ERR(mdp->m2m_dev); > > + goto err_m2m_init; > > + } > > + > > + ret = video_register_device(mdp->m2m_vdev, VFL_TYPE_VIDEO, 2); > > Why 2? Just use -1 to pick the first free device number. > > > + if (ret) { > > + dev_err(dev, "Failed to register video device\n"); > > + goto err_video_register; > > + } > > + > > + v4l2_info(&mdp->v4l2_dev, "Driver registered as /dev/video%d", > > + mdp->m2m_vdev->num); > > + return 0; > > + > > +err_video_register: > > + v4l2_m2m_release(mdp->m2m_dev); > > +err_m2m_init: > > + video_device_release(mdp->m2m_vdev); > > +err_video_alloc: > > + > > + return ret; > > +} > > + > > +void mdp_m2m_device_unregister(struct mdp_dev *mdp) > > +{ > > + video_unregister_device(mdp->m2m_vdev); > > +} > > + > > +void mdp_m2m_job_finish(struct mdp_m2m_ctx *ctx) > > +{ > > + enum vb2_buffer_state vb_state = VB2_BUF_STATE_DONE; > > + > > + mdp_m2m_process_done(ctx, vb_state); > > +} > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h > > new file mode 100644 > > index 000000000000..61ddbaf1bf13 > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h > > @@ -0,0 +1,48 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/* > > + * Copyright (c) 2022 MediaTek Inc. > > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com> > > + */ > > + > > +#ifndef __MTK_MDP3_M2M_H__ > > +#define __MTK_MDP3_M2M_H__ > > + > > +#include <media/v4l2-ctrls.h> > > +#include "mtk-mdp3-core.h" > > +#include "mtk-mdp3-vpu.h" > > +#include "mtk-mdp3-regs.h" > > + > > +#define MDP_MAX_CTRLS 10 > > + > > +enum { > > + MDP_M2M_SRC = 0, > > + MDP_M2M_DST = 1, > > + MDP_M2M_MAX, > > +}; > > + > > +struct mdp_m2m_ctrls { > > + struct v4l2_ctrl *hflip; > > + struct v4l2_ctrl *vflip; > > + struct v4l2_ctrl *rotate; > > +}; > > + > > +struct mdp_m2m_ctx { > > + u32 id; > > + struct mdp_dev *mdp_dev; > > + struct v4l2_fh fh; > > + struct v4l2_ctrl_handler ctrl_handler; > > + struct mdp_m2m_ctrls ctrls; > > + struct v4l2_m2m_ctx *m2m_ctx; > > + struct mdp_vpu_ctx vpu; > > + u32 frame_count[MDP_M2M_MAX]; > > + > > + struct mdp_frameparam curr_param; > > + /* synchronization protect for mdp m2m context */ > > + struct mutex ctx_lock; > > +}; > > + > > +int mdp_m2m_device_register(struct mdp_dev *mdp); > > +void mdp_m2m_device_unregister(struct mdp_dev *mdp); > > +void mdp_m2m_job_finish(struct mdp_m2m_ctx *ctx); > > + > > +#endif /* __MTK_MDP3_M2M_H__ */ > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c > > new file mode 100644 > > index 000000000000..18874eb7851c > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c > > @@ -0,0 +1,742 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2022 MediaTek Inc. > > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com> > > + */ > > + > > +#include <media/v4l2-common.h> > > +#include <media/videobuf2-v4l2.h> > > +#include <media/videobuf2-dma-contig.h> > > +#include "mtk-mdp3-core.h" > > +#include "mtk-mdp3-regs.h" > > +#include "mtk-mdp3-m2m.h" > > + > > +/* > > + * All 10-bit related formats are not added in the basic format list, > > + * please add the corresponding format settings before use. > > + */ > > +static const struct mdp_format mdp_formats[] = { > > + { > > + .pixelformat = V4L2_PIX_FMT_GREY, > > + .mdp_color = MDP_COLOR_GREY, > > + .depth = { 8 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_RGB565X, > > + .mdp_color = MDP_COLOR_BGR565, > > + .depth = { 16 }, > > + .row_depth = { 16 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_RGB565, > > + .mdp_color = MDP_COLOR_RGB565, > > + .depth = { 16 }, > > + .row_depth = { 16 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_RGB24, > > + .mdp_color = MDP_COLOR_RGB888, > > + .depth = { 24 }, > > + .row_depth = { 24 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_BGR24, > > + .mdp_color = MDP_COLOR_BGR888, > > + .depth = { 24 }, > > + .row_depth = { 24 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_ABGR32, > > + .mdp_color = MDP_COLOR_BGRA8888, > > + .depth = { 32 }, > > + .row_depth = { 32 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_ARGB32, > > + .mdp_color = MDP_COLOR_ARGB8888, > > + .depth = { 32 }, > > + .row_depth = { 32 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_UYVY, > > + .mdp_color = MDP_COLOR_UYVY, > > + .depth = { 16 }, > > + .row_depth = { 16 }, > > + .num_planes = 1, > > + .walign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_VYUY, > > + .mdp_color = MDP_COLOR_VYUY, > > + .depth = { 16 }, > > + .row_depth = { 16 }, > > + .num_planes = 1, > > + .walign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_YUYV, > > + .mdp_color = MDP_COLOR_YUYV, > > + .depth = { 16 }, > > + .row_depth = { 16 }, > > + .num_planes = 1, > > + .walign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_YVYU, > > + .mdp_color = MDP_COLOR_YVYU, > > + .depth = { 16 }, > > + .row_depth = { 16 }, > > + .num_planes = 1, > > + .walign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_YUV420, > > + .mdp_color = MDP_COLOR_I420, > > + .depth = { 12 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .walign = 1, > > + .halign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_YVU420, > > + .mdp_color = MDP_COLOR_YV12, > > + .depth = { 12 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .walign = 1, > > + .halign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV12, > > + .mdp_color = MDP_COLOR_NV12, > > + .depth = { 12 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .walign = 1, > > + .halign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV21, > > + .mdp_color = MDP_COLOR_NV21, > > + .depth = { 12 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .walign = 1, > > + .halign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV16, > > + .mdp_color = MDP_COLOR_NV16, > > + .depth = { 16 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .walign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV61, > > + .mdp_color = MDP_COLOR_NV61, > > + .depth = { 16 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .walign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV24, > > + .mdp_color = MDP_COLOR_NV24, > > + .depth = { 24 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV42, > > + .mdp_color = MDP_COLOR_NV42, > > + .depth = { 24 }, > > + .row_depth = { 8 }, > > + .num_planes = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_MT21C, > > + .mdp_color = MDP_COLOR_420_BLK_UFO, > > + .depth = { 8, 4 }, > > + .row_depth = { 8, 8 }, > > + .num_planes = 2, > > + .walign = 4, > > + .halign = 5, > > + .flags = MDP_FMT_FLAG_OUTPUT, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_MM21, > > + .mdp_color = MDP_COLOR_420_BLK, > > + .depth = { 8, 4 }, > > + .row_depth = { 8, 8 }, > > + .num_planes = 2, > > + .walign = 4, > > + .halign = 5, > > + .flags = MDP_FMT_FLAG_OUTPUT, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV12M, > > + .mdp_color = MDP_COLOR_NV12, > > + .depth = { 8, 4 }, > > + .row_depth = { 8, 8 }, > > + .num_planes = 2, > > + .walign = 1, > > + .halign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV21M, > > + .mdp_color = MDP_COLOR_NV21, > > + .depth = { 8, 4 }, > > + .row_depth = { 8, 8 }, > > + .num_planes = 2, > > + .walign = 1, > > + .halign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV16M, > > + .mdp_color = MDP_COLOR_NV16, > > + .depth = { 8, 8 }, > > + .row_depth = { 8, 8 }, > > + .num_planes = 2, > > + .walign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_NV61M, > > + .mdp_color = MDP_COLOR_NV61, > > + .depth = { 8, 8 }, > > + .row_depth = { 8, 8 }, > > + .num_planes = 2, > > + .walign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_YUV420M, > > + .mdp_color = MDP_COLOR_I420, > > + .depth = { 8, 2, 2 }, > > + .row_depth = { 8, 4, 4 }, > > + .num_planes = 3, > > + .walign = 1, > > + .halign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + }, { > > + .pixelformat = V4L2_PIX_FMT_YVU420M, > > + .mdp_color = MDP_COLOR_YV12, > > + .depth = { 8, 2, 2 }, > > + .row_depth = { 8, 4, 4 }, > > + .num_planes = 3, > > + .walign = 1, > > + .halign = 1, > > + .flags = MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE, > > + } > > +}; > > + > > +static const struct mdp_limit mdp_def_limit = { > > + .out_limit = { > > + .wmin = 16, > > + .hmin = 16, > > + .wmax = 8176, > > + .hmax = 8176, > > + }, > > + .cap_limit = { > > + .wmin = 2, > > + .hmin = 2, > > + .wmax = 8176, > > + .hmax = 8176, > > + }, > > + .h_scale_up_max = 32, > > + .v_scale_up_max = 32, > > + .h_scale_down_max = 20, > > + .v_scale_down_max = 128, > > +}; > > + > > +static const struct mdp_format *mdp_find_fmt(u32 pixelformat, u32 type) > > +{ > > + u32 i, flag; > > + > > + flag = V4L2_TYPE_IS_OUTPUT(type) ? MDP_FMT_FLAG_OUTPUT : > > + MDP_FMT_FLAG_CAPTURE; > > + for (i = 0; i < ARRAY_SIZE(mdp_formats); ++i) { > > + if (!(mdp_formats[i].flags & flag)) > > + continue; > > + if (mdp_formats[i].pixelformat == pixelformat) > > + return &mdp_formats[i]; > > + } > > + return NULL; > > +} > > + > > +static const struct mdp_format *mdp_find_fmt_by_index(u32 index, u32 type) > > +{ > > + u32 i, flag, num = 0; > > + > > + flag = V4L2_TYPE_IS_OUTPUT(type) ? MDP_FMT_FLAG_OUTPUT : > > + MDP_FMT_FLAG_CAPTURE; > > + for (i = 0; i < ARRAY_SIZE(mdp_formats); ++i) { > > + if (!(mdp_formats[i].flags & flag)) > > + continue; > > + if (index == num) > > + return &mdp_formats[i]; > > + num++; > > + } > > + return NULL; > > +} > > + > > +enum mdp_ycbcr_profile mdp_map_ycbcr_prof_mplane(struct v4l2_format *f, > > + u32 mdp_color) > > +{ > > + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; > > + > > + if (MDP_COLOR_IS_RGB(mdp_color)) > > + return MDP_YCBCR_PROFILE_FULL_BT601; > > + > > + switch (pix_mp->colorspace) { > > + case V4L2_COLORSPACE_JPEG: > > + return MDP_YCBCR_PROFILE_JPEG; > > + case V4L2_COLORSPACE_REC709: > > + case V4L2_COLORSPACE_DCI_P3: > > + if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE) > > + return MDP_YCBCR_PROFILE_FULL_BT709; > > + return MDP_YCBCR_PROFILE_BT709; > > + case V4L2_COLORSPACE_BT2020: > > + if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE) > > + return MDP_YCBCR_PROFILE_FULL_BT2020; > > + return MDP_YCBCR_PROFILE_BT2020; > > + default: > > + if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE) > > + return MDP_YCBCR_PROFILE_FULL_BT601; > > + return MDP_YCBCR_PROFILE_BT601; > > + } > > +} > > + > > +static void mdp_bound_align_image(u32 *w, u32 *h, > > + struct v4l2_frmsize_stepwise *s, > > + unsigned int salign) > > +{ > > + unsigned int org_w, org_h; > > + > > + org_w = *w; > > + org_h = *h; > > + v4l_bound_align_image(w, s->min_width, s->max_width, s->step_width, > > + h, s->min_height, s->max_height, s->step_height, > > + salign); > > + > > + s->min_width = org_w; > > + s->min_height = org_h; > > + v4l2_apply_frmsize_constraints(w, h, s); > > +} > > + > > +static int mdp_clamp_align(s32 *x, int min, int max, unsigned int align) > > +{ > > + unsigned int mask; > > + > > + if (min < 0 || max < 0) > > + return -ERANGE; > > + > > + /* Bits that must be zero to be aligned */ > > + mask = ~((1 << align) - 1); > > + > > + min = 0 ? 0 : ((min + ~mask) & mask); > > + max = max & mask; > > + if ((unsigned int)min > (unsigned int)max) > > + return -ERANGE; > > + > > + /* Clamp to aligned min and max */ > > + *x = clamp(*x, min, max); > > + > > + /* Round to nearest aligned value */ > > + if (align) > > + *x = (*x + (1 << (align - 1))) & mask; > > + return 0; > > +} > > + > > +int mdp_enum_fmt_mplane(struct v4l2_fmtdesc *f) > > +{ > > + const struct mdp_format *fmt; > > + > > + fmt = mdp_find_fmt_by_index(f->index, f->type); > > + if (!fmt) > > + return -EINVAL; > > + > > + f->pixelformat = fmt->pixelformat; > > + return 0; > > +} > > + > > +const struct mdp_format *mdp_try_fmt_mplane(struct v4l2_format *f, > > + struct mdp_frameparam *param, > > + u32 ctx_id) > > +{ > > + struct device *dev = ¶m->ctx->mdp_dev->pdev->dev; > > + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; > > + const struct mdp_format *fmt; > > + const struct mdp_pix_limit *pix_limit; > > + struct v4l2_frmsize_stepwise s; > > + u32 org_w, org_h; > > + unsigned int i; > > + > > + fmt = mdp_find_fmt(pix_mp->pixelformat, f->type); > > + if (!fmt) { > > + fmt = mdp_find_fmt_by_index(0, f->type); > > + if (!fmt) { > > + dev_dbg(dev, "%d: pixelformat %c%c%c%c invalid", ctx_id, > > + (pix_mp->pixelformat & 0xff), > > + (pix_mp->pixelformat >> 8) & 0xff, > > + (pix_mp->pixelformat >> 16) & 0xff, > > + (pix_mp->pixelformat >> 24) & 0xff); > > + return NULL; > > + } > > + } > > + > > + pix_mp->field = V4L2_FIELD_NONE; > > + pix_mp->flags = 0; > > + pix_mp->pixelformat = fmt->pixelformat; > > + if (!V4L2_TYPE_IS_OUTPUT(f->type)) { > > + pix_mp->colorspace = param->colorspace; > > + pix_mp->xfer_func = param->xfer_func; > > + pix_mp->ycbcr_enc = param->ycbcr_enc; > > + pix_mp->quantization = param->quant; > > + } > > + > > + pix_limit = V4L2_TYPE_IS_OUTPUT(f->type) ? ¶m->limit->out_limit : > > + ¶m->limit->cap_limit; > > + s.min_width = pix_limit->wmin; > > + s.max_width = pix_limit->wmax; > > + s.step_width = fmt->walign; > > + s.min_height = pix_limit->hmin; > > + s.max_height = pix_limit->hmax; > > + s.step_height = fmt->halign; > > + org_w = pix_mp->width; > > + org_h = pix_mp->height; > > + > > + mdp_bound_align_image(&pix_mp->width, &pix_mp->height, &s, fmt->salign); > > + if (org_w != pix_mp->width || org_h != pix_mp->height) > > + dev_dbg(dev, "%d: size change: %ux%u to %ux%u", ctx_id, > > + org_w, org_h, pix_mp->width, pix_mp->height); > > + > > + if (pix_mp->num_planes && pix_mp->num_planes != fmt->num_planes) > > + dev_dbg(dev, "%d num of planes change: %u to %u", ctx_id, > > + pix_mp->num_planes, fmt->num_planes); > > + pix_mp->num_planes = fmt->num_planes; > > + > > + for (i = 0; i < pix_mp->num_planes; ++i) { > > + u32 min_bpl = (pix_mp->width * fmt->row_depth[i]) >> 3; > > + u32 max_bpl = (pix_limit->wmax * fmt->row_depth[i]) >> 3; > > + u32 bpl = pix_mp->plane_fmt[i].bytesperline; > > + u32 min_si, max_si; > > + u32 si = pix_mp->plane_fmt[i].sizeimage; > > + > > + bpl = clamp(bpl, min_bpl, max_bpl); > > + pix_mp->plane_fmt[i].bytesperline = bpl; > > + > > + min_si = (bpl * pix_mp->height * fmt->depth[i]) / fmt->row_depth[i]; > > + max_si = (bpl * s.max_height * fmt->depth[i]) / fmt->row_depth[i]; > > + > > + si = clamp(si, min_si, max_si); > > + pix_mp->plane_fmt[i].sizeimage = si; > > + > > + dev_dbg(dev, "%d: p%u, bpl:%u [%u, %u], sizeimage:%u [%u, %u]", ctx_id, > > + i, bpl, min_bpl, max_bpl, si, min_si, max_si); > > + } > > + > > + return fmt; > > +} > > + > > +static int mdp_clamp_start(s32 *x, int min, int max, unsigned int align, > > + u32 flags) > > +{ > > + if (flags & V4L2_SEL_FLAG_GE) > > + max = *x; > > + if (flags & V4L2_SEL_FLAG_LE) > > + min = *x; > > + return mdp_clamp_align(x, min, max, align); > > +} > > + > > +static int mdp_clamp_end(s32 *x, int min, int max, unsigned int align, > > + u32 flags) > > +{ > > + if (flags & V4L2_SEL_FLAG_GE) > > + min = *x; > > + if (flags & V4L2_SEL_FLAG_LE) > > + max = *x; > > + return mdp_clamp_align(x, min, max, align); > > +} > > + > > +int mdp_try_crop(struct mdp_m2m_ctx *ctx, struct v4l2_rect *r, > > + const struct v4l2_selection *s, struct mdp_frame *frame) > > +{ > > + struct device *dev = &ctx->mdp_dev->pdev->dev; > > + s32 left, top, right, bottom; > > + u32 framew, frameh, walign, halign; > > + int ret; > > + > > + dev_dbg(dev, "%d target:%d, set:(%d,%d) %ux%u", ctx->id, > > + s->target, s->r.left, s->r.top, s->r.width, s->r.height); > > + > > + left = s->r.left; > > + top = s->r.top; > > + right = s->r.left + s->r.width; > > + bottom = s->r.top + s->r.height; > > + framew = frame->format.fmt.pix_mp.width; > > + frameh = frame->format.fmt.pix_mp.height; > > + > > + if (mdp_target_is_crop(s->target)) { > > + walign = 1; > > + halign = 1; > > + } else { > > + walign = frame->mdp_fmt->walign; > > + halign = frame->mdp_fmt->halign; > > + } > > + > > + dev_dbg(dev, "%d align:%u,%u, bound:%ux%u", ctx->id, > > + walign, halign, framew, frameh); > > + > > + ret = mdp_clamp_start(&left, 0, right, walign, s->flags); > > + if (ret) > > + return ret; > > + ret = mdp_clamp_start(&top, 0, bottom, halign, s->flags); > > + if (ret) > > + return ret; > > + ret = mdp_clamp_end(&right, left, framew, walign, s->flags); > > + if (ret) > > + return ret; > > + ret = mdp_clamp_end(&bottom, top, frameh, halign, s->flags); > > + if (ret) > > + return ret; > > + > > + r->left = left; > > + r->top = top; > > + r->width = right - left; > > + r->height = bottom - top; > > + > > + dev_dbg(dev, "%d crop:(%d,%d) %ux%u", ctx->id, > > + r->left, r->top, r->width, r->height); > > + return 0; > > +} > > + > > +int mdp_check_scaling_ratio(const struct v4l2_rect *crop, > > + const struct v4l2_rect *compose, s32 rotation, > > + const struct mdp_limit *limit) > > +{ > > + u32 crop_w, crop_h, comp_w, comp_h; > > + > > + crop_w = crop->width; > > + crop_h = crop->height; > > + if (90 == rotation || 270 == rotation) { > > + comp_w = compose->height; > > + comp_h = compose->width; > > + } else { > > + comp_w = compose->width; > > + comp_h = compose->height; > > + } > > + > > + if ((crop_w / comp_w) > limit->h_scale_down_max || > > + (crop_h / comp_h) > limit->v_scale_down_max || > > + (comp_w / crop_w) > limit->h_scale_up_max || > > + (comp_h / crop_h) > limit->v_scale_up_max) > > + return -ERANGE; > > + return 0; > > +} > > + > > +/* Stride that is accepted by MDP HW */ > > +static u32 mdp_fmt_get_stride(const struct mdp_format *fmt, > > + u32 bytesperline, unsigned int plane) > > +{ > > + enum mdp_color c = fmt->mdp_color; > > + u32 stride; > > + > > + stride = (bytesperline * MDP_COLOR_BITS_PER_PIXEL(c)) > > + / fmt->row_depth[0]; > > + if (plane == 0) > > + return stride; > > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) { > > + if (MDP_COLOR_IS_BLOCK_MODE(c)) > > + stride = stride / 2; > > + return stride; > > + } > > + return 0; > > +} > > + > > +/* Stride that is accepted by MDP HW of format with contiguous planes */ > > +static u32 mdp_fmt_get_stride_contig(const struct mdp_format *fmt, > > + u32 pix_stride, unsigned int plane) > > +{ > > + enum mdp_color c = fmt->mdp_color; > > + u32 stride = pix_stride; > > + > > + if (plane == 0) > > + return stride; > > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) { > > + stride = stride >> MDP_COLOR_GET_H_SUBSAMPLE(c); > > + if (MDP_COLOR_IS_UV_COPLANE(c) && !MDP_COLOR_IS_BLOCK_MODE(c)) > > + stride = stride * 2; > > + return stride; > > + } > > + return 0; > > +} > > + > > +/* Plane size that is accepted by MDP HW */ > > +static u32 mdp_fmt_get_plane_size(const struct mdp_format *fmt, > > + u32 stride, u32 height, unsigned int plane) > > +{ > > + enum mdp_color c = fmt->mdp_color; > > + u32 bytesperline; > > + > > + bytesperline = (stride * fmt->row_depth[0]) > > + / MDP_COLOR_BITS_PER_PIXEL(c); > > + if (plane == 0) > > + return bytesperline * height; > > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) { > > + height = height >> MDP_COLOR_GET_V_SUBSAMPLE(c); > > + if (MDP_COLOR_IS_BLOCK_MODE(c)) > > + bytesperline = bytesperline * 2; > > + return bytesperline * height; > > + } > > + return 0; > > +} > > + > > +static void mdp_prepare_buffer(struct img_image_buffer *b, > > + struct mdp_frame *frame, struct vb2_buffer *vb) > > +{ > > + struct v4l2_pix_format_mplane *pix_mp = &frame->format.fmt.pix_mp; > > + unsigned int i; > > + > > + b->format.colorformat = frame->mdp_fmt->mdp_color; > > + b->format.ycbcr_prof = frame->ycbcr_prof; > > + for (i = 0; i < pix_mp->num_planes; ++i) { > > + u32 stride = mdp_fmt_get_stride(frame->mdp_fmt, > > + pix_mp->plane_fmt[i].bytesperline, i); > > + > > + b->format.plane_fmt[i].stride = stride; > > + /* > > + * TODO : The way to pass an offset within a DMA-buf > > + * is not defined in V4L2 specification, so we abuse > > + * data_offset for now. Fix it when we have the right interface, > > + * including any necessary validation and potential alignment > > + * issues. > > + */ > > + b->format.plane_fmt[i].size = > > + mdp_fmt_get_plane_size(frame->mdp_fmt, stride, > > + pix_mp->height, i) - > > + vb->planes[i].data_offset; > > + b->iova[i] = vb2_dma_contig_plane_dma_addr(vb, i) + > > + vb->planes[i].data_offset; > > Why do you need data_offset? What is the use case? Obviously I can't > merge a driver that abuses an API. > > > + } > > + for (; i < MDP_COLOR_GET_PLANE_COUNT(b->format.colorformat); ++i) { > > + u32 stride = mdp_fmt_get_stride_contig(frame->mdp_fmt, > > + b->format.plane_fmt[0].stride, i); > > + > > + b->format.plane_fmt[i].stride = stride; > > + b->format.plane_fmt[i].size = > > + mdp_fmt_get_plane_size(frame->mdp_fmt, stride, > > + pix_mp->height, i); > > + b->iova[i] = b->iova[i - 1] + b->format.plane_fmt[i - 1].size; > > + } > > + b->usage = frame->usage; > > +} > > + > > +void mdp_set_src_config(struct img_input *in, > > + struct mdp_frame *frame, struct vb2_buffer *vb) > > +{ > > + in->buffer.format.width = frame->format.fmt.pix_mp.width; > > + in->buffer.format.height = frame->format.fmt.pix_mp.height; > > + mdp_prepare_buffer(&in->buffer, frame, vb); > > +} > > + > > +static u32 mdp_to_fixed(u32 *r, struct v4l2_fract *f) > > +{ > > + u32 q; > > + > > + if (f->denominator == 0) { > > + *r = 0; > > + return 0; > > + } > > + > > + q = f->numerator / f->denominator; > > + *r = div_u64(((u64)f->numerator - q * f->denominator) << > > + IMG_SUBPIXEL_SHIFT, f->denominator); > > + return q; > > +} > > + > > +static void mdp_set_src_crop(struct img_crop *c, struct mdp_crop *crop) > > +{ > > + c->left = crop->c.left > > + + mdp_to_fixed(&c->left_subpix, &crop->left_subpix); > > + c->top = crop->c.top > > + + mdp_to_fixed(&c->top_subpix, &crop->top_subpix); > > + c->width = crop->c.width > > + + mdp_to_fixed(&c->width_subpix, &crop->width_subpix); > > + c->height = crop->c.height > > + + mdp_to_fixed(&c->height_subpix, &crop->height_subpix); > > +} > > + > > +static void mdp_set_orientation(struct img_output *out, > > + s32 rotation, bool hflip, bool vflip) > > +{ > > + u8 flip = 0; > > + > > + if (hflip) > > + flip ^= 1; > > + if (vflip) { > > + /* > > + * A vertical flip is equivalent to > > + * a 180-degree rotation with a horizontal flip > > + */ > > + rotation += 180; > > + flip ^= 1; > > + } > > + > > + out->rotation = rotation % 360; > > + if (flip != 0) > > + out->flags |= IMG_CTRL_FLAG_HFLIP; > > + else > > + out->flags &= ~IMG_CTRL_FLAG_HFLIP; > > +} > > + > > +void mdp_set_dst_config(struct img_output *out, > > + struct mdp_frame *frame, struct vb2_buffer *vb) > > +{ > > + out->buffer.format.width = frame->compose.width; > > + out->buffer.format.height = frame->compose.height; > > + mdp_prepare_buffer(&out->buffer, frame, vb); > > + mdp_set_src_crop(&out->crop, &frame->crop); > > + mdp_set_orientation(out, frame->rotation, frame->hflip, frame->vflip); > > +} > > + > > +int mdp_frameparam_init(struct mdp_frameparam *param) > > +{ > > + struct mdp_frame *frame; > > + > > + if (!param) > > + return -EINVAL; > > + > > + INIT_LIST_HEAD(¶m->list); > > + param->limit = &mdp_def_limit; > > + param->type = MDP_STREAM_TYPE_BITBLT; > > + > > + frame = ¶m->output; > > + frame->format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; > > + frame->mdp_fmt = mdp_try_fmt_mplane(&frame->format, param, 0); > > + frame->ycbcr_prof = > > + mdp_map_ycbcr_prof_mplane(&frame->format, > > + frame->mdp_fmt->mdp_color); > > + frame->usage = MDP_BUFFER_USAGE_HW_READ; > > + > > + param->num_captures = 1; > > + frame = ¶m->captures[0]; > > + frame->format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > > + frame->mdp_fmt = mdp_try_fmt_mplane(&frame->format, param, 0); > > + frame->ycbcr_prof = > > + mdp_map_ycbcr_prof_mplane(&frame->format, > > + frame->mdp_fmt->mdp_color); > > + frame->usage = MDP_BUFFER_USAGE_MDP; > > + frame->crop.c.width = param->output.format.fmt.pix_mp.width; > > + frame->crop.c.height = param->output.format.fmt.pix_mp.height; > > + frame->compose.width = frame->format.fmt.pix_mp.width; > > + frame->compose.height = frame->format.fmt.pix_mp.height; > > + > > + return 0; > > +}
Hi Hans, Thanks for your review, and appreciate for all comments and suggestions. I'll go through each "dev_info" one by one and replace the unnecessary with "dev_dbg" or just remove it. In addition, I will adjust other inappropriate settings as suggested. On Fri, 2022-07-08 at 10:08 +0200, Hans Verkuil wrote: > Hi Moudy, > > Some comments below: > > On 7/6/22 09:54, Moudy Ho wrote: > > This patch adds driver for Mediatek's Media Data Path ver.3 (MDP3). > > It provides the following functions: > > color transform, format conversion, resize, crop, rotate, flip > > and additional image quality enhancement. > > > > The MDP3 driver is mainly used for Google Chromebook products to > > import the new architecture to set the HW settings as shown below: > > User -> V4L2 framework > > -> MDP3 driver -> SCP (setting calculations) > > -> MDP3 driver -> CMDQ (GCE driver) -> HW > > > > Each modules' related operation control is sited in mtk-mdp3-comp.c > > Each modules' register table is defined in file with "mdp_reg_" > > prefix > > GCE related API, operation control sited in mtk-mdp3-cmdq.c > > V4L2 m2m device functions are implemented in mtk-mdp3-m2m.c > > Probe, power, suspend/resume, system level functions are defined in > > mtk-mdp3-core.c > > > > v4l2-compliance 1.22.1, 32 bits, 32-bit time_t > > > > Compliance test for mtk-mdp3 device /dev/video2: > > > > Driver Info: > > Driver name : mtk-mdp3 > > Card type : 14001000.mdp3-rdma0 > > Card type is expected to be a human readable name, and that's not the > case > here. > > > Bus info : platform:mtk-mdp3 > > <snip> [snip] > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c > > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c > > new file mode 100644 > > index 000000000000..18874eb7851c > > --- /dev/null > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c [snip] > > +/* Plane size that is accepted by MDP HW */ > > +static u32 mdp_fmt_get_plane_size(const struct mdp_format *fmt, > > + u32 stride, u32 height, unsigned int > > plane) > > +{ > > + enum mdp_color c = fmt->mdp_color; > > + u32 bytesperline; > > + > > + bytesperline = (stride * fmt->row_depth[0]) > > + / MDP_COLOR_BITS_PER_PIXEL(c); > > + if (plane == 0) > > + return bytesperline * height; > > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) { > > + height = height >> MDP_COLOR_GET_V_SUBSAMPLE(c); > > + if (MDP_COLOR_IS_BLOCK_MODE(c)) > > + bytesperline = bytesperline * 2; > > + return bytesperline * height; > > + } > > + return 0; > > +} > > + > > +static void mdp_prepare_buffer(struct img_image_buffer *b, > > + struct mdp_frame *frame, struct > > vb2_buffer *vb) > > +{ > > + struct v4l2_pix_format_mplane *pix_mp = &frame- > > >format.fmt.pix_mp; > > + unsigned int i; > > + > > + b->format.colorformat = frame->mdp_fmt->mdp_color; > > + b->format.ycbcr_prof = frame->ycbcr_prof; > > + for (i = 0; i < pix_mp->num_planes; ++i) { > > + u32 stride = mdp_fmt_get_stride(frame->mdp_fmt, > > + pix_mp->plane_fmt[i].bytesperline, i); > > + > > + b->format.plane_fmt[i].stride = stride; > > + /* > > + * TODO : The way to pass an offset within a DMA-buf > > + * is not defined in V4L2 specification, so we abuse > > + * data_offset for now. Fix it when we have the right > > interface, > > + * including any necessary validation and potential > > alignment > > + * issues. > > + */ > > + b->format.plane_fmt[i].size = > > + mdp_fmt_get_plane_size(frame->mdp_fmt, stride, > > + pix_mp->height, i) - > > + vb- > > >planes[i].data_offset; > > + b->iova[i] = vb2_dma_contig_plane_dma_addr(vb, i) + > > + vb->planes[i].data_offset; > > Why do you need data_offset? What is the use case? Obviously I can't > merge a driver that abuses an API. > Apologize for not incorporating the previously discussed results into this version due to my carelessness, I will correct it in the next version. https://patchwork.kernel.org/project/linux-mediatek/patch/20210623091457.18002-1-moudy.ho@mediatek.com/ > > + } > > + for (; i < MDP_COLOR_GET_PLANE_COUNT(b->format.colorformat); > > ++i) { > > + u32 stride = mdp_fmt_get_stride_contig(frame->mdp_fmt, > > + b->format.plane_fmt[0].stride, i); > > + > > + b->format.plane_fmt[i].stride = stride; > > + b->format.plane_fmt[i].size = > > + mdp_fmt_get_plane_size(frame->mdp_fmt, stride, > > + pix_mp->height, i); > > + b->iova[i] = b->iova[i - 1] + b->format.plane_fmt[i - > > 1].size; > > + } > > + b->usage = frame->usage; > > +} > > + > > +void mdp_set_src_config(struct img_input *in, > > + struct mdp_frame *frame, struct vb2_buffer *vb) > > +{ > > + in->buffer.format.width = frame->format.fmt.pix_mp.width; > > + in->buffer.format.height = frame->format.fmt.pix_mp.height; > > + mdp_prepare_buffer(&in->buffer, frame, vb); > > +} [snip] > > Regards, > > Hans Many thanks, Moudy
On Fri, 2022-07-08 at 11:20 +0300, Laurent Pinchart wrote: > On Fri, Jul 08, 2022 at 10:08:44AM +0200, Hans Verkuil wrote: > > Hi Moudy, > > > > Some comments below: > > And one more > > > On 7/6/22 09:54, Moudy Ho wrote: > > > This patch adds driver for Mediatek's Media Data Path ver.3 > > > (MDP3). > > > It provides the following functions: > > > color transform, format conversion, resize, crop, rotate, flip > > > and additional image quality enhancement. > > > > > > The MDP3 driver is mainly used for Google Chromebook products to > > > import the new architecture to set the HW settings as shown > > > below: > > > User -> V4L2 framework > > > -> MDP3 driver -> SCP (setting calculations) > > > -> MDP3 driver -> CMDQ (GCE driver) -> HW > > > > > > Each modules' related operation control is sited in mtk-mdp3- > > > comp.c > > > Each modules' register table is defined in file with "mdp_reg_" > > > prefix > > > GCE related API, operation control sited in mtk-mdp3-cmdq.c > > > V4L2 m2m device functions are implemented in mtk-mdp3-m2m.c > > > Probe, power, suspend/resume, system level functions are defined > > > in > > > mtk-mdp3-core.c > > > > > > v4l2-compliance 1.22.1, 32 bits, 32-bit time_t > > > > > > Compliance test for mtk-mdp3 device /dev/video2: > > > > > > Driver Info: > > > Driver name : mtk-mdp3 > > > Card type : 14001000.mdp3-rdma0 > > > > Card type is expected to be a human readable name, and that's not > > the case > > here. > > > > > Bus info : platform:mtk-mdp3 > > > > <snip> > > > > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c > > > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c > > > new file mode 100644 [snip] > > > +static const struct vb2_ops mdp_m2m_qops = { > > > + .queue_setup = mdp_m2m_queue_setup, > > > + .wait_prepare = vb2_ops_wait_prepare, > > > + .wait_finish = vb2_ops_wait_finish, > > > + .buf_prepare = mdp_m2m_buf_prepare, > > > + .start_streaming = mdp_m2m_start_streaming, > > > + .stop_streaming = mdp_m2m_stop_streaming, > > > + .buf_queue = mdp_m2m_buf_queue, > > > + .buf_out_validate = mdp_m2m_buf_out_validate, > > > +}; > > > + > > > +static int mdp_m2m_querycap(struct file *file, void *fh, > > > + struct v4l2_capability *cap) > > > +{ > > > + struct mdp_m2m_ctx *ctx = fh_to_ctx(fh); > > > + > > > + strscpy(cap->driver, MDP_MODULE_NAME, sizeof(cap->driver)); > > > + strscpy(cap->card, ctx->mdp_dev->pdev->name, sizeof(cap- > > > >card)); > > > > As mentioned at the top, this is not a suitable name for cap->card. > > > > > + strscpy(cap->bus_info, "platform:mtk-mdp3", sizeof(cap- > > > >bus_info)); > > And don't override bus_info, the value isn't right. The V4L2 core > should > do the right thing for platform devices now. > Hi Laurent, Thanks for the reminder, I'll fix it in the next version along with what Hans mentioned earlier. Regards, Moudy > > > + return 0; > > > +} > > > + > > > +static int mdp_m2m_enum_fmt_mplane(struct file *file, void *fh, > > > + struct v4l2_fmtdesc *f) > > > +{ > > > + return mdp_enum_fmt_mplane(f); > > > +}