dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc
@ 2021-01-12  8:43 Daniel Vetter
  2021-01-12  8:43 ` [PATCH 02/15] drm/arc: Stop using drm_device->dev_private Daniel Vetter
                   ` (14 more replies)
  0 siblings, 15 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, Daniel Vetter,
	tiantao6, Eugeniy Paltsev

- Need to embedded the drm_device, but for now we keep the usual
  pointer chasing.
- No more devm_kzalloc, which fixes a lifetime issues on driver
  remove.
- No more drm_dev_put, that's done by devm_ now.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu.h     |  1 +
 drivers/gpu/drm/arc/arcpgu_drv.c | 33 +++++++++++++-------------------
 2 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index 6aac44b953ad..cd9e932f501e 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -9,6 +9,7 @@
 #define _ARCPGU_H_
 
 struct arcpgu_drm_private {
+	struct drm_device	drm;
 	void __iomem		*regs;
 	struct clk		*clk;
 	struct drm_framebuffer	*fb;
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index f164818ec477..68eb4a31c54b 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -42,18 +42,14 @@ static void arcpgu_setup_mode_config(struct drm_device *drm)
 
 DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
 
-static int arcpgu_load(struct drm_device *drm)
+static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 {
-	struct platform_device *pdev = to_platform_device(drm->dev);
-	struct arcpgu_drm_private *arcpgu;
+	struct platform_device *pdev = to_platform_device(arcpgu->drm.dev);
 	struct device_node *encoder_node = NULL, *endpoint_node = NULL;
+	struct drm_device *drm = &arcpgu->drm;
 	struct resource *res;
 	int ret;
 
-	arcpgu = devm_kzalloc(&pdev->dev, sizeof(*arcpgu), GFP_KERNEL);
-	if (arcpgu == NULL)
-		return -ENOMEM;
-
 	drm->dev_private = arcpgu;
 
 	arcpgu->clk = devm_clk_get(drm->dev, "pxlclk");
@@ -162,30 +158,28 @@ static struct drm_driver arcpgu_drm_driver = {
 
 static int arcpgu_probe(struct platform_device *pdev)
 {
-	struct drm_device *drm;
+	struct arcpgu_drm_private *arcpgu;
 	int ret;
 
-	drm = drm_dev_alloc(&arcpgu_drm_driver, &pdev->dev);
-	if (IS_ERR(drm))
-		return PTR_ERR(drm);
+	arcpgu = devm_drm_dev_alloc(&pdev->dev, &arcpgu_drm_driver,
+				    struct arcpgu_drm_private, drm);
+	if (IS_ERR(arcpgu))
+		return PTR_ERR(arcpgu);
 
-	ret = arcpgu_load(drm);
+	ret = arcpgu_load(arcpgu);
 	if (ret)
-		goto err_unref;
+		return ret;
 
-	ret = drm_dev_register(drm, 0);
+	ret = drm_dev_register(&arcpgu->drm, 0);
 	if (ret)
 		goto err_unload;
 
-	drm_fbdev_generic_setup(drm, 16);
+	drm_fbdev_generic_setup(&arcpgu->drm, 16);
 
 	return 0;
 
 err_unload:
-	arcpgu_unload(drm);
-
-err_unref:
-	drm_dev_put(drm);
+	arcpgu_unload(&arcpgu->drm);
 
 	return ret;
 }
@@ -196,7 +190,6 @@ static int arcpgu_remove(struct platform_device *pdev)
 
 	drm_dev_unregister(drm);
 	arcpgu_unload(drm);
-	drm_dev_put(drm);
 
 	return 0;
 }
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 02/15] drm/arc: Stop using drm_device->dev_private
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 03/15] drm/arc: Delete arcpgu_priv->fb Daniel Vetter
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, tiantao6, Alexey Brodkin, Eugeniy Paltsev, Daniel Vetter

Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.

Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu.h      | 2 ++
 drivers/gpu/drm/arc/arcpgu_crtc.c | 4 ++--
 drivers/gpu/drm/arc/arcpgu_drv.c  | 4 +---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index cd9e932f501e..87821c91a00c 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -17,6 +17,8 @@ struct arcpgu_drm_private {
 	struct drm_plane	*plane;
 };
 
+#define dev_to_arcpgu(x) container_of(x, struct arcpgu_drm_private, drm)
+
 #define crtc_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, crtc)
 
 static inline void arc_pgu_write(struct arcpgu_drm_private *arcpgu,
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 042d7b54a6de..c32038b6366f 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -178,7 +178,7 @@ static const struct drm_plane_funcs arc_pgu_plane_funcs = {
 
 static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm)
 {
-	struct arcpgu_drm_private *arcpgu = drm->dev_private;
+	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
 	struct drm_plane *plane = NULL;
 	int ret;
 
@@ -202,7 +202,7 @@ static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm)
 
 int arc_pgu_setup_crtc(struct drm_device *drm)
 {
-	struct arcpgu_drm_private *arcpgu = drm->dev_private;
+	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
 	struct drm_plane *primary;
 	int ret;
 
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 68eb4a31c54b..c6a8deb56b0f 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -50,8 +50,6 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 	struct resource *res;
 	int ret;
 
-	drm->dev_private = arcpgu;
-
 	arcpgu->clk = devm_clk_get(drm->dev, "pxlclk");
 	if (IS_ERR(arcpgu->clk))
 		return PTR_ERR(arcpgu->clk);
@@ -120,7 +118,7 @@ static int arcpgu_show_pxlclock(struct seq_file *m, void *arg)
 {
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *drm = node->minor->dev;
-	struct arcpgu_drm_private *arcpgu = drm->dev_private;
+	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
 	unsigned long clkrate = clk_get_rate(arcpgu->clk);
 	unsigned long mode_clock = arcpgu->crtc.mode.crtc_clock * 1000;
 
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 03/15] drm/arc: Delete arcpgu_priv->fb
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
  2021-01-12  8:43 ` [PATCH 02/15] drm/arc: Stop using drm_device->dev_private Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 04/15] drm/arc: Embedded a drm_simple_display_pipe Daniel Vetter
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development; +Cc: tiantao6, Alexey Brodkin, Eugeniy Paltsev, Daniel Vetter

Leftover from the conversion to the generic fbdev emulation.

Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index 87821c91a00c..ed77dd5dd5cb 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -12,7 +12,6 @@ struct arcpgu_drm_private {
 	struct drm_device	drm;
 	void __iomem		*regs;
 	struct clk		*clk;
-	struct drm_framebuffer	*fb;
 	struct drm_crtc		crtc;
 	struct drm_plane	*plane;
 };
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 04/15] drm/arc: Embedded a drm_simple_display_pipe
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
  2021-01-12  8:43 ` [PATCH 02/15] drm/arc: Stop using drm_device->dev_private Daniel Vetter
  2021-01-12  8:43 ` [PATCH 03/15] drm/arc: Delete arcpgu_priv->fb Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 05/15] drm/arc: Embedd a drm_connector for sim case Daniel Vetter
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, Daniel Vetter,
	tiantao6, Eugeniy Paltsev

This is a prep step to convert arc over to the simple kms helpers, for
now we just use this as an embedding container to drop all the various
allocations. Big change is the removal of the various devm_kzalloc,
which have the wrong lifetimes anyway.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu.h      | 7 ++++---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 9 +++------
 drivers/gpu/drm/arc/arcpgu_drv.c  | 2 +-
 drivers/gpu/drm/arc/arcpgu_hdmi.c | 5 ++---
 drivers/gpu/drm/arc/arcpgu_sim.c  | 5 ++---
 5 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index ed77dd5dd5cb..52afd638a4d2 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -8,17 +8,18 @@
 #ifndef _ARCPGU_H_
 #define _ARCPGU_H_
 
+#include <drm/drm_simple_kms_helper.h>
+
 struct arcpgu_drm_private {
 	struct drm_device	drm;
 	void __iomem		*regs;
 	struct clk		*clk;
-	struct drm_crtc		crtc;
-	struct drm_plane	*plane;
+	struct drm_simple_display_pipe pipe;
 };
 
 #define dev_to_arcpgu(x) container_of(x, struct arcpgu_drm_private, drm)
 
-#define crtc_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, crtc)
+#define crtc_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, pipe.crtc)
 
 static inline void arc_pgu_write(struct arcpgu_drm_private *arcpgu,
 				 unsigned int reg, u32 value)
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index c32038b6366f..31a85c703307 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -182,9 +182,7 @@ static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm)
 	struct drm_plane *plane = NULL;
 	int ret;
 
-	plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
-	if (!plane)
-		return ERR_PTR(-ENOMEM);
+	plane = &arcpgu->pipe.plane;
 
 	ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs,
 				       arc_pgu_supported_formats,
@@ -195,7 +193,6 @@ static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm)
 		return ERR_PTR(ret);
 
 	drm_plane_helper_add(plane, &arc_pgu_plane_helper_funcs);
-	arcpgu->plane = plane;
 
 	return plane;
 }
@@ -210,13 +207,13 @@ int arc_pgu_setup_crtc(struct drm_device *drm)
 	if (IS_ERR(primary))
 		return PTR_ERR(primary);
 
-	ret = drm_crtc_init_with_planes(drm, &arcpgu->crtc, primary, NULL,
+	ret = drm_crtc_init_with_planes(drm, &arcpgu->pipe.crtc, primary, NULL,
 					&arc_pgu_crtc_funcs, NULL);
 	if (ret) {
 		arc_pgu_plane_destroy(primary);
 		return ret;
 	}
 
-	drm_crtc_helper_add(&arcpgu->crtc, &arc_pgu_crtc_helper_funcs);
+	drm_crtc_helper_add(&arcpgu->pipe.crtc, &arc_pgu_crtc_helper_funcs);
 	return 0;
 }
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index c6a8deb56b0f..9020352816fa 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -120,7 +120,7 @@ static int arcpgu_show_pxlclock(struct seq_file *m, void *arg)
 	struct drm_device *drm = node->minor->dev;
 	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
 	unsigned long clkrate = clk_get_rate(arcpgu->clk);
-	unsigned long mode_clock = arcpgu->crtc.mode.crtc_clock * 1000;
+	unsigned long mode_clock = arcpgu->pipe.crtc.mode.crtc_clock * 1000;
 
 	seq_printf(m, "hw  : %lu\n", clkrate);
 	seq_printf(m, "mode: %lu\n", mode_clock);
diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index 52839934f2fb..dbad2c9237fe 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -18,14 +18,13 @@ static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
 
 int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
 {
+	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
 	struct drm_encoder *encoder;
 	struct drm_bridge *bridge;
 
 	int ret = 0;
 
-	encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL);
-	if (encoder == NULL)
-		return -ENOMEM;
+	encoder = &arcpgu->pipe.encoder;
 
 	/* Locate drm bridge from the hdmi encoder DT node */
 	bridge = of_drm_find_bridge(np);
diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
index 37d961668dfe..134afb9fa625 100644
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ b/drivers/gpu/drm/arc/arcpgu_sim.c
@@ -56,14 +56,13 @@ static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
 
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 {
+	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
 	struct arcpgu_drm_connector *arcpgu_connector;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
 	int ret;
 
-	encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL);
-	if (encoder == NULL)
-		return -ENOMEM;
+	encoder = &arcpgu->pipe.encoder;
 
 	encoder->possible_crtcs = 1;
 	encoder->possible_clones = 0;
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 05/15] drm/arc: Embedd a drm_connector for sim case
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (2 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 04/15] drm/arc: Embedded a drm_simple_display_pipe Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 06/15] drm/arc: Drop surplus connector registration Daniel Vetter
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, Daniel Vetter,
	tiantao6, Eugeniy Paltsev

Removes the last devm_kzalloc, which means we're now prepared to use
drmm_mode_config_cleanup!

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu.h     |  1 +
 drivers/gpu/drm/arc/arcpgu_sim.c | 14 +-------------
 2 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index 52afd638a4d2..c52cdd2274e1 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -15,6 +15,7 @@ struct arcpgu_drm_private {
 	void __iomem		*regs;
 	struct clk		*clk;
 	struct drm_simple_display_pipe pipe;
+	struct drm_connector	sim_conn;
 };
 
 #define dev_to_arcpgu(x) container_of(x, struct arcpgu_drm_private, drm)
diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
index 134afb9fa625..e42fe5d05a3d 100644
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ b/drivers/gpu/drm/arc/arcpgu_sim.c
@@ -18,10 +18,6 @@
 #define YRES_MAX	8192
 
 
-struct arcpgu_drm_connector {
-	struct drm_connector connector;
-};
-
 static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
 {
 	int count;
@@ -57,7 +53,6 @@ static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 {
 	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
-	struct arcpgu_drm_connector *arcpgu_connector;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
 	int ret;
@@ -72,14 +67,7 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 	if (ret)
 		return ret;
 
-	arcpgu_connector = devm_kzalloc(drm->dev, sizeof(*arcpgu_connector),
-					GFP_KERNEL);
-	if (!arcpgu_connector) {
-		ret = -ENOMEM;
-		goto error_encoder_cleanup;
-	}
-
-	connector = &arcpgu_connector->connector;
+	connector = &arcpgu->sim_conn;
 	drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs);
 
 	ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs,
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 06/15] drm/arc: Drop surplus connector registration
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (3 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 05/15] drm/arc: Embedd a drm_connector for sim case Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 07/15] drm/arc: Use drmm_mode_config_cleanup Daniel Vetter
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Eugeniy Paltsev, tiantao6, Alexey Brodkin, Sam Ravnborg, Daniel Vetter

drm_connector_register does nothing before drm_dev_register(), it
is meant for hotpluggable connectors only. Same for the unregister side.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu_sim.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
index e42fe5d05a3d..3772df1647aa 100644
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ b/drivers/gpu/drm/arc/arcpgu_sim.c
@@ -29,7 +29,6 @@ static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
 
 static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
 {
-	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 }
 
@@ -80,7 +79,6 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 	ret = drm_connector_attach_encoder(connector, encoder);
 	if (ret < 0) {
 		dev_err(drm->dev, "could not attach connector to encoder\n");
-		drm_connector_unregister(connector);
 		goto error_connector_cleanup;
 	}
 
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 07/15] drm/arc: Use drmm_mode_config_cleanup
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (4 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 06/15] drm/arc: Drop surplus connector registration Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12 11:29   ` Thomas Zimmermann
  2021-01-12  8:43 ` [PATCH 08/15] drm/arc: Align with simple pipe helpers Daniel Vetter
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, Daniel Vetter,
	tiantao6, Eugeniy Paltsev

With autocleanup through drm_device management we can delete all the
code. Possible now that there's no confusion against devm_kzalloc'ed
structures anymore.

I inlined arcpgu_setup_mode_config because it's tiny and the newly
needed return value handling would have been more ...

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c |  4 +---
 drivers/gpu/drm/arc/arcpgu_drv.c  | 21 +++++++++------------
 drivers/gpu/drm/arc/arcpgu_hdmi.c |  6 +-----
 drivers/gpu/drm/arc/arcpgu_sim.c  | 11 ++---------
 4 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 31a85c703307..43313adb1981 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -209,10 +209,8 @@ int arc_pgu_setup_crtc(struct drm_device *drm)
 
 	ret = drm_crtc_init_with_planes(drm, &arcpgu->pipe.crtc, primary, NULL,
 					&arc_pgu_crtc_funcs, NULL);
-	if (ret) {
-		arc_pgu_plane_destroy(primary);
+	if (ret)
 		return ret;
-	}
 
 	drm_crtc_helper_add(&arcpgu->pipe.crtc, &arc_pgu_crtc_helper_funcs);
 	return 0;
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 9020352816fa..6349e9dc770e 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -30,16 +30,6 @@ static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
 	.atomic_commit = drm_atomic_helper_commit,
 };
 
-static void arcpgu_setup_mode_config(struct drm_device *drm)
-{
-	drm_mode_config_init(drm);
-	drm->mode_config.min_width = 0;
-	drm->mode_config.min_height = 0;
-	drm->mode_config.max_width = 1920;
-	drm->mode_config.max_height = 1080;
-	drm->mode_config.funcs = &arcpgu_drm_modecfg_funcs;
-}
-
 DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
 
 static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
@@ -54,7 +44,15 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 	if (IS_ERR(arcpgu->clk))
 		return PTR_ERR(arcpgu->clk);
 
-	arcpgu_setup_mode_config(drm);
+	ret = drmm_mode_config_init(drm);
+	if (ret)
+		return ret;
+
+	drm->mode_config.min_width = 0;
+	drm->mode_config.min_height = 0;
+	drm->mode_config.max_width = 1920;
+	drm->mode_config.max_height = 1080;
+	drm->mode_config.funcs = &arcpgu_drm_modecfg_funcs;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	arcpgu->regs = devm_ioremap_resource(&pdev->dev, res);
@@ -108,7 +106,6 @@ static int arcpgu_unload(struct drm_device *drm)
 {
 	drm_kms_helper_poll_fini(drm);
 	drm_atomic_helper_shutdown(drm);
-	drm_mode_config_cleanup(drm);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index dbad2c9237fe..925d6d31bb78 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -39,9 +39,5 @@ int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
 		return ret;
 
 	/* Link drm_bridge to encoder */
-	ret = drm_bridge_attach(encoder, bridge, NULL, 0);
-	if (ret)
-		drm_encoder_cleanup(encoder);
-
-	return ret;
+	return drm_bridge_attach(encoder, bridge, NULL, 0);
 }
diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
index 3772df1647aa..afc34f8b4de0 100644
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ b/drivers/gpu/drm/arc/arcpgu_sim.c
@@ -73,21 +73,14 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 			DRM_MODE_CONNECTOR_VIRTUAL);
 	if (ret < 0) {
 		dev_err(drm->dev, "failed to initialize drm connector\n");
-		goto error_encoder_cleanup;
+		return ret;
 	}
 
 	ret = drm_connector_attach_encoder(connector, encoder);
 	if (ret < 0) {
 		dev_err(drm->dev, "could not attach connector to encoder\n");
-		goto error_connector_cleanup;
+		return ret;
 	}
 
 	return 0;
-
-error_connector_cleanup:
-	drm_connector_cleanup(connector);
-
-error_encoder_cleanup:
-	drm_encoder_cleanup(encoder);
-	return ret;
 }
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 08/15] drm/arc: Align with simple pipe helpers
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (5 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 07/15] drm/arc: Use drmm_mode_config_cleanup Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 09/15] drm/arc: Convert to drm_simple_kms_pipe_helper Daniel Vetter
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, Daniel Vetter,
	tiantao6, Eugeniy Paltsev

Simple pipe helpers only have an enable and disable hook, no more
mode_set_nofb. Call it from our enable hook to align with that
conversions.

Atomic helpers always call mode_set_nofb and enable together, so
there's no functional change here.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 43313adb1981..3538c1686ca4 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -73,10 +73,9 @@ static enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc,
 	return MODE_NOCLOCK;
 }
 
-static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc)
+static void arc_pgu_mode_set(struct arcpgu_drm_private *arcpgu)
 {
-	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
-	struct drm_display_mode *m = &crtc->state->adjusted_mode;
+	struct drm_display_mode *m = &arcpgu->pipe.crtc.state->adjusted_mode;
 	u32 val;
 
 	arc_pgu_write(arcpgu, ARCPGU_REG_FMT,
@@ -110,7 +109,7 @@ static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	arc_pgu_write(arcpgu, ARCPGU_REG_STRIDE, 0);
 	arc_pgu_write(arcpgu, ARCPGU_REG_START_SET, 1);
 
-	arc_pgu_set_pxl_fmt(crtc);
+	arc_pgu_set_pxl_fmt(&arcpgu->pipe.crtc);
 
 	clk_set_rate(arcpgu->clk, m->crtc_clock * 1000);
 }
@@ -120,6 +119,8 @@ static void arc_pgu_crtc_atomic_enable(struct drm_crtc *crtc,
 {
 	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
 
+	arc_pgu_mode_set(arcpgu);
+
 	clk_prepare_enable(arcpgu->clk);
 	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
 		      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) |
@@ -139,7 +140,6 @@ static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
 	.mode_valid	= arc_pgu_crtc_mode_valid,
-	.mode_set_nofb	= arc_pgu_crtc_mode_set_nofb,
 	.atomic_enable	= arc_pgu_crtc_atomic_enable,
 	.atomic_disable	= arc_pgu_crtc_atomic_disable,
 };
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/15] drm/arc: Convert to drm_simple_kms_pipe_helper
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (6 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 08/15] drm/arc: Align with simple pipe helpers Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 10/15] drm/arc: Drop crtc check in arc_pgu_update Daniel Vetter
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, Daniel Vetter,
	tiantao6, Eugeniy Paltsev

Really straighforward, only slight issue is that the sim connector is
created after the pipe is set up, so can't use the helpers perfectly
yet. Subsequent patches will fix that.

Aside from lots of deleting code no functional changes in here.

v2: Delete now unused crtc funcs (0day)

v3: Move endcoder setup removal to right patch (Sam)

Cc: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu.h      |   4 +-
 drivers/gpu/drm/arc/arcpgu_crtc.c | 111 ++++++++----------------------
 drivers/gpu/drm/arc/arcpgu_drv.c  |   2 +-
 drivers/gpu/drm/arc/arcpgu_hdmi.c |  18 +----
 drivers/gpu/drm/arc/arcpgu_sim.c  |  12 ----
 5 files changed, 31 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index c52cdd2274e1..b5c699d14f27 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -20,7 +20,7 @@ struct arcpgu_drm_private {
 
 #define dev_to_arcpgu(x) container_of(x, struct arcpgu_drm_private, drm)
 
-#define crtc_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, pipe.crtc)
+#define pipe_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, pipe)
 
 static inline void arc_pgu_write(struct arcpgu_drm_private *arcpgu,
 				 unsigned int reg, u32 value)
@@ -34,7 +34,7 @@ static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
 	return ioread32(arcpgu->regs + reg);
 }
 
-int arc_pgu_setup_crtc(struct drm_device *dev);
+int arc_pgu_setup_pipe(struct drm_device *dev);
 int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
 
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 3538c1686ca4..ef7dca789868 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -25,10 +25,9 @@ static const u32 arc_pgu_supported_formats[] = {
 	DRM_FORMAT_ARGB8888,
 };
 
-static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc)
+static void arc_pgu_set_pxl_fmt(struct arcpgu_drm_private *arcpgu)
 {
-	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
-	const struct drm_framebuffer *fb = crtc->primary->state->fb;
+	const struct drm_framebuffer *fb = arcpgu->pipe.plane.state->fb;
 	uint32_t pixel_format = fb->format->format;
 	u32 format = DRM_FORMAT_INVALID;
 	int i;
@@ -50,19 +49,10 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc)
 	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, reg_ctrl);
 }
 
-static const struct drm_crtc_funcs arc_pgu_crtc_funcs = {
-	.destroy = drm_crtc_cleanup,
-	.set_config = drm_atomic_helper_set_config,
-	.page_flip = drm_atomic_helper_page_flip,
-	.reset = drm_atomic_helper_crtc_reset,
-	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
-};
-
-static enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc,
-						    const struct drm_display_mode *mode)
+static enum drm_mode_status arc_pgu_mode_valid(struct drm_simple_display_pipe *pipe,
+					       const struct drm_display_mode *mode)
 {
-	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
+	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
 	long rate, clk_rate = mode->clock * 1000;
 	long diff = clk_rate / 200; /* +-0.5% allowed by HDMI spec */
 
@@ -109,15 +99,16 @@ static void arc_pgu_mode_set(struct arcpgu_drm_private *arcpgu)
 	arc_pgu_write(arcpgu, ARCPGU_REG_STRIDE, 0);
 	arc_pgu_write(arcpgu, ARCPGU_REG_START_SET, 1);
 
-	arc_pgu_set_pxl_fmt(&arcpgu->pipe.crtc);
+	arc_pgu_set_pxl_fmt(arcpgu);
 
 	clk_set_rate(arcpgu->clk, m->crtc_clock * 1000);
 }
 
-static void arc_pgu_crtc_atomic_enable(struct drm_crtc *crtc,
-				       struct drm_atomic_state *state)
+static void arc_pgu_enable(struct drm_simple_display_pipe *pipe,
+			   struct drm_crtc_state *crtc_state,
+			   struct drm_plane_state *plane_state)
 {
-	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
+	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
 
 	arc_pgu_mode_set(arcpgu);
 
@@ -127,10 +118,9 @@ static void arc_pgu_crtc_atomic_enable(struct drm_crtc *crtc,
 		      ARCPGU_CTRL_ENABLE_MASK);
 }
 
-static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
-					struct drm_atomic_state *state)
+static void arc_pgu_disable(struct drm_simple_display_pipe *pipe)
 {
-	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
+	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
 
 	clk_disable_unprepare(arcpgu->clk);
 	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
@@ -138,80 +128,33 @@ static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
 			      ~ARCPGU_CTRL_ENABLE_MASK);
 }
 
-static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
-	.mode_valid	= arc_pgu_crtc_mode_valid,
-	.atomic_enable	= arc_pgu_crtc_atomic_enable,
-	.atomic_disable	= arc_pgu_crtc_atomic_disable,
-};
-
-static void arc_pgu_plane_atomic_update(struct drm_plane *plane,
-					struct drm_plane_state *state)
+static void arc_pgu_update(struct drm_simple_display_pipe *pipe,
+			   struct drm_plane_state *state)
 {
 	struct arcpgu_drm_private *arcpgu;
 	struct drm_gem_cma_object *gem;
 
-	if (!plane->state->crtc || !plane->state->fb)
+	if (!pipe->plane.state->crtc || !pipe->plane.state->fb)
 		return;
 
-	arcpgu = crtc_to_arcpgu_priv(plane->state->crtc);
-	gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0);
+	arcpgu = pipe_to_arcpgu_priv(pipe);
+	gem = drm_fb_cma_get_gem_obj(pipe->plane.state->fb, 0);
 	arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr);
 }
 
-static const struct drm_plane_helper_funcs arc_pgu_plane_helper_funcs = {
-	.atomic_update = arc_pgu_plane_atomic_update,
-};
-
-static void arc_pgu_plane_destroy(struct drm_plane *plane)
-{
-	drm_plane_cleanup(plane);
-}
-
-static const struct drm_plane_funcs arc_pgu_plane_funcs = {
-	.update_plane		= drm_atomic_helper_update_plane,
-	.disable_plane		= drm_atomic_helper_disable_plane,
-	.destroy		= arc_pgu_plane_destroy,
-	.reset			= drm_atomic_helper_plane_reset,
-	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
-	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
+static const struct drm_simple_display_pipe_funcs arc_pgu_pipe_funcs = {
+	.update = arc_pgu_update,
+	.mode_valid = arc_pgu_mode_valid,
+	.enable	= arc_pgu_enable,
+	.disable = arc_pgu_disable,
 };
 
-static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm)
+int arc_pgu_setup_pipe(struct drm_device *drm)
 {
 	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
-	struct drm_plane *plane = NULL;
-	int ret;
-
-	plane = &arcpgu->pipe.plane;
-
-	ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs,
-				       arc_pgu_supported_formats,
-				       ARRAY_SIZE(arc_pgu_supported_formats),
-				       NULL,
-				       DRM_PLANE_TYPE_PRIMARY, NULL);
-	if (ret)
-		return ERR_PTR(ret);
-
-	drm_plane_helper_add(plane, &arc_pgu_plane_helper_funcs);
-
-	return plane;
-}
-
-int arc_pgu_setup_crtc(struct drm_device *drm)
-{
-	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
-	struct drm_plane *primary;
-	int ret;
-
-	primary = arc_pgu_plane_init(drm);
-	if (IS_ERR(primary))
-		return PTR_ERR(primary);
-
-	ret = drm_crtc_init_with_planes(drm, &arcpgu->pipe.crtc, primary, NULL,
-					&arc_pgu_crtc_funcs, NULL);
-	if (ret)
-		return ret;
 
-	drm_crtc_helper_add(&arcpgu->pipe.crtc, &arc_pgu_crtc_helper_funcs);
-	return 0;
+	return drm_simple_display_pipe_init(drm, &arcpgu->pipe, &arc_pgu_pipe_funcs,
+					    arc_pgu_supported_formats,
+					    ARRAY_SIZE(arc_pgu_supported_formats),
+					    NULL, NULL);
 }
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 6349e9dc770e..222ab28efbd0 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -70,7 +70,7 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 	if (dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32)))
 		return -ENODEV;
 
-	if (arc_pgu_setup_crtc(drm) < 0)
+	if (arc_pgu_setup_pipe(drm) < 0)
 		return -ENODEV;
 
 	/*
diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index 925d6d31bb78..d430af686cbc 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -12,32 +12,16 @@
 
 #include "arcpgu.h"
 
-static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
 int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
 {
 	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
-	struct drm_encoder *encoder;
 	struct drm_bridge *bridge;
 
-	int ret = 0;
-
-	encoder = &arcpgu->pipe.encoder;
-
 	/* Locate drm bridge from the hdmi encoder DT node */
 	bridge = of_drm_find_bridge(np);
 	if (!bridge)
 		return -EPROBE_DEFER;
 
-	encoder->possible_crtcs = 1;
-	encoder->possible_clones = 0;
-	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
-			       DRM_MODE_ENCODER_TMDS, NULL);
-	if (ret)
-		return ret;
-
 	/* Link drm_bridge to encoder */
-	return drm_bridge_attach(encoder, bridge, NULL, 0);
+	return drm_simple_display_pipe_attach_bridge(&arcpgu->pipe, bridge);
 }
diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
index afc34f8b4de0..1a63f0868504 100644
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ b/drivers/gpu/drm/arc/arcpgu_sim.c
@@ -45,10 +45,6 @@ static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 {
 	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
@@ -58,14 +54,6 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 
 	encoder = &arcpgu->pipe.encoder;
 
-	encoder->possible_crtcs = 1;
-	encoder->possible_clones = 0;
-
-	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
-			       DRM_MODE_ENCODER_VIRTUAL, NULL);
-	if (ret)
-		return ret;
-
 	connector = &arcpgu->sim_conn;
 	drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs);
 
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 10/15] drm/arc: Drop crtc check in arc_pgu_update
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (7 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 09/15] drm/arc: Convert to drm_simple_kms_pipe_helper Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 11/15] drm/arc: Inline arcpgu_crtc.c Daniel Vetter
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, tiantao6, Alexey Brodkin, Eugeniy Paltsev, Daniel Vetter

It's redundant, drm core guarantees that state->fb is set iff
state->crtc is set.

v2: I had a misconception about simple helpers here and thought they
filter this out. They don't. Issue reported by Eugeniy.

Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ef7dca789868..30fbb3052bc7 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -134,7 +134,7 @@ static void arc_pgu_update(struct drm_simple_display_pipe *pipe,
 	struct arcpgu_drm_private *arcpgu;
 	struct drm_gem_cma_object *gem;
 
-	if (!pipe->plane.state->crtc || !pipe->plane.state->fb)
+	if (!pipe->plane.state->fb)
 		return;
 
 	arcpgu = pipe_to_arcpgu_priv(pipe);
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 11/15] drm/arc: Inline arcpgu_crtc.c
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (8 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 10/15] drm/arc: Drop crtc check in arc_pgu_update Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 12/15] drm/arc: Inline arcpgu_drm_hdmi_init Daniel Vetter
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, tiantao6,
	Daniel Vetter, Eugeniy Paltsev

Really not big anymore.

Note that we no longer clamp all errors to ENODEV, highlighted by Sam.

v2: Fixup update function, bug reported by Eugeniy

v3: Delete now unused crtc funcs (0day)

v4: Move encoder removal to right patch (Sam).

Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/Makefile      |   2 +-
 drivers/gpu/drm/arc/arcpgu.h      |   1 -
 drivers/gpu/drm/arc/arcpgu_crtc.c | 160 ------------------------------
 drivers/gpu/drm/arc/arcpgu_drv.c  | 141 +++++++++++++++++++++++++-
 4 files changed, 140 insertions(+), 164 deletions(-)
 delete mode 100644 drivers/gpu/drm/arc/arcpgu_crtc.c

diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
index c7028b7427b3..c686e0287a71 100644
--- a/drivers/gpu/drm/arc/Makefile
+++ b/drivers/gpu/drm/arc/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
-arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
+arcpgu-y := arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
 obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index b5c699d14f27..cee2448a07d6 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -34,7 +34,6 @@ static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
 	return ioread32(arcpgu->regs + reg);
 }
 
-int arc_pgu_setup_pipe(struct drm_device *dev);
 int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
 
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
deleted file mode 100644
index 30fbb3052bc7..000000000000
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ /dev/null
@@ -1,160 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * ARC PGU DRM driver.
- *
- * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com)
- */
-
-#include <drm/drm_atomic_helper.h>
-#include <drm/drm_device.h>
-#include <drm/drm_fb_cma_helper.h>
-#include <drm/drm_gem_cma_helper.h>
-#include <drm/drm_plane_helper.h>
-#include <drm/drm_probe_helper.h>
-#include <linux/clk.h>
-#include <linux/platform_data/simplefb.h>
-
-#include "arcpgu.h"
-#include "arcpgu_regs.h"
-
-#define ENCODE_PGU_XY(x, y)	((((x) - 1) << 16) | ((y) - 1))
-
-static const u32 arc_pgu_supported_formats[] = {
-	DRM_FORMAT_RGB565,
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_ARGB8888,
-};
-
-static void arc_pgu_set_pxl_fmt(struct arcpgu_drm_private *arcpgu)
-{
-	const struct drm_framebuffer *fb = arcpgu->pipe.plane.state->fb;
-	uint32_t pixel_format = fb->format->format;
-	u32 format = DRM_FORMAT_INVALID;
-	int i;
-	u32 reg_ctrl;
-
-	for (i = 0; i < ARRAY_SIZE(arc_pgu_supported_formats); i++) {
-		if (arc_pgu_supported_formats[i] == pixel_format)
-			format = arc_pgu_supported_formats[i];
-	}
-
-	if (WARN_ON(format == DRM_FORMAT_INVALID))
-		return;
-
-	reg_ctrl = arc_pgu_read(arcpgu, ARCPGU_REG_CTRL);
-	if (format == DRM_FORMAT_RGB565)
-		reg_ctrl &= ~ARCPGU_MODE_XRGB8888;
-	else
-		reg_ctrl |= ARCPGU_MODE_XRGB8888;
-	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, reg_ctrl);
-}
-
-static enum drm_mode_status arc_pgu_mode_valid(struct drm_simple_display_pipe *pipe,
-					       const struct drm_display_mode *mode)
-{
-	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
-	long rate, clk_rate = mode->clock * 1000;
-	long diff = clk_rate / 200; /* +-0.5% allowed by HDMI spec */
-
-	rate = clk_round_rate(arcpgu->clk, clk_rate);
-	if ((max(rate, clk_rate) - min(rate, clk_rate) < diff) && (rate > 0))
-		return MODE_OK;
-
-	return MODE_NOCLOCK;
-}
-
-static void arc_pgu_mode_set(struct arcpgu_drm_private *arcpgu)
-{
-	struct drm_display_mode *m = &arcpgu->pipe.crtc.state->adjusted_mode;
-	u32 val;
-
-	arc_pgu_write(arcpgu, ARCPGU_REG_FMT,
-		      ENCODE_PGU_XY(m->crtc_htotal, m->crtc_vtotal));
-
-	arc_pgu_write(arcpgu, ARCPGU_REG_HSYNC,
-		      ENCODE_PGU_XY(m->crtc_hsync_start - m->crtc_hdisplay,
-				    m->crtc_hsync_end - m->crtc_hdisplay));
-
-	arc_pgu_write(arcpgu, ARCPGU_REG_VSYNC,
-		      ENCODE_PGU_XY(m->crtc_vsync_start - m->crtc_vdisplay,
-				    m->crtc_vsync_end - m->crtc_vdisplay));
-
-	arc_pgu_write(arcpgu, ARCPGU_REG_ACTIVE,
-		      ENCODE_PGU_XY(m->crtc_hblank_end - m->crtc_hblank_start,
-				    m->crtc_vblank_end - m->crtc_vblank_start));
-
-	val = arc_pgu_read(arcpgu, ARCPGU_REG_CTRL);
-
-	if (m->flags & DRM_MODE_FLAG_PVSYNC)
-		val |= ARCPGU_CTRL_VS_POL_MASK << ARCPGU_CTRL_VS_POL_OFST;
-	else
-		val &= ~(ARCPGU_CTRL_VS_POL_MASK << ARCPGU_CTRL_VS_POL_OFST);
-
-	if (m->flags & DRM_MODE_FLAG_PHSYNC)
-		val |= ARCPGU_CTRL_HS_POL_MASK << ARCPGU_CTRL_HS_POL_OFST;
-	else
-		val &= ~(ARCPGU_CTRL_HS_POL_MASK << ARCPGU_CTRL_HS_POL_OFST);
-
-	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, val);
-	arc_pgu_write(arcpgu, ARCPGU_REG_STRIDE, 0);
-	arc_pgu_write(arcpgu, ARCPGU_REG_START_SET, 1);
-
-	arc_pgu_set_pxl_fmt(arcpgu);
-
-	clk_set_rate(arcpgu->clk, m->crtc_clock * 1000);
-}
-
-static void arc_pgu_enable(struct drm_simple_display_pipe *pipe,
-			   struct drm_crtc_state *crtc_state,
-			   struct drm_plane_state *plane_state)
-{
-	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
-
-	arc_pgu_mode_set(arcpgu);
-
-	clk_prepare_enable(arcpgu->clk);
-	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
-		      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) |
-		      ARCPGU_CTRL_ENABLE_MASK);
-}
-
-static void arc_pgu_disable(struct drm_simple_display_pipe *pipe)
-{
-	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
-
-	clk_disable_unprepare(arcpgu->clk);
-	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
-			      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) &
-			      ~ARCPGU_CTRL_ENABLE_MASK);
-}
-
-static void arc_pgu_update(struct drm_simple_display_pipe *pipe,
-			   struct drm_plane_state *state)
-{
-	struct arcpgu_drm_private *arcpgu;
-	struct drm_gem_cma_object *gem;
-
-	if (!pipe->plane.state->fb)
-		return;
-
-	arcpgu = pipe_to_arcpgu_priv(pipe);
-	gem = drm_fb_cma_get_gem_obj(pipe->plane.state->fb, 0);
-	arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr);
-}
-
-static const struct drm_simple_display_pipe_funcs arc_pgu_pipe_funcs = {
-	.update = arc_pgu_update,
-	.mode_valid = arc_pgu_mode_valid,
-	.enable	= arc_pgu_enable,
-	.disable = arc_pgu_disable,
-};
-
-int arc_pgu_setup_pipe(struct drm_device *drm)
-{
-	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
-
-	return drm_simple_display_pipe_init(drm, &arcpgu->pipe, &arc_pgu_pipe_funcs,
-					    arc_pgu_supported_formats,
-					    ARRAY_SIZE(arc_pgu_supported_formats),
-					    NULL, NULL);
-}
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 222ab28efbd0..e49e80b8b089 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -12,6 +12,7 @@
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_fourcc.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_of.h>
@@ -24,6 +25,138 @@
 #include "arcpgu.h"
 #include "arcpgu_regs.h"
 
+#define ENCODE_PGU_XY(x, y)	((((x) - 1) << 16) | ((y) - 1))
+
+static const u32 arc_pgu_supported_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_ARGB8888,
+};
+
+static void arc_pgu_set_pxl_fmt(struct arcpgu_drm_private *arcpgu)
+{
+	const struct drm_framebuffer *fb = arcpgu->pipe.plane.state->fb;
+	uint32_t pixel_format = fb->format->format;
+	u32 format = DRM_FORMAT_INVALID;
+	int i;
+	u32 reg_ctrl;
+
+	for (i = 0; i < ARRAY_SIZE(arc_pgu_supported_formats); i++) {
+		if (arc_pgu_supported_formats[i] == pixel_format)
+			format = arc_pgu_supported_formats[i];
+	}
+
+	if (WARN_ON(format == DRM_FORMAT_INVALID))
+		return;
+
+	reg_ctrl = arc_pgu_read(arcpgu, ARCPGU_REG_CTRL);
+	if (format == DRM_FORMAT_RGB565)
+		reg_ctrl &= ~ARCPGU_MODE_XRGB8888;
+	else
+		reg_ctrl |= ARCPGU_MODE_XRGB8888;
+	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, reg_ctrl);
+}
+
+static enum drm_mode_status arc_pgu_mode_valid(struct drm_simple_display_pipe *pipe,
+					       const struct drm_display_mode *mode)
+{
+	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
+	long rate, clk_rate = mode->clock * 1000;
+	long diff = clk_rate / 200; /* +-0.5% allowed by HDMI spec */
+
+	rate = clk_round_rate(arcpgu->clk, clk_rate);
+	if ((max(rate, clk_rate) - min(rate, clk_rate) < diff) && (rate > 0))
+		return MODE_OK;
+
+	return MODE_NOCLOCK;
+}
+
+static void arc_pgu_mode_set(struct arcpgu_drm_private *arcpgu)
+{
+	struct drm_display_mode *m = &arcpgu->pipe.crtc.state->adjusted_mode;
+	u32 val;
+
+	arc_pgu_write(arcpgu, ARCPGU_REG_FMT,
+		      ENCODE_PGU_XY(m->crtc_htotal, m->crtc_vtotal));
+
+	arc_pgu_write(arcpgu, ARCPGU_REG_HSYNC,
+		      ENCODE_PGU_XY(m->crtc_hsync_start - m->crtc_hdisplay,
+				    m->crtc_hsync_end - m->crtc_hdisplay));
+
+	arc_pgu_write(arcpgu, ARCPGU_REG_VSYNC,
+		      ENCODE_PGU_XY(m->crtc_vsync_start - m->crtc_vdisplay,
+				    m->crtc_vsync_end - m->crtc_vdisplay));
+
+	arc_pgu_write(arcpgu, ARCPGU_REG_ACTIVE,
+		      ENCODE_PGU_XY(m->crtc_hblank_end - m->crtc_hblank_start,
+				    m->crtc_vblank_end - m->crtc_vblank_start));
+
+	val = arc_pgu_read(arcpgu, ARCPGU_REG_CTRL);
+
+	if (m->flags & DRM_MODE_FLAG_PVSYNC)
+		val |= ARCPGU_CTRL_VS_POL_MASK << ARCPGU_CTRL_VS_POL_OFST;
+	else
+		val &= ~(ARCPGU_CTRL_VS_POL_MASK << ARCPGU_CTRL_VS_POL_OFST);
+
+	if (m->flags & DRM_MODE_FLAG_PHSYNC)
+		val |= ARCPGU_CTRL_HS_POL_MASK << ARCPGU_CTRL_HS_POL_OFST;
+	else
+		val &= ~(ARCPGU_CTRL_HS_POL_MASK << ARCPGU_CTRL_HS_POL_OFST);
+
+	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, val);
+	arc_pgu_write(arcpgu, ARCPGU_REG_STRIDE, 0);
+	arc_pgu_write(arcpgu, ARCPGU_REG_START_SET, 1);
+
+	arc_pgu_set_pxl_fmt(arcpgu);
+
+	clk_set_rate(arcpgu->clk, m->crtc_clock * 1000);
+}
+
+static void arc_pgu_enable(struct drm_simple_display_pipe *pipe,
+			   struct drm_crtc_state *crtc_state,
+			   struct drm_plane_state *plane_state)
+{
+	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
+
+	arc_pgu_mode_set(arcpgu);
+
+	clk_prepare_enable(arcpgu->clk);
+	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
+		      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) |
+		      ARCPGU_CTRL_ENABLE_MASK);
+}
+
+static void arc_pgu_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct arcpgu_drm_private *arcpgu = pipe_to_arcpgu_priv(pipe);
+
+	clk_disable_unprepare(arcpgu->clk);
+	arc_pgu_write(arcpgu, ARCPGU_REG_CTRL,
+			      arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) &
+			      ~ARCPGU_CTRL_ENABLE_MASK);
+}
+
+static void arc_pgu_update(struct drm_simple_display_pipe *pipe,
+			   struct drm_plane_state *state)
+{
+	struct arcpgu_drm_private *arcpgu;
+	struct drm_gem_cma_object *gem;
+
+	if (!pipe->plane.state->fb)
+		return;
+
+	arcpgu = pipe_to_arcpgu_priv(pipe);
+	gem = drm_fb_cma_get_gem_obj(pipe->plane.state->fb, 0);
+	arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr);
+}
+
+static const struct drm_simple_display_pipe_funcs arc_pgu_pipe_funcs = {
+	.update = arc_pgu_update,
+	.mode_valid = arc_pgu_mode_valid,
+	.enable	= arc_pgu_enable,
+	.disable = arc_pgu_disable,
+};
+
 static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
 	.fb_create  = drm_gem_fb_create,
 	.atomic_check = drm_atomic_helper_check,
@@ -70,8 +203,12 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 	if (dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32)))
 		return -ENODEV;
 
-	if (arc_pgu_setup_pipe(drm) < 0)
-		return -ENODEV;
+	ret = drm_simple_display_pipe_init(drm, &arcpgu->pipe, &arc_pgu_pipe_funcs,
+					   arc_pgu_supported_formats,
+					   ARRAY_SIZE(arc_pgu_supported_formats),
+					   NULL, NULL);
+	if (ret)
+		return ret;
 
 	/*
 	 * There is only one output port inside each device. It is linked with
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 12/15] drm/arc: Inline arcpgu_drm_hdmi_init
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (9 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 11/15] drm/arc: Inline arcpgu_crtc.c Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 13/15] drm/arc: Inline remaining files Daniel Vetter
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, tiantao6, Alexey Brodkin, Eugeniy Paltsev, Daniel Vetter

Really not worth the function, much less the separate file now that
almost all the code is gone.

Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/arc/Makefile      |  2 +-
 drivers/gpu/drm/arc/arcpgu.h      |  1 -
 drivers/gpu/drm/arc/arcpgu_drv.c  | 12 +++++++++---
 drivers/gpu/drm/arc/arcpgu_hdmi.c | 27 ---------------------------
 4 files changed, 10 insertions(+), 32 deletions(-)
 delete mode 100644 drivers/gpu/drm/arc/arcpgu_hdmi.c

diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
index c686e0287a71..379a1145bc2f 100644
--- a/drivers/gpu/drm/arc/Makefile
+++ b/drivers/gpu/drm/arc/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
-arcpgu-y := arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
+arcpgu-y := arcpgu_sim.o arcpgu_drv.o
 obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index cee2448a07d6..7dce0c2313ba 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -34,7 +34,6 @@ static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
 	return ioread32(arcpgu->regs + reg);
 }
 
-int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
 
 #endif
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index e49e80b8b089..0e959e42893d 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -221,9 +221,15 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 	}
 
 	if (encoder_node) {
-		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
-		of_node_put(encoder_node);
-		if (ret < 0)
+		struct drm_bridge *bridge;
+
+		/* Locate drm bridge from the hdmi encoder DT node */
+		bridge = of_drm_find_bridge(encoder_node);
+		if (!bridge)
+			return -EPROBE_DEFER;
+
+		ret = drm_simple_display_pipe_attach_bridge(&arcpgu->pipe, bridge);
+		if (ret)
 			return ret;
 	} else {
 		dev_info(drm->dev, "no encoder found. Assumed virtual LCD on simulation platform\n");
diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
deleted file mode 100644
index d430af686cbc..000000000000
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ /dev/null
@@ -1,27 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * ARC PGU DRM driver.
- *
- * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com)
- */
-
-#include <drm/drm_bridge.h>
-#include <drm/drm_crtc.h>
-#include <drm/drm_encoder.h>
-#include <drm/drm_device.h>
-
-#include "arcpgu.h"
-
-int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
-{
-	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
-	struct drm_bridge *bridge;
-
-	/* Locate drm bridge from the hdmi encoder DT node */
-	bridge = of_drm_find_bridge(np);
-	if (!bridge)
-		return -EPROBE_DEFER;
-
-	/* Link drm_bridge to encoder */
-	return drm_simple_display_pipe_attach_bridge(&arcpgu->pipe, bridge);
-}
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 13/15] drm/arc: Inline remaining files
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (10 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 12/15] drm/arc: Inline arcpgu_drm_hdmi_init Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 14/15] drm/arc: Initialize sim connector before display pipe Daniel Vetter
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, Daniel Vetter,
	tiantao6, Eugeniy Paltsev

At less than 500 lines total feels like the right thing to do.

Also noticed that the simple wrapper around drm_connector_cleanup can
be dropped.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/arc/Makefile      |   2 +-
 drivers/gpu/drm/arc/arcpgu.h      |  39 ------------
 drivers/gpu/drm/arc/arcpgu_drv.c  | 102 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/arc/arcpgu_regs.h |  31 ---------
 drivers/gpu/drm/arc/arcpgu_sim.c  |  74 ----------------------
 5 files changed, 101 insertions(+), 147 deletions(-)
 delete mode 100644 drivers/gpu/drm/arc/arcpgu.h
 delete mode 100644 drivers/gpu/drm/arc/arcpgu_regs.h
 delete mode 100644 drivers/gpu/drm/arc/arcpgu_sim.c

diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
index 379a1145bc2f..b26f2495c532 100644
--- a/drivers/gpu/drm/arc/Makefile
+++ b/drivers/gpu/drm/arc/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
-arcpgu-y := arcpgu_sim.o arcpgu_drv.o
+arcpgu-y := arcpgu_drv.o
 obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
deleted file mode 100644
index 7dce0c2313ba..000000000000
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * ARC PGU DRM driver.
- *
- * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com)
- */
-
-#ifndef _ARCPGU_H_
-#define _ARCPGU_H_
-
-#include <drm/drm_simple_kms_helper.h>
-
-struct arcpgu_drm_private {
-	struct drm_device	drm;
-	void __iomem		*regs;
-	struct clk		*clk;
-	struct drm_simple_display_pipe pipe;
-	struct drm_connector	sim_conn;
-};
-
-#define dev_to_arcpgu(x) container_of(x, struct arcpgu_drm_private, drm)
-
-#define pipe_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, pipe)
-
-static inline void arc_pgu_write(struct arcpgu_drm_private *arcpgu,
-				 unsigned int reg, u32 value)
-{
-	iowrite32(value, arcpgu->regs + reg);
-}
-
-static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
-			       unsigned int reg)
-{
-	return ioread32(arcpgu->regs + reg);
-}
-
-int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
-
-#endif
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 0e959e42893d..0a0b54993773 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -17,13 +17,111 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_of.h>
 #include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 
-#include "arcpgu.h"
-#include "arcpgu_regs.h"
+#define ARCPGU_REG_CTRL		0x00
+#define ARCPGU_REG_STAT		0x04
+#define ARCPGU_REG_FMT		0x10
+#define ARCPGU_REG_HSYNC	0x14
+#define ARCPGU_REG_VSYNC	0x18
+#define ARCPGU_REG_ACTIVE	0x1c
+#define ARCPGU_REG_BUF0_ADDR	0x40
+#define ARCPGU_REG_STRIDE	0x50
+#define ARCPGU_REG_START_SET	0x84
+
+#define ARCPGU_REG_ID		0x3FC
+
+#define ARCPGU_CTRL_ENABLE_MASK	0x02
+#define ARCPGU_CTRL_VS_POL_MASK	0x1
+#define ARCPGU_CTRL_VS_POL_OFST	0x3
+#define ARCPGU_CTRL_HS_POL_MASK	0x1
+#define ARCPGU_CTRL_HS_POL_OFST	0x4
+#define ARCPGU_MODE_XRGB8888	BIT(2)
+#define ARCPGU_STAT_BUSY_MASK	0x02
+
+struct arcpgu_drm_private {
+	struct drm_device	drm;
+	void __iomem		*regs;
+	struct clk		*clk;
+	struct drm_simple_display_pipe pipe;
+	struct drm_connector	sim_conn;
+};
+
+#define dev_to_arcpgu(x) container_of(x, struct arcpgu_drm_private, drm)
+
+#define pipe_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, pipe)
+
+static inline void arc_pgu_write(struct arcpgu_drm_private *arcpgu,
+				 unsigned int reg, u32 value)
+{
+	iowrite32(value, arcpgu->regs + reg);
+}
+
+static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
+			       unsigned int reg)
+{
+	return ioread32(arcpgu->regs + reg);
+}
+
+#define XRES_DEF	640
+#define YRES_DEF	480
+
+#define XRES_MAX	8192
+#define YRES_MAX	8192
+
+static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
+{
+	int count;
+
+	count = drm_add_modes_noedid(connector, XRES_MAX, YRES_MAX);
+	drm_set_preferred_mode(connector, XRES_DEF, YRES_DEF);
+	return count;
+}
+
+static const struct drm_connector_helper_funcs
+arcpgu_drm_connector_helper_funcs = {
+	.get_modes = arcpgu_drm_connector_get_modes,
+};
+
+static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
+{
+	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+	int ret;
+
+	encoder = &arcpgu->pipe.encoder;
+
+	connector = &arcpgu->sim_conn;
+	drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs);
+
+	ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs,
+			DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret < 0) {
+		dev_err(drm->dev, "failed to initialize drm connector\n");
+		return ret;
+	}
+
+	ret = drm_connector_attach_encoder(connector, encoder);
+	if (ret < 0) {
+		dev_err(drm->dev, "could not attach connector to encoder\n");
+		return ret;
+	}
+
+	return 0;
+}
 
 #define ENCODE_PGU_XY(x, y)	((((x) - 1) << 16) | ((y) - 1))
 
diff --git a/drivers/gpu/drm/arc/arcpgu_regs.h b/drivers/gpu/drm/arc/arcpgu_regs.h
deleted file mode 100644
index b689a382d556..000000000000
--- a/drivers/gpu/drm/arc/arcpgu_regs.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * ARC PGU DRM driver.
- *
- * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com)
- */
-
-#ifndef _ARC_PGU_REGS_H_
-#define _ARC_PGU_REGS_H_
-
-#define ARCPGU_REG_CTRL		0x00
-#define ARCPGU_REG_STAT		0x04
-#define ARCPGU_REG_FMT		0x10
-#define ARCPGU_REG_HSYNC	0x14
-#define ARCPGU_REG_VSYNC	0x18
-#define ARCPGU_REG_ACTIVE	0x1c
-#define ARCPGU_REG_BUF0_ADDR	0x40
-#define ARCPGU_REG_STRIDE	0x50
-#define ARCPGU_REG_START_SET	0x84
-
-#define ARCPGU_REG_ID		0x3FC
-
-#define ARCPGU_CTRL_ENABLE_MASK	0x02
-#define ARCPGU_CTRL_VS_POL_MASK	0x1
-#define ARCPGU_CTRL_VS_POL_OFST	0x3
-#define ARCPGU_CTRL_HS_POL_MASK	0x1
-#define ARCPGU_CTRL_HS_POL_OFST	0x4
-#define ARCPGU_MODE_XRGB8888	BIT(2)
-#define ARCPGU_STAT_BUSY_MASK	0x02
-
-#endif
diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
deleted file mode 100644
index 1a63f0868504..000000000000
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ /dev/null
@@ -1,74 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * ARC PGU DRM driver.
- *
- * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com)
- */
-
-#include <drm/drm_atomic_helper.h>
-#include <drm/drm_device.h>
-#include <drm/drm_probe_helper.h>
-
-#include "arcpgu.h"
-
-#define XRES_DEF	640
-#define YRES_DEF	480
-
-#define XRES_MAX	8192
-#define YRES_MAX	8192
-
-
-static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
-{
-	int count;
-
-	count = drm_add_modes_noedid(connector, XRES_MAX, YRES_MAX);
-	drm_set_preferred_mode(connector, XRES_DEF, YRES_DEF);
-	return count;
-}
-
-static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_cleanup(connector);
-}
-
-static const struct drm_connector_helper_funcs
-arcpgu_drm_connector_helper_funcs = {
-	.get_modes = arcpgu_drm_connector_get_modes,
-};
-
-static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
-	.reset = drm_atomic_helper_connector_reset,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = arcpgu_drm_connector_destroy,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
-{
-	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
-	struct drm_encoder *encoder;
-	struct drm_connector *connector;
-	int ret;
-
-	encoder = &arcpgu->pipe.encoder;
-
-	connector = &arcpgu->sim_conn;
-	drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs);
-
-	ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs,
-			DRM_MODE_CONNECTOR_VIRTUAL);
-	if (ret < 0) {
-		dev_err(drm->dev, "failed to initialize drm connector\n");
-		return ret;
-	}
-
-	ret = drm_connector_attach_encoder(connector, encoder);
-	if (ret < 0) {
-		dev_err(drm->dev, "could not attach connector to encoder\n");
-		return ret;
-	}
-
-	return 0;
-}
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 14/15] drm/arc: Initialize sim connector before display pipe
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (11 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 13/15] drm/arc: Inline remaining files Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12  8:43 ` [PATCH 15/15] drm/arc: Move to drm/tiny Daniel Vetter
  2021-01-12 11:42 ` [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Thomas Zimmermann
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Sam Ravnborg, Alexey Brodkin, Daniel Vetter, Daniel Vetter,
	tiantao6, Eugeniy Paltsev

That way we can get rid of this final piece of init code, and use the
simple pipe helpers as intended.

v2: Fix indent (Sam)

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 drivers/gpu/drm/arc/arcpgu_drv.c | 53 ++++++++++----------------------
 1 file changed, 17 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 0a0b54993773..8edfe6601151 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -95,32 +95,11 @@ static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
+static int arcpgu_drm_sim_init(struct drm_device *drm, struct drm_connector *connector)
 {
-	struct arcpgu_drm_private *arcpgu = dev_to_arcpgu(drm);
-	struct drm_encoder *encoder;
-	struct drm_connector *connector;
-	int ret;
-
-	encoder = &arcpgu->pipe.encoder;
-
-	connector = &arcpgu->sim_conn;
 	drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs);
-
-	ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs,
-			DRM_MODE_CONNECTOR_VIRTUAL);
-	if (ret < 0) {
-		dev_err(drm->dev, "failed to initialize drm connector\n");
-		return ret;
-	}
-
-	ret = drm_connector_attach_encoder(connector, encoder);
-	if (ret < 0) {
-		dev_err(drm->dev, "could not attach connector to encoder\n");
-		return ret;
-	}
-
-	return 0;
+	return drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs,
+				  DRM_MODE_CONNECTOR_VIRTUAL);
 }
 
 #define ENCODE_PGU_XY(x, y)	((((x) - 1) << 16) | ((y) - 1))
@@ -267,6 +246,7 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 {
 	struct platform_device *pdev = to_platform_device(arcpgu->drm.dev);
 	struct device_node *encoder_node = NULL, *endpoint_node = NULL;
+	struct drm_connector *connector = NULL;
 	struct drm_device *drm = &arcpgu->drm;
 	struct resource *res;
 	int ret;
@@ -301,13 +281,6 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 	if (dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32)))
 		return -ENODEV;
 
-	ret = drm_simple_display_pipe_init(drm, &arcpgu->pipe, &arc_pgu_pipe_funcs,
-					   arc_pgu_supported_formats,
-					   ARRAY_SIZE(arc_pgu_supported_formats),
-					   NULL, NULL);
-	if (ret)
-		return ret;
-
 	/*
 	 * There is only one output port inside each device. It is linked with
 	 * encoder endpoint.
@@ -316,8 +289,21 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 	if (endpoint_node) {
 		encoder_node = of_graph_get_remote_port_parent(endpoint_node);
 		of_node_put(endpoint_node);
+	} else {
+		connector = &arcpgu->sim_conn;
+		dev_info(drm->dev, "no encoder found. Assumed virtual LCD on simulation platform\n");
+		ret = arcpgu_drm_sim_init(drm, connector);
+		if (ret < 0)
+			return ret;
 	}
 
+	ret = drm_simple_display_pipe_init(drm, &arcpgu->pipe, &arc_pgu_pipe_funcs,
+					   arc_pgu_supported_formats,
+					   ARRAY_SIZE(arc_pgu_supported_formats),
+					   NULL, connector);
+	if (ret)
+		return ret;
+
 	if (encoder_node) {
 		struct drm_bridge *bridge;
 
@@ -329,11 +315,6 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
 		ret = drm_simple_display_pipe_attach_bridge(&arcpgu->pipe, bridge);
 		if (ret)
 			return ret;
-	} else {
-		dev_info(drm->dev, "no encoder found. Assumed virtual LCD on simulation platform\n");
-		ret = arcpgu_drm_sim_init(drm, NULL);
-		if (ret < 0)
-			return ret;
 	}
 
 	drm_mode_config_reset(drm);
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 15/15] drm/arc: Move to drm/tiny
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (12 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 14/15] drm/arc: Initialize sim connector before display pipe Daniel Vetter
@ 2021-01-12  8:43 ` Daniel Vetter
  2021-01-12 11:42 ` [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Thomas Zimmermann
  14 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-01-12  8:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, tiantao6, Alexey Brodkin, Eugeniy Paltsev, Daniel Vetter

Because it is.

v2: Delete now unused crtc funcs (0day)

Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
 MAINTAINERS                                         |  2 +-
 drivers/gpu/drm/Kconfig                             |  2 --
 drivers/gpu/drm/Makefile                            |  1 -
 drivers/gpu/drm/arc/Kconfig                         | 10 ----------
 drivers/gpu/drm/arc/Makefile                        |  3 ---
 drivers/gpu/drm/tiny/Kconfig                        | 10 ++++++++++
 drivers/gpu/drm/tiny/Makefile                       |  1 +
 drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} |  0
 8 files changed, 12 insertions(+), 17 deletions(-)
 delete mode 100644 drivers/gpu/drm/arc/Kconfig
 delete mode 100644 drivers/gpu/drm/arc/Makefile
 rename drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 17b92e6a0f06..9330902cc9e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1307,7 +1307,7 @@ ARC PGU DRM DRIVER
 M:	Alexey Brodkin <abrodkin@synopsys.com>
 S:	Supported
 F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
-F:	drivers/gpu/drm/arc/
+F:	drivers/gpu/drm/tiny/arcpgu.c
 
 ARCNET NETWORK LAYER
 M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 0973f408d75f..041af75ee6fa 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -352,8 +352,6 @@ source "drivers/gpu/drm/vc4/Kconfig"
 
 source "drivers/gpu/drm/etnaviv/Kconfig"
 
-source "drivers/gpu/drm/arc/Kconfig"
-
 source "drivers/gpu/drm/hisilicon/Kconfig"
 
 source "drivers/gpu/drm/mediatek/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index fefaff4c832d..4e4937609269 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -110,7 +110,6 @@ obj-y			+= panel/
 obj-y			+= bridge/
 obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
 obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
-obj-$(CONFIG_DRM_ARCPGU)+= arc/
 obj-y			+= hisilicon/
 obj-$(CONFIG_DRM_ZTE)	+= zte/
 obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
deleted file mode 100644
index e8f3d63e0b91..000000000000
--- a/drivers/gpu/drm/arc/Kconfig
+++ /dev/null
@@ -1,10 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-config DRM_ARCPGU
-	tristate "ARC PGU"
-	depends on DRM && OF
-	select DRM_KMS_CMA_HELPER
-	select DRM_KMS_HELPER
-	help
-	  Choose this option if you have an ARC PGU controller.
-
-	  If M is selected the module will be called arcpgu.
diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
deleted file mode 100644
index b26f2495c532..000000000000
--- a/drivers/gpu/drm/arc/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-arcpgu-y := arcpgu_drv.o
-obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 2b6414f0fa75..9bbaa1a69050 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -1,5 +1,15 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
+config DRM_ARCPGU
+	tristate "ARC PGU"
+	depends on DRM && OF
+	select DRM_KMS_CMA_HELPER
+	select DRM_KMS_HELPER
+	help
+	  Choose this option if you have an ARC PGU controller.
+
+	  If M is selected the module will be called arcpgu.
+
 config DRM_CIRRUS_QEMU
 	tristate "Cirrus driver for QEMU emulated device"
 	depends on DRM && PCI && MMU
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 6ae4e9e5a35f..bef6780bdd6f 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
+obj-$(CONFIG_DRM_ARCPGU)		+= arcpgu.o
 obj-$(CONFIG_DRM_CIRRUS_QEMU)		+= cirrus.o
 obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
 obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/tiny/arcpgu.c
similarity index 100%
rename from drivers/gpu/drm/arc/arcpgu_drv.c
rename to drivers/gpu/drm/tiny/arcpgu.c
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 07/15] drm/arc: Use drmm_mode_config_cleanup
  2021-01-12  8:43 ` [PATCH 07/15] drm/arc: Use drmm_mode_config_cleanup Daniel Vetter
@ 2021-01-12 11:29   ` Thomas Zimmermann
  2021-02-25 11:16     ` Daniel Vetter
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2021-01-12 11:29 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Eugeniy Paltsev, Alexey Brodkin, Sam Ravnborg, tiantao6, Daniel Vetter


[-- Attachment #1.1.1: Type: text/plain, Size: 4907 bytes --]

Hi

Am 12.01.21 um 09:43 schrieb Daniel Vetter:
> With autocleanup through drm_device management we can delete all the
> code. Possible now that there's no confusion against devm_kzalloc'ed
> structures anymore.
> 
> I inlined arcpgu_setup_mode_config because it's tiny and the newly
> needed return value handling would have been more ...
> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> ---
>   drivers/gpu/drm/arc/arcpgu_crtc.c |  4 +---
>   drivers/gpu/drm/arc/arcpgu_drv.c  | 21 +++++++++------------
>   drivers/gpu/drm/arc/arcpgu_hdmi.c |  6 +-----
>   drivers/gpu/drm/arc/arcpgu_sim.c  | 11 ++---------
>   4 files changed, 13 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index 31a85c703307..43313adb1981 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -209,10 +209,8 @@ int arc_pgu_setup_crtc(struct drm_device *drm)
>   
>   	ret = drm_crtc_init_with_planes(drm, &arcpgu->pipe.crtc, primary, NULL,
>   					&arc_pgu_crtc_funcs, NULL);
> -	if (ret) {
> -		arc_pgu_plane_destroy(primary);
> +	if (ret)
>   		return ret;
> -	}
>   
>   	drm_crtc_helper_add(&arcpgu->pipe.crtc, &arc_pgu_crtc_helper_funcs);
>   	return 0;
> diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
> index 9020352816fa..6349e9dc770e 100644
> --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> @@ -30,16 +30,6 @@ static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
>   	.atomic_commit = drm_atomic_helper_commit,
>   };
>   
> -static void arcpgu_setup_mode_config(struct drm_device *drm)
> -{
> -	drm_mode_config_init(drm);
> -	drm->mode_config.min_width = 0;
> -	drm->mode_config.min_height = 0;
> -	drm->mode_config.max_width = 1920;
> -	drm->mode_config.max_height = 1080;
> -	drm->mode_config.funcs = &arcpgu_drm_modecfg_funcs;
> -}
> -
>   DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
>   
>   static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
> @@ -54,7 +44,15 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
>   	if (IS_ERR(arcpgu->clk))
>   		return PTR_ERR(arcpgu->clk);
>   
> -	arcpgu_setup_mode_config(drm);
> +	ret = drmm_mode_config_init(drm);
> +	if (ret)
> +		return ret;
> +
> +	drm->mode_config.min_width = 0;
> +	drm->mode_config.min_height = 0;
> +	drm->mode_config.max_width = 1920;
> +	drm->mode_config.max_height = 1080;
> +	drm->mode_config.funcs = &arcpgu_drm_modecfg_funcs;

It feels wrong to do this before even acquiring I/O memory. I would have 
moved all this just before the call to arc_pgu_setup_crtc().

Best regards
Thomas

>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	arcpgu->regs = devm_ioremap_resource(&pdev->dev, res);
> @@ -108,7 +106,6 @@ static int arcpgu_unload(struct drm_device *drm)
>   {
>   	drm_kms_helper_poll_fini(drm);
>   	drm_atomic_helper_shutdown(drm);
> -	drm_mode_config_cleanup(drm);
>   
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> index dbad2c9237fe..925d6d31bb78 100644
> --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
> +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> @@ -39,9 +39,5 @@ int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
>   		return ret;
>   
>   	/* Link drm_bridge to encoder */
> -	ret = drm_bridge_attach(encoder, bridge, NULL, 0);
> -	if (ret)
> -		drm_encoder_cleanup(encoder);
> -
> -	return ret;
> +	return drm_bridge_attach(encoder, bridge, NULL, 0);
>   }
> diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
> index 3772df1647aa..afc34f8b4de0 100644
> --- a/drivers/gpu/drm/arc/arcpgu_sim.c
> +++ b/drivers/gpu/drm/arc/arcpgu_sim.c
> @@ -73,21 +73,14 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
>   			DRM_MODE_CONNECTOR_VIRTUAL);
>   	if (ret < 0) {
>   		dev_err(drm->dev, "failed to initialize drm connector\n");
> -		goto error_encoder_cleanup;
> +		return ret;
>   	}
>   
>   	ret = drm_connector_attach_encoder(connector, encoder);
>   	if (ret < 0) {
>   		dev_err(drm->dev, "could not attach connector to encoder\n");
> -		goto error_connector_cleanup;
> +		return ret;
>   	}
>   
>   	return 0;
> -
> -error_connector_cleanup:
> -	drm_connector_cleanup(connector);
> -
> -error_encoder_cleanup:
> -	drm_encoder_cleanup(encoder);
> -	return ret;
>   }
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc
  2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
                   ` (13 preceding siblings ...)
  2021-01-12  8:43 ` [PATCH 15/15] drm/arc: Move to drm/tiny Daniel Vetter
