From: Thomas Zimmermann <tzimmermann@suse.de>
To: simona@ffwll.ch, airlied@gmail.com, alexander.deucher@amd.com,
christian.koenig@amd.com, lyude@redhat.com, dakr@kernel.org,
deller@gmx.de, mripard@kernel.org,
maarten.lankhorst@linux.intel.com, jason.wessel@windriver.com,
danielt@kernel.org, dianders@chromium.org
Cc: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
nouveau@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
Thomas Zimmermann <tzimmermann@suse.de>
Subject: [PATCH 3/5] drm/radeon: Do not implement mode_set_base_atomic callback
Date: Tue, 25 Nov 2025 13:52:15 +0100 [thread overview]
Message-ID: <20251125130634.1080966-4-tzimmermann@suse.de> (raw)
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>
Remove the implementation of the CRTC helper mode_set_base_atomic
from radeon. It pretends to provide mode setting for kdb debugging,
but has been broken for some time.
Kdb output has been supported only for non-atomic mode setting since
commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for atomic drivers")
from 2017.
While radeon provides non-atomic mode setting, kdb assumes that the GEM
buffer object is at a fixed location in video memory. This assumption
currently blocks radeon from converting to generic fbdev emulation.
Fbdev-ttm helpers use a shadow buffer with a movable GEM buffer object.
Triggering kdb does therefore not update the display.
Another problem is that the current implementation does not handle
USB keyboard input. Therefore a serial terminal is required. Then when
continuing from the debugger, radeon fails with an error:
[7]kdb> go
[ 40.345523][ C7] BUG: scheduling while atomic: bash/1580/0x00110003
[...]
[ 40.345613][ C7] schedule+0x27/0xd0
[ 40.345615][ C7] schedule_timeout+0x7b/0x100
[ 40.345617][ C7] ? __pfx_process_timeout+0x10/0x10
[ 40.345619][ C7] msleep+0x31/0x50
[ 40.345621][ C7] radeon_crtc_load_lut+0x2e4/0xcb0 [radeon 31c1ee785de120fcfd0babcc09babb3770252b4e]
[ 40.345698][ C7] radeon_crtc_gamma_set+0xe/0x20 [radeon 31c1ee785de120fcfd0babcc09babb3770252b4e]
[ 40.345760][ C7] drm_fb_helper_debug_leave+0xd8/0x130
[ 40.345763][ C7] kgdboc_post_exp_handler+0x54/0x70
[...]
and the system hangs.
Support for kdb feels pretty much broken. Hence remove the whole kdb
support from radeon.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/radeon/atombios_crtc.c | 74 ++++++---------------
drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 23 ++-----
drivers/gpu/drm/radeon/radeon_mode.h | 10 +--
3 files changed, 26 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 9b3a3a9d60e2..2fc0334e0d6c 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -1133,7 +1133,7 @@ static void atombios_crtc_set_pll(struct drm_crtc *crtc, struct drm_display_mode
static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- int x, int y, int atomic)
+ int x, int y)
{
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
@@ -1150,33 +1150,23 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
bool bypass_lut = false;
/* no fb bound */
- if (!atomic && !crtc->primary->fb) {
+ if (!crtc->primary->fb) {
DRM_DEBUG_KMS("No FB bound\n");
return 0;
}
- if (atomic)
- target_fb = fb;
- else
- target_fb = crtc->primary->fb;
+ target_fb = crtc->primary->fb;
- /* If atomic, assume fb object is pinned & idle & fenced and
- * just update base pointers
- */
obj = target_fb->obj[0];
rbo = gem_to_radeon_bo(obj);
r = radeon_bo_reserve(rbo, false);
if (unlikely(r != 0))
return r;
- if (atomic)
- fb_location = radeon_bo_gpu_offset(rbo);
- else {
- r = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, &fb_location);
- if (unlikely(r != 0)) {
- radeon_bo_unreserve(rbo);
- return -EINVAL;
- }
+ r = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, &fb_location);
+ if (unlikely(r != 0)) {
+ radeon_bo_unreserve(rbo);
+ return -EINVAL;
}
radeon_bo_get_tiling_flags(rbo, &tiling_flags, NULL);
@@ -1437,7 +1427,7 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
/* set pageflip to happen anywhere in vblank interval */
WREG32(EVERGREEN_MASTER_UPDATE_MODE + radeon_crtc->crtc_offset, 0);
- if (!atomic && fb && fb != crtc->primary->fb) {
+ if (fb && fb != crtc->primary->fb) {
rbo = gem_to_radeon_bo(fb->obj[0]);
r = radeon_bo_reserve(rbo, false);
if (unlikely(r != 0))
@@ -1454,7 +1444,7 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- int x, int y, int atomic)
+ int x, int y)
{
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
@@ -1470,15 +1460,12 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
bool bypass_lut = false;
/* no fb bound */
- if (!atomic && !crtc->primary->fb) {
+ if (!crtc->primary->fb) {
DRM_DEBUG_KMS("No FB bound\n");
return 0;
}
- if (atomic)
- target_fb = fb;
- else
- target_fb = crtc->primary->fb;
+ target_fb = crtc->primary->fb;
obj = target_fb->obj[0];
rbo = gem_to_radeon_bo(obj);
@@ -1486,17 +1473,10 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
if (unlikely(r != 0))
return r;
- /* If atomic, assume fb object is pinned & idle & fenced and
- * just update base pointers
- */
- if (atomic)
- fb_location = radeon_bo_gpu_offset(rbo);
- else {
- r = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, &fb_location);
- if (unlikely(r != 0)) {
- radeon_bo_unreserve(rbo);
- return -EINVAL;
- }
+ r = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, &fb_location);
+ if (unlikely(r != 0)) {
+ radeon_bo_unreserve(rbo);
+ return -EINVAL;
}
radeon_bo_get_tiling_flags(rbo, &tiling_flags, NULL);
radeon_bo_unreserve(rbo);
@@ -1645,7 +1625,7 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
/* set pageflip to happen only at start of vblank interval (front porch) */
WREG32(AVIVO_D1MODE_MASTER_UPDATE_MODE + radeon_crtc->crtc_offset, 3);
- if (!atomic && fb && fb != crtc->primary->fb) {
+ if (fb && fb != crtc->primary->fb) {
rbo = gem_to_radeon_bo(fb->obj[0]);
r = radeon_bo_reserve(rbo, false);
if (unlikely(r != 0))
@@ -1667,26 +1647,11 @@ int atombios_crtc_set_base(struct drm_crtc *crtc, int x, int y,
struct radeon_device *rdev = dev->dev_private;
if (ASIC_IS_DCE4(rdev))
- return dce4_crtc_do_set_base(crtc, old_fb, x, y, 0);
- else if (ASIC_IS_AVIVO(rdev))
- return avivo_crtc_do_set_base(crtc, old_fb, x, y, 0);
- else
- return radeon_crtc_do_set_base(crtc, old_fb, x, y, 0);
-}
-
-int atombios_crtc_set_base_atomic(struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- int x, int y, enum mode_set_atomic state)
-{
- struct drm_device *dev = crtc->dev;
- struct radeon_device *rdev = dev->dev_private;
-
- if (ASIC_IS_DCE4(rdev))
- return dce4_crtc_do_set_base(crtc, fb, x, y, 1);
+ return dce4_crtc_do_set_base(crtc, old_fb, x, y);
else if (ASIC_IS_AVIVO(rdev))
- return avivo_crtc_do_set_base(crtc, fb, x, y, 1);
+ return avivo_crtc_do_set_base(crtc, old_fb, x, y);
else
- return radeon_crtc_do_set_base(crtc, fb, x, y, 1);
+ return radeon_crtc_do_set_base(crtc, old_fb, x, y);
}
/* properly set additional regs when using atombios */
@@ -2215,7 +2180,6 @@ static const struct drm_crtc_helper_funcs atombios_helper_funcs = {
.mode_fixup = atombios_crtc_mode_fixup,
.mode_set = atombios_crtc_mode_set,
.mode_set_base = atombios_crtc_set_base,
- .mode_set_base_atomic = atombios_crtc_set_base_atomic,
.prepare = atombios_crtc_prepare,
.commit = atombios_crtc_commit,
.disable = atombios_crtc_disable,
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
index 825b351ff53c..a1054c8094d4 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
@@ -360,19 +360,12 @@ static void radeon_crtc_dpms(struct drm_crtc *crtc, int mode)
int radeon_crtc_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb)
{
- return radeon_crtc_do_set_base(crtc, old_fb, x, y, 0);
-}
-
-int radeon_crtc_set_base_atomic(struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- int x, int y, enum mode_set_atomic state)
-{
- return radeon_crtc_do_set_base(crtc, fb, x, y, 1);
+ return radeon_crtc_do_set_base(crtc, old_fb, x, y);
}
int radeon_crtc_do_set_base(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- int x, int y, int atomic)
+ int x, int y)
{
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
@@ -390,15 +383,12 @@ int radeon_crtc_do_set_base(struct drm_crtc *crtc,
DRM_DEBUG_KMS("\n");
/* no fb bound */
- if (!atomic && !crtc->primary->fb) {
+ if (!crtc->primary->fb) {
DRM_DEBUG_KMS("No FB bound\n");
return 0;
}
- if (atomic)
- target_fb = fb;
- else
- target_fb = crtc->primary->fb;
+ target_fb = crtc->primary->fb;
switch (target_fb->format->cpp[0] * 8) {
case 8:
@@ -445,7 +435,7 @@ int radeon_crtc_do_set_base(struct drm_crtc *crtc,
* We don't shutdown the display controller because new buffer
* will end up in same spot.
*/
- if (!atomic && fb && fb != crtc->primary->fb) {
+ if (fb && fb != crtc->primary->fb) {
struct radeon_bo *old_rbo;
unsigned long nsize, osize;
@@ -555,7 +545,7 @@ int radeon_crtc_do_set_base(struct drm_crtc *crtc,
WREG32(RADEON_CRTC_OFFSET + radeon_crtc->crtc_offset, crtc_offset);
WREG32(RADEON_CRTC_PITCH + radeon_crtc->crtc_offset, crtc_pitch);
- if (!atomic && fb && fb != crtc->primary->fb) {
+ if (fb && fb != crtc->primary->fb) {
rbo = gem_to_radeon_bo(fb->obj[0]);
r = radeon_bo_reserve(rbo, false);
if (unlikely(r != 0))
@@ -1108,7 +1098,6 @@ static const struct drm_crtc_helper_funcs legacy_helper_funcs = {
.mode_fixup = radeon_crtc_mode_fixup,
.mode_set = radeon_crtc_mode_set,
.mode_set_base = radeon_crtc_set_base,
- .mode_set_base_atomic = radeon_crtc_set_base_atomic,
.prepare = radeon_crtc_prepare,
.commit = radeon_crtc_commit,
.disable = radeon_crtc_disable,
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 9e34da2cacef..088af85902f7 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -804,10 +804,6 @@ extern bool radeon_encoder_is_digital(struct drm_encoder *encoder);
extern void radeon_crtc_load_lut(struct drm_crtc *crtc);
extern int atombios_crtc_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb);
-extern int atombios_crtc_set_base_atomic(struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- int x, int y,
- enum mode_set_atomic state);
extern int atombios_crtc_mode_set(struct drm_crtc *crtc,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
@@ -817,13 +813,9 @@ extern void atombios_crtc_dpms(struct drm_crtc *crtc, int mode);
extern int radeon_crtc_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb);
-extern int radeon_crtc_set_base_atomic(struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- int x, int y,
- enum mode_set_atomic state);
extern int radeon_crtc_do_set_base(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- int x, int y, int atomic);
+ int x, int y);
extern int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
struct drm_file *file_priv,
uint32_t handle,
--
2.51.1
next prev parent reply other threads:[~2025-11-25 13:06 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-25 12:52 [PATCH 0/5] drm: Remove remaining support for kdb Thomas Zimmermann
2025-11-25 12:52 ` [PATCH 1/5] drm/amdgpu: Do not implement mode_set_base_atomic callback Thomas Zimmermann
2025-11-25 12:52 ` [PATCH 2/5] drm/nouveau: " Thomas Zimmermann
2025-11-26 23:07 ` Lyude Paul
2025-11-25 12:52 ` Thomas Zimmermann [this message]
2025-11-25 12:52 ` [PATCH 4/5] drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave() Thomas Zimmermann
2025-11-25 12:52 ` [PATCH 5/5] fbcon: Remove fb_debug_enter/_leave from struct fb_ops Thomas Zimmermann
2025-11-25 13:10 ` [PATCH 0/5] drm: Remove remaining support for kdb Thomas Zimmermann
2025-11-25 13:16 ` Christian König
2025-11-25 14:24 ` Thomas Zimmermann
2025-11-25 15:26 ` Doug Anderson
2025-11-25 16:22 ` Thomas Zimmermann
2025-11-26 12:19 ` Daniel Thompson
2025-11-26 13:32 ` Thomas Zimmermann
2025-11-26 14:20 ` Daniel Thompson
2025-11-25 16:25 ` Maarten Lankhorst
2025-11-26 7:44 ` Thomas Zimmermann
2025-12-01 16:41 ` Thomas Zimmermann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251125130634.1080966-4-tzimmermann@suse.de \
--to=tzimmermann@suse.de \
--cc=airlied@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=dakr@kernel.org \
--cc=danielt@kernel.org \
--cc=deller@gmx.de \
--cc=dianders@chromium.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jason.wessel@windriver.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lyude@redhat.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=simona@ffwll.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®