From: Mark Kettenis Subject: Re: radeondrm: use the common KMS console path on sparc64 To: Jonathan Gray Cc: tbullock@comlore.com, tech@openbsd.org, kettenis@openbsd.org Date: Sat, 10 Oct 2026 13:35:11 +0200 > Date: Fri, 9 Oct 2026 19:50:35 +1100 > From: Jonathan Gray > > On Fri, Oct 09, 2026 at 12:00:50AM -0600, Ted Bullock wrote: > > Hi Jonathan, > > > > The two calls initialize different objects. fbwscons_init() sets up > > rdev->sf.sf_ro for the firmware framebuffer, while the second call > > initializes rdev->ro for the framebuffer for the KMS. Removing the > > reassignment to sf.sf_ro is what makes the second initialization > > necessary including its RI_BSWAP. > > > > The revised patch below also removes RI_VCONS and RI_WRONLY from the > > early initialization. The KMS object is the only one to have virtual > > console allocation. > > You are effectively reverting two of Mark's patches. So it would be > good to run it by him. The RI_FULLCLEAR and eraserows part you added > after perhaps should be a different patch. Right. The whole idea here was that the driver would take over the firmware framebuffer and preserve it. That is how other Sun framebuffers work. This worked at the time I implemented it. The radeondrm(4) would skip the modeset if a usable framebuffer was already set up. I also believe that OpenFirmware typically sets up an 8-bit framebuffer (which is nice since it makes scrolling considerably faster than a 24/32-bit framebuffer and leaves more VRAM available for X). I'm not surprised this got broken. One thing I noticed is that Ted is clearly getting a 24/32-bit framebuffer. If I'm right and the initial framebuffer is indeed 8-bit, this means a modeset happened and a new 32-bit framebuffer was allocated for the console. So maybe the code can be repaired by adding a bit more code that sets the drm framebuffer format based on the depth of the initial framebuffer. That information is available in the "struct sunfb". If that doesn't work, I think Ted's approach makes sense. I also must warn that on sparc64 there may be some funny byte twisting going on for PCI devices. So endianness issues may appear differently on sparc64 compared to other big-endian architectures. > ---------------------------- > revision 1.17 > date: 2013/12/22 21:03:45; author: kettenis; state: Exp; lines: +18 -7; > Try to keep the framebuffer console layout that was set up by the firmware > on sparc64. > ---------------------------- > > Index: radeon_kms.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/drm/radeon/radeon_kms.c,v > diff -u -p -r1.16 -r1.17 > --- radeon_kms.c 22 Dec 2013 19:49:23 -0000 1.16 > +++ radeon_kms.c 22 Dec 2013 21:03:45 -0000 1.17 > @@ -1,4 +1,4 @@ > -/* $OpenBSD: radeon_kms.c,v 1.16 2013/12/22 19:49:23 kettenis Exp $ */ > +/* $OpenBSD: radeon_kms.c,v 1.17 2013/12/22 21:03:45 kettenis Exp $ */ > /* > * Copyright 2008 Advanced Micro Devices, Inc. > * Copyright 2008 Red Hat Inc. > @@ -295,6 +295,9 @@ void radeondrm_free_screen(void *, void > int radeondrm_show_screen(void *, void *, int, > void (*)(void *, int, int), void *); > void radeondrm_doswitch(void *, void *); > +#ifdef __sparc64__ > +void radeondrm_setcolor(void *, u_int, u_int8_t, u_int8_t, u_int8_t); > +#endif > > struct wsscreen_descr radeondrm_stdscreen = { > "std", > @@ -396,6 +399,9 @@ radeondrm_doswitch(void *v, void *cookie > radeon_crtc->lut_b[i] = rasops_cmap[(3 * i) + 2] << 2; > } > } > +#ifdef __sparc64__ > + fbwscons_setcolormap(&rdev->sf, radeondrm_setcolor); > +#endif > drm_fb_helper_restore(); > > if (rdev->switchcb) > @@ -403,8 +409,6 @@ radeondrm_doswitch(void *v, void *cookie > } > > #ifdef __sparc64__ > -void radeondrm_setcolor(void *, u_int, u_int8_t, u_int8_t, u_int8_t); > - > void > radeondrm_setcolor(void *v, u_int index, u_int8_t r, u_int8_t g, u_int8_t b) > { > @@ -563,7 +567,7 @@ radeondrm_attach_kms(struct device *pare > * an offset from the start of video memory. > */ > rdev->fb_offset = > - bus_space_read_4(rdev->memt, rdev->rmmio, RADEON_CRTC_OFFSET); > + bus_space_read_4(rdev->memt, rdev->rmmio, RADEON_CRTC_OFFSET); > if (bus_space_map(rdev->memt, rdev->fb_aper_offset + rdev->fb_offset, > rdev->sf.sf_fbsize, BUS_SPACE_MAP_LINEAR, &rdev->memh)) { > printf("%s: can't map video memory\n", rdev->dev.dv_xname); > @@ -573,6 +577,7 @@ radeondrm_attach_kms(struct device *pare > ri = &rdev->sf.sf_ro; > ri->ri_bits = bus_space_vaddr(rdev->memt, rdev->memh); > ri->ri_hw = rdev; > + ri->ri_updatecursor = NULL; > > fbwscons_init(&rdev->sf, RI_VCONS | RI_WRONLY | RI_BSWAP, console); > if (console) > @@ -651,13 +656,14 @@ radeondrm_attachhook(void *xsc) > #endif > drm_fb_helper_restore(); > > +#ifndef __sparc64__ > ri->ri_flg = RI_CENTER | RI_VCONS | RI_WRONLY; > -#ifdef __sparc64__ > - ri->ri_flg |= RI_BSWAP; > -#endif > rasops_init(ri, 160, 160); > > ri->ri_hw = rdev; > +#else > + ri = &rdev->sf.sf_ro; > +#endif > > radeondrm_stdscreen.capabilities = ri->ri_caps; > radeondrm_stdscreen.nrows = ri->ri_rows; > @@ -1001,6 +1007,11 @@ int radeon_driver_firstopen_kms(struct d > */ > void radeon_driver_lastclose_kms(struct drm_device *dev) > { > +#ifdef __sparc64__ > + struct radeon_device *rdev = dev->dev_private; > + > + fbwscons_setcolormap(&rdev->sf, radeondrm_setcolor); > +#endif > drm_fb_helper_restore(); > #ifdef notyet > vga_switcheroo_process_delayed_switch(); > > ---------------------------- > revision 1.3 > date: 2013/08/16 19:53:53; author: kettenis; state: Exp; lines: +28 -1; > Hook up the framebuffer colormap helper code on sparc64. > ---------------------------- > > Index: radeon_kms.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/drm/radeon/radeon_kms.c,v > diff -u -p -r1.2 -r1.3 > --- radeon_kms.c 16 Aug 2013 19:36:50 -0000 1.2 > +++ radeon_kms.c 16 Aug 2013 19:53:53 -0000 1.3 > @@ -1,4 +1,4 @@ > -/* $OpenBSD: radeon_kms.c,v 1.2 2013/08/16 19:36:50 kettenis Exp $ */ > +/* $OpenBSD: radeon_kms.c,v 1.3 2013/08/16 19:53:53 kettenis Exp $ */ > /* > * Copyright 2008 Advanced Micro Devices, Inc. > * Copyright 2008 Red Hat Inc. > @@ -1530,6 +1530,30 @@ radeondrm_getchar(void *v, int row, int > return rasops_getchar(ri, row, col, cell); > } > > +#ifdef __sparc64__ > +void radeondrm_setcolor(void *, u_int, u_int8_t, u_int8_t, u_int8_t); > + > +void > +radeondrm_setcolor(void *v, u_int index, u_int8_t r, u_int8_t g, u_int8_t b) > +{ > + struct sunfb *sf = v; > + struct radeon_device *rdev = sf->sf_ro.ri_hw; > + struct drm_fb_helper *fb_helper = (void *)rdev->mode_info.rfbdev; > + u_int16_t red, green, blue; > + struct drm_crtc *crtc; > + int i; > + > + for (i = 0; i < fb_helper->crtc_count; i++) { > + crtc = fb_helper->crtc_info[i].mode_set.crtc; > + > + red = (r << 8) | r; > + green = (g << 8) | g; > + blue = (b << 8) | b; > + fb_helper->funcs->gamma_set(crtc, red, green, blue, index); > + } > +} > +#endif > + > /** > * radeon_driver_load_kms - Main load function for KMS. > * > @@ -1749,6 +1773,9 @@ radeondrm_attachhook(void *xsc) > if (ri->ri_bits == NULL) > return; > > +#ifdef __sparc64__ > + fbwscons_setcolormap(&rdev->sf, radeondrm_setcolor); > +#endif > drm_fb_helper_restore(); > > ri->ri_flg = RI_CENTER | RI_VCONS | RI_WRONLY; > > > > > I think there are two resources left behind after the handoff that > > could be cleaned up > > - the early font reference > > - kernel mapping of the firmware framebuffer > > > > I have not looked at doing that cleanup yet. > > > > Also the console isn't centered as per the firmware, I don't really > > like the centered console personally. > > > > Anyway here's a screenshot of my framebuffer right now. > > https://files.catbox.moe/nqc7f4.png > > > > Ted > > > > --- a/sys/dev/pci/drm/radeon/radeon_drv.c > > +++ b/sys/dev/pci/drm/radeon/radeon_drv.c > > @@ -741,13 +741,10 @@ > > void radeondrm_free_screen(void *, void *); > > int radeondrm_show_screen(void *, void *, int, > > void (*)(void *, int, int), void *); > > void radeondrm_doswitch(void *); > > void radeondrm_enter_ddb(void *, void *); > > -#ifdef __sparc64__ > > -void radeondrm_setcolor(void *, u_int, u_int8_t, u_int8_t, u_int8_t); > > -#endif > > void radeondrm_setpal(struct radeon_device *, struct rasops_info *); > > > > struct wsscreen_descr radeondrm_stdscreen = { > > "std", > > 0, 0, > > @@ -862,15 +859,11 @@ > > { > > struct rasops_info *ri = v; > > struct radeon_device *rdev = ri->ri_hw; > > > > rasops_show_screen(ri, rdev->switchcookie, 0, NULL, NULL); > > -#ifdef __sparc64__ > > - fbwscons_setcolormap(&rdev->sf, radeondrm_setcolor); > > -#else > > radeondrm_setpal(rdev, ri); > > -#endif > > drm_fb_helper_restore_fbdev_mode_unlocked(rdev_to_drm(rdev)->fb_helper, > > false); > > > > if (rdev->switchcb) > > (rdev->switchcb)(rdev->switchcbarg, 0, 0); > > @@ -887,29 +880,10 @@ > > return; > > > > rasops_show_screen(ri, cookie, 0, NULL, NULL); > > drm_fb_helper_debug_enter(fb_helper->info); > > } > > - > > -#ifdef __sparc64__ > > -void > > -radeondrm_setcolor(void *v, u_int index, u_int8_t r, u_int8_t g, u_int8_t b) > > -{ > > - struct sunfb *sf = v; > > - struct radeon_device *rdev = sf->sf_ro.ri_hw; > > - > > - /* see legacy_crtc_load_lut() */ > > - if (rdev->family < CHIP_RS600) { > > - WREG8(RADEON_PALETTE_INDEX, index); > > - WREG32(RADEON_PALETTE_30_DATA, > > - (r << 22) | (g << 12) | (b << 2)); > > - } else { > > - printf("%s: setcolor family %d not handled\n", > > - rdev->self.dv_xname, rdev->family); > > - } > > -} > > -#endif > > > > void > > radeondrm_setpal(struct radeon_device *rdev, struct rasops_info *ri) > > { > > struct drm_device *dev = rdev_to_drm(rdev); > > @@ -1167,11 +1141,11 @@ > > ri = &rdev->sf.sf_ro; > > ri->ri_bits = bus_space_vaddr(rdev->memt, rdev->memh); > > ri->ri_hw = rdev; > > ri->ri_updatecursor = NULL; > > > > - fbwscons_init(&rdev->sf, RI_VCONS | RI_WRONLY | RI_BSWAP, console); > > + fbwscons_init(&rdev->sf, RI_BSWAP, console); > > if (console) > > fbwscons_console_init(&rdev->sf, -1); > > } > > #endif > > > > @@ -1285,20 +1259,18 @@ > > drm_client_setup(dev, format); > > > > if (ri->ri_bits == NULL) > > return; > > > > -#ifdef __sparc64__ > > - fbwscons_setcolormap(&rdev->sf, radeondrm_setcolor); > > - ri = &rdev->sf.sf_ro; > > -#else > > radeondrm_setpal(rdev, ri); > > ri->ri_flg = RI_CENTER | RI_VCONS | RI_WRONLY; > > +#ifdef __sparc64__ > > + ri->ri_flg |= RI_BSWAP | RI_FULLCLEAR; > > +#endif > > rasops_init(ri, 160, 160); > > > > ri->ri_hw = rdev; > > -#endif > > > > radeondrm_stdscreen.capabilities = ri->ri_caps; > > radeondrm_stdscreen.nrows = ri->ri_rows; > > radeondrm_stdscreen.ncols = ri->ri_cols; > > radeondrm_stdscreen.textops = &ri->ri_ops; > > @@ -1314,10 +1286,13 @@ > > > > if (rdev->console) { > > uint32_t defattr; > > > > ri->ri_ops.pack_attr(ri->ri_active, 0, 0, 0, &defattr); > > +#ifdef __sparc64__ > > + ri->ri_ops.eraserows(ri->ri_active, 0, ri->ri_rows, defattr); > > +#endif > > wsdisplay_cnattach(&radeondrm_stdscreen, ri->ri_active, > > ri->ri_ccol, ri->ri_crow, defattr); > > } > > > > /* > > > > > >