* [PATCH v1] [media] mtk-mdp: Remove states for format checks
@ 2020-05-05 2:06 Eizan Miyamoto
2020-05-05 21:45 ` Enric Balletbo Serra
0 siblings, 1 reply; 2+ messages in thread
From: Eizan Miyamoto @ 2020-05-05 2:06 UTC (permalink / raw)
To: LKML
Cc: Francois Buergisser, Tomasz Figa, Eizan Miyamoto, Andrew-CT Chen,
Houlong Wei, Matthias Brugger, Mauro Carvalho Chehab,
Minghsiu Tsai, linux-arm-kernel, linux-media, linux-mediatek
From: Francois Buergisser <fbuergisser@chromium.org>
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@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
(cherry picked from commit 1887bb3924d030352df179347c8962248cdb903e)
Signed-off-by: Eizan Miyamoto <eizan@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v1] [media] mtk-mdp: Remove states for format checks
2020-05-05 2:06 [PATCH v1] [media] mtk-mdp: Remove states for format checks Eizan Miyamoto
@ 2020-05-05 21:45 ` Enric Balletbo Serra
0 siblings, 0 replies; 2+ messages in thread
From: Enric Balletbo Serra @ 2020-05-05 21:45 UTC (permalink / raw)
To: Eizan Miyamoto
Cc: LKML, Andrew-CT Chen, Minghsiu Tsai, Francois Buergisser,
Houlong Wei, Tomasz Figa,
moderated list:ARM/Mediatek SoC support, Matthias Brugger,
Mauro Carvalho Chehab, Linux ARM, linux-media
Hi Eizan,
Thank you for your patch. Two trivial comments, see below ...
Missatge de Eizan Miyamoto <eizan@chromium.org> del dia dt., 5 de maig
2020 a les 4:07:
>
> From: Francois Buergisser <fbuergisser@chromium.org>
>
> 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@chromium.org>
> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
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@chromium.org>
> ---
Apart from that, the patch looks good to me, so:
Reviewed-by: Enric Balletbo I Serra <enric.balletbo@collabora.com>
>
> 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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-05-05 21:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05 2:06 [PATCH v1] [media] mtk-mdp: Remove states for format checks Eizan Miyamoto
2020-05-05 21:45 ` Enric Balletbo Serra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).