Linux Framebuffer Layer development

Linux Framebuffer Layer development
 help / color / mirror / Atom feed

* Re: [PATCH 0/5] drm: Remove remaining support for kdb
From: Daniel Thompson @ 2025-11-26 12:19 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Thomas Zimmermann, simona, airlied, alexander.deucher,
	christian.koenig, lyude, dakr, deller, mripard, maarten.lankhorst,
	jason.wessel, dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc,
	linux-kernel, Nir Lichtman
In-Reply-To: <CAD=FV=X_-t2AF5osp7Hamoe7WYE_2YWJZCaPaOj=9seSbnwwVA@mail.gmail.com>

On Tue, Nov 25, 2025 at 07:26:33AM -0800, Doug Anderson wrote:
> On Tue, Nov 25, 2025 at 5:06 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > <snip>
> > Therefore remove the remaining support for kdb from the DRM drivers
> > and from DRM fbdev emulation. Also remove the hooks from fbdev, as
> > there are no fbdev drivers with kdb support.
> >
> > If we ever want to address kdb support within DRM drivers, a place to
> > start would be the scanout buffers used by DRM's panic screen. These
> > use the current display mode. They can be written and flushed without
> > mode setting involved.
> >
> > Note: kdb over serial lines is not affected by this series and continues
> > to work as before.
> >
> > Thomas Zimmermann (5):
> >   drm/amdgpu: Do not implement mode_set_base_atomic callback
> >   drm/nouveau: Do not implement mode_set_base_atomic callback
> >   drm/radeon: Do not implement mode_set_base_atomic callback
> >   drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
> >   fbcon: Remove fb_debug_enter/_leave from struct fb_ops
>
> Personally, I've never worked with kdb over anything other than
> serial, so this won't bother any of my normal workflows. That being
> said, at least as of a year ago someone on the lists was talking about
> using kdb with a keyboard and (presumably) a display. You can see a
> thread here:
>
> http://lore.kernel.org/r/20241031192350.GA26688@lichtman.org
>
> Daniel may also have comments here?

TL;DR - I'm pretty relaxed about these changes... but I'd like
        to know how to test the changes.

Like Doug I only really use kdb via serial but, since I'm maintain
the thing I do occasionally test kdb works on the qemu console. I don't
do it very often though because it's a manual test!

I'd assume that will still work since it won't involve any of the
drivers above. I'm afraid I can't double check that since patch 4
doesn't apply cleanly in v6.18-rc7 (nor to linux-next... and neither
does the base-commit appear in linux-next).

Anyhow, the only testing I do for kgdboc=kms,kdb is to boot an x86-64
defconfig+kgdb+kdb kernel in qemu with something like the following
command line, which FWIW does still work:

    qemu-system-x86_64 -enable-kvm -m 1G -smp 2 \
      -kernel arch/x86/boot/bzImage \
      -monitor none -chardev stdio,id=mon,mux=on,signal=off \
      -serial chardev:mon \
      -initrd rootfs.cpio.gz \
      -append " console=tty0 console=ttyS0,115200 kgdboc=kms,kbd,ttyS0 kgdbwait"

The reason I'm fairly relaxed about changes here is that the kbd driver
only works on PCs with legacy keyboard interfaces. If the kernel is
talking to the keyboard using USB or I2C (which almost all PCs do) then
kdb cannot be used anyway.

So... it would be a "cool project"[1] to get kdb running on
a special interrupt-free I2C mode and with the DRM panic code so you
can do live analysis if your laptop/chomebook crashes. However it is
simply not "real enough" to justify slowing down other developers.


Daniel.


[1] ... but not quite cool enough that I see myself spending time on it
    though!

^ permalink raw reply

* Re: [PATCH] gpu: Move DRM buddy allocator one level up
From: Zhi Wang @ 2025-11-26  7:44 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Christian König, John Hubbard, Joel Fernandes, linux-kernel,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Jonathan Corbet, Alex Deucher, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, Danilo Krummrich, Alice Ryhl,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, nouveau, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <CAPM=9tx5neQ=TbmK+2eAO=O-XW_67VhOGO-791kqyVDJEpTA+w@mail.gmail.com>

On Wed, 26 Nov 2025 06:17:25 +1000
Dave Airlie <airlied@gmail.com> wrote:

> On Tue, 25 Nov 2025 at 19:15, Christian König <christian.koenig@amd.com>
> wrote:
> >
> > On 11/25/25 10:08, Dave Airlie wrote:

snip

> So far I haven't heard anything about needing dma-buf interactions at
> that level, and maybe Zhi has more insight into the future there.
> 

For VFIO parts, the vendor VFIO driver relies on the core driver's support
to export the VM's framebuffer to the userspace either as type a)
VFIO_GFX_PLANE_TYPE_REGION, framebuffer as a region or b)
VFIO_GFX_PLANE_TYPE_DMABUF, framebuffer as a dmabuf, which is usually
backed by a GEM object.

NVIDIA vGPU uses VFIO_GFX_PLANE_TYPE_REGION, so dma-buf interactions
are not required.

> Dave.


^ permalink raw reply

* Re: [PATCH 0/5] drm: Remove remaining support for kdb
From: Thomas Zimmermann @ 2025-11-26  7:44 UTC (permalink / raw)
  To: Maarten Lankhorst, simona, airlied, alexander.deucher,
	christian.koenig, lyude, dakr, deller, mripard, jason.wessel,
	danielt, dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel
In-Reply-To: <82ed9798-9237-4404-9b32-9430bfb82b26@linux.intel.com>

Hi

Am 25.11.25 um 17:25 schrieb Maarten Lankhorst:
> Hey,
>
> I'm glad to see the old kdb handler gone.
>
> Could we perhaps extend the drm panic handler somehow for this to work?
> Restore could potentially be simply duplicating and committing the current state.

Yeah, I briefly outlined this in the cover letter. If we wanted to 
support kdb, we could get the scanout buffer and use the buffer's simple 
display update for showing the debugger. I think this still requires 
quite some work, but would avoid all the issues with the current approach.

Best regards
Thomas

>
> Kind regards,
> ~Maarten Lankhorst
>
> Den 2025-11-25 kl. 13:52, skrev Thomas Zimmermann:
>> Remove the rest of the kbd support from DRM. Driver support has been
>> broken for years without anyone complaining.
>>
>> Kdb cannot use regular DRM mode setting, so DRM drivers have to
>> implement an additional hook to make it work (in theory). As outlined
>> by Sima in commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for
>> atomic drivers") from 2017, kdb is not compatible with DRM atomic mode
>> setting. Non-atomic mode setting meanwhile has become rare.
>>
>> Only 3 DRM drivers implement the hooks for kdb support. Amdgpu and
>> nouveau use non-atomic mode setting on older devices. But both drivers
>> have switched to generic fbdev emulation, which isn't compatible with
>> kdb. Radeon still runs kdb, but it doesn't work in practice. See the
>> commits in this series for details
>>
>> Therefore remove the remaining support for kdb from the DRM drivers
>> and from DRM fbdev emulation. Also remove the hooks from fbdev, as
>> there are no fbdev drivers with kdb support.
>>
>> If we ever want to address kdb support within DRM drivers, a place to
>> start would be the scanout buffers used by DRM's panic screen. These
>> use the current display mode. They can be written and flushed without
>> mode setting involved.
>>
>> Note: kdb over serial lines is not affected by this series and continues
>> to work as before.
>>
>> Thomas Zimmermann (5):
>>    drm/amdgpu: Do not implement mode_set_base_atomic callback
>>    drm/nouveau: Do not implement mode_set_base_atomic callback
>>    drm/radeon: Do not implement mode_set_base_atomic callback
>>    drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
>>    fbcon: Remove fb_debug_enter/_leave from struct fb_ops
>>
>>   Documentation/process/debugging/kgdb.rst    |  28 -----
>>   drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  35 ++-----
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  35 ++-----
>>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  35 ++-----
>>   drivers/gpu/drm/drm_fb_helper.c             | 108 --------------------
>>   drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  24 +----
>>   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 +-
>>   drivers/video/fbdev/core/fbcon.c            |  24 -----
>>   drivers/video/fbdev/core/fbcon.h            |   1 -
>>   include/drm/drm_fb_helper.h                 |  21 ----
>>   include/drm/drm_modeset_helper_vtables.h    |  23 -----
>>   include/linux/fb.h                          |   4 -
>>   14 files changed, 63 insertions(+), 382 deletions(-)
>>
>>
>> base-commit: 0a21e96e0b6840d2a4e0b45a957679eeddeb4362

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

* Re: [PATCH] fbdev: core: Fix vmalloc-out-of-bounds in fb_imageblit
From: kernel test robot @ 2025-11-26  6:55 UTC (permalink / raw)
  To: ssrane_b23, Zsolt Kajtar, Simona Vetter, Helge Deller
  Cc: oe-kbuild-all, Shaurya Rane, linux-fbdev, dri-devel, linux-kernel,
	syzbot+5a40432dfe8f86ee657a
