amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/10] drm/fb-helper: Various cleanups
@ 2023-01-25 20:04 Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event Thomas Zimmermann
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

Add various cleanups and changes to DRM's fbdev helpers and the
generic fbdev emulation. There's no clear theme here, just lots
of small things that need to be updated.
 
In the end, the code will better reflect which parts are in the 
DRM client, which is fbdev emulation, and which are shared fbdev
helpers.

v3:
	* various minor fixes (Javier))
	* build with CONFIG_DRM_FBDEV_EMULATION unset (kernel test robot)
v2:
	* cleanups in drm_fbdev_fb_destroy() (Sam)
	* fix declaration of drm_fb_helper_unprepare()

Thomas Zimmermann (10):
  drm/client: Test for connectors before sending hotplug event
  drm/client: Add hotplug_failed flag
  drm/fb-helper: Introduce drm_fb_helper_unprepare()
  drm/fbdev-generic: Initialize fb-helper structure in generic setup
  drm/fb-helper: Remove preferred_bpp parameter from fbdev internals
  drm/fb-helper: Initialize fb-helper's preferred BPP in prepare
    function
  drm/fbdev-generic: Minimize hotplug error handling
  drm/fbdev-generic: Minimize client unregistering
  drm/fbdev-generic: Inline clean-up helpers into drm_fbdev_fb_destroy()
  drm/fbdev-generic: Rename struct fb_info 'fbi' to 'info'

 drivers/gpu/drm/armada/armada_fbdev.c      |   4 +-
 drivers/gpu/drm/drm_client.c               |  10 ++
 drivers/gpu/drm/drm_fb_helper.c            |  58 ++++++---
 drivers/gpu/drm/drm_fbdev_generic.c        | 131 ++++++++-------------
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c  |   4 +-
 drivers/gpu/drm/gma500/framebuffer.c       |   4 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c |  11 +-
 drivers/gpu/drm/msm/msm_fbdev.c            |   4 +-
 drivers/gpu/drm/omapdrm/omap_fbdev.c       |   4 +-
 drivers/gpu/drm/radeon/radeon_fb.c         |   4 +-
 drivers/gpu/drm/tegra/fb.c                 |   7 +-
 include/drm/drm_client.h                   |   8 ++
 include/drm/drm_fb_helper.h                |  16 ++-
 13 files changed, 138 insertions(+), 127 deletions(-)


base-commit: 7d3e7f64a42d66ba8da6e7b66a8d85457ef84570
-- 
2.39.0


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

* [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 20:52   ` Sam Ravnborg
  2023-01-27 18:02   ` Simon Ser
  2023-01-25 20:04 ` [PATCH v3 02/10] drm/client: Add hotplug_failed flag Thomas Zimmermann
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

Test for connectors in the client code and remove a similar test
from the generic fbdev emulation. Do nothing if the test fails.
Not having connectors indicates a driver bug.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_client.c        | 5 +++++
 drivers/gpu/drm/drm_fbdev_generic.c | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 262ec64d4397..09ac191c202d 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev)
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
 
+	if (!dev->mode_config.num_connector) {
+		drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n");
+		return;
+	}
+
 	mutex_lock(&dev->clientlist_mutex);
 	list_for_each_entry(client, &dev->clientlist, list) {
 		if (!client->funcs || !client->funcs->hotplug)
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
index 0a4c160e0e58..3d455a2e3fb5 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -389,11 +389,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
 	if (dev->fb_helper)
 		return drm_fb_helper_hotplug_event(dev->fb_helper);
 
-	if (!dev->mode_config.num_connector) {
-		drm_dbg_kms(dev, "No connectors found, will not create framebuffer!\n");
-		return 0;
-	}
-
 	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
 
 	ret = drm_fb_helper_init(dev, fb_helper);
-- 
2.39.0


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

* [PATCH v3 02/10] drm/client: Add hotplug_failed flag
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 20:57   ` Sam Ravnborg
  2023-01-25 20:04 ` [PATCH v3 03/10] drm/fb-helper: Introduce drm_fb_helper_unprepare() Thomas Zimmermann
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

Signal failed hotplugging with a flag in struct drm_client_dev. If set,
the client helpers will not further try to set up the fbdev display.

This used to be signalled with a combination of cleared pointers in
struct drm_fb_helper, which prevents us from initializing these pointers
early after allocation.

The change also harmonizes behavior among DRM clients. Additional DRM
clients will now handle failed hotplugging like fbdev does.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_client.c        | 5 +++++
 drivers/gpu/drm/drm_fbdev_generic.c | 4 ----
 include/drm/drm_client.h            | 8 ++++++++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 09ac191c202d..009e7b10455c 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -208,8 +208,13 @@ void drm_client_dev_hotplug(struct drm_device *dev)
 		if (!client->funcs || !client->funcs->hotplug)
 			continue;
 
+		if (client->hotplug_failed)
+			continue;
+
 		ret = client->funcs->hotplug(client);
 		drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
+		if (ret)
+			client->hotplug_failed = true;
 	}
 	mutex_unlock(&dev->clientlist_mutex);
 }
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
index 3d455a2e3fb5..135d58b8007b 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -382,10 +382,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
 	struct drm_device *dev = client->dev;
 	int ret;
 
-	/* Setup is not retried if it has failed */
-	if (!fb_helper->dev && fb_helper->funcs)
-		return 0;
-
 	if (dev->fb_helper)
 		return drm_fb_helper_hotplug_event(dev->fb_helper);
 
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 4fc8018eddda..39482527a775 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -106,6 +106,14 @@ struct drm_client_dev {
 	 * @modesets: CRTC configurations
 	 */
 	struct drm_mode_set *modesets;
+
+	/**
+	 * @hotplug failed:
+	 *
+	 * Set by client hotplug helpers if the hotplugging failed
+	 * before. It is usually not tried again.
+	 */
+	bool hotplug_failed;
 };
 
 int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
-- 
2.39.0


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

* [PATCH v3 03/10] drm/fb-helper: Introduce drm_fb_helper_unprepare()
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 02/10] drm/client: Add hotplug_failed flag Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 04/10] drm/fbdev-generic: Initialize fb-helper structure in generic setup Thomas Zimmermann
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

Move the fb-helper clean-up code into drm_fb_helper_unprepare(). No
functional changes.

v2:
	* declare as static inline (kernel test robot)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 14 +++++++++++++-
 include/drm/drm_fb_helper.h     |  5 +++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index c5c13e192b64..4379bcd7718b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -435,6 +435,18 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 }
 EXPORT_SYMBOL(drm_fb_helper_prepare);
 
+/**
+ * drm_fb_helper_unprepare - clean up a drm_fb_helper structure
+ * @fb_helper: driver-allocated fbdev helper structure to set up
+ *
+ * Cleans up the framebuffer helper. Inverse of drm_fb_helper_prepare().
+ */
+void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper)
+{
+	mutex_destroy(&fb_helper->lock);
+}
+EXPORT_SYMBOL(drm_fb_helper_unprepare);
+
 /**
  * drm_fb_helper_init - initialize a &struct drm_fb_helper
  * @dev: drm device
@@ -561,7 +573,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 	}
 	mutex_unlock(&kernel_fb_helper_lock);
 
-	mutex_destroy(&fb_helper->lock);
+	drm_fb_helper_unprepare(fb_helper);
 
 	if (!fb_helper->client.funcs)
 		drm_client_release(&fb_helper->client);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index f443e1f11654..39710c570a04 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -230,6 +230,7 @@ drm_fb_helper_from_client(struct drm_client_dev *client)
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 			   const struct drm_fb_helper_funcs *funcs);
+void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper);
 int drm_fb_helper_init(struct drm_device *dev, struct drm_fb_helper *helper);
 void drm_fb_helper_fini(struct drm_fb_helper *helper);
 int drm_fb_helper_blank(int blank, struct fb_info *info);
@@ -296,6 +297,10 @@ static inline void drm_fb_helper_prepare(struct drm_device *dev,
 {
 }
 
+static inline void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper)
+{
+}
+
 static inline int drm_fb_helper_init(struct drm_device *dev,
 		       struct drm_fb_helper *helper)
 {
-- 
2.39.0


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

* [PATCH v3 04/10] drm/fbdev-generic: Initialize fb-helper structure in generic setup
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2023-01-25 20:04 ` [PATCH v3 03/10] drm/fb-helper: Introduce drm_fb_helper_unprepare() Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 21:03   ` Sam Ravnborg
  2023-01-25 20:04 ` [PATCH v3 05/10] drm/fb-helper: Remove preferred_bpp parameter from fbdev internals Thomas Zimmermann
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

Initialize the fb-helper structure immediately after its allocation
in drm_fbdev_generic_setup(). That will make it easier to fill it with
driver-specific values, such as the preferred BPP.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fbdev_generic.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
index 135d58b8007b..63f66325a8a5 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -385,8 +385,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
 	if (dev->fb_helper)
 		return drm_fb_helper_hotplug_event(dev->fb_helper);
 
-	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
-
 	ret = drm_fb_helper_init(dev, fb_helper);
 	if (ret)
 		goto err;
@@ -456,12 +454,12 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
 	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
 	if (!fb_helper)
 		return;
+	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
 
 	ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs);
 	if (ret) {
-		kfree(fb_helper);
 		drm_err(dev, "Failed to register client: %d\n", ret);
-		return;
+		goto err_drm_client_init;
 	}
 
 	/*
@@ -484,5 +482,12 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
 		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
 
 	drm_client_register(&fb_helper->client);
+
+	return;
+
+err_drm_client_init:
+	drm_fb_helper_unprepare(fb_helper);
+	kfree(fb_helper);
+	return;
 }
 EXPORT_SYMBOL(drm_fbdev_generic_setup);
-- 
2.39.0


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

* [PATCH v3 05/10] drm/fb-helper: Remove preferred_bpp parameter from fbdev internals
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2023-01-25 20:04 ` [PATCH v3 04/10] drm/fbdev-generic: Initialize fb-helper structure in generic setup Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 06/10] drm/fb-helper: Initialize fb-helper's preferred BPP in prepare function Thomas Zimmermann
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

Store the console's preferred BPP value in struct drm_fb_helper
and remove the respective function parameters from the internal
fbdev code.

The BPP value is only required as a fallback and will now always
be available in the fb-helper instance.

No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4379bcd7718b..258103d317ac 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1786,7 +1786,7 @@ static uint32_t drm_fb_helper_find_color_mode_format(struct drm_fb_helper *fb_he
 	return drm_fb_helper_find_format(fb_helper, formats, format_count, bpp, depth);
 }
 
-static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferred_bpp,
+static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
 				      struct drm_fb_helper_surface_size *sizes)
 {
 	struct drm_client_dev *client = &fb_helper->client;
@@ -1831,7 +1831,7 @@ static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int prefe
 		surface_format = drm_fb_helper_find_color_mode_format(fb_helper,
 								      plane->format_types,
 								      plane->format_count,
-								      preferred_bpp);
+								      fb_helper->preferred_bpp);
 		if (surface_format != DRM_FORMAT_INVALID)
 			break; /* found supported format */
 	}