@ 2021-01-12 11:42 ` Thomas Zimmermann
  2021-02-25 12:19   ` Daniel Vetter
  14 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2021-01-12 11:42 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Eugeniy Paltsev, Alexey Brodkin, Sam Ravnborg, tiantao6, Daniel Vetter


[-- Attachment #1.1.1: Type: text/plain, Size: 3652 bytes --]

Hi

Am 12.01.21 um 09:43 schrieb Daniel Vetter:
> - Need to embedded the drm_device, but for now we keep the usual
>    pointer chasing.
> - No more devm_kzalloc, which fixes a lifetime issues on driver
>    remove.
> - No more drm_dev_put, that's done by devm_ now.
> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>

For the whole patchset:

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

There's a comment on patch 7.

Best regards
Thomas

> ---
>   drivers/gpu/drm/arc/arcpgu.h     |  1 +
>   drivers/gpu/drm/arc/arcpgu_drv.c | 33 +++++++++++++-------------------
>   2 files changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
> index 6aac44b953ad..cd9e932f501e 100644
> --- a/drivers/gpu/drm/arc/arcpgu.h
> +++ b/drivers/gpu/drm/arc/arcpgu.h
> @@ -9,6 +9,7 @@
>   #define _ARCPGU_H_
>   
>   struct arcpgu_drm_private {
> +	struct drm_device	drm;
>   	void __iomem		*regs;
>   	struct clk		*clk;
>   	struct drm_framebuffer	*fb;
> diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
> index f164818ec477..68eb4a31c54b 100644
> --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> @@ -42,18 +42,14 @@ static void arcpgu_setup_mode_config(struct drm_device *drm)
>   
>   DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
>   
> -static int arcpgu_load(struct drm_device *drm)
> +static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
>   {
> -	struct platform_device *pdev = to_platform_device(drm->dev);
> -	struct arcpgu_drm_private *arcpgu;
> +	struct platform_device *pdev = to_platform_device(arcpgu->drm.dev);
>   	struct device_node *encoder_node = NULL, *endpoint_node = NULL;
> +	struct drm_device *drm = &arcpgu->drm;
>   	struct resource *res;
>   	int ret;
>   
> -	arcpgu = devm_kzalloc(&pdev->dev, sizeof(*arcpgu), GFP_KERNEL);
> -	if (arcpgu == NULL)
> -		return -ENOMEM;
> -
>   	drm->dev_private = arcpgu;
>   
>   	arcpgu->clk = devm_clk_get(drm->dev, "pxlclk");
> @@ -162,30 +158,28 @@ static struct drm_driver arcpgu_drm_driver = {
>   
>   static int arcpgu_probe(struct platform_device *pdev)
>   {
> -	struct drm_device *drm;
> +	struct arcpgu_drm_private *arcpgu;
>   	int ret;
>   
> -	drm = drm_dev_alloc(&arcpgu_drm_driver, &pdev->dev);
> -	if (IS_ERR(drm))
> -		return PTR_ERR(drm);
> +	arcpgu = devm_drm_dev_alloc(&pdev->dev, &arcpgu_drm_driver,
> +				    struct arcpgu_drm_private, drm);
> +	if (IS_ERR(arcpgu))
> +		return PTR_ERR(arcpgu);
>   
> -	ret = arcpgu_load(drm);
> +	ret = arcpgu_load(arcpgu);
>   	if (ret)
> -		goto err_unref;
> +		return ret;
>   
> -	ret = drm_dev_register(drm, 0);
> +	ret = drm_dev_register(&arcpgu->drm, 0);
>   	if (ret)
>   		goto err_unload;
>   
> -	drm_fbdev_generic_setup(drm, 16);
> +	drm_fbdev_generic_setup(&arcpgu->drm, 16);
>   
>   	return 0;
>   
>   err_unload:
> -	arcpgu_unload(drm);
> -
> -err_unref:
> -	drm_dev_put(drm);
> +	arcpgu_unload(&arcpgu->drm);
>   
>   	return ret;
>   }
> @@ -196,7 +190,6 @@ static int arcpgu_remove(struct platform_device *pdev)
>   
>   	drm_dev_unregister(drm);
>   	arcpgu_unload(drm);
> -	drm_dev_put(drm);
>   
>   	return 0;
>   }
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 07/15] drm/arc: Use drmm_mode_config_cleanup
  2021-01-12 11:29   ` Thomas Zimmermann
@ 2021-02-25 11:16     ` Daniel Vetter
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-02-25 11:16 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Daniel Vetter, Alexey Brodkin, DRI Development, Eugeniy Paltsev,
	Daniel Vetter, tiantao6, Sam Ravnborg

On Tue, Jan 12, 2021 at 12:29:59PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 12.01.21 um 09:43 schrieb Daniel Vetter:
> > With autocleanup through drm_device management we can delete all the
> > code. Possible now that there's no confusion against devm_kzalloc'ed
> > structures anymore.
> > 
> > I inlined arcpgu_setup_mode_config because it's tiny and the newly
> > needed return value handling would have been more ...
> > 
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Alexey Brodkin <abrodkin@synopsys.com>
> > ---
> >   drivers/gpu/drm/arc/arcpgu_crtc.c |  4 +---
> >   drivers/gpu/drm/arc/arcpgu_drv.c  | 21 +++++++++------------
> >   drivers/gpu/drm/arc/arcpgu_hdmi.c |  6 +-----
> >   drivers/gpu/drm/arc/arcpgu_sim.c  | 11 ++---------
> >   4 files changed, 13 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > index 31a85c703307..43313adb1981 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > @@ -209,10 +209,8 @@ int arc_pgu_setup_crtc(struct drm_device *drm)
> >   	ret = drm_crtc_init_with_planes(drm, &arcpgu->pipe.crtc, primary, NULL,
> >   					&arc_pgu_crtc_funcs, NULL);
> > -	if (ret) {
> > -		arc_pgu_plane_destroy(primary);
> > +	if (ret)
> >   		return ret;
> > -	}
> >   	drm_crtc_helper_add(&arcpgu->pipe.crtc, &arc_pgu_crtc_helper_funcs);
> >   	return 0;
> > diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
> > index 9020352816fa..6349e9dc770e 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> > @@ -30,16 +30,6 @@ static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
> >   	.atomic_commit = drm_atomic_helper_commit,
> >   };
> > -static void arcpgu_setup_mode_config(struct drm_device *drm)
> > -{
> > -	drm_mode_config_init(drm);
> > -	drm->mode_config.min_width = 0;
> > -	drm->mode_config.min_height = 0;
> > -	drm->mode_config.max_width = 1920;
> > -	drm->mode_config.max_height = 1080;
> > -	drm->mode_config.funcs = &arcpgu_drm_modecfg_funcs;
> > -}
> > -
> >   DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
> >   static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
> > @@ -54,7 +44,15 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
> >   	if (IS_ERR(arcpgu->clk))
> >   		return PTR_ERR(arcpgu->clk);
> > -	arcpgu_setup_mode_config(drm);
> > +	ret = drmm_mode_config_init(drm);
> > +	if (ret)
> > +		return ret;
> > +
> > +	drm->mode_config.min_width = 0;
> > +	drm->mode_config.min_height = 0;
> > +	drm->mode_config.max_width = 1920;
> > +	drm->mode_config.max_height = 1080;
> > +	drm->mode_config.funcs = &arcpgu_drm_modecfg_funcs;
> 
> It feels wrong to do this before even acquiring I/O memory. I would have
> moved all this just before the call to arc_pgu_setup_crtc().

It really doesn't matter, as long as everything is set up before we call
drm_dev_register, or register the fbdev emulation. So I think I'll just
stick with the color choice the original author went with.

But yeah grouping all the kms stuff a bit more together can't hurt.
-Daniel

> 
> Best regards
> Thomas
> 
> >   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >   	arcpgu->regs = devm_ioremap_resource(&pdev->dev, res);
> > @@ -108,7 +106,6 @@ static int arcpgu_unload(struct drm_device *drm)
> >   {
> >   	drm_kms_helper_poll_fini(drm);
> >   	drm_atomic_helper_shutdown(drm);
> > -	drm_mode_config_cleanup(drm);
> >   	return 0;
> >   }
> > diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > index dbad2c9237fe..925d6d31bb78 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> > @@ -39,9 +39,5 @@ int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
> >   		return ret;
> >   	/* Link drm_bridge to encoder */
> > -	ret = drm_bridge_attach(encoder, bridge, NULL, 0);
> > -	if (ret)
> > -		drm_encoder_cleanup(encoder);
> > -
> > -	return ret;
> > +	return drm_bridge_attach(encoder, bridge, NULL, 0);
> >   }
> > diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
> > index 3772df1647aa..afc34f8b4de0 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_sim.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_sim.c
> > @@ -73,21 +73,14 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
> >   			DRM_MODE_CONNECTOR_VIRTUAL);
> >   	if (ret < 0) {
> >   		dev_err(drm->dev, "failed to initialize drm connector\n");
> > -		goto error_encoder_cleanup;
> > +		return ret;
> >   	}
> >   	ret = drm_connector_attach_encoder(connector, encoder);
> >   	if (ret < 0) {
> >   		dev_err(drm->dev, "could not attach connector to encoder\n");
> > -		goto error_connector_cleanup;
> > +		return ret;
> >   	}
> >   	return 0;
> > -
> > -error_connector_cleanup:
> > -	drm_connector_cleanup(connector);
> > -
> > -error_encoder_cleanup:
> > -	drm_encoder_cleanup(encoder);
> > -	return ret;
> >   }
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc
  2021-01-12 11:42 ` [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Thomas Zimmermann
@ 2021-02-25 12:19   ` Daniel Vetter
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2021-02-25 12:19 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Daniel Vetter, Alexey Brodkin, DRI Development, Eugeniy Paltsev,
	Daniel Vetter, tiantao6, Sam Ravnborg

