On 13/11/12(Tue) 13:45, Mark Kettenis wrote:
> > Date: Tue, 13 Nov 2012 12:30:29 +0100
> > From: Martin Pieuchot <[email protected]>
> >
> > While experimenting with the agp(4) interface I found that if you
> > release the interface (AGPIOC_RELEASE) before closing its file
> > descriptor you'll end up with allocated but unbinded memory blocks.
>
> That behaviour is documented in agp(4). So if we change it, we should
> change the documentation as well. That said, the documentation also
> says that the AGPIOC_RELEASE ioctl doesn't unbind any allocated
> memory. And that doesn't seem to be true.
>
> > This behavior is due to the fact that the agp_release_helper() function
> > doesn't free the memory associated to each block and this is incoherent
> > with what it says it does:
> >
> > /*
> > * Clear out the aperture and free any
> > * outstanding memory blocks.
> > */
> > ...
> >
> > So the diff below correct this by freeing all the memory block when
> > releasing the interface, this is what happens currently if you close
> > the file descriptor without releasing the interface.
>
> I;m not sure this is right. I think the idea here is that an
> application could release control to hand things over to drm, and
> later reacquire control. The comment in
> xenocara/xserver/hw/xfree86/os-support/bsd/bsd_agp.c:xf86ReleaseGART()
> suggests that this behaviour is desired.
Fair enough, so here's a diff that removes the chunk of code unbinding
the memory block from the release routine. This makes the code match
what the manual says.
Ok?
Index: agp.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/agp.c,v
retrieving revision 1.34
diff -u -p -r1.34 agp.c
--- agp.c 26 Dec 2010 15:41:00 -0000 1.34
+++ agp.c 13 Nov 2012 13:41:43 -0000
@@ -658,25 +678,13 @@ int
agp_release_helper(void *dev, enum agp_acquire_state state)
{
struct agp_softc *sc = (struct agp_softc *)dev;
- struct agp_memory* mem;
if (sc->sc_state == AGP_ACQUIRE_FREE)
return (0);
- if (sc->sc_state != state)
+ if (sc->sc_state != state)
return (EBUSY);
- /*
- * Clear out the aperture and free any
- * outstanding memory blocks.
- */
- TAILQ_FOREACH(mem, &sc->sc_memory, am_link) {
- if (mem->am_is_bound) {
- printf("agp_release_helper: mem %d is bound\n",
- mem->am_id);
- agp_unbind_memory(sc, mem);
- }
- }
sc->sc_state = AGP_ACQUIRE_FREE;
return (0);
}