@@ -1903,7 +1903,7 @@ static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int prefe
 	return 0;
 }
 
-static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferred_bpp,
+static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper,
 				    struct drm_fb_helper_surface_size *sizes)
 {
 	struct drm_client_dev *client = &fb_helper->client;
@@ -1912,7 +1912,7 @@ static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferr
 	int ret;
 
 	mutex_lock(&client->modeset_mutex);
-	ret = __drm_fb_helper_find_sizes(fb_helper, preferred_bpp, sizes);
+	ret = __drm_fb_helper_find_sizes(fb_helper, sizes);
 	mutex_unlock(&client->modeset_mutex);
 
 	if (ret)
@@ -1934,15 +1934,14 @@ static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferr
  * Allocates the backing storage and sets up the fbdev info structure through
  * the ->fb_probe callback.
  */
-static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
-					 int preferred_bpp)
+static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper)
 {
 	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_fb_helper_surface_size sizes;
 	int ret;
 
-	ret = drm_fb_helper_find_sizes(fb_helper, preferred_bpp, &sizes);
+	ret = drm_fb_helper_find_sizes(fb_helper, &sizes);
 	if (ret) {
 		/* First time: disable all crtc's.. */
 		if (!fb_helper->deferred_setup)
@@ -2125,8 +2124,7 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
 
 /* Note: Drops fb_helper->lock before returning. */
 static int
-__drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
-					  int bpp_sel)
+__drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper)
 {
 	struct drm_device *dev = fb_helper->dev;
 	struct fb_info *info;
@@ -2137,10 +2135,9 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
 	height = dev->mode_config.max_height;
 
 	drm_client_modeset_probe(&fb_helper->client, width, height);
-	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
+	ret = drm_fb_helper_single_fb_probe(fb_helper);
 	if (ret < 0) {
 		if (ret == -EAGAIN) {
-			fb_helper->preferred_bpp = bpp_sel;
 			fb_helper->deferred_setup = true;
 			ret = 0;
 		}
@@ -2231,8 +2228,10 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 	if (!drm_fbdev_emulation)
 		return 0;
 
+	fb_helper->preferred_bpp = bpp_sel;
+
 	mutex_lock(&fb_helper->lock);
-	ret = __drm_fb_helper_initial_config_and_unlock(fb_helper, bpp_sel);
+	ret = __drm_fb_helper_initial_config_and_unlock(fb_helper);
 
 	return ret;
 }
@@ -2268,8 +2267,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 
 	mutex_lock(&fb_helper->lock);
 	if (fb_helper->deferred_setup) {
-		err = __drm_fb_helper_initial_config_and_unlock(fb_helper,
-				fb_helper->preferred_bpp);
+		err = __drm_fb_helper_initial_config_and_unlock(fb_helper);
 		return err;
 	}
 
-- 
2.39.0


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

* [PATCH v3 06/10] drm/fb-helper: Initialize fb-helper's preferred BPP in prepare function
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
                   ` (4 preceding siblings ...)
  2023-01-25 20:04 ` [PATCH v3 05/10] drm/fb-helper: Remove preferred_bpp parameter from fbdev internals Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 07/10] drm/fbdev-generic: Minimize hotplug error handling Thomas Zimmermann
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

Initialize the fb-helper's preferred_bpp field early from within
drm_fb_helper_prepare(); instead of the later client hot-plugging
callback. This simplifies the generic fbdev setup function.

No real changes, but all drivers' fbdev code has to be adapted.

v3:
	* build with CONFIG_DRM_FBDEV_EMULATION unset (kernel test bot)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/armada/armada_fbdev.c      |  4 ++--
 drivers/gpu/drm/drm_fb_helper.c            | 22 ++++++++++++++++++----
 drivers/gpu/drm/drm_fbdev_generic.c        | 19 ++-----------------
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c  |  4 ++--
 drivers/gpu/drm/gma500/framebuffer.c       |  4 ++--
 drivers/gpu/drm/i915/display/intel_fbdev.c | 11 ++++++-----
 drivers/gpu/drm/msm/msm_fbdev.c            |  4 ++--
 drivers/gpu/drm/omapdrm/omap_fbdev.c       |  4 ++--
 drivers/gpu/drm/radeon/radeon_fb.c         |  4 ++--
 drivers/gpu/drm/tegra/fb.c                 |  7 +++----
 include/drm/drm_fb_helper.h                | 11 ++++++-----
 11 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c
index 584cee123bd8..07e410c62b7a 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -129,7 +129,7 @@ int armada_fbdev_init(struct drm_device *dev)
 
 	priv->fbdev = fbh;
 
