lkml.org 
[lkml]   [2004]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [rfc][patch] DRM initial function table support.
On Tue, Aug 31, 2004 at 02:11:11PM +0100, Dave Airlie wrote:

> Okay this is the first driver function table patch for the DRM, something
> similar has been running in DRM CVS for a month or so (underneath another
> 5-10 patches removing all the rest of the macros..)

I like this. Lots.
Mostly because of this..

(16:14:11:davej@delerium:~)$ grep ^-# foo.diff | wc -l
87

It's like someone revoked the drm folks preprocessor license 8-)

> One thing we've discussed on dri-devel was changing the
> if (dev->fn_tbl.function)
> dev->fn_tbl.function();
>
> to something else but in future patches I've had slightly more complex
> checks to do and this check looks to be the most obvious from a
> readability point of view, if the driver supports the function call it..

Looks ok, and pretty much matches similar conventions used in other
parts of the kernel.

> The other option was to use default noop fns, this also caused issues
> later on with some places where the code did more than just call the
> function..
>
> So please any comments on it?

One thing that would make things even nicer would be..

instead of this..

> +void gamma_driver_register_fns(drm_device_t *dev)
> +{
> + dev->fn_tbl.preinit = gamma_driver_preinit;
> + dev->fn_tbl.pretakedown = gamma_driver_pretakedown;
> + dev->fn_tbl.dma_ready = gamma_driver_dma_ready;
> + dev->fn_tbl.dma_quiescent = gamma_driver_dma_quiescent;
> + dev->fn_tbl.dma_flush_block_and_flush = gamma_flush_block_and_flush;
> + dev->fn_tbl.dma_flush_unblock = gamma_flush_unblock;
> +}

having a per-driver struct with regular C99 initialisers..

struct gamma_driver_fntbl {
.preinit = gamma_driver_preinit,
.pretakedown = gamma_driver_pretakedown,
.dma_ready = gamma_driver_dma_ready,
.dma_quiescent = gamma_driver_dma_quiescent,
.dma_flush_block_and_flush = gamma_flush_block_and_flush,
.dma_flush_unblock = gamma_flush_unblock,
};

Thanks for doing this work, it really is starting to look a little more
like a Linux driver 8-)

Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:05    [W:0.054 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site