From: Jonathan Gray Subject: Re: radeondrm: use the common KMS console path on sparc64 To: Ted Bullock Cc: tech@openbsd.org, kettenis@openbsd.org Date: Fri, 9 Oct 2026 19:50:35 +1100 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. ---------------------------- 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); > } > > /* > >