Re: [PATCH v2 3/6] drm/exynos: mic: add MIC driver

Dear Inki dae,

On Tue, 24 Mar 2015 14:51:31 +0900 Inki Dae inki.dae@samsung.com wrote:

On 2015년 03월 18일 17:16, Hyungwon Hwang wrote:

...

MIC(Mobile image compressor) is newly added IP in Exynos5433. MIC resides between decon and mipi dsim, and compresses frame data by 50%. With dsi, not display port, to send frame data to the panel, the bandwidth is not enough. That is why this compressor is introduced.

Signed-off-by: Hyungwon Hwang human.hwang@samsung.com

Changes for v2:

• make mic driver to be registered by exynos drm driver instead as

a module |

driver |

---

• change the description of mic driver in documentation
• add module author at the top of the source file removing

MODULE_OWNER, MODULE_DESCRIPTION, MODULE_LICENSE .../devicetree/bindings/video/exynos-mic.txt | 49 +++ drivers/gpu/drm/exynos/Kconfig | 6 + drivers/gpu/drm/exynos/Makefile | 1 + drivers/gpu/drm/exynos/exynos_drm_drv.c | 3 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 + drivers/gpu/drm/exynos/exynos_drm_mic.c | 481 +++++++++++++++++++++ 6 files changed, 541 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/exynos-mic.txt create mode 100644 drivers/gpu/drm/exynos/exynos_drm_mic.c

diff --git a/Documentation/devicetree/bindings/video/exynos-mic.txt b/Documentation/devicetree/bindings/video/exynos-mic.txt new file mode 100644 index 0000000..006d072 --- /dev/null +++ b/Documentation/devicetree/bindings/video/exynos-mic.txt @@ -0,0 +1,49 @@ +Device-Tree bindings for Samsung Exynos SoC mobile image compressor (MIC) + +MIC (mobile image compressor) resides between decon and mipi dsi. Mipi dsi is +not capable to transfer high resoltuion frame data as decon can send. MIC +solves this problem by compressing the frame data by 1/2 before it is transfered +through mipi dsi. The compressed frame data must be uncompressed in the panel +PCB.

+Required properties: +- compatible: value should be "samsung,exynos5433-mic". +- reg: physical base address and length of the MIC registers set and system

•   register of mic.
  

+- clocks: must include clock specifiers corresponding to entries in the

•  clock-names property.
  

+- clock-names: list of clock names sorted in the same order as the clocks

•       property. Must contain "pclk_mic0",
  

"sclk_rgb_vclk_to_mic0". +- ports: contains a port which is connected to decon node and dsi node.

• address-cells and size-cells must 1 and 0, respectively.
  

+- port: contains an endpoint node which is connected to the endpoint in the

• decon node or dsi node. The reg value must be 0 and 1

respectively. + +Example: +SoC specific DT entry: +mic: mic@13930000 {

• compatible = "samsung,exynos5433-mic";
• reg = <0x13930000 0x48 0x13B80000 0x1010>;
• clocks = <&cmu_disp CLK_PCLK_MIC0>,

•       <&cmu_disp CLK_SCLK_RGB_VCLK_TO_MIC0>;
  

• clock-names = "pclk_mic0", "sclk_rgb_vclk_to_mic0";
• ports {

• #address-cells = <1>;
  

• #size-cells = <0>;
  

• port@0 {
  

• 	reg = <0>;
  

• 	mic_to_decon: endpoint {
  

• 		remote-endpoint = <&decon_to_mic>;
  

• 	};
  

• };
  

• port@1 {
  

• 	reg = <1>;
  

• 	mic_to_dsi: endpoint {
  

• 		remote-endpoint = <&dsi_to_mic>;
  

• 	};
  

• };
  

• };

+}; diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index e15cc2e..a796175 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -103,3 +103,9 @@ config DRM_EXYNOS_GSC depends on DRM_EXYNOS_IPP && ARCH_EXYNOS5 && !ARCH_MULTIPLATFORM help Choose this option if you want to use Exynos GSC for DRM.

