* [PATCH 0/2] amdgpu,radeon: Test fbdev GEM object in helper
@ 2026-03-04 12:58 Thomas Zimmermann
2026-03-04 12:58 ` [PATCH 1/2] drm/amdgpu: Move test for fbdev GEM object into generic helper Thomas Zimmermann
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Thomas Zimmermann @ 2026-03-04 12:58 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, simona
Cc: amd-gfx, dri-devel, Thomas Zimmermann
Amdgpu and radeon handle GEM objects that refer to fbdev framebuffers
separately from the other GEM objects. Share the test logic in a single
helper that works for all.
This series is purely for cleaning up the drivers. No functional changes
intended.
Thomas Zimmermann (2):
drm/amdgpu: Move test for fbdev GEM object into generic helper
drm/radeon: Test for fbdev GEM object with generic helper
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 21 +++------------
drivers/gpu/drm/drm_fb_helper.c | 29 +++++++++++++++++++++
drivers/gpu/drm/radeon/radeon_device.c | 7 ++---
drivers/gpu/drm/radeon/radeon_fbdev.c | 17 ------------
drivers/gpu/drm/radeon/radeon_mode.h | 5 ----
include/drm/drm_fb_helper.h | 9 +++++++
6 files changed, 45 insertions(+), 43 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] drm/amdgpu: Move test for fbdev GEM object into generic helper 2026-03-04 12:58 [PATCH 0/2] amdgpu,radeon: Test fbdev GEM object in helper Thomas Zimmermann @ 2026-03-04 12:58 ` Thomas Zimmermann 2026-03-05 3:27 ` Claude review: " Claude Code Review Bot 2026-03-04 12:58 ` [PATCH 2/2] drm/radeon: Test for fbdev GEM object with " Thomas Zimmermann 2026-03-05 3:27 ` Claude review: amdgpu,radeon: Test fbdev GEM object in helper Claude Code Review Bot 2 siblings, 1 reply; 8+ messages in thread From: Thomas Zimmermann @ 2026-03-04 12:58 UTC (permalink / raw) To: alexander.deucher, christian.koenig, airlied, simona Cc: amd-gfx, dri-devel, Thomas Zimmermann Provide a generic helper that tests if fbdev emulation is backed by a specific GEM object. Not all drivers use client buffers (yet), hence also test against the first GEM object in the fbdev framebuffer. Convert amdgpu. The helper will also be useful for radeon. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 21 +++------------ drivers/gpu/drm/drm_fb_helper.c | 29 +++++++++++++++++++++ include/drm/drm_fb_helper.h | 9 +++++++ 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index bef9dce2e7ea..f5cd68542442 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -1738,21 +1738,6 @@ bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc, stime, etime, mode); } -static bool -amdgpu_display_robj_is_fb(struct amdgpu_device *adev, struct amdgpu_bo *robj) -{ - struct drm_device *dev = adev_to_drm(adev); - struct drm_fb_helper *fb_helper = dev->fb_helper; - - if (!fb_helper || !fb_helper->buffer) - return false; - - if (gem_to_amdgpu_bo(fb_helper->buffer->gem) != robj) - return false; - - return true; -} - int amdgpu_display_suspend_helper(struct amdgpu_device *adev) { struct drm_device *dev = adev_to_drm(adev); @@ -1775,7 +1760,6 @@ int amdgpu_display_suspend_helper(struct amdgpu_device *adev) list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); struct drm_framebuffer *fb = crtc->primary->fb; - struct amdgpu_bo *robj; if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) { struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo); @@ -1790,8 +1774,9 @@ int amdgpu_display_suspend_helper(struct amdgpu_device *adev) if (!fb || !fb->obj[0]) continue; - robj = gem_to_amdgpu_bo(fb->obj[0]); - if (!amdgpu_display_robj_is_fb(adev, robj)) { + if (!drm_fb_helper_gem_is_fb(dev->fb_helper, fb->obj[0])) { + struct amdgpu_bo *robj = gem_to_amdgpu_bo(fb->obj[0]); + r = amdgpu_bo_reserve(robj, true); if (r == 0) { amdgpu_bo_unpin(robj); diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 845c63ca15b5..214337ebb559 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -37,6 +37,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/drm_modeset_helper_vtables.h> #include <drm/drm_print.h> #include <drm/drm_vblank.h> @@ -1771,3 +1772,31 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) return 0; } EXPORT_SYMBOL(drm_fb_helper_hotplug_event); + +/** + * drm_fb_helper_gem_is_fb - Tests if GEM object is framebuffer + * @fb_helper: fb_helper instance, can be NULL + * @obj: The GEM object to test, can be NULL + * + * Call drm_fb_helper_gem_is_fb to test is a DRM device's fbdev emulation + * uses the specified GEM object for its framebuffer. The result is always + * false if either poiner is NULL. + * + * Returns: + * True if fbdev emulation uses the provided GEM object, or false otherwise. + */ +bool drm_fb_helper_gem_is_fb(const struct drm_fb_helper *fb_helper, + const struct drm_gem_object *obj) +{ + const struct drm_gem_object *gem = NULL; + + if (!fb_helper || !obj) + return false; + if (fb_helper->buffer && fb_helper->buffer->gem) + gem = fb_helper->buffer->gem; + else if (fb_helper->fb) + gem = drm_gem_fb_get_obj(fb_helper->fb, 0); + + return gem == obj; +} +EXPORT_SYMBOL_GPL(drm_fb_helper_gem_is_fb); diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 15274b8a1d97..c59f16c21723 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -271,6 +271,15 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper); int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper); + +bool drm_fb_helper_gem_is_fb(const struct drm_fb_helper *fb_helper, + const struct drm_gem_object *obj); +#else +static inline bool drm_fb_helper_gem_is_fb(const struct drm_fb_helper *fb_helper, + const struct drm_gem_object *obj) +{ + return false; +} #endif #endif -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Claude review: drm/amdgpu: Move test for fbdev GEM object into generic helper 2026-03-04 12:58 ` [PATCH 1/2] drm/amdgpu: Move test for fbdev GEM object into generic helper Thomas Zimmermann @ 2026-03-05 3:27 ` Claude Code Review Bot 0 siblings, 0 replies; 8+ messages in thread From: Claude Code Review Bot @ 2026-03-05 3:27 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Overall**: Good patch. The new helper correctly generalizes the amdgpu-specific check and is more flexible by also supporting the `fb_helper->fb` path (via `drm_gem_fb_get_obj`) when `buffer` isn't set. **EXPORT_SYMBOL_GPL vs EXPORT_SYMBOL**: Every other export in `drm_fb_helper.c` uses `EXPORT_SYMBOL` (non-GPL). The new function uses: ```c EXPORT_SYMBOL_GPL(drm_fb_helper_gem_is_fb); ``` This should be `EXPORT_SYMBOL` for consistency with the rest of the file. While both amdgpu and radeon are GPL-licensed so this works today, using `EXPORT_SYMBOL_GPL` here is inconsistent and could trip up any hypothetical non-GPL consumer that uses the other fb_helper functions. **Typo in kdoc**: Minor typo in the documentation comment: ```c * Call drm_fb_helper_gem_is_fb to test is a DRM device's fbdev emulation ``` "test is" should be "test if". **Logic correctness**: The helper's fallback logic is sound: ```c if (fb_helper->buffer && fb_helper->buffer->gem) gem = fb_helper->buffer->gem; else if (fb_helper->fb) gem = drm_gem_fb_get_obj(fb_helper->fb, 0); ``` This correctly handles the two paths: drivers using client buffers (amdgpu's original path checked `buffer->gem`) and drivers using `fb` directly (radeon's original path used `drm_gem_fb_get_obj(fb_helper->fb, 0)`). The original amdgpu code only checked `buffer->gem`, and the new helper adds the `fb` fallback, which is a strict superset — safe because if `buffer` is set, it will be checked first. **Scoping improvement**: Nice touch moving `robj` into the inner block where it's actually used: ```c - robj = gem_to_amdgpu_bo(fb->obj[0]); - if (!amdgpu_display_robj_is_fb(adev, robj)) { + if (!drm_fb_helper_gem_is_fb(dev->fb_helper, fb->obj[0])) { + struct amdgpu_bo *robj = gem_to_amdgpu_bo(fb->obj[0]); ``` This avoids the unnecessary conversion when the object *is* the fbdev framebuffer. **Header placement**: The `#else` static inline stub is correctly placed — the existing header had no `#else` for `CONFIG_DRM_FBDEV_EMULATION`, and this adds one cleanly. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/radeon: Test for fbdev GEM object with generic helper 2026-03-04 12:58 [PATCH 0/2] amdgpu,radeon: Test fbdev GEM object in helper Thomas Zimmermann 2026-03-04 12:58 ` [PATCH 1/2] drm/amdgpu: Move test for fbdev GEM object into generic helper Thomas Zimmermann @ 2026-03-04 12:58 ` Thomas Zimmermann 2026-03-04 15:32 ` Alex Deucher 2026-03-05 3:27 ` Claude review: " Claude Code Review Bot 2026-03-05 3:27 ` Claude review: amdgpu,radeon: Test fbdev GEM object in helper Claude Code Review Bot 2 siblings, 2 replies; 8+ messages in thread From: Thomas Zimmermann @ 2026-03-04 12:58 UTC (permalink / raw) To: alexander.deucher, christian.koenig, airlied, simona Cc: amd-gfx, dri-devel, Thomas Zimmermann Replace radeon's test for the fbdev GEM object with a call to the generic helper. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/radeon/radeon_device.c | 7 ++++--- drivers/gpu/drm/radeon/radeon_fbdev.c | 17 ----------------- drivers/gpu/drm/radeon/radeon_mode.h | 5 ----- 3 files changed, 4 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 5d523d5dae88..705c012fcf9e 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -37,6 +37,7 @@ #include <drm/drm_client_event.h> #include <drm/drm_crtc_helper.h> #include <drm/drm_device.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_file.h> #include <drm/drm_framebuffer.h> #include <drm/drm_probe_helper.h> @@ -1574,7 +1575,6 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc); struct drm_framebuffer *fb = crtc->primary->fb; - struct radeon_bo *robj; if (radeon_crtc->cursor_bo) { struct radeon_bo *robj = gem_to_radeon_bo(radeon_crtc->cursor_bo); @@ -1588,9 +1588,10 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, if (fb == NULL || fb->obj[0] == NULL) { continue; } - robj = gem_to_radeon_bo(fb->obj[0]); /* don't unpin kernel fb objects */ - if (!radeon_fbdev_robj_is_fb(rdev, robj)) { + if (!drm_fb_helper_gem_is_fb(dev->fb_helper, fb->obj[0])) { + struct radeon_bo *robj = gem_to_radeon_bo(fb->obj[0]); + r = radeon_bo_reserve(robj, false); if (r == 0) { radeon_bo_unpin(robj); diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c index 18d61f3f7344..3e243f5e2f44 100644 --- a/drivers/gpu/drm/radeon/radeon_fbdev.c +++ b/drivers/gpu/drm/radeon/radeon_fbdev.c @@ -274,20 +274,3 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper, radeon_fbdev_destroy_pinned_object(gobj); return ret; } - -bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj) -{ - struct drm_fb_helper *fb_helper = rdev_to_drm(rdev)->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; - - return true; -} diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index 088af85902f7..ae1ecdc2e189 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -936,14 +936,9 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper, struct drm_fb_helper_surface_size *sizes); #define RADEON_FBDEV_DRIVER_OPS \ .fbdev_probe = radeon_fbdev_driver_fbdev_probe -bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj); #else #define RADEON_FBDEV_DRIVER_OPS \ .fbdev_probe = NULL -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.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/radeon: Test for fbdev GEM object with generic helper 2026-03-04 12:58 ` [PATCH 2/2] drm/radeon: Test for fbdev GEM object with " Thomas Zimmermann @ 2026-03-04 15:32 ` Alex Deucher 2026-03-04 15:49 ` Thomas Zimmermann 2026-03-05 3:27 ` Claude review: " Claude Code Review Bot 1 sibling, 1 reply; 8+ messages in thread From: Alex Deucher @ 2026-03-04 15:32 UTC (permalink / raw) To: Thomas Zimmermann Cc: alexander.deucher, christian.koenig, airlied, simona, amd-gfx, dri-devel On Wed, Mar 4, 2026 at 8:03 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Replace radeon's test for the fbdev GEM object with a call to the > generic helper. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Series is: Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Did you want me to pick these up or did you want to land them in drm-misc? Alex > --- > drivers/gpu/drm/radeon/radeon_device.c | 7 ++++--- > drivers/gpu/drm/radeon/radeon_fbdev.c | 17 ----------------- > drivers/gpu/drm/radeon/radeon_mode.h | 5 ----- > 3 files changed, 4 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c > index 5d523d5dae88..705c012fcf9e 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -37,6 +37,7 @@ > #include <drm/drm_client_event.h> > #include <drm/drm_crtc_helper.h> > #include <drm/drm_device.h> > +#include <drm/drm_fb_helper.h> > #include <drm/drm_file.h> > #include <drm/drm_framebuffer.h> > #include <drm/drm_probe_helper.h> > @@ -1574,7 +1575,6 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc); > struct drm_framebuffer *fb = crtc->primary->fb; > - struct radeon_bo *robj; > > if (radeon_crtc->cursor_bo) { > struct radeon_bo *robj = gem_to_radeon_bo(radeon_crtc->cursor_bo); > @@ -1588,9 +1588,10 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, > if (fb == NULL || fb->obj[0] == NULL) { > continue; > } > - robj = gem_to_radeon_bo(fb->obj[0]); > /* don't unpin kernel fb objects */ > - if (!radeon_fbdev_robj_is_fb(rdev, robj)) { > + if (!drm_fb_helper_gem_is_fb(dev->fb_helper, fb->obj[0])) { > + struct radeon_bo *robj = gem_to_radeon_bo(fb->obj[0]); > + > r = radeon_bo_reserve(robj, false); > if (r == 0) { > radeon_bo_unpin(robj); > diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c > index 18d61f3f7344..3e243f5e2f44 100644 > --- a/drivers/gpu/drm/radeon/radeon_fbdev.c > +++ b/drivers/gpu/drm/radeon/radeon_fbdev.c > @@ -274,20 +274,3 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper, > radeon_fbdev_destroy_pinned_object(gobj); > return ret; > } > - > -bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj) > -{ > - struct drm_fb_helper *fb_helper = rdev_to_drm(rdev)->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; > - > - return true; > -} > diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h > index 088af85902f7..ae1ecdc2e189 100644 > --- a/drivers/gpu/drm/radeon/radeon_mode.h > +++ b/drivers/gpu/drm/radeon/radeon_mode.h > @@ -936,14 +936,9 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper, > struct drm_fb_helper_surface_size *sizes); > #define RADEON_FBDEV_DRIVER_OPS \ > .fbdev_probe = radeon_fbdev_driver_fbdev_probe > -bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj); > #else > #define RADEON_FBDEV_DRIVER_OPS \ > .fbdev_probe = NULL > -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.53.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/radeon: Test for fbdev GEM object with generic helper 2026-03-04 15:32 ` Alex Deucher @ 2026-03-04 15:49 ` Thomas Zimmermann 0 siblings, 0 replies; 8+ messages in thread From: Thomas Zimmermann @ 2026-03-04 15:49 UTC (permalink / raw) To: Alex Deucher Cc: alexander.deucher, christian.koenig, airlied, simona, amd-gfx, dri-devel Hi Am 04.03.26 um 16:32 schrieb Alex Deucher: > On Wed, Mar 4, 2026 at 8:03 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Replace radeon's test for the fbdev GEM object with a call to the >> generic helper. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Series is: > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Thanks > > Did you want me to pick these up or did you want to land them in drm-misc? Please add it to your tree. Best regards Thomas > > Alex > >> --- >> drivers/gpu/drm/radeon/radeon_device.c | 7 ++++--- >> drivers/gpu/drm/radeon/radeon_fbdev.c | 17 ----------------- >> drivers/gpu/drm/radeon/radeon_mode.h | 5 ----- >> 3 files changed, 4 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c >> index 5d523d5dae88..705c012fcf9e 100644 >> --- a/drivers/gpu/drm/radeon/radeon_device.c >> +++ b/drivers/gpu/drm/radeon/radeon_device.c >> @@ -37,6 +37,7 @@ >> #include <drm/drm_client_event.h> >> #include <drm/drm_crtc_helper.h> >> #include <drm/drm_device.h> >> +#include <drm/drm_fb_helper.h> >> #include <drm/drm_file.h> >> #include <drm/drm_framebuffer.h> >> #include <drm/drm_probe_helper.h> >> @@ -1574,7 +1575,6 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, >> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { >> struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc); >> struct drm_framebuffer *fb = crtc->primary->fb; >> - struct radeon_bo *robj; >> >> if (radeon_crtc->cursor_bo) { >> struct radeon_bo *robj = gem_to_radeon_bo(radeon_crtc->cursor_bo); >> @@ -1588,9 +1588,10 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, >> if (fb == NULL || fb->obj[0] == NULL) { >> continue; >> } >> - robj = gem_to_radeon_bo(fb->obj[0]); >> /* don't unpin kernel fb objects */ >> - if (!radeon_fbdev_robj_is_fb(rdev, robj)) { >> + if (!drm_fb_helper_gem_is_fb(dev->fb_helper, fb->obj[0])) { >> + struct radeon_bo *robj = gem_to_radeon_bo(fb->obj[0]); >> + >> r = radeon_bo_reserve(robj, false); >> if (r == 0) { >> radeon_bo_unpin(robj); >> diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c >> index 18d61f3f7344..3e243f5e2f44 100644 >> --- a/drivers/gpu/drm/radeon/radeon_fbdev.c >> +++ b/drivers/gpu/drm/radeon/radeon_fbdev.c >> @@ -274,20 +274,3 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper, >> radeon_fbdev_destroy_pinned_object(gobj); >> return ret; >> } >> - >> -bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj) >> -{ >> - struct drm_fb_helper *fb_helper = rdev_to_drm(rdev)->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; >> - >> - return true; >> -} >> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h >> index 088af85902f7..ae1ecdc2e189 100644 >> --- a/drivers/gpu/drm/radeon/radeon_mode.h >> +++ b/drivers/gpu/drm/radeon/radeon_mode.h >> @@ -936,14 +936,9 @@ int radeon_fbdev_driver_fbdev_probe(struct drm_fb_helper *fb_helper, >> struct drm_fb_helper_surface_size *sizes); >> #define RADEON_FBDEV_DRIVER_OPS \ >> .fbdev_probe = radeon_fbdev_driver_fbdev_probe >> -bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj); >> #else >> #define RADEON_FBDEV_DRIVER_OPS \ >> .fbdev_probe = NULL >> -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.53.0 >> -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Claude review: drm/radeon: Test for fbdev GEM object with generic helper 2026-03-04 12:58 ` [PATCH 2/2] drm/radeon: Test for fbdev GEM object with " Thomas Zimmermann 2026-03-04 15:32 ` Alex Deucher @ 2026-03-05 3:27 ` Claude Code Review Bot 1 sibling, 0 replies; 8+ messages in thread From: Claude Code Review Bot @ 2026-03-05 3:27 UTC (permalink / raw) To: dri-devel-reviews Patch Review **Overall**: Straightforward conversion. Removes radeon's `radeon_fbdev_robj_is_fb()` and replaces its call site with `drm_fb_helper_gem_is_fb()`. **Correct conversion**: The original radeon code used `drm_gem_fb_get_obj(fb_helper->fb, 0)` directly, while the new helper covers this via the `else if (fb_helper->fb)` branch. The behavior is preserved. **Same scoping improvement** as patch 1 — `robj` moves into the block where it's needed: ```c - robj = gem_to_radeon_bo(fb->obj[0]); /* don't unpin kernel fb objects */ - if (!radeon_fbdev_robj_is_fb(rdev, robj)) { + if (!drm_fb_helper_gem_is_fb(dev->fb_helper, fb->obj[0])) { + struct radeon_bo *robj = gem_to_radeon_bo(fb->obj[0]); ``` **Clean removal**: Both the implementation in `radeon_fbdev.c` and the declaration + static inline stub in `radeon_mode.h` are properly removed. **Summary of actionable items**: 1. Change `EXPORT_SYMBOL_GPL` to `EXPORT_SYMBOL` for consistency with the rest of `drm_fb_helper.c`. 2. Fix typo: "test is" → "test if" in the kdoc comment. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 8+ messages in thread
* Claude review: amdgpu,radeon: Test fbdev GEM object in helper 2026-03-04 12:58 [PATCH 0/2] amdgpu,radeon: Test fbdev GEM object in helper Thomas Zimmermann 2026-03-04 12:58 ` [PATCH 1/2] drm/amdgpu: Move test for fbdev GEM object into generic helper Thomas Zimmermann 2026-03-04 12:58 ` [PATCH 2/2] drm/radeon: Test for fbdev GEM object with " Thomas Zimmermann @ 2026-03-05 3:27 ` Claude Code Review Bot 2 siblings, 0 replies; 8+ messages in thread From: Claude Code Review Bot @ 2026-03-05 3:27 UTC (permalink / raw) To: dri-devel-reviews Overall Series Review Subject: amdgpu,radeon: Test fbdev GEM object in helper Author: Thomas Zimmermann <tzimmermann@suse.de> Patches: 5 Reviewed: 2026-03-05T13:27:24.252186 --- This is a clean, straightforward refactoring series by Thomas Zimmermann that extracts a common pattern — "is this GEM object the fbdev framebuffer?" — from both amdgpu and radeon into a single generic helper `drm_fb_helper_gem_is_fb()` in the DRM core. The series is well-structured: patch 1 introduces the helper and converts amdgpu, patch 2 converts radeon. No functional changes are intended, and the logic looks correct. One issue to flag: the export symbol type is inconsistent with the rest of the file. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-03-05 3:27 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-04 12:58 [PATCH 0/2] amdgpu,radeon: Test fbdev GEM object in helper Thomas Zimmermann 2026-03-04 12:58 ` [PATCH 1/2] drm/amdgpu: Move test for fbdev GEM object into generic helper Thomas Zimmermann 2026-03-05 3:27 ` Claude review: " Claude Code Review Bot 2026-03-04 12:58 ` [PATCH 2/2] drm/radeon: Test for fbdev GEM object with " Thomas Zimmermann 2026-03-04 15:32 ` Alex Deucher 2026-03-04 15:49 ` Thomas Zimmermann 2026-03-05 3:27 ` Claude review: " Claude Code Review Bot 2026-03-05 3:27 ` Claude review: amdgpu,radeon: Test fbdev GEM object in helper Claude Code Review Bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox