[PATCH 2/3] drm: exynos: hdmi: add exynos5 support to hdmi driver

Joonyoung Shim jy0922.shim at samsung.com
Wed Sep 12 18:23:25 PDT 2012


Hi, Rahul.

Overall, i think this patch causes messy codes.

On 09/12/2012 09:08 PM, Rahul Sharma wrote:
> Added support for exynos5 to hdmi driver. Resource init
> is splitted for exynos5 and exynos4. Exynos5 hdmi driver
> is dt based while exynos4 hdmi driver is not.
>
> 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_hdmi.c |  300 ++++++++++++++++++++++++++++++----
>   1 files changed, 269 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index a6aea6f..5236256 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -32,6 +32,9 @@
>   #include <linux/pm_runtime.h>
>   #include <linux/clk.h>
>   #include <linux/regulator/consumer.h>
> +#include <linux/io.h>
> +#include <linux/of_gpio.h>
> +#include <plat/gpio-cfg.h>
>   
>   #include <drm/exynos_drm.h>
>   
> @@ -61,11 +64,13 @@ struct hdmi_context {
>   	bool				powered;
>   	bool				is_v13;
>   	bool				dvi_mode;
> +	bool				is_soc_exynos5;
>   	struct mutex			hdmi_mutex;
>   
>   	void __iomem			*regs;
> -	unsigned int			external_irq;
> -	unsigned int			internal_irq;
> +	int						external_irq;
> +	int						internal_irq;
> +	int						hpd_gpio;
>   
>   	struct i2c_client		*ddc_port;
>   	struct i2c_client		*hdmiphy_port;
> @@ -953,6 +958,23 @@ static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
>   	writel(value, hdata->regs + reg_id);
>   }
>   
> +static void hdmi_cfg_hpd(struct hdmi_context *hdata, bool internal)
> +{
> +	if (!internal) {
> +		s3c_gpio_cfgpin(hdata->hpd_gpio, S3C_GPIO_SFN(0xf));
> +		s3c_gpio_setpull(hdata->hpd_gpio, S3C_GPIO_PULL_DOWN);
> +	} else {
> +		s3c_gpio_cfgpin(hdata->hpd_gpio, S3C_GPIO_SFN(3));
> +		s3c_gpio_setpull(hdata->hpd_gpio, S3C_GPIO_PULL_NONE);
> +	}
> +}
> +

Don't use SoC specific functions in the driver.

> +static int hdmi_get_hpd(struct hdmi_context *hdata)
> +{
> +	int gpio_stat = gpio_get_value(hdata->hpd_gpio);
> +	return gpio_stat;
> +}
> +

Actually, above two functions should come from platform data, but these
will remove soon.

