[PATCH 3/9] dri2: Change driver interface to support DRI2Drawable
Christopher James Halse Rogers
christopher.halse.rogers at canonical.com
Thu Feb 3 18:33:23 PST 2011
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?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110204/949d44a1/attachment.pgp>
More information about the xorg-devel
mailing list