> Hello Luc and Dave:
> Thank you very much for your comment on the UniChrome DRM. And sorry for
> the trouble I made. Based on your comment, we modify our UniChrome patch as
> below(I would like to call it Ver1.5 because it's for reference not for fomal
> submittion): . The attached Xorg.0.log.openchromeok.P4M800 file is the log
> file for the platform of P4M800/CN700 with the Ver 1.5 UniChrome DRM patch.
> The ver1.5 DRM can be init with OpenChrome driver under Ubuntu9.04+Kernel
> updated.
> Se will follow Thomas's suggestion to divide the the Ver 1.5 to more
> detail patches and resubmit in the near future. The following is the modified
> draft just for reference only. It's not for formal submition. Please kindly
> ignore the patch I submit on 10Sep. Sorry again for the trouble.
Generally how we do suspend/resume in old DRI1 drivers, is that on VT
switch the X driver tells the kernel to save things, and on VT enter another
ioctl is used to resume. This avoids the suspend/resume hooks getting
called by the kernel while the kernel driver is in an inconsistent or
uninitialised
state. There is also a chance that this patch will break and viafb users as
it binds to the PCI device which really you shouldn't be doing again
in a non-kms
driver.
Some comments inline to show where this goes wrong:
>
> +int via_driver_open(struct drm_device *dev, struct drm_file *priv)
> +{
> + priv->authenticated = 1;
> + return 0;
> +}
This seems wrong, totally utterly wrong, You look to be
bypassing all the drm auth security. You don't seriously ship
this sort of thing to customers?
> static struct drm_driver driver = {
> .driver_features =
> DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_HAVE_IRQ |
> DRIVER_IRQ_SHARED,
> + .open = via_driver_open,
> .load = via_driver_load,
> .unload = via_driver_unload,
> .context_dtor = via_final_context,
> @@ -66,6 +72,8 @@
> .pci_driver = {
> .name = DRIVER_NAME,
> .id_table = pciidlist,
> + .suspend = via_drm_suspend,
> + .resume = via_drm_resume,
Not something we want for non-kms drivers, though it can
be done in some cases generally though you'd want
to be restoring video in the kernel at least.
> @@ -95,6 +109,11 @@
> {
> drm_via_private_t *dev_priv;
> int ret = 0;
> + if (!associate) {
> + pci_set_drvdata(dev->pdev, dev);
> + dev->pdev->driver = &dev->driver->pci_driver;
> + associate = 1;
> + }
Setting drvdata here will most likely break viafb binding.
>
> dev_priv = kzalloc(sizeof(drm_via_private_t), GFP_KERNEL);
> if (dev_priv == NULL)
> @@ -106,14 +125,14 @@
>
> ret = drm_sman_init(&dev_priv->sman, 2, 12, 8);
> if (ret) {
> - kfree(dev_priv);
> + dev_priv = kzalloc(sizeof(drm_via_private_t), GFP_KERNEL);
^^ huh?
> return ret;
> }
>
> ret = drm_vblank_init(dev, 1);
> if (ret) {
> drm_sman_takedown(&dev_priv->sman);
> - kfree(dev_priv);
> + dev_priv = kzalloc(sizeof(drm_via_private_t), GFP_KERNEL);
^^ huh 2?
> return ret;
> }
>
> @@ -126,7 +145,25 @@
>
> drm_sman_takedown(&dev_priv->sman);
>
> - kfree(dev_priv);
> + dev_priv = kzalloc(sizeof(drm_via_private_t), GFP_KERNEL);
^ huh 3??
You'd think the second time they did this you'd wonder why this was needed.
This screams I'm hacking with to get around an oops but I've no idea
why.
> diff -ruN old/include/drm/via_drm.h new/include/drm/via_drm.h
> --- old/include/drm/via_drm.h 2009-09-11 17:36:37.000000000 +0800
> +++ new/include/drm/via_drm.h 2009-09-11 18:00:31.000000000 +0800
> @@ -24,8 +24,6 @@
> #ifndef _VIA_DRM_H_
> #define _VIA_DRM_H_
>
> -#include <linux/types.h>
Do not undo previous patches to the kernel.
> -
> /* WARNING: These defines must be the same as what the Xserver uses.
> * if you change them, you must change the defines in the Xserver.
> */
> @@ -53,6 +51,12 @@
> #define VIA_LOG_MIN_TEX_REGION_SIZE 16
> #endif
>
> +struct drm_via_info {
> + unsigned long AgpHandle;
> + unsigned long AgpSize;
> + unsigned long RegHandle;
> + unsigned long RegSize;
> +} ;
> #define VIA_UPLOAD_TEX0IMAGE 0x1 /* handled clientside */
> #define VIA_UPLOAD_TEX1IMAGE 0x2 /* handled clientside */
> #define VIA_UPLOAD_CTX 0x4
> @@ -69,7 +73,7 @@
> #define DRM_VIA_FB_INIT 0x03
> #define DRM_VIA_MAP_INIT 0x04
> #define DRM_VIA_DEC_FUTEX 0x05
> -#define NOT_USED
> +#define DRM_VIA_GET_INFO 0x06
> #define DRM_VIA_DMA_INIT 0x07
> #define DRM_VIA_CMDBUFFER 0x08
> #define DRM_VIA_FLUSH 0x09
> @@ -79,6 +83,9 @@
> #define DRM_VIA_WAIT_IRQ 0x0d
> #define DRM_VIA_DMA_BLIT 0x0e
> #define DRM_VIA_BLIT_SYNC 0x0f
> +#define DRM_VIA_AUTH_MAGIC 0x11
> +#define DRM_VIA_FLUSH_VIDEO 0x12
> +#define DRM_VIA_INIT_JUDGE 0x16
>
> #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 +93,8 @@
> #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_GET_INFO DRM_IOR(DRM_COMMAND_BASE + \
> + DRM_VIA_GET_INFO, struct drm_via_info)
> #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)
> @@ -93,8 +102,14 @@
> #define DRM_IOCTL_VIA_CMDBUF_SIZE DRM_IOWR( DRM_COMMAND_BASE +
> DRM_VIA_CMDBUF_SIZE, \
> drm_via_cmdbuf_size_t)
> #define DRM_IOCTL_VIA_WAIT_IRQ DRM_IOWR( DRM_COMMAND_BASE +
> DRM_VIA_WAIT_IRQ, drm_via_irqwait_t)
> +#define DRM_IOCTL_VIA_FLUSH_VIDEO DRM_IOW(DRM_COMMAND_BASE + \
> + DRM_VIA_FLUSH_VIDEO, struct drm_via_video_agp_cmd)
> #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)
> +#define DRM_IOCTL_VIA_AUTH_MAGIC DRM_IOW(DRM_COMMAND_BASE + \
> + DRM_VIA_AUTH_MAGIC, drm_auth_t)
> +#define DRM_IOCTL_VIA_INIT_JUDGE DRM_IOR(DRM_COMMAND_BASE + \
> + DRM_VIA_INIT_JUDGE, int)
>
> /* Indices into buf.Setup where various bits of state are mirrored per
> * context and per buffer. These can be fired at the card as a unit,
> @@ -114,21 +129,28 @@
> #define VIA_MEM_SYSTEM 2
> #define VIA_MEM_MIXED 3
> #define VIA_MEM_UNKNOWN 4
> +#define VIA_MEM_VIDEO_SAVE 2 /*For video memory need to be saved in
> ACPI */
> +
> +enum drm_agp_type {
> + AGP_RING_BUFFER,
> + AGP_DOUBLE_BUFFER,
> + DISABLED
> +};
>
> typedef struct {
> - __u32 offset;
> - __u32 size;
> + uint32_t offset;
> + uint32_t size;
> } drm_via_agp_t;
same here, undoing previous patches. and numerous other places.
> +struct drm_via_video_agp_cmd {
> + u32 offset;
> + u32 cmd_size;
> + u32 buffer_size;
> +} ;
> +
should be __u32. also should probably have a __u32 pad.
Dave.
------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