>   static void hdmi_v13_regs_dump(struct hdmi_context *hdata, char *prefix)
>   {
>   #define DUMPREG(reg_id) \
> @@ -2026,6 +2048,9 @@ static void hdmi_poweron(struct hdmi_context *hdata)
>   
>   	if (hdata->cfg_hpd)
>   		hdata->cfg_hpd(true);
> +	else
> +		hdmi_cfg_hpd(hdata, true);
> +
>   	mutex_unlock(&hdata->hdmi_mutex);
>   
>   	pm_runtime_get_sync(hdata->dev);
> @@ -2063,6 +2088,8 @@ static void hdmi_poweroff(struct hdmi_context *hdata)
>   	mutex_lock(&hdata->hdmi_mutex);
>   	if (hdata->cfg_hpd)
>   		hdata->cfg_hpd(false);
> +	else
> +		hdmi_cfg_hpd(hdata, false);
>   
>   	hdata->powered = false;
>   
> @@ -2110,17 +2137,16 @@ static irqreturn_t hdmi_external_irq_thread(int irq, void *arg)
>   	struct exynos_drm_hdmi_context *ctx = arg;
>   	struct hdmi_context *hdata = ctx->ctx;
>   
> -	if (!hdata->get_hpd)
> -		goto out;
> -
>   	mutex_lock(&hdata->hdmi_mutex);
> -	hdata->hpd = hdata->get_hpd();
> +	if (hdata->get_hpd)
> +		hdata->hpd = hdata->get_hpd();
> +	else
> +		hdata->hpd = hdmi_get_hpd(hdata);
>   	mutex_unlock(&hdata->hdmi_mutex);
>   
>   	if (ctx->drm_dev)
>   		drm_helper_hpd_irq_event(ctx->drm_dev);
>   
> -out:
>   	return IRQ_HANDLED;
>   }
>   
> @@ -2203,23 +2229,25 @@ static int __devinit hdmi_resources_init(struct hdmi_context *hdata)
>   
>   	clk_set_parent(res->sclk_hdmi, res->sclk_pixel);
>   
> -	res->regul_bulk = kzalloc(ARRAY_SIZE(supply) *
> -		sizeof(res->regul_bulk[0]), GFP_KERNEL);
> -	if (!res->regul_bulk) {
> -		DRM_ERROR("failed to get memory for regulators\n");
> -		goto fail;
> -	}
> -	for (i = 0; i < ARRAY_SIZE(supply); ++i) {
> -		res->regul_bulk[i].supply = supply[i];
> -		res->regul_bulk[i].consumer = NULL;
> -	}
> -	ret = regulator_bulk_get(dev, ARRAY_SIZE(supply), res->regul_bulk);
> -	if (ret) {
> -		DRM_ERROR("failed to get regulators\n");
> -		goto fail;
> +	if (!hdata->is_soc_exynos5) {
> +		res->regul_bulk = kzalloc(ARRAY_SIZE(supply) *
> +			sizeof(res->regul_bulk[0]), GFP_KERNEL);
> +		if (!res->regul_bulk) {
> +			DRM_ERROR("failed to get memory for regulators\n");
> +			goto fail;
> +		}
> +		for (i = 0; i < ARRAY_SIZE(supply); ++i) {
> +			res->regul_bulk[i].supply = supply[i];
> +			res->regul_bulk[i].consumer = NULL;
> +		}
> +		ret = regulator_bulk_get(dev, ARRAY_SIZE(supply),
> +			res->regul_bulk);
> +		if (ret) {
> +			DRM_ERROR("failed to get regulators\n");
> +			goto fail;
> +		}
> +		res->regul_count = ARRAY_SIZE(supply);
>   	}
> -	res->regul_count = ARRAY_SIZE(supply);
> -
>   	return 0;
>   fail:
>   	DRM_ERROR("HDMI resource init - failed\n");
> @@ -2262,7 +2290,8 @@ void hdmi_attach_hdmiphy_client(struct i2c_client *hdmiphy)
>   		hdmi_hdmiphy = hdmiphy;
>   }
>   
> -static int __devinit hdmi_probe(struct platform_device *pdev)
> +static int __devinit hdmi_resources_init_exynos4(
> +	struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct exynos_drm_hdmi_context *drm_hdmi_ctx;
> @@ -2271,7 +2300,7 @@ static int __devinit hdmi_probe(struct platform_device *pdev)
>   	struct resource *res;
>   	int ret;
>   
> -	DRM_DEBUG_KMS("[%d]\n", __LINE__);
> +	DRM_DEBUG_KMS("[%d][%s]\n", __LINE__, __func__);
>   
>   	pdata = pdev->dev.platform_data;
>   	if (!pdata) {
> @@ -2304,14 +2333,21 @@ static int __devinit hdmi_probe(struct platform_device *pdev)
>   	hdata->cfg_hpd = pdata->cfg_hpd;
>   	hdata->get_hpd = pdata->get_hpd;
>   	hdata->dev = dev;
> +	hdata->is_soc_exynos5 = false;
>   
>   	ret = hdmi_resources_init(hdata);
>   	if (ret) {
>   		ret = -EINVAL;
> +		DRM_ERROR("hdmi_resources_init failed\n");
>   		goto err_data;
>   	}
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		DRM_ERROR("failed to find registers\n");
> +		ret = -ENOENT;
> +		goto err_resource;
> +	}
>   
>   	hdata->regs = devm_request_and_ioremap(&pdev->dev, res);
>   	if (!hdata->regs) {
> @@ -2340,7 +2376,7 @@ static int __devinit hdmi_probe(struct platform_device *pdev)
>   
>   	hdata->external_irq = platform_get_irq_byname(pdev, "external_irq");
>   	if (hdata->external_irq < 0) {
> -		DRM_ERROR("failed to get platform irq\n");
> +		DRM_ERROR("failed to get platform external irq\n");
>   		ret = hdata->external_irq;
>   		goto err_hdmiphy;
>   	}
> @@ -2357,7 +2393,7 @@ static int __devinit hdmi_probe(struct platform_device *pdev)
>   			IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>   			"hdmi_external", drm_hdmi_ctx);
>   	if (ret) {
> -		DRM_ERROR("failed to register hdmi internal interrupt\n");
> +		DRM_ERROR("failed to register hdmi external interrupt\n");
>   		goto err_hdmiphy;
>   	}
>   
> @@ -2372,10 +2408,157 @@ static int __devinit hdmi_probe(struct platform_device *pdev)
>   		goto err_free_irq;
>   	}
>   
> -	/* register specific callbacks to common hdmi. */
> -	exynos_hdmi_ops_register(&hdmi_ops);
> +	return 0;
>   
> -	pm_runtime_enable(dev);
> +err_free_irq:
> +	free_irq(hdata->external_irq, drm_hdmi_ctx);
> +err_hdmiphy:
> +	i2c_del_driver(&hdmiphy_driver);
> +err_ddc:
> +	i2c_del_driver(&ddc_driver);
> +err_resource:
> +	hdmi_resources_cleanup(hdata);
> +err_data:
> +	return ret;
> +}
> +
> +static int __devinit hdmi_resources_init_exynos5(
> +			struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_drm_hdmi_context *drm_hdmi_ctx;
> +	struct hdmi_context *hdata;
> +	struct resource *res;
> +	unsigned int value;
> +	int ret;
> +	enum of_gpio_flags flags;
> +
> +	DRM_DEBUG_KMS("[%d][%s]\n", __LINE__, __func__);
> +
> +	drm_hdmi_ctx = devm_kzalloc(&pdev->dev, sizeof(*drm_hdmi_ctx),
> +								GFP_KERNEL);
> +	if (!drm_hdmi_ctx) {
> +		DRM_ERROR("failed to allocate common hdmi context.\n");
> +		return -ENOMEM;
> +	}
> +
> +	hdata = devm_kzalloc(&pdev->dev, sizeof(struct hdmi_context),
> +								GFP_KERNEL);
> +	if (!hdata) {
> +		DRM_ERROR("out of memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	mutex_init(&hdata->hdmi_mutex);
> +
> +	drm_hdmi_ctx->ctx = (void *)hdata;
> +	hdata->parent_ctx = (void *)drm_hdmi_ctx;
> +
> +	platform_set_drvdata(pdev, drm_hdmi_ctx);
> +
> +	if (!of_property_read_u32(pdev->dev.of_node,
> +				"v13_support", &value)) {
> +		hdata->is_v13 = (value == 0) ? false : true;
> +	} else {
> +		DRM_ERROR("no hdmi version property found\n");
> +		ret = -EINVAL;
> +		goto err_data;
> +	}
> +
> +	if (!of_find_property(pdev->dev.of_node,
> +				"hpd-gpio", &value)){
> +		DRM_ERROR("no hpd gpio property found\n");
> +		ret = -EINVAL;
> +		goto err_data;
> +	}
> +
> +	hdata->hpd_gpio = of_get_named_gpio_flags(pdev->dev.of_node,
> +				"hpd-gpio", 0, &flags);
> +
> +	if (!gpio_is_valid(hdata->hpd_gpio)) {
> +		DRM_ERROR("failed to get hpd gpio.");
> +		ret = -EINVAL;
> +		goto err_data;
> +	}
> +
> +	hdata->cfg_hpd = NULL;
> +	hdata->get_hpd = NULL;
> +	hdata->dev = dev;
> +	hdata->is_soc_exynos5 = true;
> +
> +	ret = hdmi_resources_init(hdata);
> +	if (ret) {
> +		ret = -EINVAL;
> +		DRM_ERROR("failed hdmi_resources_init.");
> +		goto err_data;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		DRM_ERROR("failed to find registers\n");
> +		ret = -ENOENT;
> +		goto err_resource;
> +	}
> +
> +	hdata->regs = devm_request_and_ioremap(&pdev->dev, res);
> +	if (!hdata->regs) {
> +		DRM_ERROR("failed to map registers\n");
> +		ret = -ENXIO;
> +		goto err_resource;
> +	}
> +
> +	/* DDC i2c driver */
> +	if (i2c_add_driver(&ddc_driver)) {
> +		DRM_ERROR("failed to register ddc i2c driver\n");
> +		ret = -ENOENT;
> +		goto err_resource;
> +	}
> +
> +	hdata->ddc_port = hdmi_ddc;
> +
> +	/* hdmiphy i2c driver */
> +	if (i2c_add_driver(&hdmiphy_driver)) {
> +		DRM_ERROR("failed to register hdmiphy i2c driver\n");
> +		ret = -ENOENT;
> +		goto err_ddc;
> +	}
> +
> +	hdata->hdmiphy_port = hdmi_hdmiphy;
> +
> +	hdata->external_irq = gpio_to_irq(hdata->hpd_gpio);
> +
> +	if (hdata->external_irq < 0) {
> +		DRM_ERROR("failed to get platform external irq\n");
> +		ret = hdata->external_irq;
> +		goto err_hdmiphy;
> +	}
> +
> +	hdata->internal_irq = platform_get_irq(pdev, 0);
> +
> +	if (hdata->internal_irq < 0) {
> +		DRM_ERROR("failed to get platform internal irq\n");
> +		ret = hdata->internal_irq;
> +		goto err_hdmiphy;
> +	}
> +
> +	ret = request_threaded_irq(hdata->external_irq, NULL,
> +			hdmi_external_irq_thread, IRQF_TRIGGER_RISING |
> +			IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +			"hdmi_external", drm_hdmi_ctx);
> +	if (ret) {
> +		DRM_ERROR("failed to register hdmi external interrupt\n");
> +		goto err_hdmiphy;
> +	}
> +
> +	hdmi_cfg_hpd(hdata, false);
> +
> +	ret = request_threaded_irq(hdata->internal_irq, NULL,
> +			hdmi_internal_irq_thread, IRQF_ONESHOT,
> +			"hdmi_internal", drm_hdmi_ctx);
> +	if (ret) {
> +		DRM_ERROR("failed to register hdmi internal interrupt\n");
> +		goto err_free_irq;
> +	}
>   
>   	return 0;
>   
> @@ -2391,6 +2574,41 @@ err_data:
>   	return ret;
>   }
>   
> +static int __devinit hdmi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_drm_hdmi_context *drm_hdmi_ctx;
> +	bool is_soc_exynos5 = false;
> +	int ret;
> +
> +	DRM_DEBUG_KMS("[%d][%s]\n", __LINE__, __func__);
> +
> +	if (pdev->dev.of_node &&
> +			of_device_is_compatible(pdev->dev.of_node,
> +			"samsung,exynos5-hdmi")) {
> +			is_soc_exynos5 = true;
> +	}
> +
> +	/* acquire resources: regs, irqs, clocks */
> +	if (is_soc_exynos5)
> +		ret = hdmi_resources_init_exynos5(pdev);
> +	else
> +		ret = hdmi_resources_init_exynos4(pdev);

I think it isn't good to split using is_soc_exynos5 because the exynos5
hdmi is almost same that of exynos4x12.

> +	if (ret)
> +		goto err_data;
> +
> +	drm_hdmi_ctx = platform_get_drvdata(pdev);
> +
> +	/* register specific callbacks to common hdmi. */
> +	exynos_hdmi_ops_register(&hdmi_ops);
> +
> +	pm_runtime_enable(dev);
> +
> +	return 0;
> +
> +err_data:	return ret;
> +}
> +
>   static int __devexit hdmi_remove(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> @@ -2444,12 +2662,32 @@ static int hdmi_resume(struct device *dev)
>   
>   static SIMPLE_DEV_PM_OPS(hdmi_pm_ops, hdmi_suspend, hdmi_resume);
>   
> +static struct platform_device_id hdmi_driver_types[] = {
> +	{
> +		.name		= "exynos5-hdmi",
> +	}, {
> +		.name		= "exynos4-hdmi",
> +	}, {
> +		/* end node */
> +	}
> +};
> +
> +static struct of_device_id hdmi_match_types[] = {
> +	{
> +		.compatible = "samsung,exynos5-hdmi",
> +	}, {
> +		/* end node */
> +	}
> +};
> +
>   struct platform_driver hdmi_driver = {
>   	.probe		= hdmi_probe,
>   	.remove		= __devexit_p(hdmi_remove),
> +	.id_table	= hdmi_driver_types,
>   	.driver		= {
> -		.name	= "exynos4-hdmi",
> +		.name	= "exynos-hdmi",
>   		.owner	= THIS_MODULE,
>   		.pm	= &hdmi_pm_ops,
> +		.of_match_table = hdmi_match_types,
>   	},
>   };

Thanks.


More information about the dri-devel mailing list