radeon_ms: command buffer validation use array of function pointer
authorJerome Glisse <glisse@freedesktop.org>
Tue, 8 Apr 2008 00:18:14 +0000 (02:18 +0200)
committerJerome Glisse <glisse@freedesktop.org>
Tue, 8 Apr 2008 00:18:14 +0000 (02:18 +0200)
linux-core/Makefile.kernel
linux-core/amd_cbuffer.h [deleted symlink]
linux-core/amd_legacy.h [new symlink]
linux-core/amd_legacy_cbuffer.c [new symlink]
shared-core/amd.h
shared-core/amd_legacy.h [moved from shared-core/amd_cbuffer.h with 68% similarity]
shared-core/amd_legacy_cbuffer.c [new file with mode: 0644]
shared-core/radeon_ms.h
shared-core/radeon_ms_drm.c
shared-core/radeon_ms_exec.c

index ad62eff..91decfc 100644 (file)
@@ -40,7 +40,8 @@ radeon_ms-objs := radeon_ms_drv.o radeon_ms_drm.o radeon_ms_family.o \
                radeon_ms_cp.o radeon_ms_cp_mc.o radeon_ms_i2c.o \
                radeon_ms_output.o radeon_ms_crtc.o radeon_ms_fb.o \
                radeon_ms_exec.o radeon_ms_gpu.o radeon_ms_dac.o \
-               radeon_ms_properties.o radeon_ms_rom.o radeon_ms_combios.o
+               radeon_ms_properties.o radeon_ms_rom.o radeon_ms_combios.o \
+               amd_legacy_cbuffer.o
 sis-objs    := sis_drv.o sis_mm.o
 ffb-objs    := ffb_drv.o ffb_context.o
 savage-objs := savage_drv.o savage_bci.o savage_state.o
diff --git a/linux-core/amd_cbuffer.h b/linux-core/amd_cbuffer.h
deleted file mode 120000 (symlink)
index 780b9d8..0000000
+++ /dev/null
@@ -1 +0,0 @@
-../shared-core/amd_cbuffer.h
\ No newline at end of file
diff --git a/linux-core/amd_legacy.h b/linux-core/amd_legacy.h
new file mode 120000 (symlink)
index 0000000..1a7786f
--- /dev/null
@@ -0,0 +1 @@
+../shared-core/amd_legacy.h
\ No newline at end of file
diff --git a/linux-core/amd_legacy_cbuffer.c b/linux-core/amd_legacy_cbuffer.c
new file mode 120000 (symlink)
index 0000000..eab329b
--- /dev/null
@@ -0,0 +1 @@
+../shared-core/amd_legacy_cbuffer.c
\ No newline at end of file
index ac6195e..31cf3ef 100644 (file)
 #ifndef __AMD_H__
 #define __AMD_H__
 
+/* struct amd_cbuffer are for command buffer, this is the structure passed
+ * around during command validation (ie check that user have the right to
+ * execute the given command).
+ */
+struct amd_cbuffer_arg
+{
+       struct list_head         list;
+       struct drm_buffer_object *buffer;
+       int32_t                  dw_id;
+};
+
+struct amd_cbuffer
+{
+       uint32_t                 *cbuffer;
+       uint32_t                 cbuffer_dw_count;
+       struct amd_cbuffer_arg   arg_unused;
+       struct amd_cbuffer_arg   arg_used;
+       struct amd_cbuffer_arg   *args;
+       void                     *driver;
+};
+
+struct amd_cbuffer_checker
+{
+       uint32_t numof_p0_checkers;
+       uint32_t numof_p3_checkers;
+       int (*check)(struct drm_device *dev, struct amd_cbuffer *cbuffer);
+       int (**check_p0)(struct drm_device *dev, struct amd_cbuffer *cbuffer,
+                        int dw_id, int reg);
+       int (**check_p3)(struct drm_device *dev, struct amd_cbuffer *cbuffer,
+                        int dw_id, int op, int count);
+};
+
+struct amd_cbuffer_arg *
+amd_cbuffer_arg_from_dw_id(struct amd_cbuffer_arg *head, uint32_t dw_id);
+int amd_cbuffer_check(struct drm_device *dev, struct amd_cbuffer *cbuffer);
+
+
 /* struct amd_fb amd is for storing amd framebuffer informations
  */
 struct amd_fb
similarity index 68%
rename from shared-core/amd_cbuffer.h
rename to shared-core/amd_legacy.h
index bca0575..92997dc 100644 (file)
  * Authors:
  *    Jerome Glisse <glisse@freedesktop.org>
  */
-#ifndef __AMD_CBUFFER_H__
-#define __AMD_CBUFFER_H__
+#ifndef __AMD_LEGACY_H__
+#define __AMD_LEGACY_H__
 
-/* struct amd_cbuffer are for command buffer, this is the structure passed
- * around during command validation (ie check that user have the right to
- * execute the given command).
- */
-
-struct amd_cbuffer_arg
-{
-       struct list_head         list;
-       struct drm_buffer_object *buffer;
-       int32_t                  dw_id;
-};
-
-struct amd_cbuffer
-{
-       uint32_t                 *cbuffer;
-       uint32_t                 cbuffer_dw_count;
-       struct amd_cbuffer_arg   arg_unused;
-       struct amd_cbuffer_arg   arg_used;
-       struct amd_cbuffer_arg   *args;
-};
+int amd_legacy_cbuffer_destroy(struct drm_device *dev);
+int amd_legacy_cbuffer_initialize(struct drm_device *dev);
 
 #endif
diff --git a/shared-core/amd_legacy_cbuffer.c b/shared-core/amd_legacy_cbuffer.c
new file mode 100644 (file)
index 0000000..f1b7a44
--- /dev/null
@@ -0,0 +1,306 @@
+/*
+ * Copyright 2008 Jérôme Glisse
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Jerome Glisse <glisse@freedesktop.org>
+ */
+#include "radeon_ms.h"
+#include "amd.h"
+
+#define RADEON_DST_Y_X                      0x1438
+#define RADEON_SRC_Y_X                      0x1434
+#define RADEON_DP_CNTL                      0x16c0
+#define RADEON_DP_GUI_MASTER_CNTL           0x146c
+#define RADEON_DP_BRUSH_BKGD_CLR            0x1478
+#define RADEON_DP_BRUSH_FRGD_CLR            0x147c
+#define RADEON_DP_WRITE_MASK                0x16cc
+#define RADEON_DST_PITCH_OFFSET             0x142c
+#define RADEON_SRC_PITCH_OFFSET             0x1428
+#define RADEON_DST_HEIGHT_WIDTH             0x143c
+
+struct legacy_check
+{
+       /* for 2D operations */
+       uint32_t                dp_gui_master_cntl;
+       uint32_t                dst_offset;
+       uint32_t                dst_pitch;
+       struct amd_cbuffer_arg  *dst;
+       uint32_t                dst_x;
+       uint32_t                dst_y;
+       uint32_t                dst_h;
+       uint32_t                dst_w;
+       struct amd_cbuffer_arg  *src;
+       uint32_t                src_pitch;
+       uint32_t                src_x;
+       uint32_t                src_y;
+};
+
+static int check_blit(struct drm_device *dev, struct amd_cbuffer *cbuffer)
+{
+       struct legacy_check *legacy_check;
+       long bpp, start, end;
+       
+       legacy_check = (struct legacy_check *)cbuffer->driver;
+       /* check that gui master cntl have been set */
+       if (legacy_check->dp_gui_master_cntl == 0xffffffff) {
+               return -EINVAL;
+       }
+       switch ((legacy_check->dp_gui_master_cntl >> 8) & 0xf) {
+       case 2:
+               bpp = 1;
+               break;
+       case 3:
+       case 4:
+               bpp = 2;
+               break;
+       case 6:
+               bpp = 4;
+               break;
+       default:
+               return -EINVAL;
+       }
+       /* check that a destination have been set */
+       if (legacy_check->dst == (void *)-1) {
+               return -EINVAL;
+       }
+       if (legacy_check->dst_pitch == 0xffffffff) {
+               return -EINVAL;
+       }
+       if (legacy_check->dst_x == 0xffffffff) {
+               return -EINVAL;
+       }
+       if (legacy_check->dst_y == 0xffffffff) {
+               return -EINVAL;
+       }
+       if (legacy_check->dst_w == 0xffffffff) {
+               return -EINVAL;
+       }
+       if (legacy_check->dst_h == 0xffffffff) {
+               return -EINVAL;
+       }
+       /* compute start offset of blit */
+       start = legacy_check->dst_pitch * legacy_check->dst_y +
+               legacy_check->dst_x * bpp;
+       /* compute end offset of blit */
+       end = legacy_check->dst_pitch * legacy_check->dst_h +
+             legacy_check->dst_w * bpp;
+       /* FIXME: check that end offset is inside dst bo */
+
+       /* check that a destination have been set */
+       if (legacy_check->dp_gui_master_cntl & 1) {
+               if (legacy_check->src == (void *)-1) {
+                       return -EINVAL;
+               }
+               if (legacy_check->src_pitch == 0xffffffff) {
+                       return -EINVAL;
+               }
+               if (legacy_check->src_x == 0xffffffff) {
+                       return -EINVAL;
+               }
+               if (legacy_check->src_y == 0xffffffff) {
+                       return -EINVAL;
+               }
+               /* compute start offset of blit */
+               start = legacy_check->src_pitch * legacy_check->src_y +
+                       legacy_check->src_x * bpp;
+               /* compute end offset of blit */
+               end = legacy_check->src_pitch * legacy_check->dst_h +
+                     legacy_check->dst_w * bpp + start;
+               /* FIXME: check that end offset is inside src bo */
+       }
+       return 0;
+}
+
+static int p0_dp_gui_master_cntl(struct drm_device *dev,
+                                struct amd_cbuffer *cbuffer,
+                                int dw_id, int reg)
+{
+       struct legacy_check *legacy_check;
+       
+       legacy_check = (struct legacy_check *)cbuffer->driver;
+       legacy_check->dp_gui_master_cntl = cbuffer->cbuffer[dw_id];
+       /* we only accept src data type to be same as dst */
+       if (((legacy_check->dp_gui_master_cntl >> 12) & 0x3) != 3) {
+               return -EINVAL;
+       }
+       return 0;
+}
+
+static int p0_dst_pitch_offset(struct drm_device *dev,
+                              struct amd_cbuffer *cbuffer,
+                              int dw_id, int reg)
+{
+       struct legacy_check *legacy_check;
+       uint32_t gpu_addr;
+       int ret;
+       
+       legacy_check = (struct legacy_check *)cbuffer->driver;
+       legacy_check->dst_pitch = ((cbuffer->cbuffer[dw_id] >> 22) & 0xff) << 6;
+       legacy_check->dst = amd_cbuffer_arg_from_dw_id(&cbuffer->arg_unused,
+                                                      dw_id);
+       if (legacy_check->dst == NULL) {
+               return -EINVAL;
+       }
+       ret = radeon_ms_bo_get_gpu_addr(dev, &legacy_check->dst->buffer->mem,
+                                       &gpu_addr);
+       if (ret) {
+               return -EINVAL;
+       }
+       cbuffer->cbuffer[dw_id] &= 0xffc00000;
+       cbuffer->cbuffer[dw_id] |= (gpu_addr >> 10);
+       return 0;
+}
+
+static int p0_src_pitch_offset(struct drm_device *dev,
+                              struct amd_cbuffer *cbuffer,
+                              int dw_id, int reg)
+{
+       struct legacy_check *legacy_check;
+       uint32_t gpu_addr;
+       int ret;
+       
+       legacy_check = (struct legacy_check *)cbuffer->driver;
+       legacy_check->src_pitch = ((cbuffer->cbuffer[dw_id] >> 22) & 0xff) << 6;
+       legacy_check->src = amd_cbuffer_arg_from_dw_id(&cbuffer->arg_unused,
+                                                      dw_id);
+       if (legacy_check->dst == NULL) {
+               return -EINVAL;
+       }
+       ret = radeon_ms_bo_get_gpu_addr(dev, &legacy_check->src->buffer->mem,
+                                       &gpu_addr);
+       if (ret) {
+               return -EINVAL;
+       }
+       cbuffer->cbuffer[dw_id] &= 0xffc00000;
+       cbuffer->cbuffer[dw_id] |= (gpu_addr >> 10);
+       return 0;
+}
+
+static int p0_dst_y_x(struct drm_device *dev,
+                     struct amd_cbuffer *cbuffer,
+                     int dw_id, int reg)
+{
+       struct legacy_check *legacy_check;
+       
+       legacy_check = (struct legacy_check *)cbuffer->driver;
+       legacy_check->dst_x = cbuffer->cbuffer[dw_id] & 0xffff;
+       legacy_check->dst_y = (cbuffer->cbuffer[dw_id] >> 16) & 0xffff;
+       return 0;
+}
+
+static int p0_src_y_x(struct drm_device *dev,
+                     struct amd_cbuffer *cbuffer,
+                     int dw_id, int reg)
+{
+       struct legacy_check *legacy_check;
+       
+       legacy_check = (struct legacy_check *)cbuffer->driver;
+       legacy_check->src_x = cbuffer->cbuffer[dw_id] & 0xffff;
+       legacy_check->src_y = (cbuffer->cbuffer[dw_id] >> 16) & 0xffff;
+       return 0;
+}
+
+static int p0_dst_h_w(struct drm_device *dev,
+                     struct amd_cbuffer *cbuffer,
+                     int dw_id, int reg)
+{
+       struct legacy_check *legacy_check;
+       
+       legacy_check = (struct legacy_check *)cbuffer->driver;
+       legacy_check->dst_w = cbuffer->cbuffer[dw_id] & 0xffff;
+       legacy_check->dst_h = (cbuffer->cbuffer[dw_id] >> 16) & 0xffff;
+       return check_blit(dev, cbuffer);
+}
+
+static int legacy_cbuffer_check(struct drm_device *dev,
+                               struct amd_cbuffer *cbuffer)
+{
+       struct legacy_check legacy_check;
+
+       memset(&legacy_check, 0xff, sizeof(struct legacy_check));
+       cbuffer->driver = &legacy_check;
+       return amd_cbuffer_check(dev, cbuffer);
+}
+
+int amd_legacy_cbuffer_destroy(struct drm_device *dev)
+{
+       struct drm_radeon_private *dev_priv = dev->dev_private;
+
+       dev_priv->cbuffer_checker.check = NULL;
+       if (dev_priv->cbuffer_checker.numof_p0_checkers) {
+               drm_free(dev_priv->cbuffer_checker.check_p0,
+                        dev_priv->cbuffer_checker.numof_p0_checkers *
+                        sizeof(void*), DRM_MEM_DRIVER);
+               dev_priv->cbuffer_checker.numof_p0_checkers = 0;
+       }
+       if (dev_priv->cbuffer_checker.numof_p3_checkers) {
+               drm_free(dev_priv->cbuffer_checker.check_p3,
+                        dev_priv->cbuffer_checker.numof_p3_checkers *
+                        sizeof(void*), DRM_MEM_DRIVER);
+               dev_priv->cbuffer_checker.numof_p3_checkers = 0;
+       }
+       return 0;
+}
+
+int amd_legacy_cbuffer_initialize(struct drm_device *dev)
+{
+       struct drm_radeon_private *dev_priv = dev->dev_private;
+       struct amd_cbuffer_checker *checker = &dev_priv->cbuffer_checker;
+       long size;
+
+       /* packet 0 */
+       checker->numof_p0_checkers = 0x5000 >> 2;
+       size = checker->numof_p0_checkers * sizeof(void*);
+       checker->check_p0 = drm_alloc(size, DRM_MEM_DRIVER);
+       if (checker->check_p0 == NULL) {
+               amd_legacy_cbuffer_destroy(dev);
+               return -ENOMEM;
+       }
+       /* initialize to -1 */
+       memset(checker->check_p0, 0xff, size);
+
+       /* packet 3 */
+       checker->numof_p3_checkers = 20;
+       size = checker->numof_p3_checkers * sizeof(void*);
+       checker->check_p3 = drm_alloc(size, DRM_MEM_DRIVER);
+       if (checker->check_p3 == NULL) {
+               amd_legacy_cbuffer_destroy(dev);
+               return -ENOMEM;
+       }
+       /* initialize to -1 */
+       memset(checker->check_p3, 0xff, size);
+
+       /* initialize packet0 checker */
+       checker->check_p0[RADEON_DST_Y_X >> 2] = p0_dst_y_x;
+       checker->check_p0[RADEON_SRC_Y_X >> 2] = p0_src_y_x;
+       checker->check_p0[RADEON_DST_HEIGHT_WIDTH >> 2] = p0_dst_h_w;
+       checker->check_p0[RADEON_DST_PITCH_OFFSET >> 2] = p0_dst_pitch_offset;
+       checker->check_p0[RADEON_SRC_PITCH_OFFSET >> 2] = p0_src_pitch_offset;
+       checker->check_p0[RADEON_DP_GUI_MASTER_CNTL>>2] = p0_dp_gui_master_cntl;
+       checker->check_p0[RADEON_DP_BRUSH_FRGD_CLR >> 2] = NULL;
+       checker->check_p0[RADEON_DP_WRITE_MASK >> 2] = NULL;
+       checker->check_p0[RADEON_DP_CNTL >> 2] = NULL;
+
+       checker->check = legacy_cbuffer_check;
+       return 0;
+}
index bbd0477..ec26420 100644 (file)
@@ -36,7 +36,7 @@
 #include "radeon_ms_rom.h"
 #include "radeon_ms_properties.h"
 #include "amd.h"
