On 04/02/11 13:33 +1100, ext Christopher James Halse Rogers wrote: > On Thu, 2011-02-03 at 19:48 +0200, Pauli wrote: > > From: Pauli Nieminen <[email protected]> > > > > To let DRI2Drawable exists longer than Drawable driver has to use > > DRI2DrawablePtr to complete swaps and MSC waits. This allows DRI2 to > > clean up after all operations complete without accessing the freed > > DrawablePtr. > > > > Signed-off-by: Pauli Nieminen <[email protected]> > > --- > > hw/xfree86/dri2/dri2.c | 69 > > +++++++++++++++++++++++------------------------ > > hw/xfree86/dri2/dri2.h | 14 +++++++-- > > 2 files changed, 45 insertions(+), 38 deletions(-) > > > > diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c > > index 91ae1a0..f564822 100644 > > --- a/hw/xfree86/dri2/dri2.c > > +++ b/hw/xfree86/dri2/dri2.c > > @@ -137,6 +137,12 @@ DRI2DrawableGetDrawable(DRI2DrawablePtr pPriv) > > return pPriv->drawable; > > } > > > > +ScreenPtr > > +DRI2DrawableGetScreen(DRI2DrawablePtr pPriv) > > +{ > > + return pPriv->dri2_screen->screen; > > +} > > + > > static unsigned long > > DRI2DrawableSerial(DrawablePtr pDraw) > > { > > @@ -323,6 +329,14 @@ static int DRI2DrawableGone(pointer p, XID id) > > return Success; > > > > pDraw = pPriv->drawable; > > + > > + if (pPriv->buffers != NULL) { > > + for (i = 0; i < pPriv->bufferCount; i++) > > + (*ds->DestroyBuffer)(pPriv, pPriv->buffers[i]); > > + > > + free(pPriv->buffers); > > + } > > + > > if (pDraw->type == DRAWABLE_WINDOW) { > > pWin = (WindowPtr) pDraw; > > dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL); > > @@ -331,13 +345,6 @@ static int DRI2DrawableGone(pointer p, XID id) > > dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL); > > } > > > > - if (pPriv->buffers != NULL) { > > - for (i = 0; i < pPriv->bufferCount; i++) > > - (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]); > > - > > - free(pPriv->buffers); > > - } > > - > > I can't see any reason to move the buffer-destruction loop? If there > isn't, not moving it would make the functional part of this diff more > obvious. > > > free(pPriv); > > > > return Success; > > @@ -394,7 +401,7 @@ update_dri2_drawable_buffers(DRI2DrawablePtr pPriv, > > DrawablePtr pDraw, > > if (pPriv->buffers != NULL) { > > for (i = 0; i < pPriv->bufferCount; i++) { > > if (pPriv->buffers[i] != NULL) { > > - (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]); > > + (*ds->DestroyBuffer)(pPriv, pPriv->buffers[i]); > > } > > } > > > > @@ -531,7 +538,7 @@ err_out: > > > > for (i = 0; i < count; i++) { > > if (buffers[i] != NULL) > > - (*ds->DestroyBuffer)(pDraw, buffers[i]); > > + (*ds->DestroyBuffer)(pPriv, buffers[i]); > > } > > > > free(buffers); > > @@ -684,14 +691,10 @@ DRI2CanExchange(DrawablePtr pDraw) > > } > > > > void > > -DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame, > > +DRI2WaitMSCComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame, > > unsigned int tv_sec, unsigned int tv_usec) > > { > > - DRI2DrawablePtr pPriv; > > > > - pPriv = DRI2GetDrawable(pDraw); > > - if (pPriv == NULL) > > - return; > > > > ProcDRI2WaitMSCReply(client, ((CARD64)tv_sec * 1000000) + tv_usec, > > frame, pPriv->swap_count); > > @@ -740,33 +743,29 @@ DRI2WakeClient(ClientPtr client, DRI2DrawablePtr > > pPriv, int frame, > > } > > > > void > > -DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, > > +DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame, > > unsigned int tv_sec, unsigned int tv_usec, int type, > > DRI2SwapEventPtr swap_complete, void *swap_data) > > { > > - ScreenPtr pScreen = pDraw->pScreen; > > - DRI2DrawablePtr pPriv; > > + DrawablePtr pDraw = pPriv->drawable; > > CARD64 ust = 0; > > - BoxRec box; > > - RegionRec region; > > - > > - pPriv = DRI2GetDrawable(pDraw); > > - if (pPriv == NULL) { > > - xf86DrvMsg(pScreen->myNum, X_ERROR, > > - "[DRI2] %s: bad drawable\n", __func__); > > - return; > > - } > > > > pPriv->swapsPending--; > > pPriv->swap_count++; > > > > - box.x1 = 0; > > - box.y1 = 0; > > - box.x2 = pDraw->width; > > - box.y2 = pDraw->height; > > - RegionInit(®ion, &box, 0); > > - DRI2CopyRegion(pPriv, ®ion, DRI2BufferFakeFrontLeft, > > - DRI2BufferFrontLeft); > > + > > + if (pDraw) { > > + BoxRec box; > > + RegionRec region; > > + > > + box.x1 = 0; > > + box.y1 = 0; > > + box.x2 = pDraw->width; > > + box.y2 = pDraw->height; > > + RegionInit(®ion, &box, 0); > > + DRI2CopyRegion(pPriv, ®ion, DRI2BufferFakeFrontLeft, > > + DRI2BufferFrontLeft); > > + } > > > > ust = ((CARD64)tv_sec * 1000000) + tv_usec; > > if (swap_complete) > > @@ -839,7 +838,7 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr > > pPriv, CARD64 target_msc, > > pPriv->swapsPending++; > > > > (*ds->CopyRegion)(pDraw, ®ion, pDestBuffer, pSrcBuffer); > > - DRI2SwapComplete(client, pDraw, target_msc, 0, 0, DRI2_BLIT_COMPLETE, > > + DRI2SwapComplete(client, pPriv, target_msc, 0, 0, DRI2_BLIT_COMPLETE, > > func, data); > > return Success; > > } > > @@ -954,7 +953,7 @@ DRI2WaitMSC(ClientPtr client, DRI2DrawablePtr pPriv, > > CARD64 target_msc, > > > > /* Old DDX just completes immediately */ > > if (!ds->ScheduleWaitMSC) { > > - DRI2WaitMSCComplete(client, pDraw, target_msc, 0, 0); > > + DRI2WaitMSCComplete(client, pPriv, target_msc, 0, 0); > > > > return Success; > > } > > diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h > > index 509d4f6..0182700 100644 > > --- a/hw/xfree86/dri2/dri2.h > > +++ b/hw/xfree86/dri2/dri2.h > > @@ -111,7 +111,7 @@ typedef int > > (*DRI2ScheduleSwapProcPtr)(ClientPtr client, > > typedef DRI2BufferPtr (*DRI2CreateBufferProcPtr)(DrawablePtr pDraw, > > unsigned int attachment, > > unsigned int format); > > -typedef void (*DRI2DestroyBufferProcPtr)(DrawablePtr pDraw, > > +typedef void (*DRI2DestroyBufferProcPtr)(DRI2DrawablePtr > > pPriv, > > DRI2BufferPtr buffer); > > /** > > * Get current media stamp counter values > > @@ -282,12 +282,12 @@ extern _X_EXPORT Bool DRI2CanExchange(DrawablePtr > > pDraw); > > /* Note: use *only* for MSC related waits */ > > extern _X_EXPORT void DRI2BlockClient(ClientPtr client, DrawablePtr pDraw); > > > > -extern _X_EXPORT void DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, > > +extern _X_EXPORT void DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr > > pPriv, > > int frame, unsigned int tv_sec, > > unsigned int tv_usec, int type, > > DRI2SwapEventPtr swap_complete, > > void *swap_data); > > -extern _X_EXPORT void DRI2WaitMSCComplete(ClientPtr client, DrawablePtr > > pDraw, > > +extern _X_EXPORT void DRI2WaitMSCComplete(ClientPtr client, > > DRI2DrawablePtr pPriv, > > int frame, unsigned int tv_sec, > > unsigned int tv_usec); > > /** > > @@ -301,6 +301,14 @@ extern _X_EXPORT void DRI2WaitMSCComplete(ClientPtr > > client, DrawablePtr pDraw, > > extern _X_EXPORT DrawablePtr DRI2DrawableGetDrawable(DRI2DrawablePtr > > pPriv); > > > > /** > > + * Provides access to ScreenPtr trough DRI2DrawablePtr > > + * > > + * \param pPriv DRI2 private drawable > > + * \return valid pointer to Screen > > + */ > > +extern _X_EXPORT ScreenPtr DRI2DrawableGetScreen(DRI2DrawablePtr pPriv); > > + > > +/** > > * Provides access to DRI2DrawablePtr trough DrawablePtr > > * > > * \param pDraw drawable pointer > > You've changed the driver DRI2 interface here in a way that could cause > old drivers to crash the server. Shouldn't you also bump > DRI2INFOREC_VERSION and refuse to load old drivers in DRI2ScreenInit? >
Right. I forgot the ABI bump. I think it is possible to support older ABI if I add a bit code server. But I suspect that refusing to load older drivers would be more popular. But in any case I have to fix that newer driver can be used with older server. _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