-	drm_fb_helper_prepare(dev, fbh, &armada_fb_helper_funcs);
+	drm_fb_helper_prepare(dev, fbh, 32, &armada_fb_helper_funcs);
 
 	ret = drm_fb_helper_init(dev, fbh);
 	if (ret) {
@@ -137,7 +137,7 @@ int armada_fbdev_init(struct drm_device *dev)
 		goto err_fb_helper;
 	}
 
-	ret = drm_fb_helper_initial_config(fbh, 32);
+	ret = drm_fb_helper_initial_config(fbh);
 	if (ret) {
 		DRM_ERROR("failed to set initial config\n");
 		goto err_fb_setup;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 258103d317ac..28c428e9c530 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -416,14 +416,30 @@ static void drm_fb_helper_damage_work(struct work_struct *work)
  * drm_fb_helper_prepare - setup a drm_fb_helper structure
  * @dev: DRM device
  * @helper: driver-allocated fbdev helper structure to set up
+ * @preferred_bpp: Preferred bits per pixel for the device.
  * @funcs: pointer to structure of functions associate with this helper
  *
  * Sets up the bare minimum to make the framebuffer helper usable. This is
  * useful to implement race-free initialization of the polling helpers.
  */
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
+			   unsigned int preferred_bpp,
 			   const struct drm_fb_helper_funcs *funcs)
 {
+	/*
+	 * Pick a preferred bpp of 32 if no value has been given. This
+	 * will select XRGB8888 for the framebuffer formats. All drivers
+	 * have to support XRGB8888 for backwards compatibility with legacy
+	 * userspace, so it's the safe choice here.
+	 *
+	 * TODO: Replace struct drm_mode_config.preferred_depth and this
+	 *       bpp value with a preferred format that is given as struct
+	 *       drm_format_info. Then derive all other values from the
+	 *       format.
+	 */
+	if (!preferred_bpp)
+		preferred_bpp = 32;
+
 	INIT_LIST_HEAD(&helper->kernel_fb_list);
 	spin_lock_init(&helper->damage_lock);
 	INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker);
@@ -432,6 +448,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 	mutex_init(&helper->lock);
 	helper->funcs = funcs;
 	helper->dev = dev;
+	helper->preferred_bpp = preferred_bpp;
 }
 EXPORT_SYMBOL(drm_fb_helper_prepare);
 
@@ -2183,7 +2200,6 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper)
 /**
  * drm_fb_helper_initial_config - setup a sane initial connector configuration
  * @fb_helper: fb_helper device struct
- * @bpp_sel: bpp value to use for the framebuffer configuration
  *
  * Scans the CRTCs and connectors and tries to put together an initial setup.
  * At the moment, this is a cloned configuration across all heads with
@@ -2221,15 +2237,13 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper)
  * RETURNS:
  * Zero if everything went ok, nonzero otherwise.
  */
-int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
+int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper)
 {
 	int ret;
 
 	if (!drm_fbdev_emulation)
 		return 0;
 
-	fb_helper->preferred_bpp = bpp_sel;
-
 	mutex_lock(&fb_helper->lock);
 	ret = __drm_fb_helper_initial_config_and_unlock(fb_helper);
 
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
index 63f66325a8a5..6ae014040df3 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -392,7 +392,7 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
 	if (!drm_drv_uses_atomic_modeset(dev))
 		drm_helper_disable_unused_functions(dev);
 
-	ret = drm_fb_helper_initial_config(fb_helper, fb_helper->preferred_bpp);
+	ret = drm_fb_helper_initial_config(fb_helper);
 	if (ret)
 		goto err_cleanup;
 
@@ -454,7 +454,7 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
 	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
 	if (!fb_helper)
 		return;
-	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
+	drm_fb_helper_prepare(dev, fb_helper, preferred_bpp, &drm_fb_helper_generic_funcs);
 
 	ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs);
 	if (ret) {
@@ -462,21 +462,6 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
 		goto err_drm_client_init;
 	}
 
-	/*
-	 * Pick a preferred bpp of 32 if no value has been given. This
-	 * will select XRGB8888 for the framebuffer formats. All drivers
-	 * have to support XRGB8888 for backwards compatibility with legacy
-	 * userspace, so it's the safe choice here.
-	 *
-	 * TODO: Replace struct drm_mode_config.preferred_depth and this
-	 *       bpp value with a preferred format that is given as struct
-	 *       drm_format_info. Then derive all other values from the
-	 *       format.
-	 */
-	if (!preferred_bpp)
-		preferred_bpp = 32;
-	fb_helper->preferred_bpp = preferred_bpp;
-
 	ret = drm_fbdev_client_hotplug(&fb_helper->client);
 	if (ret)
 		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 55c92372fca0..b89e33af8da8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -163,7 +163,7 @@ int exynos_drm_fbdev_init(struct drm_device *dev)
 
 	private->fb_helper = helper = &fbdev->drm_fb_helper;
 
-	drm_fb_helper_prepare(dev, helper, &exynos_drm_fb_helper_funcs);
+	drm_fb_helper_prepare(dev, helper, PREFERRED_BPP, &exynos_drm_fb_helper_funcs);
 
 	ret = drm_fb_helper_init(dev, helper);
 	if (ret < 0) {
@@ -172,7 +172,7 @@ int exynos_drm_fbdev_init(struct drm_device *dev)
 		goto err_init;
 	}
 
-	ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP);
+	ret = drm_fb_helper_initial_config(helper);
 	if (ret < 0) {
 		DRM_DEV_ERROR(dev->dev,
 			      "failed to set up hw configuration.\n");
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 52ae3ade9a61..1f04c07ee180 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -409,7 +409,7 @@ int psb_fbdev_init(struct drm_device *dev)
 
 	dev_priv->fb_helper = fb_helper;
 
-	drm_fb_helper_prepare(dev, fb_helper, &psb_fb_helper_funcs);
+	drm_fb_helper_prepare(dev, fb_helper, 32, &psb_fb_helper_funcs);
 
 	ret = drm_fb_helper_init(dev, fb_helper);
 	if (ret)
@@ -418,7 +418,7 @@ int psb_fbdev_init(struct drm_device *dev)
 	/* disable all the possible outputs/crtcs before entering KMS mode */
 	drm_helper_disable_unused_functions(dev);
 
-	ret = drm_fb_helper_initial_config(fb_helper, 32);
+	ret = drm_fb_helper_initial_config(fb_helper);
 	if (ret)
 		goto fini;
 
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 19f3b5d92a55..ed197db5861d 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -520,10 +520,12 @@ int intel_fbdev_init(struct drm_device *dev)
 		return -ENOMEM;
 
 	mutex_init(&ifbdev->hpd_lock);
-	drm_fb_helper_prepare(dev, &ifbdev->helper, &intel_fb_helper_funcs);
+	drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs);
 
-	if (!intel_fbdev_init_bios(dev, ifbdev))
-		ifbdev->preferred_bpp = 32;
+	if (intel_fbdev_init_bios(dev, ifbdev))
+		ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp;
+	else
+		ifbdev->preferred_bpp = ifbdev->helper.preferred_bpp;
 
 	ret = drm_fb_helper_init(dev, &ifbdev->helper);
 	if (ret) {
@@ -542,8 +544,7 @@ static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
 	struct intel_fbdev *ifbdev = data;
 
 	/* Due to peculiar init order wrt to hpd handling this is separate. */
-	if (drm_fb_helper_initial_config(&ifbdev->helper,
-					 ifbdev->preferred_bpp))
+	if (drm_fb_helper_initial_config(&ifbdev->helper))
 		intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
 }
 
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index 31e1e30cb52a..915b213f3a5c 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -146,7 +146,7 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 
 	helper = &fbdev->base;
 