-#include "amd_cbuffer.h"
+#include "amd_legacy.h"
 
 #define DRIVER_AUTHOR      "Jerome Glisse, Dave Airlie,  Gareth Hughes, "\
                           "Keith Whitwell, others."
@@ -329,6 +329,8 @@ struct drm_radeon_private {
        uint8_t                     cp_ready;
        uint8_t                     bus_ready;
        uint8_t                     write_back;
+       /* command buffer informations */
+       struct amd_cbuffer_checker  cbuffer_checker;
        /* abstract asic specific structures */
        struct radeon_ms_rom        rom;
        struct radeon_ms_properties properties;
index 869ccac..0d32792 100644 (file)
@@ -246,6 +246,13 @@ int radeon_ms_driver_load(struct drm_device *dev, unsigned long flags)
                return ret;
        }
 
+       /* initialze driver specific */
+       ret = amd_legacy_cbuffer_initialize(dev);
+       if (ret != 0) {
+               radeon_ms_driver_unload(dev);
+               return ret;
+       }
+
        if (dev->primary && dev->control) {
                DRM_INFO("[radeon_ms] control 0x%lx, render 0x%lx\n",
                          (long)dev->primary->device, (long)dev->control->device);
@@ -277,6 +284,9 @@ int radeon_ms_driver_unload(struct drm_device *dev)
        radeon_ms_outputs_restore(dev, &dev_priv->load_state);
        radeon_ms_connectors_destroy(dev);
        radeon_ms_outputs_destroy(dev);
+
+       /* shutdown specific driver */
+       amd_legacy_cbuffer_destroy(dev);
        
        /* shutdown cp engine */
        radeon_ms_cp_finish(dev);
index 32b55ea..28fcc18 100644 (file)
@@ -25,7 +25,7 @@
  *    Jerome Glisse <glisse@freedesktop.org>
  */
 #include "radeon_ms.h"
-#include "amd_cbuffer.h"
+#include "amd.h"
 
 static void radeon_ms_execbuffer_args_clean(struct drm_device *dev,
                                            struct amd_cbuffer *cbuffer,
@@ -118,155 +118,69 @@ out_err:
        return ret;
 }
 
-enum {
-       REGISTER_FORBIDDEN = 0,
-       REGISTER_SAFE,
-       REGISTER_SET_OFFSET,
-};
-static uint8_t _r3xx_register_right[0x5000 >> 2];
-
-static int amd_cbuffer_packet0_set_offset(struct drm_device *dev,
-                                         struct amd_cbuffer *cbuffer,
-                                         uint32_t reg, int dw_id,
-                                         struct amd_cbuffer_arg *arg)
-{
-       uint32_t gpu_addr;
-       int ret;
-
-       ret = radeon_ms_bo_get_gpu_addr(dev, &arg->buffer->mem, &gpu_addr);
-       if (ret) {
-               return ret;
-       }
-       switch (reg) {
-       default:
-               return -EINVAL;
-       }
-       return 0;
-}
-
-static struct amd_cbuffer_arg *
-amd_cbuffer_arg_from_dw_id(struct amd_cbuffer_arg *head, uint32_t dw_id)
+static int cbuffer_packet0_check(struct drm_device *dev,
+                                struct amd_cbuffer *cbuffer,
+                                int dw_id)
 {
-       struct amd_cbuffer_arg *arg;
-
-       list_for_each_entry(arg, &head->list, list) {
-               if (arg->dw_id == dw_id) {
-                       return arg;
-               }
-       }
-       /* no buffer at this dw index */
-       return NULL;
-}
-
-static int amd_cbuffer_packet0_check(struct drm_device *dev,
-                                    struct drm_file *file_priv,
-                                    struct amd_cbuffer *cbuffer,
-                                    int dw_id,
-                                    uint8_t *register_right)
-{
-       struct amd_cbuffer_arg *arg;
+       struct drm_radeon_private *dev_priv = dev->dev_private;
        uint32_t reg, count, r, i;
        int ret;
 
        reg = cbuffer->cbuffer[dw_id] & PACKET0_REG_MASK;
        count = (cbuffer->cbuffer[dw_id] & PACKET0_COUNT_MASK) >>
                PACKET0_COUNT_SHIFT;
+       if (reg + count > dev_priv->cbuffer_checker.numof_p0_checkers) {
+               return -EINVAL;
+       }
        for (r = reg, i = 0; i <= count; i++, r++) {
-               switch (register_right[i]) {
-               case REGISTER_FORBIDDEN:
+               if (dev_priv->cbuffer_checker.check_p0[r] == NULL) {
+                       continue;
+               }
+               if (dev_priv->cbuffer_checker.check_p0[r] == (void *)-1) {
+                       DRM_INFO("[radeon_ms] check_f: %d %d -1 checker\n",
+                                r, r << 2);
+                       return -EINVAL;
+               }
+               ret = dev_priv->cbuffer_checker.check_p0[r](dev, cbuffer,
+                                                           dw_id + i + 1, reg);
+               if (ret) {
+                       DRM_INFO("[radeon_ms] check_f: %d %d checker ret=%d\n",
+                                r, r << 2, ret);
                        return -EINVAL;
-               case REGISTER_SAFE:
-                       break;
-               case REGISTER_SET_OFFSET:
-                       arg = amd_cbuffer_arg_from_dw_id(&cbuffer->arg_unused,
-                                                        dw_id + i +1);
-                       if (arg == NULL) {
-                               return -EINVAL;
-                       }
-                       /* remove from unparsed list */
-                       list_del(&arg->list);
-                       list_add_tail(&arg->list, &cbuffer->arg_used.list);
-                       /* set the offset */
-                       ret =  amd_cbuffer_packet0_set_offset(dev, cbuffer,
-                                                             r, dw_id + i + 1,
-                                                             arg);
-                       if (ret) {
-                               return ret;
-                       }
-                       break;
                }
        }
        /* header + N + 1 dword passed test */
        return count + 2;
 }
 
-static int amd_cbuffer_packet3_check(struct drm_device *dev,
-                                    struct drm_file *file_priv,
-                                    struct amd_cbuffer *cbuffer,
-                                    int dw_id)
+static int cbuffer_packet3_check(struct drm_device *dev,
+                                struct amd_cbuffer *cbuffer,
+                                int dw_id)
 {
-       struct amd_cbuffer_arg *arg;
+       struct drm_radeon_private *dev_priv = dev->dev_private;
        uint32_t opcode, count;
-       uint32_t s_auth, s_mask;
-       uint32_t gpu_addr;
        int ret;
 
        opcode = (cbuffer->cbuffer[dw_id] & PACKET3_OPCODE_MASK) >>
                 PACKET3_OPCODE_SHIFT;
+       if (opcode > dev_priv->cbuffer_checker.numof_p3_checkers) {
+               return -EINVAL;
+       }
        count = (cbuffer->cbuffer[dw_id] & PACKET3_COUNT_MASK) >>
                PACKET3_COUNT_SHIFT;
-       switch (opcode) {
-       case PACKET3_OPCODE_NOP:
-               break;
-       case PACKET3_OPCODE_BITBLT:
-       case PACKET3_OPCODE_BITBLT_MULTI:
-               /* we only alow simple blit */
-               if (count != 5) {
-                       return -EINVAL;
-               }
-               s_mask = 0xf;
-               s_auth = 0x3;
-               if ((cbuffer->cbuffer[dw_id + 1] & s_mask) != s_auth) {
-                       return -EINVAL;
-               }
-               arg = amd_cbuffer_arg_from_dw_id(&cbuffer->arg_unused, dw_id+2);
-               if (arg == NULL) {
-                       return -EINVAL;
-               }
-               ret = radeon_ms_bo_get_gpu_addr(dev, &arg->buffer->mem,
-                                               &gpu_addr);
-               if (ret) {
-                       return ret;
-               }
-               gpu_addr = (gpu_addr >> 10) & 0x003FFFFF;
-               cbuffer->cbuffer[dw_id + 2] &= 0xFFC00000;
-               cbuffer->cbuffer[dw_id + 2] |= gpu_addr;
-
-               arg = amd_cbuffer_arg_from_dw_id(&cbuffer->arg_unused, dw_id+3);
-               if (arg == NULL) {
-                       return -EINVAL;
-               }
-               ret = radeon_ms_bo_get_gpu_addr(dev, &arg->buffer->mem,
-                                               &gpu_addr);
-               if (ret) {
-                       return ret;
-               }
-               gpu_addr = (gpu_addr >> 10) & 0x003FFFFF;
-               cbuffer->cbuffer[dw_id + 3] &= 0xFFC00000;
-               cbuffer->cbuffer[dw_id + 3] |= gpu_addr;
-               /* FIXME: check that source & destination are big enough
-                * for requested blit */
-               break;
-       default:
+       if (dev_priv->cbuffer_checker.check_p3[opcode] == NULL) {
+               return -EINVAL;
+       }
+       ret = dev_priv->cbuffer_checker.check_p3[opcode](dev, cbuffer,
+                                                        dw_id + 1, opcode,
+                                                        count);
+       if (ret) {
                return -EINVAL;
        }
-       /* header + N + 1 dword passed test */
        return count + 2;
 }
 
-static int amd_cbuffer_check(struct drm_device *dev,
-                            struct drm_file *file_priv,
-                            struct amd_cbuffer *cbuffer)
+int amd_cbuffer_check(struct drm_device *dev, struct amd_cbuffer *cbuffer)
 {
        uint32_t i;
        int ret;
@@ -274,9 +188,7 @@ static int amd_cbuffer_check(struct drm_device *dev,
        for (i = 0; i < cbuffer->cbuffer_dw_count;) {
                switch (PACKET_HEADER_GET(cbuffer->cbuffer[i])) {
                case 0:
-                       ret = amd_cbuffer_packet0_check(dev, file_priv,
-                                                       cbuffer, i,
-                                                       _r3xx_register_right);
+                       ret = cbuffer_packet0_check(dev, cbuffer, i);
                        if (ret <= 0) {
                                return ret;
                        }
@@ -287,12 +199,10 @@ static int amd_cbuffer_check(struct drm_device *dev,
                        /* we don't accept packet 1 */
                        return -EINVAL;
                case 2:
-                       /* packet 2 */
-                       i += 1;
-                       break;
+                       /* FIXME: accept packet 2 */
+                       return -EINVAL;
                case 3:
-                       ret = amd_cbuffer_packet3_check(dev, file_priv,
-                                                       cbuffer, i);
+                       ret = cbuffer_packet3_check(dev, cbuffer, i);
                        if (ret <= 0) {
                                return ret;
                        }
@@ -304,9 +214,25 @@ static int amd_cbuffer_check(struct drm_device *dev,
        return 0;
 }
 
+struct amd_cbuffer_arg *
+amd_cbuffer_arg_from_dw_id(struct amd_cbuffer_arg *head, uint32_t dw_id)
+{
+       struct amd_cbuffer_arg *arg;
+
+       list_for_each_entry(arg, &head->list, list) {
+               if (arg->dw_id == dw_id) {
+                       return arg;
+               }
+       }
+       /* no buffer at this dw index */
+       return NULL;
+}
+
+
 int radeon_ms_execbuffer(struct drm_device *dev, void *data,
                         struct drm_file *file_priv)
 {
+       struct drm_radeon_private *dev_priv = dev->dev_private;
        struct drm_radeon_execbuffer *execbuffer = data;
        struct drm_fence_arg *fence_arg = &execbuffer->fence_arg;
        struct drm_bo_kmap_obj cmd_kmap;
@@ -365,8 +291,13 @@ int radeon_ms_execbuffer(struct drm_device *dev, void *data,
        list_add_tail(&cbuffer.args[0].list , &cbuffer.arg_used.list);
 
        /* do cmd checking & relocations */
-       ret = amd_cbuffer_check(dev, file_priv, &cbuffer);
-       if (ret) {
+       if (dev_priv->cbuffer_checker.check) {
+               ret = dev_priv->cbuffer_checker.check(dev, &cbuffer);
+               if (ret) {
+                       drm_putback_buffer_objects(dev);
+                       goto out_free_release;
+               }
+       } else {
                drm_putback_buffer_objects(dev);
                goto out_free_release;
        }