[PATCH 3/9] dri2: Change driver interface to support DRI2Drawable
Pauli Nieminen
ext-pauli.nieminen at nokia.com
Fri Feb 4 05:03:07 PST 2011
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 <ext-pauli.nieminen at nokia.com>
> >
> > 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 <ext-pauli.nieminen at nokia.com>
> > ---
> > 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.
More information about the xorg-devel
mailing list