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 at samsung.com>
> Signed-off-by: Shirish S <s.shirish at samsung.com>
> Signed-off-by: Fahad Kunnathadi <fahad.k at samsung.com>
> ---
> 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)
> 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", ctx);
> + 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.
Thanks.
