[PATCH 0/4] drm/exynos/dsi: remove global display variable

[PATCH 0/4] drm/exynos/dsi: remove global display variable

[Andrzej Hajda]

Hi Inki,

Many Exynos DRM drivers uses global variables to represent associated devices
in Exynos DRM internal framework. It is quite confusing, it adds data duplication
and finally it does not allow to handle more than one device in system.
It seems better to embed such structures in private context of the device.

The patchset is based on exynos_drm_next plus my patch [1]:
'drm/exynos: remove explicit encoder/connector de-initialization'.

If the patchset is OK for you I can prepare similar patches for other Exynos DRM components.

[1]: https://lkml.org/lkml/2014/9/22/148

Regards
Andrzej


Andrzej Hajda (4):
  drm/exynos/dsi: remove global variable exynos_dsi_display
  drm/exynos/dsi: simplify device pointer evaluation
  drm/exynos/dsi: remove redundant encoder field
  drm/exynos/dsi: stop using display->ctx pointer

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 96 ++++++++++++++++-----------------
 1 file changed, 47 insertions(+), 49 deletions(-)

-- 
1.9.1

[PATCH 1/4] drm/exynos/dsi: remove global variable exynos_dsi_display

[Andrzej Hajda]

exynos_dsi_display is used by internal Exynos DRM framework for
representing pair encoder->connecter. As it should be mapped 1:1 to dsi
private context it seems more reasonable to embed it directly
in that context. As a result further code simplification will be possible.
Moreover it will be possible to handle multiple DSI devices in the system.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 38 ++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index acf7e9e..84e3808 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -268,6 +268,7 @@ struct exynos_dsi_driver_data {
 };
 
 struct exynos_dsi {
+	struct exynos_drm_display display;
 	struct mipi_dsi_host dsi_host;
 	struct drm_connector connector;
 	struct drm_encoder *encoder;
@@ -1531,10 +1532,6 @@ static struct exynos_drm_display_ops exynos_dsi_display_ops = {
 	.dpms = exynos_dsi_dpms
 };
 
-static struct exynos_drm_display exynos_dsi_display = {
-	.type = EXYNOS_DISPLAY_TYPE_LCD,
-	.ops = &exynos_dsi_display_ops,
-};
 MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);
 
 /* of_* functions will be removed after merge of of_graph patches */
@@ -1640,28 +1637,28 @@ end:
 static int exynos_dsi_bind(struct device *dev, struct device *master,
 				void *data)
 {
+	struct exynos_drm_display *display = dev_get_drvdata(dev);
+	struct exynos_dsi *dsi = display->ctx;
 	struct drm_device *drm_dev = data;
-	struct exynos_dsi *dsi;
 	int ret;
 
-	ret = exynos_drm_create_enc_conn(drm_dev, &exynos_dsi_display);
+	ret = exynos_drm_create_enc_conn(drm_dev, display);
 	if (ret) {
 		DRM_ERROR("Encoder create [%d] failed with %d\n",
-				exynos_dsi_display.type, ret);
+			  display->type, ret);
 		return ret;
 	}
 
-	dsi = exynos_dsi_display.ctx;
-
 	return mipi_dsi_host_register(&dsi->dsi_host);
 }
 
 static void exynos_dsi_unbind(struct device *dev, struct device *master,
 				void *data)
 {
-	struct exynos_dsi *dsi = exynos_dsi_display.ctx;
+	struct exynos_drm_display *display = dev_get_drvdata(dev);
+	struct exynos_dsi *dsi = display->ctx;
 
-	exynos_dsi_dpms(&exynos_dsi_display, DRM_MODE_DPMS_OFF);
+	exynos_dsi_dpms(display, DRM_MODE_DPMS_OFF);
 
 	mipi_dsi_host_unregister(&dsi->dsi_host);
 }
@@ -1673,22 +1670,23 @@ static const struct component_ops exynos_dsi_component_ops = {
 
 static int exynos_dsi_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct exynos_dsi *dsi;
 	int ret;
 
-	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
-					exynos_dsi_display.type);
+	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
+	if (!dsi)
+		return -ENOMEM;
+
+	dsi->display.type = EXYNOS_DISPLAY_TYPE_LCD;
+	dsi->display.ops = &exynos_dsi_display_ops;
+
+	ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
+				       dsi->display.type);
 	if (ret)
 		return ret;
 
-	dsi = devm_kzalloc(&pdev->dev, sizeof(*dsi), GFP_KERNEL);
-	if (!dsi) {
-		dev_err(&pdev->dev, "failed to allocate dsi object.\n");
-		ret = -ENOMEM;
-		goto err_del_component;
-	}
-
 	/* To be checked as invalid one */
 	dsi->te_gpio = -ENOENT;
 
-- 
1.9.1