In-Reply-To: <20251119133821.89998-1-ssranevjti@gmail.com>

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.18-rc7 next-20251126]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/ssrane_b23-ee-vjti-ac-in/fbdev-core-Fix-vmalloc-out-of-bounds-in-fb_imageblit/20251119-215054
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20251119133821.89998-1-ssranevjti%40gmail.com
patch subject: [PATCH] fbdev: core: Fix vmalloc-out-of-bounds in fb_imageblit
config: x86_64-rhel-9.4-ltp (https://download.01.org/0day-ci/archive/20251126/202511260749.KJgv3MyF-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251126/202511260749.KJgv3MyF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511260749.KJgv3MyF-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/video/fbdev/core/cfbimgblt.c:17:
   drivers/video/fbdev/core/fb_imageblit.h: In function 'fb_imageblit':
>> drivers/video/fbdev/core/fb_imageblit.h:490:23: warning: unused variable 'max_offset_bytes' [-Wunused-variable]
     490 |         unsigned long max_offset_bytes;
         |                       ^~~~~~~~~~~~~~~~


vim +/max_offset_bytes +490 drivers/video/fbdev/core/fb_imageblit.h

   480	
   481	static inline void fb_imageblit(struct fb_info *p, const struct fb_image *image)
   482	{
   483		int bpp = p->var.bits_per_pixel;
   484		unsigned int bits_per_line = BYTES_TO_BITS(p->fix.line_length);
   485		struct fb_address dst = fb_address_init(p);
   486		struct fb_reverse reverse = fb_reverse_init(p);
   487		const u32 *palette = fb_palette(p);
   488		struct fb_image clipped_image;
   489		u32 max_x, max_y;
 > 490		unsigned long max_offset_bytes;

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH] gpu: Move DRM buddy allocator one level up
From: Dave Airlie @ 2025-11-25 20:17 UTC (permalink / raw)
  To: Christian König
  Cc: John Hubbard, Joel Fernandes, linux-kernel, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
	Lucas De Marchi, Thomas Hellström, Helge Deller,
	Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, Alistair Popple, Timur Tabi,
	Edwin Peer, Alexandre Courbot, nouveau, dri-devel, rust-for-linux,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev, Zhi Wang
In-Reply-To: <cc0db376-6cff-45d7-b3a3-d13be664700f@amd.com>

On Tue, 25 Nov 2025 at 19:15, Christian König <christian.koenig@amd.com> wrote:
>
> On 11/25/25 10:08, Dave Airlie wrote:
> > On Tue, 25 Nov 2025 at 18:11, Christian König <christian.koenig@amd.com> wrote:
> >>
> >> On 11/25/25 08:59, John Hubbard wrote:
> >>> On 11/24/25 11:54 PM, Christian König wrote:
> >>>> On 11/25/25 08:49, Dave Airlie wrote:
> >>>>> On Tue, 25 Nov 2025 at 17:45, Christian König <christian.koenig@amd.com> wrote:
> >>> ...
> >>>> My question is why exactly is nova separated into nova-core and nova-drm? That doesn't seem to be necessary in the first place.
> >>>>
> >>> The idea is that nova-core allows building up a separate software stack for
> >>> VFIO, without pulling in any DRM-specific code that a hypervisor (for example)
> >>> wouldn't need. That makes for a smaller, more security-auditable set of code
> >>> for that case.
> >>
> >> Well that is the same argument used by some AMD team to maintain a separate out of tree hypervisor for nearly a decade.
> >>
> >> Additional to that the same argument has also been used to justify the KFD node as alternative API to DRM for compute.
> >>
> >> Both cases have proven to be extremely bad ideas.
> >>
> >> Background is that except for all the legacy stuff the DRM API is actually very well thought through and it is actually quite hard to come up with something similarly well.
> >>
> >
> > Well you just answered your own question, why is AMD maintaining GIM
> > instead of solving this upstream with a split model? the nova-core/drm
> > split would be perfect for GIM.
>
> No, it won't.
>
> We have the requirement to work with GEM objects and DMA-buf file descriptors in the hypervisor as well.
>
> And my suspicion is that you end up with the same requirements in nova as well in which case you end up interchanging handles with DRM as well.
>
> We have seen the same for KFD and it turned out to be an absolutely horrible interaction.
>
> > kfd was a terrible idea, and we don't intend to offer userspace
> > multiple APIs with nova, nova-drm will be the primary userspace API
> > provider. nova-core will not provide userspace API, it will provide an
> > API to nova-drm and an API to the vgpu driver which will provide it's
> > own userspace API without graphics or compute, just enough to setup
> > VFs.
>
> Ok, then why do you need nova-core in the first place? E.g. where should be the vgpu driver and what interface does it provide?

The ask is for a driver for cloud providers to run on their
hypervisors that does just enough to manage the VFs through VFIO
without having a complete drm driver or any drm infrastructure loaded.

The nice pictures are here
https://lore.kernel.org/all/20250903221111.3866249-1-zhiw@nvidia.com/

You will only be loading one of nova-drm or the vfio driver at least
in supported systems, depending on the GPU configuration, whether we
allow users to do things like that isn't well decided.

So far I haven't heard anything about needing dma-buf interactions at
that level, and maybe Zhi has more insight into the future there.

Dave.

^ permalink raw reply

* Re: [PATCH 0/5] drm: Remove remaining support for kdb
From: Maarten Lankhorst @ 2025-11-25 16:25 UTC (permalink / raw)
  To: Thomas Zimmermann, simona, airlied, alexander.deucher,
	christian.koenig, lyude, dakr, deller, mripard, jason.wessel,
	danielt, dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>

Hey,

I'm glad to see the old kdb handler gone.

Could we perhaps extend the drm panic handler somehow for this to work?
Restore could potentially be simply duplicating and committing the current state.

Kind regards,
~Maarten Lankhorst

Den 2025-11-25 kl. 13:52, skrev Thomas Zimmermann:
> Remove the rest of the kbd support from DRM. Driver support has been
> broken for years without anyone complaining.
> 
> Kdb cannot use regular DRM mode setting, so DRM drivers have to
> implement an additional hook to make it work (in theory). As outlined
> by Sima in commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for
> atomic drivers") from 2017, kdb is not compatible with DRM atomic mode
> setting. Non-atomic mode setting meanwhile has become rare.
> 
> Only 3 DRM drivers implement the hooks for kdb support. Amdgpu and
> nouveau use non-atomic mode setting on older devices. But both drivers
> have switched to generic fbdev emulation, which isn't compatible with
> kdb. Radeon still runs kdb, but it doesn't work in practice. See the
> commits in this series for details
> 
> Therefore remove the remaining support for kdb from the DRM drivers
> and from DRM fbdev emulation. Also remove the hooks from fbdev, as
> there are no fbdev drivers with kdb support.
> 
> If we ever want to address kdb support within DRM drivers, a place to
> start would be the scanout buffers used by DRM's panic screen. These
> use the current display mode. They can be written and flushed without
> mode setting involved.
> 
> Note: kdb over serial lines is not affected by this series and continues
> to work as before.
> 
> Thomas Zimmermann (5):
>   drm/amdgpu: Do not implement mode_set_base_atomic callback
>   drm/nouveau: Do not implement mode_set_base_atomic callback
>   drm/radeon: Do not implement mode_set_base_atomic callback
>   drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
>   fbcon: Remove fb_debug_enter/_leave from struct fb_ops
> 
>  Documentation/process/debugging/kgdb.rst    |  28 -----
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  35 ++-----
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  35 ++-----
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  35 ++-----
>  drivers/gpu/drm/drm_fb_helper.c             | 108 --------------------
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  24 +----
>  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 +-
>  drivers/video/fbdev/core/fbcon.c            |  24 -----
>  drivers/video/fbdev/core/fbcon.h            |   1 -
>  include/drm/drm_fb_helper.h                 |  21 ----
>  include/drm/drm_modeset_helper_vtables.h    |  23 -----
>  include/linux/fb.h                          |   4 -
>  14 files changed, 63 insertions(+), 382 deletions(-)
> 
> 
> base-commit: 0a21e96e0b6840d2a4e0b45a957679eeddeb4362


^ permalink raw reply

* Re: [PATCH 0/5] drm: Remove remaining support for kdb
From: Thomas Zimmermann @ 2025-11-25 16:22 UTC (permalink / raw)
  To: Doug Anderson
  Cc: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Nir Lichtman
In-Reply-To: <CAD=FV=X_-t2AF5osp7Hamoe7WYE_2YWJZCaPaOj=9seSbnwwVA@mail.gmail.com>

Hi

Am 25.11.25 um 16:26 schrieb Doug Anderson:
> Hi,
>
> On Tue, Nov 25, 2025 at 5:06 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Remove the rest of the kbd support from DRM. Driver support has been
>> broken for years without anyone complaining.
>>
>> Kdb cannot use regular DRM mode setting, so DRM drivers have to
>> implement an additional hook to make it work (in theory). As outlined
>> by Sima in commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for
>> atomic drivers") from 2017, kdb is not compatible with DRM atomic mode
>> setting. Non-atomic mode setting meanwhile has become rare.
>>
>> Only 3 DRM drivers implement the hooks for kdb support. Amdgpu and
>> nouveau use non-atomic mode setting on older devices. But both drivers
>> have switched to generic fbdev emulation, which isn't compatible with
>> kdb. Radeon still runs kdb, but it doesn't work in practice. See the
>> commits in this series for details
>>
>> Therefore remove the remaining support for kdb from the DRM drivers
>> and from DRM fbdev emulation. Also remove the hooks from fbdev, as
>> there are no fbdev drivers with kdb support.
>>
>> If we ever want to address kdb support within DRM drivers, a place to
>> start would be the scanout buffers used by DRM's panic screen. These
>> use the current display mode. They can be written and flushed without
>> mode setting involved.
>>
>> Note: kdb over serial lines is not affected by this series and continues
>> to work as before.
>>
>> Thomas Zimmermann (5):
>>    drm/amdgpu: Do not implement mode_set_base_atomic callback
>>    drm/nouveau: Do not implement mode_set_base_atomic callback
>>    drm/radeon: Do not implement mode_set_base_atomic callback
>>    drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
>>    fbcon: Remove fb_debug_enter/_leave from struct fb_ops
> Personally, I've never worked with kdb over anything other than
> serial, so this won't bother any of my normal workflows. That being
> said, at least as of a year ago someone on the lists was talking about
> using kdb with a keyboard and (presumably) a display. You can see a
> thread here:
>
> http://lore.kernel.org/r/20241031192350.GA26688@lichtman.org

I wonder which driver or kernel that person was using. None of the 
current drivers would be working.

Best regards
Thomas

>
> Daniel may also have comments here?
>
> -Doug
>

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

* [PATCH v3 2/2] mfd: cgbc: Add support for backlight
From: Petri Karhula via B4 Relay @ 2025-11-25 15:29 UTC (permalink / raw)
  To: Thomas Richard, Lee Jones, Daniel Thompson, Jingoo Han,
	Helge Deller
  Cc: linux-kernel, dri-devel, linux-fbdev, Petri Karhula
In-Reply-To: <20251125-cgbc-backlight-v3-0-18ae42689411@novatron.fi>

From: Petri Karhula <petri.karhula@novatron.fi>

The Board Controller has control for display backlight.
Add backlight cell for the cgbc-backlight driver which
adds support for backlight brightness control.

Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
---
 drivers/mfd/cgbc-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
index 4782ff1114a9..10bb4b414c34 100644
--- a/drivers/mfd/cgbc-core.c
+++ b/drivers/mfd/cgbc-core.c
@@ -237,6 +237,7 @@ static struct mfd_cell cgbc_devs[] = {
 	{ .name = "cgbc-i2c", .id = 1 },
 	{ .name = "cgbc-i2c", .id = 2 },
 	{ .name = "cgbc-hwmon"	},
+	{ .name = "cgbc-backlight" },
 };
 
 static int cgbc_map(struct cgbc_device_data *cgbc)

-- 
2.34.1



^ permalink raw reply related

* [PATCH v3 1/2] backlight: Add Congatec Board Controller (CGBC) backlight support
From: Petri Karhula via B4 Relay @ 2025-11-25 15:29 UTC (permalink / raw)
  To: Thomas Richard, Lee Jones, Daniel Thompson, Jingoo Han,
	Helge Deller
  Cc: linux-kernel, dri-devel, linux-fbdev, Petri Karhula
In-Reply-To: <20251125-cgbc-backlight-v3-0-18ae42689411@novatron.fi>

From: Petri Karhula <petri.karhula@novatron.fi>

This driver provides backlight brightness control through the Linux
backlight subsystem. It communicates with the board controller to
adjust LCD backlight using PWM signals. Communication is done
through Congatec Board Controller core driver.

Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
---
 drivers/video/backlight/Kconfig   |  11 +++
 drivers/video/backlight/Makefile  |   1 +
 drivers/video/backlight/cgbc_bl.c | 177 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 189 insertions(+)

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index d9374d208cee..702f3b8ed036 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -249,6 +249,17 @@ config BACKLIGHT_PWM
 	  If you have a LCD backlight adjustable by PWM, say Y to enable
 	  this driver.
 
+config BACKLIGHT_CGBC
+	tristate "Congatec Board Controller (CGBC) backlight support"
+	depends on MFD_CGBC && X86
+	help
+	  Say Y here to enable support for LCD backlight control on Congatec
+	  x86-based boards via the CGBC (Congatec Board Controller).
+
+	  This driver provides backlight brightness control through the Linux
+	  backlight subsystem. It communicates with the board controller to
+	  adjust LCD backlight using PWM signals.
+
 config BACKLIGHT_DA903X
 	tristate "Backlight Driver for DA9030/DA9034 using WLED"
 	depends on PMIC_DA903X
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index dfbb169bf6ea..0169fd8873ed 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_BACKLIGHT_APPLE_DWI)	+= apple_dwi_bl.o
 obj-$(CONFIG_BACKLIGHT_AS3711)		+= as3711_bl.o
 obj-$(CONFIG_BACKLIGHT_BD6107)		+= bd6107.o
 obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE)	+= backlight.o
