[Mesa-dev] [PATCH 3/3] radeon/winsys: add async dma infrastructure

[Christian König]

On 04.01.2013 23:19, j.glisse at gmail.com wrote:
[SNIP]
> diff --git a/src/gallium/drivers/r300/r300_emit.c b/src/gallium/drivers/r300/r300_emit.c
> index d1ed4b3..c824821 100644
> --- a/src/gallium/drivers/r300/r300_emit.c
> +++ b/src/gallium/drivers/r300/r300_emit.c
> @@ -1184,7 +1184,8 @@ validate:
>               assert(tex && tex->buf && "cbuf is marked, but NULL!");
>               r300->rws->cs_add_reloc(r300->cs, tex->cs_buf,
>                                       RADEON_USAGE_READWRITE,
> -                                    r300_surface(fb->cbufs[i])->domain);
> +                                    r300_surface(fb->cbufs[i])->domain,
> +                                    RADEON_RING_DMA);

??? DMA ring on R300? At least on first glance that looks quite odd, 
should probably be GFX ring instead.

>           }
>           /* ...depth buffer... */
>           if (fb->zsbuf) {
> @@ -1192,7 +1193,8 @@ validate:
>               assert(tex && tex->buf && "zsbuf is marked, but NULL!");
>               r300->rws->cs_add_reloc(r300->cs, tex->cs_buf,
>                                       RADEON_USAGE_READWRITE,
> -                                    r300_surface(fb->zsbuf)->domain);
> +                                    r300_surface(fb->zsbuf)->domain,
> +                                    RADEON_RING_DMA);

Same here and repeats on a couple of more places.
[SNIP]

> diff --git a/src/gallium/winsys/radeon/drm/radeon_winsys.h b/src/gallium/winsys/radeon/drm/radeon_winsys.h
> index 16536dc..5ff463e 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_winsys.h
> +++ b/src/gallium/winsys/radeon/drm/radeon_winsys.h
> @@ -43,11 +43,13 @@
>   #include "pipebuffer/pb_buffer.h"
>   #include "libdrm/radeon_surface.h"
>   
> -#define RADEON_MAX_CMDBUF_DWORDS (16 * 1024)
> +#define RADEON_MAX_CMDBUF_DWORDS        (16 * 1024)
>   
> -#define RADEON_FLUSH_ASYNC		(1 << 0)
> -#define RADEON_FLUSH_KEEP_TILING_FLAGS	(1 << 1) /* needs DRM 2.12.0 */
> -#define RADEON_FLUSH_COMPUTE		(1 << 2)
> +#define RADEON_FLUSH_ASYNC              (1 << 0)
> +#define RADEON_FLUSH_KEEP_TILING_FLAGS  (1 << 1) /* needs DRM 2.12.0 */
> +#define RADEON_FLUSH_COMPUTE            (1 << 2)
> +#define RADEON_FLUSH_DMA                (1 << 3)
> +#define RADEON_FLUSH_GFX                (1 << 4)
>   
>   /* Tiling flags. */
>   enum radeon_bo_layout {
> @@ -137,12 +139,19 @@ enum chip_class {
>       TAHITI,
>   };
>   
> +enum radeon_ring_type {
> +    RADEON_RING_PM4 = 0,
> +    RADEON_RING_DMA = 1,
> +};
> +

Don't use PM4 as identifier here, the PM4 packet format is used for 
other ring types beside GFX/Compute as well, but those rings can't 
necessary execute GFX/Compute commands.

>   struct winsys_handle;
>   struct radeon_winsys_cs_handle;
>   
>   struct radeon_winsys_cs {
> -    unsigned cdw;  /* Number of used dwords. */
> -    uint32_t *buf; /* The command buffer. */
> +    unsigned                    cdw;      /* Number of used dwords. */
> +    uint32_t                    *buf;     /* The command buffer. */
> +    unsigned                    dma_cdw;  /* Number of used dwords. */
> +    uint32_t                    *dma_buf; /* The command buffer. */
>   };

Why like this? Can't we just have separate instances of the 
radeon_winsys_cs structure for each ring type we are dealing with?

The rest looks quite good,
Christian.