On Tue, Jan 12, 2021 at 12:42:16PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 12.01.21 um 09:43 schrieb Daniel Vetter:
> > - Need to embedded the drm_device, but for now we keep the usual
> >    pointer chasing.
> > - No more devm_kzalloc, which fixes a lifetime issues on driver
> >    remove.
> > - No more drm_dev_put, that's done by devm_ now.
> > 
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Alexey Brodkin <abrodkin@synopsys.com>
> 
> For the whole patchset:
> 
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> There's a comment on patch 7.

Queued up the entire pile for 5.13, thanks for taking a look.
-Daniel

> 
> Best regards
> Thomas
> 
> > ---
> >   drivers/gpu/drm/arc/arcpgu.h     |  1 +
> >   drivers/gpu/drm/arc/arcpgu_drv.c | 33 +++++++++++++-------------------
> >   2 files changed, 14 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
> > index 6aac44b953ad..cd9e932f501e 100644
> > --- a/drivers/gpu/drm/arc/arcpgu.h
> > +++ b/drivers/gpu/drm/arc/arcpgu.h
> > @@ -9,6 +9,7 @@
> >   #define _ARCPGU_H_
> >   struct arcpgu_drm_private {
> > +	struct drm_device	drm;
> >   	void __iomem		*regs;
> >   	struct clk		*clk;
> >   	struct drm_framebuffer	*fb;
> > diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
> > index f164818ec477..68eb4a31c54b 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> > @@ -42,18 +42,14 @@ static void arcpgu_setup_mode_config(struct drm_device *drm)
> >   DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
> > -static int arcpgu_load(struct drm_device *drm)
> > +static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
> >   {
> > -	struct platform_device *pdev = to_platform_device(drm->dev);
> > -	struct arcpgu_drm_private *arcpgu;
> > +	struct platform_device *pdev = to_platform_device(arcpgu->drm.dev);
> >   	struct device_node *encoder_node = NULL, *endpoint_node = NULL;
> > +	struct drm_device *drm = &arcpgu->drm;
> >   	struct resource *res;
> >   	int ret;
> > -	arcpgu = devm_kzalloc(&pdev->dev, sizeof(*arcpgu), GFP_KERNEL);
> > -	if (arcpgu == NULL)
> > -		return -ENOMEM;
> > -
> >   	drm->dev_private = arcpgu;
> >   	arcpgu->clk = devm_clk_get(drm->dev, "pxlclk");
> > @@ -162,30 +158,28 @@ static struct drm_driver arcpgu_drm_driver = {
> >   static int arcpgu_probe(struct platform_device *pdev)
> >   {
> > -	struct drm_device *drm;
> > +	struct arcpgu_drm_private *arcpgu;
> >   	int ret;
> > -	drm = drm_dev_alloc(&arcpgu_drm_driver, &pdev->dev);
> > -	if (IS_ERR(drm))
> > -		return PTR_ERR(drm);
> > +	arcpgu = devm_drm_dev_alloc(&pdev->dev, &arcpgu_drm_driver,
> > +				    struct arcpgu_drm_private, drm);
> > +	if (IS_ERR(arcpgu))
> > +		return PTR_ERR(arcpgu);
> > -	ret = arcpgu_load(drm);
> > +	ret = arcpgu_load(arcpgu);
> >   	if (ret)
> > -		goto err_unref;
> > +		return ret;
> > -	ret = drm_dev_register(drm, 0);
> > +	ret = drm_dev_register(&arcpgu->drm, 0);
> >   	if (ret)
> >   		goto err_unload;
> > -	drm_fbdev_generic_setup(drm, 16);
> > +	drm_fbdev_generic_setup(&arcpgu->drm, 16);
> >   	return 0;
> >   err_unload:
> > -	arcpgu_unload(drm);
> > -
> > -err_unref:
> > -	drm_dev_put(drm);
> > +	arcpgu_unload(&arcpgu->drm);
> >   	return ret;
> >   }
> > @@ -196,7 +190,6 @@ static int arcpgu_remove(struct platform_device *pdev)
> >   	drm_dev_unregister(drm);
> >   	arcpgu_unload(drm);
> > -	drm_dev_put(drm);
> >   	return 0;
> >   }
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-02-25 12:19 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12  8:43 [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Daniel Vetter
2021-01-12  8:43 ` [PATCH 02/15] drm/arc: Stop using drm_device->dev_private Daniel Vetter
2021-01-12  8:43 ` [PATCH 03/15] drm/arc: Delete arcpgu_priv->fb Daniel Vetter
2021-01-12  8:43 ` [PATCH 04/15] drm/arc: Embedded a drm_simple_display_pipe Daniel Vetter
2021-01-12  8:43 ` [PATCH 05/15] drm/arc: Embedd a drm_connector for sim case Daniel Vetter
2021-01-12  8:43 ` [PATCH 06/15] drm/arc: Drop surplus connector registration Daniel Vetter
2021-01-12  8:43 ` [PATCH 07/15] drm/arc: Use drmm_mode_config_cleanup Daniel Vetter
2021-01-12 11:29   ` Thomas Zimmermann
2021-02-25 11:16     ` Daniel Vetter
2021-01-12  8:43 ` [PATCH 08/15] drm/arc: Align with simple pipe helpers Daniel Vetter
2021-01-12  8:43 ` [PATCH 09/15] drm/arc: Convert to drm_simple_kms_pipe_helper Daniel Vetter
2021-01-12  8:43 ` [PATCH 10/15] drm/arc: Drop crtc check in arc_pgu_update Daniel Vetter
2021-01-12  8:43 ` [PATCH 11/15] drm/arc: Inline arcpgu_crtc.c Daniel Vetter
2021-01-12  8:43 ` [PATCH 12/15] drm/arc: Inline arcpgu_drm_hdmi_init Daniel Vetter
2021-01-12  8:43 ` [PATCH 13/15] drm/arc: Inline remaining files Daniel Vetter
2021-01-12  8:43 ` [PATCH 14/15] drm/arc: Initialize sim connector before display pipe Daniel Vetter
2021-01-12  8:43 ` [PATCH 15/15] drm/arc: Move to drm/tiny Daniel Vetter
2021-01-12 11:42 ` [PATCH 01/15] drm/arc: Switch to devm_drm_dev_alloc Thomas Zimmermann
2021-02-25 12:19   ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).