Linux Media Controller development
 help / color / mirror / Atom feed
* [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-05 10:54 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
	mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
	bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu
In-Reply-To: <20200605105412.18813-1-dongchun.zhu@mediatek.com>

Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
control to set the desired focus via IIC serial interface.

Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
 MAINTAINERS                |   1 +
 drivers/media/i2c/Kconfig  |  13 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9768.c | 566 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 581 insertions(+)
 create mode 100644 drivers/media/i2c/dw9768.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d72c41..c92dc99 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5157,6 +5157,7 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
+F:	drivers/media/i2c/dw9768.c
 
 DONGWOON DW9807 LENS VOICE COIL DRIVER
 M:	Sakari Ailus <sakari.ailus@linux.intel.com>
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 125d596..afdf994 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1040,6 +1040,19 @@ config VIDEO_DW9714
 	  capability. This is designed for linear control of
 	  voice coil motors, controlled via I2C serial interface.
 
+config VIDEO_DW9768
+	tristate "DW9768 lens voice coil support"
+	depends on I2C && VIDEO_V4L2
+	depends on PM
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select V4L2_FWNODE
+	help
+	  This is a driver for the DW9768 camera lens voice coil.
+	  DW9768 is a 10 bit DAC with 100mA output current sink
+	  capability. This is designed for linear control of
+	  voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_DW9807_VCM
 	tristate "DW9807 lens voice coil support"
 	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 77bf7d0..4057476 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
 obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
+obj-$(CONFIG_VIDEO_DW9768)  += dw9768.o
 obj-$(CONFIG_VIDEO_DW9807_VCM)  += dw9807-vcm.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c
new file mode 100644
index 0000000..f34a8ed
--- /dev/null
+++ b/drivers/media/i2c/dw9768.c
@@ -0,0 +1,566 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 MediaTek Inc.
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+
+#define DW9768_NAME				"dw9768"
+#define DW9768_MAX_FOCUS_POS			(1024 - 1)
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define DW9768_FOCUS_STEPS			1
+
+/*
+ * Ring control and Power control register
+ * Bit[1] RING_EN
+ * 0: Direct mode
+ * 1: AAC mode (ringing control mode)
+ * Bit[0] PD
+ * 0: Normal operation mode
+ * 1: Power down mode
+ * DW9768 requires waiting time of Topr after PD reset takes place.
+ */
+#define DW9768_RING_PD_CONTROL_REG		0x02
+#define DW9768_PD_MODE_OFF			0x00
+#define DW9768_PD_MODE_EN			BIT(0)
+#define DW9768_AAC_MODE_EN			BIT(1)
+
+/*
+ * DW9768 separates two registers to control the VCM position.
+ * One for MSB value, another is LSB value.
+ * DAC_MSB: D[9:8] (ADD: 0x03)
+ * DAC_LSB: D[7:0] (ADD: 0x04)
+ * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
+ */
+#define DW9768_MSB_ADDR				0x03
+#define DW9768_LSB_ADDR				0x04
+#define DW9768_STATUS_ADDR			0x05
+
+/*
+ * AAC mode control & prescale register
+ * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
+ * 001 AAC2 0.48 x Tvib
+ * 010 AAC3 0.70 x Tvib
+ * 011 AAC4 0.75 x Tvib
+ * 101 AAC8 1.13 x Tvib
+ * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
+ * 000 2
+ * 001 1
+ * 010 1/2
+ * 011 1/4
+ * 100 8
+ * 101 4
+ */
+#define DW9768_AAC_PRESC_REG			0x06
+#define DW9768_AAC_MODE_SEL_MASK		GENMASK(7, 5)
+#define DW9768_CLOCK_PRE_SCALE_SEL_MASK		GENMASK(2, 0)
+
+/*
+ * VCM period of vibration register
+ * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
+ * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
+ * Dividing Rate is the internal clock dividing rate that is defined at
+ * PRESCALE register (ADD: 0x06)
+ */
+#define DW9768_AAC_TIME_REG			0x07
+
+/*
+ * DW9768 requires waiting time (delay time) of t_OPR after power-up,
+ * or in the case of PD reset taking place.
+ */
+#define DW9768_T_OPR_US				1000
+#define DW9768_Tvib_MS_BASE10			(64 - 1)
+#define DW9768_AAC_MODE_DEFAULT			2
+#define DW9768_AAC_TIME_DEFAULT			0x20
+#define DW9768_CLOCK_PRE_SCALE_DEFAULT		1
+
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define DW9768_MOVE_STEPS			16
+
+static const char * const dw9768_supply_names[] = {
+	"vin",	/* Digital I/O power */
+	"vdd",	/* Digital core power */
+};
+
+/* dw9768 device structure */
+struct dw9768 {
+	struct regulator_bulk_data supplies[ARRAY_SIZE(dw9768_supply_names)];
+	struct v4l2_ctrl_handler ctrls;
+	struct v4l2_ctrl *focus;
+	struct v4l2_subdev sd;
+
+	u32 aac_mode;
+	u32 aac_timing;
+	u32 clock_presc;
+};
+
+static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
+{
+	return container_of(subdev, struct dw9768, sd);
+}
+
+struct regval_list {
+	u8 reg_num;
+	u8 value;
+};
+
+struct dw9768_aac_mode_ot_multi {
+	u32 aac_mode_enum;
+	u32 ot_multi_base100;
+};
+
+struct dw9768_clk_presc_dividing_rate {
+	u32 clk_presc_enum;
+	u32 dividing_rate_base100;
+};
+
+static const struct dw9768_aac_mode_ot_multi aac_mode_ot_multi[] = {
+	{1,  48},
+	{2,  70},
+	{3,  75},
+	{5, 113},
+};
+
+static const struct dw9768_clk_presc_dividing_rate presc_dividing_rate[] = {
+	{0, 200},
+	{1, 100},
+	{2,  50},
+	{3,  25},
+	{4, 800},
+	{5, 400},
+};
+
+static u32 dw9768_find_ot_multi(u32 aac_mode_param)
+{
+	u32 cur_ot_multi_base100 = 70;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(aac_mode_ot_multi); i++) {
+		if (aac_mode_ot_multi[i].aac_mode_enum == aac_mode_param) {
+			cur_ot_multi_base100 =
+				aac_mode_ot_multi[i].ot_multi_base100;
+		}
+	}
+
+	return cur_ot_multi_base100;
+}
+
+static u32 dw9768_find_dividing_rate(u32 presc_param)
+{
+	u32 cur_clk_dividing_rate_base100 = 100;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(presc_dividing_rate); i++) {
+		if (presc_dividing_rate[i].clk_presc_enum == presc_param) {
+			cur_clk_dividing_rate_base100 =
+				presc_dividing_rate[i].dividing_rate_base100;
+		}
+	}
+
+	return cur_clk_dividing_rate_base100;
+}
+
+/*
+ * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
+ * For current VCM mode: AAC3, Operation Time would be 0.70 x Tvib.
+ * Tvib = (6.3ms + AACT[5:0] * 0.1MS) * Dividing Rate.
+ * Below is calculation of the operation delay for each step.
+ */
+static inline u32 dw9768_cal_move_delay(u32 aac_mode_param, u32 presc_param,
+					u32 aac_timing_param)
+{
+	u32 Tvib_us;
+	u32 ot_multi_base100;
+	u32 clk_dividing_rate_base100;
+
+	ot_multi_base100 = dw9768_find_ot_multi(aac_mode_param);
+
+	clk_dividing_rate_base100 = dw9768_find_dividing_rate(presc_param);
+
+	Tvib_us = (DW9768_Tvib_MS_BASE10 + aac_timing_param) *
+		  clk_dividing_rate_base100;
+
+	return Tvib_us * ot_multi_base100;
+}
+
+static int dw9768_mod_reg(struct dw9768 *dw9768, u8 reg, u8 mask, u8 val)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client, reg);
+	if (ret < 0)
+		return ret;
+
+	val = ((unsigned char)ret & ~mask) | (val & mask);
+
+	return i2c_smbus_write_byte_data(client, reg, val);
+}
+
+static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+
+	/* Write VCM position to registers */
+	return i2c_smbus_write_word_swapped(client, DW9768_MSB_ADDR, val);
+}
+
+static int dw9768_init(struct dw9768 *dw9768)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+	u32 move_delay_us;
+	int ret, val;
+
+	/* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
+	ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+					DW9768_PD_MODE_OFF);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * DW9769 requires waiting delay time of t_OPR
+	 * after PD reset takes place.
+	 */
+	usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+	/* Set DW9768_RING_PD_CONTROL_REG to DW9768_AAC_MODE_EN(0x01) */
+	ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+					DW9768_AAC_MODE_EN);
+	if (ret < 0)
+		return ret;
+
+	/* Set AAC mode */
+	ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
+			     DW9768_AAC_MODE_SEL_MASK,
+			     dw9768->aac_mode << 5);
+	if (ret < 0)
+		return ret;
+
+	/* Set clock presc */
+	if (dw9768->clock_presc != DW9768_CLOCK_PRE_SCALE_DEFAULT) {
+		ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
+				     DW9768_CLOCK_PRE_SCALE_SEL_MASK,
+				     dw9768->clock_presc);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Set AAC Timing */
+	if (dw9768->aac_timing != DW9768_AAC_TIME_DEFAULT) {
+		ret = i2c_smbus_write_byte_data(client, DW9768_AAC_TIME_REG,
+						dw9768->aac_timing);
+		if (ret < 0)
+			return ret;
+	}
+
+	move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
+					      dw9768->clock_presc,
+					      dw9768->aac_timing) / 100;
+
+	for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
+	     val <= dw9768->focus->val;
+	     val += DW9768_MOVE_STEPS) {
+		ret = dw9768_set_dac(dw9768, val);
+		if (ret) {
+			dev_err(&client->dev, "%s I2C failure: %d",
+				__func__, ret);
+			return ret;
+		}
+		usleep_range(move_delay_us, move_delay_us + 1000);
+	}
+
+	return 0;
+}
+
+static int dw9768_release(struct dw9768 *dw9768)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+	u32 move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
+						  dw9768->clock_presc,
+						  dw9768->aac_timing) / 100;
+	int ret, val;
+
+	val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
+	for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
+		ret = dw9768_set_dac(dw9768, val);
+		if (ret) {
+			dev_err(&client->dev, "I2C write fail: %d", ret);
+			return ret;
+		}
+		usleep_range(move_delay_us, move_delay_us + 1000);
+	}
+
+	ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+					DW9768_PD_MODE_EN);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * DW9769 requires waiting delay time of t_OPR
+	 * after PD reset takes place.
+	 */
+	usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+	return 0;
+}
+
+static int dw9768_runtime_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct dw9768 *dw9768 = sd_to_dw9768(sd);
+
+	dw9768_release(dw9768);
+	regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
+			       dw9768->supplies);
+
+	return 0;
+}
+
+static int dw9768_runtime_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct dw9768 *dw9768 = sd_to_dw9768(sd);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(dw9768_supply_names),
+				    dw9768->supplies);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable regulators\n");
+		return ret;
+	}
+
+	/*
+	 * The datasheet refers to t_OPR that needs to be waited before sending
+	 * I2C commands after power-up.
+	 */
+	usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+	ret = dw9768_init(dw9768);
+	if (ret < 0)
+		goto disable_regulator;
+
+	return 0;
+
+disable_regulator:
+	regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
+			       dw9768->supplies);
+
+	return ret;
+}
+
+static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct dw9768 *dw9768 = container_of(ctrl->handler,
+					     struct dw9768, ctrls);
+
+	if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+		return dw9768_set_dac(dw9768, ctrl->val);
+
+	return 0;
+}
+
+static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
+	.s_ctrl = dw9768_set_ctrl,
+};
+
+static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	int ret;
+
+	ret = pm_runtime_get_sync(sd->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(sd->dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	pm_runtime_put(sd->dev);
+
+	return 0;
+}
+
+static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
+	.open = dw9768_open,
+	.close = dw9768_close,
+};
+
+static const struct v4l2_subdev_ops dw9768_ops = { };
+
+static int dw9768_init_controls(struct dw9768 *dw9768)
+{
+	struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
+	const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
+
+	v4l2_ctrl_handler_init(hdl, 1);
+
+	dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE, 0,
+					  DW9768_MAX_FOCUS_POS,
+					  DW9768_FOCUS_STEPS, 0);
+
+	if (hdl->error)
+		return hdl->error;
+
+	dw9768->sd.ctrl_handler = hdl;
+
+	return 0;
+}
+
+static int dw9768_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct dw9768 *dw9768;
+	u32 aac_mode_select;
+	u32 aac_timing_select;
+	u32 clock_presc_select;
+	unsigned int i;
+	int ret;
+
+	dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
+	if (!dw9768)
+		return -ENOMEM;
+
+	/* Initialize subdev */
+	v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
+
+	dw9768->aac_mode = DW9768_AAC_MODE_DEFAULT;
+	dw9768->aac_timing = DW9768_AAC_TIME_DEFAULT;
+	dw9768->clock_presc = DW9768_CLOCK_PRE_SCALE_DEFAULT;
+
+	/* Optional indication of AAC mode select */
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-mode",
+				       &aac_mode_select);
+
+	if (!ret)
+		dw9768->aac_mode = aac_mode_select;
+
+	/* Optional indication of clock pre-scale select */
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,clock-presc",
+				       &clock_presc_select);
+
+	if (!ret)
+		dw9768->clock_presc = clock_presc_select;
+
+	/* Optional indication of AAC Timing */
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-timing",
+				       &aac_timing_select);
+
+	if (!ret)
+		dw9768->aac_timing = aac_timing_select;
+
+	for (i = 0; i < ARRAY_SIZE(dw9768_supply_names); i++)
+		dw9768->supplies[i].supply = dw9768_supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dw9768_supply_names),
+				      dw9768->supplies);
+	if (ret) {
+		dev_err(dev, "failed to get regulators\n");
+		return ret;
+	}
+
+	/* Initialize controls */
+	ret = dw9768_init_controls(dw9768);
+	if (ret)
+		goto err_free_handler;
+
+	/* Initialize subdev */
+	dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	dw9768->sd.internal_ops = &dw9768_int_ops;
+
+	ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
+	if (ret < 0)
+		goto err_free_handler;
+
+	dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
+
+	pm_runtime_enable(dev);
+	if (!pm_runtime_enabled(dev)) {
+		ret = dw9768_runtime_resume(dev);
+		if (ret < 0) {
+			dev_err(dev, "failed to power on: %d\n", ret);
+			goto err_clean_entity;
+		}
+	}
+
+	ret = v4l2_async_register_subdev(&dw9768->sd);
+	if (ret < 0) {
+		dev_err(dev, "failed to register V4L2 subdev: %d", ret);
+		goto error_async_register;
+	}
+
+	return 0;
+
+error_async_register:
+	if (!pm_runtime_enabled(dev))
+		dw9768_runtime_suspend(dev);
+err_clean_entity:
+	media_entity_cleanup(&dw9768->sd.entity);
+err_free_handler:
+	v4l2_ctrl_handler_free(&dw9768->ctrls);
+
+	return ret;
+}
+
+static int dw9768_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct dw9768 *dw9768 = sd_to_dw9768(sd);
+
+	v4l2_async_unregister_subdev(&dw9768->sd);
+	v4l2_ctrl_handler_free(&dw9768->ctrls);
+	media_entity_cleanup(&dw9768->sd.entity);
+	pm_runtime_disable(&client->dev);
+	if (!pm_runtime_status_suspended(&client->dev))
+		dw9768_runtime_suspend(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+
+	return 0;
+}
+
+static const struct of_device_id dw9768_of_table[] = {
+	{ .compatible = "dongwoon,dw9768" },
+	{ .compatible = "giantec,gt9769" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, dw9768_of_table);
+
+static const struct dev_pm_ops dw9768_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
+};
+
+static struct i2c_driver dw9768_i2c_driver = {
+	.driver = {
+		.name = DW9768_NAME,
+		.pm = &dw9768_pm_ops,
+		.of_match_table = dw9768_of_table,
+	},
+	.probe_new  = dw9768_probe,
+	.remove = dw9768_remove,
+};
+module_i2c_driver(dw9768_i2c_driver);
+
+MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
+MODULE_DESCRIPTION("DW9768 VCM driver");
+MODULE_LICENSE("GPL v2");
-- 
2.9.2

^ permalink raw reply related

* [V7, 1/2] media: dt-bindings: media: i2c: Document DW9768 bindings
From: Dongchun Zhu @ 2020-06-05 10:54 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
	mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
	bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu
In-Reply-To: <20200605105412.18813-1-dongchun.zhu@mediatek.com>

Add DeviceTree binding documentation for Dongwoon Anatech DW9768 voice
coil actuator.

Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
 .../bindings/media/i2c/dongwoon,dw9768.yaml        | 100 +++++++++++++++++++++
 MAINTAINERS                                        |   7 ++
 2 files changed, 107 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
new file mode 100644
index 0000000..cb96e95
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) 2020 MediaTek Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9768.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dongwoon Anatech DW9768 Voice Coil Motor (VCM) Lens Device Tree Bindings
+
+maintainers:
+  - Dongchun Zhu <dongchun.zhu@mediatek.com>
+
+description: |-
+  The Dongwoon DW9768 is a single 10-bit digital-to-analog (DAC) converter
+  with 100 mA output current sink capability. VCM current is controlled with
+  a linear mode driver. The DAC is controlled via a 2-wire (I2C-compatible)
+  serial interface that operates at clock rates up to 1MHz. This chip
+  integrates Advanced Actuator Control (AAC) technology and is intended for
+  driving voice coil lenses in camera modules.
+
+properties:
+  compatible:
+    enum:
+      - dongwoon,dw9768 # for DW9768 VCM
+      - giantec,gt9769  # for GT9769 VCM
+
+  reg:
+    maxItems: 1
+
+  vin-supply:
+    description:
+      Definition of the regulator used as Digital I/O voltage supply.
+
+  vdd-supply:
+    description:
+      Definition of the regulator used as Digital core voltage supply.
+
+  dongwoon,aac-mode:
+    description:
+      Indication of AAC mode select.
+    allOf:
+      - $ref: "/schemas/types.yaml#/definitions/uint32"
+      - enum:
+          - 1    #  AAC2 mode(operation time# 0.48 x Tvib)
+          - 2    #  AAC3 mode(operation time# 0.70 x Tvib)
+          - 3    #  AAC4 mode(operation time# 0.75 x Tvib)
+          - 5    #  AAC8 mode(operation time# 1.13 x Tvib)
+        default: 2
+
+  dongwoon,aac-timing:
+    description:
+      Number of AAC Timing count that controlled by one 6-bit period of
+      vibration register AACT[5:0], the unit of which is 100 us.
+    allOf:
+      - $ref: "/schemas/types.yaml#/definitions/uint32"
+      - default: 0x20
+        minimum: 0x00
+        maximum: 0x3f
+
+  dongwoon,clock-presc:
+    description:
+      Indication of VCM internal clock dividing rate select, as one multiple
+      factor to calculate VCM ring periodic time Tvib.
+    allOf:
+      - $ref: "/schemas/types.yaml#/definitions/uint32"
+      - enum:
+          - 0    #  Dividing Rate -  2
+          - 1    #  Dividing Rate -  1
+          - 2    #  Dividing Rate -  1/2
+          - 3    #  Dividing Rate -  1/4
+          - 4    #  Dividing Rate -  8
+          - 5    #  Dividing Rate -  4
+        default: 1
+
+required:
+  - compatible
+  - reg
+  - vin-supply
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dw9768: camera-lens@c {
+            compatible = "dongwoon,dw9768";
+            reg = <0x0c>;
+
+            vin-supply = <&mt6358_vcamio_reg>;
+            vdd-supply = <&mt6358_vcama2_reg>;
+            dongwoon,aac-timing = <0x39>;
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e64e5db..8d72c41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5151,6 +5151,13 @@ T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt
 F:	drivers/media/i2c/dw9714.c
 
+DONGWOON DW9768 LENS VOICE COIL DRIVER
+M:	Dongchun Zhu <dongchun.zhu@mediatek.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
+
 DONGWOON DW9807 LENS VOICE COIL DRIVER
 M:	Sakari Ailus <sakari.ailus@linux.intel.com>
 L:	linux-media@vger.kernel.org
-- 
2.9.2

^ permalink raw reply related

* [V7, 0/2] media: i2c: Add support for DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-05 10:54 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
	mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
	bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu

Hello,

This series adds DT bindings and V4L2 sub-device driver for Dongwoon's DW9768,
which is a 10-bit DAC with 100mA output current sink capability.

The driver controls the position with 10-bit DAC data D[9:0] and seperates
two 8-bit registers to control the VCM position as belows.
DAC_MSB: D[9:8](ADDR: 0x03):
     +---+---+---+---+---+---+---+---+
     |---|---|---|---|---|---|D09|D08|
     +---+---+---+---+---+---+---+---+
DAC_LSB: D[7:0](ADDR: 0x04):
     +---+---+---+---+---+---+---+---+
     |D07|D06|D05|D04|D03|D02|D01|D00|
     +---+---+---+---+---+---+---+---+

This driver supports:
 - set DW9768 to standby mode once suspend and turn it back to active if resume
 - set the desired focus via V4L2_CID_FOCUS_ABSOLUTE ctrl

Previous versions of this patch-set can be found here:
v6: https://lore.kernel.org/linux-media/20200518132731.20855-1-dongchun.zhu@mediatek.com/
v5: https://lore.kernel.org/linux-media/20200502161727.30463-1-dongchun.zhu@mediatek.com/
v4: https://lore.kernel.org/linux-media/20200330123634.363-1-dongchun.zhu@mediatek.com/
v3: https://lore.kernel.org/linux-media/20200228155958.20657-1-dongchun.zhu@mediatek.com/
v2: https://lore.kernel.org/linux-media/20190905072142.14606-1-dongchun.zhu@mediatek.com/
v1: https://lore.kernel.org/linux-media/20190708100641.2702-1-dongchun.zhu@mediatek.com/

Mainly changes of v7 are addressing comments from Rob, Sakari, Tomasz.
Compared to v6:
 - Refine DT bindings
 - Use i2c_smbus_read_byte_data() directly to replace of dw9768_read_smbus()
 - Calculate operation time based on the configured board-specific DT settings
 - Abstract async register error handling case
 - Fix other review comments in v6

Please review.
Thanks.

Dongchun Zhu (2):
  media: dt-bindings: media: i2c: Document DW9768 bindings
  media: i2c: dw9768: Add DW9768 VCM driver

 .../bindings/media/i2c/dongwoon,dw9768.yaml        | 100 ++++
 MAINTAINERS                                        |   8 +
 drivers/media/i2c/Kconfig                          |  13 +
 drivers/media/i2c/Makefile                         |   1 +
 drivers/media/i2c/dw9768.c                         | 566 +++++++++++++++++++++
 5 files changed, 688 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
 create mode 100644 drivers/media/i2c/dw9768.c

-- 
2.9.2

^ permalink raw reply

* mainline: v4l2 modules missing Kconfigs
From: Naresh Kamboju @ 2020-06-05 10:19 UTC (permalink / raw)
  To: Linux Media Mailing List, Linux Kernel Mailing List
  Cc: : Mauro Carvalho Chehab, Hans Verkuil, Sumit Semwal, Ulf Hansson,
	robert.foss, stanimir.varbanov, lkft-triage

The test v4l2-compliance modprobe-vivid failed

Test error,
VIDEO_DRIVER=vivid.ko
v4l2-existence-check pass
ln: failed to create symbolic link '/lib/modules/5.7.0+/' -> '': No
such file or directory
modprobe: FATAL: Module vivid not found in directory /lib/modules/5.7.0+
modprobe-vivid fail

The investigations shows that,
the following kernel configs are missing on latest mainline builds on
x86, i386 and arm64.
These are still found on arm config.

CONFIG_VIDEOBUF2_CORE=m
CONFIG_VIDEOBUF2_V4L2=m
CONFIG_VIDEOBUF2_MEMOPS=m
CONFIG_VIDEOBUF2_DMA_CONTIG=m
CONFIG_VIDEOBUF2_VMALLOC=m
CONFIG_VIDEO_V4L2_TPG=m

Please suggest the configs required for v4l2 testing.
and any additional test coverage.

We add V4L2 config fragments like this
# Vivid driver for V4L2 tests
CONFIG_VIDEO_VIVID=m
CONFIG_MEDIA_SUPPORT=y
CONFIG_V4L_TEST_DRIVERS=y
CONFIG_MEDIA_CAMERA_SUPPORT=y
CONFIG_VIDEO_V4L2=y
CONFIG_VIDEO_DEV=y

git branch: master
git repo: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
GOOD:
   git commit: cb8e59cc87201af93dfbb6c3dccc8fcad72a09c2
   good config: https://builds.tuxbuild.com/600ssEELJrReeIBY_a4Ghw/kernel.config
BAD:
   git commit: 6929f71e46bdddbf1c4d67c2728648176c67c555
    bad config: https://builds.tuxbuild.com/2eAqpM2VDo7nnt8HXX2Org/kernel.config

Test ref:
https://lkft.validation.linaro.org/scheduler/job/1471386#L1310

Test results comparison,
https://qa-reports.linaro.org/lkft/linux-mainline-oe/build/v5.7-8602-g15a2bc4dbb9c/testrun/2785598/suite/v4l2-compliance/test/modprobe-vivid/history/

- Naresh

^ permalink raw reply

* Re: [RFC v3 1/2] v4l2: add support for colorspace conversion for video capture
From: Dafna Hirschfeld @ 2020-06-05 10:11 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
	laurent.pinchart, linux-rockchip, sakari.ailus, Philipp Zabel,
	Hans Verkuil
In-Reply-To: <20200604173901.GA76282@chromium.org>

Hi,

On 04.06.20 19:39, Tomasz Figa wrote:
> Hi Dafna,
> 
> On Thu, Apr 16, 2020 at 04:56:04PM +0200, Dafna Hirschfeld wrote:
>> From: Philipp Zabel <p.zabel@pengutronix.de>
>>
>> For video capture it is the driver that reports the colorspace,
>> Y'CbCr/HSV encoding, quantization range and transfer function
>> used by the video, and there is no way to request something
>> different, even though many HDTV receivers have some sort of
>> colorspace conversion capabilities.
>>
> 
> Thanks for working on this! Please see my comments inline.
> 
>> For output video this feature already exists since the application
>> specifies this information for the video format it will send out, and
>> the transmitter will enable any available CSC if a format conversion has
>> to be performed in order to match the capabilities of the sink.
>>
>> For video capture we propose adding new pix_format flag:
>> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application,
>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields
>> as the requested colorspace information and will attempt to
>> do the conversion it supports.
>>
>> Drivers set the flags
>> V4L2_FMT_FLAG_CSC_YCBCR_ENC,
>> V4L2_FMT_FLAG_CSC_HSV_ENC,
>> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION,
> 
> Do we need this level of granularity? The drivers would ignore
> unsupported encoding/quantization settings and reset them to supported
> ones anyway, so if one doesn't support changing given parameter, it
> would just return back the original stream parameter.

I think this granularity allows userspace to know ahead what is supported
and what is not so it doesn't have to guess.

> 
>> in the flags field of the struct v4l2_fmtdesc during enumeration to
>> indicate that they support colorspace conversion for the respective field.
>> Currently the conversion of the fields colorspace, xfer_func is not
>> supported since there are no drivers that support it.
>>
>> The same API is added for the subdevices. With the flag V4L2_MBUS_FRAMEFMT_HAS_CSC
>> set by the application in the VIDIOC_SUBDEV_S_FMT ioctl and the flags
>> V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC, V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION
>> set by the driver in the VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl.
>>
>> For subdevices, new 'flags' fields were added to the structs
>> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the 'reserved' field
>>
>> Drivers do not have to actually look at the flags: if the flags are not
>> set, then the colorspace, ycbcr_enc and quantization fields are set to
>> the default values by the core, i.e. just pass on the received format
>> without conversion.
>>
>> Signed-off-by: Hans Verkuil <Hans Verkuil@cisco.com>
> 
> Something wrong with the email address here.
> 
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>> This is v3 of the RFC suggested originaly by Hans Verkuil:
>>
>>   https://patchwork.linuxtv.org/patch/28847/
>>
>> And then a v2 from Philipp Zabel:
>>
>>   https://patchwork.kernel.org/project/linux-media/list/?series=15483
>>
>> changes in v3:
>> I added the API to subdevices as well and added fixes according to
>> comments from Hans.
>> I also added a usecase for the new API for the rkisp1 driver.
>>
>> changes in v2 (reported by Philipp Zabel):
>>   - convert to rst
>>   - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for
>>     colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func
>>   - let driver set flags to indicate supported features
>>
>> [1] https://patchwork.linuxtv.org/patch/28847/
>>
>>   .../media/v4l/pixfmt-reserved.rst             |  6 +++
>>   .../media/v4l/pixfmt-v4l2-mplane.rst          | 16 ++-----
>>   .../userspace-api/media/v4l/pixfmt-v4l2.rst   | 36 +++++++++++++---
>>   .../media/v4l/subdev-formats.rst              | 42 +++++++++++++++++--
>>   .../media/v4l/vidioc-enum-fmt.rst             | 18 ++++++++
>>   .../v4l/vidioc-subdev-enum-mbus-code.rst      | 23 ++++++++++
>>   drivers/media/v4l2-core/v4l2-ioctl.c          | 19 ++++++++-
>>   drivers/media/v4l2-core/v4l2-subdev.c         |  7 ++++
>>   include/uapi/linux/v4l2-mediabus.h            |  5 ++-
>>   include/uapi/linux/v4l2-subdev.h              |  5 ++-
>>   include/uapi/linux/videodev2.h                |  4 ++
>>   11 files changed, 158 insertions(+), 23 deletions(-)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
>> index 59b9e7238f90..fa8dada69f8c 100644
>> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
>> @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list.
>>   	by RGBA values (128, 192, 255, 128), the same pixel described with
>>   	premultiplied colors would be described by RGBA values (64, 96,
>>   	128, 128)
>> +    * - ``V4L2_PIX_FMT_FLAG_HAS_CSC``
>> +      - 0x00000002
>> +      - Set by the application. It is only used for capture and is
>> +	ignored for output streams. If set, then request the driver to do
>> +	colorspace conversion from the received colorspace, only conversions
> 
> Should this be:
> 
>    colorspace. Only conversions
> 
> ?
> 
>> +	of Ycbcr encoding, HSV encoding and quantization are supported.
> 
> YCbCr?

I raformulate in coming v4 this doc according to Verkuil's comments.

> 
>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
>> index 444b4082684c..66f3365d7b72 100644
>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
>> @@ -105,29 +105,21 @@ describing all planes of that format.
>>       * - __u8
>>         - ``ycbcr_enc``
>>         - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>> -        This information supplements the ``colorspace`` and must be set by
>> -	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	See struct :c:type:`v4l2_pix_format`.
>>       * - __u8
>>         - ``hsv_enc``
>>         - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
>> -        This information supplements the ``colorspace`` and must be set by
>> -	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	See struct :c:type:`v4l2_pix_format`.
>>       * - }
>>         -
>>       * - __u8
>>         - ``quantization``
>>         - Quantization range, from enum :c:type:`v4l2_quantization`.
>> -        This information supplements the ``colorspace`` and must be set by
>> -	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	See struct :c:type:`v4l2_pix_format`.
>>       * - __u8
>>         - ``xfer_func``
>>         - Transfer function, from enum :c:type:`v4l2_xfer_func`.
>> -        This information supplements the ``colorspace`` and must be set by
>> -	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	See struct :c:type:`v4l2_pix_format`.
>>       * - __u8
>>         - ``reserved[7]``
>>         - Reserved for future extensions. Should be zeroed by drivers and
>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
>> index 759420a872d6..ce57718cd66b 100644
>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
>> @@ -110,8 +110,8 @@ Single-planar format structure
>>         - ``colorspace``
>>         - Image colorspace, from enum :c:type:`v4l2_colorspace`.
>>           This information supplements the ``pixelformat`` and must be set
>> -	by the driver for capture streams and by the application for
>> -	output streams, see :ref:`colorspaces`.
>> +	by the driver for capture streams and by the application for output
>> +	streams, see :ref:`colorspace`.
> 
> Is it expected that the colorspace itself can't be converted? Also is
> the change of the reference name expected?

