-----Original Message-----
From: Joonyoung Shim [mailto:jy0922.shim@xxxxxxxxxxx]
Sent: Thursday, September 13, 2012 10:44 AM
To: Rahul Sharma
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; sw0312.kim@xxxxxxxxxxx;
inki.dae@xxxxxxxxxxx; kyungmin.park@xxxxxxxxxxx; prashanth.g@xxxxxxxxxxx;
joshi@xxxxxxxxxxx; s.shirish@xxxxxxxxxxx; fahad.k@xxxxxxxxxxx;
l.krishna@xxxxxxxxxxx; r.sh.open@xxxxxxxxx
Subject: Re: [PATCH 1/3] drm: exynos: hdmi: add exynos5 support to mixer
driver
Hi, Rahul.
On 09/12/2012 09:08 PM, Rahul Sharma wrote:
Added support for exynos5 to drm mixer driver. Exynos5 works
with dt enabled while in exynos4 mixer device information can
be passed either way (dt or plf data). This situation is taken
cared.
Signed-off-by: Rahul Sharma <rahul.sharma@xxxxxxxxxxx>
Signed-off-by: Shirish S <s.shirish@xxxxxxxxxxx>
Signed-off-by: Fahad Kunnathadi <fahad.k@xxxxxxxxxxx>
---
drivers/gpu/drm/exynos/exynos_mixer.c | 153
++++++++++++++++++++++++++++++---
drivers/gpu/drm/exynos/regs-mixer.h | 2 +
2 files changed, 142 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 8a43ee1..7d04a40 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -71,6 +71,7 @@ struct mixer_resources {
struct clk *sclk_mixer;
struct clk *sclk_hdmi;
struct clk *sclk_dac;
+ bool is_soc_exynos5;
};
struct mixer_context {
@@ -251,7 +252,8 @@ static void mixer_vsync_set_update(struct
mixer_context *ctx, bool enable)
mixer_reg_writemask(res, MXR_STATUS, enable ?
MXR_STATUS_SYNC_ENABLE : 0, MXR_STATUS_SYNC_ENABLE);
- vp_reg_write(res, VP_SHADOW_UPDATE, enable ?
+ if (!res->is_soc_exynos5)
+ vp_reg_write(res, VP_SHADOW_UPDATE, enable ?
VP_SHADOW_UPDATE_ENABLE : 0);
}
@@ -615,15 +617,21 @@ static void mixer_win_reset(struct mixer_context
*ctx)
val = MXR_GRP_CFG_ALPHA_VAL(0);
mixer_reg_write(res, MXR_VIDEO_CFG, val);
- /* configuration of Video Processor Registers */
- vp_win_reset(ctx);
- vp_default_filter(res);
+ if (!res->is_soc_exynos5) {
+ /* configuration of Video Processor Registers */
+ vp_win_reset(ctx);
+ vp_default_filter(res);
+ }
/* disable all layers */
mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_GRP0_ENABLE);
mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_GRP1_ENABLE);
mixer_reg_writemask(res, MXR_CFG, 0, MXR_CFG_VP_ENABLE);
+ /* enable vsync interrupt after mixer reset*/
+ mixer_reg_writemask(res, MXR_INT_EN, MXR_INT_EN_VSYNC,
+ MXR_INT_EN_VSYNC);
+
mixer_vsync_set_update(ctx, true);
spin_unlock_irqrestore(&res->reg_slock, flags);
}
@@ -645,7 +653,8 @@ static void mixer_poweron(struct mixer_context *ctx)
pm_runtime_get_sync(ctx->dev);
clk_enable(res->mixer);
- clk_enable(res->vp);
+ if (!res->is_soc_exynos5)
+ clk_enable(res->vp);
clk_enable(res->sclk_mixer);
mixer_reg_write(res, MXR_INT_EN, ctx->int_en);
@@ -666,7 +675,8 @@ static void mixer_poweroff(struct mixer_context
ctx->int_en = mixer_reg_read(res, MXR_INT_EN);
clk_disable(res->mixer);
- clk_disable(res->vp);
+ if (!res->is_soc_exynos5)
+ clk_disable(res->vp);
clk_disable(res->sclk_mixer);
pm_runtime_put_sync(ctx->dev);
@@ -797,11 +807,16 @@ static void mixer_win_mode_set(void *ctx,
static void mixer_win_commit(void *ctx, int win)
{
struct mixer_context *mixer_ctx = ctx;
+ struct mixer_resources *res = &mixer_ctx->mixer_res;
DRM_DEBUG_KMS("[%d] %s, win: %d\n", __LINE__, __func__, win);
- if (win > 1)
- vp_video_buffer(mixer_ctx, win);
+ if (!res->is_soc_exynos5) {
+ if (win > 1)
+ vp_video_buffer(mixer_ctx, win);
+ else
+ mixer_graph_buffer(mixer_ctx, win);
+ }
else
mixer_graph_buffer(mixer_ctx, win);
}
@@ -888,6 +903,12 @@ static irqreturn_t mixer_irq_handler(int irq, void
*arg)
/* handling VSYNC */
if (val & MXR_INT_STATUS_VSYNC) {
+ /* layer update mandatory for exynos5 soc,and not present
+ * in exynos4 */
+ if (res->is_soc_exynos5)
+ mixer_reg_writemask(res, MXR_CFG, ~0,
+ MXR_CFG_LAYER_UPDATE);
+
/* interlace scan need to check shadow register */
if (ctx->interlace) {
base = mixer_reg_read(res, MXR_GRAPHIC_BASE(0));
@@ -919,8 +940,81 @@ out:
return IRQ_HANDLED;
}
-static int __devinit mixer_resources_init(struct
exynos_drm_hdmi_context *ctx,
- struct platform_device *pdev)
+static int __devinit mixer_resources_init_exynos5(
+ struct exynos_drm_hdmi_context *ctx,
+ struct platform_device *pdev)
+{
+ struct mixer_context *mixer_ctx = ctx->ctx;
+ struct device *dev = &pdev->dev;
+ struct mixer_resources *mixer_res = &mixer_ctx->mixer_res;
+ struct resource *res;
+ int ret;
+
+ DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
+
+ mixer_res->is_soc_exynos5 = true;
+ spin_lock_init(&mixer_res->reg_slock);
+
+ mixer_res->mixer = clk_get(dev, "mixer");
+ if (IS_ERR_OR_NULL(mixer_res->mixer)) {
+ dev_err(dev, "failed to get clock 'mixer'\n");
+ ret = -ENODEV;
+ goto fail;
+ }
+
+ mixer_res->sclk_hdmi = clk_get(dev, "sclk_hdmi");
+ if (IS_ERR_OR_NULL(mixer_res->sclk_hdmi)) {
+ dev_err(dev, "failed to get clock 'sclk_hdmi'\n");
+ ret = -ENODEV;
+ goto fail;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (res == NULL) {
+ dev_err(dev, "get memory resource failed.\n");
+ ret = -ENXIO;
+ goto fail;
+ }
+
+ mixer_res->mixer_regs = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
+ if (mixer_res->mixer_regs == NULL) {
+ dev_err(dev, "register mapping failed.\n");
+ ret = -ENXIO;
+ goto fail;
+ }
+
+
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (res == NULL) {
+ dev_err(dev, "get interrupt resource failed.\n");
+ ret = -ENXIO;
+ goto fail;
+ }
+
+ ret = devm_request_irq(&pdev->dev, res->start, mixer_irq_handler,
+ 0, "drm_mixer",
+ if (ret) {
+ dev_err(dev, "request interrupt failed.\n");
+ goto fail;
+ }
+ mixer_res->irq = res->start;
+
+ return 0;
+
+fail:
+ if (!IS_ERR_OR_NULL(mixer_res->sclk_dac))
+ clk_put(mixer_res->sclk_dac);
+ if (!IS_ERR_OR_NULL(mixer_res->sclk_hdmi))
+ clk_put(mixer_res->sclk_hdmi);
+ if (!IS_ERR_OR_NULL(mixer_res->mixer))
+ clk_put(mixer_res->mixer);
+ return ret;
+}
+
+static int __devinit mixer_resources_init_exynos4(
+ struct exynos_drm_hdmi_context *ctx,
+ struct platform_device *pdev)
{
struct mixer_context *mixer_ctx = ctx->ctx;
struct device *dev = &pdev->dev;
@@ -928,6 +1022,10 @@ static int __devinit mixer_resources_init(struct
exynos_drm_hdmi_context *ctx,
struct resource *res;
int ret;
+ DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
+
+ mixer_res->is_soc_exynos5 = false;
+
spin_lock_init(&mixer_res->reg_slock);
mixer_res->mixer = clk_get(dev, "mixer");
@@ -1028,10 +1126,17 @@ static int __devinit mixer_probe(struct
platform_device *pdev)
struct device *dev = &pdev->dev;
struct exynos_drm_hdmi_context *drm_hdmi_ctx;
struct mixer_context *ctx;
+ bool is_soc_exynos5 = false;
int ret;
dev_info(dev, "probe start\n");
+ if (pdev->dev.of_node &&
+ of_device_is_compatible(pdev->dev.of_node,
+ "samsung,exynos5-mixer")) {
+ is_soc_exynos5 = true;
+ }
+
drm_hdmi_ctx = devm_kzalloc(&pdev->dev, sizeof(*drm_hdmi_ctx),
GFP_KERNEL);
if (!drm_hdmi_ctx) {
@@ -1053,7 +1158,10 @@ static int __devinit mixer_probe(struct
platform_device *pdev)
platform_set_drvdata(pdev, drm_hdmi_ctx);
/* acquire resources: regs, irqs, clocks */
- ret = mixer_resources_init(drm_hdmi_ctx, pdev);
+ if (is_soc_exynos5)
+ ret = mixer_resources_init_exynos5(drm_hdmi_ctx, pdev);
+ else
+ ret = mixer_resources_init_exynos4(drm_hdmi_ctx, pdev);
I don't like this. These share many same codes.
if (ret)
goto fail;
@@ -1064,7 +1172,6 @@ static int __devinit mixer_probe(struct
platform_device *pdev)
return 0;
-
fail:
dev_info(dev, "probe failed\n");
return ret;
@@ -1093,12 +1200,32 @@ static int mixer_suspend(struct device *dev)
static SIMPLE_DEV_PM_OPS(mixer_pm_ops, mixer_suspend, NULL);
+static struct platform_device_id mixer_driver_types[] = {
+ {
+ .name = "exynos5-mixer",
+ }, {
+ .name = "exynos4-mixer",
+ }, {
+ /* end node */
+ }
+};
+
These names should consider devname of clock.
+static struct of_device_id mixer_match_types[] = {
+ {
+ .compatible = "samsung,exynos5-mixer",
+ }, {
+ /* end node */
+ }
+};
+
struct platform_driver mixer_driver = {
.driver = {
- .name = "s5p-mixer",
+ .name = "exynos-mixer",
.owner = THIS_MODULE,
.pm = &mixer_pm_ops,
+ .of_match_table = mixer_match_types,
},
.probe = mixer_probe,
.remove = __devexit_p(mixer_remove),
+ .id_table = mixer_driver_types,
};
diff --git a/drivers/gpu/drm/exynos/regs-mixer.h
b/drivers/gpu/drm/exynos/regs-mixer.h
index fd2f4d1..0491ad8 100644
--- a/drivers/gpu/drm/exynos/regs-mixer.h
+++ b/drivers/gpu/drm/exynos/regs-mixer.h
@@ -69,6 +69,7 @@
(((val) << (low_bit)) & MXR_MASK(high_bit, low_bit))
/* bits for MXR_STATUS */
+#define MXR_STATUS_SOFT_RESET (1 << 8)
#define MXR_STATUS_16_BURST (1 << 7)
#define MXR_STATUS_BURST_MASK (1 << 7)
#define MXR_STATUS_BIG_ENDIAN (1 << 3)
@@ -77,6 +78,7 @@
#define MXR_STATUS_REG_RUN (1 << 0)
/* bits for MXR_CFG */
+#define MXR_CFG_LAYER_UPDATE (1 << 31)
#define MXR_CFG_RGB601_0_255 (0 << 9)
#define MXR_CFG_RGB601_16_235 (1 << 9)
#define MXR_CFG_RGB709_0_255 (2 << 9)
Overall, i think to use is_soc_exynos5 causes too many if statements.
Let's look other good idea to solve basically.