-	drm_fb_helper_prepare(dev, helper, &msm_fb_helper_funcs);
+	drm_fb_helper_prepare(dev, helper, 32, &msm_fb_helper_funcs);
 
 	ret = drm_fb_helper_init(dev, helper);
 	if (ret) {
@@ -159,7 +159,7 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 	if (ret)
 		goto fini;
 
-	ret = drm_fb_helper_initial_config(helper, 32);
+	ret = drm_fb_helper_initial_config(helper);
 	if (ret)
 		goto fini;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 98d8758048fc..fc5f52d567c6 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -239,13 +239,13 @@ void omap_fbdev_init(struct drm_device *dev)
 
 	helper = &fbdev->base;
 
-	drm_fb_helper_prepare(dev, helper, &omap_fb_helper_funcs);
+	drm_fb_helper_prepare(dev, helper, 32, &omap_fb_helper_funcs);
 
 	ret = drm_fb_helper_init(dev, helper);
 	if (ret)
 		goto fail;
 
-	ret = drm_fb_helper_initial_config(helper, 32);
+	ret = drm_fb_helper_initial_config(helper);
 	if (ret)
 		goto fini;
 
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index fe4087bfdb3c..6e5eed0e157c 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -348,7 +348,7 @@ int radeon_fbdev_init(struct radeon_device *rdev)
 	rfbdev->rdev = rdev;
 	rdev->mode_info.rfbdev = rfbdev;
 
-	drm_fb_helper_prepare(rdev->ddev, &rfbdev->helper,
+	drm_fb_helper_prepare(rdev->ddev, &rfbdev->helper, bpp_sel,
 			      &radeon_fb_helper_funcs);
 
 	ret = drm_fb_helper_init(rdev->ddev, &rfbdev->helper);
@@ -358,7 +358,7 @@ int radeon_fbdev_init(struct radeon_device *rdev)
 	/* disable all the possible outputs/crtcs before entering KMS mode */
 	drm_helper_disable_unused_functions(rdev->ddev);
 
-	ret = drm_fb_helper_initial_config(&rfbdev->helper, bpp_sel);
+	ret = drm_fb_helper_initial_config(&rfbdev->helper);
 	if (ret)
 		goto fini;
 
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index a900300ae5bd..153c39c32c71 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -308,7 +308,7 @@ static struct tegra_fbdev *tegra_fbdev_create(struct drm_device *drm)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	drm_fb_helper_prepare(drm, &fbdev->base, &tegra_fb_helper_funcs);
+	drm_fb_helper_prepare(drm, &fbdev->base, 32, &tegra_fb_helper_funcs);
 
 	return fbdev;
 }
@@ -319,7 +319,6 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev)
 }
 
 static int tegra_fbdev_init(struct tegra_fbdev *fbdev,
-			    unsigned int preferred_bpp,
 			    unsigned int num_crtc,
 			    unsigned int max_connectors)
 {
@@ -333,7 +332,7 @@ static int tegra_fbdev_init(struct tegra_fbdev *fbdev,
 		return err;
 	}
 
-	err = drm_fb_helper_initial_config(&fbdev->base, preferred_bpp);
+	err = drm_fb_helper_initial_config(&fbdev->base);
 	if (err < 0) {
 		dev_err(drm->dev, "failed to set initial configuration: %d\n",
 			err);
@@ -396,7 +395,7 @@ int tegra_drm_fb_init(struct drm_device *drm)
 	struct tegra_drm *tegra = drm->dev_private;
 	int err;
 
-	err = tegra_fbdev_init(tegra->fbdev, 32, drm->mode_config.num_crtc,
+	err = tegra_fbdev_init(tegra->fbdev, drm->mode_config.num_crtc,
 			       drm->mode_config.num_connector);
 	if (err < 0)
 		return err;
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 39710c570a04..93bc8f29f5a4 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -229,6 +229,7 @@ drm_fb_helper_from_client(struct drm_client_dev *client)
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
+			   unsigned int preferred_bpp,
 			   const struct drm_fb_helper_funcs *funcs);
 void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper);
 int drm_fb_helper_init(struct drm_device *dev, struct drm_fb_helper *helper);
@@ -284,7 +285,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 			unsigned long arg);
 
 int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);
-int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel);
+int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper);
 int drm_fb_helper_debug_enter(struct fb_info *info);
 int drm_fb_helper_debug_leave(struct fb_info *info);
 
@@ -292,8 +293,9 @@ void drm_fb_helper_lastclose(struct drm_device *dev);
 void drm_fb_helper_output_poll_changed(struct drm_device *dev);
 #else
 static inline void drm_fb_helper_prepare(struct drm_device *dev,
-					struct drm_fb_helper *helper,
-					const struct drm_fb_helper_funcs *funcs)
+					 struct drm_fb_helper *helper,
+					 unsigned int preferred_bpp,
+					 const struct drm_fb_helper_funcs *funcs)
 {
 }
 
@@ -455,8 +457,7 @@ static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 	return 0;
 }
 
-static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
-					       int bpp_sel)
+static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper)
 {
 	return 0;
 }
-- 
2.39.0


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

* [PATCH v3 07/10] drm/fbdev-generic: Minimize hotplug error handling
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
                   ` (5 preceding siblings ...)
  2023-01-25 20:04 ` [PATCH v3 06/10] drm/fb-helper: Initialize fb-helper's preferred BPP in prepare function Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 08/10] drm/fbdev-generic: Minimize client unregistering Thomas Zimmermann
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

Call drm_fb_helper_fini() in the generic-fbdev hotplug helper
to revert the effects of drm_fb_helper_init(). No full cleanup
is required.

v3:
	* fix error in commit message (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fbdev_generic.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
index 6ae014040df3..dd8be5e0f271 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -387,25 +387,21 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
 
 	ret = drm_fb_helper_init(dev, fb_helper);
 	if (ret)
-		goto err;
+		goto err_drm_err;
 
 	if (!drm_drv_uses_atomic_modeset(dev))
 		drm_helper_disable_unused_functions(dev);
 
 	ret = drm_fb_helper_initial_config(fb_helper);
 	if (ret)
-		goto err_cleanup;
+		goto err_drm_fb_helper_fini;
 
 	return 0;
 
-err_cleanup:
-	drm_fbdev_cleanup(fb_helper);
-err:
-	fb_helper->dev = NULL;
-	fb_helper->info = NULL;
-
+err_drm_fb_helper_fini:
+	drm_fb_helper_fini(fb_helper);
+err_drm_err:
 	drm_err(dev, "fbdev: Failed to setup generic emulation (ret=%d)\n", ret);
-
 	return ret;
 }
 
-- 
2.39.0


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

* [PATCH v3 08/10] drm/fbdev-generic: Minimize client unregistering
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
                   ` (6 preceding siblings ...)
  2023-01-25 20:04 ` [PATCH v3 07/10] drm/fbdev-generic: Minimize hotplug error handling Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 09/10] drm/fbdev-generic: Inline clean-up helpers into drm_fbdev_fb_destroy() Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 10/10] drm/fbdev-generic: Rename struct fb_info 'fbi' to 'info' Thomas Zimmermann
  9 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

For uninitialized framebuffers, only release the DRM client and
free the fbdev memory. Do not attempt to clean up the framebuffer.

DRM fbdev clients have a two-step initialization: first create
the DRM client; then create the framebuffer device on the first
successful hotplug event. In cases where the client never creates
the framebuffer, only the client state needs to be released. We
can detect which case it is, full or client-only cleanup, by
looking at the presence of fb_helper's info field.

v3:
	* fix typo in commit message (Javier)
	* release client before unpreparing fbdev
v2:
	* remove test for (fbi != NULL) in drm_fbdev_cleanup() (Sam)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fbdev_generic.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
index dd8be5e0f271..a9c519001019 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -51,12 +51,10 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
 	if (!fb_helper->dev)
 		return;
 
-	if (fbi) {
-		if (fbi->fbdefio)
-			fb_deferred_io_cleanup(fbi);
-		if (drm_fbdev_use_shadow_fb(fb_helper))
-			shadow = fbi->screen_buffer;
-	}
+	if (fbi->fbdefio)
+		fb_deferred_io_cleanup(fbi);
+	if (drm_fbdev_use_shadow_fb(fb_helper))
+		shadow = fbi->screen_buffer;
 
 	drm_fb_helper_fini(fb_helper);
 
