diff mbox

[1/3] drm: exynos: hdmi: add exynos5 support to mixer driver

Message ID 1347451727-13386-2-git-send-email-rahul.sharma@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rahul Sharma Sept. 12, 2012, 12:08 p.m. UTC
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@samsung.com>
Signed-off-by: Shirish S <s.shirish@samsung.com>
Signed-off-by: Fahad Kunnathadi <fahad.k@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(-)

Comments

Joonyoung Shim Sept. 13, 2012, 1:43 a.m. UTC | #1
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@samsung.com>
> Signed-off-by: Shirish S <s.shirish@samsung.com>
> Signed-off-by: Fahad Kunnathadi <fahad.k@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.
대인기/Tizen Platform Lab(SR)/삼성전자 Sept. 13, 2012, 2:53 a.m. UTC | #2
> -----Original Message-----
> From: Joonyoung Shim [mailto:jy0922.shim@samsung.com]
> Sent: Thursday, September 13, 2012 10:44 AM
> To: Rahul Sharma
> Cc: dri-devel@lists.freedesktop.org; sw0312.kim@samsung.com;
> inki.dae@samsung.com; kyungmin.park@samsung.com; prashanth.g@samsung.com;
> joshi@samsung.com; s.shirish@samsung.com; fahad.k@samsung.com;
> l.krishna@samsung.com; r.sh.open@gmail.com
> 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@samsung.com>
> > Signed-off-by: Shirish S <s.shirish@samsung.com>
> > Signed-off-by: Fahad Kunnathadi <fahad.k@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.
> 

Please, share mixer_resources_init function. I think we could reuse same
codes such things related to mixer clock, sclk_hdmi, io resource and irq.

> >   	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.
> 

For this, you could check mixer version reading MIXER_VER register but not
check hdmi. actually hdmi has no any register we can check version. this
would be the reason we used is_v13 variable to identify exynos4210 and
exynos4412 SoC. I tend to agree with Rahul. I will merge it as is if there
is no any better way and next we can fix it later. any opinions?


> Thanks.
Joonyoung Shim Sept. 13, 2012, 3:34 a.m. UTC | #3
On 09/13/2012 11:53 AM, Inki Dae wrote:
>
>> -----Original Message-----
>> From: Joonyoung Shim [mailto:jy0922.shim@samsung.com]
>> Sent: Thursday, September 13, 2012 10:44 AM
>> To: Rahul Sharma
>> Cc: dri-devel@lists.freedesktop.org; sw0312.kim@samsung.com;
>> inki.dae@samsung.com; kyungmin.park@samsung.com; prashanth.g@samsung.com;
>> joshi@samsung.com; s.shirish@samsung.com; fahad.k@samsung.com;
>> l.krishna@samsung.com; r.sh.open@gmail.com
>> 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@samsung.com>
>>> Signed-off-by: Shirish S <s.shirish@samsung.com>
>>> Signed-off-by: Fahad Kunnathadi <fahad.k@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.
>>
> Please, share mixer_resources_init function. I think we could reuse same
> codes such things related to mixer clock, sclk_hdmi, io resource and irq.
>
>>>    	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.
>>
> For this, you could check mixer version reading MIXER_VER register but not
> check hdmi. actually hdmi has no any register we can check version. this
> would be the reason we used is_v13 variable to identify exynos4210 and
> exynos4412 SoC. I tend to agree with Rahul. I will merge it as is if there
> is no any better way and next we can fix it later. any opinions?
>

Let's think to disassociate hdmi and mixer. I have plan to unify to one
many things of exynos hdmi. Above problem occurs because exynos5 doesn't
have video processor ip. Even if we use a field such is_soc_exynos5, the
is_soc_exynos5 is unsuitable naming if other exynos SoC also doesn't
have video processor ip.
대인기/Tizen Platform Lab(SR)/삼성전자 Sept. 13, 2012, 4:37 a.m. UTC | #4
2012? 9? 13? ???? Joonyoung Shim<jy0922.shim@samsung.com>?? ??:
> On 09/13/2012 11:53 AM, Inki Dae wrote:
>
> -----Original Message-----
> From: Joonyoung Shim [mailto:jy0922.shim@samsung.com]
> Sent: Thursday, September 13, 2012 10:44 AM
> To: Rahul Sharma
> Cc: dri-devel@lists.freedesktop.org; sw0312.kim@samsung.com;
> inki.dae@samsung.com; kyungmin.park@samsung.com; prashanth.g@samsung.com;
> joshi@samsung.com; s.shirish@samsung.com; fahad.k@samsung.com;
> l.krishna@samsung.com; r.sh.open@gmail.com
> 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@samsung.com>
> Signed-off-by: Shirish S <s.shirish@samsung.com>
> Signed-off-by: Fahad Kunnathadi <fahad.k@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
>
> Let's think to disassociate hdmi and mixer. I have plan to unify to one
> many things of exynos hdmi. Above problem occurs because exynos5 doesn't
> have video processor ip. Even if we use a field such is_soc_exynos5, the
> is_soc_exynos5 is unsuitable naming if other exynos SoC also doesn't
> have video processor ip.
>

one more thing, exynos5 uses GScaler instead of Video processor. the
GScaler can be also used as post processor but exynos5 spec has no any
descriptions to this. so we should check that first and next let's update
things related to hdmi.
diff mbox

Patch

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);
 	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 */
+	}
+};
+
+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)