+config DRM_EXYNOS_MIC

• bool "Exynos DRM MIC"
• depends on (DRM_EXYNOS && DRM_EXYNOS5433_DECON)
• help

•  Choose this option if you want to use Exynos MIC for DRM.
  

diff --git a/drivers/gpu/drm/exynos/Makefile b/drivers/gpu/drm/exynos/Makefile index fbd084d..7de0b10 100644 --- a/drivers/gpu/drm/exynos/Makefile +++ b/drivers/gpu/drm/exynos/Makefile @@ -22,5 +22,6 @@ exynosdrm-$(CONFIG_DRM_EXYNOS_IPP) += exynos_drm_ipp.o exynosdrm-$(CONFIG_DRM_EXYNOS_FIMC) += exynos_drm_fimc.o exynosdrm-$(CONFIG_DRM_EXYNOS_ROTATOR) += exynos_drm_rotator.o exynosdrm-$(CONFIG_DRM_EXYNOS_GSC) += exynos_drm_gsc.o +exynosdrm-$(CONFIG_DRM_EXYNOS_MIC) += exynos_drm_mic.o

obj-$(CONFIG_DRM_EXYNOS) += exynosdrm.o diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 1fa0dd0..ec9984d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -562,6 +562,9 @@ static struct platform_driver *const exynos_drm_kms_drivers[] = { #ifdef CONFIG_DRM_EXYNOS7_DECON &decon_driver, #endif +#ifdef CONFIG_DRM_EXYNOS_MIC

• &mic_driver,

+#endif #ifdef CONFIG_DRM_EXYNOS_DP &dp_driver, #endif diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index 40996d8..9c94dc9 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -357,4 +357,5 @@ extern struct platform_driver fimc_driver; extern struct platform_driver rotator_driver; extern struct platform_driver gsc_driver; extern struct platform_driver ipp_driver; +extern struct platform_driver mic_driver; #endif diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c new file mode 100644 index 0000000..b898a2a --- /dev/null +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c @@ -0,0 +1,481 @@ +/*

• • Copyright (C) 2015 Samsung Electronics Co.Ltd
• • Authors:
• • Hyungwon Hwang human.hwang@samsung.com
• • This program is free software; you can redistribute it and/or

modify

• • it under the terms of the GNU General Public License version 2

as

• • published by the Free Software Foundationr
• */

+#include <linux/platform_device.h> +#include <video/of_videomode.h> +#include <linux/of_address.h> +#include <video/videomode.h> +#include <linux/module.h> +#include <linux/delay.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/of_graph.h> +#include <linux/clk.h> +#include <drm/drmP.h>

+/* Sysreg registers for MIC */ +#define DSD_CFG_MUX 0x1004 +#define MIC0_RGB_MUX (1 << 0) +#define MIC0_I80_MUX (1 << 1) +#define MIC0_ON_MUX (1 << 5)

+/* MIC registers */ +#define MIC_OP 0x0 +#define MIC_IP_VER 0x0004 +#define MIC_V_TIMING_0 0x0008 +#define MIC_V_TIMING_1 0x000C +#define MIC_IMG_SIZE 0x0010 +#define MIC_INPUT_TIMING_0 0x0014 +#define MIC_INPUT_TIMING_1 0x0018 +#define MIC_2D_OUTPUT_TIMING_0 0x001C +#define MIC_2D_OUTPUT_TIMING_1 0x0020 +#define MIC_2D_OUTPUT_TIMING_2 0x0024 +#define MIC_3D_OUTPUT_TIMING_0 0x0028 +#define MIC_3D_OUTPUT_TIMING_1 0x002C +#define MIC_3D_OUTPUT_TIMING_2 0x0030 +#define MIC_CORE_PARA_0 0x0034 +#define MIC_CORE_PARA_1 0x0038 +#define MIC_CTC_CTRL 0x0040 +#define MIC_RD_DATA 0x0044

+#define MIC_UPD_REG (1 << 31) +#define MIC_ON_REG (1 << 30) +#define MIC_TD_ON_REG (1 << 29) +#define MIC_BS_CHG_OUT (1 << 16) +#define MIC_VIDEO_TYPE(x) (((x) & 0xf) << 12) +#define MIC_PSR_EN (1 << 5) +#define MIC_SW_RST (1 << 4) +#define MIC_ALL_RST (1 << 3) +#define MIC_CORE_VER_CONTROL (1 << 2) +#define MIC_MODE_SEL_COMMAND_MODE (1 << 1) +#define MIC_MODE_SEL_MASK (1 << 1) +#define MIC_CORE_EN (1 << 0)

+#define MIC_V_PULSE_WIDTH(x) (((x) & 0x3fff) << 16) +#define MIC_V_PERIOD_LINE(x) ((x) & 0x3fff)

+#define MIC_VBP_SIZE(x) (((x) & 0x3fff) << 16) +#define MIC_VFP_SIZE(x) ((x) & 0x3fff)

+#define MIC_IMG_V_SIZE(x) (((x) & 0x3fff) << 16) +#define MIC_IMG_H_SIZE(x) ((x) & 0x3fff)

+#define MIC_H_PULSE_WIDTH_IN(x) (((x) & 0x3fff) << 16) +#define MIC_H_PERIOD_PIXEL_IN(x) ((x) & 0x3fff)

+#define MIC_HBP_SIZE_IN(x) (((x) & 0x3fff) << 16) +#define MIC_HFP_SIZE_IN(x) ((x) & 0x3fff)

+#define MIC_H_PULSE_WIDTH_2D(x) (((x) & 0x3fff) << 16) +#define MIC_H_PERIOD_PIXEL_2D(x) ((x) & 0x3fff)

+#define MIC_HBP_SIZE_2D(x) (((x) & 0x3fff) << 16) +#define MIC_HFP_SIZE_2D(x) ((x) & 0x3fff)

+#define MIC_BS_SIZE_2D(x) ((x) & 0x3fff)

+enum {

• ENDPOINT_DECON_NODE,
• ENDPOINT_DSI_NODE,
• NUM_ENDPOINTS

+};

+static char *clk_names[] = { "pclk_mic0", "sclk_rgb_vclk_to_mic0" }; +#define NUM_CLKS ARRAY_SIZE(clk_names) +static DEFINE_MUTEX(mic_mutex);

+struct exynos_mic {

• struct device *dev;
• void __iomem *reg;
• void __iomem *sysreg;
• struct clk *clks[NUM_CLKS];
• bool i80_mode;
• struct videomode vm;
• struct drm_encoder *encoder;
• struct drm_bridge bridge;
• bool enabled;

+};

+static void mic_set_path(struct exynos_mic *mic, bool enable) +{

• u32 reg;
• reg = readl(mic->sysreg + DSD_CFG_MUX);
• if (enable)

• reg |= MIC0_RGB_MUX | MIC0_I80_MUX | MIC0_ON_MUX;
  

• else

• reg &= ~(MIC0_RGB_MUX | MIC0_I80_MUX |
  

MIC0_ON_MUX);

exynos_mic has already which video mode - RGB or i80 - is used so it'd be reasonable to select one of them to avoid confusing.

Yes. I agree.

...

• writel(reg, mic->sysreg + DSD_CFG_MUX);

+}

+static int mic_sw_reset(struct exynos_mic *mic) +{

• unsigned int retry = 100;
• int ret;
• writel(MIC_SW_RST, mic->reg + MIC_OP);
• while (retry-- > 0) {

• ret = readl(mic->reg + MIC_OP);
  

• if (!(ret & MIC_SW_RST))
  

• 	return 0;
  

• udelay(10);
  

• }
• return -ETIMEDOUT;

+}

+static void mic_set_porch_timing(struct exynos_mic *mic) +{

• struct videomode vm;
• u32 reg;
• if (!mic->i80_mode) {

It'd be better to check it before this function is called. This funcion call really is unnecessary in case of i80 mode.

...

• vm = mic->vm;
  

• reg = MIC_V_PULSE_WIDTH(vm.vsync_len) +
  

• 	MIC_V_PERIOD_LINE(vm.vsync_len +
  

vm.vactive +

• 			vm.vback_porch +
  

vm.vfront_porch);

• writel(reg, mic->reg + MIC_V_TIMING_0);
  

• reg = MIC_VBP_SIZE(vm.vback_porch) +
  

MIC_VFP_SIZE(vm.vfront_porch);

• writel(reg, mic->reg + MIC_V_TIMING_1);
  

• reg = MIC_V_PULSE_WIDTH(vm.hsync_len) +
  

• 	MIC_V_PERIOD_LINE(vm.hsync_len +
  

vm.hactive +

• 			vm.hback_porch +
  

vm.hfront_porch);

• writel(reg, mic->reg + MIC_INPUT_TIMING_0);
  

• reg = MIC_VBP_SIZE(vm.hback_porch) +
  

MIC_VFP_SIZE(vm.hfront_porch);

• writel(reg, mic->reg + MIC_INPUT_TIMING_1);
  

• }

+}

+static void mic_set_img_size(struct exynos_mic *mic) +{

• struct videomode *vm = &mic->vm;
• u32 reg;
• reg = MIC_IMG_H_SIZE(vm->hactive) +

• MIC_IMG_V_SIZE(vm->vactive);
  

• writel(reg, mic->reg + MIC_IMG_SIZE);

+}

+static void mic_set_output_timing(struct exynos_mic *mic) +{

• struct videomode vm = mic->vm;
• u32 reg, bs_size_2d;
• DRM_DEBUG("w: %u, h: %u\n", vm.hactive, vm.vactive);
• bs_size_2d = ((vm.hactive >> 2) << 1) + (vm.vactive % 4);
• reg = MIC_BS_SIZE_2D(bs_size_2d);
• writel(reg, mic->reg + MIC_2D_OUTPUT_TIMING_2);
• if (!mic->i80_mode) {

Ditto.

...

• reg = MIC_H_PULSE_WIDTH_2D(vm.hsync_len) +
  

• 	MIC_H_PERIOD_PIXEL_2D(vm.hsync_len +
  

bs_size_2d +

• 			vm.hback_porch +
  

vm.hfront_porch);

• writel(reg, mic->reg + MIC_2D_OUTPUT_TIMING_0);
  

• reg = MIC_HBP_SIZE_2D(vm.hback_porch) +
  

• 	MIC_H_PERIOD_PIXEL_2D(vm.hfront_porch);
  

• writel(reg, mic->reg + MIC_2D_OUTPUT_TIMING_1);
  

• }

+}

+static void mic_set_reg_on(struct exynos_mic *mic, bool enable) +{

• u32 reg = readl(mic->reg + MIC_OP);
• if (enable) {

• reg &= ~(MIC_MODE_SEL_MASK | MIC_CORE_VER_CONTROL
  

| MIC_PSR_EN);

• reg |= (MIC_CORE_EN | MIC_BS_CHG_OUT | MIC_ON_REG);
  

• if (mic->i80_mode)
  

• 	reg |= MIC_MODE_SEL_COMMAND_MODE;
  

Set explicitly it to video mode in case of RGB mode even though MODE_SET field of this register has 0x0 as default.

i.e., reg &= ~MIC_MODE_SEL_COMMAND_MODE; if (mic->i80_mode) reg |= MIC_MODE_SEL_COMMAND_MODE;

...

• } else {

• reg &= ~MIC_CORE_EN;
  

• }
• reg |= MIC_UPD_REG;
• writel(reg, mic->reg + MIC_OP);

+}

+static struct device_node *get_remote_node(struct device_node *from, int reg) +{

• struct device_node *endpoint = NULL, *remote_node = NULL;
• endpoint = of_graph_get_endpoint_by_regs(from, reg, -1);
• if (!endpoint) {

• DRM_ERROR("mic: Failed to find remote port from
  

%s",

• 		from->full_name);
  

• goto exit;
  

• }
• remote_node = of_graph_get_remote_port_parent(endpoint);
• if (!remote_node) {

• DRM_ERROR("mic: Failed to find remote port parent
  

from %s",

from->full_name);

• goto exit;
  

• }

+exit:

• of_node_put(endpoint);
• return remote_node;

+}

+static int parse_dt(struct exynos_mic *mic) +{

• int ret = 0, i, j;
• struct device_node *remote_node;
• struct device_node *nodes[3];
• /*

• * The order of endpoints does matter.
  

• * The first node must be for decon and the second one
  

must be for dsi.

• */
  

• for (i = 0, j = 0; i < NUM_ENDPOINTS; i++) {

• remote_node = get_remote_node(mic->dev->of_node,
  

i);

• if (!remote_node) {
  

• 	ret = -EPIPE;
  

• 	goto exit;
  

• }
  

• nodes[j++] = remote_node;
  

• switch (i) {
  

• case ENDPOINT_DECON_NODE:
  

• 	/* decon node */
  

• 	if (of_get_child_by_name(remote_node,
  

• 				"i80-if-timings"))
  

• 		mic->i80_mode = 1;
  

• 	break;
  

• case ENDPOINT_DSI_NODE:
  

• 	/* panel node */
  

• 	remote_node = get_remote_node(remote_node,
  

1);

• 	if (!remote_node) {
  

• 		ret = -EPIPE;
  

• 		goto exit;
  

• 	}
  

• 	nodes[j++] = remote_node;
  

• 	ret = of_get_videomode(remote_node,
  

• 					&mic->vm,
  

0);

• 	if (ret) {
  

• 		DRM_ERROR("mic: failed to get
  

videomode");

• 		goto exit;
  

• 	}
  

• 	break;
  

• default:
  

• 	DRM_ERROR("mic: Unknown endpoint from
  

MIC");

• 	break;
  

• }
  

• }

+exit:

• while (--j > -1)

• of_node_put(nodes[j]);
  

• return ret;

+}

+void mic_disable(struct drm_bridge *bridge) { }

It seems that drm_encoder_disable function of drm_crtc_helper.c doesn't check if the disable callback is NULL or not. Anyway, no problem but this funtion could be removed with the disable callback checking later.

...

+void mic_post_disable(struct drm_bridge *bridge) +{

• struct exynos_mic *mic = bridge->driver_private;
• int i;
• mutex_lock(&mic_mutex);
• if (!mic->enabled)

• goto already_disabled;
  

• mic_set_path(mic, 0);
• for (i = NUM_CLKS - 1; i > -1; i--)

• clk_disable_unprepare(mic->clks[i]);
  

• mic->enabled = 0;

+already_disabled:

• mutex_unlock(&mic_mutex);

+}

+void mic_pre_enable(struct drm_bridge *bridge) +{

• struct exynos_mic *mic = bridge->driver_private;
• int ret, i;
• mutex_lock(&mic_mutex);
• if (mic->enabled)

• goto already_enabled;
  

• for (i = 0; i < NUM_CLKS; i++) {

• ret = clk_prepare_enable(mic->clks[i]);
  

• if (ret < 0) {
  

• 	DRM_ERROR("Failed to enable clock (%s)\n",
  

clk_names[i]);

• 	goto turn_off_clks;
  

• }
  

• }
• mic_set_path(mic, 1);
• ret = mic_sw_reset(mic);
• if (ret) {

• DRM_ERROR("Failed to reset\n");
  

• goto turn_off_clks;
  

• }
• mic_set_porch_timing(mic);
• mic_set_img_size(mic);
• mic_set_output_timing(mic);
• mic_set_reg_on(mic, 1);
• mic->enabled = 1;
• mutex_unlock(&mic_mutex);
• return;

+turn_off_clks:

• while (--i > -1)

• clk_disable_unprepare(mic->clks[i]);
  

+already_enabled:

• mutex_unlock(&mic_mutex);

+}

+void mic_enable(struct drm_bridge *bridge) { }

+void mic_destroy(struct drm_bridge *bridge) +{

• struct exynos_mic *mic = bridge->driver_private;
• int i;
• mutex_lock(&mic_mutex);
• if (!mic->enabled)

• goto already_disabled;
  

• for (i = NUM_CLKS - 1; i > -1; i--)

• clk_disable_unprepare(mic->clks[i]);
  

+already_disabled:

• mutex_unlock(&mic_mutex);

+}

+struct drm_bridge_funcs mic_bridge_funcs = {

• .disable = mic_disable,
• .post_disable = mic_post_disable,
• .pre_enable = mic_pre_enable,
• .enable = mic_enable,

+};

+int exynos_mic_probe(struct platform_device *pdev) +{

• struct device *dev = &pdev->dev;
• struct exynos_mic *mic;
• struct resource res;
• int ret, i;
• mic = devm_kzalloc(dev, sizeof(*mic), GFP_KERNEL);
• if (!mic) {

• DRM_ERROR("mic: Failed to allocate memory for MIC
  

object\n");

• ret = -ENOMEM;
  

• goto err;
  

• }
• mic->dev = dev;
• ret = parse_dt(mic);
• if (ret)

• goto err;
  

• ret = of_address_to_resource(dev->of_node, 0, &res);
• if (ret) {

• DRM_ERROR("mic: Failed to get mem region for
  

MIC\n");

• goto err;
  

• }
• mic->reg = devm_ioremap(dev, res.start,

resource_size(&res));

• if (!mic->reg) {

• DRM_ERROR("mic: Failed to remap for MIC\n");
  

• ret = -ENOMEM;
  

• goto err;
  

• }
• ret = of_address_to_resource(dev->of_node, 1, &res);
• if (ret) {

• DRM_ERROR("mic: Failed to get mem region for
  

MIC\n");

• goto err;
  

• }
• mic->sysreg = devm_ioremap(dev, res.start,

resource_size(&res));

Consider to use syscon framework to access system registers.

...

• if (!mic->sysreg) {

• DRM_ERROR("mic: Failed to remap for MIC\n");
  

• goto err;
  

• }
• mic->bridge.funcs = &mic_bridge_funcs;
• mic->bridge.of_node = dev->of_node;
• mic->bridge.driver_private = mic;
• ret = drm_bridge_add(&mic->bridge);
• if (ret) {

• DRM_ERROR("mic: Failed to add MIC to the global
  

bridge list\n");

• goto err;
  

• }
• for (i = 0; i < NUM_CLKS; i++) {

• mic->clks[i] = of_clk_get_by_name(dev->of_node,
  

clk_names[i]);

• if (IS_ERR(mic->clks[i])) {
  

• 	DRM_ERROR("mic: Failed to get clock
  

(%s)\n",

clk_names[i]);

• 	ret = PTR_ERR(mic->clks[i]);
  

• 	goto err;
  

• }
  

• }
• DRM_INFO("MIC has been probed\n");

Use DRM_DEBUG_KMS instead.

Thanks, Inki Dae

...

+err:

• return ret;

+}

+static int exynos_mic_remove(struct platform_device *pdev) +{

• struct exynos_mic *mic = platform_get_drvdata(pdev);
• int i;
• drm_bridge_remove(&mic->bridge);
• for (i = NUM_CLKS - 1; i > -1; i--)

• clk_put(mic->clks[i]);
  

• return 0;

+}

+static const struct of_device_id exynos_mic_of_match[] = {

• { .compatible = "samsung,exynos5433-mic" },
• { }

+}; +MODULE_DEVICE_TABLE(of, exynos_mic_of_match);

+struct platform_driver mic_driver = {

• .probe = exynos_mic_probe,
• .remove = exynos_mic_remove,
• .driver = {

• .name	= "exynos-mic",
  

• .owner	= THIS_MODULE,
  

• .of_match_table = exynos_mic_of_match,
  

• },

+};

1.9.1