On Mon, Jun 8, 2026 at 11:52 AM Andrius V <[email protected]> wrote: > > Forgot to comment on these: > > On Mon, Jun 8, 2026 at 1:50 AM Taylor R Campbell <[email protected]> wrote: > > > + u_int32_t initial_aperture; /* aperture size at > > > startup */ > > > [...] > > > + u_int32_t wbc_mask; > > > > Since this is effectively new code without an upstream (assuming it's > > going to be deleted in FreeBSD soon), use the standard C99 uint32_t > > types. > > > > I kept u_int32_t to stay consistent with the other AGP drives, > agp_methods signatures use it. > For local variables I've already use uint32_t in nvidia_init_iorr(), > and I can do the same in the other methods. > > >> + /* Wait no more than 3 seconds. */ > >> + for (i = 0; i < 3000; i++) { > >> + wbc_reg = pci_conf_read(sc->as_pc, nsc->mc1_tag, > >> AGP_NVIDIA_1_WBC); > >> + if ((nsc->wbc_mask & wbc_reg) == 0) > >> + break; > >> + else > >> + DELAY(1000); > > Seems like a very long time to wait in a DELAY loop? Wonder if this > > should be a preempt_point. > > Good point, but lack technical understanding here. > I need to investigate on how to rewrite or improve this part. Advice > welcome if you already know what is the best way to do it or what to > look at. > > Regards, > Andrius V
I believe I've addressed all of the review comments, along with a few additional issues I found myself. I also added preempt_point();. I believe an actual long timeout should not occur though... To make the build work with lexicographically sorted imports, I had to add <dev/pci/pcivar.h> to /dev/pci/agpvar.h. I will likely commit this change separately in advance and then reorder the imports in the other agp_<vendor>.c files. The latest state is available in the same PR: https://github.com/vezhlys/netbsd-src/pull/4/changes
