dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
@ 2023-03-16  9:37 Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 01/10] drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers Thomas Zimmermann
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Convert radeon's fbdev code to drm_client. Replaces the current
ad-hoc integration. The conversion includes a number of cleanups.
Only build fbdev support if the config option has been set.

Thomas Zimmermann (10):
  drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
  drm/radeon: Improve fbdev object-test helper
  drm/radeon: Remove struct radeon_fbdev
  drm/radeon: Remove test for !screen_base in fbdev probing
  drm/radeon: Move fbdev object helpers before struct fb_ops et al
  drm/radeon: Fix coding style in fbdev emulation
  drm/radeon: Move fbdev cleanup code into fb_destroy callback
  drm/radeon: Correctly clean up failed display probing
  drm/radeon: Implement client-based fbdev emulation
  drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set

 drivers/gpu/drm/radeon/Makefile         |   3 +-
 drivers/gpu/drm/radeon/radeon.h         |   2 +
 drivers/gpu/drm/radeon/radeon_display.c |   4 -
 drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
 drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
 drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
 drivers/gpu/drm/radeon/radeon_fbdev.c   | 422 ++++++++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
 drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
 drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
 10 files changed, 464 insertions(+), 433 deletions(-)
 delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
 create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c


base-commit: ec0708e846b819c8d5b642de42448a87d7526564
-- 
2.39.2


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

* [PATCH 01/10] drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 02/10] drm/radeon: Improve fbdev object-test helper Thomas Zimmermann
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Move radeon_align_pitch() next to its caller in the dumb-buffer
code. Removes the only dependency on the radeon's fbdev source file
that is not related to fbdev emulation. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon.h      |  2 ++
 drivers/gpu/drm/radeon/radeon_fb.c   | 25 -------------------------
 drivers/gpu/drm/radeon/radeon_gem.c  | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_mode.h |  2 --
 4 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index d19a4b1c1a8f..8afb03bbce29 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -530,6 +530,8 @@ struct radeon_gem {
 
 extern const struct drm_gem_object_funcs radeon_gem_object_funcs;
 
+int radeon_align_pitch(struct radeon_device *rdev, int width, int cpp, bool tiled);
+
 int radeon_gem_init(struct radeon_device *rdev);
 void radeon_gem_fini(struct radeon_device *rdev);
 int radeon_gem_object_create(struct radeon_device *rdev, unsigned long size,
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index c4807f0c43bc..bbb0de2196d3 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -87,31 +87,6 @@ static const struct fb_ops radeonfb_ops = {
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
 };
 
-
-int radeon_align_pitch(struct radeon_device *rdev, int width, int cpp, bool tiled)
-{
-	int aligned = width;
-	int align_large = (ASIC_IS_AVIVO(rdev)) || tiled;
-	int pitch_mask = 0;
-
-	switch (cpp) {
-	case 1:
-		pitch_mask = align_large ? 255 : 127;
-		break;
-	case 2:
-		pitch_mask = align_large ? 127 : 31;
-		break;
-	case 3:
-	case 4:
-		pitch_mask = align_large ? 63 : 15;
-		break;
-	}
-
-	aligned += pitch_mask;
-	aligned &= ~pitch_mask;
-	return aligned * cpp;
-}
-
 static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
 {
 	struct radeon_bo *rbo = gem_to_radeon_bo(gobj);
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 261fcbae88d7..bdc5af23f005 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -822,6 +822,30 @@ int radeon_gem_op_ioctl(struct drm_device *dev, void *data,
 	return r;
 }
 
+int radeon_align_pitch(struct radeon_device *rdev, int width, int cpp, bool tiled)
+{
+	int aligned = width;
+	int align_large = (ASIC_IS_AVIVO(rdev)) || tiled;
+	int pitch_mask = 0;
+
+	switch (cpp) {
+	case 1:
+		pitch_mask = align_large ? 255 : 127;
+		break;
+	case 2:
+		pitch_mask = align_large ? 127 : 31;
+		break;
+	case 3:
+	case 4:
+		pitch_mask = align_large ? 63 : 15;
+		break;
+	}
+
+	aligned += pitch_mask;
+	aligned &= ~pitch_mask;
+	return aligned * cpp;
+}
+
 int radeon_mode_dumb_create(struct drm_file *file_priv,
 			    struct drm_device *dev,
 			    struct drm_mode_create_dumb *args)
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 3a59d016e8cd..63724ecb8d1b 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -952,8 +952,6 @@ void radeon_crtc_handle_vblank(struct radeon_device *rdev, int crtc_id);
 
 void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id);
 
-int radeon_align_pitch(struct radeon_device *rdev, int width, int bpp, bool tiled);
-
 int radeon_atom_pick_dig_encoder(struct drm_encoder *encoder, int fe_idx);
 void radeon_atom_release_dig_encoder(struct radeon_device *rdev, int enc_idx);
 #endif
-- 
2.39.2


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

* [PATCH 02/10] drm/radeon: Improve fbdev object-test helper
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 01/10] drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 03/10] drm/radeon: Remove struct radeon_fbdev Thomas Zimmermann
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Look up the framebuffer GEM object in fbdev object test with the
respective helper drm_gem_fb_get_obj(). The look-up helper warns if
no GEM object has been installed. Upcasting types prevents runtime
type checking, so avoid upcast to struct radeon_bo.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_fb.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index bbb0de2196d3..b1700ce8166a 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -35,6 +35,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/radeon_drm.h>
 
 #include "radeon.h"
@@ -366,10 +367,17 @@ void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state)
 
 bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj)
 {
-	if (!rdev->mode_info.rfbdev)
+	struct drm_fb_helper *fb_helper = rdev->ddev->fb_helper;
+	struct drm_gem_object *gobj;
+
+	if (!fb_helper)
+		return false;
+
+	gobj = drm_gem_fb_get_obj(fb_helper->fb, 0);
+	if (!gobj)
+		return false;
+	if (gobj != &robj->tbo.base)
 		return false;
 
-	if (robj == gem_to_radeon_bo(rdev->mode_info.rfbdev->fb.obj[0]))
-		return true;
-	return false;
+	return true;
 }
-- 
2.39.2


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

* [PATCH 03/10] drm/radeon: Remove struct radeon_fbdev
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 01/10] drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 02/10] drm/radeon: Improve fbdev object-test helper Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 04/10] drm/radeon: Remove test for !screen_base in fbdev probing Thomas Zimmermann
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Both data fields in struct radeon_fbdev, the framebuffer and the
device, are already available in struct drm_fb_helper. Simplify
radeon by converting all callers and removing struct radeon_fbdev.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_fb.c   | 100 ++++++++++++---------------
 drivers/gpu/drm/radeon/radeon_mode.h |   4 --
 2 files changed, 46 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index b1700ce8166a..cab10c40b49c 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -40,21 +40,11 @@
 
 #include "radeon.h"
 
