Hi Eizan, Thank you for your patch. Two trivial comments, see below ... Missatge de Eizan Miyamoto <eizan@xxxxxxxxxxxx> del dia dt., 5 de maig 2020 a les 4:07: > > From: Francois Buergisser <fbuergisser@xxxxxxxxxxxx> > > The mtk-mdp driver uses states to check if the formats have been set > on the capture and output when turning the streaming on, setting > controls or setting the selection rectangles. > Those states are reset when 0 buffers are requested like when checking > capabilities. > This patch removes all format checks and set one by default as queues in > V4L2 are expected to always have a format set. > > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-streamon.html > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-g-ctrl.html > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-g-selection.html > > Signed-off-by: Francois Buergisser <fbuergisser@xxxxxxxxxxxx> > Reviewed-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> I guess that this Reviewed-by comes from a previous Gerrit workflow. Usually, when you submit a patch to upstream you should remove the Reviewed-by internally done, so I'd remove it, and ask Tomasz to give you the Reviewed-by on the upstream patch. > (cherry picked from commit 1887bb3924d030352df179347c8962248cdb903e) Also, drop this, only has sense in the context of ChromeOS tree. > Signed-off-by: Eizan Miyamoto <eizan@xxxxxxxxxxxx> > --- Apart from that, the patch looks good to me, so: Reviewed-by: Enric Balletbo I Serra <enric.balletbo@xxxxxxxxxxxxx> > > drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 2 - > drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 90 +++++++------------ > 2 files changed, 34 insertions(+), 58 deletions(-) > > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h > index bafcccd71f31..dd130cc218c9 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h > @@ -28,8 +28,6 @@ > #define MTK_MDP_FMT_FLAG_CAPTURE BIT(1) > > #define MTK_MDP_VPU_INIT BIT(0) > -#define MTK_MDP_SRC_FMT BIT(1) > -#define MTK_MDP_DST_FMT BIT(2) > #define MTK_MDP_CTX_ERROR BIT(5) > > /** > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > index 821f2cf325f0..bb9caaf513bc 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > @@ -369,13 +369,6 @@ void mtk_mdp_ctx_state_lock_set(struct mtk_mdp_ctx *ctx, u32 state) > mutex_unlock(&ctx->slock); > } > > -static void mtk_mdp_ctx_state_lock_clear(struct mtk_mdp_ctx *ctx, u32 state) > -{ > - mutex_lock(&ctx->slock); > - ctx->state &= ~state; > - mutex_unlock(&ctx->slock); > -} > - > static bool mtk_mdp_ctx_state_is_set(struct mtk_mdp_ctx *ctx, u32 mask) > { > bool ret; > @@ -726,11 +719,6 @@ static int mtk_mdp_m2m_s_fmt_mplane(struct file *file, void *fh, > ctx->quant = pix_mp->quantization; > } > > - if (V4L2_TYPE_IS_OUTPUT(f->type)) > - mtk_mdp_ctx_state_lock_set(ctx, MTK_MDP_SRC_FMT); > - else > - mtk_mdp_ctx_state_lock_set(ctx, MTK_MDP_DST_FMT); > - > mtk_mdp_dbg(2, "[%d] type:%d, frame:%dx%d", ctx->id, f->type, > frame->width, frame->height); > > @@ -742,13 +730,6 @@ static int mtk_mdp_m2m_reqbufs(struct file *file, void *fh, > { > struct mtk_mdp_ctx *ctx = fh_to_ctx(fh); > > - if (reqbufs->count == 0) { > - if (reqbufs->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) > - mtk_mdp_ctx_state_lock_clear(ctx, MTK_MDP_SRC_FMT); > - else > - mtk_mdp_ctx_state_lock_clear(ctx, MTK_MDP_DST_FMT); > - } > - > return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs); > } > > @@ -758,14 +739,6 @@ static int mtk_mdp_m2m_streamon(struct file *file, void *fh, > struct mtk_mdp_ctx *ctx = fh_to_ctx(fh); > int ret; > > - /* The source and target color format need to be set */ > - if (V4L2_TYPE_IS_OUTPUT(type)) { > - if (!mtk_mdp_ctx_state_is_set(ctx, MTK_MDP_SRC_FMT)) > - return -EINVAL; > - } else if (!mtk_mdp_ctx_state_is_set(ctx, MTK_MDP_DST_FMT)) { > - return -EINVAL; > - } > - > if (!mtk_mdp_ctx_state_is_set(ctx, MTK_MDP_VPU_INIT)) { > ret = mtk_mdp_vpu_init(&ctx->vpu); > if (ret < 0) { > @@ -899,24 +872,21 @@ static int mtk_mdp_m2m_s_selection(struct file *file, void *fh, > frame = &ctx->d_frame; > > /* Check to see if scaling ratio is within supported range */ > - if (mtk_mdp_ctx_state_is_set(ctx, MTK_MDP_DST_FMT | MTK_MDP_SRC_FMT)) { > - if (V4L2_TYPE_IS_OUTPUT(s->type)) { > - ret = mtk_mdp_check_scaler_ratio(variant, new_r.width, > - new_r.height, ctx->d_frame.crop.width, > - ctx->d_frame.crop.height, > - ctx->ctrls.rotate->val); > - } else { > - ret = mtk_mdp_check_scaler_ratio(variant, > - ctx->s_frame.crop.width, > - ctx->s_frame.crop.height, new_r.width, > - new_r.height, ctx->ctrls.rotate->val); > - } > + if (V4L2_TYPE_IS_OUTPUT(s->type)) > + ret = mtk_mdp_check_scaler_ratio(variant, new_r.width, > + new_r.height, ctx->d_frame.crop.width, > + ctx->d_frame.crop.height, > + ctx->ctrls.rotate->val); > + else > + ret = mtk_mdp_check_scaler_ratio(variant, > + ctx->s_frame.crop.width, > + ctx->s_frame.crop.height, new_r.width, > + new_r.height, ctx->ctrls.rotate->val); > > - if (ret) { > - dev_info(&ctx->mdp_dev->pdev->dev, > - "Out of scaler range"); > - return -EINVAL; > - } > + if (ret) { > + dev_info(&ctx->mdp_dev->pdev->dev, > + "Out of scaler range"); > + return -EINVAL; > } > > s->r = new_r; > @@ -989,7 +959,6 @@ static int mtk_mdp_s_ctrl(struct v4l2_ctrl *ctrl) > struct mtk_mdp_ctx *ctx = ctrl_to_ctx(ctrl); > struct mtk_mdp_dev *mdp = ctx->mdp_dev; > struct mtk_mdp_variant *variant = mdp->variant; > - u32 state = MTK_MDP_DST_FMT | MTK_MDP_SRC_FMT; > int ret = 0; > > if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE) > @@ -1003,17 +972,15 @@ static int mtk_mdp_s_ctrl(struct v4l2_ctrl *ctrl) > ctx->vflip = ctrl->val; > break; > case V4L2_CID_ROTATE: > - if (mtk_mdp_ctx_state_is_set(ctx, state)) { > - ret = mtk_mdp_check_scaler_ratio(variant, > - ctx->s_frame.crop.width, > - ctx->s_frame.crop.height, > - ctx->d_frame.crop.width, > - ctx->d_frame.crop.height, > - ctx->ctrls.rotate->val); > - > - if (ret) > - return -EINVAL; > - } > + ret = mtk_mdp_check_scaler_ratio(variant, > + ctx->s_frame.crop.width, > + ctx->s_frame.crop.height, > + ctx->d_frame.crop.width, > + ctx->d_frame.crop.height, > + ctx->ctrls.rotate->val); > + > + if (ret) > + return -EINVAL; > > ctx->rotation = ctrl->val; > break; > @@ -1090,6 +1057,7 @@ static int mtk_mdp_m2m_open(struct file *file) > struct video_device *vfd = video_devdata(file); > struct mtk_mdp_ctx *ctx = NULL; > int ret; > + struct v4l2_format default_format; > > ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > if (!ctx) > @@ -1144,6 +1112,16 @@ static int mtk_mdp_m2m_open(struct file *file) > list_add(&ctx->list, &mdp->ctx_list); > mutex_unlock(&mdp->lock); > > + /* Default format */ > + memset(&default_format, 0, sizeof(default_format)); > + 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; > + mtk_mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format); > + default_format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > + mtk_mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format); > + > mtk_mdp_dbg(0, "%s [%d]", dev_name(&mdp->pdev->dev), ctx->id); > > return 0; > -- > 2.26.2.526.g744177e7f7-goog > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-mediatek
