On Mon, Jun 8, 2026 at 11:52 AM Andrius V <vezhlys%gmail.com@localhost> wrote:
>
> Forgot to comment on these:
>
> On Mon, Jun 8, 2026 at 1:50 AM Taylor R Campbell <riastradh%netbsd.org@localhost> 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