[PATCH 2/4] drm/exynos/dsi: simplify device pointer evaluation

[Andrzej Hajda]

The patch replaces multiple evaluation of device address
with local variable.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 40 ++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 84e3808..39ec450 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1695,9 +1695,9 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&dsi->transfer_list);
 
 	dsi->dsi_host.ops = &exynos_dsi_ops;
-	dsi->dsi_host.dev = &pdev->dev;
+	dsi->dsi_host.dev = dev;
 
-	dsi->dev = &pdev->dev;
+	dsi->dev = dev;
 	dsi->driver_data = exynos_dsi_get_driver_data(pdev);
 
 	ret = exynos_dsi_parse_dt(dsi);
@@ -1706,70 +1706,70 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 
 	dsi->supplies[0].supply = "vddcore";
 	dsi->supplies[1].supply = "vddio";
-	ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(dsi->supplies),
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dsi->supplies),
 				      dsi->supplies);
 	if (ret) {
-		dev_info(&pdev->dev, "failed to get regulators: %d\n", ret);
+		dev_info(dev, "failed to get regulators: %d\n", ret);
 		return -EPROBE_DEFER;
 	}
 
-	dsi->pll_clk = devm_clk_get(&pdev->dev, "pll_clk");
+	dsi->pll_clk = devm_clk_get(dev, "pll_clk");
 	if (IS_ERR(dsi->pll_clk)) {
-		dev_info(&pdev->dev, "failed to get dsi pll input clock\n");
+		dev_info(dev, "failed to get dsi pll input clock\n");
 		ret = PTR_ERR(dsi->pll_clk);
 		goto err_del_component;
 	}
 
-	dsi->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
+	dsi->bus_clk = devm_clk_get(dev, "bus_clk");
 	if (IS_ERR(dsi->bus_clk)) {
-		dev_info(&pdev->dev, "failed to get dsi bus clock\n");
+		dev_info(dev, "failed to get dsi bus clock\n");
 		ret = PTR_ERR(dsi->bus_clk);
 		goto err_del_component;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	dsi->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	dsi->reg_base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(dsi->reg_base)) {
-		dev_err(&pdev->dev, "failed to remap io region\n");
+		dev_err(dev, "failed to remap io region\n");
 		ret = PTR_ERR(dsi->reg_base);
 		goto err_del_component;
 	}
 
-	dsi->phy = devm_phy_get(&pdev->dev, "dsim");
+	dsi->phy = devm_phy_get(dev, "dsim");
 	if (IS_ERR(dsi->phy)) {
-		dev_info(&pdev->dev, "failed to get dsim phy\n");
+		dev_info(dev, "failed to get dsim phy\n");
 		ret = PTR_ERR(dsi->phy);
 		goto err_del_component;
 	}
 
 	dsi->irq = platform_get_irq(pdev, 0);
 	if (dsi->irq < 0) {
-		dev_err(&pdev->dev, "failed to request dsi irq resource\n");
+		dev_err(dev, "failed to request dsi irq resource\n");
 		ret = dsi->irq;
 		goto err_del_component;
 	}
 
 	irq_set_status_flags(dsi->irq, IRQ_NOAUTOEN);
-	ret = devm_request_threaded_irq(&pdev->dev, dsi->irq, NULL,
+	ret = devm_request_threaded_irq(dev, dsi->irq, NULL,
 					exynos_dsi_irq, IRQF_ONESHOT,
-					dev_name(&pdev->dev), dsi);
+					dev_name(dev), dsi);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to request dsi irq\n");
+		dev_err(dev, "failed to request dsi irq\n");
 		goto err_del_component;
 	}
 
-	exynos_dsi_display.ctx = dsi;
+	dsi->display.ctx = dsi;
 
-	platform_set_drvdata(pdev, &exynos_dsi_display);
+	platform_set_drvdata(pdev, &dsi->display);
 
-	ret = component_add(&pdev->dev, &exynos_dsi_component_ops);
+	ret = component_add(dev, &exynos_dsi_component_ops);
 	if (ret)
 		goto err_del_component;
 
 	return ret;
 
 err_del_component:
-	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
+	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
 	return ret;
 }
 
-- 
1.9.1

[PATCH 3/4] drm/exynos/dsi: remove redundant encoder field

[Andrzej Hajda]

