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

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