-/* object hierarchy -
- * this contains a helper + a radeon fb
- * the helper contains a pointer to radeon framebuffer baseclass.
- */
-struct radeon_fbdev {
-	struct drm_fb_helper helper; /* must be first */
-	struct drm_framebuffer fb;
-	struct radeon_device *rdev;
-};
-
 static int
 radeonfb_open(struct fb_info *info, int user)
 {
-	struct radeon_fbdev *rfbdev = info->par;
-	struct radeon_device *rdev = rfbdev->rdev;
+	struct drm_fb_helper *fb_helper = info->par;
+	struct radeon_device *rdev = fb_helper->dev->dev_private;
 	int ret = pm_runtime_get_sync(rdev->ddev->dev);
 
 	if (ret < 0 && ret != -EACCES) {
@@ -68,8 +58,8 @@ radeonfb_open(struct fb_info *info, int user)
 static int
 radeonfb_release(struct fb_info *info, int user)
 {
-	struct radeon_fbdev *rfbdev = info->par;
-	struct radeon_device *rdev = rfbdev->rdev;
+	struct drm_fb_helper *fb_helper = info->par;
+	struct radeon_device *rdev = fb_helper->dev->dev_private;
 
 	pm_runtime_mark_last_busy(rdev->ddev->dev);
 	pm_runtime_put_autosuspend(rdev->ddev->dev);
@@ -102,12 +92,12 @@ static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
 	drm_gem_object_put(gobj);
 }
 
-static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev,
+static int radeonfb_create_pinned_object(struct drm_fb_helper *fb_helper,
 					 struct drm_mode_fb_cmd2 *mode_cmd,
 					 struct drm_gem_object **gobj_p)
 {
 	const struct drm_format_info *info;
-	struct radeon_device *rdev = rfbdev->rdev;
+	struct radeon_device *rdev = fb_helper->dev->dev_private;
 	struct drm_gem_object *gobj = NULL;
 	struct radeon_bo *rbo = NULL;
 	bool fb_tiled = false; /* useful for testing */
@@ -191,9 +181,7 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev,
 static int radeonfb_create(struct drm_fb_helper *helper,
 			   struct drm_fb_helper_surface_size *sizes)
 {
-	struct radeon_fbdev *rfbdev =
-		container_of(helper, struct radeon_fbdev, helper);
-	struct radeon_device *rdev = rfbdev->rdev;
+	struct radeon_device *rdev = helper->dev->dev_private;
 	struct fb_info *info;
 	struct drm_framebuffer *fb = NULL;
 	struct drm_mode_fb_cmd2 mode_cmd;
@@ -212,7 +200,7 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
 							  sizes->surface_depth);
 
-	ret = radeonfb_create_pinned_object(rfbdev, &mode_cmd, &gobj);
+	ret = radeonfb_create_pinned_object(helper, &mode_cmd, &gobj);
 	if (ret) {
 		DRM_ERROR("failed to create fbcon object %d\n", ret);
 		return ret;
@@ -230,16 +218,20 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	/* radeon resume is fragile and needs a vt switch to help it along */
 	info->skip_vt_switch = false;
 
-	ret = radeon_framebuffer_init(rdev->ddev, &rfbdev->fb, &mode_cmd, gobj);
+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+	if (!fb) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = radeon_framebuffer_init(rdev->ddev, fb, &mode_cmd, gobj);
 	if (ret) {
 		DRM_ERROR("failed to initialize framebuffer %d\n", ret);
 		goto out;
 	}
 
-	fb = &rfbdev->fb;
-
 	/* setup helper */
-	rfbdev->helper.fb = fb;
+	helper->fb = fb;
 
 	memset_io(rbo->kptr, 0x0, radeon_bo_size(rbo));
 
@@ -251,7 +243,7 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	info->screen_base = rbo->kptr;
 	info->screen_size = radeon_bo_size(rbo);
 
-	drm_fb_helper_fill_info(info, &rfbdev->helper, sizes);
+	drm_fb_helper_fill_info(info, helper, sizes);
 
 	/* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
 
@@ -279,19 +271,23 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	return ret;
 }
 
-static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfbdev)
+static int radeon_fbdev_destroy(struct drm_device *dev, struct drm_fb_helper *fb_helper)
 {
-	struct drm_framebuffer *fb = &rfbdev->fb;
+	struct drm_framebuffer *fb = fb_helper->fb;
 
-	drm_fb_helper_unregister_info(&rfbdev->helper);
+	drm_fb_helper_unregister_info(fb_helper);
 
-	if (fb->obj[0]) {
-		radeonfb_destroy_pinned_object(fb->obj[0]);
-		fb->obj[0] = NULL;
-		drm_framebuffer_unregister_private(fb);
-		drm_framebuffer_cleanup(fb);
+	if (fb) {
+		if (fb->obj[0]) {
+			radeonfb_destroy_pinned_object(fb->obj[0]);
+			fb->obj[0] = NULL;
+			drm_framebuffer_unregister_private(fb);
+			drm_framebuffer_cleanup(fb);
+		}
+		kfree(fb);
+		fb_helper->fb = NULL;
 	}
-	drm_fb_helper_fini(&rfbdev->helper);
+	drm_fb_helper_fini(fb_helper);
 
 	return 0;
 }
@@ -302,7 +298,7 @@ static const struct drm_fb_helper_funcs radeon_fb_helper_funcs = {
 
 int radeon_fbdev_init(struct radeon_device *rdev)
 {
-	struct radeon_fbdev *rfbdev;
+	struct drm_fb_helper *fb_helper;
 	int bpp_sel = 32;
 	int ret;
 
@@ -317,52 +313,48 @@ int radeon_fbdev_init(struct radeon_device *rdev)
 		 rdev->mc.real_vram_size <= (32*1024*1024))
 		bpp_sel = 16;
 
-	rfbdev = kzalloc(sizeof(struct radeon_fbdev), GFP_KERNEL);
-	if (!rfbdev)
+	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
+	if (!fb_helper)
 		return -ENOMEM;
 
-	rfbdev->rdev = rdev;
-	rdev->mode_info.rfbdev = rfbdev;
-
-	drm_fb_helper_prepare(rdev->ddev, &rfbdev->helper, bpp_sel,
-			      &radeon_fb_helper_funcs);
+	drm_fb_helper_prepare(rdev->ddev, fb_helper, bpp_sel, &radeon_fb_helper_funcs);
 
-	ret = drm_fb_helper_init(rdev->ddev, &rfbdev->helper);
+	ret = drm_fb_helper_init(rdev->ddev, fb_helper);
 	if (ret)
 		goto free;
 
 	/* 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);
+	ret = drm_fb_helper_initial_config(fb_helper);
 	if (ret)
 		goto fini;
 
 	return 0;
 
 fini:
-	drm_fb_helper_fini(&rfbdev->helper);
+	drm_fb_helper_fini(fb_helper);
 free:
-	drm_fb_helper_unprepare(&rfbdev->helper);
-	kfree(rfbdev);
+	drm_fb_helper_unprepare(fb_helper);
+	kfree(fb_helper);
 	return ret;
 }
 
 void radeon_fbdev_fini(struct radeon_device *rdev)
 {
-	if (!rdev->mode_info.rfbdev)
+	if (!rdev->ddev->fb_helper)
 		return;
 
-	radeon_fbdev_destroy(rdev->ddev, rdev->mode_info.rfbdev);
-	drm_fb_helper_unprepare(&rdev->mode_info.rfbdev->helper);
-	kfree(rdev->mode_info.rfbdev);
-	rdev->mode_info.rfbdev = NULL;
+	radeon_fbdev_destroy(rdev->ddev, rdev->ddev->fb_helper);
+	drm_fb_helper_unprepare(rdev->ddev->fb_helper);
+	kfree(rdev->ddev->fb_helper);
+	rdev->ddev->fb_helper = NULL;
 }
 
 void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state)
 {
-	if (rdev->mode_info.rfbdev)
-		drm_fb_helper_set_suspend(&rdev->mode_info.rfbdev->helper, state);
+	if (rdev->ddev->fb_helper)
+		drm_fb_helper_set_suspend(rdev->ddev->fb_helper, state);
 }
 
 bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj)
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 63724ecb8d1b..64cf263ae646 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -229,8 +229,6 @@ enum radeon_dvo_chip {
 	DVO_SIL1178,
 };
 
-struct radeon_fbdev;
-
 struct radeon_afmt {
 	bool enabled;
 	int offset;
@@ -267,8 +265,6 @@ struct radeon_mode_info {
 	struct edid *bios_hardcoded_edid;
 	int bios_hardcoded_edid_size;
 
-	/* pointer to fbdev info structure */
-	struct radeon_fbdev *rfbdev;
 	/* firmware flags */
 	u16 firmware_flags;
 	/* pointer to backlight encoder */
-- 
2.39.2


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

* [PATCH 04/10] drm/radeon: Remove test for !screen_base in fbdev probing
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2023-03-16  9:37 ` [PATCH 03/10] drm/radeon: Remove struct radeon_fbdev Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 05/10] drm/radeon: Move fbdev object helpers before struct fb_ops et al Thomas Zimmermann
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

The screen_base field comes from the fbdev BO and contains the fbdev
framebuffer base address. We get the BO memory via radeon_bo_kmap(),
which already reports the error -ENOMEM if the buffer could not be
mapped. So remove the later test for screen_base, which will never
be NULL at this point.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_fb.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index cab10c40b49c..7af038ed0d2d 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -247,11 +247,6 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 
 	/* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
 
-	if (info->screen_base == NULL) {
-		ret = -ENOSPC;
-		goto out;
-	}
-
 	DRM_INFO("fb mappable at 0x%lX\n",  info->fix.smem_start);
 	DRM_INFO("vram apper at 0x%lX\n",  (unsigned long)rdev->mc.aper_base);
 	DRM_INFO("size %lu\n", (unsigned long)radeon_bo_size(rbo));
-- 
2.39.2


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

* [PATCH 05/10] drm/radeon: Move fbdev object helpers before struct fb_ops et al
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2023-03-16  9:37 ` [PATCH 04/10] drm/radeon: Remove test for !screen_base in fbdev probing Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 06/10] drm/radeon: Fix coding style in fbdev emulation Thomas Zimmermann
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Move the helpers for creating and destroying fbdev GEM objects
to the top of the source file. Makes them available to fb_ops
functions. This will allow to implement framebuffer cleanup in
fb_destroy. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_fb.c | 76 +++++++++++++++---------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 7af038ed0d2d..152563c6e55a 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -40,44 +40,6 @@
 
 #include "radeon.h"
 
-static int
-radeonfb_open(struct fb_info *info, int user)
-{
-	struct drm_fb_helper *fb_helper = info->par;
-	struct radeon_device *rdev = fb_helper->dev->dev_private;
-	int ret = pm_runtime_get_sync(rdev->ddev->dev);
-
-	if (ret < 0 && ret != -EACCES) {
-		pm_runtime_mark_last_busy(rdev->ddev->dev);
-		pm_runtime_put_autosuspend(rdev->ddev->dev);
-		return ret;
-	}
-	return 0;
-}
-
-static int
-radeonfb_release(struct fb_info *info, int user)
-{
-	struct drm_fb_helper *fb_helper = info->par;
-	struct radeon_device *rdev = fb_helper->dev->dev_private;
-
-	pm_runtime_mark_last_busy(rdev->ddev->dev);
-	pm_runtime_put_autosuspend(rdev->ddev->dev);
-	return 0;
-}
-
-static const struct fb_ops radeonfb_ops = {
-	.owner = THIS_MODULE,
-	DRM_FB_HELPER_DEFAULT_OPS,
-	.fb_open = radeonfb_open,
-	.fb_release = radeonfb_release,
-	.fb_read = drm_fb_helper_cfb_read,
-	.fb_write = drm_fb_helper_cfb_write,
-	.fb_fillrect = drm_fb_helper_cfb_fillrect,
-	.fb_copyarea = drm_fb_helper_cfb_copyarea,
-	.fb_imageblit = drm_fb_helper_cfb_imageblit,
-};
-
 static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
 {
 	struct radeon_bo *rbo = gem_to_radeon_bo(gobj);
@@ -178,6 +140,44 @@ static int radeonfb_create_pinned_object(struct drm_fb_helper *fb_helper,
 	return ret;
 }
 
+static int
+radeonfb_open(struct fb_info *info, int user)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct radeon_device *rdev = fb_helper->dev->dev_private;
+	int ret = pm_runtime_get_sync(rdev->ddev->dev);
+
+	if (ret < 0 && ret != -EACCES) {
+		pm_runtime_mark_last_busy(rdev->ddev->dev);
+		pm_runtime_put_autosuspend(rdev->ddev->dev);
+		return ret;
+	}
+	return 0;
+}
+
+static int
+radeonfb_release(struct fb_info *info, int user)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct radeon_device *rdev = fb_helper->dev->dev_private;
+
+	pm_runtime_mark_last_busy(rdev->ddev->dev);
+	pm_runtime_put_autosuspend(rdev->ddev->dev);
+	return 0;
+}
+
+static const struct fb_ops radeonfb_ops = {
+	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
+	.fb_open = radeonfb_open,
+	.fb_release = radeonfb_release,
+	.fb_read = drm_fb_helper_cfb_read,
+	.fb_write = drm_fb_helper_cfb_write,
+	.fb_fillrect = drm_fb_helper_cfb_fillrect,
+	.fb_copyarea = drm_fb_helper_cfb_copyarea,
+	.fb_imageblit = drm_fb_helper_cfb_imageblit,
+};
+
 static int radeonfb_create(struct drm_fb_helper *helper,
 			   struct drm_fb_helper_surface_size *sizes)
 {
-- 
2.39.2


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

* [PATCH 06/10] drm/radeon: Fix coding style in fbdev emulation
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
                   ` (4 preceding siblings ...)
  2023-03-16  9:37 ` [PATCH 05/10] drm/radeon: Move fbdev object helpers before struct fb_ops et al Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 07/10] drm/radeon: Move fbdev cleanup code into fb_destroy callback Thomas Zimmermann
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Fix the coding style in several places in the fbdev-emulation
code. Also rename functions and structure file by comments. No
functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_fb.c | 81 +++++++++++++++++-------------
 1 file changed, 46 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 152563c6e55a..f55aaea10406 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -40,7 +40,7 @@
 
 #include "radeon.h"
 
-static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
+static void radeon_fbdev_destroy_pinned_object(struct drm_gem_object *gobj)
 {
 	struct radeon_bo *rbo = gem_to_radeon_bo(gobj);
 	int ret;
@@ -54,9 +54,9 @@ static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
 	drm_gem_object_put(gobj);
 }
 
-static int radeonfb_create_pinned_object(struct drm_fb_helper *fb_helper,
-					 struct drm_mode_fb_cmd2 *mode_cmd,
-					 struct drm_gem_object **gobj_p)
+static int radeon_fbdev_create_pinned_object(struct drm_fb_helper *fb_helper,
+					     struct drm_mode_fb_cmd2 *mode_cmd,
+					     struct drm_gem_object **gobj_p)
 {
 	const struct drm_format_info *info;
 	struct radeon_device *rdev = fb_helper->dev->dev_private;
@@ -113,64 +113,71 @@ static int radeonfb_create_pinned_object(struct drm_fb_helper *fb_helper,
 			dev_err(rdev->dev, "FB failed to set tiling flags\n");
 	}
 
-
 	ret = radeon_bo_reserve(rbo, false);
 	if (unlikely(ret != 0))
-		goto out_unref;
+		goto err_radeon_fbdev_destroy_pinned_object;
 	/* Only 27 bit offset for legacy CRTC */
 	ret = radeon_bo_pin_restricted(rbo, RADEON_GEM_DOMAIN_VRAM,
 				       ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27,
 				       NULL);
 	if (ret) {
 		radeon_bo_unreserve(rbo);
-		goto out_unref;
+		goto err_radeon_fbdev_destroy_pinned_object;
 	}
 	if (fb_tiled)
 		radeon_bo_check_tiling(rbo, 0, 0);
 	ret = radeon_bo_kmap(rbo, NULL);
 	radeon_bo_unreserve(rbo);
 	if (ret)
-		goto out_unref;
+		goto err_radeon_fbdev_destroy_pinned_object;
 
 	*gobj_p = gobj;
 	return 0;
-out_unref:
-	radeonfb_destroy_pinned_object(gobj);
+
+err_radeon_fbdev_destroy_pinned_object:
+	radeon_fbdev_destroy_pinned_object(gobj);
 	*gobj_p = NULL;
 	return ret;
 }
 
-static int
-radeonfb_open(struct fb_info *info, int user)
+/*
+ * Fbdev ops and struct fb_ops
+ */
+
+static int radeon_fbdev_fb_open(struct fb_info *info, int user)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 	struct radeon_device *rdev = fb_helper->dev->dev_private;
-	int ret = pm_runtime_get_sync(rdev->ddev->dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(rdev->ddev->dev);
+	if (ret < 0 && ret != -EACCES)
+		goto err_pm_runtime_mark_last_busy;
 
-	if (ret < 0 && ret != -EACCES) {
-		pm_runtime_mark_last_busy(rdev->ddev->dev);
-		pm_runtime_put_autosuspend(rdev->ddev->dev);
-		return ret;
-	}
 	return 0;
+
+err_pm_runtime_mark_last_busy:
+	pm_runtime_mark_last_busy(rdev->ddev->dev);
+	pm_runtime_put_autosuspend(rdev->ddev->dev);
+	return ret;
 }
 
-static int
-radeonfb_release(struct fb_info *info, int user)
+static int radeon_fbdev_fb_release(struct fb_info *info, int user)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 	struct radeon_device *rdev = fb_helper->dev->dev_private;
 
 	pm_runtime_mark_last_busy(rdev->ddev->dev);
 	pm_runtime_put_autosuspend(rdev->ddev->dev);
+
 	return 0;
 }
 
-static const struct fb_ops radeonfb_ops = {
+static const struct fb_ops radeon_fbdev_fb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
-	.fb_open = radeonfb_open,
-	.fb_release = radeonfb_release,
+	.fb_open = radeon_fbdev_fb_open,
+	.fb_release = radeon_fbdev_fb_release,
 	.fb_read = drm_fb_helper_cfb_read,
 	.fb_write = drm_fb_helper_cfb_write,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
@@ -178,10 +185,14 @@ static const struct fb_ops radeonfb_ops = {
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
 };
 
-static int radeonfb_create(struct drm_fb_helper *helper,
-			   struct drm_fb_helper_surface_size *sizes)
+/*
+ * Fbdev helpers and struct drm_fb_helper_funcs
+ */
+
+static int radeon_fbdev_fb_helper_fb_probe(struct drm_fb_helper *fb_helper,
+					   struct drm_fb_helper_surface_size *sizes)
 {
-	struct radeon_device *rdev = helper->dev->dev_private;
+	struct radeon_device *rdev = fb_helper->dev->dev_private;
 	struct fb_info *info;
 	struct drm_framebuffer *fb = NULL;
 	struct drm_mode_fb_cmd2 mode_cmd;
@@ -200,7 +211,7 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
 							  sizes->surface_depth);
 
-	ret = radeonfb_create_pinned_object(helper, &mode_cmd, &gobj);
+	ret = radeon_fbdev_create_pinned_object(fb_helper, &mode_cmd, &gobj);
 	if (ret) {
 		DRM_ERROR("failed to create fbcon object %d\n", ret);
 		return ret;
@@ -209,7 +220,7 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	rbo = gem_to_radeon_bo(gobj);
 
 	/* okay we have an object now allocate the framebuffer */
-	info = drm_fb_helper_alloc_info(helper);
+	info = drm_fb_helper_alloc_info(fb_helper);
 	if (IS_ERR(info)) {
 		ret = PTR_ERR(info);
 		goto out;
@@ -231,11 +242,11 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	}
 
 	/* setup helper */
-	helper->fb = fb;
+	fb_helper->fb = fb;
 
 	memset_io(rbo->kptr, 0x0, radeon_bo_size(rbo));
 
-	info->fbops = &radeonfb_ops;
+	info->fbops = &radeon_fbdev_fb_ops;
 
 	tmp = radeon_bo_gpu_offset(rbo) - rdev->mc.vram_start;
 	info->fix.smem_start = rdev->mc.aper_base + tmp;
@@ -243,7 +254,7 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 	info->screen_base = rbo->kptr;
 	info->screen_size = radeon_bo_size(rbo);
 
-	drm_fb_helper_fill_info(info, helper, sizes);
+	drm_fb_helper_fill_info(info, fb_helper, sizes);
 
 	/* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
 
@@ -274,7 +285,7 @@ static int radeon_fbdev_destroy(struct drm_device *dev, struct drm_fb_helper *fb
 
 	if (fb) {
 		if (fb->obj[0]) {
-			radeonfb_destroy_pinned_object(fb->obj[0]);
+			radeon_fbdev_destroy_pinned_object(fb->obj[0]);
 			fb->obj[0] = NULL;
 			drm_framebuffer_unregister_private(fb);
 			drm_framebuffer_cleanup(fb);
@@ -287,8 +298,8 @@ static int radeon_fbdev_destroy(struct drm_device *dev, struct drm_fb_helper *fb
 	return 0;
 }
 
-static const struct drm_fb_helper_funcs radeon_fb_helper_funcs = {
-	.fb_probe = radeonfb_create,
+static const struct drm_fb_helper_funcs radeon_fbdev_fb_helper_funcs = {
+	.fb_probe = radeon_fbdev_fb_helper_fb_probe,
 };
 
 int radeon_fbdev_init(struct radeon_device *rdev)
@@ -312,7 +323,7 @@ int radeon_fbdev_init(struct radeon_device *rdev)
 	if (!fb_helper)
 		return -ENOMEM;
 
-	drm_fb_helper_prepare(rdev->ddev, fb_helper, bpp_sel, &radeon_fb_helper_funcs);
+	drm_fb_helper_prepare(rdev->ddev, fb_helper, bpp_sel, &radeon_fbdev_fb_helper_funcs);
 
 	ret = drm_fb_helper_init(rdev->ddev, fb_helper);
 	if (ret)
-- 
2.39.2


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

* [PATCH 07/10] drm/radeon: Move fbdev cleanup code into fb_destroy callback
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
                   ` (5 preceding siblings ...)
  2023-03-16  9:37 ` [PATCH 06/10] drm/radeon: Fix coding style in fbdev emulation Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 08/10] drm/radeon: Correctly clean up failed display probing Thomas Zimmermann
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Fbdev calls struct fb_ops.fb_destroy after cleaning up the final
reference to an fbdev framebuffer. Move radeon's fbdev cleanup code
there.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_fb.c | 43 +++++++++++++++---------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index f55aaea10406..d85f99b5aa49 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -173,6 +173,25 @@ static int radeon_fbdev_fb_release(struct fb_info *info, int user)
 	return 0;
 }
 
+static void radeon_fbdev_fb_destroy(struct fb_info *info)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_framebuffer *fb = fb_helper->fb;
+
+	if (fb) {
+		if (fb->obj[0]) {
+			radeon_fbdev_destroy_pinned_object(fb->obj[0]);
+			fb->obj[0] = NULL;
+			drm_framebuffer_unregister_private(fb);
+			drm_framebuffer_cleanup(fb);
+		}
+		kfree(fb);
+		fb_helper->fb = NULL;
+	}
+
+	drm_fb_helper_fini(fb_helper);
+}
+
 static const struct fb_ops radeon_fbdev_fb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
@@ -183,6 +202,7 @@ static const struct fb_ops radeon_fbdev_fb_ops = {
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
 	.fb_copyarea = drm_fb_helper_cfb_copyarea,
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
+	.fb_destroy = radeon_fbdev_fb_destroy,
 };
 
 /*
@@ -277,27 +297,6 @@ static int radeon_fbdev_fb_helper_fb_probe(struct drm_fb_helper *fb_helper,
 	return ret;
 }
 
-static int radeon_fbdev_destroy(struct drm_device *dev, struct drm_fb_helper *fb_helper)
-{
-	struct drm_framebuffer *fb = fb_helper->fb;
-
-	drm_fb_helper_unregister_info(fb_helper);
-
-	if (fb) {
-		if (fb->obj[0]) {
-			radeon_fbdev_destroy_pinned_object(fb->obj[0]);
-			fb->obj[0] = NULL;
-			drm_framebuffer_unregister_private(fb);
-			drm_framebuffer_cleanup(fb);
-		}
-		kfree(fb);
-		fb_helper->fb = NULL;
-	}
-	drm_fb_helper_fini(fb_helper);
-
-	return 0;
-}
-
 static const struct drm_fb_helper_funcs radeon_fbdev_fb_helper_funcs = {
 	.fb_probe = radeon_fbdev_fb_helper_fb_probe,
 };
@@ -351,7 +350,7 @@ void radeon_fbdev_fini(struct radeon_device *rdev)
 	if (!rdev->ddev->fb_helper)
 		return;
 
-	radeon_fbdev_destroy(rdev->ddev, rdev->ddev->fb_helper);
+	drm_fb_helper_unregister_info(rdev->ddev->fb_helper);
 	drm_fb_helper_unprepare(rdev->ddev->fb_helper);
 	kfree(rdev->ddev->fb_helper);
 	rdev->ddev->fb_helper = NULL;
-- 
2.39.2


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

* [PATCH 08/10] drm/radeon: Correctly clean up failed display probing
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
                   ` (6 preceding siblings ...)
  2023-03-16  9:37 ` [PATCH 07/10] drm/radeon: Move fbdev cleanup code into fb_destroy callback Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 09/10] drm/radeon: Implement client-based fbdev emulation Thomas Zimmermann
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Improve the fbdev probing function to fully clean up if it failed.
Allows to remove special cases from fb_destroy as well.

This change is reorders the operations within radeonfb_probe(). It
first allocates a buffer object, then builds a DRM framebuffer for
the object and finally creates the fbdev device. If every step
succeeded, the probe function clears the framebuffer memory. This
is the optimal order to rollback any changes if any of the steps
fails.

No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_fb.c | 72 ++++++++++++++----------------
 1 file changed, 34 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index d85f99b5aa49..2b629bb23be8 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -177,17 +177,14 @@ static void radeon_fbdev_fb_destroy(struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_framebuffer *fb = fb_helper->fb;
+	struct drm_gem_object *gobj = drm_gem_fb_get_obj(fb, 0);
 
-	if (fb) {
-		if (fb->obj[0]) {
-			radeon_fbdev_destroy_pinned_object(fb->obj[0]);
-			fb->obj[0] = NULL;
-			drm_framebuffer_unregister_private(fb);
-			drm_framebuffer_cleanup(fb);
-		}
-		kfree(fb);
-		fb_helper->fb = NULL;
-	}
+	drm_framebuffer_unregister_private(fb);
+	drm_framebuffer_cleanup(fb);
+	kfree(fb);
+	fb_helper->fb = NULL;
+
+	radeon_fbdev_destroy_pinned_object(gobj);
 
 	drm_fb_helper_fini(fb_helper);
 }
@@ -213,11 +210,11 @@ static int radeon_fbdev_fb_helper_fb_probe(struct drm_fb_helper *fb_helper,
 					   struct drm_fb_helper_surface_size *sizes)
 {
 	struct radeon_device *rdev = fb_helper->dev->dev_private;
+	struct drm_mode_fb_cmd2 mode_cmd = { };
 	struct fb_info *info;
-	struct drm_framebuffer *fb = NULL;
-	struct drm_mode_fb_cmd2 mode_cmd;
-	struct drm_gem_object *gobj = NULL;
-	struct radeon_bo *rbo = NULL;
+	struct drm_gem_object *gobj;
+	struct radeon_bo *rbo;
+	struct drm_framebuffer *fb;
 	int ret;
 	unsigned long tmp;
 
@@ -236,45 +233,43 @@ static int radeon_fbdev_fb_helper_fb_probe(struct drm_fb_helper *fb_helper,
 		DRM_ERROR("failed to create fbcon object %d\n", ret);
 		return ret;
 	}
-
 	rbo = gem_to_radeon_bo(gobj);
 
-	/* okay we have an object now allocate the framebuffer */
-	info = drm_fb_helper_alloc_info(fb_helper);
-	if (IS_ERR(info)) {
-		ret = PTR_ERR(info);
-		goto out;
-	}
-
-	/* radeon resume is fragile and needs a vt switch to help it along */
-	info->skip_vt_switch = false;
-
 	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
 	if (!fb) {
 		ret = -ENOMEM;
-		goto out;
+		goto err_radeon_fbdev_destroy_pinned_object;
 	}
-
 	ret = radeon_framebuffer_init(rdev->ddev, fb, &mode_cmd, gobj);
 	if (ret) {
 		DRM_ERROR("failed to initialize framebuffer %d\n", ret);
-		goto out;
+		goto err_kfree;
 	}
 
 	/* setup helper */
 	fb_helper->fb = fb;
 
-	memset_io(rbo->kptr, 0x0, radeon_bo_size(rbo));
+	/* okay we have an object now allocate the framebuffer */
+	info = drm_fb_helper_alloc_info(fb_helper);
+	if (IS_ERR(info)) {
+		ret = PTR_ERR(info);
+		goto err_drm_framebuffer_unregister_private;
+	}
 
 	info->fbops = &radeon_fbdev_fb_ops;
+	info->flags = FBINFO_DEFAULT;
+	/* radeon resume is fragile and needs a vt switch to help it along */
+	info->skip_vt_switch = false;
+
+	drm_fb_helper_fill_info(info, fb_helper, sizes);
 
 	tmp = radeon_bo_gpu_offset(rbo) - rdev->mc.vram_start;
 	info->fix.smem_start = rdev->mc.aper_base + tmp;
 	info->fix.smem_len = radeon_bo_size(rbo);
-	info->screen_base = rbo->kptr;
+	info->screen_base = (__force void __iomem *)rbo->kptr;
 	info->screen_size = radeon_bo_size(rbo);
 
-	drm_fb_helper_fill_info(info, fb_helper, sizes);
+	memset_io(info->screen_base, 0, info->screen_size);
 
 	/* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
 
@@ -287,13 +282,14 @@ static int radeon_fbdev_fb_helper_fb_probe(struct drm_fb_helper *fb_helper,
 	vga_switcheroo_client_fb_set(rdev->pdev, info);
 	return 0;
 
-out:
-	if (fb && ret) {
-		drm_gem_object_put(gobj);
-		drm_framebuffer_unregister_private(fb);
-		drm_framebuffer_cleanup(fb);
-		kfree(fb);
-	}
+err_drm_framebuffer_unregister_private:
+	fb_helper->fb = NULL;
+	drm_framebuffer_unregister_private(fb);
+	drm_framebuffer_cleanup(fb);
+err_kfree:
+	kfree(fb);
+err_radeon_fbdev_destroy_pinned_object:
+	radeon_fbdev_destroy_pinned_object(gobj);
 	return ret;
 }
 
-- 
2.39.2


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

* [PATCH 09/10] drm/radeon: Implement client-based fbdev emulation
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
                   ` (7 preceding siblings ...)
  2023-03-16  9:37 ` [PATCH 08/10] drm/radeon: Correctly clean up failed display probing Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-16  9:37 ` [PATCH 10/10] drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set Thomas Zimmermann
  2023-03-17  8:53 ` [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Christian König
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Implement fbdev emulation on top of struct drm_client and its helpers.
Replaces ad-hoc interfaces for restoring and closing fbdev emulation with
per-client callbacks for hotplugging, restoring and unregistering.

A single function, radeon_fbdev_setup(), starts fbdev emulation after
the DRM device has been registered. Hence, fbdev acts like a regular
DRM client.

The setup call prepares the fbdev emulation and invokes connector
hotplugging. The first successful hotplug event initializes fbdev
emulation with a framebuffer, device file, etc.

Unregistering depends on the hotplug status. Fully initialized emulation
is cleaned up through drm_fb_helper_unregister_info() and fb_destroy.
For prepared-only setups, unregistering unprepares the emulation and
releases all resources. In both cases, fbdev emulation will be cleaned
up.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_display.c |   4 -
 drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
 drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
 drivers/gpu/drm/radeon/radeon_fb.c      | 126 ++++++++++++++++--------
 drivers/gpu/drm/radeon/radeon_kms.c     |  18 ----
 drivers/gpu/drm/radeon/radeon_mode.h    |   3 +-
 6 files changed, 89 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index f34a7f63261d..901e75ec70ff 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -34,7 +34,6 @@
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_edid.h>
-#include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -1354,7 +1353,6 @@ radeon_user_framebuffer_create(struct drm_device *dev,
 
 static const struct drm_mode_config_funcs radeon_mode_funcs = {
 	.fb_create = radeon_user_framebuffer_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 };
 
 static const struct drm_prop_enum_list radeon_tmds_pll_enum_list[] =
@@ -1642,7 +1640,6 @@ int radeon_modeset_init(struct radeon_device *rdev)
 	/* setup afmt */
 	radeon_afmt_init(rdev);
 
-	radeon_fbdev_init(rdev);
 	drm_kms_helper_poll_init(rdev->ddev);
 
 	/* do pm late init */
@@ -1657,7 +1654,6 @@ void radeon_modeset_fini(struct radeon_device *rdev)
 		drm_kms_helper_poll_fini(rdev->ddev);
 		radeon_hpd_fini(rdev);
 		drm_helper_force_disable_all(rdev->ddev);
-		radeon_fbdev_fini(rdev);
 		radeon_afmt_fini(rdev);
 		drm_mode_config_cleanup(rdev->ddev);
 		rdev->mode_info.mode_config_initialized = false;
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 716ab85a376b..e4374814f0ef 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -341,6 +341,8 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto err_agp;
 
+	radeon_fbdev_setup(dev->dev_private);
+
 	return 0;
 
 err_agp:
@@ -595,7 +597,6 @@ static const struct drm_driver kms_driver = {
 	.load = radeon_driver_load_kms,
 	.open = radeon_driver_open_kms,
 	.postclose = radeon_driver_postclose_kms,
-	.lastclose = radeon_driver_lastclose_kms,
 	.unload = radeon_driver_unload_kms,
 	.ioctls = radeon_ioctls_kms,
 	.num_ioctls = ARRAY_SIZE(radeon_ioctls_kms),
diff --git a/drivers/gpu/drm/radeon/radeon_drv.h b/drivers/gpu/drm/radeon/radeon_drv.h
index ac7970919c4d..2ffe0975ee54 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.h
+++ b/drivers/gpu/drm/radeon/radeon_drv.h
@@ -120,7 +120,6 @@ long radeon_drm_ioctl(struct file *filp,
 
 int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags);
 void radeon_driver_unload_kms(struct drm_device *dev);
-void radeon_driver_lastclose_kms(struct drm_device *dev);
 int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv);
 void radeon_driver_postclose_kms(struct drm_device *dev,
 				 struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 2b629bb23be8..fe76e29910ef 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -24,19 +24,16 @@
  *     David Airlie
  */
 
-#include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>
-#include <linux/slab.h>
 #include <linux/vga_switcheroo.h>
 
-#include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_framebuffer_helper.h>
-#include <drm/radeon_drm.h>
 
 #include "radeon.h"
 
@@ -179,14 +176,16 @@ static void radeon_fbdev_fb_destroy(struct fb_info *info)
 	struct drm_framebuffer *fb = fb_helper->fb;
 	struct drm_gem_object *gobj = drm_gem_fb_get_obj(fb, 0);
 
+	drm_fb_helper_fini(fb_helper);
+
 	drm_framebuffer_unregister_private(fb);
 	drm_framebuffer_cleanup(fb);
 	kfree(fb);
-	fb_helper->fb = NULL;
-
 	radeon_fbdev_destroy_pinned_object(gobj);
 
-	drm_fb_helper_fini(fb_helper);
+	drm_client_release(&fb_helper->client);
+	drm_fb_helper_unprepare(fb_helper);
+	kfree(fb_helper);
 }
 
 static const struct fb_ops radeon_fbdev_fb_ops = {
@@ -279,7 +278,6 @@ static int radeon_fbdev_fb_helper_fb_probe(struct drm_fb_helper *fb_helper,
 	DRM_INFO("fb depth is %d\n", fb->format->depth);
 	DRM_INFO("   pitch is %d\n", fb->pitches[0]);
 
-	vga_switcheroo_client_fb_set(rdev->pdev, info);
 	return 0;
 
 err_drm_framebuffer_unregister_private:
@@ -297,59 +295,107 @@ static const struct drm_fb_helper_funcs radeon_fbdev_fb_helper_funcs = {
 	.fb_probe = radeon_fbdev_fb_helper_fb_probe,
 };
 
-int radeon_fbdev_init(struct radeon_device *rdev)
+/*
+ * Fbdev client and struct drm_client_funcs
+ */
+
+static void radeon_fbdev_client_unregister(struct drm_client_dev *client)
 {
-	struct drm_fb_helper *fb_helper;
-	int bpp_sel = 32;
-	int ret;
+	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+	struct drm_device *dev = fb_helper->dev;
+	struct radeon_device *rdev = dev->dev_private;
+
+	if (fb_helper->info) {
+		vga_switcheroo_client_fb_set(rdev->pdev, NULL);
+		drm_fb_helper_unregister_info(fb_helper);
+	} else {
+		drm_client_release(&fb_helper->client);
+		drm_fb_helper_unprepare(fb_helper);
+		kfree(fb_helper);
+	}
+}
 
-	/* don't enable fbdev if no connectors */
-	if (list_empty(&rdev->ddev->mode_config.connector_list))
-		return 0;
+static int radeon_fbdev_client_restore(struct drm_client_dev *client)
+{
+	drm_fb_helper_lastclose(client->dev);
+	vga_switcheroo_process_delayed_switch();
 
-	/* select 8 bpp console on 8MB cards, or 16 bpp on RN50 or 32MB */
-	if (rdev->mc.real_vram_size <= (8*1024*1024))
-		bpp_sel = 8;
-	else if (ASIC_IS_RN50(rdev) ||
-		 rdev->mc.real_vram_size <= (32*1024*1024))
-		bpp_sel = 16;
+	return 0;
+}
 
-	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
-	if (!fb_helper)
-		return -ENOMEM;
+static int radeon_fbdev_client_hotplug(struct drm_client_dev *client)
+{
+	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+	struct drm_device *dev = client->dev;
+	struct radeon_device *rdev = dev->dev_private;
+	int ret;
 
-	drm_fb_helper_prepare(rdev->ddev, fb_helper, bpp_sel, &radeon_fbdev_fb_helper_funcs);
+	if (dev->fb_helper)
+		return drm_fb_helper_hotplug_event(dev->fb_helper);
 
-	ret = drm_fb_helper_init(rdev->ddev, fb_helper);
+	ret = drm_fb_helper_init(dev, fb_helper);
 	if (ret)
-		goto free;
+		goto err_drm_err;
 
-	/* disable all the possible outputs/crtcs before entering KMS mode */
-	drm_helper_disable_unused_functions(rdev->ddev);
+	if (!drm_drv_uses_atomic_modeset(dev))
+		drm_helper_disable_unused_functions(dev);
 
 	ret = drm_fb_helper_initial_config(fb_helper);
 	if (ret)
-		goto fini;
+		goto err_drm_fb_helper_fini;
+
+	vga_switcheroo_client_fb_set(rdev->pdev, fb_helper->info);
 
 	return 0;
 
-fini:
+err_drm_fb_helper_fini:
 	drm_fb_helper_fini(fb_helper);
-free:
-	drm_fb_helper_unprepare(fb_helper);
-	kfree(fb_helper);
+err_drm_err:
+	drm_err(dev, "Failed to setup radeon fbdev emulation (ret=%d)\n", ret);
 	return ret;
 }
 
-void radeon_fbdev_fini(struct radeon_device *rdev)
+static const struct drm_client_funcs radeon_fbdev_client_funcs = {
+	.owner		= THIS_MODULE,
+	.unregister	= radeon_fbdev_client_unregister,
+	.restore	= radeon_fbdev_client_restore,
+	.hotplug	= radeon_fbdev_client_hotplug,
+};
+
+void radeon_fbdev_setup(struct radeon_device *rdev)
 {
-	if (!rdev->ddev->fb_helper)
+	struct drm_fb_helper *fb_helper;
+	int bpp_sel = 32;
+	int ret;
+
+	if (rdev->mc.real_vram_size <= (8 * 1024 * 1024))
+		bpp_sel = 8;
+	else if (ASIC_IS_RN50(rdev) || rdev->mc.real_vram_size <= (32 * 1024 * 1024))
+		bpp_sel = 16;
+
+	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
+	if (!fb_helper)
 		return;
+	drm_fb_helper_prepare(rdev->ddev, fb_helper, bpp_sel, &radeon_fbdev_fb_helper_funcs);
+
+	ret = drm_client_init(rdev->ddev, &fb_helper->client, "radeon-fbdev",
+			      &radeon_fbdev_client_funcs);
+	if (ret) {
+		drm_err(rdev->ddev, "Failed to register client: %d\n", ret);
+		goto err_drm_client_init;
+	}
 
-	drm_fb_helper_unregister_info(rdev->ddev->fb_helper);
-	drm_fb_helper_unprepare(rdev->ddev->fb_helper);
-	kfree(rdev->ddev->fb_helper);
-	rdev->ddev->fb_helper = NULL;
+	ret = radeon_fbdev_client_hotplug(&fb_helper->client);
+	if (ret)
+		drm_dbg_kms(rdev->ddev, "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);
 }
 
 void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state)
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 965161b8565b..e0214cf1b43b 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -32,7 +32,6 @@
 #include <linux/uaccess.h>
 #include <linux/vga_switcheroo.h>
 
-#include <drm/drm_fb_helper.h>
 #include <drm/drm_file.h>
 #include <drm/drm_ioctl.h>
 #include <drm/radeon_drm.h>
@@ -622,23 +621,6 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	return 0;
 }
 
-
-/*
- * Outdated mess for old drm with Xorg being in charge (void function now).
- */
-/**
- * radeon_driver_lastclose_kms - drm callback for last close
- *
- * @dev: drm dev pointer
- *
- * Switch vga_switcheroo state after last close (all asics).
- */
-void radeon_driver_lastclose_kms(struct drm_device *dev)
-{
-	drm_fb_helper_lastclose(dev);
-	vga_switcheroo_process_delayed_switch();
-}
-
 /**
  * radeon_driver_open_kms - drm callback for open
  *
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 64cf263ae646..d81f61e5a95c 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -939,8 +939,7 @@ void dce4_program_fmt(struct drm_encoder *encoder);
 void dce8_program_fmt(struct drm_encoder *encoder);
 
 /* fbdev layer */
-int radeon_fbdev_init(struct radeon_device *rdev);
-void radeon_fbdev_fini(struct radeon_device *rdev);
+void radeon_fbdev_setup(struct radeon_device *rdev);
 void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state);
 bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj);
 
-- 
2.39.2


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

* [PATCH 10/10] drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
                   ` (8 preceding siblings ...)
  2023-03-16  9:37 ` [PATCH 09/10] drm/radeon: Implement client-based fbdev emulation Thomas Zimmermann
@ 2023-03-16  9:37 ` Thomas Zimmermann
  2023-03-17  8:53 ` [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Christian König
  10 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-16  9:37 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	javierm
  Cc: Thomas Zimmermann, amd-gfx, dri-devel

Make building fbdev emulation depend on DRM_FBDEV_EMULATION. Also
rename the source file to radeon_fbdev.c to align with other fbdev
files.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/Makefile                       |  3 ++-
 .../gpu/drm/radeon/{radeon_fb.c => radeon_fbdev.c}    |  0
 drivers/gpu/drm/radeon/radeon_mode.h                  | 11 +++++++++++
 3 files changed, 13 insertions(+), 1 deletion(-)
 rename drivers/gpu/drm/radeon/{radeon_fb.c => radeon_fbdev.c} (100%)

diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile
index bb4e56f2f170..a8734b7d0485 100644
--- a/drivers/gpu/drm/radeon/Makefile
+++ b/drivers/gpu/drm/radeon/Makefile
@@ -36,7 +36,7 @@ radeon-y += radeon_device.o radeon_asic.o radeon_kms.o \
 	atom.o radeon_fence.o radeon_ttm.o radeon_object.o radeon_gart.o \
 	radeon_legacy_crtc.o radeon_legacy_encoders.o radeon_connectors.o \
 	radeon_encoders.o radeon_display.o radeon_cursor.o radeon_i2c.o \
-	radeon_clocks.o radeon_fb.o radeon_gem.o radeon_ring.o radeon_irq_kms.o \
+	radeon_clocks.o radeon_gem.o radeon_ring.o radeon_irq_kms.o \
 	radeon_cs.o radeon_bios.o radeon_benchmark.o r100.o r300.o r420.o \
 	rs400.o rs600.o rs690.o rv515.o r520.o r600.o rv770.o radeon_test.o \
 	r200.o radeon_legacy_tv.o r600_cs.o \
@@ -76,6 +76,7 @@ radeon-y += \
 	vce_v1_0.o \
 	vce_v2_0.o
 
+radeon-$(CONFIG_DRM_FBDEV_EMULATION) += radeon_fbdev.o
 radeon-$(CONFIG_VGA_SWITCHEROO) += radeon_atpx_handler.o
 radeon-$(CONFIG_ACPI) += radeon_acpi.o
 
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fbdev.c
similarity index 100%
rename from drivers/gpu/drm/radeon/radeon_fb.c
rename to drivers/gpu/drm/radeon/radeon_fbdev.c
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index d81f61e5a95c..1decdcec0264 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -939,9 +939,20 @@ void dce4_program_fmt(struct drm_encoder *encoder);
 void dce8_program_fmt(struct drm_encoder *encoder);
 
 /* fbdev layer */
+#if defined(CONFIG_DRM_FBDEV_EMULATION)
 void radeon_fbdev_setup(struct radeon_device *rdev);
 void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state);
 bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj);
+#else
+static inline void radeon_fbdev_setup(struct radeon_device *rdev)
+{ }
+static inline void radeon_fbdev_set_suspend(struct radeon_device *rdev, int state)
+{ }
+static inline bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj)
+{
+	return false;
+}
+#endif
 
 void radeon_crtc_handle_vblank(struct radeon_device *rdev, int crtc_id);
 