+obj-$(CONFIG_BACKLIGHT_CGBC)		+= cgbc_bl.o
 obj-$(CONFIG_BACKLIGHT_DA903X)		+= da903x_bl.o
 obj-$(CONFIG_BACKLIGHT_DA9052)		+= da9052_bl.o
 obj-$(CONFIG_BACKLIGHT_EP93XX)		+= ep93xx_bl.o
diff --git a/drivers/video/backlight/cgbc_bl.c b/drivers/video/backlight/cgbc_bl.c
new file mode 100644
index 000000000000..73ee6d440bcd
--- /dev/null
+++ b/drivers/video/backlight/cgbc_bl.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Congatec Board Controller (CGBC) Backlight Driver
+ *
+ * This driver provides backlight control for LCD displays connected to
+ * Congatec boards via the CGBC (Congatec Board Controller). It integrates
+ * with the Linux backlight subsystem and communicates with hardware through
+ * the cgbc-core module.
+ *
+ * Copyright (C) 2025 Novatron Oy
+ *
+ * Author: Petri Karhula <petri.karhula@novatron.fi>
+ */
+
+#include <linux/backlight.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/mfd/cgbc.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define BLT_PWM_DUTY_MASK          GENMASK(6, 0)
+
+/* CGBC command for PWM brightness control*/
+#define CGBC_CMD_BLT0_PWM          0x75
+
+#define CGBC_BL_MAX_BRIGHTNESS     100
+
+/**
+ * CGBC backlight driver data
+ * @dev: Pointer to the platform device
+ * @cgbc: Pointer to the parent CGBC device data
+ * @current_brightness: Current brightness level (0-100)
+ */
+struct cgbc_bl_data {
+	struct device *dev;
+	struct cgbc_device_data *cgbc;
+	unsigned int current_brightness;
+};
+
+static int cgbc_bl_update_status(struct backlight_device *bl)
+{
+	struct cgbc_bl_data *bl_data = bl_get_data(bl);
+	u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM };
+	u8 reply_buf[3];
+	u8 brightness;
+	int ret;
+
+	brightness = backlight_get_brightness(bl);
+
+	if (brightness != bl_data->current_brightness) {
+		/* Read the current values */
+		ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+				   sizeof(reply_buf), NULL);
+		if (ret < 0) {
+			dev_err(bl_data->dev, "Failed to read PWM settings: %d\n", ret);
+			return ret;
+		}
+
+		/*
+		 * Prepare command buffer for writing new settings. Only 2nd byte is changed
+		 * to set new brightness (PWM duty cycle %). Other values (polarity, frequency)
+		 * are preserved from the read values.
+		 */
+		cmd_buf[1] = (reply_buf[0] & ~BLT_PWM_DUTY_MASK) |
+			FIELD_PREP(BLT_PWM_DUTY_MASK, brightness);
+		cmd_buf[2] = reply_buf[1];
+		cmd_buf[3] = reply_buf[2];
+
+		ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+				   sizeof(reply_buf), NULL);
+		if (ret < 0) {
+			dev_err(bl_data->dev, "Failed to set brightness: %d\n", ret);
+			return ret;
+		}
+
+		bl_data->current_brightness = reply_buf[0] & BLT_PWM_DUTY_MASK;
+
+		/* Verify the setting was applied correctly */
+		if (bl_data->current_brightness != brightness) {
+			dev_err(bl_data->dev,
+				"Brightness setting verification failed\n");
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
+static int cgbc_bl_get_brightness(struct backlight_device *bl)
+{
+	struct cgbc_bl_data *bl_data = bl_get_data(bl);
+	u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM };
+	u8 reply_buf[3];
+	int ret;
+
+	ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+			   sizeof(reply_buf), NULL);
+
+	if (ret < 0) {
+		dev_err(bl_data->dev, "Failed to read brightness: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Only return PWM duty factor percentage,
+	 * ignore polarity inversion bit (bit 7)
+	 */
+	bl_data->current_brightness = FIELD_GET(BLT_PWM_DUTY_MASK, reply_buf[0]);
+
+	return bl_data->current_brightness;
+}
+
+static const struct backlight_ops cgbc_bl_ops = {
+	.options = BL_CORE_SUSPENDRESUME,
+	.update_status = cgbc_bl_update_status,
+	.get_brightness = cgbc_bl_get_brightness,
+};
+
+static int cgbc_bl_probe(struct platform_device *pdev)
+{
+	struct cgbc_device_data *cgbc = dev_get_drvdata(pdev->dev.parent);
+	struct backlight_properties props = { };
+	u8 cmd_buf[4] = { CGBC_CMD_BLT0_PWM };
+	struct backlight_device *bl_dev;
+	struct cgbc_bl_data *bl_data;
+	u8 reply_buf[3];
+	int ret;
+
+	bl_data = devm_kzalloc(&pdev->dev, sizeof(*bl_data), GFP_KERNEL);
+	if (!bl_data)
+		return -ENOMEM;
+
+	bl_data->dev = &pdev->dev;
+	bl_data->cgbc = cgbc;
+
+	ret = cgbc_command(bl_data->cgbc, cmd_buf, sizeof(cmd_buf), reply_buf,
+			   sizeof(reply_buf), NULL);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to read initial brightness\n");
+
+	/*
+	 * Get only PWM duty factor percentage,
+	 * ignore polarity inversion bit (bit 7)
+	 */
+	bl_data->current_brightness = FIELD_GET(BLT_PWM_DUTY_MASK, reply_buf[0]);
+
+	props.type = BACKLIGHT_PLATFORM;
+	props.max_brightness = CGBC_BL_MAX_BRIGHTNESS;
+	props.brightness = bl_data->current_brightness;
+
+	bl_dev = devm_backlight_device_register(&pdev->dev, "cgbc-backlight",
+						&pdev->dev, bl_data,
+						&cgbc_bl_ops, &props);
+	if (IS_ERR(bl_dev))
+		return dev_err_probe(&pdev->dev, PTR_ERR(bl_dev),
+			     "Failed to register backlight device\n");
+
+	platform_set_drvdata(pdev, bl_data);
+
+	return 0;
+}
+
+static struct platform_driver cgbc_bl_driver = {
+	.driver = {
+		.name = "cgbc-backlight",
+	},
+	.probe = cgbc_bl_probe,
+};
+
+module_platform_driver(cgbc_bl_driver);
+
+MODULE_AUTHOR("Petri Karhula <petri.karhula@novatron.fi>");
+MODULE_DESCRIPTION("Congatec Board Controller (CGBC) Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:cgbc-backlight");

-- 
2.34.1



^ permalink raw reply related

* [PATCH v3 0/2] Backlight driver to control backlight behind Congatec Board Controller.
From: Petri Karhula via B4 Relay @ 2025-11-25 15:29 UTC (permalink / raw)
  To: Thomas Richard, Lee Jones, Daniel Thompson, Jingoo Han,
	Helge Deller
  Cc: linux-kernel, dri-devel, linux-fbdev, Petri Karhula

This driver provides backlight brightness control through the Linux
backlight subsystem. It communicates with the board controller to
adjust LCD backlight using PWM signals. Communication is done
through Congatec Board Controller core driver.
    

Signed-off-by: Petri Karhula <petri.karhula@novatron.fi>
---
Changes in v3:
- Fixed review comments and simplified the structure of the driver
- Link to v2: https://lore.kernel.org/r/20251119-cgbc-backlight-v2-0-4d4edd7ca662@novatron.fi

Changes in v2:
- Separated Board Controller core driver change into its own patch
- Link to v1: https://lore.kernel.org/r/20251118-cgbc-backlight-v1-1-cc6ac5301034@novatron.fi

---
Petri Karhula (2):
      backlight: Add Congatec Board Controller (CGBC) backlight support
      mfd: cgbc: Add support for backlight

 drivers/mfd/cgbc-core.c           |   1 +
 drivers/video/backlight/Kconfig   |  11 +++
 drivers/video/backlight/Makefile  |   1 +
 drivers/video/backlight/cgbc_bl.c | 177 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 190 insertions(+)
---
base-commit: ac3fd01e4c1efce8f2c054cdeb2ddd2fc0fb150d
change-id: 20251118-cgbc-backlight-35c1109db0b8

Best regards,
-- 
Petri Karhula <petri.karhula@novatron.fi>



^ permalink raw reply

* Re: [PATCH 0/5] drm: Remove remaining support for kdb
From: Doug Anderson @ 2025-11-25 15:26 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Nir Lichtman
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>

Hi,

On Tue, Nov 25, 2025 at 5:06 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Remove the rest of the kbd support from DRM. Driver support has been
> broken for years without anyone complaining.
>
> Kdb cannot use regular DRM mode setting, so DRM drivers have to
> implement an additional hook to make it work (in theory). As outlined
> by Sima in commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for
> atomic drivers") from 2017, kdb is not compatible with DRM atomic mode
> setting. Non-atomic mode setting meanwhile has become rare.
>
> Only 3 DRM drivers implement the hooks for kdb support. Amdgpu and
> nouveau use non-atomic mode setting on older devices. But both drivers
> have switched to generic fbdev emulation, which isn't compatible with
> kdb. Radeon still runs kdb, but it doesn't work in practice. See the
> commits in this series for details
>
> Therefore remove the remaining support for kdb from the DRM drivers
> and from DRM fbdev emulation. Also remove the hooks from fbdev, as
> there are no fbdev drivers with kdb support.
>
> If we ever want to address kdb support within DRM drivers, a place to
> start would be the scanout buffers used by DRM's panic screen. These
> use the current display mode. They can be written and flushed without
> mode setting involved.
>
> Note: kdb over serial lines is not affected by this series and continues
> to work as before.
>
> Thomas Zimmermann (5):
>   drm/amdgpu: Do not implement mode_set_base_atomic callback
>   drm/nouveau: Do not implement mode_set_base_atomic callback
>   drm/radeon: Do not implement mode_set_base_atomic callback
>   drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
>   fbcon: Remove fb_debug_enter/_leave from struct fb_ops

Personally, I've never worked with kdb over anything other than
serial, so this won't bother any of my normal workflows. That being
said, at least as of a year ago someone on the lists was talking about
using kdb with a keyboard and (presumably) a display. You can see a
thread here:

http://lore.kernel.org/r/20241031192350.GA26688@lichtman.org

Daniel may also have comments here?

-Doug

^ permalink raw reply

* Re: [PATCH 0/5] drm: Remove remaining support for kdb
From: Thomas Zimmermann @ 2025-11-25 14:24 UTC (permalink / raw)
  To: Christian König, simona, airlied, alexander.deucher, lyude,
	dakr, deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Timur Kristóf
In-Reply-To: <11ac809d-0432-4c9f-8279-7df318c9a448@amd.com>

Hi

Am 25.11.25 um 14:16 schrieb Christian König:
> On 11/25/25 13:52, Thomas Zimmermann wrote:
>> Remove the rest of the kbd support from DRM. Driver support has been
>> broken for years without anyone complaining.
>>
>> Kdb cannot use regular DRM mode setting, so DRM drivers have to
>> implement an additional hook to make it work (in theory). As outlined
>> by Sima in commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for
>> atomic drivers") from 2017, kdb is not compatible with DRM atomic mode
>> setting. Non-atomic mode setting meanwhile has become rare.
>>
>> Only 3 DRM drivers implement the hooks for kdb support. Amdgpu and
>> nouveau use non-atomic mode setting on older devices. But both drivers
>> have switched to generic fbdev emulation, which isn't compatible with
>> kdb. Radeon still runs kdb, but it doesn't work in practice. See the
>> commits in this series for details
> Amdgpu is heavily pushing on switching to atomic for older GPUs as well. Timur and our DC guys have made quite some progress on that recently.

 From what I've seen, I really like the work they are doing there.

>
> Feel free to add Acked-by: Christian König <christian.koenig@amd.com> to the amdgpu and radeon changes.

Thanks.

Best regards
Thomas

>
> Regards,
> Christian.
>
>> Therefore remove the remaining support for kdb from the DRM drivers
>> and from DRM fbdev emulation. Also remove the hooks from fbdev, as
>> there are no fbdev drivers with kdb support.
>>
>> If we ever want to address kdb support within DRM drivers, a place to
>> start would be the scanout buffers used by DRM's panic screen. These
>> use the current display mode. They can be written and flushed without
>> mode setting involved.
>>
>> Note: kdb over serial lines is not affected by this series and continues
>> to work as before.
>>
>> Thomas Zimmermann (5):
>>    drm/amdgpu: Do not implement mode_set_base_atomic callback
>>    drm/nouveau: Do not implement mode_set_base_atomic callback
>>    drm/radeon: Do not implement mode_set_base_atomic callback
>>    drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
>>    fbcon: Remove fb_debug_enter/_leave from struct fb_ops
>>
>>   Documentation/process/debugging/kgdb.rst    |  28 -----
>>   drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  35 ++-----
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  35 ++-----
>>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  35 ++-----
>>   drivers/gpu/drm/drm_fb_helper.c             | 108 --------------------
>>   drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  24 +----
>>   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 +-
>>   drivers/video/fbdev/core/fbcon.c            |  24 -----
>>   drivers/video/fbdev/core/fbcon.h            |   1 -
>>   include/drm/drm_fb_helper.h                 |  21 ----
>>   include/drm/drm_modeset_helper_vtables.h    |  23 -----
>>   include/linux/fb.h                          |   4 -
>>   14 files changed, 63 insertions(+), 382 deletions(-)
>>
>>
>> base-commit: 0a21e96e0b6840d2a4e0b45a957679eeddeb4362

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

* Re: [PATCH 0/5] drm: Remove remaining support for kdb
From: Christian König @ 2025-11-25 13:16 UTC (permalink / raw)
  To: Thomas Zimmermann, simona, airlied, alexander.deucher, lyude,
	dakr, deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Timur Kristóf
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>

On 11/25/25 13:52, Thomas Zimmermann wrote:
> Remove the rest of the kbd support from DRM. Driver support has been
> broken for years without anyone complaining.
> 
> Kdb cannot use regular DRM mode setting, so DRM drivers have to
> implement an additional hook to make it work (in theory). As outlined
> by Sima in commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for
> atomic drivers") from 2017, kdb is not compatible with DRM atomic mode
> setting. Non-atomic mode setting meanwhile has become rare.
> 
> Only 3 DRM drivers implement the hooks for kdb support. Amdgpu and
> nouveau use non-atomic mode setting on older devices. But both drivers
> have switched to generic fbdev emulation, which isn't compatible with
> kdb. Radeon still runs kdb, but it doesn't work in practice. See the
> commits in this series for details

Amdgpu is heavily pushing on switching to atomic for older GPUs as well. Timur and our DC guys have made quite some progress on that recently.

Feel free to add Acked-by: Christian König <christian.koenig@amd.com> to the amdgpu and radeon changes.

Regards,
Christian.

> 
> Therefore remove the remaining support for kdb from the DRM drivers
> and from DRM fbdev emulation. Also remove the hooks from fbdev, as
> there are no fbdev drivers with kdb support.
> 
> If we ever want to address kdb support within DRM drivers, a place to
> start would be the scanout buffers used by DRM's panic screen. These
> use the current display mode. They can be written and flushed without
> mode setting involved.
> 
> Note: kdb over serial lines is not affected by this series and continues
> to work as before.
> 
> Thomas Zimmermann (5):
>   drm/amdgpu: Do not implement mode_set_base_atomic callback
>   drm/nouveau: Do not implement mode_set_base_atomic callback
>   drm/radeon: Do not implement mode_set_base_atomic callback
>   drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
>   fbcon: Remove fb_debug_enter/_leave from struct fb_ops
> 
>  Documentation/process/debugging/kgdb.rst    |  28 -----
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  35 ++-----
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  35 ++-----
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  35 ++-----
>  drivers/gpu/drm/drm_fb_helper.c             | 108 --------------------
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  24 +----
>  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 +-
>  drivers/video/fbdev/core/fbcon.c            |  24 -----
>  drivers/video/fbdev/core/fbcon.h            |   1 -
>  include/drm/drm_fb_helper.h                 |  21 ----
>  include/drm/drm_modeset_helper_vtables.h    |  23 -----
>  include/linux/fb.h                          |   4 -
>  14 files changed, 63 insertions(+), 382 deletions(-)
> 
> 
> base-commit: 0a21e96e0b6840d2a4e0b45a957679eeddeb4362


^ permalink raw reply

* Re: [PATCH 0/5] drm: Remove remaining support for kdb
From: Thomas Zimmermann @ 2025-11-25 13:10 UTC (permalink / raw)
  To: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>



Am 25.11.25 um 13:52 schrieb Thomas Zimmermann:
> Remove the rest of the kbd support from DRM. Driver support has been

s/kbd/kdb/g  as in 'kernel debugger'

> broken for years without anyone complaining.
>
> Kdb cannot use regular DRM mode setting, so DRM drivers have to
> implement an additional hook to make it work (in theory). As outlined
> by Sima in commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for
> atomic drivers") from 2017, kdb is not compatible with DRM atomic mode
> setting. Non-atomic mode setting meanwhile has become rare.
>
> Only 3 DRM drivers implement the hooks for kdb support. Amdgpu and
> nouveau use non-atomic mode setting on older devices. But both drivers
> have switched to generic fbdev emulation, which isn't compatible with
> kdb. Radeon still runs kdb, but it doesn't work in practice. See the
> commits in this series for details
>
> Therefore remove the remaining support for kdb from the DRM drivers
> and from DRM fbdev emulation. Also remove the hooks from fbdev, as
> there are no fbdev drivers with kdb support.
>
> If we ever want to address kdb support within DRM drivers, a place to
> start would be the scanout buffers used by DRM's panic screen. These
> use the current display mode. They can be written and flushed without
> mode setting involved.
>
> Note: kdb over serial lines is not affected by this series and continues
> to work as before.
>
> Thomas Zimmermann (5):
>    drm/amdgpu: Do not implement mode_set_base_atomic callback
>    drm/nouveau: Do not implement mode_set_base_atomic callback
>    drm/radeon: Do not implement mode_set_base_atomic callback
>    drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
>    fbcon: Remove fb_debug_enter/_leave from struct fb_ops
>
>   Documentation/process/debugging/kgdb.rst    |  28 -----
>   drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  35 ++-----
>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  35 ++-----
>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  35 ++-----
>   drivers/gpu/drm/drm_fb_helper.c             | 108 --------------------
>   drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  24 +----
>   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 +-
>   drivers/video/fbdev/core/fbcon.c            |  24 -----
>   drivers/video/fbdev/core/fbcon.h            |   1 -
>   include/drm/drm_fb_helper.h                 |  21 ----
>   include/drm/drm_modeset_helper_vtables.h    |  23 -----
>   include/linux/fb.h                          |   4 -
>   14 files changed, 63 insertions(+), 382 deletions(-)
>
>
> base-commit: 0a21e96e0b6840d2a4e0b45a957679eeddeb4362

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

* [PATCH 5/5] fbcon: Remove fb_debug_enter/_leave from struct fb_ops
From: Thomas Zimmermann @ 2025-11-25 12:52 UTC (permalink / raw)
  To: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Thomas Zimmermann
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>

There are no implementations of fb_debug_enter and fb_debug_leave.
Remove the callbacks from struct fb_ops and clean up the caller.

The field save_graphics in fbcon_par is also no longer required.
Remove it as well.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 Documentation/process/debugging/kgdb.rst | 28 ------------------------
 drivers/video/fbdev/core/fbcon.c         | 24 --------------------
 drivers/video/fbdev/core/fbcon.h         |  1 -
 include/linux/fb.h                       |  4 ----
 4 files changed, 57 deletions(-)

diff --git a/Documentation/process/debugging/kgdb.rst b/Documentation/process/debugging/kgdb.rst
index b29b0aac2717..773b19aa1382 100644
--- a/Documentation/process/debugging/kgdb.rst
+++ b/Documentation/process/debugging/kgdb.rst
@@ -889,34 +889,6 @@ in the virtual console layer. On resuming kernel execution, the kernel
 debugger calls kgdboc_post_exp_handler() which in turn calls
 con_debug_leave().
 
-Any video driver that wants to be compatible with the kernel debugger
-and the atomic kms callbacks must implement the ``mode_set_base_atomic``,
-``fb_debug_enter`` and ``fb_debug_leave operations``. For the
-``fb_debug_enter`` and ``fb_debug_leave`` the option exists to use the
-generic drm fb helper functions or implement something custom for the
-hardware. The following example shows the initialization of the
-.mode_set_base_atomic operation in
-drivers/gpu/drm/i915/intel_display.c::
-
-
-    static const struct drm_crtc_helper_funcs intel_helper_funcs = {
-    [...]
-            .mode_set_base_atomic = intel_pipe_set_base_atomic,
-    [...]
-    };
-
-
-Here is an example of how the i915 driver initializes the
-fb_debug_enter and fb_debug_leave functions to use the generic drm
-helpers in ``drivers/gpu/drm/i915/intel_fb.c``::
-
-
-    static struct fb_ops intelfb_ops = {
-    [...]
-           .fb_debug_enter = drm_fb_helper_debug_enter,
-           .fb_debug_leave = drm_fb_helper_debug_leave,
-    [...]
-    };
 
 
 Credits
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 7be9e865325d..34ea14412ace 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2289,28 +2289,6 @@ static bool fbcon_blank(struct vc_data *vc, enum vesa_blank_mode blank,
 	return false;
 }
 
-static void fbcon_debug_enter(struct vc_data *vc)
-{
-	struct fb_info *info = fbcon_info_from_console(vc->vc_num);
-	struct fbcon_par *par = info->fbcon_par;
-
-	par->save_graphics = par->graphics;
-	par->graphics = 0;
-	if (info->fbops->fb_debug_enter)
-		info->fbops->fb_debug_enter(info);
-	fbcon_set_palette(vc, color_table);
-}
-
-static void fbcon_debug_leave(struct vc_data *vc)
-{
-	struct fb_info *info = fbcon_info_from_console(vc->vc_num);
-	struct fbcon_par *par = info->fbcon_par;
-
-	par->graphics = par->save_graphics;
-	if (info->fbops->fb_debug_leave)
-		info->fbops->fb_debug_leave(info);
-}
-
 static int fbcon_get_font(struct vc_data *vc, struct console_font *font, unsigned int vpitch)
 {
 	u8 *fontdata = vc->vc_font.data;
@@ -3214,8 +3192,6 @@ static const struct consw fb_con = {
 	.con_set_palette 	= fbcon_set_palette,
 	.con_invert_region 	= fbcon_invert_region,
 	.con_resize             = fbcon_resize,
-	.con_debug_enter	= fbcon_debug_enter,
-	.con_debug_leave	= fbcon_debug_leave,
 };
 
 static ssize_t rotate_store(struct device *device,
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 44ea4ae4bba0..1cd10a7faab0 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -79,7 +79,6 @@ struct fbcon_par {
 	int    cursor_reset;
 	int    blank_state;
 	int    graphics;
-	int    save_graphics; /* for debug enter/leave */
 	bool   initialized;
 	int    rotate;
 	int    cur_rotate;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 05cc251035da..65fb70382675 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -304,10 +304,6 @@ struct fb_ops {
 
 	/* teardown any resources to do with this framebuffer */
 	void (*fb_destroy)(struct fb_info *info);
-
-	/* called at KDB enter and leave time to prepare the console */
-	int (*fb_debug_enter)(struct fb_info *info);
-	int (*fb_debug_leave)(struct fb_info *info);
 };
 
 #ifdef CONFIG_FB_TILEBLITTING
-- 
2.51.1


^ permalink raw reply related

* [PATCH 4/5] drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
From: Thomas Zimmermann @ 2025-11-25 12:52 UTC (permalink / raw)
  To: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Thomas Zimmermann
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>

Remove the debug_enter/debug_leave helpers, as there are no DRM
drivers supporting debugging with kgdb. Remove code to keep track
of existing fbdev-emulation state. None of this required any longer.

Also remove mode_set_base_atomic from struct drm_crtc_helper_funcs,
which has no callers or implementations.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fb_helper.c          | 108 -----------------------
 include/drm/drm_fb_helper.h              |  21 -----
 include/drm/drm_modeset_helper_vtables.h |  23 -----
 3 files changed, 152 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4a7f72044ab8..29c3c7bb7d67 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -75,9 +75,6 @@ MODULE_PARM_DESC(drm_leak_fbdev_smem,
 		 "Allow unsafe leaking fbdev physical smem address [default=false]");
 #endif
 
-static LIST_HEAD(kernel_fb_helper_list);
-static DEFINE_MUTEX(kernel_fb_helper_lock);
-
 /**
  * DOC: fbdev helpers
  *
@@ -115,101 +112,6 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
  * mmap page writes.
  */
 
-static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
-{
-	uint16_t *r_base, *g_base, *b_base;
-
-	if (crtc->funcs->gamma_set == NULL)
-		return;
-
-	r_base = crtc->gamma_store;
-	g_base = r_base + crtc->gamma_size;
-	b_base = g_base + crtc->gamma_size;
-
-	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base,
-			       crtc->gamma_size, NULL);
-}
-
-/**
- * drm_fb_helper_debug_enter - implementation for &fb_ops.fb_debug_enter
- * @info: fbdev registered by the helper
- */
-int drm_fb_helper_debug_enter(struct fb_info *info)
-{
-	struct drm_fb_helper *helper = info->par;
-	const struct drm_crtc_helper_funcs *funcs;
-	struct drm_mode_set *mode_set;
-
-	list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
-		mutex_lock(&helper->client.modeset_mutex);
-		drm_client_for_each_modeset(mode_set, &helper->client) {
-			if (!mode_set->crtc->enabled)
-				continue;
-
-			funcs =	mode_set->crtc->helper_private;
-			if (funcs->mode_set_base_atomic == NULL)
-				continue;
-
-			if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
-				continue;
-
-			funcs->mode_set_base_atomic(mode_set->crtc,
-						    mode_set->fb,
-						    mode_set->x,
-						    mode_set->y,
-						    ENTER_ATOMIC_MODE_SET);
-		}
-		mutex_unlock(&helper->client.modeset_mutex);
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(drm_fb_helper_debug_enter);
-
-/**
- * drm_fb_helper_debug_leave - implementation for &fb_ops.fb_debug_leave
- * @info: fbdev registered by the helper
- */
-int drm_fb_helper_debug_leave(struct fb_info *info)
-{
-	struct drm_fb_helper *helper = info->par;
-	struct drm_client_dev *client = &helper->client;
-	struct drm_device *dev = helper->dev;
-	struct drm_crtc *crtc;
-	const struct drm_crtc_helper_funcs *funcs;
-	struct drm_mode_set *mode_set;
-	struct drm_framebuffer *fb;
-
-	mutex_lock(&client->modeset_mutex);
-	drm_client_for_each_modeset(mode_set, client) {
-		crtc = mode_set->crtc;
-		if (drm_drv_uses_atomic_modeset(crtc->dev))
-			continue;
-
-		funcs = crtc->helper_private;
-		fb = crtc->primary->fb;
-
-		if (!crtc->enabled)
-			continue;
-
-		if (!fb) {
-			drm_err(dev, "no fb to restore?\n");
-			continue;
-		}
-
-		if (funcs->mode_set_base_atomic == NULL)
-			continue;
-
-		drm_fb_helper_restore_lut_atomic(mode_set->crtc);
-		funcs->mode_set_base_atomic(mode_set->crtc, fb, crtc->x,
-					    crtc->y, LEAVE_ATOMIC_MODE_SET);
-	}
-	mutex_unlock(&client->modeset_mutex);
-
-	return 0;
-}
-EXPORT_SYMBOL(drm_fb_helper_debug_leave);
-
 static int
 __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
 					    bool force)
@@ -397,7 +299,6 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 	if (!preferred_bpp)
 		preferred_bpp = 32;
 
-	INIT_LIST_HEAD(&helper->kernel_fb_list);
 	spin_lock_init(&helper->damage_lock);
 	INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker);
 	INIT_WORK(&helper->damage_work, drm_fb_helper_damage_work);
@@ -534,11 +435,6 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 
 	drm_fb_helper_release_info(fb_helper);
 
-	mutex_lock(&kernel_fb_helper_lock);
-	if (!list_empty(&fb_helper->kernel_fb_list))
-		list_del(&fb_helper->kernel_fb_list);
-	mutex_unlock(&kernel_fb_helper_lock);
-
 	if (!fb_helper->client.funcs)
 		drm_client_release(&fb_helper->client);
 }
@@ -1766,10 +1662,6 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper)
 	drm_info(dev, "fb%d: %s frame buffer device\n",
 		 info->node, info->fix.id);
 