The patch removes redundant encoder field from private DSI context.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 39ec450..634904a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -271,7 +271,6 @@ struct exynos_dsi {
 	struct exynos_drm_display display;
 	struct mipi_dsi_host dsi_host;
 	struct drm_connector connector;
-	struct drm_encoder *encoder;
 	struct device_node *panel_node;
 	struct drm_panel *panel;
 	struct device *dev;
@@ -1105,7 +1104,7 @@ static irqreturn_t exynos_dsi_irq(int irq, void *dev_id)
 static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
 {
 	struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
-	struct drm_encoder *encoder = dsi->encoder;
+	struct drm_encoder *encoder = dsi->display.encoder;
 
 	if (dsi->state & DSIM_STATE_ENABLED)
 		exynos_drm_crtc_te_handler(encoder->crtc);
@@ -1475,7 +1474,7 @@ exynos_dsi_best_encoder(struct drm_connector *connector)
 {
 	struct exynos_dsi *dsi = connector_to_dsi(connector);
 
-	return dsi->encoder;
+	return dsi->display.encoder;
 }
 
 static struct drm_connector_helper_funcs exynos_dsi_connector_helper_funcs = {
@@ -1491,8 +1490,6 @@ static int exynos_dsi_create_connector(struct exynos_drm_display *display,
 	struct drm_connector *connector = &dsi->connector;
 	int ret;
 
-	dsi->encoder = encoder;
-
 	connector->polled = DRM_CONNECTOR_POLL_HPD;
 
 	ret = drm_connector_init(encoder->dev, connector,
-- 
1.9.1

[PATCH 4/4] drm/exynos/dsi: stop using display->ctx pointer

[Andrzej Hajda]

The patch replaces accesses to display->ctx pointer by container_of
construct. It will allow to remove ctx field in the future.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 634904a..abdc3d1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -304,6 +304,11 @@ struct exynos_dsi {
 #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
 #define connector_to_dsi(c) container_of(c, struct exynos_dsi, connector)
 
+static inline struct exynos_dsi *display_to_dsi(struct exynos_drm_display *d)
+{
+	return container_of(d, struct exynos_dsi, display);
+}
+
 static struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
 	.plltmr_reg = 0x50,
 	.has_freqband = 1,
@@ -1397,7 +1402,7 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
 
 static void exynos_dsi_dpms(struct exynos_drm_display *display, int mode)
 {
-	struct exynos_dsi *dsi = display->ctx;
+	struct exynos_dsi *dsi = display_to_dsi(display);
 
 	if (dsi->panel) {
 		switch (mode) {
@@ -1486,7 +1491,7 @@ static struct drm_connector_helper_funcs exynos_dsi_connector_helper_funcs = {
 static int exynos_dsi_create_connector(struct exynos_drm_display *display,
 				       struct drm_encoder *encoder)
 {
-	struct exynos_dsi *dsi = display->ctx;
+	struct exynos_dsi *dsi = display_to_dsi(display);
 	struct drm_connector *connector = &dsi->connector;
 	int ret;
 
@@ -1510,7 +1515,7 @@ static int exynos_dsi_create_connector(struct exynos_drm_display *display,
 static void exynos_dsi_mode_set(struct exynos_drm_display *display,
 			 struct drm_display_mode *mode)
 {
-	struct exynos_dsi *dsi = display->ctx;
+	struct exynos_dsi *dsi = display_to_dsi(display);
 	struct videomode *vm = &dsi->vm;
 
 	vm->hactive = mode->hdisplay;
@@ -1635,7 +1640,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 				void *data)
 {
 	struct exynos_drm_display *display = dev_get_drvdata(dev);
-	struct exynos_dsi *dsi = display->ctx;
+	struct exynos_dsi *dsi = display_to_dsi(display);
 	struct drm_device *drm_dev = data;
 	int ret;
 
@@ -1653,7 +1658,7 @@ static void exynos_dsi_unbind(struct device *dev, struct device *master,
 				void *data)
 {
 	struct exynos_drm_display *display = dev_get_drvdata(dev);
-	struct exynos_dsi *dsi = display->ctx;
+	struct exynos_dsi *dsi = display_to_dsi(display);
 
 	exynos_dsi_dpms(display, DRM_MODE_DPMS_OFF);
 
@@ -1755,8 +1760,6 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 		goto err_del_component;
 	}
 
-	dsi->display.ctx = dsi;
-
 	platform_set_drvdata(pdev, &dsi->display);
 
 	ret = component_add(dev, &exynos_dsi_component_ops);
-- 
1.9.1

Re: [PATCH 0/4] drm/exynos/dsi: remove global display variable

[Andrzej Hajda]

On 10/07/2014 02:01 PM, Andrzej Hajda wrote:
> Hi Inki,
>
> Many Exynos DRM drivers uses global variables to represent associated devices
> in Exynos DRM internal framework. It is quite confusing, it adds data duplication
> and finally it does not allow to handle more than one device in system.
> It seems better to embed such structures in private context of the device.

Gently ping.

Regards
Andrzej

>
> The patchset is based on exynos_drm_next plus my patch [1]:
> 'drm/exynos: remove explicit encoder/connector de-initialization'.
>
> If the patchset is OK for you I can prepare similar patches for other Exynos DRM components.
>
> [1]: https://lkml.org/lkml/2014/9/22/148
>
> Regards
> Andrzej
>
>
> Andrzej Hajda (4):
>   drm/exynos/dsi: remove global variable exynos_dsi_display
>   drm/exynos/dsi: simplify device pointer evaluation
>   drm/exynos/dsi: remove redundant encoder field
>   drm/exynos/dsi: stop using display->ctx pointer
>
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 96 ++++++++++++++++-----------------
>  1 file changed, 47 insertions(+), 49 deletions(-)
>

Re: [PATCH 0/4] drm/exynos/dsi: remove global display variable

[Inki Dae]

On 2014년 10월 30일 21:39, Andrzej Hajda wrote:
> On 10/07/2014 02:01 PM, Andrzej Hajda wrote:
>> Hi Inki,
>>
>> Many Exynos DRM drivers uses global variables to represent associated devices
>> in Exynos DRM internal framework. It is quite confusing, it adds data duplication
>> and finally it does not allow to handle more than one device in system.
>> It seems better to embed such structures in private context of the device.
> 
> Gently ping.

This patch series already is in exynos-drm-next-todo branch. I will move
it to exynos-drm-next after checking.

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
>>
>> The patchset is based on exynos_drm_next plus my patch [1]:
>> 'drm/exynos: remove explicit encoder/connector de-initialization'.
>>
>> If the patchset is OK for you I can prepare similar patches for other Exynos DRM components.
>>
>> [1]: https://lkml.org/lkml/2014/9/22/148
>>
>> Regards
>> Andrzej
>>
>>
>> Andrzej Hajda (4):
>>   drm/exynos/dsi: remove global variable exynos_dsi_display
>>   drm/exynos/dsi: simplify device pointer evaluation
>>   drm/exynos/dsi: remove redundant encoder field
>>   drm/exynos/dsi: stop using display->ctx pointer
>>
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 96 ++++++++++++++++-----------------
>>  1 file changed, 47 insertions(+), 49 deletions(-)
>>
> 
> 

Re: [PATCH 0/4] drm/exynos/dsi: remove global display variable

[Inki Dae]

On 2014년 10월 07일 21:01, Andrzej Hajda wrote:
> Hi Inki,
> 
> Many Exynos DRM drivers uses global variables to represent associated devices
> in Exynos DRM internal framework. It is quite confusing, it adds data duplication
> and finally it does not allow to handle more than one device in system.
> It seems better to embed such structures in private context of the device.
> 
> The patchset is based on exynos_drm_next plus my patch [1]:
> 'drm/exynos: remove explicit encoder/connector de-initialization'.
> 
> If the patchset is OK for you I can prepare similar patches for other Exynos DRM components.

Sorry for late. Applied. Can you prepare similar patches for other? If
so, I'd happy.

Thanks,
Inki Dae

> 
> [1]: https://lkml.org/lkml/2014/9/22/148
> 
> Regards
> Andrzej
> 
> 
> Andrzej Hajda (4):
>   drm/exynos/dsi: remove global variable exynos_dsi_display
>   drm/exynos/dsi: simplify device pointer evaluation
>   drm/exynos/dsi: remove redundant encoder field
>   drm/exynos/dsi: stop using display->ctx pointer
> 
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 96 ++++++++++++++++-----------------
>  1 file changed, 47 insertions(+), 49 deletions(-)
> 

Re: [PATCH 0/4] drm/exynos/dsi: remove global display variable

[Andrzej Hajda]

On 11/13/2014 09:50 AM, Inki Dae wrote:
> On 2014년 10월 07일 21:01, Andrzej Hajda wrote:
>> Hi Inki,
>>
>> Many Exynos DRM drivers uses global variables to represent associated devices
>> in Exynos DRM internal framework. It is quite confusing, it adds data duplication
>> and finally it does not allow to handle more than one device in system.
>> It seems better to embed such structures in private context of the device.
>>
>> The patchset is based on exynos_drm_next plus my patch [1]:
>> 'drm/exynos: remove explicit encoder/connector de-initialization'.
>>
>> If the patchset is OK for you I can prepare similar patches for other Exynos DRM components.
> Sorry for late. Applied. Can you prepare similar patches for other? If
> so, I'd happy.

Beginning of the next week is OK?

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
>> [1]: https://lkml.org/lkml/2014/9/22/148
>>
>> Regards
>> Andrzej
>>
>>
>> Andrzej Hajda (4):
>>   drm/exynos/dsi: remove global variable exynos_dsi_display
>>   drm/exynos/dsi: simplify device pointer evaluation
>>   drm/exynos/dsi: remove redundant encoder field
>>   drm/exynos/dsi: stop using display->ctx pointer
>>
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 96 ++++++++++++++++-----------------
>>  1 file changed, 47 insertions(+), 49 deletions(-)
>>
>