This chunk will be removed in v4

> 
>>       * - __u32
>>         - ``priv``
>>         - This field indicates whether the remaining fields of the
>> @@ -148,13 +148,31 @@ Single-planar format structure
>>         - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the
>> +	flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set
>> +	this field for a capture stream to request a specific Y'CbCr encoding
>> +	for the captured image data. The driver will attempt to do the
>> +	conversion to the specified Y'CbCr encoding or return the encoding it
>> +	will use if it can't do the conversion. This field is ignored for
> 
> nit: The driver will attempt to do the conversion when the streaming
> starts, so that's not an entirely correct description. How about
> 
> "[...] captured image data. If the driver cannot handle requested
> conversion, it will return another, supported encoding."
> 
> ?

I'll change that

> 
>> +	non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion
>> +	is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the
>> +	on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration.
>> +	See :ref:`fmtdesc-flags`
>>       * - __u32
>>         - ``hsv_enc``
>>         - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the flag
>> +	``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this
>> +	field for a capture stream to request a specific HSV encoding for the
>> +	captured image data. The driver will attempt to do the conversion to
>> +	the specified HSV encoding or return the encoding it will use if it
>> +	can't do the conversion. This field is ignored for non-HSV
> 
> Ditto.
> 
>> +	pixelformats. The driver indicates that hsv_enc conversion
>> +	is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the
>> +	on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration.
>> +	See :ref:`fmtdesc-flags`
>>       * - }
>>         -
>>       * - __u32
>> @@ -162,7 +180,15 @@ Single-planar format structure
>>         - Quantization range, from enum :c:type:`v4l2_quantization`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the flag
>> +	``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set
>> +	this field for a capture stream to request a specific quantization
>> +	range for the captured image data. The driver will attempt to do the
>> +	conversion to the specified quantization range or return the
>> +	quantization it will use if it can't do the conversion.  The driver
> 
> Ditto.
> 
>> +	indicates that quantization conversion is supported by setting the flag
>> +	V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct
>> +	:c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
>>       * - __u32
>>         - ``xfer_func``
>>         - Transfer function, from enum :c:type:`v4l2_xfer_func`.
>> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> index 9a4d61b0d76f..f1d4ca29a3e8 100644
>> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> @@ -49,13 +49,33 @@ Media Bus Formats
>>         - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the
>> +	flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set
>> +	this field for a capture stream to request a specific Y'CbCr encoding
>> +	for the mbus data. The driver will attempt to do the
>> +	conversion to the specified Y'CbCr encoding or return the encoding it
>> +	will use if it can't do the conversion. This field is ignored for
> 
> Ditto.
> 
>> +	non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion
>> +	is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on
>> +	the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during
>> +	enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>`
>> +
>>       * - __u16
>>         - ``quantization``
>>         - Quantization range, from enum :c:type:`v4l2_quantization`.
>>           This information supplements the ``colorspace`` and must be set by
>>   	the driver for capture streams and by the application for output
>> -	streams, see :ref:`colorspaces`.
>> +	streams, see :ref:`colorspaces`. If the application sets the
>> +	flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set
>> +	this field for a capture stream to request a specific quantization
>> +	encoding for the mbus data. The driver will attempt to do the
>> +	conversion to the specified quantization or return the quantization it
>> +	will use if it can't do the conversion. The driver indicates that
> 
> Ditto.
> 
>> +	quantization conversion is supported by setting the flag
>> +	V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct
>> +	c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
>> +	See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>`
>> +
>>       * - __u16
>>         - ``xfer_func``
>>         - Transfer function, from enum :c:type:`v4l2_xfer_func`.
>> @@ -63,10 +83,26 @@ Media Bus Formats
>>   	the driver for capture streams and by the application for output
>>   	streams, see :ref:`colorspaces`.
>>       * - __u16
>> -      - ``reserved``\ [11]
>> +      - ``flags``
>> +      - flags see:  :ref:v4l2-mbus-framefmt-flags
>> +    * - __u16
>> +      - ``reserved``\ [10]
>>         - Reserved for future extensions. Applications and drivers must set
>>   	the array to zero.
>>   
>> +.. _v4l2-mbus-framefmt-flags:
>> +
>> +.. flat-table:: v4l2_mbus_framefmt Flags
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       3 1 4
>> +
>> +    * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC``
>> +      - 0x0001
>> +      - Set by the application. It is only used for capture and is
>> +	ignored for output streams. If set, then request the subdevice to do
>> +	colorspace conversion from the received colorspace, only conversions
>> +	of Ycbcr encoding, and quantization are supported.
> 
> YCbCr?
> 
>>   
>>   
>>   .. _v4l2-mbus-pixelcode:
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> index 7e3142e11d77..125f074543af 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before
>>   	parameters are detected. This flag can only be used in combination
>>   	with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
>>   	compressed formats only. It is also only applies to stateful codecs.
>> +    * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC``
>> +      - 0x0010
>> +      - The driver allows the application to try to change the default
>> +	ycbcr encoding. This flag is relevant only for capture devices.
> 
> YCbCr
> 
>> +	The application can ask to configure the ycbcr_enc of the capture device
>> +	when calling the :c:func:`VIDIOC_S_FMT` ioctl.
>> +    * - ``V4L2_FMT_FLAG_CSC_HSV_ENC``
>> +      - 0x0010
>> +      - The driver allows the application to try to change the default
>> +	hsv encoding. This flag is relevant only for capture devices.
>> +	The application can ask to configure the hsv_enc of the capture device
>> +	when calling the :c:func:`VIDIOC_S_FMT` ioctl.
>> +    * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION``
>> +      - 0x0020
>> +      - The driver allows the application to try to change the default
>> +	quantization. This flag is relevant only for capture devices.
>> +	The application can ask to configure the quantization of the capture device
>> +	when calling the :c:func:`VIDIOC_S_FMT` ioctl.
>>   
>>   
>>   Return Value
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
>> index 35b8607203a4..4ad87cb74f57 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
>> @@ -78,11 +78,34 @@ information about the try formats.
>>         - ``which``
>>         - Media bus format codes to be enumerated, from enum
>>   	:ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`.
>> +    * - __u32
>> +      - ``flags``
>> +      - see :ref:`v4l2-subdev-mbus-code-flags`
>>       * - __u32
>>         - ``reserved``\ [8]
>>         - Reserved for future extensions. Applications and drivers must set
>>   	the array to zero.
>>   
>> +.. _v4l2-subdev-mbus-code-flags:
>> +
>> +
>> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}|
>> +
>> +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       1 1 2
>> +
>> +    * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC
>> +      - 0x00000001
>> +      - The driver supports setting the ycbcr encoding by the application
>> +	when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format`
>> +	on how to do this.
>> +    * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION
>> +      - 0x00000002
>> +      - The driver supports setting the quantization by the application
>> +	when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format`
>> +	on how to do this.
>>   
>>   Return Value
>>   ============
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index b2ef8e60ea7d..3c7ffb6b15cb 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt)
>>   		fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes,
>>   					       VIDEO_MAX_PLANES);
>>   
>> +	if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>> +		if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC)
>> +			return;
> 
> This return statement might end up being confusing in the future,
> because one might add further fixups below. Perhaps instead, the inner
> condition negated could be added to the outer one?
> 
>> +		fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT;
>> +		fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +		fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT;
>> +		fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>> +	}
>> +
>>   	/*
>>   	 * The v4l2_pix_format structure has been extended with fields that were
>>   	 * not previously required to be set to zero by applications. The priv
>> @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt)
>>   	    fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
>>   		return;
>>   
>> -	if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC)
>> +	if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) {
>> +		if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
>> +		    fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC)
>> +			return;
>> +		fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT;
>> +		fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +		fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT;
>> +		fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>>   		return;
>> +	}
>>   
>>   	fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
>>   
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index a376b351135f..51e009936aad 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>>   	case VIDIOC_SUBDEV_S_FMT: {
>>   		struct v4l2_subdev_format *format = arg;
>>   
>> +		if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) {
>> +			format->format.colorspace = V4L2_COLORSPACE_DEFAULT;
>> +			format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>> +			format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +			format->format.quantization = V4L2_QUANTIZATION_DEFAULT;
>> +		}
> 
> Wouldn't this break setting the colorspaces on the sink pads, for which
> the flag isn't required? Actually there is some unfortunate statement in
> the documentation related to this:
> 
> "This information supplements the colorspace and must be set by the
> driver for capture streams and by the application for output streams,
> see Colorspaces."
> 
> However, I don't think there is any notion of "capture" and "output" for
> subdevices, right? Instead, the pad direction would have to be checked,
> but AFAICT there is no access to this kind of information from this
> wrapper.

Actually in coming v4 there is no new code added accept of the new fields and
macros of the API. I think there is no need to change any code.


> 
>> +
>>   		memset(format->reserved, 0, sizeof(format->reserved));
>>   		memset(format->format.reserved, 0, sizeof(format->format.reserved));
>>   		return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format);
>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
>> index 123a231001a8..89ff0ad6a631 100644
>> --- a/include/uapi/linux/v4l2-mediabus.h
>> +++ b/include/uapi/linux/v4l2-mediabus.h
>> @@ -16,6 +16,8 @@
>>   #include <linux/types.h>
>>   #include <linux/videodev2.h>
>>   
>> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC	0x0001
>> +
>>   /**
>>    * struct v4l2_mbus_framefmt - frame format on the media bus
>>    * @width:	image width
>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
>>   	__u16			ycbcr_enc;
>>   	__u16			quantization;
>>   	__u16			xfer_func;
>> -	__u16			reserved[11];
>> +	__u16			flags;
> 
> Are we okay with a u16 for flags?

I am fine with it, though don't mind changing it if there are objections.

Thanks,
Dafna

> 
> Best regards,
> Tomasz
> 

^ permalink raw reply

* Re: [PATCH 13/18] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
From: Pierre-Eric Pelloux-Prayer @ 2020-06-05  8:30 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: linux-rdma, Intel Graphics Development, Maarten Lankhorst, LKML,
	amd-gfx, Chris Wilson, linaro-mm-sig, Daniel Vetter,
	Christian König, linux-media
In-Reply-To: <20200604081224.863494-14-daniel.vetter@ffwll.ch>

Hi Daniel,

On 04/06/2020 10:12, Daniel Vetter wrote:
[...]
> @@ -6910,7 +6910,11 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>  		 * explicitly on fences instead
>  		 * and in general should be called for
>  		 * blocking commit to as per framework helpers
> +		 *
> +		 * Yes, this deadlocks, since you're calling dma_resv_lock in a
> +		 * path that leads to a dma_fence_signal(). Don't do that.
>  		 */
> +#if 0
>  		r = amdgpu_bo_reserve(abo, true);
>  		if (unlikely(r != 0))
>  			DRM_ERROR("failed to reserve buffer before flip\n");
> @@ -6920,6 +6924,12 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>  		tmz_surface = amdgpu_bo_encrypted(abo);
>  
>  		amdgpu_bo_unreserve(abo);
> +#endif
> +		/*
> +		 * this races anyway, so READ_ONCE isn't any better or worse
> +		 * than the stuff above. Except the stuff above can deadlock.
> +		 */
> +		tiling_flags = READ_ONCE(abo->tiling_flags);

With this change "tmz_surface" won't be initialized properly.
Adding the following line should fix it:

  tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;


Pierre-Eric


>  
>  		fill_dc_plane_info_and_addr(
>  			dm->adev, new_plane_state, tiling_flags,
> 

^ permalink raw reply

* Re: [PATCH v8 14/14] media: platform: Add jpeg dec/enc feature
From: Xia Jiang @ 2020-06-05  8:07 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger, Rick Chang,
	linux-media, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Marek Szyprowski, Tomasz Figa, srv_heupstream,
	senozhatsky, mojahsu, drinkcat, maoguang.meng, sj.huang
In-Reply-To: <b62b303c-10cd-fdf6-52fa-90d63124487a@xs4all.nl>

On Mon, 2020-05-11 at 11:04 +0200, Hans Verkuil wrote:
> On 03/04/2020 11:40, Xia Jiang wrote:
> > Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg
> > decode and encode have great similarities with function operation.
> > 
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8:jpeg encoder and decoder use separate callbacks instead of repeating
> >    the if/else in every callback.
> >    improve vidioc_try_fmt() implementation that can be shared by jpeg
> >    encoder and decoder.
> >    fix the bug of jpeg encoder s_selection implementation.
> >    cancel the state of the jpeg encoder.
> >    improve jpeg encoder and decoder set default params flow.
> >    put the clock names and other datas in a match_data struct.
> >    fix the bug of geting correctly quality value.
> >    do the all the bits' settings of one register in one function.
> >    move the code of mtk_jpeg_enc_reg.h to mtk_jpeg_enc_hw.h and delete
> >    mtk_jpeg_enc_reg.h.
> > 
> > v7: reverse spin lock and unlock operation in device run function for
> >     multi-instance.
> > 
> > v6: add space to arounding '+'.
> >     alignment 'struct mtk_jpeg_fmt *fmt' match open parenthesis.
> >     change 'mtk_jpeg_enc_set_encFormat' to 'mtk_jpeg_enc_set_enc_format'.
> >     make 'mtk_jpeg_ctrls_setup' to static prototype.
> >     delete unused variables 'jpeg'/'align_h'/'align_w'/'flags'.
> >     initialize 'yuv_format'/'enc_quality' variables.
> >     
> > v5: support crop for encoder and compose for decoder in s_selection and
> >     g_selection function.
> >     use clamp() to replace mtk_jpeg_bound_align_image() and round_up()
> >     to replace mtk_jpeg_align().
> >     delete jpeg_enc_param/mtk_jpeg_enc_param structure and
> >     mtk_jpeg_set_param(), program the registers directly based on
> >     the original V4L2 values.
> >     move macro definition about hw to mtk_jpeg_enc_reg.h.
> >     delete unnecessary V4L2 logs in driver.
> >     cancel spin lock and unlock operation in deviec run function.
> >     change jpeg enc register offset hex numberals from upercase to lowercase.
> > 
> > v4: split mtk_jpeg_try_fmt_mplane() to two functions, one for encoder,                                                      
> >     one for decoder.                                                          
> >     split mtk_jpeg_set_default_params() to two functions, one for                                                          
> >     encoder, one for decoder.                                                          
> >     add cropping support for encoder in g/s_selection ioctls.                                                          
> >     change exif mode support by using V4L2_JPEG_ACTIVE_MARKER_APP1.                                                         
> >     change MTK_JPEG_MAX_WIDTH/MTK_JPEG_MAX_HEIGH from 8192 to 65535 by                                                      
> >     specification.                                                          
> >     move width shifting operation behind aligning operation in                                                          
> >     mtk_jpeg_try_enc_fmt_mplane() for bug fix.                                                          
> >     fix user abuseing data_offset issue for DMABUF in                                                          
> >     mtk_jpeg_set_enc_src().                                                          
> >     fix kbuild warings: change MTK_JPEG_MIN_HEIGHT/MTK_JPEG_MAX_HEIGHT                                                      
> >                         and MTK_JPEG_MIN_WIDTH/MTK_JPEG_MAX_WIDTH from                                                      
> >                         'int' type to 'unsigned int' type.                                                          
> >                         fix msleadingly indented of 'else'.                                                                                                              
> > v3: delete Change-Id.                                                          
> >     only test once handler->error after the last v4l2_ctrl_new_std().                                                       
> >     seperate changes of v4l2-ctrls.c and v4l2-controls.h to new patch.                                                      
> > v2: fix compliance test fail, check created buffer size in driver.
> > ---
> >  drivers/media/platform/mtk-jpeg/Makefile      |    5 +-
> >  .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 1038 +++++++++++++----
> >  .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |   51 +-
> >  .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h |    7 +-
> >  .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c |  193 +++
> >  .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h |  123 ++
> >  6 files changed, 1188 insertions(+), 229 deletions(-)
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> > 
> > diff --git a/drivers/media/platform/mtk-jpeg/Makefile b/drivers/media/platform/mtk-jpeg/Makefile
> > index 48516dcf96e6..76c33aad0f3f 100644
> > --- a/drivers/media/platform/mtk-jpeg/Makefile
> > +++ b/drivers/media/platform/mtk-jpeg/Makefile
> > @@ -1,3 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > -mtk_jpeg-objs := mtk_jpeg_core.o mtk_jpeg_dec_hw.o mtk_jpeg_dec_parse.o
> > +mtk_jpeg-objs := mtk_jpeg_core.o \
> > +		 mtk_jpeg_dec_hw.o \
> > +		 mtk_jpeg_dec_parse.o \
> > +		 mtk_jpeg_enc_hw.o
> >  obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 77a95185584c..18a759ce2c46 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -3,6 +3,7 @@
> >   * Copyright (c) 2016 MediaTek Inc.
> >   * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> >   *         Rick Chang <rick.chang@mediatek.com>
> > + *         Xia Jiang <xia.jiang@mediatek.com>
> >   */
> >  
> >  #include <linux/clk.h>
> > @@ -23,11 +24,60 @@
> >  #include <media/videobuf2-dma-contig.h>
> >  #include <soc/mediatek/smi.h>
> >  
> > +#include "mtk_jpeg_enc_hw.h"
> >  #include "mtk_jpeg_dec_hw.h"
> >  #include "mtk_jpeg_core.h"
> >  #include "mtk_jpeg_dec_parse.h"
> >  
> > -static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > +static struct mtk_jpeg_fmt mtk_jpeg_enc_formats[] = {
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_JPEG,
> > +		.colplanes	= 1,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_CAPTURE,
> > +	},
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_NV12M,
> > +		.hw_format	= JPEG_ENC_YUV_FORMAT_NV12,
> > +		.h_sample	= {4, 4},
> > +		.v_sample	= {4, 2},
> > +		.colplanes	= 2,
> > +		.h_align	= 4,
> > +		.v_align	= 4,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > +	},
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_NV21M,
> > +		.hw_format	= JEPG_ENC_YUV_FORMAT_NV21,
> > +		.h_sample	= {4, 4},
> > +		.v_sample	= {4, 2},
> > +		.colplanes	= 2,
> > +		.h_align	= 4,
> > +		.v_align	= 4,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > +	},
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_YUYV,
> > +		.hw_format	= JPEG_ENC_YUV_FORMAT_YUYV,
> > +		.h_sample	= {8},
> > +		.v_sample	= {4},
> > +		.colplanes	= 1,
> > +		.h_align	= 5,
> > +		.v_align	= 3,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > +	},
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_YVYU,
> > +		.hw_format	= JPEG_ENC_YUV_FORMAT_YVYU,
> > +		.h_sample	= {8},
> > +		.v_sample	= {4},
> > +		.colplanes	= 1,
> > +		.h_align	= 5,
> > +		.v_align	= 3,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > +	},
> > +};
> > +
> > +static struct mtk_jpeg_fmt mtk_jpeg_dec_formats[] = {
> >  	{
> >  		.fourcc		= V4L2_PIX_FMT_JPEG,
> >  		.colplanes	= 1,
> > @@ -53,7 +103,8 @@ static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> >  	},
> >  };
> >  
> > -#define MTK_JPEG_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_formats)
> > +#define MTK_JPEG_ENC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_enc_formats)
> > +#define MTK_JPEG_DEC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_dec_formats)
> >  
> >  enum {
> >  	MTK_JPEG_BUF_FLAGS_INIT			= 0,
> > @@ -70,6 +121,11 @@ struct mtk_jpeg_src_buf {
> >  static int debug;
> >  module_param(debug, int, 0644);
> >  
> > +static inline struct mtk_jpeg_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl)
> > +{
> > +	return container_of(ctrl->handler, struct mtk_jpeg_ctx, ctrl_hdl);
> > +}
> > +
> >  static inline struct mtk_jpeg_ctx *mtk_jpeg_fh_to_ctx(struct v4l2_fh *fh)
> >  {
> >  	return container_of(fh, struct mtk_jpeg_ctx, fh);
> > @@ -81,12 +137,25 @@ static inline struct mtk_jpeg_src_buf *mtk_jpeg_vb2_to_srcbuf(
> >  	return container_of(to_vb2_v4l2_buffer(vb), struct mtk_jpeg_src_buf, b);
> >  }
> >  
> > -static int mtk_jpeg_querycap(struct file *file, void *priv,
> > -			     struct v4l2_capability *cap)
> > +static int mtk_jpeg_enc_querycap(struct file *file, void *priv,
> > +				 struct v4l2_capability *cap)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +
> > +	strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> > +	strscpy(cap->card, MTK_JPEG_NAME " encoder", sizeof(cap->card));
> > +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > +		 dev_name(jpeg->dev));
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_querycap(struct file *file, void *priv,
> > +				 struct v4l2_capability *cap)
> >  {
> >  	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> >  
> > -	strscpy(cap->driver, MTK_JPEG_NAME " decoder", sizeof(cap->driver));
> > +	strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> >  	strscpy(cap->card, MTK_JPEG_NAME " decoder", sizeof(cap->card));
> >  	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> >  		 dev_name(jpeg->dev));
> > @@ -94,6 +163,54 @@ static int mtk_jpeg_querycap(struct file *file, void *priv,
> >  	return 0;
> >  }
> >  
> > +static int vidioc_jpeg_enc_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = ctrl_to_ctx(ctrl);
> > +
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_JPEG_RESTART_INTERVAL:
> > +		ctx->restart_interval = ctrl->val;
> > +		break;
> > +	case V4L2_CID_JPEG_COMPRESSION_QUALITY:
> > +		ctx->enc_quality = ctrl->val;
> > +		break;
> > +	case V4L2_CID_JPEG_ACTIVE_MARKER:
> > +		ctx->enable_exif = ctrl->val & V4L2_JPEG_ACTIVE_MARKER_APP1 ?
> > +				   true : false;
> > +		break;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_jpeg_enc_ctrl_ops = {
> > +	.s_ctrl = vidioc_jpeg_enc_s_ctrl,
> > +};
> > +
> > +static int mtk_jpeg_enc_ctrls_setup(struct mtk_jpeg_ctx *ctx)
> > +{
> > +	const struct v4l2_ctrl_ops *ops = &mtk_jpeg_enc_ctrl_ops;
> > +	struct v4l2_ctrl_handler *handler = &ctx->ctrl_hdl;
> > +
> > +	v4l2_ctrl_handler_init(handler, 3);
> > +
> > +	v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_RESTART_INTERVAL, 0, 100,
> > +			  1, 0);
> > +	v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_COMPRESSION_QUALITY, 48,
> > +			  100, 1, 90);
> > +	v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_ACTIVE_MARKER, 0,
> > +			  V4L2_JPEG_ACTIVE_MARKER_APP1, 0, 0);
> > +
> > +	if (handler->error) {
> > +		v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> > +		return handler->error;
> > +	}
> > +
> > +	v4l2_ctrl_handler_setup(&ctx->ctrl_hdl);
> > +
> > +	return 0;
> > +}
> > +
> >  static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> >  			     struct v4l2_fmtdesc *f, u32 type)
> >  {
> > @@ -115,117 +232,105 @@ static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> >  	return 0;
> >  }
> >  
> > -static int mtk_jpeg_enum_fmt_vid_cap(struct file *file, void *priv,
> > -				     struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_cap(struct file *file, void *priv,
> > +					 struct v4l2_fmtdesc *f)
> >  {
> > -	return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > +				 MTK_JPEG_ENC_NUM_FORMATS, f,
> > +				 MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_cap(struct file *file, void *priv,
> > +					 struct v4l2_fmtdesc *f)
> > +{
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats,
> > +				 MTK_JPEG_DEC_NUM_FORMATS, f,
> >  				 MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> >  }
> >  
> > -static int mtk_jpeg_enum_fmt_vid_out(struct file *file, void *priv,
> > -				     struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_out(struct file *file, void *priv,
> > +					 struct v4l2_fmtdesc *f)
> > +{
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > +				 MTK_JPEG_ENC_NUM_FORMATS, f,
> > +				 MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_out(struct file *file, void *priv,
> > +					 struct v4l2_fmtdesc *f)
> >  {
> > -	return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > -				 MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats, MTK_JPEG_DEC_NUM_FORMATS,
> > +				 f, MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> >  }
> 
> OK, so this patch is very hard to read because there are two independent changes
> taking place:
> 
> 1) rename existing functions/defines/variables with a _dec prefix to prepare
>    for the addition of the encoder feature.
> 
> 2) add the encoder feature.
> 
> Please split up this patch into two parts: one that does the rename and as much of
> the preparation to support both decoder and encoder without changing the
> functionality, and a second one that actually adds the new encoder feature.
> 
> In fact, once that's done it is likely that most of this patch series can be
> merged, even if there are still things that need to be changed for the last
> patch adding the encoder support. I see nothing objectionable in patches 1-10
> and 13. So merging those together with a new rename patch wouldn't be an issue,
> I think.
> 
> In any case, the diffs should be a lot cleaner and easier to review by splitting
> it up like that.
Dear Hans,

Thanks for your good advice. I have splited up this patch into two
patches in v9.

Best Regards,
Xia Jiang 
> 
> Regards,
> 
> 	Hans
> 
> >  
> > -static struct mtk_jpeg_q_data *mtk_jpeg_get_q_data(struct mtk_jpeg_ctx *ctx,
> > -						   enum v4l2_buf_type type)
> > +static struct mtk_jpeg_q_data *
> > +mtk_jpeg_get_q_data(struct mtk_jpeg_ctx *ctx, enum v4l2_buf_type type)
> >  {
> >  	if (V4L2_TYPE_IS_OUTPUT(type))
> >  		return &ctx->out_q;
> >  	return &ctx->cap_q;
> >  }
> >  
> > -static struct mtk_jpeg_fmt *mtk_jpeg_find_format(struct mtk_jpeg_ctx *ctx,
> > -						 u32 pixelformat,
> > +static struct mtk_jpeg_fmt *mtk_jpeg_find_format(u32 pixelformat,
> >  						 unsigned int fmt_type)
> >  {
> > -	unsigned int k, fmt_flag;
> > -
> > -	fmt_flag = (fmt_type == MTK_JPEG_FMT_TYPE_OUTPUT) ?
> > -		   MTK_JPEG_FMT_FLAG_DEC_OUTPUT :
> > -		   MTK_JPEG_FMT_FLAG_DEC_CAPTURE;
> > +	unsigned int k;
> > +	struct mtk_jpeg_fmt *fmt;
> >  
> > -	for (k = 0; k < MTK_JPEG_NUM_FORMATS; k++) {
> > -		struct mtk_jpeg_fmt *fmt = &mtk_jpeg_formats[k];
> > +	for (k = 0; k < MTK_JPEG_ENC_NUM_FORMATS; k++) {
> > +		fmt = &mtk_jpeg_enc_formats[k];
> >  
> > -		if (fmt->fourcc == pixelformat && fmt->flags & fmt_flag)
> > +		if (fmt->fourcc == pixelformat && fmt->flags & fmt_type)
> >  			return fmt;
> >  	}
> >  
> > -	return NULL;
> > -}
> > +	for (k = 0; k < MTK_JPEG_DEC_NUM_FORMATS; k++) {
> > +		fmt = &mtk_jpeg_dec_formats[k];
> >  
> > -static void mtk_jpeg_adjust_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > -				       struct v4l2_format *f)
> > -{
> > -	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > -	struct mtk_jpeg_q_data *q_data;
> > -	int i;
> > -
> > -	q_data = mtk_jpeg_get_q_data(ctx, f->type);
> > -
> > -	pix_mp->width = q_data->w;
> > -	pix_mp->height = q_data->h;
> > -	pix_mp->pixelformat = q_data->fmt->fourcc;
> > -	pix_mp->num_planes = q_data->fmt->colplanes;
> > -
> > -	for (i = 0; i < pix_mp->num_planes; i++) {
> > -		pix_mp->plane_fmt[i].bytesperline = q_data->bytesperline[i];
> > -		pix_mp->plane_fmt[i].sizeimage = q_data->sizeimage[i];
> > +		if (fmt->fourcc == pixelformat && fmt->flags & fmt_type)
> > +			return fmt;
> >  	}
> > +
> > +	return NULL;
> >  }
> >  
> > -static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > -				   struct mtk_jpeg_fmt *fmt,
> > -				   struct mtk_jpeg_ctx *ctx, int q_type)
> > +static int vidioc_try_fmt(struct v4l2_format *f, struct mtk_jpeg_fmt *fmt)
> >  {
> >  	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> >  	int i;
> >  
> > -	memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
> >  	pix_mp->field = V4L2_FIELD_NONE;
> > -
> > -	if (ctx->state != MTK_JPEG_INIT) {
> > -		mtk_jpeg_adjust_fmt_mplane(ctx, f);
> > -		return 0;
> > -	}
> > -
> >  	pix_mp->num_planes = fmt->colplanes;
> >  	pix_mp->pixelformat = fmt->fourcc;
> >  
> > -	if (q_type == MTK_JPEG_FMT_TYPE_OUTPUT) {
> > -		struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[0];
> > -
> > +	if (fmt->fourcc == V4L2_PIX_FMT_JPEG) {
> >  		pix_mp->height = clamp(pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> >  				       MTK_JPEG_MAX_HEIGHT);
> >  		pix_mp->width = clamp(pix_mp->width, MTK_JPEG_MIN_WIDTH,
> >  				      MTK_JPEG_MAX_WIDTH);
> > -
> > -		memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> > -		pfmt->bytesperline = 0;
> > -		/* Source size must be aligned to 128 */
> > -		pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> > -		if (pfmt->sizeimage == 0)
> > -			pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > -		return 0;
> > +		pix_mp->plane_fmt[0].bytesperline = 0;
> > +		pix_mp->plane_fmt[0].sizeimage =
> > +				round_up(pix_mp->plane_fmt[0].sizeimage, 128);
> > +		if (pix_mp->plane_fmt[0].sizeimage == 0)
> > +			pix_mp->plane_fmt[0].sizeimage =
> > +				MTK_JPEG_DEFAULT_SIZEIMAGE;
> > +	} else {
> > +		pix_mp->height = clamp(round_up(pix_mp->height, fmt->v_align),
> > +				       MTK_JPEG_MIN_HEIGHT,
> > +				       MTK_JPEG_MAX_HEIGHT);
> > +		pix_mp->width = clamp(round_up(pix_mp->width, fmt->h_align),
> > +				      MTK_JPEG_MIN_WIDTH, MTK_JPEG_MAX_WIDTH);
> > +		for (i = 0; i < pix_mp->num_planes; i++) {
> > +			struct v4l2_plane_pix_format *pfmt =
> > +							&pix_mp->plane_fmt[i];
> > +			u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> > +			u32 h = pix_mp->height * fmt->v_sample[i] / 4;
> > +
> > +			pfmt->bytesperline = stride;
> > +			pfmt->sizeimage = stride * h;
> > +		}
> >  	}
> >  
> > -	/* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > -	pix_mp->height = clamp(round_up(pix_mp->height, fmt->v_align),
> > -			       MTK_JPEG_MIN_HEIGHT, MTK_JPEG_MAX_HEIGHT);
> > -	pix_mp->width = clamp(round_up(pix_mp->width, fmt->h_align),
> > -			      MTK_JPEG_MIN_WIDTH, MTK_JPEG_MAX_WIDTH);
> > -
> > -	for (i = 0; i < fmt->colplanes; i++) {
> > -		struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[i];
> > -		u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> > -		u32 h = pix_mp->height * fmt->v_sample[i] / 4;
> > -
> > -		pfmt->bytesperline = stride;
> > -		pfmt->sizeimage = stride * h;
> > -	}
> >  	return 0;
> >  }
> >  
> > @@ -280,14 +385,35 @@ static int mtk_jpeg_g_fmt_vid_mplane(struct file *file, void *priv,
> >  	return 0;
> >  }
> >  
> > -static int mtk_jpeg_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> > -					   struct v4l2_format *f)
> > +static int mtk_jpeg_enc_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> > +					       struct v4l2_format *f)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +	struct mtk_jpeg_fmt *fmt;
> > +
> > +	fmt = mtk_jpeg_find_format(f->fmt.pix_mp.pixelformat,
> > +				   MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +	if (!fmt)
> > +		fmt = ctx->cap_q.fmt;
> > +
> > +	v4l2_dbg(2, debug, &ctx->jpeg->v4l2_dev, "(%d) try_fmt:%c%c%c%c\n",
> > +		 f->type,
> > +		 (fmt->fourcc & 0xff),
> > +		 (fmt->fourcc >>  8 & 0xff),
> > +		 (fmt->fourcc >> 16 & 0xff),
> > +		 (fmt->fourcc >> 24 & 0xff));
> > +
> > +	return vidioc_try_fmt(f, fmt);
> > +}
> > +
> > +static int mtk_jpeg_dec_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> > +					       struct v4l2_format *f)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> >  	struct mtk_jpeg_fmt *fmt;
> >  
> > -	fmt = mtk_jpeg_find_format(ctx, f->fmt.pix_mp.pixelformat,
> > -				   MTK_JPEG_FMT_TYPE_CAPTURE);
> > +	fmt = mtk_jpeg_find_format(f->fmt.pix_mp.pixelformat,
> > +				   MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> >  	if (!fmt)
> >  		fmt = ctx->cap_q.fmt;
> >  
> > @@ -298,17 +424,43 @@ static int mtk_jpeg_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> >  		 (fmt->fourcc >> 16 & 0xff),
> >  		 (fmt->fourcc >> 24 & 0xff));
> >  
> > -	return mtk_jpeg_try_fmt_mplane(f, fmt, ctx, MTK_JPEG_FMT_TYPE_CAPTURE);
> > +	if (ctx->state != MTK_JPEG_INIT) {
> > +		mtk_jpeg_g_fmt_vid_mplane(file, priv, f);
> > +		return 0;
> > +	}
> > +
> > +	return vidioc_try_fmt(f, fmt);
> > +}
> > +
> > +static int mtk_jpeg_enc_try_fmt_vid_out_mplane(struct file *file, void *priv,
> > +					       struct v4l2_format *f)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +	struct mtk_jpeg_fmt *fmt;
> > +
> > +	fmt = mtk_jpeg_find_format(f->fmt.pix_mp.pixelformat,
> > +				   MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +	if (!fmt)
> > +		fmt = ctx->out_q.fmt;
> > +
> > +	v4l2_dbg(2, debug, &ctx->jpeg->v4l2_dev, "(%d) try_fmt:%c%c%c%c\n",
> > +		 f->type,
> > +		 (fmt->fourcc & 0xff),
> > +		 (fmt->fourcc >>  8 & 0xff),
> > +		 (fmt->fourcc >> 16 & 0xff),
> > +		 (fmt->fourcc >> 24 & 0xff));
> > +
> > +	return vidioc_try_fmt(f, fmt);
> >  }
> >  
> > -static int mtk_jpeg_try_fmt_vid_out_mplane(struct file *file, void *priv,
> > -					   struct v4l2_format *f)
> > +static int mtk_jpeg_dec_try_fmt_vid_out_mplane(struct file *file, void *priv,
> > +					       struct v4l2_format *f)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> >  	struct mtk_jpeg_fmt *fmt;
> >  
> > -	fmt = mtk_jpeg_find_format(ctx, f->fmt.pix_mp.pixelformat,
> > -				   MTK_JPEG_FMT_TYPE_OUTPUT);
> > +	fmt = mtk_jpeg_find_format(f->fmt.pix_mp.pixelformat,
> > +				   MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> >  	if (!fmt)
> >  		fmt = ctx->out_q.fmt;
> >  
> > @@ -319,17 +471,21 @@ static int mtk_jpeg_try_fmt_vid_out_mplane(struct file *file, void *priv,
> >  		 (fmt->fourcc >> 16 & 0xff),
> >  		 (fmt->fourcc >> 24 & 0xff));
> >  
> > -	return mtk_jpeg_try_fmt_mplane(f, fmt, ctx, MTK_JPEG_FMT_TYPE_OUTPUT);
> > +	if (ctx->state != MTK_JPEG_INIT) {
> > +		mtk_jpeg_g_fmt_vid_mplane(file, priv, f);
> > +		return 0;
> > +	}
> > +
> > +	return vidioc_try_fmt(f, fmt);
> >  }
> >  
> >  static int mtk_jpeg_s_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > -				 struct v4l2_format *f)
> > +				 struct v4l2_format *f, unsigned int fmt_type)
> >  {
> >  	struct vb2_queue *vq;
> >  	struct mtk_jpeg_q_data *q_data = NULL;
> >  	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> >  	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > -	unsigned int f_type;
> >  	int i;
> >  
> >  	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > @@ -343,10 +499,7 @@ static int mtk_jpeg_s_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> >  		return -EBUSY;
> >  	}
> >  
> > -	f_type = V4L2_TYPE_IS_OUTPUT(f->type) ?
> > -			 MTK_JPEG_FMT_TYPE_OUTPUT : MTK_JPEG_FMT_TYPE_CAPTURE;
> > -
> > -	q_data->fmt = mtk_jpeg_find_format(ctx, pix_mp->pixelformat, f_type);
> > +	q_data->fmt = mtk_jpeg_find_format(pix_mp->pixelformat, fmt_type);
> >  	q_data->w = pix_mp->width;
> >  	q_data->h = pix_mp->height;
> >  	ctx->colorspace = pix_mp->colorspace;
> > @@ -374,28 +527,56 @@ static int mtk_jpeg_s_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> >  	return 0;
> >  }
> >  
> > -static int mtk_jpeg_s_fmt_vid_out_mplane(struct file *file, void *priv,
> > -					 struct v4l2_format *f)
> > +static int mtk_jpeg_enc_s_fmt_vid_out_mplane(struct file *file, void *priv,
> > +					     struct v4l2_format *f)
> > +{
> > +	int ret;
> > +
> > +	ret = mtk_jpeg_enc_try_fmt_vid_out_mplane(file, priv, f);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f,
> > +				     MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +}
> > +
> > +static int mtk_jpeg_dec_s_fmt_vid_out_mplane(struct file *file, void *priv,
> > +					     struct v4l2_format *f)
> >  {
> >  	int ret;
> >  
> > -	ret = mtk_jpeg_try_fmt_vid_out_mplane(file, priv, f);
> > +	ret = mtk_jpeg_dec_try_fmt_vid_out_mplane(file, priv, f);
> >  	if (ret)
> >  		return ret;
> >  
> > -	return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f);
> > +	return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f,
> > +				     MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> >  }
> >  
> > -static int mtk_jpeg_s_fmt_vid_cap_mplane(struct file *file, void *priv,
> > -					 struct v4l2_format *f)
> > +static int mtk_jpeg_enc_s_fmt_vid_cap_mplane(struct file *file, void *priv,
> > +					     struct v4l2_format *f)
> >  {
> >  	int ret;
> >  
> > -	ret = mtk_jpeg_try_fmt_vid_cap_mplane(file, priv, f);
> > +	ret = mtk_jpeg_enc_try_fmt_vid_cap_mplane(file, priv, f);
> >  	if (ret)
> >  		return ret;
> >  
> > -	return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f);
> > +	return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f,
> > +				     MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_dec_s_fmt_vid_cap_mplane(struct file *file, void *priv,
> > +					     struct v4l2_format *f)
> > +{
> > +	int ret;
> > +
> > +	ret = mtk_jpeg_dec_try_fmt_vid_cap_mplane(file, priv, f);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f,
> > +				     MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> >  }
> >  
> >  static void mtk_jpeg_queue_src_chg_event(struct mtk_jpeg_ctx *ctx)
> > @@ -420,8 +601,31 @@ static int mtk_jpeg_subscribe_event(struct v4l2_fh *fh,
> >  	return v4l2_ctrl_subscribe_event(fh, sub);
> >  }
> >  
> > -static int mtk_jpeg_g_selection(struct file *file, void *priv,
> > -				struct v4l2_selection *s)
> > +static int mtk_jpeg_enc_g_selection(struct file *file, void *priv,
> > +				    struct v4l2_selection *s)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +
> > +	if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > +		return -EINVAL;
> > +
> > +	switch (s->target) {
> > +	case V4L2_SEL_TGT_CROP:
> > +	case V4L2_SEL_TGT_CROP_BOUNDS:
> > +	case V4L2_SEL_TGT_CROP_DEFAULT:
> > +		s->r.width = ctx->out_q.w;
> > +		s->r.height = ctx->out_q.h;
> > +		s->r.left = 0;
> > +		s->r.top = 0;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_g_selection(struct file *file, void *priv,
> > +				    struct v4l2_selection *s)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> >  
> > @@ -446,11 +650,34 @@ static int mtk_jpeg_g_selection(struct file *file, void *priv,
> >  	default:
> >  		return -EINVAL;
> >  	}
> > +
> >  	return 0;
> >  }
> >  
> > -static int mtk_jpeg_s_selection(struct file *file, void *priv,
> > -				struct v4l2_selection *s)
> > +static int mtk_jpeg_enc_s_selection(struct file *file, void *priv,
> > +				    struct v4l2_selection *s)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +
> > +	if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > +		return -EINVAL;
> > +
> > +	switch (s->target) {
> > +	case V4L2_SEL_TGT_CROP:
> > +		s->r.left = 0;
> > +		s->r.top = 0;
> > +		ctx->out_q.w = min(s->r.width, ctx->out_q.w);
> > +		ctx->out_q.h = min(s->r.height, ctx->out_q.h);
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_s_selection(struct file *file, void *priv,
> > +				    struct v4l2_selection *s)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> >  
> > @@ -467,6 +694,7 @@ static int mtk_jpeg_s_selection(struct file *file, void *priv,
> >  	default:
> >  		return -EINVAL;
> >  	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -495,20 +723,47 @@ static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> >  	return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
> >  }
> >  
> > -static const struct v4l2_ioctl_ops mtk_jpeg_ioctl_ops = {
> > -	.vidioc_querycap                = mtk_jpeg_querycap,
> > -	.vidioc_enum_fmt_vid_cap	= mtk_jpeg_enum_fmt_vid_cap,
> > -	.vidioc_enum_fmt_vid_out	= mtk_jpeg_enum_fmt_vid_out,
> > -	.vidioc_try_fmt_vid_cap_mplane	= mtk_jpeg_try_fmt_vid_cap_mplane,
> > -	.vidioc_try_fmt_vid_out_mplane	= mtk_jpeg_try_fmt_vid_out_mplane,
> > +static const struct v4l2_ioctl_ops mtk_jpeg_enc_ioctl_ops = {
> > +	.vidioc_querycap                = mtk_jpeg_enc_querycap,
> > +	.vidioc_enum_fmt_vid_cap	= mtk_jpeg_enc_enum_fmt_vid_cap,
> > +	.vidioc_enum_fmt_vid_out	= mtk_jpeg_enc_enum_fmt_vid_out,
> > +	.vidioc_try_fmt_vid_cap_mplane	= mtk_jpeg_enc_try_fmt_vid_cap_mplane,
> > +	.vidioc_try_fmt_vid_out_mplane	= mtk_jpeg_enc_try_fmt_vid_out_mplane,
> > +	.vidioc_g_fmt_vid_cap_mplane    = mtk_jpeg_g_fmt_vid_mplane,
> > +	.vidioc_g_fmt_vid_out_mplane    = mtk_jpeg_g_fmt_vid_mplane,
> > +	.vidioc_s_fmt_vid_cap_mplane    = mtk_jpeg_enc_s_fmt_vid_cap_mplane,
> > +	.vidioc_s_fmt_vid_out_mplane    = mtk_jpeg_enc_s_fmt_vid_out_mplane,
> > +	.vidioc_qbuf                    = mtk_jpeg_qbuf,
> > +	.vidioc_subscribe_event         = mtk_jpeg_subscribe_event,
> > +	.vidioc_g_selection		= mtk_jpeg_enc_g_selection,
> > +	.vidioc_s_selection		= mtk_jpeg_enc_s_selection,
> > +
> > +	.vidioc_create_bufs		= v4l2_m2m_ioctl_create_bufs,
> > +	.vidioc_prepare_buf		= v4l2_m2m_ioctl_prepare_buf,
> > +	.vidioc_reqbufs                 = v4l2_m2m_ioctl_reqbufs,
> > +	.vidioc_querybuf                = v4l2_m2m_ioctl_querybuf,
> > +	.vidioc_dqbuf                   = v4l2_m2m_ioctl_dqbuf,
> > +	.vidioc_expbuf                  = v4l2_m2m_ioctl_expbuf,
> > +	.vidioc_streamon                = v4l2_m2m_ioctl_streamon,
> > +	.vidioc_streamoff               = v4l2_m2m_ioctl_streamoff,
> > +
> > +	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
> > +};
> > +
> > +static const struct v4l2_ioctl_ops mtk_jpeg_dec_ioctl_ops = {
> > +	.vidioc_querycap                = mtk_jpeg_dec_querycap,
> > +	.vidioc_enum_fmt_vid_cap	= mtk_jpeg_dec_enum_fmt_vid_cap,
> > +	.vidioc_enum_fmt_vid_out	= mtk_jpeg_dec_enum_fmt_vid_out,
> > +	.vidioc_try_fmt_vid_cap_mplane	= mtk_jpeg_dec_try_fmt_vid_cap_mplane,
> > +	.vidioc_try_fmt_vid_out_mplane	= mtk_jpeg_dec_try_fmt_vid_out_mplane,
> >  	.vidioc_g_fmt_vid_cap_mplane    = mtk_jpeg_g_fmt_vid_mplane,
> >  	.vidioc_g_fmt_vid_out_mplane    = mtk_jpeg_g_fmt_vid_mplane,
> > -	.vidioc_s_fmt_vid_cap_mplane    = mtk_jpeg_s_fmt_vid_cap_mplane,
> > -	.vidioc_s_fmt_vid_out_mplane    = mtk_jpeg_s_fmt_vid_out_mplane,
> > +	.vidioc_s_fmt_vid_cap_mplane    = mtk_jpeg_dec_s_fmt_vid_cap_mplane,
> > +	.vidioc_s_fmt_vid_out_mplane    = mtk_jpeg_dec_s_fmt_vid_out_mplane,
> >  	.vidioc_qbuf                    = mtk_jpeg_qbuf,
> >  	.vidioc_subscribe_event         = mtk_jpeg_subscribe_event,
> > -	.vidioc_g_selection		= mtk_jpeg_g_selection,
> > -	.vidioc_s_selection		= mtk_jpeg_s_selection,
> > +	.vidioc_g_selection		= mtk_jpeg_dec_g_selection,
> > +	.vidioc_s_selection		= mtk_jpeg_dec_s_selection,
> >  
> >  	.vidioc_create_bufs		= v4l2_m2m_ioctl_create_bufs,
> >  	.vidioc_prepare_buf		= v4l2_m2m_ioctl_prepare_buf,
> > @@ -586,8 +841,9 @@ static bool mtk_jpeg_check_resolution_change(struct mtk_jpeg_ctx *ctx,
> >  	}
> >  
> >  	q_data = &ctx->cap_q;
> > -	if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc,
> > -						MTK_JPEG_FMT_TYPE_CAPTURE)) {
> > +	if (q_data->fmt !=
> > +	    mtk_jpeg_find_format(param->dst_fourcc,
> > +				 MTK_JPEG_FMT_FLAG_DEC_CAPTURE)) {
> >  		v4l2_dbg(1, debug, &jpeg->v4l2_dev, "format change\n");
> >  		return true;
> >  	}
> > @@ -608,9 +864,8 @@ static void mtk_jpeg_set_queue_data(struct mtk_jpeg_ctx *ctx,
> >  	q_data = &ctx->cap_q;
> >  	q_data->w = param->dec_w;
> >  	q_data->h = param->dec_h;
> > -	q_data->fmt = mtk_jpeg_find_format(ctx,
> > -					   param->dst_fourcc,
> > -					   MTK_JPEG_FMT_TYPE_CAPTURE);
> > +	q_data->fmt = mtk_jpeg_find_format(param->dst_fourcc,
> > +					   MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> >  
> >  	for (i = 0; i < q_data->fmt->colplanes; i++) {
> >  		q_data->bytesperline[i] = param->mem_stride[i];
> > @@ -627,7 +882,18 @@ static void mtk_jpeg_set_queue_data(struct mtk_jpeg_ctx *ctx,
> >  		 param->dec_w, param->dec_h);
> >  }
> >  
> > -static void mtk_jpeg_buf_queue(struct vb2_buffer *vb)
> > +static void mtk_jpeg_enc_buf_queue(struct vb2_buffer *vb)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +
> > +	v4l2_dbg(2, debug, &jpeg->v4l2_dev, "(%d) buf_q id=%d, vb=%p\n",
> > +		 vb->vb2_queue->type, vb->index, vb);
> > +
> > +	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, to_vb2_v4l2_buffer(vb));
> > +}
> > +
> > +static void mtk_jpeg_dec_buf_queue(struct vb2_buffer *vb)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> >  	struct mtk_jpeg_dec_param *param;
> > @@ -679,7 +945,16 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> >  		return v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> >  }
> >  
> > -static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> > +static void mtk_jpeg_enc_stop_streaming(struct vb2_queue *q)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > +	struct vb2_v4l2_buffer *vb;
> > +
> > +	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> > +		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > +}
> > +
> > +static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> >  	struct vb2_v4l2_buffer *vb;
> > @@ -705,13 +980,22 @@ static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> >  		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> >  }
> >  
> > -static const struct vb2_ops mtk_jpeg_qops = {
> > +static const struct vb2_ops mtk_jpeg_dec_qops = {
> >  	.queue_setup        = mtk_jpeg_queue_setup,
> >  	.buf_prepare        = mtk_jpeg_buf_prepare,
> > -	.buf_queue          = mtk_jpeg_buf_queue,
> > +	.buf_queue          = mtk_jpeg_dec_buf_queue,
> >  	.wait_prepare       = vb2_ops_wait_prepare,
> >  	.wait_finish        = vb2_ops_wait_finish,
> > -	.stop_streaming     = mtk_jpeg_stop_streaming,
> > +	.stop_streaming     = mtk_jpeg_dec_stop_streaming,
> > +};
> > +
> > +static const struct vb2_ops mtk_jpeg_enc_qops = {
> > +	.queue_setup        = mtk_jpeg_queue_setup,
> > +	.buf_prepare        = mtk_jpeg_buf_prepare,
> > +	.buf_queue          = mtk_jpeg_enc_buf_queue,
> > +	.wait_prepare       = vb2_ops_wait_prepare,
> > +	.wait_finish        = vb2_ops_wait_finish,
> > +	.stop_streaming     = mtk_jpeg_enc_stop_streaming,
> >  };
> >  
> >  static void mtk_jpeg_set_dec_src(struct mtk_jpeg_ctx *ctx,
> > @@ -751,7 +1035,86 @@ static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
> >  	return 0;
> >  }
> >  
> > -static void mtk_jpeg_device_run(void *priv)
> > +static void mtk_jpeg_set_enc_dst(struct mtk_jpeg_ctx *ctx, void __iomem *base,
> > +				 struct vb2_buffer *dst_buf,
> > +				 struct mtk_jpeg_enc_bs *bs)
> > +{
> > +	bs->dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
> > +	bs->dma_addr_offset = ctx->enable_exif ? MTK_JPEG_DEFAULT_EXIF_SIZE : 0;
> > +	bs->dma_addr_offsetmask = bs->dma_addr & JPEG_ENC_DST_ADDR_OFFSET_MASK;
> > +	bs->size = vb2_plane_size(dst_buf, 0);
> > +
> > +	mtk_jpeg_enc_set_dst_addr(base, bs->dma_addr, bs->size,
> > +				  bs->dma_addr_offset,
> > +				  bs->dma_addr_offsetmask);
> > +}
> > +
> > +static void mtk_jpeg_set_enc_src(struct mtk_jpeg_ctx *ctx, void __iomem *base,
> > +				 struct vb2_buffer *src_buf)
> > +{
> > +	int i;
> > +	dma_addr_t	dma_addr;
> > +
> > +	mtk_jpeg_enc_set_img_size(base, ctx->out_q.w, ctx->out_q.h);
> > +	mtk_jpeg_enc_set_blk_num(base, ctx->out_q.fmt->fourcc, ctx->out_q.w,
> > +				 ctx->out_q.h);
> > +	mtk_jpeg_enc_set_stride(base, ctx->out_q.fmt->fourcc, ctx->out_q.w,
> > +				ctx->out_q.h, ctx->out_q.bytesperline[0]);
> > +
> > +	for (i = 0; i < src_buf->num_planes; i++) {
> > +		dma_addr = vb2_dma_contig_plane_dma_addr(src_buf, i) +
> > +			   src_buf->planes[i].data_offset;
> > +		mtk_jpeg_enc_set_src_addr(base, dma_addr, i);
> > +	}
> > +}
> > +
> > +static void mtk_jpeg_enc_device_run(void *priv)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = priv;
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > +	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > +	unsigned long flags;
> > +	struct mtk_jpeg_src_buf *jpeg_src_buf;
> > +	struct mtk_jpeg_enc_bs enc_bs;
> > +	int i, ret;
> > +
> > +	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > +	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > +	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> > +
> > +	if (jpeg_src_buf->flags & MTK_JPEG_BUF_FLAGS_LAST_FRAME) {
> > +		for (i = 0; i < dst_buf->vb2_buf.num_planes; i++)
> > +			vb2_set_plane_payload(&dst_buf->vb2_buf, i, 0);
> > +		buf_state = VB2_BUF_STATE_DONE;
> > +		goto enc_end;
> > +	}
> > +
> > +	ret = pm_runtime_get_sync(jpeg->dev);
> > +	if (ret < 0)
> > +		goto enc_end;
> > +
> > +	spin_lock_irqsave(&jpeg->hw_lock, flags);
> > +	mtk_jpeg_enc_reset(jpeg->reg_base);
> > +
> > +	mtk_jpeg_set_enc_dst(ctx, jpeg->reg_base, &dst_buf->vb2_buf, &enc_bs);
> > +	mtk_jpeg_set_enc_src(ctx, jpeg->reg_base, &src_buf->vb2_buf);
> > +	mtk_jpeg_enc_set_config(jpeg->reg_base, ctx->out_q.fmt->hw_format,
> > +				ctx->enable_exif, ctx->enc_quality,
> > +				ctx->restart_interval);
> > +	mtk_jpeg_enc_start(jpeg->reg_base);
> > +	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> > +	return;
> > +
> > +enc_end:
> > +	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > +	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > +	v4l2_m2m_buf_done(src_buf, buf_state);
> > +	v4l2_m2m_buf_done(dst_buf, buf_state);
> > +	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +}
> > +
> > +static void mtk_jpeg_dec_device_run(void *priv)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = priv;
> >  	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > @@ -786,15 +1149,16 @@ static void mtk_jpeg_device_run(void *priv)
> >  		goto dec_end;
> >  
> >  	mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
> > -	if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
> > +	if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param,
> > +				 &dst_buf->vb2_buf, &fb))
> >  		goto dec_end;
> >  
> >  	spin_lock_irqsave(&jpeg->hw_lock, flags);
> > -	mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > -	mtk_jpeg_dec_set_config(jpeg->dec_reg_base,
> > +	mtk_jpeg_dec_reset(jpeg->reg_base);
> > +	mtk_jpeg_dec_set_config(jpeg->reg_base,
> >  				&jpeg_src_buf->dec_param, &bs, &fb);
> >  
> > -	mtk_jpeg_dec_start(jpeg->dec_reg_base);
> > +	mtk_jpeg_dec_start(jpeg->reg_base);
> >  	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> >  	return;
> >  
> > @@ -806,20 +1170,30 @@ static void mtk_jpeg_device_run(void *priv)
> >  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> >  }
> >  
> > -static int mtk_jpeg_job_ready(void *priv)
> > +static int mtk_jpeg_enc_job_ready(void *priv)
> > +{
> > +		return 1;
> > +}
> > +
> > +static int mtk_jpeg_dec_job_ready(void *priv)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = priv;
> >  
> >  	return (ctx->state == MTK_JPEG_RUNNING) ? 1 : 0;
> >  }
> >  
> > -static const struct v4l2_m2m_ops mtk_jpeg_m2m_ops = {
> > -	.device_run = mtk_jpeg_device_run,
> > -	.job_ready  = mtk_jpeg_job_ready,
> > +static const struct v4l2_m2m_ops mtk_jpeg_enc_m2m_ops = {
> > +	.device_run = mtk_jpeg_enc_device_run,
> > +	.job_ready  = mtk_jpeg_enc_job_ready,
> >  };
> >  
> > -static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> > -			       struct vb2_queue *dst_vq)
> > +static const struct v4l2_m2m_ops mtk_jpeg_dec_m2m_ops = {
> > +	.device_run = mtk_jpeg_dec_device_run,
> > +	.job_ready  = mtk_jpeg_dec_job_ready,
> > +};
> > +
> > +static int mtk_jpeg_dec_queue_init(void *priv, struct vb2_queue *src_vq,
> > +				   struct vb2_queue *dst_vq)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = priv;
> >  	int ret;
> > @@ -828,7 +1202,7 @@ static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> >  	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> >  	src_vq->drv_priv = ctx;
> >  	src_vq->buf_struct_size = sizeof(struct mtk_jpeg_src_buf);
> > -	src_vq->ops = &mtk_jpeg_qops;
> > +	src_vq->ops = &mtk_jpeg_dec_qops;
> >  	src_vq->mem_ops = &vb2_dma_contig_memops;
> >  	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> >  	src_vq->lock = &ctx->jpeg->lock;
> > @@ -841,7 +1215,7 @@ static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> >  	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> >  	dst_vq->drv_priv = ctx;
> >  	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > -	dst_vq->ops = &mtk_jpeg_qops;
> > +	dst_vq->ops = &mtk_jpeg_dec_qops;
> >  	dst_vq->mem_ops = &vb2_dma_contig_memops;
> >  	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> >  	dst_vq->lock = &ctx->jpeg->lock;
> > @@ -851,24 +1225,112 @@ static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> >  	return ret;
> >  }
> >  
> > -static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
> > +static int mtk_jpeg_enc_queue_init(void *priv, struct vb2_queue *src_vq,
> > +				   struct vb2_queue *dst_vq)
> >  {
> > +	struct mtk_jpeg_ctx *ctx = priv;
> >  	int ret;
> >  
> > +	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > +	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> > +	src_vq->drv_priv = ctx;
> > +	src_vq->buf_struct_size = sizeof(struct mtk_jpeg_src_buf);
> > +	src_vq->ops = &mtk_jpeg_enc_qops;
> > +	src_vq->mem_ops = &vb2_dma_contig_memops;
> > +	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > +	src_vq->lock = &ctx->jpeg->lock;
> > +	src_vq->dev = ctx->jpeg->dev;
> > +	ret = vb2_queue_init(src_vq);
> > +	if (ret)
> > +		return ret;
> > +
> > +	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> > +	dst_vq->drv_priv = ctx;
> > +	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > +	dst_vq->ops = &mtk_jpeg_enc_qops;
> > +	dst_vq->mem_ops = &vb2_dma_contig_memops;
> > +	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > +	dst_vq->lock = &ctx->jpeg->lock;
> > +	dst_vq->dev = ctx->jpeg->dev;
> > +	ret = vb2_queue_init(dst_vq);
> > +
> > +	return ret;
> > +}
> > +
> > +static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
> > +{
> > +	int ret, i;
> > +
> >  	ret = mtk_smi_larb_get(jpeg->larb);
> >  	if (ret)
> >  		dev_err(jpeg->dev, "mtk_smi_larb_get larbvdec fail %d\n", ret);
> > -	clk_prepare_enable(jpeg->clk_jdec_smi);
> > -	clk_prepare_enable(jpeg->clk_jdec);
> > +
> > +	for (i = 0; i < jpeg->variant->num_clocks; i++) {
> > +		ret = clk_prepare_enable(jpeg->clocks[i]);
> > +		if (ret) {
> > +			while (--i >= 0)
> > +				clk_disable_unprepare(jpeg->clocks[i]);
> > +		}
> > +	}
> >  }
> >  
> >  static void mtk_jpeg_clk_off(struct mtk_jpeg_dev *jpeg)
> >  {
> > -	clk_disable_unprepare(jpeg->clk_jdec);
> > -	clk_disable_unprepare(jpeg->clk_jdec_smi);
> > +	int i;
> > +
> > +	for (i = jpeg->variant->num_clocks - 1; i >= 0; i--)
> > +		clk_disable_unprepare(jpeg->clocks[i]);
> >  	mtk_smi_larb_put(jpeg->larb);
> >  }
> >  
> > +static irqreturn_t mtk_jpeg_enc_irq(int irq, void *priv)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = priv;
> > +	struct mtk_jpeg_ctx *ctx;
> > +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > +	struct mtk_jpeg_src_buf *jpeg_src_buf;
> > +	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > +	u32 enc_irq_ret;
> > +	u32 enc_ret, result_size;
> > +
> > +	spin_lock(&jpeg->hw_lock);
> > +
> > +	ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> > +	if (!ctx) {
> > +		v4l2_err(&jpeg->v4l2_dev, "Context is NULL\n");
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > +	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > +	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> > +
> > +	enc_ret = mtk_jpeg_enc_get_and_clear_int_status(jpeg->reg_base);
> > +	enc_irq_ret = mtk_jpeg_enc_enum_result(jpeg->reg_base, enc_ret);
> > +
> > +	if (enc_irq_ret >= MTK_JPEG_ENC_RESULT_STALL)
> > +		mtk_jpeg_enc_reset(jpeg->reg_base);
> > +
> > +	if (enc_irq_ret != MTK_JPEG_ENC_RESULT_DONE) {
> > +		dev_err(jpeg->dev, "encode failed\n");
> > +		goto enc_end;
> > +	}
> > +
> > +	result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base);
> > +	vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
> > +
> > +	buf_state = VB2_BUF_STATE_DONE;
> > +
> > +enc_end:
> > +	v4l2_m2m_buf_done(src_buf, buf_state);
> > +	v4l2_m2m_buf_done(dst_buf, buf_state);
> > +	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +	spin_unlock(&jpeg->hw_lock);
> > +	pm_runtime_put_sync(ctx->jpeg->dev);
> > +	return IRQ_HANDLED;
> > +}
> > +
> >  static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> >  {
> >  	struct mtk_jpeg_dev *jpeg = priv;
> > @@ -876,13 +1338,13 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> >  	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> >  	struct mtk_jpeg_src_buf *jpeg_src_buf;
> >  	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > -	u32	dec_irq_ret;
> > +	u32 dec_irq_ret;
> >  	u32 dec_ret;
> >  	int i;
> >  
> >  	spin_lock(&jpeg->hw_lock);
> >  
> > -	dec_ret = mtk_jpeg_dec_get_int_status(jpeg->dec_reg_base);
> > +	dec_ret = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
> >  	dec_irq_ret = mtk_jpeg_dec_enum_result(dec_ret);
> >  	ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> >  	if (!ctx) {
> > @@ -895,7 +1357,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> >  	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> >  
> >  	if (dec_irq_ret >= MTK_JPEG_DEC_RESULT_UNDERFLOW)
> > -		mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > +		mtk_jpeg_dec_reset(jpeg->reg_base);
> >  
> >  	if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE) {
> >  		dev_err(jpeg->dev, "decode failed\n");
> > @@ -917,39 +1379,131 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static void mtk_jpeg_set_default_params(struct mtk_jpeg_ctx *ctx)
> > +static void mtk_jpeg_set_enc_default_params(struct mtk_jpeg_ctx *ctx)
> >  {
> >  	struct mtk_jpeg_q_data *q = &ctx->out_q;
> > -	int i;
> > +	struct v4l2_pix_format_mplane *pix_mp;
> > +
> > +	pix_mp = kmalloc(sizeof(*pix_mp), GFP_KERNEL);
> >  
> > +	ctx->fh.ctrl_handler = &ctx->ctrl_hdl;
> >  	ctx->colorspace = V4L2_COLORSPACE_JPEG,
> >  	ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> >  	ctx->quantization = V4L2_QUANTIZATION_DEFAULT;
> >  	ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > -
> > -	q->fmt = mtk_jpeg_find_format(ctx, V4L2_PIX_FMT_JPEG,
> > -					      MTK_JPEG_FMT_TYPE_OUTPUT);
> > -	q->w = MTK_JPEG_MIN_WIDTH;
> > -	q->h = MTK_JPEG_MIN_HEIGHT;
> > -	q->bytesperline[0] = 0;
> > -	q->sizeimage[0] = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > +	pix_mp->width = MTK_JPEG_MIN_WIDTH;
> > +	pix_mp->height = MTK_JPEG_MIN_HEIGHT;
> > +
> > +	q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_YUYV,
> > +				      MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +	vidioc_try_fmt(container_of(pix_mp, struct v4l2_format,
> > +				    fmt.pix_mp), q->fmt);
> > +	q->w = pix_mp->width;
> > +	q->h = pix_mp->height;
> > +	q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage;
> > +	q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline;
> >  
> >  	q = &ctx->cap_q;
> > -	q->fmt = mtk_jpeg_find_format(ctx, V4L2_PIX_FMT_YUV420M,
> > -					      MTK_JPEG_FMT_TYPE_CAPTURE);
> > -	q->w = MTK_JPEG_MIN_WIDTH;
> > -	q->h = MTK_JPEG_MIN_HEIGHT;
> > +	q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_JPEG,
> > +				      MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +	pix_mp->width = MTK_JPEG_MIN_WIDTH;
> > +	pix_mp->height = MTK_JPEG_MIN_HEIGHT;
> > +	vidioc_try_fmt(container_of(pix_mp, struct v4l2_format,
> > +				    fmt.pix_mp), q->fmt);
> > +	q->w = pix_mp->width;
> > +	q->h = pix_mp->height;
> > +	q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage;
> > +	q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline;
> > +}
> > +
> > +static void mtk_jpeg_set_dec_default_params(struct mtk_jpeg_ctx *ctx)
> > +{
> > +	struct mtk_jpeg_q_data *q = &ctx->out_q;
> > +	struct v4l2_pix_format_mplane *pix_mp;
> > +	int i;
> > +
> > +	pix_mp = kmalloc(sizeof(*pix_mp), GFP_KERNEL);
> >  
> > +	ctx->fh.ctrl_handler = &ctx->ctrl_hdl;
> > +	ctx->colorspace = V4L2_COLORSPACE_JPEG,
> > +	ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > +	ctx->quantization = V4L2_QUANTIZATION_DEFAULT;
> > +	ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > +	pix_mp->width = MTK_JPEG_MIN_WIDTH;
> > +	pix_mp->height = MTK_JPEG_MIN_HEIGHT;
> > +
> > +	q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_JPEG,
> > +				      MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > +	vidioc_try_fmt(container_of(pix_mp, struct v4l2_format,
> > +				    fmt.pix_mp), q->fmt);
> > +	q->w = pix_mp->width;
> > +	q->h = pix_mp->height;
> > +	q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage;
> > +	q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline;
> > +
> > +	q = &ctx->cap_q;
> > +	q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_YUV420M,
> > +				      MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> > +	pix_mp->width = MTK_JPEG_MIN_WIDTH;
> > +	pix_mp->height = MTK_JPEG_MIN_HEIGHT;
> > +	vidioc_try_fmt(container_of(pix_mp, struct v4l2_format,
> > +				    fmt.pix_mp), q->fmt);
> > +	q->w = pix_mp->width;
> > +	q->h = pix_mp->height;
> >  	for (i = 0; i < q->fmt->colplanes; i++) {
> > -		u32 stride = q->w * q->fmt->h_sample[i] / 4;
> > -		u32 h = q->h * q->fmt->v_sample[i] / 4;
> > +		q->sizeimage[i] = pix_mp->plane_fmt[i].sizeimage;
> > +		q->bytesperline[i] = pix_mp->plane_fmt[i].bytesperline;
> > +	}
> > +}
> > +
> > +static int mtk_jpeg_enc_open(struct file *file)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +	struct video_device *vfd = video_devdata(file);
> > +	struct mtk_jpeg_ctx *ctx;
> > +	int ret = 0;
> >  
> > -		q->bytesperline[i] = stride;
> > -		q->sizeimage[i] = stride * h;
> > +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	if (mutex_lock_interruptible(&jpeg->lock)) {
> > +		ret = -ERESTARTSYS;
> > +		goto free;
> > +	}
> > +
> > +	v4l2_fh_init(&ctx->fh, vfd);
> > +	file->private_data = &ctx->fh;
> > +	v4l2_fh_add(&ctx->fh);
> > +
> > +	ctx->jpeg = jpeg;
> > +	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
> > +					    mtk_jpeg_enc_queue_init);
> > +	if (IS_ERR(ctx->fh.m2m_ctx)) {
> > +		ret = PTR_ERR(ctx->fh.m2m_ctx);
> > +		goto error;
> >  	}
> > +
> > +	ret = mtk_jpeg_enc_ctrls_setup(ctx);
> > +	if (ret) {
> > +		v4l2_err(&jpeg->v4l2_dev, "Failed to setup jpeg enc controls\n");
> > +		goto error;
> > +	}
> > +	mtk_jpeg_set_enc_default_params(ctx);
> > +
> > +	mutex_unlock(&jpeg->lock);
> > +	return 0;
> > +
> > +error:
> > +	v4l2_fh_del(&ctx->fh);
> > +	v4l2_fh_exit(&ctx->fh);
> > +	mutex_unlock(&jpeg->lock);
> > +free:
> > +	kfree(ctx);
> > +	return ret;
> >  }
> >  
> > -static int mtk_jpeg_open(struct file *file)
> > +static int mtk_jpeg_dec_open(struct file *file)
> >  {
> >  	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> >  	struct video_device *vfd = video_devdata(file);
> > @@ -971,13 +1525,20 @@ static int mtk_jpeg_open(struct file *file)
> >  
> >  	ctx->jpeg = jpeg;
> >  	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
> > -					    mtk_jpeg_queue_init);
> > +					    mtk_jpeg_dec_queue_init);
> >  	if (IS_ERR(ctx->fh.m2m_ctx)) {
> >  		ret = PTR_ERR(ctx->fh.m2m_ctx);
> >  		goto error;
> >  	}
> >  
> > -	mtk_jpeg_set_default_params(ctx);
> > +	v4l2_ctrl_handler_init(&ctx->ctrl_hdl, 0);
> > +	ret = v4l2_ctrl_handler_setup(&ctx->ctrl_hdl);
> > +	if (ret) {
> > +		v4l2_err(&jpeg->v4l2_dev, "Failed to setup jpeg dec controls\n");
> > +		goto error;
> > +	}
> > +	mtk_jpeg_set_dec_default_params(ctx);
> > +
> >  	mutex_unlock(&jpeg->lock);
> >  	return 0;
> >  
> > @@ -997,6 +1558,7 @@ static int mtk_jpeg_release(struct file *file)
> >  
> >  	mutex_lock(&jpeg->lock);
> >  	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
> > +	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> >  	v4l2_fh_del(&ctx->fh);
> >  	v4l2_fh_exit(&ctx->fh);
> >  	kfree(ctx);
> > @@ -1004,9 +1566,18 @@ static int mtk_jpeg_release(struct file *file)
> >  	return 0;
> >  }
> >  
> > -static const struct v4l2_file_operations mtk_jpeg_fops = {
> > +static const struct v4l2_file_operations mtk_jpeg_enc_fops = {
> >  	.owner          = THIS_MODULE,
> > -	.open           = mtk_jpeg_open,
> > +	.open           = mtk_jpeg_enc_open,
> > +	.release        = mtk_jpeg_release,
> > +	.poll           = v4l2_m2m_fop_poll,
> > +	.unlocked_ioctl = video_ioctl2,
> > +	.mmap           = v4l2_m2m_fop_mmap,
> > +};
> > +
> > +static const struct v4l2_file_operations mtk_jpeg_dec_fops = {
> > +	.owner          = THIS_MODULE,
> > +	.open           = mtk_jpeg_dec_open,
> >  	.release        = mtk_jpeg_release,
> >  	.poll           = v4l2_m2m_fop_poll,
> >  	.unlocked_ioctl = video_ioctl2,
> > @@ -1017,6 +1588,7 @@ static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
> >  {
> >  	struct device_node *node;
> >  	struct platform_device *pdev;
> > +	int i;
> >  
> >  	node = of_parse_phandle(jpeg->dev->of_node, "mediatek,larb", 0);
> >  	if (!node)
> > @@ -1030,19 +1602,24 @@ static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
> >  
> >  	jpeg->larb = &pdev->dev;
> >  
> > -	jpeg->clk_jdec = devm_clk_get(jpeg->dev, "jpgdec");
> > -	if (IS_ERR(jpeg->clk_jdec))
> > -		return PTR_ERR(jpeg->clk_jdec);
> > +	for (i = 0; i < jpeg->variant->num_clocks; i++) {
> > +		jpeg->clocks[i] = devm_clk_get(jpeg->dev,
> > +					       jpeg->variant->clk_names[i]);
> > +		if (IS_ERR(jpeg->clocks[i])) {
> > +			dev_err(&pdev->dev, "failed to get clock: %s\n",
> > +				jpeg->variant->clk_names[i]);
> > +			return PTR_ERR(jpeg->clocks[i]);
> > +		}
> > +	}
> >  
> > -	jpeg->clk_jdec_smi = devm_clk_get(jpeg->dev, "jpgdec-smi");
> > -	return PTR_ERR_OR_ZERO(jpeg->clk_jdec_smi);
> > +	return 0;
> >  }
> >  
> >  static int mtk_jpeg_probe(struct platform_device *pdev)
> >  {
> >  	struct mtk_jpeg_dev *jpeg;
> >  	struct resource *res;
> > -	int dec_irq;
> > +	int jpeg_irq;
> >  	int ret;
> >  
> >  	jpeg = devm_kzalloc(&pdev->dev, sizeof(*jpeg), GFP_KERNEL);
> > @@ -1052,25 +1629,30 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
> >  	mutex_init(&jpeg->lock);
> >  	spin_lock_init(&jpeg->hw_lock);
> >  	jpeg->dev = &pdev->dev;
> > +	jpeg->variant = of_device_get_match_data(jpeg->dev);
> >  
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	jpeg->dec_reg_base = devm_ioremap_resource(&pdev->dev, res);
> > -	if (IS_ERR(jpeg->dec_reg_base)) {
> > -		ret = PTR_ERR(jpeg->dec_reg_base);
> > +	jpeg->reg_base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(jpeg->reg_base)) {
> > +		ret = PTR_ERR(jpeg->reg_base);
> >  		return ret;
> >  	}
> >  
> > -	dec_irq = platform_get_irq(pdev, 0);
> > -	if (dec_irq < 0) {
> > -		dev_err(&pdev->dev, "Failed to get dec_irq %d.\n", dec_irq);
> > -		return dec_irq;
> > +	jpeg_irq = platform_get_irq(pdev, 0);
> > +	if (jpeg_irq < 0) {
> > +		dev_err(&pdev->dev, "Failed to get jpeg_irq %d.\n", jpeg_irq);
> > +		return jpeg_irq;
> >  	}
> >  
> > -	ret = devm_request_irq(&pdev->dev, dec_irq, mtk_jpeg_dec_irq, 0,
> > -			       pdev->name, jpeg);
> > +	if (jpeg->variant->is_encoder)
> > +		ret = devm_request_irq(&pdev->dev, jpeg_irq, mtk_jpeg_enc_irq,
> > +				       0, pdev->name, jpeg);
> > +	else
> > +		ret = devm_request_irq(&pdev->dev, jpeg_irq, mtk_jpeg_dec_irq,
> > +				       0, pdev->name, jpeg);
> >  	if (ret) {
> > -		dev_err(&pdev->dev, "Failed to request dec_irq %d (%d)\n",
> > -			dec_irq, ret);
> > +		dev_err(&pdev->dev, "Failed to request jpeg_irq %d (%d)\n",
> > +			jpeg_irq, ret);
> >  		goto err_req_irq;
> >  	}
> >  
> > @@ -1087,40 +1669,50 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
> >  		goto err_dev_register;
> >  	}
> >  
> > -	jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_m2m_ops);
> > +	if (jpeg->variant->is_encoder)
> > +		jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_enc_m2m_ops);
> > +	else
> > +		jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_dec_m2m_ops);
> >  	if (IS_ERR(jpeg->m2m_dev)) {
> >  		v4l2_err(&jpeg->v4l2_dev, "Failed to init mem2mem device\n");
> >  		ret = PTR_ERR(jpeg->m2m_dev);
> >  		goto err_m2m_init;
> >  	}
> >  
> > -	jpeg->dec_vdev = video_device_alloc();
> > -	if (!jpeg->dec_vdev) {
> > +	jpeg->vdev = video_device_alloc();
> > +	if (!jpeg->vdev) {
> >  		ret = -ENOMEM;
> > -		goto err_dec_vdev_alloc;
> > +		goto err_vfd_jpeg_alloc;
> >  	}
> > -	snprintf(jpeg->dec_vdev->name, sizeof(jpeg->dec_vdev->name),
> > -		 "%s-dec", MTK_JPEG_NAME);
> > -	jpeg->dec_vdev->fops = &mtk_jpeg_fops;
> > -	jpeg->dec_vdev->ioctl_ops = &mtk_jpeg_ioctl_ops;
> > -	jpeg->dec_vdev->minor = -1;
> > -	jpeg->dec_vdev->release = video_device_release;
> > -	jpeg->dec_vdev->lock = &jpeg->lock;
> > -	jpeg->dec_vdev->v4l2_dev = &jpeg->v4l2_dev;
> > -	jpeg->dec_vdev->vfl_dir = VFL_DIR_M2M;
> > -	jpeg->dec_vdev->device_caps = V4L2_CAP_STREAMING |
> > +	snprintf(jpeg->vdev->name, sizeof(jpeg->vdev->name),
> > +		 "%s-%s", MTK_JPEG_NAME,
> > +		 jpeg->variant->is_encoder ? "enc" : "dec");
> > +	if (jpeg->variant->is_encoder) {
> > +		jpeg->vdev->fops = &mtk_jpeg_enc_fops;
> > +		jpeg->vdev->ioctl_ops = &mtk_jpeg_enc_ioctl_ops;
> > +	} else {
> > +		jpeg->vdev->fops = &mtk_jpeg_dec_fops;
> > +		jpeg->vdev->ioctl_ops = &mtk_jpeg_dec_ioctl_ops;
> > +	}
> > +	jpeg->vdev->minor = -1;
> > +	jpeg->vdev->release = video_device_release;
> > +	jpeg->vdev->lock = &jpeg->lock;
> > +	jpeg->vdev->v4l2_dev = &jpeg->v4l2_dev;
> > +	jpeg->vdev->vfl_dir = VFL_DIR_M2M;
> > +	jpeg->vdev->device_caps = V4L2_CAP_STREAMING |
> >  				      V4L2_CAP_VIDEO_M2M_MPLANE;
> >  
> > -	ret = video_register_device(jpeg->dec_vdev, VFL_TYPE_GRABBER, -1);
> > +	ret = video_register_device(jpeg->vdev, VFL_TYPE_GRABBER, -1);
> >  	if (ret) {
> >  		v4l2_err(&jpeg->v4l2_dev, "Failed to register video device\n");
> > -		goto err_dec_vdev_register;
> > +		goto err_vfd_jpeg_register;
> >  	}
> >  
> > -	video_set_drvdata(jpeg->dec_vdev, jpeg);
> > +	video_set_drvdata(jpeg->vdev, jpeg);
> >  	v4l2_info(&jpeg->v4l2_dev,
> > -		  "decoder device registered as /dev/video%d (%d,%d)\n",
> > -		  jpeg->dec_vdev->num, VIDEO_MAJOR, jpeg->dec_vdev->minor);
> > +		  "jpeg %s device registered as /dev/video%d (%d,%d)\n",
> > +		  jpeg->variant->is_encoder ? "enc" : "dec", jpeg->vdev->num,
> > +		  VIDEO_MAJOR, jpeg->vdev->minor);
> >  
> >  	platform_set_drvdata(pdev, jpeg);
> >  
> > @@ -1128,10 +1720,10 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
> >  
> >  	return 0;
> >  
> > -err_dec_vdev_register:
> > -	video_device_release(jpeg->dec_vdev);
> > +err_vfd_jpeg_register:
> > +	video_device_release(jpeg->vdev);
> >  
> > -err_dec_vdev_alloc:
> > +err_vfd_jpeg_alloc:
> >  	v4l2_m2m_release(jpeg->m2m_dev);
> >  
> >  err_m2m_init:
> > @@ -1151,8 +1743,8 @@ static int mtk_jpeg_remove(struct platform_device *pdev)
> >  	struct mtk_jpeg_dev *jpeg = platform_get_drvdata(pdev);
> >  
> >  	pm_runtime_disable(&pdev->dev);
> > -	video_unregister_device(jpeg->dec_vdev);
> > -	video_device_release(jpeg->dec_vdev);
> > +	video_unregister_device(jpeg->vdev);
> > +	video_device_release(jpeg->vdev);
> >  	v4l2_m2m_release(jpeg->m2m_dev);
> >  	v4l2_device_unregister(&jpeg->v4l2_dev);
> >  
> > @@ -1211,14 +1803,36 @@ static const struct dev_pm_ops mtk_jpeg_pm_ops = {
> >  	SET_RUNTIME_PM_OPS(mtk_jpeg_pm_suspend, mtk_jpeg_pm_resume, NULL)
> >  };
> >  
> > +static struct mtk_jpeg_variant mt8173_jpeg_drvdata = {
> > +	.is_encoder	= false,
> > +	.clk_names	= {"jpgdec-smi", "jpgdec"},
> > +	.num_clocks	= 2,
> > +};
> > +
> > +static struct mtk_jpeg_variant mt2701_jpeg_drvdata = {
> > +	.is_encoder	= false,
> > +	.clk_names	= {"jpgdec-smi", "jpgdec"},
> > +	.num_clocks	= 2,
> > +};
> > +
> > +static struct mtk_jpeg_variant mtk_jpeg_drvdata = {
> > +	.is_encoder	= true,
> > +	.clk_names	= {"jpgenc"},
> > +	.num_clocks	= 1,
> > +};
> > +
> >  static const struct of_device_id mtk_jpeg_match[] = {
> >  	{
> >  		.compatible = "mediatek,mt8173-jpgdec",
> > -		.data       = NULL,
> > +		.data = &mt8173_jpeg_drvdata,
> >  	},
> >  	{
> >  		.compatible = "mediatek,mt2701-jpgdec",
> > -		.data       = NULL,
> > +		.data = &mt2701_jpeg_drvdata,
> > +	},
> > +	{
> > +		.compatible = "mediatek,mtk-jpgenc",
> > +		.data = &mtk_jpeg_drvdata,
> >  	},
> >  	{},
> >  };
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 9bbd615b1067..8f80f2a69d45 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -3,6 +3,7 @@
> >   * Copyright (c) 2016 MediaTek Inc.
> >   * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> >   *         Rick Chang <rick.chang@mediatek.com>
> > + *         Xia Jiang <xia.jiang@mediatek.com>
> >   */
> >  
> >  #ifndef _MTK_JPEG_CORE_H
> > @@ -16,19 +17,21 @@
> >  #define MTK_JPEG_NAME		"mtk-jpeg"
> >  
> >  #define MTK_JPEG_COMP_MAX		3
> > +#define MTK_JPEG_MAX_CLOCKS		2
> > +
> >  
> >  #define MTK_JPEG_FMT_FLAG_DEC_OUTPUT	BIT(0)
> >  #define MTK_JPEG_FMT_FLAG_DEC_CAPTURE	BIT(1)
> > -
> > -#define MTK_JPEG_FMT_TYPE_OUTPUT	1
> > -#define MTK_JPEG_FMT_TYPE_CAPTURE	2
> > +#define MTK_JPEG_FMT_FLAG_ENC_OUTPUT	BIT(2)
> > +#define MTK_JPEG_FMT_FLAG_ENC_CAPTURE	BIT(3)
> >  
> >  #define MTK_JPEG_MIN_WIDTH	32U
> >  #define MTK_JPEG_MIN_HEIGHT	32U
> > -#define MTK_JPEG_MAX_WIDTH	8192U
> > -#define MTK_JPEG_MAX_HEIGHT	8192U
> > +#define MTK_JPEG_MAX_WIDTH	65535U
> > +#define MTK_JPEG_MAX_HEIGHT	65535U
> >  
> >  #define MTK_JPEG_DEFAULT_SIZEIMAGE	(1 * 1024 * 1024)
> > +#define MTK_JPEG_DEFAULT_EXIF_SIZE	(64 * 1024)
> >  
> >  /**
> >   * enum mtk_jpeg_ctx_state - contex state of jpeg
> > @@ -39,6 +42,18 @@ enum mtk_jpeg_ctx_state {
> >  	MTK_JPEG_SOURCE_CHANGE,
> >  };
> >  
> > +/**
> > + * mtk_jpeg_variant - mtk jpeg driver variant
> > + * @is_encoder:		driver mode is jpeg encoder
> > + * @clk_names:		clock names
> > + * @num_clocks:		numbers of clock
> > + */
> > +struct mtk_jpeg_variant {
> > +	bool is_encoder;
> > +	const char		*clk_names[MTK_JPEG_MAX_CLOCKS];
> > +	int			num_clocks;
> > +};
> > +
> >  /**
> >   * struct mt_jpeg - JPEG IP abstraction
> >   * @lock:		the mutex protecting this structure
> > @@ -48,11 +63,11 @@ enum mtk_jpeg_ctx_state {
> >   * @v4l2_dev:		v4l2 device for mem2mem mode
> >   * @m2m_dev:		v4l2 mem2mem device data
> >   * @alloc_ctx:		videobuf2 memory allocator's context
> > - * @dec_vdev:		video device node for decoder mem2mem mode
> > - * @dec_reg_base:	JPEG registers mapping
> > - * @clk_jdec:		JPEG hw working clock
> > - * @clk_jdec_smi:	JPEG SMI bus clock
> > + * @vdev:		video device node for jpeg mem2mem mode
> > + * @reg_base:		JPEG registers mapping
> >   * @larb:		SMI device
> > + * @clocks:		JPEG IP clock(s)
> > + * @variant:		driver variant to be used
> >   */
> >  struct mtk_jpeg_dev {
> >  	struct mutex		lock;
> > @@ -62,16 +77,17 @@ struct mtk_jpeg_dev {
> >  	struct v4l2_device	v4l2_dev;
> >  	struct v4l2_m2m_dev	*m2m_dev;
> >  	void			*alloc_ctx;
> > -	struct video_device	*dec_vdev;
> > -	void __iomem		*dec_reg_base;
> > -	struct clk		*clk_jdec;
> > -	struct clk		*clk_jdec_smi;
> > +	struct video_device	*vdev;
> > +	void __iomem		*reg_base;
> >  	struct device		*larb;
> > +	struct clk		*clocks[MTK_JPEG_MAX_CLOCKS];
> > +	const struct mtk_jpeg_variant *variant;
> >  };
> >  
> >  /**
> >   * struct jpeg_fmt - driver's internal color format data
> >   * @fourcc:	the fourcc code, 0 if not applicable
> > + * @hw_format:	hardware format value
> >   * @h_sample:	horizontal sample count of plane in 4 * 4 pixel image
> >   * @v_sample:	vertical sample count of plane in 4 * 4 pixel image
> >   * @colplanes:	number of color planes (1 for packed formats)
> > @@ -81,6 +97,7 @@ struct mtk_jpeg_dev {
> >   */
> >  struct mtk_jpeg_fmt {
> >  	u32	fourcc;
> > +	u32	hw_format;
> >  	int	h_sample[VIDEO_MAX_PLANES];
> >  	int	v_sample[VIDEO_MAX_PLANES];
> >  	int	colplanes;
> > @@ -113,6 +130,10 @@ struct mtk_jpeg_q_data {
> >   * @cap_q:		destination (capture) queue queue information
> >   * @fh:			V4L2 file handle
> >   * @state:		state of the context
> > + * @enable_exif:	enable exif mode of jpeg encoder
> > + * @enc_quality:	jpeg encoder quality
> > + * @restart_interval:	jpeg encoder restart interval
> > + * @ctrl_hdl:		controls handler
> >   * @colorspace: enum v4l2_colorspace; supplemental to pixelformat
> >   * @ycbcr_enc: enum v4l2_ycbcr_encoding, Y'CbCr encoding
> >   * @quantization: enum v4l2_quantization, colorspace quantization
> > @@ -124,6 +145,10 @@ struct mtk_jpeg_ctx {
> >  	struct mtk_jpeg_q_data		cap_q;
> >  	struct v4l2_fh			fh;
> >  	enum mtk_jpeg_ctx_state		state;
> > +	bool				enable_exif;
> > +	u8				enc_quality;
> > +	u8				restart_interval;
> > +	struct v4l2_ctrl_handler	ctrl_hdl;
> >  
> >  	enum v4l2_colorspace colorspace;
> >  	enum v4l2_ycbcr_encoding ycbcr_enc;
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
> > index 1cc37dbfc8e7..ce263db5f30a 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
> > @@ -3,10 +3,11 @@
> >   * Copyright (c) 2016 MediaTek Inc.
> >   * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> >   *         Rick Chang <rick.chang@mediatek.com>
> > + *         Xia Jiang <xia.jiang@mediatek.com>
> >   */
> >  
> > -#ifndef _MTK_JPEG_HW_H
> > -#define _MTK_JPEG_HW_H
> > +#ifndef _MTK_JPEG_DEC_HW_H
> > +#define _MTK_JPEG_DEC_HW_H
> >  
> >  #include <media/videobuf2-core.h>
> >  
> > @@ -75,4 +76,4 @@ void mtk_jpeg_dec_set_config(void __iomem *base,
> >  void mtk_jpeg_dec_reset(void __iomem *dec_reg_base);
> >  void mtk_jpeg_dec_start(void __iomem *dec_reg_base);
> >  
> > -#endif /* _MTK_JPEG_HW_H */
> > +#endif /* _MTK_JPEG_DEC_HW_H */
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > new file mode 100644
> > index 000000000000..7fc1de920a75
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > @@ -0,0 +1,193 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2019 MediaTek Inc.
> > + * Author: Xia Jiang <xia.jiang@mediatek.com>
> > + *
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <media/videobuf2-core.h>
> > +
> > +#include "mtk_jpeg_enc_hw.h"
> > +
> > +static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = {
> > +	{.quality_param = 34, .hardware_value = JPEG_ENC_QUALITY_Q34},
> > +	{.quality_param = 39, .hardware_value = JPEG_ENC_QUALITY_Q39},
> > +	{.quality_param = 48, .hardware_value = JPEG_ENC_QUALITY_Q48},
> > +	{.quality_param = 60, .hardware_value = JPEG_ENC_QUALITY_Q60},
> > +	{.quality_param = 64, .hardware_value = JPEG_ENC_QUALITY_Q64},
> > +	{.quality_param = 68, .hardware_value = JPEG_ENC_QUALITY_Q68},
> > +	{.quality_param = 74, .hardware_value = JPEG_ENC_QUALITY_Q74},
> > +	{.quality_param = 80, .hardware_value = JPEG_ENC_QUALITY_Q80},
> > +	{.quality_param = 82, .hardware_value = JPEG_ENC_QUALITY_Q82},
> > +	{.quality_param = 84, .hardware_value = JPEG_ENC_QUALITY_Q84},
> > +	{.quality_param = 87, .hardware_value = JPEG_ENC_QUALITY_Q87},
> > +	{.quality_param = 90, .hardware_value = JPEG_ENC_QUALITY_Q90},
> > +	{.quality_param = 92, .hardware_value = JPEG_ENC_QUALITY_Q92},
> > +	{.quality_param = 95, .hardware_value = JPEG_ENC_QUALITY_Q95},
> > +	{.quality_param = 97, .hardware_value = JPEG_ENC_QUALITY_Q97},
> > +};
> > +
> > +void mtk_jpeg_enc_reset(void __iomem *base)
> > +{
> > +	writel(0x00, base + JPEG_ENC_RSTB);
> > +	writel(JPEG_ENC_RESET_BIT, base + JPEG_ENC_RSTB);
> > +	writel(0x00, base + JPEG_ENC_CODEC_SEL);
> > +}
> > +
> > +u32 mtk_jpeg_enc_get_and_clear_int_status(void __iomem *base)
> > +{
> > +	u32 ret;
> > +
> > +	ret = readl(base + JPEG_ENC_INT_STS) &
> > +		    JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
> > +	if (ret)
> > +		writel(0, base + JPEG_ENC_INT_STS);
> > +
> > +	return ret;
> > +}
> > +
> > +u32 mtk_jpeg_enc_get_file_size(void __iomem *base)
> > +{
> > +	return readl(base + JPEG_ENC_DMA_ADDR0) -
> > +	       readl(base + JPEG_ENC_DST_ADDR0);
> > +}
> > +
> > +u32 mtk_jpeg_enc_enum_result(void __iomem *base, u32 irq_status)
> > +{
> > +	if (irq_status & JPEG_ENC_INT_STATUS_DONE)
> > +		return MTK_JPEG_ENC_RESULT_DONE;
> > +	else if (irq_status & JPEG_ENC_INT_STATUS_STALL)
> > +		return MTK_JPEG_ENC_RESULT_STALL;
> > +	else
> > +		return MTK_JPEG_ENC_RESULT_VCODEC_IRQ;
> > +}
> > +
> > +void mtk_jpeg_enc_set_img_size(void __iomem *base, u32 width, u32 height)
> > +{
> > +	u32 value;
> > +
> > +	value = width << 16 | height;
> > +	writel(value, base + JPEG_ENC_IMG_SIZE);
> > +}
> > +
> > +void mtk_jpeg_enc_set_blk_num(void __iomem *base, u32 enc_format, u32 width,
> > +			      u32 height)
> > +{
> > +	u32 blk_num;
> > +	u32 is_420;
> > +	u32 padding_width;
> > +	u32 padding_height;
> > +	u32 luma_blocks;
> > +	u32 chroma_blocks;
> > +
> > +	is_420 = (enc_format == V4L2_PIX_FMT_NV12M ||
> > +		  enc_format == V4L2_PIX_FMT_NV21M) ? 1 : 0;
> > +	padding_width = round_up(width, 16);
> > +	padding_height = round_up(height, is_420 ? 16 : 8);
> > +
> > +	luma_blocks = padding_width / 8 * padding_height / 8;
> > +	if (is_420)
> > +		chroma_blocks = luma_blocks / 4;
> > +	else
> > +		chroma_blocks = luma_blocks / 2;
> > +
> > +	blk_num = luma_blocks + 2 * chroma_blocks - 1;
> > +
> > +	writel(blk_num, base + JPEG_ENC_BLK_NUM);
> > +}
> > +
> > +void mtk_jpeg_enc_set_stride(void __iomem *base, u32 enc_format, u32 width,
> > +			     u32 height, u32 bytesperline)
> > +{
> > +	u32 img_stride;
> > +	u32 mem_stride;
> > +
> > +	if (enc_format == V4L2_PIX_FMT_NV12M ||
> > +	    enc_format == V4L2_PIX_FMT_NV21M) {
> > +		img_stride = round_up(width, 16);
> > +		mem_stride = bytesperline;
> > +	} else {
> > +		img_stride = round_up(width * 2, 32);
> > +		mem_stride = img_stride;
> > +	}
> > +
> > +	writel(img_stride, base + JPEG_ENC_IMG_STRIDE);
> > +	writel(mem_stride, base + JPEG_ENC_STRIDE);
> > +}
> > +
> > +void mtk_jpeg_enc_set_src_addr(void __iomem *base, u32 src_addr,
> > +			       u32 plane_index)
> > +{
> > +	if (!plane_index)
> > +		writel(src_addr, base + JPEG_ENC_SRC_LUMA_ADDR);
> > +	else
> > +		writel(src_addr, base + JPEG_ENC_SRC_CHROMA_ADDR);
> > +}
> > +
> > +void mtk_jpeg_enc_set_dst_addr(void __iomem *base, u32 dst_addr,
> > +			       u32 stall_size, u32 init_offset,
> > +			       u32 offset_mask)
> > +{
> > +	writel(init_offset & ~0xf, base + JPEG_ENC_OFFSET_ADDR);
> > +	writel(offset_mask & 0xf, base + JPEG_ENC_BYTE_OFFSET_MASK);
> > +	writel(dst_addr & ~0xf, base + JPEG_ENC_DST_ADDR0);
> > +	writel((dst_addr + stall_size) & ~0xf, base + JPEG_ENC_STALL_ADDR0);
> > +}
> > +
> > +static void mtk_jpeg_enc_set_quality(void __iomem *base, u32 quality)
> > +{
> > +	u32 value;
> > +	u32 i, enc_quality;
> > +
> > +	enc_quality = mtk_jpeg_enc_quality[0].hardware_value;
> > +	for (i = 0; i < ARRAY_SIZE(mtk_jpeg_enc_quality); i++) {
> > +		if (quality <= mtk_jpeg_enc_quality[i].quality_param) {
> > +			enc_quality = mtk_jpeg_enc_quality[i].hardware_value;
> > +			break;
> > +		}
> > +	}
> > +
> > +	value = readl(base + JPEG_ENC_QUALITY);
> > +	value = (value & JPEG_ENC_QUALITY_MASK) | enc_quality;
> > +	writel(value, base + JPEG_ENC_QUALITY);
> > +}
> > +
> > +static void mtk_jpeg_enc_set_ctrl(void __iomem *base, u32 enc_format,
> > +				  bool exif_en, u32 restart_interval)
> > +{
> > +	u32 value;
> > +
> > +	value = readl(base + JPEG_ENC_CTRL);
> > +	value &= ~JPEG_ENC_CTRL_YUV_FORMAT_MASK;
> > +	value |= (enc_format & 3) << 3;
> > +	if (exif_en)
> > +		value |= JPEG_ENC_CTRL_FILE_FORMAT_BIT;
> > +	else
> > +		value &= ~JPEG_ENC_CTRL_FILE_FORMAT_BIT;
> > +	if (restart_interval)
> > +		value |= JPEG_ENC_CTRL_RESTART_EN_BIT;
> > +	else
> > +		value &= ~JPEG_ENC_CTRL_RESTART_EN_BIT;
> > +	writel(value, base + JPEG_ENC_CTRL);
> > +}
> > +
> > +void mtk_jpeg_enc_set_config(void __iomem *base, u32 enc_format, bool exif_en,
> > +			     u32 quality, u32 restart_interval)
> > +{
> > +	mtk_jpeg_enc_set_quality(base, quality);
> > +
> > +	mtk_jpeg_enc_set_ctrl(base, enc_format, exif_en, restart_interval);
> > +
> > +	writel(restart_interval, base + JPEG_ENC_RST_MCU_NUM);
> > +}
> > +
> > +void mtk_jpeg_enc_start(void __iomem *base)
> > +{
> > +	u32 value;
> > +
> > +	value = readl(base + JPEG_ENC_CTRL);
> > +	value |= JPEG_ENC_CTRL_INT_EN_BIT | JPEG_ENC_CTRL_ENABLE_BIT;
> > +	writel(value, base + JPEG_ENC_CTRL);
> > +}
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> > new file mode 100644
> > index 000000000000..73faf49b667c
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> > @@ -0,0 +1,123 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2019 MediaTek Inc.
> > + * Author: Xia Jiang <xia.jiang@mediatek.com>
> > + *
> > + */
> > +
> > +#ifndef _MTK_JPEG_ENC_HW_H
> > +#define _MTK_JPEG_ENC_HW_H
> > +
> > +#include <media/videobuf2-core.h>
> > +
> > +#include "mtk_jpeg_core.h"
> > +
> > +#define JPEG_ENC_INT_STATUS_DONE	BIT(0)
> > +#define JPEG_ENC_INT_STATUS_STALL	BIT(1)
> > +#define JPEG_ENC_INT_STATUS_VCODEC_IRQ	BIT(4)
> > +#define JPEG_ENC_INT_STATUS_MASK_ALLIRQ	0x13
> > +
> > +#define JPEG_ENC_DST_ADDR_OFFSET_MASK	GENMASK(3, 0)
> > +#define JPEG_ENC_QUALITY_MASK		GENMASK(31, 16)
> > +
> > +#define JPEG_ENC_CTRL_YUV_FORMAT_MASK	0x18
> > +#define JPEG_ENC_CTRL_RESTART_EN_BIT	BIT(10)
> > +#define JPEG_ENC_CTRL_FILE_FORMAT_BIT	BIT(5)
> > +#define JPEG_ENC_CTRL_INT_EN_BIT	BIT(2)
> > +#define JPEG_ENC_CTRL_ENABLE_BIT	BIT(0)
> > +#define JPEG_ENC_RESET_BIT		BIT(0)
> > +
> > +#define JPEG_ENC_YUV_FORMAT_YUYV	0
> > +#define JPEG_ENC_YUV_FORMAT_YVYU	1
> > +#define JPEG_ENC_YUV_FORMAT_NV12	2
> > +#define JEPG_ENC_YUV_FORMAT_NV21	3
> > +
> > +#define JPEG_ENC_QUALITY_Q60		0x0
> > +#define JPEG_ENC_QUALITY_Q80		0x1
> > +#define JPEG_ENC_QUALITY_Q90		0x2
> > +#define JPEG_ENC_QUALITY_Q95		0x3
> > +#define JPEG_ENC_QUALITY_Q39		0x4
> > +#define JPEG_ENC_QUALITY_Q68		0x5
> > +#define JPEG_ENC_QUALITY_Q84		0x6
> > +#define JPEG_ENC_QUALITY_Q92		0x7
> > +#define JPEG_ENC_QUALITY_Q48		0x8
> > +#define JPEG_ENC_QUALITY_Q74		0xa
> > +#define JPEG_ENC_QUALITY_Q87		0xb
> > +#define JPEG_ENC_QUALITY_Q34		0xc
> > +#define JPEG_ENC_QUALITY_Q64		0xe
> > +#define JPEG_ENC_QUALITY_Q82		0xf
> > +#define JPEG_ENC_QUALITY_Q97		0x10
> > +
> > +#define JPEG_ENC_RSTB			0x100
> > +#define JPEG_ENC_CTRL			0x104
> > +#define JPEG_ENC_QUALITY		0x108
> > +#define JPEG_ENC_BLK_NUM		0x10C
> > +#define JPEG_ENC_BLK_CNT		0x110
> > +#define JPEG_ENC_INT_STS		0x11c
> > +#define JPEG_ENC_DST_ADDR0		0x120
> > +#define JPEG_ENC_DMA_ADDR0		0x124
> > +#define JPEG_ENC_STALL_ADDR0		0x128
> > +#define JPEG_ENC_OFFSET_ADDR		0x138
> > +#define JPEG_ENC_RST_MCU_NUM		0x150
> > +#define JPEG_ENC_IMG_SIZE		0x154
> > +#define JPEG_ENC_DEBUG_INFO0		0x160
> > +#define JPEG_ENC_DEBUG_INFO1		0x164
> > +#define JPEG_ENC_TOTAL_CYCLE		0x168
> > +#define JPEG_ENC_BYTE_OFFSET_MASK	0x16c
> > +#define JPEG_ENC_SRC_LUMA_ADDR		0x170
> > +#define JPEG_ENC_SRC_CHROMA_ADDR	0x174
> > +#define JPEG_ENC_STRIDE			0x178
> > +#define JPEG_ENC_IMG_STRIDE		0x17c
> > +#define JPEG_ENC_DCM_CTRL		0x300
> > +#define JPEG_ENC_CODEC_SEL		0x314
> > +#define JPEG_ENC_ULTRA_THRES		0x318
> > +
> > +enum {
> > +	MTK_JPEG_ENC_RESULT_DONE,
> > +	MTK_JPEG_ENC_RESULT_STALL,
> > +	MTK_JPEG_ENC_RESULT_VCODEC_IRQ
> > +};
> > +
> > +/**
> > + * struct mtk_jpeg_enc_qlt - JPEG encoder quality data
> > + * @quality_param:	quality value
> > + * @hardware_value:	hardware value of quality
> > + */
> > +struct mtk_jpeg_enc_qlt {
> > +	u8	quality_param;
> > +	u8	hardware_value;
> > +};
> > +
> > +/**
> > + * struct mt_jpeg_enc_bs - JPEG encoder bitstream  buffer
> > + * @dma_addr:			JPEG encoder destination address
> > + * @size:			JPEG encoder bistream size
> > + * @dma_addr_offset:		JPEG encoder offset address
> > + * @dma_addr_offsetmask:	JPEG encoder destination address offset mask
> > + */
> > +struct mtk_jpeg_enc_bs {
> > +	dma_addr_t	dma_addr;
> > +	size_t		size;
> > +	u32		dma_addr_offset;
> > +	u32		dma_addr_offsetmask;
> > +};
> > +
> > +void mtk_jpeg_enc_reset(void __iomem *base);
> > +u32 mtk_jpeg_enc_get_and_clear_int_status(void __iomem *base);
> > +u32 mtk_jpeg_enc_get_file_size(void __iomem *base);
> > +u32 mtk_jpeg_enc_enum_result(void __iomem *base, u32 irq_status);
> > +void mtk_jpeg_enc_set_img_size(void __iomem *base, u32 width, u32 height);
> > +void mtk_jpeg_enc_set_blk_num(void __iomem *base, u32 enc_format, u32 width,
> > +			      u32 height);
> > +void mtk_jpeg_enc_set_stride(void __iomem *base, u32 enc_format, u32 width,
> > +			     u32 height, u32 bytesperline);
> > +void mtk_jpeg_enc_set_src_addr(void __iomem *base, u32 src_addr,
> > +			       u32 plane_index);
> > +void mtk_jpeg_enc_set_dst_addr(void __iomem *base, u32 dst_addr,
> > +			       u32 stall_size, u32 init_offset,
> > +			       u32 offset_mask);
> > +void mtk_jpeg_enc_set_config(void __iomem *base, u32 enc_format, bool exif_en,
> > +			     u32 quality, u32 restart_interval);
> > +void mtk_jpeg_enc_start(void __iomem *enc_reg_base);
> > +
> > +#endif /* _MTK_JPEG_ENC_HW_H */
> > 
> 


^ permalink raw reply

* Re: [PATCH v8 08/14] media: platform: Change case for improving code quality
From: Xia Jiang @ 2020-06-05  8:04 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger, Rick Chang,
	linux-media, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Marek Szyprowski, Tomasz Figa, srv_heupstream,
	senozhatsky, mojahsu, drinkcat, maoguang.meng, sj.huang
In-Reply-To: <4b8cc41e-5171-0d48-f588-96e4212ab22c@xs4all.nl>

On Mon, 2020-05-11 at 10:37 +0200, Hans Verkuil wrote:
> On 03/04/2020 11:40, Xia Jiang wrote:
> > Change register offset hex numberals from upercase to lowercase.
> 
> Typos:
> 
> numberals -> numerals
> 
> upercase -> uppercase
Done.
> 
> Regards,
> 
> 	Hans
> 
> > 
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8: no changes
> > ---
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h
> > index 94db04e9cdb6..2945da842dfa 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h
> > @@ -20,29 +20,29 @@
> >  #define BIT_INQST_MASK_ALLIRQ		0x37
> >  
> >  #define JPGDEC_REG_RESET		0x0090
> > -#define JPGDEC_REG_BRZ_FACTOR		0x00F8
> > -#define JPGDEC_REG_DU_NUM		0x00FC
> > +#define JPGDEC_REG_BRZ_FACTOR		0x00f8
> > +#define JPGDEC_REG_DU_NUM		0x00fc
> >  #define JPGDEC_REG_DEST_ADDR0_Y		0x0140
> >  #define JPGDEC_REG_DEST_ADDR0_U		0x0144
> >  #define JPGDEC_REG_DEST_ADDR0_V		0x0148
> > -#define JPGDEC_REG_DEST_ADDR1_Y		0x014C
> > +#define JPGDEC_REG_DEST_ADDR1_Y		0x014c
> >  #define JPGDEC_REG_DEST_ADDR1_U		0x0150
> >  #define JPGDEC_REG_DEST_ADDR1_V		0x0154
> >  #define JPGDEC_REG_STRIDE_Y		0x0158
> > -#define JPGDEC_REG_STRIDE_UV		0x015C
> > +#define JPGDEC_REG_STRIDE_UV		0x015c
> >  #define JPGDEC_REG_IMG_STRIDE_Y		0x0160
> >  #define JPGDEC_REG_IMG_STRIDE_UV	0x0164
> > -#define JPGDEC_REG_WDMA_CTRL		0x016C
> > +#define JPGDEC_REG_WDMA_CTRL		0x016c
> >  #define JPGDEC_REG_PAUSE_MCU_NUM	0x0170
> > -#define JPGDEC_REG_OPERATION_MODE	0x017C
> > +#define JPGDEC_REG_OPERATION_MODE	0x017c
> >  #define JPGDEC_REG_FILE_ADDR		0x0200
> > -#define JPGDEC_REG_COMP_ID		0x020C
> > +#define JPGDEC_REG_COMP_ID		0x020c
> >  #define JPGDEC_REG_TOTAL_MCU_NUM	0x0210
> >  #define JPGDEC_REG_COMP0_DATA_UNIT_NUM	0x0224
> > -#define JPGDEC_REG_DU_CTRL		0x023C
> > +#define JPGDEC_REG_DU_CTRL		0x023c
> >  #define JPGDEC_REG_TRIG			0x0240
> >  #define JPGDEC_REG_FILE_BRP		0x0248
> > -#define JPGDEC_REG_FILE_TOTAL_SIZE	0x024C
> > +#define JPGDEC_REG_FILE_TOTAL_SIZE	0x024c
> >  #define JPGDEC_REG_QT_ID		0x0270
> >  #define JPGDEC_REG_INTERRUPT_STATUS	0x0274
> >  #define JPGDEC_REG_STATUS		0x0278
> > 
> 


^ permalink raw reply

* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
From: Stanimir Varbanov @ 2020-06-05  7:19 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Nicolas Dufresne, Tomasz Figa, Michael Tretter
In-Reply-To: <20200526100932.2626420-2-hverkuil-cisco@xs4all.nl>



On 5/26/20 1:09 PM, Hans Verkuil wrote:
> From: Tomasz Figa <tfiga@chromium.org>
> 
> Due to complexity of the video encoding process, the V4L2 drivers of
> stateful encoder hardware require specific sequences of V4L2 API calls
> to be followed. These include capability enumeration, initialization,
> encoding, encode parameters change, drain and reset.
> 
> Specifics of the above have been discussed during Media Workshops at
> LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> originated at those events was later implemented by the drivers we already
> have merged in mainline, such as s5p-mfc or coda.
> 
> The only thing missing was the real specification included as a part of
> Linux Media documentation. Fix it now and document the encoder part of
> the Codec API.
> 
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  .../userspace-api/media/v4l/dev-encoder.rst   | 728 ++++++++++++++++++
>  .../userspace-api/media/v4l/dev-mem2mem.rst   |   1 +
>  .../userspace-api/media/v4l/pixfmt-v4l2.rst   |   5 +
>  .../userspace-api/media/v4l/v4l2.rst          |   2 +
>  .../media/v4l/vidioc-encoder-cmd.rst          |  51 +-
>  5 files changed, 767 insertions(+), 20 deletions(-)
>  create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> 
> diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> new file mode 100644
> index 000000000000..aace7b812a9c
> --- /dev/null
> +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> @@ -0,0 +1,728 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _encoder:
> +
> +*************************************************
> +Memory-to-Memory Stateful Video Encoder Interface
> +*************************************************
> +
> +A stateful video encoder takes raw video frames in display order and encodes
> +them into a bytestream. It generates complete chunks of the bytestream, including
> +all metadata, headers, etc. The resulting bytestream does not require any
> +further post-processing by the client.
> +
> +Performing software stream processing, header generation etc. in the driver
> +in order to support this interface is strongly discouraged. In case such
> +operations are needed, use of the Stateless Video Encoder Interface (in
> +development) is strongly advised.
> +

<cut>

> +Encoding Parameter Changes
> +==========================
> +
> +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> +parameters at any time. The availability of parameters is encoder-specific
> +and the client must query the encoder to find the set of available controls.
> +
> +The ability to change each parameter during encoding is encoder-specific, as
> +per the standard semantics of the V4L2 control interface. The client may
> +attempt to set a control during encoding and if the operation fails with the
> +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> +configuration change to be allowed. To do this, it may follow the `Drain`
> +sequence to avoid losing the already queued/encoded frames.
> +
> +The timing of parameter updates is encoder-specific, as per the standard
> +semantics of the V4L2 control interface. If the client needs to apply the
> +parameters exactly at specific frame, using the Request API
> +(:ref:`media-request-api`) should be considered, if supported by the encoder.

In request-api case does the control will return EBUSY immediately when
S_CTRL is called from the client? I'm asking in the context of the
controls which are not dynamic (cannot set during streaming and Reset
sequence is needed).

> +
> +Drain
> +=====
> +
> +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
> +related ``CAPTURE`` buffers are given to the client, the client must follow the
> +drain sequence described below. After the drain sequence ends, the client has
> +received all encoded frames for all ``OUTPUT`` buffers queued before the
> +sequence was started.
> +
> +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> +
> +   * **Required fields:**
> +
> +     ``cmd``
> +         set to ``V4L2_ENC_CMD_STOP``.
> +
> +     ``flags``
> +         set to 0.
> +
> +     ``pts``
> +         set to 0.
> +
> +   .. warning::
> +
> +      The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
> +      queues are streaming. For compatibility reasons, the call to
> +      :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
> +      not streaming, but at the same time it will not initiate the `Drain`
> +      sequence and so the steps described below would not be applicable.
> +
> +2. Any ``OUTPUT`` buffers queued by the client before the
> +   :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
> +   normal. The client must continue to handle both queues independently,
> +   similarly to normal encode operation. This includes:
> +
> +   * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
> +     ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
> +
> +     .. warning::
> +
> +        The last buffer may be empty (with :c:type:`v4l2_buffer`
> +        ``bytesused`` = 0) and in that case it must be ignored by the client,
> +        as it does not contain an encoded frame.
> +
> +     .. note::
> +
> +        Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
> +        marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> +        :c:func:`VIDIOC_DQBUF`.
> +
> +   * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
> +     before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
> +
> +   * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
> +
> +   .. note::
> +
> +      For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
> +      event when the last frame has been encoded and all frames are ready to be
> +      dequeued. It is deprecated behavior and the client must not rely on it.
> +      The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
> +
> +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
> +   dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
> +   and it will accept, but not process any newly queued ``OUTPUT`` buffers
> +   until the client issues any of the following operations:
> +
> +   * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
> +     operation normally, with all the state from before the drain,
> +
> +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> +     ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
> +     and then resume encoding,
> +
> +   * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> +     ``OUTPUT`` queue - the encoder will resume operation normally, however any
> +     source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
> +     and :c:func:`VIDIOC_STREAMOFF` will be discarded.
> +
> +.. note::
> +
> +   Once the drain sequence is initiated, the client needs to drive it to
> +   completion, as described by the steps above, unless it aborts the process by
> +   issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
> +   queues.  The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
> +   ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
> +   will fail with -EBUSY error code if attempted.
> +
> +   For reference, handling of various corner cases is described below:
> +
> +   * In case of no buffer in the ``OUTPUT`` queue at the time the
> +     ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
> +     immediately and the encoder returns an empty ``CAPTURE`` buffer with the
> +     ``V4L2_BUF_FLAG_LAST`` flag set.
> +
> +   * In case of no buffer in the ``CAPTURE`` queue at the time the drain
> +     sequence completes, the next time the client queues a ``CAPTURE`` buffer
> +     it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
> +     flag set.
> +
> +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
> +     middle of the drain sequence, the drain sequence is canceled and all
> +     ``CAPTURE`` buffers are implicitly returned to the client.
> +
> +   * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
> +     middle of the drain sequence, the drain sequence completes immediately and
> +     next ``CAPTURE`` buffer will be returned empty with the
> +     ``V4L2_BUF_FLAG_LAST`` flag set.
> +
> +   Although not mandatory, the availability of encoder commands may be queried
> +   using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
> +
> +Reset
> +=====
> +
> +The client may want to request the encoder to reinitialize the encoding, so
> +that the following stream data becomes independent from the stream data
> +generated before. Depending on the coded format, that may imply that:
> +
> +* encoded frames produced after the restart must not reference any frames
> +  produced before the stop, e.g. no long term references for H.264/HEVC,
> +
> +* any headers that must be included in a standalone stream must be produced
> +  again, e.g. SPS and PPS for H.264/HEVC.

It seems that clients needs to know SPS/PPS (for muxers?) and thus they
will need to extract or parse the encoder output buffers to find them.
Maybe the spec should consider adding some buffer flag to mark such
buffers which contain SPS/PPS only.

[1] - see at "type AvcCBox struct"

Nicolas, how the gstreamer handle this?

> +
> +This can be achieved by performing the reset sequence.
> +
> +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> +   and respective buffers are dequeued.
> +
> +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> +   will return all currently queued ``CAPTURE`` buffers to the client, without
> +   valid frame data.
> +
> +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> +   continue with regular encoding sequence. The encoded frames produced into
> +   ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> +   decoded without the need for frames encoded before the reset sequence,
> +   starting at the first ``OUTPUT`` buffer queued after issuing the
> +   `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> +
> +This sequence may be also used to change encoding parameters for encoders
> +without the ability to change the parameters on the fly.
> +

<cut>

-- 
regards,
Stan

[1]
https://pkg.go.dev/github.com/Afrostream/afrostream-media-server/src/mp4?tab=doc

^ permalink raw reply

* Re: [RFC] METADATA design using V4l2 Request API
From: dikshita @ 2020-06-05  7:02 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Nicolas Dufresne, linux-media, stanimir.varbanov, linux-kernel,
	linux-arm-msm, vgarodia, majja, jdas, linux-media-owner
In-Reply-To: <ad61378b-e279-d161-adaa-17349adf183e@xs4all.nl>

Hi Hans, Nicolas,

On 2020-05-29 13:01, Hans Verkuil wrote:
> On 29/05/2020 04:18, Nicolas Dufresne wrote:
>> Le jeudi 28 mai 2020 à 16:18 +0530, dikshita@codeaurora.org a écrit :
>>>> not allowed. So I need to know more about this.
>>>> Regards,
>>>>        Hans
>>> 
>>> we need this for use cases like HDR10+ where metadata info is part of
>>> the bitstream.
>>> 
>>> To handle such frame specific data, support for request api on 
>>> capture
>>> plane would be needed.
>> 
>> I have a bit of a mixed impression here. Considering how large the 
>> ioctl
>> interface overhead is, relying on HW parser to extract this medata 
>> woud be the
>> last thing I would consider.
>> 
>> Instead, I'm quite convince we can achieve greater and likely 
>> zero-copy
>> perfromance by locating this header in userspace. So everytime I see 
>> this kind
>> of API, were the HW is *needed* to parse a trivial bit of bistream, I 
>> ended
>> thinking that we simply craft this API to expose this because the HW 
>> can do it,
>> but no further logical thinking or higher level design seems to have 
>> been
>> applied.
>> 
>> I'm sorry for this open critic, but are we designing this because the 
>> HW
>> designer exposed that feature? This is so low complexity to extract in
>> userspace, with the bonus that we are not forced into expanding the
>> representation to another form immediatly, as maybe the display will 
>> be able to
>> handle that form directly (where converting to a C structure and then 
>> back to
>> some binary format to satisfy DRM property seems very sub-optimal).
>> 
>> Nicolas
>> 
> 
> Nicolas raises good questions: it would help if you can give more
> detailed information
> about the hardware. We had similar discussions with Xilinx about
> HDR10+ (see this
> thread: https://www.spinics.net/lists/linux-media/msg163297.html).
> 
> There is no clear answer at the moment on how to handle dynamic HDR 
> metadata.
> It will help if we have some more information how different SoCs handle 
> this
> in hardware.
> 
> Regards,
> 
> 	Hans

As per Widevine Level 1 requirement, it needs “Hardware Protected Video 
Path”.
Hence, in case of secure bitstream decoding, we need decoder metadata 
delivered from HW.
CPU cannot parse secure bitstream and extract them.
Apart from this, there are other metadata like "histogram" which is not 
part of the bitstream
and generated by hardware

Thanks,
Dikshita

^ permalink raw reply

* Re: [PATCH v8 10/14] media: platform: Delete redundant code for improving code quality
From: Xia Jiang @ 2020-06-05  6:41 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Rick Chang, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
	srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
	sj.huang
In-Reply-To: <20200521154958.GI209565@chromium.org>

On Thu, 2020-05-21 at 15:49 +0000, Tomasz Figa wrote:
> Hi Xia,
> 
> On Fri, Apr 03, 2020 at 05:40:29PM +0800, Xia Jiang wrote:
> > Delete unused member variables annotation.
> > Delete unused variable definition.
> > Delete redundant log print, because V4L2 debug logs already print it.
> > 
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8: no changes
> > ---
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 16 ++--------------
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h |  5 +++--
> >  2 files changed, 5 insertions(+), 16 deletions(-)
> > 
> 
> Thank you for the patch. Please see my comments inline.
> 
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 4e64046a6854..9e59b9a51ef0 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -182,7 +182,6 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> >  				   struct mtk_jpeg_ctx *ctx, int q_type)
> >  {
> >  	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > -	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> >  	int i;
> >  
> >  	memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
> > @@ -190,7 +189,7 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> >  
> >  	if (ctx->state != MTK_JPEG_INIT) {
> >  		mtk_jpeg_adjust_fmt_mplane(ctx, f);
> > -		goto end;
> > +		return 0;
> >  	}
> >  
> >  	pix_mp->num_planes = fmt->colplanes;
> > @@ -210,7 +209,7 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> >  		pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> >  		if (pfmt->sizeimage == 0)
> >  			pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > -		goto end;
> > +		return 0;
> >  	}
> >  
> >  	/* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > @@ -224,20 +223,9 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> >  		u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> >  		u32 h = pix_mp->height * fmt->v_sample[i] / 4;
> >  
> > -		memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> 
> This change is not mentioned in the description. I'd suggest moving it
> to a separate patch, because it's a functional change.
> 
> >  		pfmt->bytesperline = stride;
> >  		pfmt->sizeimage = stride * h;
> >  	}
> > -end:
> > -	v4l2_dbg(2, debug, &jpeg->v4l2_dev, "wxh:%ux%u\n",
> > -		 pix_mp->width, pix_mp->height);
> > -	for (i = 0; i < pix_mp->num_planes; i++) {
> > -		v4l2_dbg(2, debug, &jpeg->v4l2_dev,
> > -			 "plane[%d] bpl=%u, size=%u\n",
> > -			 i,
> > -			 pix_mp->plane_fmt[i].bytesperline,
> > -			 pix_mp->plane_fmt[i].sizeimage);
> > -	}
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 64a731261214..9bbd615b1067 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -30,6 +30,9 @@
> >  
> >  #define MTK_JPEG_DEFAULT_SIZEIMAGE	(1 * 1024 * 1024)
> >  
> > +/**
> > + * enum mtk_jpeg_ctx_state - contex state of jpeg
> 
> typo: s/contex/context/
> 
> But I'd rephrase it to "states of the context state machine".
> 
> > + */
> 
> Not mentioned in the description. Also, the documentation of an enum
> should have descriptions for the values.
Done.
> 
> Best regards,
> Tomasz