-	mutex_lock(&kernel_fb_helper_lock);
-	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
-	mutex_unlock(&kernel_fb_helper_lock);
-
 	return 0;
 
 err_drm_fb_helper_release_info:
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index dd9a18f8de5a..05cca77b7249 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -166,13 +166,6 @@ struct drm_fb_helper {
 	 */
 	struct mutex lock;
 
-	/**
-	 * @kernel_fb_list:
-	 *
-	 * Entry on the global kernel_fb_helper_list, used for kgdb entry/exit.
-	 */
-	struct list_head kernel_fb_list;
-
 	/**
 	 * @delayed_hotplug:
 	 *
@@ -236,8 +229,6 @@ drm_fb_helper_from_client(struct drm_client_dev *client)
 	.fb_setcmap	= drm_fb_helper_setcmap, \
 	.fb_blank	= drm_fb_helper_blank, \
 	.fb_pan_display	= drm_fb_helper_pan_display, \
-	.fb_debug_enter = drm_fb_helper_debug_enter, \
-	.fb_debug_leave = drm_fb_helper_debug_leave, \
 	.fb_ioctl	= drm_fb_helper_ioctl
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
@@ -280,8 +271,6 @@ 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);
-int drm_fb_helper_debug_enter(struct fb_info *info);
-int drm_fb_helper_debug_leave(struct fb_info *info);
 #else
 static inline void drm_fb_helper_prepare(struct drm_device *dev,
 					 struct drm_fb_helper *helper,
@@ -387,16 +376,6 @@ static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper)
 {
 	return 0;
 }
-
-static inline int drm_fb_helper_debug_enter(struct fb_info *info)
-{
-	return 0;
-}
-
-static inline int drm_fb_helper_debug_leave(struct fb_info *info)
-{
-	return 0;
-}
 #endif
 
 #endif
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index fe32854b7ffe..3e68213958dd 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -52,11 +52,6 @@ struct drm_scanout_buffer;
 struct drm_writeback_connector;
 struct drm_writeback_job;
 
-enum mode_set_atomic {
-	LEAVE_ATOMIC_MODE_SET,
-	ENTER_ATOMIC_MODE_SET,
-};
-
 /**
  * struct drm_crtc_helper_funcs - helper operations for CRTCs
  *
@@ -253,24 +248,6 @@ struct drm_crtc_helper_funcs {
 	int (*mode_set_base)(struct drm_crtc *crtc, int x, int y,
 			     struct drm_framebuffer *old_fb);
 
-	/**
-	 * @mode_set_base_atomic:
-	 *
-	 * This callback is used by the fbdev helpers to set a new framebuffer
-	 * and scanout without sleeping, i.e. from an atomic calling context. It
-	 * is only used to implement kgdb support.
-	 *
-	 * This callback is optional and only needed for kgdb support in the fbdev
-	 * helpers.
-	 *
-	 * RETURNS:
-	 *
-	 * 0 on success or a negative error code on failure.
-	 */
-	int (*mode_set_base_atomic)(struct drm_crtc *crtc,
-				    struct drm_framebuffer *fb, int x, int y,
-				    enum mode_set_atomic);
-
 	/**
 	 * @disable:
 	 *
-- 
2.51.1


^ permalink raw reply related

* [PATCH 3/5] drm/radeon: Do not implement mode_set_base_atomic callback
From: Thomas Zimmermann @ 2025-11-25 12:52 UTC (permalink / raw)
  To: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Thomas Zimmermann
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


^ permalink raw reply related

* [PATCH 2/5] drm/nouveau: Do not implement mode_set_base_atomic callback
From: Thomas Zimmermann @ 2025-11-25 12:52 UTC (permalink / raw)
  To: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Thomas Zimmermann
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>

Remove the implementation of the CRTC helper mode_set_base_atomic
from nouveau. 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 nouveau provides non-atomic mode setting for some devices, kdb
assumes that the GEM buffer object is at a fixed location in video
memory. This has not been the case since
commit 4a16dd9d18a0 ("drm/nouveau/kms: switch to drm fbdev helpers")
from 2022. Fbdev-ttm helpers use a shadow buffer with a movable GEM
buffer object. Triggering kdb does therefore not update the display.

Hence remove the whole kdb support from nouveau.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index c063756eaea3..80493224eb6c 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -837,7 +837,7 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 static int
 nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
 			   struct drm_framebuffer *passed_fb,
-			   int x, int y, bool atomic)
+			   int x, int y)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
@@ -850,19 +850,12 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
 	NV_DEBUG(drm, "index %d\n", nv_crtc->index);
 
 	/* no fb bound */
-	if (!atomic && !crtc->primary->fb) {
+	if (!crtc->primary->fb) {
 		NV_DEBUG(drm, "No FB bound\n");
 		return 0;
 	}
 
-	/* If atomic, we want to switch to the fb we were passed, so
-	 * now we update pointers to do that.
-	 */
-	if (atomic) {
-		drm_fb = passed_fb;
-	} else {
-		drm_fb = crtc->primary->fb;
-	}
+	drm_fb = crtc->primary->fb;
 
 	nvbo = nouveau_gem_object(drm_fb->obj[0]);
 	nv_crtc->fb.offset = nvbo->offset;
@@ -920,15 +913,7 @@ nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	int ret = nv_crtc_swap_fbs(crtc, old_fb);
 	if (ret)
 		return ret;
-	return nv04_crtc_do_mode_set_base(crtc, old_fb, x, y, false);
-}
-
-static int
-nv04_crtc_mode_set_base_atomic(struct drm_crtc *crtc,
-			       struct drm_framebuffer *fb,
-			       int x, int y, enum mode_set_atomic state)
-{
-	return nv04_crtc_do_mode_set_base(crtc, fb, x, y, true);
+	return nv04_crtc_do_mode_set_base(crtc, old_fb, x, y);
 }
 
 static void nv04_cursor_upload(struct drm_device *dev, struct nouveau_bo *src,
@@ -1274,7 +1259,6 @@ static const struct drm_crtc_helper_funcs nv04_crtc_helper_funcs = {
 	.commit = nv_crtc_commit,
 	.mode_set = nv_crtc_mode_set,
 	.mode_set_base = nv04_crtc_mode_set_base,
-	.mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
 	.disable = nv_crtc_disable,
 	.get_scanout_position = nouveau_display_scanoutpos,
 };
-- 
2.51.1


^ permalink raw reply related

* [PATCH 1/5] drm/amdgpu: Do not implement mode_set_base_atomic callback
From: Thomas Zimmermann @ 2025-11-25 12:52 UTC (permalink / raw)
  To: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Thomas Zimmermann
In-Reply-To: <20251125130634.1080966-1-tzimmermann@suse.de>

Remove all implementations of the CRTC helper mode_set_base_atomic
from amdgpu. 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 amdgpu provides non-atomic mode setting for some devices, kdb
assumes that the GEM buffer object is at a fixed location in video
memory. This has not been the case since commit 087451f372bf ("drm/amdgpu:
use generic fb helpers instead of setting up AMD own's.") from 2021.
Fbdev-ttm helpers use a shadow buffer with a movable GEM buffer object.
Triggering kdb does not update the display.

Hence remove the whole kdb support from amdgpu.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 35 ++++++++------------------
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c  | 35 ++++++++------------------
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c  | 35 ++++++++------------------
 3 files changed, 33 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 72ca6538b2e4..61302204e9b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1838,7 +1838,7 @@ static void dce_v10_0_grph_enable(struct drm_crtc *crtc, bool enable)
 
 static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
 				     struct drm_framebuffer *fb,
-				     int x, int y, int atomic)
+				     int x, int y)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
@@ -1855,15 +1855,12 @@ static int dce_v10_0_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
@@ -1874,13 +1871,11 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
 	if (unlikely(r != 0))
 		return r;
 
-	if (!atomic) {
-		abo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
-		r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM);
-		if (unlikely(r != 0)) {
-			amdgpu_bo_unreserve(abo);
-			return -EINVAL;
-		}
+	abo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+	r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM);
+	if (unlikely(r != 0)) {
+		amdgpu_bo_unreserve(abo);
+		return -EINVAL;
 	}
 	fb_location = amdgpu_bo_gpu_offset(abo);
 
