Re: [RFC 1/21] Add VIA DRM driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> commit 1fcf23d361375645d586756d126b436796ba4fba
> Author: James Simmons <jsimmons@xxxxxxxxxxxxx>
> Date:   Sat Jun 8 09:31:57 2013 -0400
>
>     via: New KMS ioctls and hardware to support.
>
>     Add new VIA pci ids to support newer hardware. Cleanup userspace
>     api structs to remove kernel types and add the new KMS ioctls we
>     will be supporting.

why remove all the __u32? seems to just undo
1d7f83d5ad6c30b385ba549c1c3a287cc872b7ae

and definitely shouldn't be in this patch.

Also I don't think its really necessary to add all the pci ids there,
just put them in the driver.

>
>  #define DRM_IOCTL_VIA_ALLOCMEM   DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
>  #define DRM_IOCTL_VIA_FREEMEM    DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
> @@ -86,6 +89,7 @@
>  #define DRM_IOCTL_VIA_FB_INIT    DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_FB_INIT, drm_via_fb_t)
>  #define DRM_IOCTL_VIA_MAP_INIT   DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_MAP_INIT, drm_via_init_t)
>  #define DRM_IOCTL_VIA_DEC_FUTEX   DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_DEC_FUTEX, drm_via_futex_t)
> +#define DRM_IOCTL_VIA_OLD_GEM_CREATE  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_OLD_GEM_CREATE, struct drm_via_gem_create)
>  #define DRM_IOCTL_VIA_DMA_INIT   DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_DMA_INIT, drm_via_dma_init_t)
>  #define DRM_IOCTL_VIA_CMDBUFFER          DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_CMDBUFFER, drm_via_cmdbuffer_t)
>  #define DRM_IOCTL_VIA_FLUSH      DRM_IO(  DRM_COMMAND_BASE + DRM_VIA_FLUSH)
> @@ -96,6 +100,13 @@
>  #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
>  #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
>
> +/* KMS ioctls */
> +#define DRM_IOCTL_VIA_GETPARAM    DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GETPARAM, struct drm_via_param)
> +#define DRM_IOCTL_VIA_SETPARAM    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_SETPARAM, struct drm_via_param)
> +#define DRM_IOCTL_VIA_GEM_CREATE  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> +#define DRM_IOCTL_VIA_GEM_WAIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_GEM_WAIT, struct drm_via_gem_wait)
> +#define DRM_IOCTL_VIA_GEM_STATE   DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_STATE, struct drm_via_gem_create)

why does gem_state take a gem_create struct? is it the same info it returns?

>  typedef struct drm_via_dmablit {
> -       __u32 num_lines;
> -       __u32 line_length;
> +       uint32_t num_lines;
> +       uint32_t line_length;
>
> -       __u32 fb_addr;
> -       __u32 fb_stride;
> +       uint32_t fb_addr;
> +       uint32_t fb_stride;
>
>         unsigned char *mem_addr;
> -       __u32 mem_stride;
> +       uint32_t  mem_stride;
>
> -       __u32 flags;
> +       int bounce_buffer;

^ totally wtf here? no explains.

>         int to_fb;
>
>         drm_via_blitsync_t sync;
>  } drm_via_dmablit_t;
>
> -struct via_file_private {
> -       struct list_head obj_list;
> +/* Ioctl to query kernel params:
> + */
> +#define VIA_PARAM_CHIPSET_ID           0
> +#define VIA_PARAM_REVISION_ID          1
> +
> +struct drm_via_param {
> +       uint64_t param;
> +       uint64_t value;
> +};
> +
> +struct drm_via_gem_create {
> +       /**
> +        * Requested size for the object.
> +        *
> +        * The (page-aligned) allocated size for the object will be returned.
> +        */
> +       uint64_t size;
> +
> +       /*
> +        * Place the memory at the proper byte alignment.
> +        */
> +       uint32_t alignment;
> +
> +       /**
> +        * Format of data i.e tile pitch, for linear it is zero
> +        */
> +       uint32_t pitch;
> +
> +       /**
> +        * Give hints where to allocate this object.
> +        */
> +       uint32_t domains;
> +
> +       /**
> +        * chmod values applied to a buffer.
> +        */
> +       uint32_t mode_t;
> +
> +       /**
> +        * Offset to start of memory region.
> +        */
> +       uint64_t offset;
> +
> +       /**
> +        * Returned handle need to mmap the buffer.
> +        */
> +       uint64_t map_handle;
> +
> +       /**
> +        * Returned handle for the object.
> +        *
> +        * Object handles are nonzero.
> +        */
> +       uint32_t handle;
> +
> +       /**
> +        * Padding for future expansion.
> +        */
> +       uint32_t pad1;
> +       uint64_t pad2;
> +       uint64_t pad3;
> +       uint64_t pad4;


Don't bother with padding here, if you need to create another ioctl,
create another ioctl.

This is just my initial interface pass, because mostly everything else
can be cleaned up in tree,
the ABI not so much.

Dave.
Dave.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux