diff mbox series

[RFC,v3,5/9] media: platform: Add Mediatek ISP P1 V4L2 control

Message ID 20190611035344.29814-6-jungo.lin@mediatek.com (mailing list archive)
State RFC
Headers show
Series [RFC,v3,1/9] dt-bindings: mt8183: Added camera ISP Pass 1 | expand

Commit Message

Jungo Lin June 11, 2019, 3:53 a.m. UTC
Reserved Mediatek ISP P1 V4L2 control number with 16.
Moreover, add two V4L2 controls for ISP P1 user space
usage.

1. V4L2_CID_MTK_GET_BIN_INFO
- Provide the image output width & height in case
camera binning mode is enabled.

2. V4L2_CID_MTK_RAW_PATH
- Export the path control of the main stream to user space.
One is pure raw and the other is processing raw.
The default value is 0 which outputs the pure raw bayer image
from sesnor, without image processing in ISP HW.

Signed-off-by: Jungo Lin <jungo.lin@mediatek.com>
---
 drivers/media/platform/mtk-isp/Makefile       |   3 +
 .../media/platform/mtk-isp/isp_50/Makefile    |   5 +
 .../platform/mtk-isp/isp_50/cam/Makefile      |   5 +
 .../mtk-isp/isp_50/cam/mtk_cam-ctrl.c         | 138 ++++++++++++++++++
 .../mtk-isp/isp_50/cam/mtk_cam-ctrl.h         |  38 +++++
 include/uapi/linux/v4l2-controls.h            |   4 +
 6 files changed, 193 insertions(+)
 create mode 100644 drivers/media/platform/mtk-isp/Makefile
 create mode 100644 drivers/media/platform/mtk-isp/isp_50/Makefile
 create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/Makefile
 create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-ctrl.c
 create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-ctrl.h

Comments

Tomasz Figa July 1, 2019, 5:50 a.m. UTC | #1
Hi Jungo,

On Tue, Jun 11, 2019 at 11:53:40AM +0800, Jungo Lin wrote:
> Reserved Mediatek ISP P1 V4L2 control number with 16.
> Moreover, add two V4L2 controls for ISP P1 user space
> usage.
> 
> 1. V4L2_CID_MTK_GET_BIN_INFO
> - Provide the image output width & height in case
> camera binning mode is enabled.

Could you explain with a bit more details what these binned width and height
would mean? How would they relate to the video CAPTURE node width and height?
Isn't this something that should be rather exposed as an appropriate
selection rectangle, instead of custom controls?

> 
> 2. V4L2_CID_MTK_RAW_PATH
> - Export the path control of the main stream to user space.
> One is pure raw and the other is processing raw.
> The default value is 0 which outputs the pure raw bayer image
> from sesnor, without image processing in ISP HW.

Is it just effectively a full processing bypass? The driver seems to only
update the related configuration when the streaming starts. Can it be
controlled per-frame?

Generally this sounds more like something that should be modelled using the
media topology, similar to the example below.

/----------------\   /-------------------\   /--------------\
|                |---|                   |   |              |
| Capture Subdev |   | Processing Subdev |-o-| CAPTURE node |
|                |-\ |                   | | |              |
\----------------/ | \-------------------/ | \--------------/
                   |                       |
                   \-----------------------/

Then the userspace can select whether it wants the image from the capture
interface directly or procesed by the ISP by configuring the media links
appropriately.

The current limitation of this model is that it can't be easily configured
per-frame, as media configurations are not included in the requests yet.

[snip]

> +static int handle_ctrl_get_bin_info(struct v4l2_ctrl *ctrl, int is_width)
> +{
> +	struct mtk_cam_dev *cam_dev = ctrl->priv;
> +	struct v4l2_format *fmt;
> +
> +	fmt = &cam_dev->vdev_nodes[MTK_CAM_P1_MAIN_STREAM_OUT].vdev_fmt;
> +
> +	dev_dbg(&cam_dev->pdev->dev, "Get bin info w*h:%d*%d is_width:%d",
> +		fmt->fmt.pix_mp.width, fmt->fmt.pix_mp.height, is_width);
> +
> +	if (is_width)
> +		ctrl->val = fmt->fmt.pix_mp.width;
> +	else
> +		ctrl->val = fmt->fmt.pix_mp.height;

This seems to contradict to what the comment in the header says, because it just
always returns the video node format and doesn't seem to care about whether
binning is enabled or not.

> +
> +	return 0;
> +}
> +
> +static int handle_ctrl_get_process_raw(struct v4l2_ctrl *ctrl)
> +{
> +	struct mtk_cam_dev *cam_dev = ctrl->priv;
> +	struct isp_p1_device *p1_dev = get_p1_device(&cam_dev->pdev->dev);
> +
> +	ctrl->val = (p1_dev->isp_ctx.isp_raw_path == ISP_PROCESS_RAW_PATH);
> +
> +	dev_dbg(&cam_dev->pdev->dev, "Get process raw:%d", ctrl->val);
> +
> +	return 0;
> +}
> +
> +static int handle_ctrl_set_process_raw(struct v4l2_ctrl *ctrl)
> +{
> +	struct mtk_cam_dev *cam_dev = ctrl->priv;
> +	struct isp_p1_device *p1_dev = get_p1_device(&cam_dev->pdev->dev);
> +
> +	p1_dev->isp_ctx.isp_raw_path = (ctrl->val) ?
> +		ISP_PROCESS_RAW_PATH : ISP_PURE_RAW_PATH;
> +	dev_dbg(&cam_dev->pdev->dev, "Set process raw:%d", ctrl->val);
> +	return 0;
> +}
> +
> +static int mtk_cam_dev_g_ctrl(struct v4l2_ctrl *ctrl)

This is g_volatile_ctrl not, g_ctrl.