@@ -362,11 +360,13 @@ static void drm_fbdev_client_unregister(struct drm_client_dev *client)
 {
 	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
 
-	if (fb_helper->info)
-		/* drm_fbdev_fb_destroy() takes care of cleanup */
+	if (fb_helper->info) {
 		drm_fb_helper_unregister_info(fb_helper);
-	else
-		drm_fbdev_release(fb_helper);
+	} else {
+		drm_client_release(&fb_helper->client);
+		drm_fb_helper_unprepare(fb_helper);
+		kfree(fb_helper);
+	}
 }
 
 static int drm_fbdev_client_restore(struct drm_client_dev *client)
-- 
2.39.0


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

* [PATCH v3 09/10] drm/fbdev-generic: Inline clean-up helpers into drm_fbdev_fb_destroy()
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
                   ` (7 preceding siblings ...)
  2023-01-25 20:04 ` [PATCH v3 08/10] drm/fbdev-generic: Minimize client unregistering Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  2023-01-25 20:04 ` [PATCH v3 10/10] drm/fbdev-generic: Rename struct fb_info 'fbi' to 'info' Thomas Zimmermann
  9 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

The fbdev framebuffer cleanup in drm_fbdev_fb_destroy() calls
drm_fbdev_release() and drm_fbdev_cleanup(). Inline both into the
caller. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fbdev_generic.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
index a9c519001019..68ce652e3a14 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -43,8 +43,9 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user)
 	return 0;
 }
 
-static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
+static void drm_fbdev_fb_destroy(struct fb_info *info)
 {
+	struct drm_fb_helper *fb_helper = info->par;
 	struct fb_info *fbi = fb_helper->info;
 	void *shadow = NULL;
 
@@ -64,24 +65,10 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
 		drm_client_buffer_vunmap(fb_helper->buffer);
 
 	drm_client_framebuffer_delete(fb_helper->buffer);
-}
-
-static void drm_fbdev_release(struct drm_fb_helper *fb_helper)
-{
-	drm_fbdev_cleanup(fb_helper);
 	drm_client_release(&fb_helper->client);
 	kfree(fb_helper);
 }
 
-/*
- * fb_ops.fb_destroy is called by the last put_fb_info() call at the end of
- * unregister_framebuffer() or fb_release().
- */
-static void drm_fbdev_fb_destroy(struct fb_info *info)
-{
-	drm_fbdev_release(info->par);
-}
-
 static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct drm_fb_helper *fb_helper = info->par;
-- 
2.39.0


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

* [PATCH v3 10/10] drm/fbdev-generic: Rename struct fb_info 'fbi' to 'info'
  2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
                   ` (8 preceding siblings ...)
  2023-01-25 20:04 ` [PATCH v3 09/10] drm/fbdev-generic: Inline clean-up helpers into drm_fbdev_fb_destroy() Thomas Zimmermann
@ 2023-01-25 20:04 ` Thomas Zimmermann
  9 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-25 20:04 UTC (permalink / raw)
  To: airlied, daniel, maarten.lankhorst, mripard, javierm
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, dri-devel, amd-gfx,
	Thomas Zimmermann, linux-tegra, freedreno, linux-arm-kernel

The generic fbdev emulation names variables of type struct fb_info
both 'fbi' and 'info'. The latter seems to be more common in fbdev
code, so name fbi accordingly.

