[PATCH] Add EXA support

Michel Dänzer michel at daenzer.net
Fri Jul 6 03:52:13 PDT 2012


On Don, 2012-07-05 at 17:51 -0700, Connor Behan wrote: 
> For real this time. This allows the r128 driver to continue having 2D

That this patch is for the r128 driver should be mentioned on the
subject line (even if it was submitted to the xorg-driver-ati list :),
e.g. as '[PATCH xf86-video-r128] ...'

OTOH it doesn't need to be mentioned in the Git commit log, which is
everything between the Subject: line and the '---' line. You can include
additional information for the patch submission between the latter and
the actual patch.

BTW, as you seem to be the most active r128 driver developer ATM, you
should consider applying for a freedesktop.org account and maintaining
the driver in Git.


> acceleration without XAA. Implemented hooks are Solid, Copy and
> Composite. They appear to pass all rendercheck tests, except the
> gradient test which XAA also fails. Tested on multiple color depths,
> with and without DRI, with and without the composite extension. Hardware
> cursor, Xvideo and page flipping are supported as well.
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=47866
> 
> Signed-off-by: Connor Behan <connor.behan at gmail.com>

This patch introduces the following compiler warnings, which may
indicate problems:

r128_cursor.c: In function 'R128CursorInit':
r128_cursor.c:332:35: warning: 'y2' may be used uninitialized in this function [-Wuninitialized]
r128_cursor.c:331:37: warning: 'y1' may be used uninitialized in this function [-Wuninitialized]
r128_cursor.c:332:30: warning: 'x2' may be used uninitialized in this function [-Wuninitialized]
r128_cursor.c:331:32: warning: 'x1' may be used uninitialized in this function [-Wuninitialized]

r128_driver.c: In function 'R128VerboseInitEXA':
r128_driver.c:2200:5: warning: implicit declaration of function 'R128EXAInit' [-Wimplicit-function-declaration]
r128_driver.c: In function 'R128VerboseInitEXA':
r128_driver.c:2211:1: warning: control reaches end of non-void function [-Wreturn-type]

r128_driver.c: In function 'R128ScreenInit':
r128_driver.c:2555:49: warning: 'osArea' may be used uninitialized in this function [-Wuninitialized]
r128_driver.c:2562:16: warning: 'y2' may be used uninitialized in this function [-Wuninitialized]
r128_driver.c:2562:16: warning: 'y1' may be used uninitialized in this function [-Wuninitialized]
r128_driver.c:2562:16: warning: 'x2' may be used uninitialized in this function [-Wuninitialized]
r128_driver.c:2562:16: warning: 'x1' may be used uninitialized in this function [-Wuninitialized]
r128_driver.c:2555:21: warning: 'fbarea' may be used uninitialized in this function [-Wuninitialized]

r128_video.c: In function 'R128AllocateMemory':
r128_video.c:614:10: warning: return makes integer from pointer without a cast [enabled by default]
r128_video.c: In function 'R128PutImage':
r128_video.c:825:36: warning: variable 'bpp' set but not used [-Wunused-but-set-variable]
r128_video.c: In function 'R128VideoTimerCallback':
r128_video.c:1065:9: warning: implicit declaration of function 'exaOffscreenAreaFree' [-Wimplicit-function-declaration]

r128_exa_render.c: In function 'R128CheckCompositeTexture':
r128_exa_render.c:151:17: warning: unused variable 'info' [-Wunused-variable]
r128_exa_render.c: In function 'R128CCECheckComposite':
r128_exa_render.c:193:17: warning: unused variable 'info' [-Wunused-variable]


> +#ifdef USE_EXA    
> +    else {
> +	osArea = exaOffscreenAlloc(pScreen, width * height, 16,
> +				   TRUE, NULL, NULL);
> +
> +	if (osArea) {
> +	    x1 = osArea->offset % width_bytes;
> +	    x2 = (osArea->offset + osArea->size) % width_bytes;
> +	    y1 = osArea->offset / width_bytes;
> +	    y2 = (osArea->offset + osArea->size) / width_bytes;
> +	}

Seems weird retrofitting the linear range from exaOffscreenAlloc into a
rectangle, just to turn it back into a linear range. 

> -	info->cursor_start    = R128_ALIGN((fbarea->box.x1
> -					    + width * fbarea->box.y1)
> -					   * info->CurrentLayout.pixel_bytes, 16);
> -	info->cursor_end      = info->cursor_start + size;
> +	info->cursor_start = x1 * cpp + y1 * pScrn->virtualX * cpp;
> +	info->cursor_end = x2 * cpp + y2 * pScrn->virtualX * cpp;

FWIW though, this needs to multiply by width_bytes instead of
pScrn->virtualX * cpp.


> @@ -305,6 +305,9 @@ static void R128EnterServer(ScreenPtr pScreen)
>      R128InfoPtr info = R128PTR(pScrn);
>  
>      if (info->accel) info->accel->NeedToSync = TRUE;
> +#ifdef USE_EXA
> +    if (info->ExaDriver) exaMarkSync(pScreen);
> +#endif
>  }
>  
>  /* Called when the X server goes to sleep to allow the X server's
> @@ -331,6 +334,9 @@ static void R128LeaveServer(ScreenPtr pScreen)
>  
>  	info->CCEInUse = FALSE;
>      }
> +#ifdef USE_EXA
> +    if (info->ExaDriver) exaMarkSync(pScreen);
> +#endif
>  }
>  
>  /* Contexts can be swapped by the X server if necessary.  This callback

EnterServer should always be called before any server rendering
operations, so the exaMarkSync call in LeaveServer is probably
superfluous.


> @@ -1471,6 +1516,10 @@ static void R128DRITransitionTo3d(ScreenPtr pScreen)
>  
>      if (info->cursor_start)
>          xf86ForceHWCursor(pScreen, TRUE);
> +
> +#ifdef USE_EXA
> +    info->state_2d.composite_setup = FALSE;
> +#endif

I think this should also be done in EnterServer, as the 3D driver may
change 3D state between LeaveServer and EnterServer.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer


More information about the xorg-devel mailing list