> +{
> +	switch (ctrl->id) {
> +	case V4L2_CID_MTK_PROCESSING_RAW:
> +		handle_ctrl_get_process_raw(ctrl);
> +		break;

No need to provide getters for non-volatile controls. The
framework manages them.

> +	case V4L2_CID_MTK_GET_BIN_WIDTH:
> +		handle_ctrl_get_bin_info(ctrl, 1);
> +		break;
> +	case V4L2_CID_MTK_GET_BIN_HEIGTH:
> +		handle_ctrl_get_bin_info(ctrl, 0);

It's trivial to get the value, so there isn't much benefit in having a
function to do so, especially if one needs something like a is_width
argument that further complicates the code.

> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int mtk_cam_dev_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	switch (ctrl->id) {
> +	case V4L2_CID_MTK_PROCESSING_RAW:
> +		return handle_ctrl_set_process_raw(ctrl);

Same as above. The operation is too trivial to deserve a function.

> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct v4l2_ctrl_ops mtk_cam_dev_ctrl_ops = {
> +	.g_volatile_ctrl = mtk_cam_dev_g_ctrl,
> +	.s_ctrl = mtk_cam_dev_s_ctrl,
> +};
> +
> +struct v4l2_ctrl_config mtk_cam_controls[] = {
> +	{
> +	.ops = &mtk_cam_dev_ctrl_ops,
> +	.id = V4L2_CID_MTK_PROCESSING_RAW,
> +	.name = "MTK CAM PROCESSING RAW",
> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
> +	.min = 0,
> +	.max = 1,
> +	.step = 1,
> +	.def = 1,
> +	},
> +	{
> +	.ops = &mtk_cam_dev_ctrl_ops,
> +	.id = V4L2_CID_MTK_GET_BIN_WIDTH,
> +	.name = "MTK CAM GET BIN WIDTH",
> +	.type = V4L2_CTRL_TYPE_INTEGER,
> +	.min = IMG_MIN_WIDTH,
> +	.max = IMG_MAX_WIDTH,
> +	.step = 1,
> +	.def = IMG_MAX_WIDTH,
> +	.flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
> +	},
> +	{
> +	.ops = &mtk_cam_dev_ctrl_ops,
> +	.id = V4L2_CID_MTK_GET_BIN_HEIGTH,
> +	.name = "MTK CAM GET BIN HEIGHT",
> +	.type = V4L2_CTRL_TYPE_INTEGER,
> +	.min = IMG_MIN_HEIGHT,
> +	.max = IMG_MAX_HEIGHT,
> +	.step = 1,
> +	.def = IMG_MAX_HEIGHT,
> +	.flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
> +	},
> +};
> +
> +int mtk_cam_ctrl_init(struct mtk_cam_dev *cam_dev,
> +		      struct v4l2_ctrl_handler *hdl)
> +{
> +	unsigned int i;
> +
> +	/* Initialized HW controls, allow V4L2_CID_MTK_CAM_MAX ctrls */
> +	v4l2_ctrl_handler_init(hdl, V4L2_CID_MTK_CAM_MAX);
> +	if (hdl->error) {

This should be checked at the end, after all the controls are added.

Best regards,
Tomasz
Jungo Lin July 2, 2019, 11:34 a.m. UTC | #2
Hi Tomasz,

On Mon, 2019-07-01 at 14:50 +0900, Tomasz Figa wrote:
> Hi Jungo,
> 
> On Tue, Jun 11, 2019 at 11:53:40AM +0800, Jungo Lin wrote:
> > Reserved Mediatek ISP P1 V4L2 control number with 16.
> > Moreover, add two V4L2 controls for ISP P1 user space
> > usage.
> > 
> > 1. V4L2_CID_MTK_GET_BIN_INFO
> > - Provide the image output width & height in case
> > camera binning mode is enabled.
> 
> Could you explain with a bit more details what these binned width and height
> would mean? How would they relate to the video CAPTURE node width and height?
> Isn't this something that should be rather exposed as an appropriate
> selection rectangle, instead of custom controls?
> 

Frontal binning is Mediatek ISP P1 HW function and it is designed for
low power saving. The input sensor resolution will be divided by 2
including width & height in the source side of ISP HW and lower the
overall ISP throughput. So the ISP clock could be running in the low
speed for power saving. However, the image quality will be bad in this
mode. If the frontal binning is enabled by ISP driver, user space 3A
algorithm needs to know the new resolution of TG. Btw, current ISP
driver doesn't support this function. We just export this V4L2 contorl
for future support. But, after internal discussing, we will remove this
v4l2 control in next patch.

> > 
> > 2. V4L2_CID_MTK_RAW_PATH
> > - Export the path control of the main stream to user space.
> > One is pure raw and the other is processing raw.
> > The default value is 0 which outputs the pure raw bayer image
> > from sesnor, without image processing in ISP HW.
> 
> Is it just effectively a full processing bypass? The driver seems to only
> update the related configuration when the streaming starts. Can it be
> controlled per-frame?
> 
> Generally this sounds more like something that should be modelled using the
> media topology, similar to the example below.
> 
> /----------------\   /-------------------\   /--------------\
> |                |---|                   |   |              |
> | Capture Subdev |   | Processing Subdev |-o-| CAPTURE node |
> |                |-\ |                   | | |              |
> \----------------/ | \-------------------/ | \--------------/
>                    |                       |
>                    \-----------------------/
> 
> Then the userspace can select whether it wants the image from the capture
> interface directly or procesed by the ISP by configuring the media links
> appropriately.
> 
> The current limitation of this model is that it can't be easily configured
> per-frame, as media configurations are not included in the requests yet.
> 
> [snip]
> 

For this V4L2 control, it is designed for per stream, not per frame and
just implemented for future usage. As the same conclusion with the above
one, we will remove this, either in current version. If we have strong
requirement on this, we could adopt your suggestion to use media
topology to per stream control.


> > +static int handle_ctrl_get_bin_info(struct v4l2_ctrl *ctrl, int is_width)
> > +{
> > +	struct mtk_cam_dev *cam_dev = ctrl->priv;
> > +	struct v4l2_format *fmt;
> > +
> > +	fmt = &cam_dev->vdev_nodes[MTK_CAM_P1_MAIN_STREAM_OUT].vdev_fmt;
> > +
> > +	dev_dbg(&cam_dev->pdev->dev, "Get bin info w*h:%d*%d is_width:%d",
> > +		fmt->fmt.pix_mp.width, fmt->fmt.pix_mp.height, is_width);
> > +
> > +	if (is_width)
> > +		ctrl->val = fmt->fmt.pix_mp.width;
> > +	else
> > +		ctrl->val = fmt->fmt.pix_mp.height;
> 
> This seems to contradict to what the comment in the header says, because it just
> always returns the video node format and doesn't seem to care about whether
> binning is enabled or not.
> 

This is because binning mode is not supported in current driver's
implementation and no request on this function. We just create this for
future usage. In order to avoid confusion, we will remove this.

> > +
> > +	return 0;
> > +}
> > +
> > +static int handle_ctrl_get_process_raw(struct v4l2_ctrl *ctrl)
> > +{
> > +	struct mtk_cam_dev *cam_dev = ctrl->priv;
> > +	struct isp_p1_device *p1_dev = get_p1_device(&cam_dev->pdev->dev);
> > +
> > +	ctrl->val = (p1_dev->isp_ctx.isp_raw_path == ISP_PROCESS_RAW_PATH);
> > +
> > +	dev_dbg(&cam_dev->pdev->dev, "Get process raw:%d", ctrl->val);
> > +
> > +	return 0;
> > +}
> > +
> > +static int handle_ctrl_set_process_raw(struct v4l2_ctrl *ctrl)
> > +{
> > +	struct mtk_cam_dev *cam_dev = ctrl->priv;
> > +	struct isp_p1_device *p1_dev = get_p1_device(&cam_dev->pdev->dev);
> > +
> > +	p1_dev->isp_ctx.isp_raw_path = (ctrl->val) ?
> > +		ISP_PROCESS_RAW_PATH : ISP_PURE_RAW_PATH;
> > +	dev_dbg(&cam_dev->pdev->dev, "Set process raw:%d", ctrl->val);
> > +	return 0;
> > +}
> > +
> > +static int mtk_cam_dev_g_ctrl(struct v4l2_ctrl *ctrl)
> 
> This is g_volatile_ctrl not, g_ctrl.
> 

Ok, thanks for your correction.

> > +{
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_MTK_PROCESSING_RAW:
> > +		handle_ctrl_get_process_raw(ctrl);
> > +		break;
> 
> No need to provide getters for non-volatile controls. The
> framework manages them.
> 

Ok, thanks for your correction.

> > +	case V4L2_CID_MTK_GET_BIN_WIDTH:
> > +		handle_ctrl_get_bin_info(ctrl, 1);
> > +		break;
> > +	case V4L2_CID_MTK_GET_BIN_HEIGTH:
> > +		handle_ctrl_get_bin_info(ctrl, 0);
> 
> It's trivial to get the value, so there isn't much benefit in having a
> function to do so, especially if one needs something like a is_width
> argument that further complicates the code.
> 

Ok, we will pay attention in this of implementation next time.

> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int mtk_cam_dev_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_MTK_PROCESSING_RAW:
> > +		return handle_ctrl_set_process_raw(ctrl);
> 
> Same as above. The operation is too trivial to deserve a function.
> 

Ok, got it.

> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_cam_dev_ctrl_ops = {
> > +	.g_volatile_ctrl = mtk_cam_dev_g_ctrl,
> > +	.s_ctrl = mtk_cam_dev_s_ctrl,
> > +};
> > +
> > +struct v4l2_ctrl_config mtk_cam_controls[] = {
> > +	{
> > +	.ops = &mtk_cam_dev_ctrl_ops,
> > +	.id = V4L2_CID_MTK_PROCESSING_RAW,
> > +	.name = "MTK CAM PROCESSING RAW",
> > +	.type = V4L2_CTRL_TYPE_BOOLEAN,
> > +	.min = 0,
> > +	.max = 1,
> > +	.step = 1,
> > +	.def = 1,
> > +	},
> > +	{
> > +	.ops = &mtk_cam_dev_ctrl_ops,
> > +	.id = V4L2_CID_MTK_GET_BIN_WIDTH,
> > +	.name = "MTK CAM GET BIN WIDTH",
> > +	.type = V4L2_CTRL_TYPE_INTEGER,
> > +	.min = IMG_MIN_WIDTH,
> > +	.max = IMG_MAX_WIDTH,
> > +	.step = 1,
> > +	.def = IMG_MAX_WIDTH,
> > +	.flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
> > +	},
> > +	{
> > +	.ops = &mtk_cam_dev_ctrl_ops,
> > +	.id = V4L2_CID_MTK_GET_BIN_HEIGTH,
> > +	.name = "MTK CAM GET BIN HEIGHT",
> > +	.type = V4L2_CTRL_TYPE_INTEGER,
> > +	.min = IMG_MIN_HEIGHT,
> > +	.max = IMG_MAX_HEIGHT,
> > +	.step = 1,
> > +	.def = IMG_MAX_HEIGHT,
> > +	.flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
> > +	},
> > +};
> > +
> > +int mtk_cam_ctrl_init(struct mtk_cam_dev *cam_dev,
> > +		      struct v4l2_ctrl_handler *hdl)
> > +{
> > +	unsigned int i;
> > +
> > +	/* Initialized HW controls, allow V4L2_CID_MTK_CAM_MAX ctrls */
> > +	v4l2_ctrl_handler_init(hdl, V4L2_CID_MTK_CAM_MAX);
> > +	if (hdl->error) {
> 
> This should be checked at the end, after all the controls are added.
> 

Ok, got it.

> Best regards,
> Tomasz
> 

Based on above comments, we will remove mtk_cam_ctrl.h & mtk_cam_ctrl.c
in next patch to avoid over design.

Thanks for your comments.

Jungo
diff mbox series

Patch

diff --git a/drivers/media/platform/mtk-isp/Makefile b/drivers/media/platform/mtk-isp/Makefile
new file mode 100644
index 000000000000..c17fb3fc3340
--- /dev/null
+++ b/drivers/media/platform/mtk-isp/Makefile
@@ -0,0 +1,3 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+obj-y += isp_50/
diff --git a/drivers/media/platform/mtk-isp/isp_50/Makefile b/drivers/media/platform/mtk-isp/isp_50/Makefile
new file mode 100644
index 000000000000..8498fe70e418
--- /dev/null
+++ b/drivers/media/platform/mtk-isp/isp_50/Makefile
@@ -0,0 +1,5 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+ifeq ($(CONFIG_VIDEO_MEDIATEK_ISP_PASS1),y)
+obj-y += cam/
+endif
diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/Makefile b/drivers/media/platform/mtk-isp/isp_50/cam/Makefile
new file mode 100644
index 000000000000..53fb69d3add6
--- /dev/null
+++ b/drivers/media/platform/mtk-isp/isp_50/cam/Makefile
@@ -0,0 +1,5 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+mtk-cam-isp-objs += mtk_cam-ctrl.o
+
+obj-$(CONFIG_VIDEO_MEDIATEK_ISP_PASS1) += mtk-cam-isp.o
\ No newline at end of file
diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-ctrl.c b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-ctrl.c
new file mode 100644
index 000000000000..31d801c82495
--- /dev/null
+++ b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-ctrl.c
@@ -0,0 +1,138 @@ 
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (c) 2018 MediaTek Inc.
+
+#include <linux/device.h>
+#include <linux/platform_device.h>
+
+#include "mtk_cam-ctrl.h"
+#include "mtk_cam.h"
+
+static int handle_ctrl_get_bin_info(struct v4l2_ctrl *ctrl, int is_width)
+{
+	struct mtk_cam_dev *cam_dev = ctrl->priv;
+	struct v4l2_format *fmt;
+
+	fmt = &cam_dev->vdev_nodes[MTK_CAM_P1_MAIN_STREAM_OUT].vdev_fmt;
+
+	dev_dbg(&cam_dev->pdev->dev, "Get bin info w*h:%d*%d is_width:%d",
+		fmt->fmt.pix_mp.width, fmt->fmt.pix_mp.height, is_width);
+
+	if (is_width)
+		ctrl->val = fmt->fmt.pix_mp.width;
+	else
+		ctrl->val = fmt->fmt.pix_mp.height;
+
+	return 0;
+}
+
+static int handle_ctrl_get_process_raw(struct v4l2_ctrl *ctrl)
+{
+	struct mtk_cam_dev *cam_dev = ctrl->priv;
+	struct isp_p1_device *p1_dev = get_p1_device(&cam_dev->pdev->dev);
+
+	ctrl->val = (p1_dev->isp_ctx.isp_raw_path == ISP_PROCESS_RAW_PATH);
+
+	dev_dbg(&cam_dev->pdev->dev, "Get process raw:%d", ctrl->val);
+
+	return 0;
+}
+
+static int handle_ctrl_set_process_raw(struct v4l2_ctrl *ctrl)
+{
+	struct mtk_cam_dev *cam_dev = ctrl->priv;
+	struct isp_p1_device *p1_dev = get_p1_device(&cam_dev->pdev->dev);
+
+	p1_dev->isp_ctx.isp_raw_path = (ctrl->val) ?
+		ISP_PROCESS_RAW_PATH : ISP_PURE_RAW_PATH;
+	dev_dbg(&cam_dev->pdev->dev, "Set process raw:%d", ctrl->val);
+	return 0;
+}
+
+static int mtk_cam_dev_g_ctrl(struct v4l2_ctrl *ctrl)
+{
+	switch (ctrl->id) {
+	case V4L2_CID_MTK_PROCESSING_RAW:
+		handle_ctrl_get_process_raw(ctrl);
+		break;
+	case V4L2_CID_MTK_GET_BIN_WIDTH:
+		handle_ctrl_get_bin_info(ctrl, 1);
+		break;
+	case V4L2_CID_MTK_GET_BIN_HEIGTH:
+		handle_ctrl_get_bin_info(ctrl, 0);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int mtk_cam_dev_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	switch (ctrl->id) {
+	case V4L2_CID_MTK_PROCESSING_RAW:
+		return handle_ctrl_set_process_raw(ctrl);
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct v4l2_ctrl_ops mtk_cam_dev_ctrl_ops = {
+	.g_volatile_ctrl = mtk_cam_dev_g_ctrl,
+	.s_ctrl = mtk_cam_dev_s_ctrl,
+};
+
+struct v4l2_ctrl_config mtk_cam_controls[] = {
+	{
+	.ops = &mtk_cam_dev_ctrl_ops,
+	.id = V4L2_CID_MTK_PROCESSING_RAW,
+	.name = "MTK CAM PROCESSING RAW",
+	.type = V4L2_CTRL_TYPE_BOOLEAN,
+	.min = 0,
+	.max = 1,
+	.step = 1,
+	.def = 1,
+	},
+	{
+	.ops = &mtk_cam_dev_ctrl_ops,
+	.id = V4L2_CID_MTK_GET_BIN_WIDTH,
+	.name = "MTK CAM GET BIN WIDTH",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.min = IMG_MIN_WIDTH,
+	.max = IMG_MAX_WIDTH,
+	.step = 1,
+	.def = IMG_MAX_WIDTH,
+	.flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
+	},
+	{
+	.ops = &mtk_cam_dev_ctrl_ops,
+	.id = V4L2_CID_MTK_GET_BIN_HEIGTH,
+	.name = "MTK CAM GET BIN HEIGHT",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.min = IMG_MIN_HEIGHT,
+	.max = IMG_MAX_HEIGHT,
+	.step = 1,
+	.def = IMG_MAX_HEIGHT,
+	.flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
+	},
+};
+
+int mtk_cam_ctrl_init(struct mtk_cam_dev *cam_dev,
+		      struct v4l2_ctrl_handler *hdl)
+{
+	unsigned int i;
+
+	/* Initialized HW controls, allow V4L2_CID_MTK_CAM_MAX ctrls */
+	v4l2_ctrl_handler_init(hdl, V4L2_CID_MTK_CAM_MAX);
+	if (hdl->error) {
+		v4l2_ctrl_handler_free(hdl);
+		return hdl->error;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(mtk_cam_controls); i++)
+		v4l2_ctrl_new_custom(hdl, &mtk_cam_controls[i], cam_dev);
+
+	dev_dbg(&cam_dev->pdev->dev, "%s done", __func__);
+
+	return 0;
+}
diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-ctrl.h b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-ctrl.h
new file mode 100644
index 000000000000..0f9349ae0b07
--- /dev/null
+++ b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-ctrl.h
@@ -0,0 +1,38 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ */
+
+#ifndef __MTK_CAM_CTRL_H__
+#define __MTK_CAM_CTRL_H__
+
+#include <media/v4l2-ctrls.h>
+
+#include "mtk_cam-v4l2-util.h"
+
+/* The base for the MTK Camera ISP P1 driver controls.
+ * We reserve 16 controls for this driver.
+ */
+#define V4L2_CID_MTK_CAM_BASE			V4L2_CID_USER_MTK_CAM_BASE
+
+/* Control MTK ISP P1 main stream to process raw image data or not.
+ * The default value is 0 which outputs the pure raw bayer data from sensor,
+ * without image processing in ISP HW.
+ */
+#define V4L2_CID_MTK_PROCESSING_RAW		(V4L2_CID_MTK_CAM_BASE + 1)
+
+/* MTK ISP P1 HW supports frontal binning function.
+ * If this function is enabled, the 3A algo. may get the new image resolution
+ * which is binned by ISP P1. If this function is disabled or no supported,
+ * the image resolution will be equal to configured image format.
+ * For this control, it is read only.
+ */
+#define V4L2_CID_MTK_GET_BIN_WIDTH		(V4L2_CID_MTK_CAM_BASE + 2)
+#define V4L2_CID_MTK_GET_BIN_HEIGTH		(V4L2_CID_MTK_CAM_BASE + 3)
+
+#define V4L2_CID_MTK_CAM_MAX			16
+
+int mtk_cam_ctrl_init(struct mtk_cam_dev *cam_dev,
+		      struct v4l2_ctrl_handler *hdl);
+
+#endif /* __MTK_CAM_CTRL_H__ */
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 37807f23231e..2db99716f40d 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -192,6 +192,10 @@  enum v4l2_colorfx {
  * We reserve 16 controls for this driver. */
 #define V4L2_CID_USER_IMX_BASE			(V4L2_CID_USER_BASE + 0x10b0)
 
+/* The base for the mediatek ISP Pass 1 driver controls */
+/* We reserve 16 controls for this driver. */
+#define V4L2_CID_USER_MTK_CAM_BASE		(V4L2_CID_USER_BASE + 0x10c0)
+
 /* MPEG-class control IDs */
 /* The MPEG controls are applicable to all codec controls
  * and the 'MPEG' part of the define is historical */