Dear Inki dae,
Sorry for the previous mail which is not completed. I typed something and it was the shortcut for maybe.
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.
Yes. I agree.
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.
In this case, MIC_2D_OUTPUT_TIMING_2 must be set whether it is being used with i80 or RGB interface. MIC_2D_OUTPUT_TIMING_{0/1/2} have similar names, so I thought that it is good to set them in a function. Do you think that make a new function for MIC_2D_OUTPUT_TIMING_2?
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;
Yes. That will make the code more clear.
- } 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.
Yes. After the checking code is inserted, it would be better to remove this function.
+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.
Yes. I will investigate that more and use that if it is possible.
- 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.
That would be better.
Thanks for your review.
Best regards, Hyungwon Hwang
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