@@ -2068,7 +2063,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
 	/* set pageflip to happen anywhere in vblank interval */
 	WREG32(mmMASTER_UPDATE_MODE + amdgpu_crtc->crtc_offset, 0);
 
-	if (!atomic && fb && fb != crtc->primary->fb) {
+	if (fb && fb != crtc->primary->fb) {
 		abo = gem_to_amdgpu_bo(fb->obj[0]);
 		r = amdgpu_bo_reserve(abo, true);
 		if (unlikely(r != 0))
@@ -2611,7 +2606,7 @@ static int dce_v10_0_crtc_mode_set(struct drm_crtc *crtc,
 
 	amdgpu_atombios_crtc_set_pll(crtc, adjusted_mode);
 	amdgpu_atombios_crtc_set_dtd_timing(crtc, adjusted_mode);
-	dce_v10_0_crtc_do_set_base(crtc, old_fb, x, y, 0);
+	dce_v10_0_crtc_do_set_base(crtc, old_fb, x, y);
 	amdgpu_atombios_crtc_overscan_setup(crtc, mode, adjusted_mode);
 	amdgpu_atombios_crtc_scaler_setup(crtc);
 	dce_v10_0_cursor_reset(crtc);
@@ -2659,14 +2654,7 @@ static bool dce_v10_0_crtc_mode_fixup(struct drm_crtc *crtc,
 static int dce_v10_0_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 				  struct drm_framebuffer *old_fb)
 {
-	return dce_v10_0_crtc_do_set_base(crtc, old_fb, x, y, 0);
-}
-
-static int dce_v10_0_crtc_set_base_atomic(struct drm_crtc *crtc,
-					 struct drm_framebuffer *fb,
-					 int x, int y, enum mode_set_atomic state)
-{
-	return dce_v10_0_crtc_do_set_base(crtc, fb, x, y, 1);
+	return dce_v10_0_crtc_do_set_base(crtc, old_fb, x, y);
 }
 
 static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = {
@@ -2674,7 +2662,6 @@ static const struct drm_crtc_helper_funcs dce_v10_0_crtc_helper_funcs = {
 	.mode_fixup = dce_v10_0_crtc_mode_fixup,
 	.mode_set = dce_v10_0_crtc_mode_set,
 	.mode_set_base = dce_v10_0_crtc_set_base,
-	.mode_set_base_atomic = dce_v10_0_crtc_set_base_atomic,
 	.prepare = dce_v10_0_crtc_prepare,
 	.commit = dce_v10_0_crtc_commit,
 	.disable = dce_v10_0_crtc_disable,
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index acc887a58518..8f4b4c2e36b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1876,7 +1876,7 @@ static void dce_v6_0_grph_enable(struct drm_crtc *crtc, bool enable)
 
 static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
 				     struct drm_framebuffer *fb,
-				     int x, int y, int atomic)
+				     int x, int y)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
@@ -1892,15 +1892,12 @@ static int dce_v6_0_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
@@ -1911,13 +1908,11 @@ static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
 	if (unlikely(r != 0))
 		return r;
 
-	if (!atomic) {
-		abo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
-		r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM);
-		if (unlikely(r != 0)) {
-			amdgpu_bo_unreserve(abo);
-			return -EINVAL;
-		}
+	abo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+	r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM);
+	if (unlikely(r != 0)) {
+		amdgpu_bo_unreserve(abo);
+		return -EINVAL;
 	}
 	fb_location = amdgpu_bo_gpu_offset(abo);
 
@@ -2083,7 +2078,7 @@ static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
 	/* set pageflip to happen anywhere in vblank interval */
 	WREG32(mmMASTER_UPDATE_MODE + amdgpu_crtc->crtc_offset, 0);
 
-	if (!atomic && fb && fb != crtc->primary->fb) {
+	if (fb && fb != crtc->primary->fb) {
 		abo = gem_to_amdgpu_bo(fb->obj[0]);
 		r = amdgpu_bo_reserve(abo, true);
 		if (unlikely(r != 0))
@@ -2578,7 +2573,7 @@ static int dce_v6_0_crtc_mode_set(struct drm_crtc *crtc,
 
 	amdgpu_atombios_crtc_set_pll(crtc, adjusted_mode);
 	amdgpu_atombios_crtc_set_dtd_timing(crtc, adjusted_mode);
-	dce_v6_0_crtc_do_set_base(crtc, old_fb, x, y, 0);
+	dce_v6_0_crtc_do_set_base(crtc, old_fb, x, y);
 	amdgpu_atombios_crtc_overscan_setup(crtc, mode, adjusted_mode);
 	amdgpu_atombios_crtc_scaler_setup(crtc);
 	dce_v6_0_cursor_reset(crtc);
@@ -2626,14 +2621,7 @@ static bool dce_v6_0_crtc_mode_fixup(struct drm_crtc *crtc,
 static int dce_v6_0_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 				  struct drm_framebuffer *old_fb)
 {
-	return dce_v6_0_crtc_do_set_base(crtc, old_fb, x, y, 0);
-}
-
-static int dce_v6_0_crtc_set_base_atomic(struct drm_crtc *crtc,
-					 struct drm_framebuffer *fb,
-					 int x, int y, enum mode_set_atomic state)
-{
-	return dce_v6_0_crtc_do_set_base(crtc, fb, x, y, 1);
+	return dce_v6_0_crtc_do_set_base(crtc, old_fb, x, y);
 }
 
 static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
@@ -2641,7 +2629,6 @@ static const struct drm_crtc_helper_funcs dce_v6_0_crtc_helper_funcs = {
 	.mode_fixup = dce_v6_0_crtc_mode_fixup,
 	.mode_set = dce_v6_0_crtc_mode_set,
 	.mode_set_base = dce_v6_0_crtc_set_base,
-	.mode_set_base_atomic = dce_v6_0_crtc_set_base_atomic,
 	.prepare = dce_v6_0_crtc_prepare,
 	.commit = dce_v6_0_crtc_commit,
 	.disable = dce_v6_0_crtc_disable,
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 2ccd6aad8dd6..9d1853c41fcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1785,7 +1785,7 @@ static void dce_v8_0_grph_enable(struct drm_crtc *crtc, bool enable)
 
 static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
 				     struct drm_framebuffer *fb,
-				     int x, int y, int atomic)
+				     int x, int y)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
@@ -1802,15 +1802,12 @@ static int dce_v8_0_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
@@ -1821,13 +1818,11 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
 	if (unlikely(r != 0))
 		return r;
 
-	if (!atomic) {
-		abo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
-		r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM);
-		if (unlikely(r != 0)) {
-			amdgpu_bo_unreserve(abo);
-			return -EINVAL;
-		}
+	abo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+	r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM);
+	if (unlikely(r != 0)) {
+		amdgpu_bo_unreserve(abo);
+		return -EINVAL;
 	}
 	fb_location = amdgpu_bo_gpu_offset(abo);
 
@@ -1995,7 +1990,7 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
 	/* set pageflip to happen anywhere in vblank interval */
 	WREG32(mmMASTER_UPDATE_MODE + amdgpu_crtc->crtc_offset, 0);
 
-	if (!atomic && fb && fb != crtc->primary->fb) {
+	if (fb && fb != crtc->primary->fb) {
 		abo = gem_to_amdgpu_bo(fb->obj[0]);
 		r = amdgpu_bo_reserve(abo, true);
 		if (unlikely(r != 0))
@@ -2537,7 +2532,7 @@ static int dce_v8_0_crtc_mode_set(struct drm_crtc *crtc,
 
 	amdgpu_atombios_crtc_set_pll(crtc, adjusted_mode);
 	amdgpu_atombios_crtc_set_dtd_timing(crtc, adjusted_mode);
-	dce_v8_0_crtc_do_set_base(crtc, old_fb, x, y, 0);
+	dce_v8_0_crtc_do_set_base(crtc, old_fb, x, y);
 	amdgpu_atombios_crtc_overscan_setup(crtc, mode, adjusted_mode);
 	amdgpu_atombios_crtc_scaler_setup(crtc);
 	dce_v8_0_cursor_reset(crtc);
@@ -2585,14 +2580,7 @@ static bool dce_v8_0_crtc_mode_fixup(struct drm_crtc *crtc,
 static int dce_v8_0_crtc_set_base(struct drm_crtc *crtc, int x, int y,
 				  struct drm_framebuffer *old_fb)
 {
-	return dce_v8_0_crtc_do_set_base(crtc, old_fb, x, y, 0);
-}
-
-static int dce_v8_0_crtc_set_base_atomic(struct drm_crtc *crtc,
-					 struct drm_framebuffer *fb,
-					 int x, int y, enum mode_set_atomic state)
-{
-	return dce_v8_0_crtc_do_set_base(crtc, fb, x, y, 1);
+	return dce_v8_0_crtc_do_set_base(crtc, old_fb, x, y);
 }
 
 static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = {
@@ -2600,7 +2588,6 @@ static const struct drm_crtc_helper_funcs dce_v8_0_crtc_helper_funcs = {
 	.mode_fixup = dce_v8_0_crtc_mode_fixup,
 	.mode_set = dce_v8_0_crtc_mode_set,
 	.mode_set_base = dce_v8_0_crtc_set_base,
-	.mode_set_base_atomic = dce_v8_0_crtc_set_base_atomic,
 	.prepare = dce_v8_0_crtc_prepare,
 	.commit = dce_v8_0_crtc_commit,
 	.disable = dce_v8_0_crtc_disable,
-- 
2.51.1


^ permalink raw reply related

* [PATCH 0/5] drm: Remove remaining support for kdb
From: Thomas Zimmermann @ 2025-11-25 12:52 UTC (permalink / raw)
  To: simona, airlied, alexander.deucher, christian.koenig, lyude, dakr,
	deller, mripard, maarten.lankhorst, jason.wessel, danielt,
	dianders
  Cc: dri-devel, amd-gfx, nouveau, linux-fbdev, linux-doc, linux-kernel,
	Thomas Zimmermann

Remove the rest of the kbd support from DRM. Driver support has been
broken for years without anyone complaining.

Kdb cannot use regular DRM mode setting, so DRM drivers have to
implement an additional hook to make it work (in theory). As outlined
by Sima in commit 9c79e0b1d096 ("drm/fb-helper: Give up on kgdb for
atomic drivers") from 2017, kdb is not compatible with DRM atomic mode
setting. Non-atomic mode setting meanwhile has become rare.

Only 3 DRM drivers implement the hooks for kdb support. Amdgpu and
nouveau use non-atomic mode setting on older devices. But both drivers
have switched to generic fbdev emulation, which isn't compatible with
kdb. Radeon still runs kdb, but it doesn't work in practice. See the
commits in this series for details

Therefore remove the remaining support for kdb from the DRM drivers
and from DRM fbdev emulation. Also remove the hooks from fbdev, as
there are no fbdev drivers with kdb support.

If we ever want to address kdb support within DRM drivers, a place to
start would be the scanout buffers used by DRM's panic screen. These
use the current display mode. They can be written and flushed without
mode setting involved.

Note: kdb over serial lines is not affected by this series and continues
to work as before.

Thomas Zimmermann (5):
  drm/amdgpu: Do not implement mode_set_base_atomic callback
  drm/nouveau: Do not implement mode_set_base_atomic callback
  drm/radeon: Do not implement mode_set_base_atomic callback
  drm/fbdev-helper: Remove drm_fb_helper_debug_enter/_leave()
  fbcon: Remove fb_debug_enter/_leave from struct fb_ops

 Documentation/process/debugging/kgdb.rst    |  28 -----
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  35 ++-----
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  35 ++-----
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  35 ++-----
 drivers/gpu/drm/drm_fb_helper.c             | 108 --------------------
 drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  24 +----
 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 +-
 drivers/video/fbdev/core/fbcon.c            |  24 -----
 drivers/video/fbdev/core/fbcon.h            |   1 -
 include/drm/drm_fb_helper.h                 |  21 ----
 include/drm/drm_modeset_helper_vtables.h    |  23 -----
 include/linux/fb.h                          |   4 -
 14 files changed, 63 insertions(+), 382 deletions(-)

base-commit: 0a21e96e0b6840d2a4e0b45a957679eeddeb4362
-- 
2.51.1

^ permalink raw reply

* Re: [PATCH] backlight: pwm_bl: apply the initial backlight state with sane defaults
From: Lee Jones @ 2025-11-25 12:54 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Michael Grzeschik, Daniel Thompson, Mark Brown, linux-pwm,
	linux-fbdev, Jingoo Han, Helge Deller, linux-kernel, dri-devel,
	Uwe Kleine-König, Pengutronix
In-Reply-To: <aSWUOoyusb2BJ6QA@aspen.lan>

On Tue, 25 Nov 2025, Daniel Thompson wrote:

> On Tue, Nov 25, 2025 at 09:24:26AM +0100, Michael Grzeschik wrote:
> > On Tue, Nov 18, 2025 at 12:52:14PM +0000, Daniel Thompson wrote:
> > > On Fri, Nov 14, 2025 at 02:09:56PM +0000, Mark Brown wrote:
> > > > On Thu, Jul 31, 2025 at 10:47:18AM +0200, Michael Grzeschik wrote:
> > > > > Currently when calling pwm_apply_might_sleep in the probe routine
> > > > > the pwm will be configured with an not fully defined state.
> > > > >
> > > > > The duty_cycle is not yet set in that moment. There is a final
> > > > > backlight_update_status call that will have a properly setup state.
> > > > > However this change in the backlight can create a short flicker if the
> > > > > backlight was already preinitialised.
> > > >
> > > > I'm seeing the libre.computer Renegade Elite producing warnings during
> > > > boot in -next which bisect to this patch.  The warnings are:
> > > >
> > > > [   24.175095] input: adc-keys as /devices/platform/adc-keys/input/input1
> > > > [   24.176612] ------------[ cut here ]------------
> > > > [   24.177048] WARNING: CPU: 0 PID: 0 at kernel/context_tracking.c:127 ct_kernel_exit.constprop.0+0x98/0xa0
> > > >
> > > > ...
> > > >
> > > > [   24.190106] Call trace:
> > > > [   24.190325]  ct_kernel_exit.constprop.0+0x98/0xa0 (P)
> > > > [   24.190775]  ct_idle_enter+0x10/0x20
> > > > [   24.191096]  cpuidle_enter_state+0x1fc/0x320
> > > > [   24.191476]  cpuidle_enter+0x38/0x50
> > > > [   24.191802]  do_idle+0x1e4/0x260
> > > > [   24.192094]  cpu_startup_entry+0x34/0x3c
> > > > [   24.192444]  rest_init+0xdc/0xe0
> > > > [   24.192734]  console_on_rootfs+0x0/0x6c
> > > > [   24.193082]  __primary_switched+0x88/0x90
> > > > [   24.193445] ---[ end trace 0000000000000000 ]---
> > > >
> > > > which seems a little surprising but there is some console stuff there
> > > > that looks relevant.
> > > >
> > > > Full log:
> > > >
> > > >     https://lava.sirena.org.uk/scheduler/job/2086528#L897
> > >
> > > Michael, reading these logs it looks to me like the underlying oops
> > > is this backtrace (which makes a lot more sense given the code you
> > > altered):
> > >
> > > [   24.133631] Call trace:
> > > [   24.133853]  pwm_backlight_probe+0x830/0x868 [pwm_bl] (P)
> > > [   24.134341]  platform_probe+0x5c/0xa4
> > > [   24.134679]  really_probe+0xbc/0x2c0
> > > [   24.135001]  __driver_probe_device+0x78/0x120
> > > [   24.135391]  driver_probe_device+0x3c/0x154
> > > [   24.135765]  __driver_attach+0x90/0x1a0
> > > [   24.136111]  bus_for_each_dev+0x7c/0xdc
> > > [   24.136462]  driver_attach+0x24/0x38
> > > [   24.136785]  bus_add_driver+0xe4/0x208
> > > [   24.137124]  driver_register+0x68/0x130
> > > [   24.137468]  __platform_driver_register+0x24/0x30
> > > [   24.137888]  pwm_backlight_driver_init+0x20/0x1000 [pwm_bl]
> > > [   24.138389]  do_one_initcall+0x60/0x1d4
> > > [   24.138735]  do_init_module+0x54/0x23c
> > > [   24.139073]  load_module+0x1760/0x1cf0
> > > [   24.139407]  init_module_from_file+0x88/0xcc
> > > [   24.139787]  __arm64_sys_finit_module+0x1bc/0x338
> > > [   24.140207]  invoke_syscall+0x48/0x104
> > > [   24.140549]  el0_svc_common.constprop.0+0x40/0xe0
> > > [   24.140970]  do_el0_svc+0x1c/0x28
> > > [   24.141268]  el0_svc+0x34/0xec
> > > [   24.141548]  el0t_64_sync_handler+0xa0/0xf0
> > > [   24.141920]  el0t_64_sync+0x198/0x19c
> > >
> > > Should we back out the patch for now?
> >
> > I would be fine with that. But actually I would like to see the
> > proof that without the patch, this backtrace will not trigger.
> > Looking through the codepath, I could not directly find a case
> > where this should happen.
> 
> I took a look at the logs Mark provided and I think the problem
> is a divide-by-zero caused by calling pwm_backlight_brightness_default()
> when state.period is zero.
> 
> It emerges as a BRK because the compiler recognised there is undefined
> behaviour. The zero that we divide by comes from a ternary condition in
> fls(). The compiler recognises one of the conditional code paths will
> result in undefined behaviour so, it doesn't need to generating code for
> the bad code path, it just injects a brk instruction.
> 
> 
> > Mark, is there a way to rerun this without my patch?
> 
> I have to admit I thought this was why Mark provided a bisect log!
> 
> Anyhow, unless someone can refute the analysis above I do think we need
> to pull the patch.

Un-applied now, thanks.

-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH] gpu: Move DRM buddy allocator one level up
From: Zhi Wang @ 2025-11-25 12:36 UTC (permalink / raw)
  To: Christian König
  Cc: John Hubbard, Dave Airlie, Joel Fernandes, linux-kernel,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Jonathan Corbet, Alex Deucher, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, Danilo Krummrich, Alice Ryhl,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, nouveau, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <9f433dee-7ad9-4d0f-8ac1-e67deb409b70@amd.com>

On Tue, 25 Nov 2025 09:10:54 +0100
Christian König <christian.koenig@amd.com> wrote:

> On 11/25/25 08:59, John Hubbard wrote:
> > On 11/24/25 11:54 PM, Christian König wrote:
> >> On 11/25/25 08:49, Dave Airlie wrote:
> >>> On Tue, 25 Nov 2025 at 17:45, Christian König
> >>> <christian.koenig@amd.com> wrote:
> > ...
> >> My question is why exactly is nova separated into nova-core and
> >> nova-drm? That doesn't seem to be necessary in the first place.
> >>
> > The idea is that nova-core allows building up a separate software
> > stack for VFIO, without pulling in any DRM-specific code that a
> > hypervisor (for example) wouldn't need. That makes for a smaller,
> > more security-auditable set of code for that case.
> 
> Well that is the same argument used by some AMD team to maintain a
> separate out of tree hypervisor for nearly a decade.
> 

I guess you mean the VFIO driver? [1]  In the code, it is bascially to
support the migration, which is simiar as any other in-tree VFIO
drivers. The questionable parts might be how to clean up those
callbacks supporting migration and get them into mainline.

Those callbacks stays in the PF driver, either talks to HW or firmware
interface to control VF states, obtain the bitstream, which contains VF
states and data.

IMO, they should be quite self-contained and userspace shouldn't be
invovled. Userspace (QEMU) only talks to VFIO.

This is just my initial impression from briefly looking at the code. :)

[1]
https://github.com/amd/MxGPU-Virtualization/blob/staging/amd-vfio-pci/amd-vfio.c

> Additional to that the same argument has also been used to justify
> the KFD node as alternative API to DRM for compute.
> 
> Both cases have proven to be extremely bad ideas.
> 
> Background is that except for all the legacy stuff the DRM API is
> actually very well thought through and it is actually quite hard to
> come up with something similarly well.
> 
> Regards,
> Christian. 
> 
> > 
> > thanks,
> 
> 


^ permalink raw reply

* Re: [PATCH] backlight: pwm_bl: apply the initial backlight state with sane defaults
From: Mark Brown @ 2025-11-25 12:36 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Michael Grzeschik, Daniel Thompson, linux-pwm, linux-fbdev,
	Jingoo Han, Helge Deller, Lee Jones, linux-kernel, dri-devel,
	Uwe Kleine-König, Pengutronix
In-Reply-To: <aSWUOoyusb2BJ6QA@aspen.lan>

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

On Tue, Nov 25, 2025 at 11:34:18AM +0000, Daniel Thompson wrote:
> On Tue, Nov 25, 2025 at 09:24:26AM +0100, Michael Grzeschik wrote:
> > On Tue, Nov 18, 2025 at 12:52:14PM +0000, Daniel Thompson wrote:

> > > Should we back out the patch for now?

> > I would be fine with that. But actually I would like to see the
> > proof that without the patch, this backtrace will not trigger.
> > Looking through the codepath, I could not directly find a case
> > where this should happen.

...

> > Mark, is there a way to rerun this without my patch?

> I have to admit I thought this was why Mark provided a bisect log!

Yeah, there should be a before and after for applying the patch in the
bisect log otherwise it wouldn't have got a result out.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH] backlight: pwm_bl: apply the initial backlight state with sane defaults
From: Daniel Thompson @ 2025-11-25 11:34 UTC (permalink / raw)
  To: Michael Grzeschik
  Cc: Daniel Thompson, Mark Brown, linux-pwm, linux-fbdev, Jingoo Han,
	Helge Deller, Lee Jones, linux-kernel, dri-devel,
	Uwe Kleine-König, Pengutronix
In-Reply-To: <aSVnulk0yfAd4UCx@pengutronix.de>

On Tue, Nov 25, 2025 at 09:24:26AM +0100, Michael Grzeschik wrote:
> On Tue, Nov 18, 2025 at 12:52:14PM +0000, Daniel Thompson wrote:
> > On Fri, Nov 14, 2025 at 02:09:56PM +0000, Mark Brown wrote:
> > > On Thu, Jul 31, 2025 at 10:47:18AM +0200, Michael Grzeschik wrote:
> > > > Currently when calling pwm_apply_might_sleep in the probe routine
> > > > the pwm will be configured with an not fully defined state.
> > > >
> > > > The duty_cycle is not yet set in that moment. There is a final
> > > > backlight_update_status call that will have a properly setup state.
> > > > However this change in the backlight can create a short flicker if the
> > > > backlight was already preinitialised.
> > >
> > > I'm seeing the libre.computer Renegade Elite producing warnings during
> > > boot in -next which bisect to this patch.  The warnings are:
> > >
> > > [   24.175095] input: adc-keys as /devices/platform/adc-keys/input/input1
> > > [   24.176612] ------------[ cut here ]------------
> > > [   24.177048] WARNING: CPU: 0 PID: 0 at kernel/context_tracking.c:127 ct_kernel_exit.constprop.0+0x98/0xa0
> > >
> > > ...
> > >
> > > [   24.190106] Call trace:
> > > [   24.190325]  ct_kernel_exit.constprop.0+0x98/0xa0 (P)
> > > [   24.190775]  ct_idle_enter+0x10/0x20
> > > [   24.191096]  cpuidle_enter_state+0x1fc/0x320
> > > [   24.191476]  cpuidle_enter+0x38/0x50
> > > [   24.191802]  do_idle+0x1e4/0x260
> > > [   24.192094]  cpu_startup_entry+0x34/0x3c
> > > [   24.192444]  rest_init+0xdc/0xe0
> > > [   24.192734]  console_on_rootfs+0x0/0x6c
> > > [   24.193082]  __primary_switched+0x88/0x90
> > > [   24.193445] ---[ end trace 0000000000000000 ]---
> > >
> > > which seems a little surprising but there is some console stuff there
> > > that looks relevant.
> > >
> > > Full log:
> > >
> > >     https://lava.sirena.org.uk/scheduler/job/2086528#L897
> >
> > Michael, reading these logs it looks to me like the underlying oops
> > is this backtrace (which makes a lot more sense given the code you
> > altered):
> >
> > [   24.133631] Call trace:
> > [   24.133853]  pwm_backlight_probe+0x830/0x868 [pwm_bl] (P)
> > [   24.134341]  platform_probe+0x5c/0xa4
> > [   24.134679]  really_probe+0xbc/0x2c0
> > [   24.135001]  __driver_probe_device+0x78/0x120
> > [   24.135391]  driver_probe_device+0x3c/0x154
> > [   24.135765]  __driver_attach+0x90/0x1a0
> > [   24.136111]  bus_for_each_dev+0x7c/0xdc
> > [   24.136462]  driver_attach+0x24/0x38
> > [   24.136785]  bus_add_driver+0xe4/0x208
> > [   24.137124]  driver_register+0x68/0x130
> > [   24.137468]  __platform_driver_register+0x24/0x30
> > [   24.137888]  pwm_backlight_driver_init+0x20/0x1000 [pwm_bl]
> > [   24.138389]  do_one_initcall+0x60/0x1d4
> > [   24.138735]  do_init_module+0x54/0x23c
> > [   24.139073]  load_module+0x1760/0x1cf0
> > [   24.139407]  init_module_from_file+0x88/0xcc
> > [   24.139787]  __arm64_sys_finit_module+0x1bc/0x338
> > [   24.140207]  invoke_syscall+0x48/0x104
> > [   24.140549]  el0_svc_common.constprop.0+0x40/0xe0
> > [   24.140970]  do_el0_svc+0x1c/0x28
> > [   24.141268]  el0_svc+0x34/0xec
> > [   24.141548]  el0t_64_sync_handler+0xa0/0xf0
> > [   24.141920]  el0t_64_sync+0x198/0x19c
> >
> > Should we back out the patch for now?
>
> I would be fine with that. But actually I would like to see the
> proof that without the patch, this backtrace will not trigger.
> Looking through the codepath, I could not directly find a case
> where this should happen.

I took a look at the logs Mark provided and I think the problem
is a divide-by-zero caused by calling pwm_backlight_brightness_default()
when state.period is zero.

It emerges as a BRK because the compiler recognised there is undefined
behaviour. The zero that we divide by comes from a ternary condition in
fls(). The compiler recognises one of the conditional code paths will
result in undefined behaviour so, it doesn't need to generating code for
the bad code path, it just injects a brk instruction.


> Mark, is there a way to rerun this without my patch?

I have to admit I thought this was why Mark provided a bisect log!

Anyhow, unless someone can refute the analysis above I do think we need
to pull the patch.


Daniel.

^ permalink raw reply

* Re: [PATCH] gpu: Move DRM buddy allocator one level up
From: Christian König @ 2025-11-25  9:14 UTC (permalink / raw)
  To: Dave Airlie
  Cc: John Hubbard, Joel Fernandes, linux-kernel, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
	Lucas De Marchi, Thomas Hellström, Helge Deller,
	Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, Alistair Popple, Timur Tabi,
	Edwin Peer, Alexandre Courbot, nouveau, dri-devel, rust-for-linux,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <CAPM=9tyN_A3oEyQZCOWaLO1orO6oKX0ZukJHR7cFy12Go+7d=A@mail.gmail.com>

On 11/25/25 10:08, Dave Airlie wrote:
> On Tue, 25 Nov 2025 at 18:11, Christian König <christian.koenig@amd.com> wrote:
>>
>> On 11/25/25 08:59, John Hubbard wrote:
>>> On 11/24/25 11:54 PM, Christian König wrote:
>>>> On 11/25/25 08:49, Dave Airlie wrote:
>>>>> On Tue, 25 Nov 2025 at 17:45, Christian König <christian.koenig@amd.com> wrote:
>>> ...
>>>> My question is why exactly is nova separated into nova-core and nova-drm? That doesn't seem to be necessary in the first place.
>>>>
>>> The idea is that nova-core allows building up a separate software stack for
>>> VFIO, without pulling in any DRM-specific code that a hypervisor (for example)
>>> wouldn't need. That makes for a smaller, more security-auditable set of code
>>> for that case.
>>
>> Well that is the same argument used by some AMD team to maintain a separate out of tree hypervisor for nearly a decade.
>>
>> Additional to that the same argument has also been used to justify the KFD node as alternative API to DRM for compute.
>>
>> Both cases have proven to be extremely bad ideas.
>>
>> Background is that except for all the legacy stuff the DRM API is actually very well thought through and it is actually quite hard to come up with something similarly well.
>>
> 
> Well you just answered your own question, why is AMD maintaining GIM
> instead of solving this upstream with a split model? the nova-core/drm
> split would be perfect for GIM.

No, it won't.

We have the requirement to work with GEM objects and DMA-buf file descriptors in the hypervisor as well.

And my suspicion is that you end up with the same requirements in nova as well in which case you end up interchanging handles with DRM as well.

We have seen the same for KFD and it turned out to be an absolutely horrible interaction.

> kfd was a terrible idea, and we don't intend to offer userspace
> multiple APIs with nova, nova-drm will be the primary userspace API
> provider. nova-core will not provide userspace API, it will provide an
> API to nova-drm and an API to the vgpu driver which will provide it's
> own userspace API without graphics or compute, just enough to setup
> VFs.

Ok, then why do you need nova-core in the first place? E.g. where should be the vgpu driver and what interface does it provide?

Christian.

> 
> Dave.


^ permalink raw reply

page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox