On Thu, Aug 19, 2004 at 03:26:07PM -0700, Erdi Chen wrote:
> This patch adds two new ioctl's to the VIA Unichrome/Pro DRM driver:
>       DRM_IOCTL_VIA_DMA_INIT
>       DRM_IOCTL_VIA_CMDBUFFER
> 
> The first call sets up an area in AGP memory that will be used as the 
> ring buffer.  The second call copies a command buffer from user space 
> memory to the ring buffer.

A few comments to the coding style - nothing to the functionality.
[inlining your patch would make it soo much easier to review and comment]

1) Defines are mixed casisng. Usual style is upper case only except if
casing match definitions in datasheet.

2) via_dma.c - remove emacs tagging "-*- linux-c -*-

3) What's the purpose of "#define __NO_VERSION__"

4) viaCheckDma => via_check_dma

5) via_dma_cleanup always return 0. No need to return anything.
   - Also drop extern in prototype in .h file

6) SetRag2DAGP 
   - no mixed casing
   - why is vb casted to uint32_t* - vb is of correct type.
   - Put the define close to the (sole) user

7) viaAlingBuffer
   - no mixed casing in name
   - use tabs for indention

8) via_cmdbuf_wait
   - no mixed casing in name
   - use tabs for indention

9) In general use tabs for indention

10) What doeis this do?
    qwPadCount = (CMDBUF_ALIGNMENT_SIZE>>3) -
                 ((((uint32_t)vb) & CMDBUF_ALIGNMENT_MASK) >> 3);
    There is _no_ guarantee a pointer will fir into uint_32
    Above construction used in several places.

11) via_drm.h
        - Drop typedef of drm_via_dma_init
                - Linux style is to _not_ hide when you work with a struct
        - Same for drm_via_cmdbuffer_t

12) Comment about typedef is also valid for via_drv.h

13) unsigned int dmaLow
    - I'm suspicious this is OK on 64 bit platforms?


        Sam


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285
--
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to