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

2012년 9월 13일 목요일에 Joonyoung Shimjy0922.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. _______________________________________________

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

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.

Hi, Rahul.

On 09/12/2012 09:08 PM, Rahul Sharma wrote:

exynos-drm-hdmi need context pointers from hdmi and mixer. These pointers were expected from the plf data. Cleaned this dependency

What does plf data mean?

by exporting i/f which are called by hdmi, mixer driver probes for setting their context.

It is reasonable to me. This can remove struct exynos_drm_common_hdmi_pd.

Thanks.

Signed-off-by: Rahul Sharma rahul.sharma@samsung.com

drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 47 +++++++++++++++-------------- drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 2 + drivers/gpu/drm/exynos/exynos_hdmi.c | 3 ++ drivers/gpu/drm/exynos/exynos_mixer.c | 3 ++ 4 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c index 0584132..4c8d933 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c @@ -29,6 +29,11 @@ #define get_ctx_from_subdrv(subdrv) container_of(subdrv,\ struct drm_hdmi_context, subdrv);

+/* Common hdmi subdrv needs to access the hdmi and mixer though context. +* These should be initialied by the repective drivers */ +static struct exynos_drm_hdmi_context *hdmi_ctx; +static struct exynos_drm_hdmi_context *mixer_ctx;

• /* these callback points shoud be set by specific drivers. */ static struct exynos_hdmi_ops *hdmi_ops; static struct exynos_mixer_ops *mixer_ops;

@@ -41,6 +46,18 @@ struct drm_hdmi_context { bool enabled[MIXER_WIN_NR]; };

+void exynos_hdmi_drv_attach(struct exynos_drm_hdmi_context *ctx) +{

• if (ctx)

• hdmi_ctx = ctx;
  

+}

+void exynos_mixer_drv_attach(struct exynos_drm_hdmi_context *ctx) +{

• if (ctx)

• mixer_ctx = ctx;
  

+}

• void exynos_hdmi_ops_register(struct exynos_hdmi_ops *ops) { DRM_DEBUG_KMS("%s\n", __FILE__);

@@ -303,46 +320,30 @@ static int hdmi_subdrv_probe(struct drm_device *drm_dev, { struct exynos_drm_subdrv *subdrv = to_subdrv(dev); struct drm_hdmi_context *ctx;

• struct platform_device *pdev = to_platform_device(dev);

• struct exynos_drm_common_hdmi_pd *pd;

  DRM_DEBUG_KMS("%s\n", __FILE__);

• pd = pdev->dev.platform_data;

• if (!pd) {

• DRM_DEBUG_KMS("platform data is null.\n");
  

• return -EFAULT;
  

• }

• if (!pd->hdmi_dev) {

• if (!hdmi_ctx) { DRM_DEBUG_KMS("hdmi device is null.\n"); return -EFAULT; }

• if (!pd->mixer_dev) {

• if (!mixer_ctx) { DRM_DEBUG_KMS("mixer device is null.\n"); return -EFAULT; }

  ctx = get_ctx_from_subdrv(subdrv);

• ctx->hdmi_ctx = (struct exynos_drm_hdmi_context *)

• 		to_context(pd->hdmi_dev);
  

• if (!ctx->hdmi_ctx) {

• DRM_DEBUG_KMS("hdmi context is null.\n");
  

• if (!ctx) {

• DRM_DEBUG_KMS("context is null.\n");
  

  return -EFAULT; }

• ctx->hdmi_ctx->drm_dev = drm_dev;
• ctx->mixer_ctx = (struct exynos_drm_hdmi_context *)

• 		to_context(pd->mixer_dev);
  

• if (!ctx->mixer_ctx) {

• DRM_DEBUG_KMS("mixer context is null.\n");
  

• return -EFAULT;
  

• }

• ctx->hdmi_ctx = hdmi_ctx;

• ctx->mixer_ctx = mixer_ctx;

• ctx->hdmi_ctx->drm_dev = drm_dev; ctx->mixer_ctx->drm_dev = drm_dev;

  return 0;

diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h index d9f9e9f..2da5ffd 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h @@ -73,6 +73,8 @@ struct exynos_mixer_ops { void (*win_disable)(void *ctx, int zpos); };

+void exynos_hdmi_drv_attach(struct exynos_drm_hdmi_context *ctx); +void exynos_mixer_drv_attach(struct exynos_drm_hdmi_context *ctx); void exynos_hdmi_ops_register(struct exynos_hdmi_ops *ops); void exynos_mixer_ops_register(struct exynos_mixer_ops *ops); #endif diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 5236256..82ee810 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2599,6 +2599,9 @@ static int __devinit hdmi_probe(struct platform_device *pdev)

drm_hdmi_ctx = platform_get_drvdata(pdev);

• /* Attach HDMI Driver to common hdmi. */
• exynos_hdmi_drv_attach(drm_hdmi_ctx);
• /* register specific callbacks to common hdmi. */ exynos_hdmi_ops_register(&hdmi_ops);

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 7d04a40..f9e26f2 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1165,6 +1165,9 @@ static int __devinit mixer_probe(struct platform_device *pdev) if (ret) goto fail;

• /* attach mixer driver to common hdmi. */
• exynos_mixer_drv_attach(drm_hdmi_ctx);
• /* register specific callback point to common hdmi. */ exynos_mixer_ops_register(&mixer_ops);