Also replace the duplicate variable in drm_fbdev_fb_destroy().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fbdev_generic.c | 47 ++++++++++++++---------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
index 68ce652e3a14..43f94aa9e015 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -46,16 +46,15 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user)
 static void drm_fbdev_fb_destroy(struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
-	struct fb_info *fbi = fb_helper->info;
 	void *shadow = NULL;
 
 	if (!fb_helper->dev)
 		return;
 
-	if (fbi->fbdefio)
-		fb_deferred_io_cleanup(fbi);
+	if (info->fbdefio)
+		fb_deferred_io_cleanup(info);
 	if (drm_fbdev_use_shadow_fb(fb_helper))
-		shadow = fbi->screen_buffer;
+		shadow = info->screen_buffer;
 
 	drm_fb_helper_fini(fb_helper);
 
@@ -171,7 +170,7 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper,
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_client_buffer *buffer;
 	struct drm_framebuffer *fb;
-	struct fb_info *fbi;
+	struct fb_info *info;
 	u32 format;
 	struct iosys_map map;
 	int ret;
@@ -190,35 +189,35 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper,
 	fb_helper->fb = buffer->fb;
 	fb = buffer->fb;
 
-	fbi = drm_fb_helper_alloc_info(fb_helper);
-	if (IS_ERR(fbi))
-		return PTR_ERR(fbi);
+	info = drm_fb_helper_alloc_info(fb_helper);
+	if (IS_ERR(info))
+		return PTR_ERR(info);
 
-	fbi->fbops = &drm_fbdev_fb_ops;
-	fbi->screen_size = sizes->surface_height * fb->pitches[0];
-	fbi->fix.smem_len = fbi->screen_size;
-	fbi->flags = FBINFO_DEFAULT;
+	info->fbops = &drm_fbdev_fb_ops;
+	info->screen_size = sizes->surface_height * fb->pitches[0];
+	info->fix.smem_len = info->screen_size;
+	info->flags = FBINFO_DEFAULT;
 
-	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
+	drm_fb_helper_fill_info(info, fb_helper, sizes);
 
 	if (drm_fbdev_use_shadow_fb(fb_helper)) {
-		fbi->screen_buffer = vzalloc(fbi->screen_size);
-		if (!fbi->screen_buffer)
+		info->screen_buffer = vzalloc(info->screen_size);
+		if (!info->screen_buffer)
 			return -ENOMEM;
-		fbi->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST;
+		info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST;
 
-		fbi->fbdefio = &drm_fbdev_defio;
-		fb_deferred_io_init(fbi);
+		info->fbdefio = &drm_fbdev_defio;
+		fb_deferred_io_init(info);
 	} else {
 		/* buffer is mapped for HW framebuffer */
 		ret = drm_client_buffer_vmap(fb_helper->buffer, &map);
 		if (ret)
 			return ret;
 		if (map.is_iomem) {
-			fbi->screen_base = map.vaddr_iomem;
+			info->screen_base = map.vaddr_iomem;
 		} else {
-			fbi->screen_buffer = map.vaddr;
-			fbi->flags |= FBINFO_VIRTFB;
+			info->screen_buffer = map.vaddr;
+			info->flags |= FBINFO_VIRTFB;
 		}
 
 		/*
@@ -227,10 +226,10 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper,
 		 * case.
 		 */
 #if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
-		if (fb_helper->hint_leak_smem_start && fbi->fix.smem_start == 0 &&
+		if (fb_helper->hint_leak_smem_start && info->fix.smem_start == 0 &&
 		    !drm_WARN_ON_ONCE(dev, map.is_iomem))
-			fbi->fix.smem_start =
-				page_to_phys(virt_to_page(fbi->screen_buffer));
+			info->fix.smem_start =
+				page_to_phys(virt_to_page(info->screen_buffer));
 #endif
 	}
 
-- 
2.39.0


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

* Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
  2023-01-25 20:04 ` [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event Thomas Zimmermann
@ 2023-01-25 20:52   ` Sam Ravnborg
  2023-01-27 14:13     ` Thomas Zimmermann
  2023-01-27 18:02   ` Simon Ser
  1 sibling, 1 reply; 21+ messages in thread
From: Sam Ravnborg @ 2023-01-25 20:52 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: freedreno, linux-samsung-soc, amd-gfx, linux-arm-msm, intel-gfx,
	maarten.lankhorst, javierm, mripard, dri-devel, daniel,
	linux-tegra, airlied, linux-arm-kernel

Hi Thomas,

On Wed, Jan 25, 2023 at 09:04:06PM +0100, Thomas Zimmermann wrote:
> Test for connectors in the client code and remove a similar test
> from the generic fbdev emulation. Do nothing if the test fails.
> Not having connectors indicates a driver bug.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>  drivers/gpu/drm/drm_client.c        | 5 +++++
>  drivers/gpu/drm/drm_fbdev_generic.c | 5 -----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 262ec64d4397..09ac191c202d 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev)
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		return;
>  
> +	if (!dev->mode_config.num_connector) {
> +		drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n");
> +		return;
This deserves a more visible logging - if a driver fails here it would
be good to spot it in the normal kernel log.
drm_info or drm_notice?

The original code had this on the debug level, but when moving the log
level could also be updated.

	Sam

> +	}
> +
>  	mutex_lock(&dev->clientlist_mutex);
>  	list_for_each_entry(client, &dev->clientlist, list) {
>  		if (!client->funcs || !client->funcs->hotplug)
> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
> index 0a4c160e0e58..3d455a2e3fb5 100644
> --- a/drivers/gpu/drm/drm_fbdev_generic.c
> +++ b/drivers/gpu/drm/drm_fbdev_generic.c
> @@ -389,11 +389,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>  	if (dev->fb_helper)
>  		return drm_fb_helper_hotplug_event(dev->fb_helper);
>  
> -	if (!dev->mode_config.num_connector) {
> -		drm_dbg_kms(dev, "No connectors found, will not create framebuffer!\n");
> -		return 0;
> -	}
> -
>  	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
>  
>  	ret = drm_fb_helper_init(dev, fb_helper);
> -- 
> 2.39.0

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

* Re: [PATCH v3 02/10] drm/client: Add hotplug_failed flag
  2023-01-25 20:04 ` [PATCH v3 02/10] drm/client: Add hotplug_failed flag Thomas Zimmermann
@ 2023-01-25 20:57   ` Sam Ravnborg
  2023-01-27 14:17     ` Thomas Zimmermann
  0 siblings, 1 reply; 21+ messages in thread
From: Sam Ravnborg @ 2023-01-25 20:57 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: freedreno, linux-samsung-soc, amd-gfx, linux-arm-msm, intel-gfx,
	maarten.lankhorst, javierm, mripard, dri-devel, daniel,
	linux-tegra, airlied, linux-arm-kernel

Hi Thomas,

On Wed, Jan 25, 2023 at 09:04:07PM +0100, Thomas Zimmermann wrote:
> Signal failed hotplugging with a flag in struct drm_client_dev. If set,
> the client helpers will not further try to set up the fbdev display.
> 
> This used to be signalled with a combination of cleared pointers in
> struct drm_fb_helper,
I failed to find where we clear the pointers. What do I miss?
(I had assumed we would stop clearing the pointers after this change).

	Sam

which prevents us from initializing these pointers
> early after allocation.
> 
> The change also harmonizes behavior among DRM clients. Additional DRM
> clients will now handle failed hotplugging like fbdev does.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>  drivers/gpu/drm/drm_client.c        | 5 +++++
>  drivers/gpu/drm/drm_fbdev_generic.c | 4 ----
>  include/drm/drm_client.h            | 8 ++++++++
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 09ac191c202d..009e7b10455c 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -208,8 +208,13 @@ void drm_client_dev_hotplug(struct drm_device *dev)
>  		if (!client->funcs || !client->funcs->hotplug)
>  			continue;
>  
> +		if (client->hotplug_failed)
> +			continue;
> +
>  		ret = client->funcs->hotplug(client);
>  		drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
> +		if (ret)
> +			client->hotplug_failed = true;
>  	}
>  	mutex_unlock(&dev->clientlist_mutex);
>  }
> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
> index 3d455a2e3fb5..135d58b8007b 100644
> --- a/drivers/gpu/drm/drm_fbdev_generic.c
> +++ b/drivers/gpu/drm/drm_fbdev_generic.c
> @@ -382,10 +382,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>  	struct drm_device *dev = client->dev;
>  	int ret;
>  
> -	/* Setup is not retried if it has failed */
> -	if (!fb_helper->dev && fb_helper->funcs)
> -		return 0;
> -
>  	if (dev->fb_helper)
>  		return drm_fb_helper_hotplug_event(dev->fb_helper);
>  
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 4fc8018eddda..39482527a775 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -106,6 +106,14 @@ struct drm_client_dev {
>  	 * @modesets: CRTC configurations
>  	 */
>  	struct drm_mode_set *modesets;
> +
> +	/**
> +	 * @hotplug failed:
> +	 *
> +	 * Set by client hotplug helpers if the hotplugging failed
> +	 * before. It is usually not tried again.
> +	 */
> +	bool hotplug_failed;
>  };
>  
>  int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
> -- 
> 2.39.0

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

* Re: [PATCH v3 04/10] drm/fbdev-generic: Initialize fb-helper structure in generic setup
  2023-01-25 20:04 ` [PATCH v3 04/10] drm/fbdev-generic: Initialize fb-helper structure in generic setup Thomas Zimmermann
@ 2023-01-25 21:03   ` Sam Ravnborg
  2023-01-27 14:21     ` Thomas Zimmermann
  0 siblings, 1 reply; 21+ messages in thread
From: Sam Ravnborg @ 2023-01-25 21:03 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: freedreno, linux-samsung-soc, amd-gfx, linux-arm-msm, intel-gfx,
	maarten.lankhorst, javierm, mripard, dri-devel, daniel,
	linux-tegra, airlied, linux-arm-kernel

Hi Thomas,

On Wed, Jan 25, 2023 at 09:04:09PM +0100, Thomas Zimmermann wrote:
> Initialize the fb-helper structure immediately after its allocation
> in drm_fbdev_generic_setup(). That will make it easier to fill it with
> driver-specific values, such as the preferred BPP.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>  drivers/gpu/drm/drm_fbdev_generic.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
> index 135d58b8007b..63f66325a8a5 100644
> --- a/drivers/gpu/drm/drm_fbdev_generic.c
> +++ b/drivers/gpu/drm/drm_fbdev_generic.c
> @@ -385,8 +385,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>  	if (dev->fb_helper)
>  		return drm_fb_helper_hotplug_event(dev->fb_helper);
>  
> -	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
> -
>  	ret = drm_fb_helper_init(dev, fb_helper);
>  	if (ret)
>  		goto err;

From the documentation:
The drm_fb_helper_prepare()
helper must be called first to initialize the minimum required to make
hotplug detection work.
...
To finish up the fbdev helper initialization, the
drm_fb_helper_init() function is called.

So this change do not follow the documentation as drm_fb_helper_init()
is now called before drm_fb_helper_prepare()

I did not follow all the code - but my gut feeling is that the
documentation is right.

	Sam


> @@ -456,12 +454,12 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
>  	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
>  	if (!fb_helper)
>  		return;
> +	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
>  
>  	ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs);
>  	if (ret) {
> -		kfree(fb_helper);
>  		drm_err(dev, "Failed to register client: %d\n", ret);
> -		return;
> +		goto err_drm_client_init;
>  	}
>  
>  	/*
> @@ -484,5 +482,12 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
>  		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
>  
>  	drm_client_register(&fb_helper->client);
> +
> +	return;
> +
> +err_drm_client_init:
> +	drm_fb_helper_unprepare(fb_helper);
> +	kfree(fb_helper);
> +	return;
>  }
>  EXPORT_SYMBOL(drm_fbdev_generic_setup);
> -- 
> 2.39.0

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

* Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
  2023-01-25 20:52   ` Sam Ravnborg
@ 2023-01-27 14:13     ` Thomas Zimmermann
  2023-01-27 17:33       ` Sam Ravnborg
  0 siblings, 1 reply; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-27 14:13 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, javierm, amd-gfx,
	dri-devel, linux-tegra, freedreno, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2641 bytes --]

Hi

Am 25.01.23 um 21:52 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Wed, Jan 25, 2023 at 09:04:06PM +0100, Thomas Zimmermann wrote:
>> Test for connectors in the client code and remove a similar test
>> from the generic fbdev emulation. Do nothing if the test fails.
>> Not having connectors indicates a driver bug.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>>   drivers/gpu/drm/drm_client.c        | 5 +++++
>>   drivers/gpu/drm/drm_fbdev_generic.c | 5 -----
>>   2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index 262ec64d4397..09ac191c202d 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev)
>>   	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>>   		return;
>>   
>> +	if (!dev->mode_config.num_connector) {
>> +		drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n");
>> +		return;
> This deserves a more visible logging - if a driver fails here it would
> be good to spot it in the normal kernel log.
> drm_info or drm_notice?

But is that really noteworthy? AFAIK, this situation can legally happen. 
So if it's expected, why should we print a message about it?

Best regards
Thomas

> 
> The original code had this on the debug level, but when moving the log
> level could also be updated.
> 
> 	Sam
> 
>> +	}
>> +
>>   	mutex_lock(&dev->clientlist_mutex);
>>   	list_for_each_entry(client, &dev->clientlist, list) {
>>   		if (!client->funcs || !client->funcs->hotplug)
>> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
>> index 0a4c160e0e58..3d455a2e3fb5 100644
>> --- a/drivers/gpu/drm/drm_fbdev_generic.c
>> +++ b/drivers/gpu/drm/drm_fbdev_generic.c
>> @@ -389,11 +389,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>>   	if (dev->fb_helper)
>>   		return drm_fb_helper_hotplug_event(dev->fb_helper);
>>   
>> -	if (!dev->mode_config.num_connector) {
>> -		drm_dbg_kms(dev, "No connectors found, will not create framebuffer!\n");
>> -		return 0;
>> -	}
>> -
>>   	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
>>   
>>   	ret = drm_fb_helper_init(dev, fb_helper);
>> -- 
>> 2.39.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: Ivo Totev

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

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

* Re: [PATCH v3 02/10] drm/client: Add hotplug_failed flag
  2023-01-25 20:57   ` Sam Ravnborg
@ 2023-01-27 14:17     ` Thomas Zimmermann
  0 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-27 14:17 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: freedreno, linux-samsung-soc, amd-gfx, linux-arm-msm, intel-gfx,
	maarten.lankhorst, javierm, mripard, dri-devel, daniel,
	linux-tegra, airlied, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 3388 bytes --]

Hi

Am 25.01.23 um 21:57 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Wed, Jan 25, 2023 at 09:04:07PM +0100, Thomas Zimmermann wrote:
>> Signal failed hotplugging with a flag in struct drm_client_dev. If set,
>> the client helpers will not further try to set up the fbdev display.
>>
>> This used to be signalled with a combination of cleared pointers in
>> struct drm_fb_helper,
> I failed to find where we clear the pointers. What do I miss?

Those pointer fields, dev and funcs, where allocated with kzalloc(). The 
error path in drm_fbdev_client_hotplug() later reset them to NULL again 
if an error occured.

Best regards
Thomas

> (I had assumed we would stop clearing the pointers after this change).
> 
> 	Sam
> 
> which prevents us from initializing these pointers
>> early after allocation.
>>
>> The change also harmonizes behavior among DRM clients. Additional DRM
>> clients will now handle failed hotplugging like fbdev does.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>>   drivers/gpu/drm/drm_client.c        | 5 +++++
>>   drivers/gpu/drm/drm_fbdev_generic.c | 4 ----
>>   include/drm/drm_client.h            | 8 ++++++++
>>   3 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index 09ac191c202d..009e7b10455c 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -208,8 +208,13 @@ void drm_client_dev_hotplug(struct drm_device *dev)
>>   		if (!client->funcs || !client->funcs->hotplug)
>>   			continue;
>>   
>> +		if (client->hotplug_failed)
>> +			continue;
>> +
>>   		ret = client->funcs->hotplug(client);
>>   		drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
>> +		if (ret)
>> +			client->hotplug_failed = true;
>>   	}
>>   	mutex_unlock(&dev->clientlist_mutex);
>>   }
>> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
>> index 3d455a2e3fb5..135d58b8007b 100644
>> --- a/drivers/gpu/drm/drm_fbdev_generic.c
>> +++ b/drivers/gpu/drm/drm_fbdev_generic.c
>> @@ -382,10 +382,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>>   	struct drm_device *dev = client->dev;
>>   	int ret;
>>   
>> -	/* Setup is not retried if it has failed */
>> -	if (!fb_helper->dev && fb_helper->funcs)
>> -		return 0;
>> -
>>   	if (dev->fb_helper)
>>   		return drm_fb_helper_hotplug_event(dev->fb_helper);
>>   
>> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
>> index 4fc8018eddda..39482527a775 100644
>> --- a/include/drm/drm_client.h
>> +++ b/include/drm/drm_client.h
>> @@ -106,6 +106,14 @@ struct drm_client_dev {
>>   	 * @modesets: CRTC configurations
>>   	 */
>>   	struct drm_mode_set *modesets;
>> +
>> +	/**
>> +	 * @hotplug failed:
>> +	 *
>> +	 * Set by client hotplug helpers if the hotplugging failed
>> +	 * before. It is usually not tried again.
>> +	 */
>> +	bool hotplug_failed;
>>   };
>>   
>>   int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
>> -- 
>> 2.39.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: Ivo Totev

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

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

* Re: [PATCH v3 04/10] drm/fbdev-generic: Initialize fb-helper structure in generic setup
  2023-01-25 21:03   ` Sam Ravnborg
@ 2023-01-27 14:21     ` Thomas Zimmermann
  2023-01-27 17:35       ` Sam Ravnborg
  0 siblings, 1 reply; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-27 14:21 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: freedreno, linux-samsung-soc, amd-gfx, linux-arm-msm, intel-gfx,
	maarten.lankhorst, javierm, mripard, dri-devel, daniel,
	linux-tegra, airlied, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 3176 bytes --]

Hi

Am 25.01.23 um 22:03 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Wed, Jan 25, 2023 at 09:04:09PM +0100, Thomas Zimmermann wrote:
>> Initialize the fb-helper structure immediately after its allocation
>> in drm_fbdev_generic_setup(). That will make it easier to fill it with
>> driver-specific values, such as the preferred BPP.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>>   drivers/gpu/drm/drm_fbdev_generic.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
>> index 135d58b8007b..63f66325a8a5 100644
>> --- a/drivers/gpu/drm/drm_fbdev_generic.c
>> +++ b/drivers/gpu/drm/drm_fbdev_generic.c
>> @@ -385,8 +385,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>>   	if (dev->fb_helper)
>>   		return drm_fb_helper_hotplug_event(dev->fb_helper);
>>   
>> -	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
>> -
>>   	ret = drm_fb_helper_init(dev, fb_helper);
>>   	if (ret)
>>   		goto err;
> 
>  From the documentation:
> The drm_fb_helper_prepare()
> helper must be called first to initialize the minimum required to make
> hotplug detection work.
> ...
> To finish up the fbdev helper initialization, the
> drm_fb_helper_init() function is called.
> 
> So this change do not follow the documentation as drm_fb_helper_init()
> is now called before drm_fb_helper_prepare()

No, we now call drm_fb_helper_prepare() from within 
drm_fbdev_generic_setup(), right after allocating the fb_helper. 
drm_fb_helper_init() will only be called after the client received a 
hot-plug event.

> 
> I did not follow all the code - but my gut feeling is that the
> documentation is right.

The docs are of low quality. The _prepare() helper is the actual init 
function and _init() only sets the fb_helper in the device instance.

Best regards
Thomas

> 
> 	Sam
> 
> 
>> @@ -456,12 +454,12 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
>>   	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
>>   	if (!fb_helper)
>>   		return;
>> +	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
>>   
>>   	ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs);
>>   	if (ret) {
>> -		kfree(fb_helper);
>>   		drm_err(dev, "Failed to register client: %d\n", ret);
>> -		return;
>> +		goto err_drm_client_init;
>>   	}
>>   
>>   	/*
>> @@ -484,5 +482,12 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
>>   		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
>>   
>>   	drm_client_register(&fb_helper->client);
>> +
>> +	return;
>> +
>> +err_drm_client_init:
>> +	drm_fb_helper_unprepare(fb_helper);
>> +	kfree(fb_helper);
>> +	return;
>>   }
>>   EXPORT_SYMBOL(drm_fbdev_generic_setup);
>> -- 
>> 2.39.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: Ivo Totev

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

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

* Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
  2023-01-27 14:13     ` Thomas Zimmermann
@ 2023-01-27 17:33       ` Sam Ravnborg
  0 siblings, 0 replies; 21+ messages in thread
From: Sam Ravnborg @ 2023-01-27 17:33 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, javierm, amd-gfx,
	dri-devel, linux-tegra, freedreno, linux-arm-kernel

On Fri, Jan 27, 2023 at 03:13:50PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 25.01.23 um 21:52 schrieb Sam Ravnborg:
> > Hi Thomas,
> > 
> > On Wed, Jan 25, 2023 at 09:04:06PM +0100, Thomas Zimmermann wrote:
> > > Test for connectors in the client code and remove a similar test
> > > from the generic fbdev emulation. Do nothing if the test fails.
> > > Not having connectors indicates a driver bug.
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> > > ---
> > >   drivers/gpu/drm/drm_client.c        | 5 +++++
> > >   drivers/gpu/drm/drm_fbdev_generic.c | 5 -----
> > >   2 files changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> > > index 262ec64d4397..09ac191c202d 100644
> > > --- a/drivers/gpu/drm/drm_client.c
> > > +++ b/drivers/gpu/drm/drm_client.c
> > > @@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev)
> > >   	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > >   		return;
> > > +	if (!dev->mode_config.num_connector) {
> > > +		drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n");
> > > +		return;
> > This deserves a more visible logging - if a driver fails here it would
> > be good to spot it in the normal kernel log.
> > drm_info or drm_notice?
> 
> But is that really noteworthy? AFAIK, this situation can legally happen. So
> if it's expected, why should we print a message about it?
I was reading it as a driver error - as that's not the case current code
is fine.

	Sam

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

* Re: [PATCH v3 04/10] drm/fbdev-generic: Initialize fb-helper structure in generic setup
  2023-01-27 14:21     ` Thomas Zimmermann
@ 2023-01-27 17:35       ` Sam Ravnborg
  0 siblings, 0 replies; 21+ messages in thread
From: Sam Ravnborg @ 2023-01-27 17:35 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: freedreno, linux-samsung-soc, amd-gfx, linux-arm-msm, intel-gfx,
	maarten.lankhorst, javierm, mripard, dri-devel, daniel,
	linux-tegra, airlied, linux-arm-kernel

On Fri, Jan 27, 2023 at 03:21:30PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 25.01.23 um 22:03 schrieb Sam Ravnborg:
> > Hi Thomas,
> > 
> > On Wed, Jan 25, 2023 at 09:04:09PM +0100, Thomas Zimmermann wrote:
> > > Initialize the fb-helper structure immediately after its allocation
> > > in drm_fbdev_generic_setup(). That will make it easier to fill it with
> > > driver-specific values, such as the preferred BPP.
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> > > ---
> > >   drivers/gpu/drm/drm_fbdev_generic.c | 13 +++++++++----
> > >   1 file changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c
> > > index 135d58b8007b..63f66325a8a5 100644
> > > --- a/drivers/gpu/drm/drm_fbdev_generic.c
> > > +++ b/drivers/gpu/drm/drm_fbdev_generic.c
> > > @@ -385,8 +385,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
> > >   	if (dev->fb_helper)
> > >   		return drm_fb_helper_hotplug_event(dev->fb_helper);
> > > -	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
> > > -
> > >   	ret = drm_fb_helper_init(dev, fb_helper);
> > >   	if (ret)
> > >   		goto err;
> > 
> >  From the documentation:
> > The drm_fb_helper_prepare()
> > helper must be called first to initialize the minimum required to make
> > hotplug detection work.
> > ...
> > To finish up the fbdev helper initialization, the
> > drm_fb_helper_init() function is called.
> > 
> > So this change do not follow the documentation as drm_fb_helper_init()
> > is now called before drm_fb_helper_prepare()
> 
> No, we now call drm_fb_helper_prepare() from within
> drm_fbdev_generic_setup(), right after allocating the fb_helper.
> drm_fb_helper_init() will only be called after the client received a
> hot-plug event.
> 
> > 
> > I did not follow all the code - but my gut feeling is that the
> > documentation is right.
> 
> The docs are of low quality. The _prepare() helper is the actual init
> function and _init() only sets the fb_helper in the device instance.
OK, thanks for the follow-up.


	Sam

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

* Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
  2023-01-25 20:04 ` [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event Thomas Zimmermann
  2023-01-25 20:52   ` Sam Ravnborg
@ 2023-01-27 18:02   ` Simon Ser
  2023-01-30  8:40     ` Thomas Zimmermann
  1 sibling, 1 reply; 21+ messages in thread
From: Simon Ser @ 2023-01-27 18:02 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: freedreno, linux-samsung-soc, amd-gfx, linux-arm-msm, intel-gfx,
	maarten.lankhorst, javierm, mripard, dri-devel, daniel,
	linux-tegra, airlied, linux-arm-kernel

On Wednesday, January 25th, 2023 at 21:04, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Not having connectors indicates a driver bug.

Is it? What if all connectors are of the DP-MST type, ie. they are
created on-the-fly?

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

* Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
  2023-01-27 18:02   ` Simon Ser
@ 2023-01-30  8:40     ` Thomas Zimmermann
  0 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2023-01-30  8:40 UTC (permalink / raw)
  To: Simon Ser
  Cc: linux-samsung-soc, linux-arm-msm, intel-gfx, javierm, amd-gfx,
	dri-devel, linux-tegra, freedreno, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 607 bytes --]

Hi

Am 27.01.23 um 19:02 schrieb Simon Ser:
> On Wednesday, January 25th, 2023 at 21:04, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
>> Not having connectors indicates a driver bug.
> 
> Is it? What if all connectors are of the DP-MST type, ie. they are
> created on-the-fly?

My commit message was nonsense. I even write this here that having no 
connectors is legitimate.

Best regards
Thomas


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

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

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

end of thread, other threads:[~2023-01-30  8:40 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 20:04 [PATCH v3 00/10] drm/fb-helper: Various cleanups Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event Thomas Zimmermann
2023-01-25 20:52   ` Sam Ravnborg
2023-01-27 14:13     ` Thomas Zimmermann
2023-01-27 17:33       ` Sam Ravnborg
2023-01-27 18:02   ` Simon Ser
2023-01-30  8:40     ` Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 02/10] drm/client: Add hotplug_failed flag Thomas Zimmermann
2023-01-25 20:57   ` Sam Ravnborg
2023-01-27 14:17     ` Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 03/10] drm/fb-helper: Introduce drm_fb_helper_unprepare() Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 04/10] drm/fbdev-generic: Initialize fb-helper structure in generic setup Thomas Zimmermann
2023-01-25 21:03   ` Sam Ravnborg
2023-01-27 14:21     ` Thomas Zimmermann
2023-01-27 17:35       ` Sam Ravnborg
2023-01-25 20:04 ` [PATCH v3 05/10] drm/fb-helper: Remove preferred_bpp parameter from fbdev internals Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 06/10] drm/fb-helper: Initialize fb-helper's preferred BPP in prepare function Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 07/10] drm/fbdev-generic: Minimize hotplug error handling Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 08/10] drm/fbdev-generic: Minimize client unregistering Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 09/10] drm/fbdev-generic: Inline clean-up helpers into drm_fbdev_fb_destroy() Thomas Zimmermann
2023-01-25 20:04 ` [PATCH v3 10/10] drm/fbdev-generic: Rename struct fb_info 'fbi' to 'info' Thomas Zimmermann

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