-- 
2.39.2


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

* Re: [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
  2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
                   ` (9 preceding siblings ...)
  2023-03-16  9:37 ` [PATCH 10/10] drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set Thomas Zimmermann
@ 2023-03-17  8:53 ` Christian König
  2023-03-17  9:20   ` Thomas Zimmermann
  10 siblings, 1 reply; 19+ messages in thread
From: Christian König @ 2023-03-17  8:53 UTC (permalink / raw)
  To: Thomas Zimmermann, alexander.deucher, Xinhui.Pan, airlied,
	daniel, javierm
  Cc: amd-gfx, dri-devel

Am 16.03.23 um 10:37 schrieb Thomas Zimmermann:
> Convert radeon's fbdev code to drm_client. Replaces the current
> ad-hoc integration. The conversion includes a number of cleanups.
> Only build fbdev support if the config option has been set.

I'm torn apart on that. On the one hand it looks like a really nice 
cleanup on the other hand we don't really want to touch radeon any more.

Alex what do you think? Is that worth the risk of breaking stuff?

Christian.

>
> Thomas Zimmermann (10):
>    drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
>    drm/radeon: Improve fbdev object-test helper
>    drm/radeon: Remove struct radeon_fbdev
>    drm/radeon: Remove test for !screen_base in fbdev probing
>    drm/radeon: Move fbdev object helpers before struct fb_ops et al
>    drm/radeon: Fix coding style in fbdev emulation
>    drm/radeon: Move fbdev cleanup code into fb_destroy callback
>    drm/radeon: Correctly clean up failed display probing
>    drm/radeon: Implement client-based fbdev emulation
>    drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
>
>   drivers/gpu/drm/radeon/Makefile         |   3 +-
>   drivers/gpu/drm/radeon/radeon.h         |   2 +
>   drivers/gpu/drm/radeon/radeon_display.c |   4 -
>   drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
>   drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
>   drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
>   drivers/gpu/drm/radeon/radeon_fbdev.c   | 422 ++++++++++++++++++++++++
>   drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
>   drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
>   drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
>   10 files changed, 464 insertions(+), 433 deletions(-)
>   delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
>   create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c
>
>
> base-commit: ec0708e846b819c8d5b642de42448a87d7526564


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

* Re: [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
  2023-03-17  8:53 ` [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Christian König
@ 2023-03-17  9:20   ` Thomas Zimmermann
  2023-03-20 15:11     ` Christian König
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-17  9:20 UTC (permalink / raw)
  To: Christian König, alexander.deucher, Xinhui.Pan, airlied,
	daniel, javierm
  Cc: dri-devel, amd-gfx


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

Hi Christian

Am 17.03.23 um 09:53 schrieb Christian König:
> Am 16.03.23 um 10:37 schrieb Thomas Zimmermann:
>> Convert radeon's fbdev code to drm_client. Replaces the current
>> ad-hoc integration. The conversion includes a number of cleanups.
>> Only build fbdev support if the config option has been set.
> 
> I'm torn apart on that. On the one hand it looks like a really nice 
> cleanup on the other hand we don't really want to touch radeon any more.

It's a driver in the upstream kernel. You have to expect at least some 
changes.

> 
> Alex what do you think? Is that worth the risk of breaking stuff?

Moving all fbdev emulation to struct drm_client is required for new 
in-kernel DRM clients, such as a DRM kernel logger or a boot splash.

Best regards
Thomas

> 
> Christian.
> 
>>
>> Thomas Zimmermann (10):
>>    drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
>>    drm/radeon: Improve fbdev object-test helper
>>    drm/radeon: Remove struct radeon_fbdev
>>    drm/radeon: Remove test for !screen_base in fbdev probing
>>    drm/radeon: Move fbdev object helpers before struct fb_ops et al
>>    drm/radeon: Fix coding style in fbdev emulation
>>    drm/radeon: Move fbdev cleanup code into fb_destroy callback
>>    drm/radeon: Correctly clean up failed display probing
>>    drm/radeon: Implement client-based fbdev emulation
>>    drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
>>
>>   drivers/gpu/drm/radeon/Makefile         |   3 +-
>>   drivers/gpu/drm/radeon/radeon.h         |   2 +
>>   drivers/gpu/drm/radeon/radeon_display.c |   4 -
>>   drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
>>   drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
>>   drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
>>   drivers/gpu/drm/radeon/radeon_fbdev.c   | 422 ++++++++++++++++++++++++
>>   drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
>>   drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
>>   drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
>>   10 files changed, 464 insertions(+), 433 deletions(-)
>>   delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
>>   create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c
>>
>>
>> base-commit: ec0708e846b819c8d5b642de42448a87d7526564
> 

-- 
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] 19+ messages in thread

* Re: [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
  2023-03-17  9:20   ` Thomas Zimmermann
@ 2023-03-20 15:11     ` Christian König
  2023-03-20 15:19       ` Thomas Zimmermann
  0 siblings, 1 reply; 19+ messages in thread
From: Christian König @ 2023-03-20 15:11 UTC (permalink / raw)
  To: Thomas Zimmermann, Christian König, alexander.deucher,
	Xinhui.Pan, airlied, daniel, javierm
  Cc: amd-gfx, dri-devel

Am 17.03.23 um 10:20 schrieb Thomas Zimmermann:
> Hi Christian
>
> Am 17.03.23 um 09:53 schrieb Christian König:
>> Am 16.03.23 um 10:37 schrieb Thomas Zimmermann:
>>> Convert radeon's fbdev code to drm_client. Replaces the current
>>> ad-hoc integration. The conversion includes a number of cleanups.
>>> Only build fbdev support if the config option has been set.
>>
>> I'm torn apart on that. On the one hand it looks like a really nice 
>> cleanup on the other hand we don't really want to touch radeon any more.
>
> It's a driver in the upstream kernel. You have to expect at least some 
> changes.

Some changes is not the problem, but we need a justification to change 
something. Just that it's nice to have won't do it without extensive 
testing.

>
>>
>> Alex what do you think? Is that worth the risk of breaking stuff?
>
> Moving all fbdev emulation to struct drm_client is required for new 
> in-kernel DRM clients, such as a DRM kernel logger or a boot splash.

Well that's a rather good justification. I suggest to add that to the 
cover-letter.

Regards,
Christian.

>
> Best regards
> Thomas
>
>>
>> Christian.
>>
>>>
>>> Thomas Zimmermann (10):
>>>    drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
>>>    drm/radeon: Improve fbdev object-test helper
>>>    drm/radeon: Remove struct radeon_fbdev
>>>    drm/radeon: Remove test for !screen_base in fbdev probing
>>>    drm/radeon: Move fbdev object helpers before struct fb_ops et al
>>>    drm/radeon: Fix coding style in fbdev emulation
>>>    drm/radeon: Move fbdev cleanup code into fb_destroy callback
>>>    drm/radeon: Correctly clean up failed display probing
>>>    drm/radeon: Implement client-based fbdev emulation
>>>    drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
>>>
>>>   drivers/gpu/drm/radeon/Makefile         |   3 +-
>>>   drivers/gpu/drm/radeon/radeon.h         |   2 +
>>>   drivers/gpu/drm/radeon/radeon_display.c |   4 -
>>>   drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
>>>   drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
>>>   drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
>>>   drivers/gpu/drm/radeon/radeon_fbdev.c   | 422 
>>> ++++++++++++++++++++++++
>>>   drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
>>>   drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
>>>   drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
>>>   10 files changed, 464 insertions(+), 433 deletions(-)
>>>   delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
>>>   create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c
>>>
>>>
>>> base-commit: ec0708e846b819c8d5b642de42448a87d7526564
>>
>


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

* Re: [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
  2023-03-20 15:11     ` Christian König
@ 2023-03-20 15:19       ` Thomas Zimmermann
  2023-03-20 15:23         ` Alex Deucher
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-20 15:19 UTC (permalink / raw)
  To: Christian König, Christian König, alexander.deucher,
	Xinhui.Pan, airlied, daniel, javierm
  Cc: dri-devel, amd-gfx


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

Hi

Am 20.03.23 um 16:11 schrieb Christian König:
> Am 17.03.23 um 10:20 schrieb Thomas Zimmermann:
>> Hi Christian
>>
>> Am 17.03.23 um 09:53 schrieb Christian König:
>>> Am 16.03.23 um 10:37 schrieb Thomas Zimmermann:
>>>> Convert radeon's fbdev code to drm_client. Replaces the current
>>>> ad-hoc integration. The conversion includes a number of cleanups.
>>>> Only build fbdev support if the config option has been set.
>>>
>>> I'm torn apart on that. On the one hand it looks like a really nice 
>>> cleanup on the other hand we don't really want to touch radeon any more.
>>
>> It's a driver in the upstream kernel. You have to expect at least some 
>> changes.
> 
> Some changes is not the problem, but we need a justification to change 
> something. Just that it's nice to have won't do it without extensive 
> testing.
> 
>>
>>>
>>> Alex what do you think? Is that worth the risk of breaking stuff?
>>
>> Moving all fbdev emulation to struct drm_client is required for new 
>> in-kernel DRM clients, such as a DRM kernel logger or a boot splash.
> 
> Well that's a rather good justification. I suggest to add that to the 
> cover-letter.

Ok, will go into a possible v2. The mid-term plan is to convert the 
fbdev code in all remaining drivers to struct drm_client and remove the 
old ad-hoc callbacks.

With struct drm_client, we can select in-kernel clients at compile time 
or runtime just like userspace clients. I guess, we can have a bootup 
screen and then switch to the console or the DRM logger. Or go from any 
client to the logger on kernel panics (or something like that). There's 
been occasional talk about userspace consoles, which would use such 
functionality.

Best regards
Thomas

> 
> Regards,
> Christian.
> 
>>
>> Best regards
>> Thomas
>>
>>>
>>> Christian.
>>>
>>>>
>>>> Thomas Zimmermann (10):
>>>>    drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
>>>>    drm/radeon: Improve fbdev object-test helper
>>>>    drm/radeon: Remove struct radeon_fbdev
>>>>    drm/radeon: Remove test for !screen_base in fbdev probing
>>>>    drm/radeon: Move fbdev object helpers before struct fb_ops et al
>>>>    drm/radeon: Fix coding style in fbdev emulation
>>>>    drm/radeon: Move fbdev cleanup code into fb_destroy callback
>>>>    drm/radeon: Correctly clean up failed display probing
>>>>    drm/radeon: Implement client-based fbdev emulation
>>>>    drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
>>>>
>>>>   drivers/gpu/drm/radeon/Makefile         |   3 +-
>>>>   drivers/gpu/drm/radeon/radeon.h         |   2 +
>>>>   drivers/gpu/drm/radeon/radeon_display.c |   4 -
>>>>   drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
>>>>   drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
>>>>   drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
>>>>   drivers/gpu/drm/radeon/radeon_fbdev.c   | 422 
>>>> ++++++++++++++++++++++++
>>>>   drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
>>>>   drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
>>>>   drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
>>>>   10 files changed, 464 insertions(+), 433 deletions(-)
>>>>   delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
>>>>   create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c
>>>>
>>>>
>>>> base-commit: ec0708e846b819c8d5b642de42448a87d7526564
>>>
>>
> 

-- 
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] 19+ messages in thread

* Re: [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
  2023-03-20 15:19       ` Thomas Zimmermann
@ 2023-03-20 15:23         ` Alex Deucher
  2023-03-21  9:33           ` Thomas Zimmermann
  0 siblings, 1 reply; 19+ messages in thread
From: Alex Deucher @ 2023-03-20 15:23 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Christian König, Xinhui.Pan, javierm, dri-devel, amd-gfx,
	alexander.deucher, Christian König

On Mon, Mar 20, 2023 at 11:19 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 20.03.23 um 16:11 schrieb Christian König:
> > Am 17.03.23 um 10:20 schrieb Thomas Zimmermann:
> >> Hi Christian
> >>
> >> Am 17.03.23 um 09:53 schrieb Christian König:
> >>> Am 16.03.23 um 10:37 schrieb Thomas Zimmermann:
> >>>> Convert radeon's fbdev code to drm_client. Replaces the current
> >>>> ad-hoc integration. The conversion includes a number of cleanups.
> >>>> Only build fbdev support if the config option has been set.
> >>>
> >>> I'm torn apart on that. On the one hand it looks like a really nice
> >>> cleanup on the other hand we don't really want to touch radeon any more.
> >>
> >> It's a driver in the upstream kernel. You have to expect at least some
> >> changes.
> >
> > Some changes is not the problem, but we need a justification to change
> > something. Just that it's nice to have won't do it without extensive
> > testing.
> >
> >>
> >>>
> >>> Alex what do you think? Is that worth the risk of breaking stuff?
> >>
> >> Moving all fbdev emulation to struct drm_client is required for new
> >> in-kernel DRM clients, such as a DRM kernel logger or a boot splash.
> >
> > Well that's a rather good justification. I suggest to add that to the
> > cover-letter.
>
> Ok, will go into a possible v2. The mid-term plan is to convert the
> fbdev code in all remaining drivers to struct drm_client and remove the
> old ad-hoc callbacks.
>
> With struct drm_client, we can select in-kernel clients at compile time
> or runtime just like userspace clients. I guess, we can have a bootup
> screen and then switch to the console or the DRM logger. Or go from any
> client to the logger on kernel panics (or something like that). There's
> been occasional talk about userspace consoles, which would use such
> functionality.

Patches look good to me.  I have a pretty limited set of HW I can test
on since I don't have a functional AGP system anymore and most of my
older PCIe radeons are packed up in the attic.  Feel free to add my:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
to the series.

Alex

>
> Best regards
> Thomas
>
> >
> > Regards,
> > Christian.
> >
> >>
> >> Best regards
> >> Thomas
> >>
> >>>
> >>> Christian.
> >>>
> >>>>
> >>>> Thomas Zimmermann (10):
> >>>>    drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
> >>>>    drm/radeon: Improve fbdev object-test helper
> >>>>    drm/radeon: Remove struct radeon_fbdev
> >>>>    drm/radeon: Remove test for !screen_base in fbdev probing
> >>>>    drm/radeon: Move fbdev object helpers before struct fb_ops et al
> >>>>    drm/radeon: Fix coding style in fbdev emulation
> >>>>    drm/radeon: Move fbdev cleanup code into fb_destroy callback
> >>>>    drm/radeon: Correctly clean up failed display probing
> >>>>    drm/radeon: Implement client-based fbdev emulation
> >>>>    drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
> >>>>
> >>>>   drivers/gpu/drm/radeon/Makefile         |   3 +-
> >>>>   drivers/gpu/drm/radeon/radeon.h         |   2 +
> >>>>   drivers/gpu/drm/radeon/radeon_display.c |   4 -
> >>>>   drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
> >>>>   drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
> >>>>   drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
> >>>>   drivers/gpu/drm/radeon/radeon_fbdev.c   | 422
> >>>> ++++++++++++++++++++++++
> >>>>   drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
> >>>>   drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
> >>>>   drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
> >>>>   10 files changed, 464 insertions(+), 433 deletions(-)
> >>>>   delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
> >>>>   create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c
> >>>>
> >>>>
> >>>> base-commit: ec0708e846b819c8d5b642de42448a87d7526564
> >>>
> >>
> >
>
> --
> 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

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

* Re: [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
  2023-03-20 15:23         ` Alex Deucher
@ 2023-03-21  9:33           ` Thomas Zimmermann
  2023-03-24 21:44             ` Alex Deucher
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2023-03-21  9:33 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Christian König, Xinhui.Pan, javierm, amd-gfx, dri-devel,
	alexander.deucher, Christian König


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

Hi

Am 20.03.23 um 16:23 schrieb Alex Deucher:
> On Mon, Mar 20, 2023 at 11:19 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 20.03.23 um 16:11 schrieb Christian König:
>>> Am 17.03.23 um 10:20 schrieb Thomas Zimmermann:
>>>> Hi Christian
>>>>
>>>> Am 17.03.23 um 09:53 schrieb Christian König:
>>>>> Am 16.03.23 um 10:37 schrieb Thomas Zimmermann:
>>>>>> Convert radeon's fbdev code to drm_client. Replaces the current
>>>>>> ad-hoc integration. The conversion includes a number of cleanups.
>>>>>> Only build fbdev support if the config option has been set.
>>>>>
>>>>> I'm torn apart on that. On the one hand it looks like a really nice
>>>>> cleanup on the other hand we don't really want to touch radeon any more.
>>>>
>>>> It's a driver in the upstream kernel. You have to expect at least some
>>>> changes.
>>>
>>> Some changes is not the problem, but we need a justification to change
>>> something. Just that it's nice to have won't do it without extensive
>>> testing.
>>>
>>>>
>>>>>
>>>>> Alex what do you think? Is that worth the risk of breaking stuff?
>>>>
>>>> Moving all fbdev emulation to struct drm_client is required for new
>>>> in-kernel DRM clients, such as a DRM kernel logger or a boot splash.
>>>
>>> Well that's a rather good justification. I suggest to add that to the
>>> cover-letter.
>>
>> Ok, will go into a possible v2. The mid-term plan is to convert the
>> fbdev code in all remaining drivers to struct drm_client and remove the
>> old ad-hoc callbacks.
>>
>> With struct drm_client, we can select in-kernel clients at compile time
>> or runtime just like userspace clients. I guess, we can have a bootup
>> screen and then switch to the console or the DRM logger. Or go from any
>> client to the logger on kernel panics (or something like that). There's
>> been occasional talk about userspace consoles, which would use such
>> functionality.
> 
> Patches look good to me.  I have a pretty limited set of HW I can test
> on since I don't have a functional AGP system anymore and most of my
> older PCIe radeons are packed up in the attic.  Feel free to add my:

I've tested the patches with an R5-based card.

> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> to the series.

Thank you so much. Do you want to take the patches into the amd tree?

Best regards
Thomas

> 
> Alex
> 
>>
>> Best regards
>> Thomas
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>>
>>>>> Christian.
>>>>>
>>>>>>
>>>>>> Thomas Zimmermann (10):
>>>>>>     drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
>>>>>>     drm/radeon: Improve fbdev object-test helper
>>>>>>     drm/radeon: Remove struct radeon_fbdev
>>>>>>     drm/radeon: Remove test for !screen_base in fbdev probing
>>>>>>     drm/radeon: Move fbdev object helpers before struct fb_ops et al
>>>>>>     drm/radeon: Fix coding style in fbdev emulation
>>>>>>     drm/radeon: Move fbdev cleanup code into fb_destroy callback
>>>>>>     drm/radeon: Correctly clean up failed display probing
>>>>>>     drm/radeon: Implement client-based fbdev emulation
>>>>>>     drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
>>>>>>
>>>>>>    drivers/gpu/drm/radeon/Makefile         |   3 +-
>>>>>>    drivers/gpu/drm/radeon/radeon.h         |   2 +
>>>>>>    drivers/gpu/drm/radeon/radeon_display.c |   4 -
>>>>>>    drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
>>>>>>    drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
>>>>>>    drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
>>>>>>    drivers/gpu/drm/radeon/radeon_fbdev.c   | 422
>>>>>> ++++++++++++++++++++++++
>>>>>>    drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
>>>>>>    drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
>>>>>>    drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
>>>>>>    10 files changed, 464 insertions(+), 433 deletions(-)
>>>>>>    delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
>>>>>>    create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c
>>>>>>
>>>>>>
>>>>>> base-commit: ec0708e846b819c8d5b642de42448a87d7526564
>>>>>
>>>>
>>>
>>
>> --
>> 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

-- 
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] 19+ messages in thread

* Re: [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
  2023-03-21  9:33           ` Thomas Zimmermann
@ 2023-03-24 21:44             ` Alex Deucher
  2023-03-27 22:39               ` Alex Deucher
  0 siblings, 1 reply; 19+ messages in thread
From: Alex Deucher @ 2023-03-24 21:44 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Christian König, Xinhui.Pan, javierm, amd-gfx, dri-devel,
	alexander.deucher, Christian König

On Tue, Mar 21, 2023 at 5:33 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 20.03.23 um 16:23 schrieb Alex Deucher:
> > On Mon, Mar 20, 2023 at 11:19 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> Hi
> >>
> >> Am 20.03.23 um 16:11 schrieb Christian König:
> >>> Am 17.03.23 um 10:20 schrieb Thomas Zimmermann:
> >>>> Hi Christian
> >>>>
> >>>> Am 17.03.23 um 09:53 schrieb Christian König:
> >>>>> Am 16.03.23 um 10:37 schrieb Thomas Zimmermann:
> >>>>>> Convert radeon's fbdev code to drm_client. Replaces the current
> >>>>>> ad-hoc integration. The conversion includes a number of cleanups.
> >>>>>> Only build fbdev support if the config option has been set.
> >>>>>
> >>>>> I'm torn apart on that. On the one hand it looks like a really nice
> >>>>> cleanup on the other hand we don't really want to touch radeon any more.
> >>>>
> >>>> It's a driver in the upstream kernel. You have to expect at least some
> >>>> changes.
> >>>
> >>> Some changes is not the problem, but we need a justification to change
> >>> something. Just that it's nice to have won't do it without extensive
> >>> testing.
> >>>
> >>>>
> >>>>>
> >>>>> Alex what do you think? Is that worth the risk of breaking stuff?
> >>>>
> >>>> Moving all fbdev emulation to struct drm_client is required for new
> >>>> in-kernel DRM clients, such as a DRM kernel logger or a boot splash.
> >>>
> >>> Well that's a rather good justification. I suggest to add that to the
> >>> cover-letter.
> >>
> >> Ok, will go into a possible v2. The mid-term plan is to convert the
> >> fbdev code in all remaining drivers to struct drm_client and remove the
> >> old ad-hoc callbacks.
> >>
> >> With struct drm_client, we can select in-kernel clients at compile time
> >> or runtime just like userspace clients. I guess, we can have a bootup
> >> screen and then switch to the console or the DRM logger. Or go from any
> >> client to the logger on kernel panics (or something like that). There's
> >> been occasional talk about userspace consoles, which would use such
> >> functionality.
> >
> > Patches look good to me.  I have a pretty limited set of HW I can test
> > on since I don't have a functional AGP system anymore and most of my
> > older PCIe radeons are packed up in the attic.  Feel free to add my:
>
> I've tested the patches with an R5-based card.
>
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > to the series.
>
> Thank you so much. Do you want to take the patches into the amd tree?

I haven't forgotten these.  Will pick them up next week.

Thanks,

Alex

>
> Best regards
> Thomas
>
> >
> > Alex
> >
> >>
> >> Best regards
> >> Thomas
> >>
> >>>
> >>> Regards,
> >>> Christian.
> >>>
> >>>>
> >>>> Best regards
> >>>> Thomas
> >>>>
> >>>>>
> >>>>> Christian.
> >>>>>
> >>>>>>
> >>>>>> Thomas Zimmermann (10):
> >>>>>>     drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
> >>>>>>     drm/radeon: Improve fbdev object-test helper
> >>>>>>     drm/radeon: Remove struct radeon_fbdev
> >>>>>>     drm/radeon: Remove test for !screen_base in fbdev probing
> >>>>>>     drm/radeon: Move fbdev object helpers before struct fb_ops et al
> >>>>>>     drm/radeon: Fix coding style in fbdev emulation
> >>>>>>     drm/radeon: Move fbdev cleanup code into fb_destroy callback
> >>>>>>     drm/radeon: Correctly clean up failed display probing
> >>>>>>     drm/radeon: Implement client-based fbdev emulation
> >>>>>>     drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
> >>>>>>
> >>>>>>    drivers/gpu/drm/radeon/Makefile         |   3 +-
> >>>>>>    drivers/gpu/drm/radeon/radeon.h         |   2 +
> >>>>>>    drivers/gpu/drm/radeon/radeon_display.c |   4 -
> >>>>>>    drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
> >>>>>>    drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
> >>>>>>    drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
> >>>>>>    drivers/gpu/drm/radeon/radeon_fbdev.c   | 422
> >>>>>> ++++++++++++++++++++++++
> >>>>>>    drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
> >>>>>>    drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
> >>>>>>    drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
> >>>>>>    10 files changed, 464 insertions(+), 433 deletions(-)
> >>>>>>    delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
> >>>>>>    create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c
> >>>>>>
> >>>>>>
> >>>>>> base-commit: ec0708e846b819c8d5b642de42448a87d7526564
> >>>>>
> >>>>
> >>>
> >>
> >> --
> >> 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
>
> --
> 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

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

* Re: [PATCH 00/10] drm/radeon: Convert fbdev to DRM client
  2023-03-24 21:44             ` Alex Deucher
@ 2023-03-27 22:39               ` Alex Deucher
  0 siblings, 0 replies; 19+ messages in thread
From: Alex Deucher @ 2023-03-27 22:39 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Christian König, Xinhui.Pan, javierm, amd-gfx, dri-devel,
	alexander.deucher, Christian König

Applied.  Thanks!

On Fri, Mar 24, 2023 at 5:44 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Tue, Mar 21, 2023 at 5:33 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > Hi
> >
> > Am 20.03.23 um 16:23 schrieb Alex Deucher:
> > > On Mon, Mar 20, 2023 at 11:19 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > >>
> > >> Hi
> > >>
> > >> Am 20.03.23 um 16:11 schrieb Christian König:
> > >>> Am 17.03.23 um 10:20 schrieb Thomas Zimmermann:
> > >>>> Hi Christian
> > >>>>
> > >>>> Am 17.03.23 um 09:53 schrieb Christian König:
> > >>>>> Am 16.03.23 um 10:37 schrieb Thomas Zimmermann:
> > >>>>>> Convert radeon's fbdev code to drm_client. Replaces the current
> > >>>>>> ad-hoc integration. The conversion includes a number of cleanups.
> > >>>>>> Only build fbdev support if the config option has been set.
> > >>>>>
> > >>>>> I'm torn apart on that. On the one hand it looks like a really nice
> > >>>>> cleanup on the other hand we don't really want to touch radeon any more.
> > >>>>
> > >>>> It's a driver in the upstream kernel. You have to expect at least some
> > >>>> changes.
> > >>>
> > >>> Some changes is not the problem, but we need a justification to change
> > >>> something. Just that it's nice to have won't do it without extensive
> > >>> testing.
> > >>>
> > >>>>
> > >>>>>
> > >>>>> Alex what do you think? Is that worth the risk of breaking stuff?
> > >>>>
> > >>>> Moving all fbdev emulation to struct drm_client is required for new
> > >>>> in-kernel DRM clients, such as a DRM kernel logger or a boot splash.
> > >>>
> > >>> Well that's a rather good justification. I suggest to add that to the
> > >>> cover-letter.
> > >>
> > >> Ok, will go into a possible v2. The mid-term plan is to convert the
> > >> fbdev code in all remaining drivers to struct drm_client and remove the
> > >> old ad-hoc callbacks.
> > >>
> > >> With struct drm_client, we can select in-kernel clients at compile time
> > >> or runtime just like userspace clients. I guess, we can have a bootup
> > >> screen and then switch to the console or the DRM logger. Or go from any
> > >> client to the logger on kernel panics (or something like that). There's
> > >> been occasional talk about userspace consoles, which would use such
> > >> functionality.
> > >
> > > Patches look good to me.  I have a pretty limited set of HW I can test
> > > on since I don't have a functional AGP system anymore and most of my
> > > older PCIe radeons are packed up in the attic.  Feel free to add my:
> >
> > I've tested the patches with an R5-based card.
> >
> > > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > > to the series.
> >
> > Thank you so much. Do you want to take the patches into the amd tree?
>
> I haven't forgotten these.  Will pick them up next week.
>
> Thanks,
>
> Alex
>
> >
> > Best regards
> > Thomas
> >
> > >
> > > Alex
> > >
> > >>
> > >> Best regards
> > >> Thomas
> > >>
> > >>>
> > >>> Regards,
> > >>> Christian.
> > >>>
> > >>>>
> > >>>> Best regards
> > >>>> Thomas
> > >>>>
> > >>>>>
> > >>>>> Christian.
> > >>>>>
> > >>>>>>
> > >>>>>> Thomas Zimmermann (10):
> > >>>>>>     drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers
> > >>>>>>     drm/radeon: Improve fbdev object-test helper
> > >>>>>>     drm/radeon: Remove struct radeon_fbdev
> > >>>>>>     drm/radeon: Remove test for !screen_base in fbdev probing
> > >>>>>>     drm/radeon: Move fbdev object helpers before struct fb_ops et al
> > >>>>>>     drm/radeon: Fix coding style in fbdev emulation
> > >>>>>>     drm/radeon: Move fbdev cleanup code into fb_destroy callback
> > >>>>>>     drm/radeon: Correctly clean up failed display probing
> > >>>>>>     drm/radeon: Implement client-based fbdev emulation
> > >>>>>>     drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set
> > >>>>>>
> > >>>>>>    drivers/gpu/drm/radeon/Makefile         |   3 +-
> > >>>>>>    drivers/gpu/drm/radeon/radeon.h         |   2 +
> > >>>>>>    drivers/gpu/drm/radeon/radeon_display.c |   4 -
> > >>>>>>    drivers/gpu/drm/radeon/radeon_drv.c     |   3 +-
> > >>>>>>    drivers/gpu/drm/radeon/radeon_drv.h     |   1 -
> > >>>>>>    drivers/gpu/drm/radeon/radeon_fb.c      | 400 ----------------------
> > >>>>>>    drivers/gpu/drm/radeon/radeon_fbdev.c   | 422
> > >>>>>> ++++++++++++++++++++++++
> > >>>>>>    drivers/gpu/drm/radeon/radeon_gem.c     |  24 ++
> > >>>>>>    drivers/gpu/drm/radeon/radeon_kms.c     |  18 -
> > >>>>>>    drivers/gpu/drm/radeon/radeon_mode.h    |  20 +-
> > >>>>>>    10 files changed, 464 insertions(+), 433 deletions(-)
> > >>>>>>    delete mode 100644 drivers/gpu/drm/radeon/radeon_fb.c
> > >>>>>>    create mode 100644 drivers/gpu/drm/radeon/radeon_fbdev.c
> > >>>>>>
> > >>>>>>
> > >>>>>> base-commit: ec0708e846b819c8d5b642de42448a87d7526564
> > >>>>>
> > >>>>
> > >>>
> > >>
> > >> --
> > >> 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
> >
> > --
> > 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

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

end of thread, other threads:[~2023-03-27 22:39 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-16  9:37 [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 01/10] drm/radeon: Move radeon_align_pitch() next to dumb-buffer helpers Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 02/10] drm/radeon: Improve fbdev object-test helper Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 03/10] drm/radeon: Remove struct radeon_fbdev Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 04/10] drm/radeon: Remove test for !screen_base in fbdev probing Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 05/10] drm/radeon: Move fbdev object helpers before struct fb_ops et al Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 06/10] drm/radeon: Fix coding style in fbdev emulation Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 07/10] drm/radeon: Move fbdev cleanup code into fb_destroy callback Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 08/10] drm/radeon: Correctly clean up failed display probing Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 09/10] drm/radeon: Implement client-based fbdev emulation Thomas Zimmermann
2023-03-16  9:37 ` [PATCH 10/10] drm/radeon: Only build fbdev if DRM_FBDEV_EMULATION is set Thomas Zimmermann
2023-03-17  8:53 ` [PATCH 00/10] drm/radeon: Convert fbdev to DRM client Christian König
2023-03-17  9:20   ` Thomas Zimmermann
2023-03-20 15:11     ` Christian König
2023-03-20 15:19       ` Thomas Zimmermann
2023-03-20 15:23         ` Alex Deucher
2023-03-21  9:33           ` Thomas Zimmermann
2023-03-24 21:44             ` Alex Deucher
2023-03-27 22:39               ` Alex Deucher

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