^ permalink raw reply

* Re: [PATCH v8 07/14] media: platform: Use kernel native functions for improving code quality
From: Xia Jiang @ 2020-06-05  6:41 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Rick Chang, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
	srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
	sj.huang
In-Reply-To: <20200521154137.GG209565@chromium.org>

On Thu, 2020-05-21 at 15:41 +0000, Tomasz Figa wrote:
> Hi Xia,
> 
> On Fri, Apr 03, 2020 at 05:40:26PM +0800, Xia Jiang wrote:
> 
> Thank you for the patch. Please see my comments inline.
> 
> nit: I'd remove "for improving code quality" from the subject, as it's
> obvious that we don't intend to make the code quality worse. ;)
> On the contrary, I'd make it more specific, e.g.
> 
> media: mtk-jpeg: Use generic rounding helpers
> 
> WDYT?
Done
> 
> > Use clamp() to replace mtk_jpeg_bound_align_image() and round() to
> > replace mtk_jpeg_align().
> > 
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8: no changes
> > ---
> >  .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 41 +++++--------------
> >  .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |  8 ++--
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c |  8 ++--
> >  drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h |  5 ---
> >  4 files changed, 19 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 2fa3711fdc9b..4e64046a6854 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -157,25 +157,6 @@ static struct mtk_jpeg_fmt *mtk_jpeg_find_format(struct mtk_jpeg_ctx *ctx,
> >  	return NULL;
> >  }
> >  
> > -static void mtk_jpeg_bound_align_image(u32 *w, unsigned int wmin,
> > -				       unsigned int wmax, unsigned int walign,
> > -				       u32 *h, unsigned int hmin,
> > -				       unsigned int hmax, unsigned int halign)
> > -{
> > -	int width, height, w_step, h_step;
> > -
> > -	width = *w;
> > -	height = *h;
> > -	w_step = 1 << walign;
> > -	h_step = 1 << halign;
> > -
> > -	v4l_bound_align_image(w, wmin, wmax, walign, h, hmin, hmax, halign, 0);
> > -	if (*w < width && (*w + w_step) <= wmax)
> > -		*w += w_step;
> > -	if (*h < height && (*h + h_step) <= hmax)
> > -		*h += h_step;
> > -}
> > -
> >  static void mtk_jpeg_adjust_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> >  				       struct v4l2_format *f)
> >  {
> > @@ -218,25 +199,25 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> >  	if (q_type == MTK_JPEG_FMT_TYPE_OUTPUT) {
> >  		struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[0];
> >  
> > -		mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > -					   MTK_JPEG_MAX_WIDTH, 0,
> > -					   &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > -					   MTK_JPEG_MAX_HEIGHT, 0);
> > +		pix_mp->height = clamp(pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > +				       MTK_JPEG_MAX_HEIGHT);
> > +		pix_mp->width = clamp(pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > +				      MTK_JPEG_MAX_WIDTH);
> >  
> >  		memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> >  		pfmt->bytesperline = 0;
> >  		/* Source size must be aligned to 128 */
> > -		pfmt->sizeimage = mtk_jpeg_align(pfmt->sizeimage, 128);
> > +		pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> >  		if (pfmt->sizeimage == 0)
> >  			pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> >  		goto end;
> >  	}
> >  
> >  	/* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > -	mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > -				   MTK_JPEG_MAX_WIDTH, fmt->h_align,
> > -				   &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > -				   MTK_JPEG_MAX_HEIGHT, fmt->v_align);
> > +	pix_mp->height = clamp(round_up(pix_mp->height, fmt->v_align),
> > +			       MTK_JPEG_MIN_HEIGHT, MTK_JPEG_MAX_HEIGHT);
> > +	pix_mp->width = clamp(round_up(pix_mp->width, fmt->h_align),
> > +			      MTK_JPEG_MIN_WIDTH, MTK_JPEG_MAX_WIDTH);
> >  
> >  	for (i = 0; i < fmt->colplanes; i++) {
> >  		struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[i];
> > @@ -751,8 +732,8 @@ static void mtk_jpeg_set_dec_src(struct mtk_jpeg_ctx *ctx,
> >  {
> >  	bs->str_addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
> >  	bs->end_addr = bs->str_addr +
> > -			 mtk_jpeg_align(vb2_get_plane_payload(src_buf, 0), 16);
> > -	bs->size = mtk_jpeg_align(vb2_plane_size(src_buf, 0), 128);
> > +		       round_up(vb2_get_plane_payload(src_buf, 0), 16);
> > +	bs->size = round_up(vb2_plane_size(src_buf, 0), 128);
> >  }
> >  
> >  static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 999bd1427809..28e9b30ad5c3 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -21,10 +21,10 @@
> >  #define MTK_JPEG_FMT_TYPE_OUTPUT	1
> >  #define MTK_JPEG_FMT_TYPE_CAPTURE	2
> >  
> > -#define MTK_JPEG_MIN_WIDTH	32
> > -#define MTK_JPEG_MIN_HEIGHT	32
> > -#define MTK_JPEG_MAX_WIDTH	8192
> > -#define MTK_JPEG_MAX_HEIGHT	8192
> > +#define MTK_JPEG_MIN_WIDTH	32U
> > +#define MTK_JPEG_MIN_HEIGHT	32U
> > +#define MTK_JPEG_MAX_WIDTH	8192U
> > +#define MTK_JPEG_MAX_HEIGHT	8192U
> 
> This change is not mentioned in the commit message. It should go to a
> separate patch, possibly merged with other really minor stylistic changes
> like this, e.g. patch 08/14.
Done
> 
> Otherwise the patch looks good, so after addressing the above minor changes
> please feel free to add
> 
> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
> 
> Best regards,
> Tomasz
> 


^ permalink raw reply

* Re: [PATCH v8 05/14] media: platform: Improve power on and power off flow
From: Xia Jiang @ 2020-06-05  6:03 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Rick Chang, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
	srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
	sj.huang
In-Reply-To: <20200521152253.GE209565@chromium.org>

On Thu, 2020-05-21 at 15:22 +0000, Tomasz Figa wrote:
> Hi Xia,
> 
> On Fri, Apr 03, 2020 at 05:40:24PM +0800, Xia Jiang wrote:
> > Call pm_runtime_get_sync() before starting a frame and then
> > pm_runtime_put() after completing it. This can save power for the time
> > between processing two frames.
> > 
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> >  .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 27 +++++--------------
> >  1 file changed, 6 insertions(+), 21 deletions(-)
> > 
> 
> Thank you for the patch. Please see my comments inline.
> 
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index a536fa95b3d6..dd5cadd101ef 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -710,23 +710,6 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> >  		return v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> >  }
> >  
> > -static int mtk_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
> > -{
> > -	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > -	struct vb2_v4l2_buffer *vb;
> > -	int ret = 0;
> > -
> > -	ret = pm_runtime_get_sync(ctx->jpeg->dev);
> > -	if (ret < 0)
> > -		goto err;
> > -
> > -	return 0;
> > -err:
> > -	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> > -		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_QUEUED);
> > -	return ret;
> > -}
> > -
> >  static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> >  {
> >  	struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > @@ -751,8 +734,6 @@ static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> >  
> >  	while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> >  		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > -
> > -	pm_runtime_put_sync(ctx->jpeg->dev);
> >  }
> >  
> >  static const struct vb2_ops mtk_jpeg_qops = {
> > @@ -761,7 +742,6 @@ static const struct vb2_ops mtk_jpeg_qops = {
> >  	.buf_queue          = mtk_jpeg_buf_queue,
> >  	.wait_prepare       = vb2_ops_wait_prepare,
> >  	.wait_finish        = vb2_ops_wait_finish,
> > -	.start_streaming    = mtk_jpeg_start_streaming,
> >  	.stop_streaming     = mtk_jpeg_stop_streaming,
> >  };
> >  
> > @@ -812,7 +792,7 @@ static void mtk_jpeg_device_run(void *priv)
> >  	struct mtk_jpeg_src_buf *jpeg_src_buf;
> >  	struct mtk_jpeg_bs bs;
> >  	struct mtk_jpeg_fb fb;
> > -	int i;
> > +	int i, ret;
> >  
> >  	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> >  	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > @@ -832,6 +812,10 @@ static void mtk_jpeg_device_run(void *priv)
> >  		return;
> >  	}
> >  
> > +	ret = pm_runtime_get_sync(jpeg->dev);
> > +	if (ret < 0)
> > +		goto dec_end;
> > +
> >  	mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
> >  	if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
> >  		goto dec_end;
> > @@ -957,6 +941,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> >  	v4l2_m2m_buf_done(src_buf, buf_state);
> >  	v4l2_m2m_buf_done(dst_buf, buf_state);
> >  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +	pm_runtime_put_sync(ctx->jpeg->dev);
> 
> The _sync variant explicitly waits until the asynchronous PM operation
> completes. This is usually undesired, because the CPU stays blocked for
> no good reason. In this context it is actually a bug, because this is an
> interrupt handler and it's not allowed to sleep. I wonder why this
> actually didn't crash in your testing. Please change to the regular
> pm_runtime_put().
Done.
> 
> Best regards,
> Tomasz


^ permalink raw reply

* Re: [PATCH v8 04/14] media: platform: Change the fixed device node number to unfixed value
From: Xia Jiang @ 2020-06-05  6:02 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Rick Chang, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
	srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
	sj.huang
In-Reply-To: <20200521135937.GD209565@chromium.org>

On Thu, 2020-05-21 at 13:59 +0000, Tomasz Figa wrote:
> Hi Xia,
> 
> On Fri, Apr 03, 2020 at 05:40:23PM +0800, Xia Jiang wrote:
> > Change device node number from 3 to -1 because that the driver will
> > also support jpeg encoder.
> > 
> 
> Thanks for the patch. The change is correct, but I think the commit
> message doesn't really explain the real reason for it. Perhaps something
> like
> 
> "The driver can be instantiated multiple times, e.g. for a decoder and
> an encoder. Moreover, other drivers could coexist on the same system.
> This makes the static video node number assignment pointless, so switch
> to automatic assignment instead."
> 
> WDYT?
Dear Tomasz,
Thanks for your advice.I have changed it in new v9 .

Best Regards,
Xia Jiang
> 
> Best regards,
> Tomasz


^ permalink raw reply

* Re: [PATCH] media: staging: tegra-vde: add missing pm_runtime_put_autosuspend
From: Jon Hunter @ 2020-06-05  6:00 UTC (permalink / raw)
  To: Navid Emamdoost, Dmitry Osipenko, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Thierry Reding, linux-media, linux-tegra,
	devel, linux-kernel
  Cc: emamd001, wu000273, kjlu, smccaman
In-Reply-To: <20200602054841.15746-1-navid.emamdoost@gmail.com>


On 02/06/2020 06:48, Navid Emamdoost wrote:
> Call to pm_runtime_get_sync increments counter even in case of
> failure leading to incorrect ref count.
> Call pm_runtime_put_autosuspend if pm_runtime_get_sync fails.
> 
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
>  drivers/staging/media/tegra-vde/vde.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
> index d3e63512a765..52cdd4a91e93 100644
> --- a/drivers/staging/media/tegra-vde/vde.c
> +++ b/drivers/staging/media/tegra-vde/vde.c
> @@ -776,8 +776,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  		goto release_dpb_frames;
>  
>  	ret = pm_runtime_get_sync(dev);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		pm_runtime_put_autosuspend(dev);
>  		goto unlock;
> +	}
>  
>  	/*
>  	 * We rely on the VDE registers reset value, otherwise VDE

Please use the put in the error path.

Jon

-- 
nvpublic

^ permalink raw reply

* Re: possible deadlock in media_devnode_release
From: Eric Biggers @ 2020-06-05  4:20 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: syzbot, brendanhiggins, gregkh, heikki.krogerus, laurent.pinchart,
	linux-kernel, linux-media, mchehab, rafael.j.wysocki,
	sakari.ailus, syzkaller-bugs
In-Reply-To: <08f732a9-986f-6dcf-87dd-075a9b4605d7@infradead.org>

On Sat, May 23, 2020 at 10:38:50AM -0700, Randy Dunlap wrote:
> On 5/23/20 10:04 AM, syzbot wrote:
> > Hello,
> > 
> > syzbot found the following crash on:
> > 
> > HEAD commit:    c11d28ab Add linux-next specific files for 20200522
> > git tree:       linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=17330172100000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=3f6dbdea4159fb66
> > dashboard link: https://syzkaller.appspot.com/bug?extid=e3c234427cd464510547
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=122eacba100000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12fffa9a100000
> > 
> > The bug was bisected to:
> > 
> > commit 4ef12f7198023c09ad6d25b652bd8748c965c7fa
> > Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Date:   Wed May 13 15:18:40 2020 +0000
> > 
> >     kobject: Make sure the parent does not get released before its children
> 
> Hi,
> 
> Greg just sent a revert for this patch:
> https://lore.kernel.org/lkml/20200523152922.GA224858@kroah.com/
> 
> so all 3 of these reports should be cleared up soon.

Commit was reverted, so invalidating this bug report:

#syz invalid

^ permalink raw reply

* cron job: media_tree daily build: WARNINGS
From: Hans Verkuil @ 2020-06-05  3:41 UTC (permalink / raw)
  To: linux-media

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:			Fri Jun  5 05:00:11 CEST 2020
media-tree git hash:	938b29db3aa9c293c7c1366b16e55e308f1a1ddd
media_build git hash:	337283131d6117aa9b0c0c62d32e323da54a9359
v4l-utils git hash:	74377da4f5f3b63203c599d5dd75db6af91fdbb9
edid-decode git hash:	f20c85d7b4c537e0d458f85c4da9f45cd3c0fbd2
gcc version:		i686-linux-gcc (GCC) 9.3.0
sparse repo:            https://git.linuxtv.org/mchehab/sparse.git
sparse version:		0.6.1
smatch repo:            https://git.linuxtv.org/mchehab/smatch.git
smatch version:		0.6.1-rc1
build-scripts repo:     https://git.linuxtv.org/hverkuil/build-scripts.git
build-scripts git hash: a0e63d2e6cdc689a8af8c9ae6df1674d0fe38c74
host hardware:		x86_64
host os:		5.6.0-1-amd64

linux-git-sh: OK
linux-git-arm-davinci: OK
linux-git-arm-at91: OK
linux-git-arm-stm32: OK
linux-git-arm-pxa: OK
linux-git-mips: OK
linux-git-arm64: OK
linux-git-powerpc64: OK
linux-git-arm-multi: OK
linux-git-i686: WARNINGS
linux-git-x86_64: WARNINGS
Check COMPILE_TEST: OK
Check for strcpy/strncpy/strlcpy: WARNINGS: found 4 strcpy(), 4 strncpy(), 4 strlcpy()
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.81-i686: OK
linux-3.16.81-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.136-i686: OK
linux-3.18.136-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.0.9-i686: OK
linux-4.0.9-x86_64: OK
linux-4.1.52-i686: OK
linux-4.1.52-x86_64: OK
linux-4.2.8-i686: OK
linux-4.2.8-x86_64: OK
linux-4.3.6-i686: OK
linux-4.3.6-x86_64: OK
linux-4.4.212-i686: OK
linux-4.4.212-x86_64: OK
linux-4.5.7-i686: OK
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: OK
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.212-i686: OK
linux-4.9.212-x86_64: OK
linux-4.10.17-i686: OK
linux-4.10.17-x86_64: OK
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: OK
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.169-i686: OK
linux-4.14.169-x86_64: OK
linux-4.15.18-i686: OK
linux-4.15.18-x86_64: OK
linux-4.16.18-i686: OK
linux-4.16.18-x86_64: OK
linux-4.17.19-i686: OK
linux-4.17.19-x86_64: OK
linux-4.18.20-i686: OK
linux-4.18.20-x86_64: OK
linux-4.19.101-i686: OK
linux-4.19.101-x86_64: OK
linux-4.20.15-i686: OK
linux-4.20.15-x86_64: OK
linux-5.0.15-i686: OK
linux-5.0.15-x86_64: OK
linux-5.1.1-i686: OK
linux-5.1.1-x86_64: OK
linux-5.2.1-i686: OK
linux-5.2.1-x86_64: OK
linux-5.3.1-i686: OK
linux-5.3.1-x86_64: OK
linux-5.4.17-i686: OK
linux-5.4.17-x86_64: OK
linux-5.5.1-i686: OK
linux-5.5.1-x86_64: OK
linux-5.6.1-i686: OK
linux-5.6.1-x86_64: OK
linux-5.7-rc1-i686: OK
linux-5.7-rc1-x86_64: OK
apps: OK
spec-git: OK
virtme: WARNINGS: Final Summary: 2943, Succeeded: 2943, Failed: 0, Warnings: 4
virtme-32: OK: Final Summary: 2779, Succeeded: 2779, Failed: 0, Warnings: 0
sparse: OK
smatch: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.log

Detailed regression test results are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday-test-media.log
http://www.xs4all.nl/~hverkuil/logs/Friday-test-media-32.log
http://www.xs4all.nl/~hverkuil/logs/Friday-test-media-dmesg.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

^ permalink raw reply

* Re: [V6, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-05  3:28 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Tomasz Figa, Linus Walleij, Bartosz Golaszewski,
	Mauro Carvalho Chehab, Andy Shevchenko, Rob Herring, Mark Rutland,
	Nicolas Boichat, Matthias Brugger, Cao Bing Bu, srv_heupstream,
	moderated list:ARM/Mediatek SoC support,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg  Roedel <joro@8bytes.org>,,
	Sj Huang, Linux Media Mailing List, linux-devicetree, Louis Kuo,
	Shengnan Wang (王圣男), dongchun.zhu
In-Reply-To: <20200604081032.GG16711@paasikivi.fi.intel.com>

Hi Sakari,

On Thu, 2020-06-04 at 11:10 +0300, Sakari Ailus wrote:
> On Thu, Jun 04, 2020 at 10:33:38AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz,
> > 
> > On Mon, 2020-06-01 at 20:47 +0200, Tomasz Figa wrote:
> > > On Wed, May 27, 2020 at 11:03 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Mon, 2020-05-25 at 13:45 +0200, Tomasz Figa wrote:
> > > > > On Fri, May 22, 2020 at 11:27 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > > > >
> > > > > > Hi Tomasz,
> > > > > >
> > > > > > Thanks for the review. My replies are as below.
> > > > > >
> > > > > > On Thu, 2020-05-21 at 19:51 +0000, Tomasz Figa wrote:
> > > > > > > Hi Dongchun, Sakari,
> > > > > > >
> > > > > > > On Mon, May 18, 2020 at 09:27:31PM +0800, Dongchun Zhu wrote:
> > > > > [snip]
> > > > > > > > +   pm_runtime_enable(dev);
> > > > > > > > +   if (!pm_runtime_enabled(dev)) {
> > > > > > > > +           ret = dw9768_runtime_resume(dev);
> > > > > > > > +           if (ret < 0) {
> > > > > > > > +                   dev_err(dev, "failed to power on: %d\n", ret);
> > > > > > > > +                   goto entity_cleanup;
> > > > > > > > +           }
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > > > > > +   if (ret < 0)
> > > > > > > > +           goto entity_cleanup;
> > > > > > > > +
> > > > > > > > +   return 0;
> > > > > > > > +
> > > > > > > > +entity_cleanup:
> > > > > > >
> > > > > > > Need to power off if the code above powered on.
> > > > > > >
> > > > > >
> > > > > > Thanks for the reminder.
> > > > > > If there is something wrong with runtime PM, actuator is to be powered
> > > > > > on via dw9768_runtime_resume() API.
> > > > > > When actuator sub-device is powered on completely and async registered
> > > > > > successfully, we shall power off it afterwards.
> > > > > >
> > > > >
> > > > > The code above calls dw9768_runtime_resume() if
> > > > > !pm_runtime_enabled(dev), but the clean-up code below the
> > > > > entity_cleanup label doesn't have the corresponding
> > > > > dw9768_runtime_suspend() call.
> > > > >
> > > >
> > > > Did you mean the 'entity_cleanup' after v4l2_async_register_subdev()?
> > > 
> > > Yes.
> > > 
> > > > Actually I made some changes for OV02A V9, according to this comment.
> > > > Could you help review that change? Thanks.
> > > 
> > > Sure, I will take a look.
> > > 
> > 
> > Thanks.
> > Sorry, I just wanna make sure the change is okay for next release.
> > May we use the check like OV02A V9 did?
> > ret = v4l2_async_register_subdev(&dw9768->sd);
> > if (ret < 0) {
> > 	if (!pm_runtime_enabled(dev))
> > 		dw9768_runtime_suspend(dev);
> 
> Please do this part where you're jumping to, if possible.
> 

Fine.
Fixed in next release.

> > 	goto entity_cleanup;
> > }
> > 
> 


^ permalink raw reply

* Re: [V9, 2/2] media: i2c: ov02a10: Add OV02A10 image sensor driver
From: Dongchun Zhu @ 2020-06-05  3:19 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
	mark.rutland, drinkcat, tfiga, matthias.bgg, bingbu.cao,
	srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang
In-Reply-To: <20200604092616.GJ16711@paasikivi.fi.intel.com>

Hi Sakari,

On Thu, 2020-06-04 at 12:26 +0300, Sakari Ailus wrote:
> Hi Dongchun,
> 
> On Thu, Jun 04, 2020 at 10:14:05AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz, Sakari, and sirs,
> > 
> > Could anyone help to review this patch?
> > 
> > On Sat, 2020-05-23 at 16:41 +0800, Dongchun Zhu wrote:
> > > Add a V4L2 sub-device driver for OV02A10 image sensor.
> > > 
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > >  MAINTAINERS                 |    1 +
> > >  drivers/media/i2c/Kconfig   |   13 +
> > >  drivers/media/i2c/Makefile  |    1 +
> > >  drivers/media/i2c/ov02a10.c | 1025 +++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 1040 insertions(+)
> > >  create mode 100644 drivers/media/i2c/ov02a10.c
> > > 
> > 
> > [snip]
> > 
> > > +static int ov02a10_probe(struct i2c_client *client)
> > > +{
> > > +	struct device *dev = &client->dev;
> > > +	struct ov02a10 *ov02a10;
> > > +	unsigned int rotation;
> > > +	unsigned int clock_lane_tx_speed;
> > > +	unsigned int i;
> > > +	int ret;
> > > +
> > > +	ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > > +	if (!ov02a10)
> > > +		return -ENOMEM;
> > > +
> > > +	ret = ov02a10_check_hwcfg(dev, ov02a10);
> > > +	if (ret) {
> > > +		dev_err(dev, "failed to check HW configuration: %d", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > > +	ov02a10->mipi_clock_tx_speed = OV02A10_MIPI_TX_SPEED_DEFAULT;
> > > +	ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > > +
> > > +	/* Optional indication of physical rotation of sensor */
> > > +	ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation", &rotation);
> > > +	if (!ret && rotation == 180) {
> > > +		ov02a10->upside_down = true;
> > > +		ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > > +	}
> > > +
> > > +	/* Optional indication of mipi TX speed */
> > > +	ret = fwnode_property_read_u32(dev_fwnode(dev), "ovti,mipi-tx-speed",
> > > +				       &clock_lane_tx_speed);
> > > +
> > > +	if (!ret)
> > > +		ov02a10->mipi_clock_tx_speed = clock_lane_tx_speed;
> > > +
> > > +	/* Get system clock (eclk) */
> > > +	ov02a10->eclk = devm_clk_get(dev, "eclk");
> > > +	if (IS_ERR(ov02a10->eclk)) {
> > > +		ret = PTR_ERR(ov02a10->eclk);
> > > +		dev_err(dev, "failed to get eclk %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> > > +				       &ov02a10->eclk_freq);
> > > +	if (ret) {
> > > +		dev_err(dev, "failed to get eclk frequency\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = clk_set_rate(ov02a10->eclk, ov02a10->eclk_freq);
> > > +	if (ret) {
> > > +		dev_err(dev, "failed to set eclk frequency (24MHz)\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	if (clk_get_rate(ov02a10->eclk) != OV02A10_ECLK_FREQ) {
> > > +		dev_warn(dev, "wrong eclk frequency %d Hz, expected: %d Hz\n",
> > > +			 ov02a10->eclk_freq, OV02A10_ECLK_FREQ);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	ov02a10->pd_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
> > > +	if (IS_ERR(ov02a10->pd_gpio)) {
> > > +		ret = PTR_ERR(ov02a10->pd_gpio);
> > > +		dev_err(dev, "failed to get powerdown-gpios %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	ov02a10->n_rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > +	if (IS_ERR(ov02a10->n_rst_gpio)) {
> > > +		ret = PTR_ERR(ov02a10->n_rst_gpio);
> > > +		dev_err(dev, "failed to get reset-gpios %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(ov02a10_supply_names); i++)
> > > +		ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > > +
> > > +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov02a10_supply_names),
> > > +				      ov02a10->supplies);
> > > +	if (ret) {
> > > +		dev_err(dev, "failed to get regulators\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	mutex_init(&ov02a10->mutex);
> > > +	ov02a10->cur_mode = &supported_modes[0];
> > > +	ret = ov02a10_initialize_controls(ov02a10);
> > > +	if (ret) {
> > > +		dev_err(dev, "failed to initialize controls\n");
> > > +		goto err_destroy_mutex;
> > > +	}
> > > +
> > > +	ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > +	ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > > +	ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > +	ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > +	ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > > +	if (ret < 0) {
> > > +		dev_err(dev, "failed to init entity pads: %d", ret);
> > > +		goto err_free_handler;
> > > +	}
> > > +
> > > +	pm_runtime_enable(dev);
> > > +	if (!pm_runtime_enabled(dev)) {
> > > +		ret = ov02a10_power_on(dev);
> > > +		if (ret < 0) {
> > > +			dev_err(dev, "failed to power on: %d\n", ret);
> > > +			goto err_free_handler;

This is actually wrong, which should be replaced of "err_clean_entity".

> > > +		}
> > > +	}
> > > +
> > > +	ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > > +	if (ret) {
> > > +		dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > > +		if (!pm_runtime_enabled(dev))
> > > +			ov02a10_power_off(dev);
> 
> This should be moved to error handling section below.
> 

Fine.
It would be abstracted as "err_async_register" in next release.
Something like:
err_async_register:
	if (!pm_runtime_enabled(dev))
		ov02a10_power_off(dev);
err_clean_entity:
	media_entity_cleanup(&ov02a10->subdev.entity);
...

> > > +		goto err_clean_entity;
> > > +	}
> > 
> > Tomasz, Sakari, is this ok?
> > or coding like this:
> > 
> > ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > if (!pm_runtime_enabled(dev))
> > 	ov02a10_power_off(dev);
> > if (ret) {
> > 	dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > 	goto err_clean_entity;
> > }
> > 
> > What's your opinions about the change?
> 
> This turns power off if runtime PM is disabled. I'd keep it as-is, as it'd
> require re-implementing what runtime PM is used for now --- and that's not
> a sensor driver's job.
> 
> > 
> > > +
> > > +	return 0;
> > > +
> > > +err_clean_entity:
> > > +	media_entity_cleanup(&ov02a10->subdev.entity);
> > > +err_free_handler:
> > > +	v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > > +err_destroy_mutex:
> > > +	mutex_destroy(&ov02a10->mutex);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int ov02a10_remove(struct i2c_client *client)
> > > +{
> > > +	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > +	struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > > +
> > > +	v4l2_async_unregister_subdev(sd);
> > > +	media_entity_cleanup(&sd->entity);
> > > +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> > > +	pm_runtime_disable(&client->dev);
> > > +	if (!pm_runtime_status_suspended(&client->dev))
> > > +		ov02a10_power_off(&client->dev);
> > > +	pm_runtime_set_suspended(&client->dev);
> > > +	mutex_destroy(&ov02a10->mutex);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct of_device_id ov02a10_of_match[] = {
> > > +	{ .compatible = "ovti,ov02a10" },
> > > +	{}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> > > +
> > > +static struct i2c_driver ov02a10_i2c_driver = {
> > > +	.driver = {
> > > +		.name = "ov02a10",
> > > +		.pm = &ov02a10_pm_ops,
> > > +		.of_match_table = ov02a10_of_match,
> > > +	},
> > > +	.probe_new	= &ov02a10_probe,
> > > +	.remove		= &ov02a10_remove,
> > > +};
> > > +
> > > +module_i2c_driver(ov02a10_i2c_driver);
> > > +
> > > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > > +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +
> > 
> 


^ permalink raw reply

* Re: [PATCH v4 1/3] virtio: add dma-buf support for exported objects
From: David Stevens @ 2020-06-05  1:28 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Gerd Hoffmann, David Airlie, Daniel Vetter, Sumit Semwal,
	Jason Wang, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	open list, ML dri-devel, open list:VIRTIO GPU DRIVER,
	Linux Media Mailing List,
	moderated list:DMA BUFFER SHARING FRAMEWORK, virtio-dev
In-Reply-To: <20200604145620-mutt-send-email-mst@kernel.org>

On Fri, Jun 5, 2020 at 4:05 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, May 26, 2020 at 07:58:09PM +0900, David Stevens wrote:
> > This change adds a new flavor of dma-bufs that can be used by virtio
> > drivers to share exported objects. A virtio dma-buf can be queried by
> > virtio drivers to obtain the UUID which identifies the underlying
> > exported object.
> >
> > Signed-off-by: David Stevens <stevensd@chromium.org>
>
> Is this just for graphics? If yes I'd rather we put it in the graphics
> driver. We can always move it later ...

As stated in the cover letter, this will be used by virtio-video.

The proposed virtio-video patches: https://markmail.org/thread/p5d3k566srtdtute
The patch which imports these dma-bufs (slightly out of data, uses v3
of this patch set): https://markmail.org/thread/j4xlqaaim266qpks

> > ---
> >  drivers/virtio/Makefile         |  2 +-
> >  drivers/virtio/virtio.c         |  6 +++
> >  drivers/virtio/virtio_dma_buf.c | 89 +++++++++++++++++++++++++++++++++
> >  include/linux/virtio.h          |  1 +
> >  include/linux/virtio_dma_buf.h  | 58 +++++++++++++++++++++
> >  5 files changed, 155 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/virtio/virtio_dma_buf.c
> >  create mode 100644 include/linux/virtio_dma_buf.h
> >
> > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> > index 29a1386ecc03..ecdae5b596de 100644
> > --- a/drivers/virtio/Makefile
> > +++ b/drivers/virtio/Makefile
> > @@ -1,5 +1,5 @@
> >  # SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
> > +obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o virtio_dma_buf.o
> >  obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
> >  obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
> >  virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
> > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> > index a977e32a88f2..5d46f0ded92d 100644
> > --- a/drivers/virtio/virtio.c
> > +++ b/drivers/virtio/virtio.c
> > @@ -357,6 +357,12 @@ int register_virtio_device(struct virtio_device *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(register_virtio_device);
> >
> > +bool is_virtio_device(struct device *dev)
> > +{
> > +     return dev->bus == &virtio_bus;
> > +}
> > +EXPORT_SYMBOL_GPL(is_virtio_device);
> > +
> >  void unregister_virtio_device(struct virtio_device *dev)
> >  {
> >       int index = dev->index; /* save for after device release */
> > diff --git a/drivers/virtio/virtio_dma_buf.c b/drivers/virtio/virtio_dma_buf.c
> > new file mode 100644
> > index 000000000000..23e3399b11ed
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_dma_buf.c
> > @@ -0,0 +1,89 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * dma-bufs for virtio exported objects
> > + *
> > + * Copyright (C) 2020 Google, Inc.
> > + */
> > +
> > +#include <linux/virtio_dma_buf.h>
> > +
> > +/**
> > + * virtio_dma_buf_export - Creates a new dma-buf for a virtio exported object
> > + *
> > + * This wraps dma_buf_export() to allow virtio drivers to create a dma-buf
> > + * for an virtio exported object that can be queried by other virtio drivers
> > + * for the object's UUID.
> > + */
> > +struct dma_buf *virtio_dma_buf_export(
> > +             const struct virtio_dma_buf_export_info *virtio_exp_info)
> > +{
> > +     struct dma_buf_export_info exp_info;
> > +
> > +     if (!virtio_exp_info->ops
> > +             || virtio_exp_info->ops->ops.attach != &virtio_dma_buf_attach
> > +             || !virtio_exp_info->ops->get_uuid) {
> > +             return ERR_PTR(-EINVAL);
> > +     }
> > +
> > +     exp_info.exp_name = virtio_exp_info->exp_name;
> > +     exp_info.owner = virtio_exp_info->owner;
> > +     exp_info.ops = &virtio_exp_info->ops->ops;
> > +     exp_info.size = virtio_exp_info->size;
> > +     exp_info.flags = virtio_exp_info->flags;
> > +     exp_info.resv = virtio_exp_info->resv;
> > +     exp_info.priv = virtio_exp_info->priv;
> > +     BUILD_BUG_ON(sizeof(struct virtio_dma_buf_export_info)
> > +                  != sizeof(struct dma_buf_export_info));
>
> This is the only part that gives me pause. Why do we need this hack?
> What's wrong with just using dma_buf_export_info directly,
> and if you want the virtio ops, just using container_off?

This approach provides a more explicit type signature and a little
more type safety, I think. If others don't think it's a worthwhile
tradeoff, I can remove it.

-David

^ permalink raw reply

* drivers/staging/media/usbvision/usbvision-core.c:1432: undefined reference to `usb_submit_urb'
From: kernel test robot @ 2020-06-05  0:25 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: kbuild-all, linux-kernel, Mauro Carvalho Chehab, linux-media

[-- Attachment #1: Type: text/plain, Size: 19599 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   15a2bc4dbb9cfed1c661a657fcb10798150b7598
commit: 8fb12ce2ec9d569e1b3051f01cee13ff27e29466 media: usbvision: deprecate driver
date:   3 months ago
config: powerpc64-randconfig-r023-20200604 (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 8fb12ce2ec9d569e1b3051f01cee13ff27e29466
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

powerpc64le-linux-ld: warning: orphan section `.gnu.hash' from `linker stubs' being placed in section `.gnu.hash'
powerpc64le-linux-ld: warning: orphan section `__timer_of_table' from `drivers/clocksource/timer-microchip-pit64b.o' being placed in section `__timer_of_table'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_write_reg_irq':
>> drivers/staging/media/usbvision/usbvision-core.c:1432: undefined reference to `usb_submit_urb'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_isoc_irq':
drivers/staging/media/usbvision/usbvision-core.c:1323: undefined reference to `usb_submit_urb'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_read_reg':
>> drivers/staging/media/usbvision/usbvision-core.c:1353: undefined reference to `usb_control_msg'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_write_reg':
drivers/staging/media/usbvision/usbvision-core.c:1383: undefined reference to `usb_control_msg'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_set_output':
drivers/staging/media/usbvision/usbvision-core.c:1695: undefined reference to `usb_control_msg'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_set_input':
drivers/staging/media/usbvision/usbvision-core.c:2005: undefined reference to `usb_control_msg'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_set_video_format':
drivers/staging/media/usbvision/usbvision-core.c:1633: undefined reference to `usb_control_msg'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o:drivers/staging/media/usbvision/usbvision-core.c:2080: more undefined references to `usb_control_msg' follow
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_set_alternate':
>> drivers/staging/media/usbvision/usbvision-core.c:2233: undefined reference to `usb_set_interface'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_init_isoc':
>> drivers/staging/media/usbvision/usbvision-core.c:2286: undefined reference to `usb_alloc_urb'
>> powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.c:2291: undefined reference to `usb_alloc_coherent'
>> powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.c:2318: undefined reference to `usb_submit_urb'
>> powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.c:2318: undefined reference to `usb_submit_urb'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.o: in function `usbvision_stop_isoc':
>> drivers/staging/media/usbvision/usbvision-core.c:2351: undefined reference to `usb_kill_urb'
>> powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.c:2353: undefined reference to `usb_free_coherent'
>> powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.c:2358: undefined reference to `usb_free_urb'
>> powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-core.c:2368: undefined reference to `usb_set_interface'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-video.o: in function `usbvision_release':
>> drivers/staging/media/usbvision/usbvision-video.c:1360: undefined reference to `usb_free_urb'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-video.o: in function `usbvision_disconnect':
>> drivers/staging/media/usbvision/usbvision-video.c:1578: undefined reference to `usb_put_dev'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-video.o: in function `usbvision_radio_close':
>> drivers/staging/media/usbvision/usbvision-video.c:1118: undefined reference to `usb_set_interface'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-video.o: in function `usbvision_probe':
>> drivers/staging/media/usbvision/usbvision-video.c:1414: undefined reference to `usb_get_dev'
>> powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-video.c:1543: undefined reference to `usb_put_dev'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-video.o: in function `usbvision_alloc':
>> drivers/staging/media/usbvision/usbvision-video.c:1329: undefined reference to `usb_alloc_urb'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-video.o: in function `usbvision_exit':
>> drivers/staging/media/usbvision/usbvision-video.c:1638: undefined reference to `usb_deregister'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-video.o: in function `usbvision_init':
>> drivers/staging/media/usbvision/usbvision-video.c:1625: undefined reference to `usb_register_driver'
powerpc64le-linux-ld: drivers/staging/media/usbvision/usbvision-i2c.o: in function `usbvision_i2c_write_max4':
>> drivers/staging/media/usbvision/usbvision-i2c.c:348: undefined reference to `usb_control_msg'

vim +1432 drivers/staging/media/usbvision/usbvision-core.c

781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1337  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1338  /*
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1339   * usbvision_read_reg()
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1340   *
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1341   * return  < 0 -> Error
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1342   *        >= 0 -> Data
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1343   */
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1344  
483dfdb64fd4a9 drivers/media/video/usbvision/usbvision-core.c Thierry MERLE         2006-12-04  1345  int usbvision_read_reg(struct usb_usbvision *usbvision, unsigned char reg)
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1346  {
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1347  	int err_code = 0;
8926e8453476ef drivers/media/usb/usbvision/usbvision-core.c   Hans Verkuil          2015-07-20  1348  	unsigned char *buffer = usbvision->ctrl_urb_buffer;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1349  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1350  	if (!USBVISION_IS_OPERATIONAL(usbvision))
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1351  		return -1;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1352  
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19 @1353  	err_code = usb_control_msg(usbvision->dev, usb_rcvctrlpipe(usbvision->dev, 1),
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1354  				USBVISION_OP_CODE,
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1355  				USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_ENDPOINT,
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1356  				0, (__u16) reg, buffer, 1, HZ);
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1357  
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1358  	if (err_code < 0) {
be9ed5117d95cd drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2009-01-08  1359  		dev_err(&usbvision->dev->dev,
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1360  			"%s: failed: error %d\n", __func__, err_code);
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1361  		return err_code;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1362  	}
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1363  	return buffer[0];
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1364  }
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1365  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1366  /*
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1367   * usbvision_write_reg()
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1368   *
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1369   * return 1 -> Reg written
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1370   *        0 -> usbvision is not yet ready
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1371   *       -1 -> Something went wrong
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1372   */
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1373  
483dfdb64fd4a9 drivers/media/video/usbvision/usbvision-core.c Thierry MERLE         2006-12-04  1374  int usbvision_write_reg(struct usb_usbvision *usbvision, unsigned char reg,
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1375  			    unsigned char value)
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1376  {
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1377  	int err_code = 0;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1378  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1379  	if (!USBVISION_IS_OPERATIONAL(usbvision))
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1380  		return 0;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1381  
8926e8453476ef drivers/media/usb/usbvision/usbvision-core.c   Hans Verkuil          2015-07-20  1382  	usbvision->ctrl_urb_buffer[0] = value;
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1383  	err_code = usb_control_msg(usbvision->dev, usb_sndctrlpipe(usbvision->dev, 1),
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1384  				USBVISION_OP_CODE,
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1385  				USB_DIR_OUT | USB_TYPE_VENDOR |
8926e8453476ef drivers/media/usb/usbvision/usbvision-core.c   Hans Verkuil          2015-07-20  1386  				USB_RECIP_ENDPOINT, 0, (__u16) reg,
8926e8453476ef drivers/media/usb/usbvision/usbvision-core.c   Hans Verkuil          2015-07-20  1387  				usbvision->ctrl_urb_buffer, 1, HZ);
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1388  
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1389  	if (err_code < 0) {
be9ed5117d95cd drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2009-01-08  1390  		dev_err(&usbvision->dev->dev,
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1391  			"%s: failed: error %d\n", __func__, err_code);
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1392  	}
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1393  	return err_code;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1394  }
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1395  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1396  
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1397  static void usbvision_ctrl_urb_complete(struct urb *urb)
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1398  {
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1399  	struct usb_usbvision *usbvision = (struct usb_usbvision *)urb->context;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1400  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1401  	PDEBUG(DBG_IRQ, "");
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1402  	usbvision->ctrl_urb_busy = 0;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1403  }
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1404  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1405  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1406  static int usbvision_write_reg_irq(struct usb_usbvision *usbvision, int address,
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1407  				unsigned char *data, int len)
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1408  {
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1409  	int err_code = 0;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1410  
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1411  	PDEBUG(DBG_IRQ, "");
6d6a48e51fd3bc drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-29  1412  	if (len > 8)
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1413  		return -EFAULT;
6d6a48e51fd3bc drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-29  1414  	if (usbvision->ctrl_urb_busy)
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1415  		return -EBUSY;
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1416  	usbvision->ctrl_urb_busy = 1;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1417  
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1418  	usbvision->ctrl_urb_setup.bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_ENDPOINT;
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1419  	usbvision->ctrl_urb_setup.bRequest     = USBVISION_OP_CODE;
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1420  	usbvision->ctrl_urb_setup.wValue       = 0;
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1421  	usbvision->ctrl_urb_setup.wIndex       = cpu_to_le16(address);
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1422  	usbvision->ctrl_urb_setup.wLength      = cpu_to_le16(len);
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1423  	usb_fill_control_urb(usbvision->ctrl_urb, usbvision->dev,
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1424  							usb_sndctrlpipe(usbvision->dev, 1),
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1425  							(unsigned char *)&usbvision->ctrl_urb_setup,
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1426  							(void *)usbvision->ctrl_urb_buffer, len,
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1427  							usbvision_ctrl_urb_complete,
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1428  							(void *)usbvision);
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1429  
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1430  	memcpy(usbvision->ctrl_urb_buffer, data, len);
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1431  
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19 @1432  	err_code = usb_submit_urb(usbvision->ctrl_urb, GFP_ATOMIC);
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1433  	if (err_code < 0) {
52cb0bf275debe drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1434  		/* error in usb_submit_urb() */
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1435  		usbvision->ctrl_urb_busy = 0;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1436  	}
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1437  	PDEBUG(DBG_IRQ, "submit %d byte: error %d", len, err_code);
5490a7cbe65d63 drivers/media/video/usbvision/usbvision-core.c Hans Verkuil          2010-12-19  1438  	return err_code;
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1439  }
781aa1d1ab7ba1 drivers/media/video/usbvision/usbvision-core.c Mauro Carvalho Chehab 2006-12-04  1440  

:::::: The code at line 1432 was first introduced by commit
:::::: 5490a7cbe65d63c6ec45f1013287af1e390c95d7 [media] usbvision: get rid of camelCase

:::::: TO: Hans Verkuil <hverkuil@xs4all.nl>
:::::: CC: Mauro Carvalho Chehab <mchehab@redhat.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32816 bytes --]

^ permalink raw reply

* Re: [PATCH v2 0/6] seqlock: seqcount_t call sites bugfixes
From: David Miller @ 2020-06-04 22:50 UTC (permalink / raw)
  To: a.darwish
  Cc: peterz, mingo, will, tglx, paulmck, bigeasy, rostedt,
	linux-kernel, kuba, edumazet, axboe, vgoyal, linux-block, airlied,
	daniel, sumit.semwal, linux-media, dri-devel
In-Reply-To: <20200603144949.1122421-1-a.darwish@linutronix.de>

From: "Ahmed S. Darwish" <a.darwish@linutronix.de>
Date: Wed,  3 Jun 2020 16:49:43 +0200

> Since patch #7 and #8 from the series:
> 
>    [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks
>    https://lore.kernel.org/lkml/20200519214547.352050-1-a.darwish@linutronix.de
> 
> are now pending on the lockdep/x86 IRQ state tracking patch series:
> 
>    [PATCH 00/14] x86/entry: disallow #DB more and x86/entry lockdep/nmi
>    https://lkml.kernel.org/r/20200529212728.795169701@infradead.org
> 
>    [PATCH v3 0/5] lockdep: Change IRQ state tracking to use per-cpu variables
>    https://lkml.kernel.org/r/20200529213550.683440625@infradead.org
> 
> This is a repost only of the seqcount_t call sites bugfixes that were on
> top of the seqlock patch series.
> 
> These fixes are independent, and can thus be merged on their own. I'm
> reposting them now so they can at least hit -rc2 or -rc3.
> 
> Changelog-v2:
> 
>   - patch #1: Add a missing up_read() on netdev_get_name() error path
>               exit. Thanks to Dan/kbuild-bot report.
> 
>   - patch #4: new patch, invalid preemptible context found by the new
>               lockdep checks added in the seqlock series + kbuild-bot.

I'll apply patches 1-4 to the net tree.

^ permalink raw reply

* [PATCH 0/5] Improve ACPI detection code for atomisp
From: Mauro Carvalho Chehab @ 2020-06-04 20:47 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, linux-kernel, Sakari Ailus,
	Greg Kroah-Hartman, devel, linux-media

Add support to read the sensor configuration directly from the BIOS,
instead of relying on DMI match tables.

As we don't have the legacy models there which has their own
DMI match tables, we can't remove them, but this change should
make this driver to better detect unlisted devices.

Mauro Carvalho Chehab (5):
  media: atomisp: improve sensor detection code to use _DSM table
  Revert "media: atomisp: Add some ACPI detection info"
  Revert "media: atomisp: add Asus Transform T101HA ACPI vars"
  media: atomisp: change clock source default for ISP2401
  media: atomisp: improve ACPI/DMI detection logs

 drivers/staging/media/atomisp/TODO            |  43 +----
 .../media/atomisp/i2c/atomisp-gc0310.c        |  11 --
 .../media/atomisp/i2c/atomisp-gc2235.c        |  11 --
 .../media/atomisp/i2c/atomisp-lm3554.c        |  11 --
 .../media/atomisp/i2c/atomisp-mt9m114.c       |  11 --
 .../media/atomisp/i2c/atomisp-ov2680.c        |  11 --
 .../media/atomisp/i2c/atomisp-ov2722.c        |  11 --
 .../media/atomisp/i2c/ov5693/atomisp-ov5693.c |  11 --
 .../media/atomisp/pci/atomisp_gmin_platform.c | 157 ++++++++++++++----
 9 files changed, 129 insertions(+), 148 deletions(-)

-- 
2.26.2



^ permalink raw reply

* [PATCH 4/5] media: atomisp: change clock source default for ISP2401
From: Mauro Carvalho Chehab @ 2020-06-04 20:47 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, devel, linux-kernel
In-Reply-To: <cover.1591303518.git.mchehab+huawei@kernel.org>

There's a notice there stating that the PLL is not reliable
for CHT. Yet, it tries to read it via the BIOS. Well,
this will fail (at least with the devices I checked the
DSDT tables). So, change the logic in a way that it will
change the default, depending on the ISP version.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../media/atomisp/pci/atomisp_gmin_platform.c  | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
index cb02806274d1..a14326111b26 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
@@ -493,9 +493,23 @@ static struct gmin_subdev *gmin_subdev_add(struct v4l2_subdev *subdev)
 
 	gmin_subdevs[i].subdev = subdev;
 	gmin_subdevs[i].clock_num = gmin_get_var_int(dev, false, "CamClk", 0);
-	/*WA:CHT requires XTAL clock as PLL is not stable.*/
+	/*
+	 * FIXME:
+	 * 	WA:CHT requires XTAL clock as PLL is not stable.
+	 *
+	 * However, such data doesn't seem to be present at the _DSM
+	 * table under the GUID dc2f6c4f-045b-4f1d-97b9-882a6860a4be.
+	 * So, let's change the default according with the ISP version,
+	 * but allowing it to be overridden by BIOS or by DMI match tables.
+	 */
+	if (IS_ISP2401)
+		gmin_subdevs[i].clock_src = VLV2_CLK_XTAL_25_0MHz;
+	else
+		gmin_subdevs[i].clock_src = VLV2_CLK_PLL_19P2MHZ;
+
 	gmin_subdevs[i].clock_src = gmin_get_var_int(dev, false, "ClkSrc",
-				    VLV2_CLK_PLL_19P2MHZ);
+						     gmin_subdevs[i].clock_src);
+
 	gmin_subdevs[i].csi_port = gmin_get_var_int(dev, false, "CsiPort", 0);
 	gmin_subdevs[i].csi_lanes = gmin_get_var_int(dev, false, "CsiLanes", 1);
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH 3/5] Revert "media: atomisp: add Asus Transform T101HA ACPI vars"
From: Mauro Carvalho Chehab @ 2020-06-04 20:47 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, devel, linux-kernel
In-Reply-To: <cover.1591303518.git.mchehab+huawei@kernel.org>

Now that the EFI _DSM table is parsed by the driver, we don't
need a DMI match anymore for Asus Transform T101HA.

This reverts commit 0a76fd8e8d202dcaabc714850205d5d75c9b8271.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../media/atomisp/pci/atomisp_gmin_platform.c    | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
index 5f34b2be5153..cb02806274d1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
@@ -328,15 +328,6 @@ static struct gmin_cfg_var i8880_vars[] = {
 	{},
 };
 
-static struct gmin_cfg_var asus_vars[] = {
-	{"OVTI2680:00_CsiPort", "1"},
-	{"OVTI2680:00_CsiLanes", "1"},
-	{"OVTI2680:00_CsiFmt", "15"},
-	{"OVTI2680:00_CsiBayer", "0"},
-	{"OVTI2680:00_CamClk", "1"},
-	{},
-};
-
 static const struct dmi_system_id gmin_vars[] = {
 	{
 		.ident = "BYT-T FFD8",
@@ -374,13 +365,6 @@ static const struct dmi_system_id gmin_vars[] = {
 		},
 		.driver_data = i8880_vars,
 	},
-	{
-		.ident = "T101HA",
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "T101HA"),
-		},
-		.driver_data = asus_vars,
-	},
 	{}
 };
 
-- 
2.26.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox