Re: New DRM driver model - gets rid of DRM() macros!

Keith Whitwell
2004-09-29T14:36:14Z

Jon Smirl wrote:
>
> Drivers provide these callbacks......
>
> struct drm_driver_fn {
> u32 driver_features;
> int dev_priv_size;
> int permanent_maps;
> drm_ioctl_desc_t *ioctls;
> int num_ioctls;

> int (*preinit)(struct drm_device *, unsigned long flags);
> void (*prerelease)(struct drm_device *, struct file *filp);
> void (*pretakedown)(struct drm_device *);
> int (*postcleanup)(struct drm_device *);
> int (*presetup)(struct drm_device *);
> int (*postsetup)(struct drm_device *);
> int (*dma_ioctl)( DRM_IOCTL_ARGS );
> /* these are opposites at the moment */
> int (*open_helper)(struct drm_device *, drm_file_t *);
> void (*free_filp_priv)(struct drm_device *, drm_file_t *);

> void (*release)(struct drm_device *, struct file *filp);
> void (*dma_ready)(struct drm_device *);

Is this used by any driver?

> int (*dma_quiescent)(struct drm_device *);

> int (*context_ctor)(struct drm_device *dev, int context);
> int (*context_dtor)(struct drm_device *dev, int context);
> int (*kernel_context_switch)(struct drm_device *dev, int old, int new);
> int (*kernel_context_switch_unlock)(struct drm_device *dev);

The whole context thing in the kernel is pretty much cruft. The gamma module
used to rely on it, maybe the ffb module if that still exists? It would be
good to see this disappear.

Though the drivers don't rely on it, I don't know if the server-side code
persists in setting it up regardless, which might make it hard to get rid of.

> int (*vblank_wait)(struct drm_device *dev, unsigned int *sequence);
> /* these have to be filled in */
> int (*postinit)(struct drm_device *, unsigned long flags);

Maybe move this up with the other init/cleanup functions so that they are
visibly grouped? Can the large number of init/cleanup functions be
rationalized in some way?

> irqreturn_t (*irq_handler)( DRM_IRQ_ARGS );
> void (*irq_preinstall)(struct drm_device *dev);
> void (*irq_postinstall)(struct drm_device *dev);
> void (*irq_uninstall)(struct drm_device *dev);

> void (*reclaim_buffers)(struct file *filp);

Maybe rename this to dma_reclaim_buffers() to make clear what code it is
associated with?

> unsigned long (*get_map_ofs)(drm_map_t *map);
> unsigned long (*get_reg_ofs)(struct drm_device *dev);
> void (*set_version)(struct drm_device *dev, drm_set_version_t *sv);
> int (*version)(drm_version_t *version);
> };
>