On 07/12/2016 13:58, Chris Wilson wrote:
The phys object is a rarely used device (only very old machines require
a chunk of physically contiguous pages for a few hardware interactions).
As such, it is not exercised by CI and to combat that we want to add a
test that exercises the phys object on all platforms.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem.c | 168 +++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_mock_selftests.h | 1 +
2 files changed, 169 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9794dd655877..1ab5fdae2d47 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4949,3 +4949,171 @@ i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj,
sg = i915_gem_object_get_sg(obj, n, &offset);
return sg_dma_address(sg) + (offset << PAGE_SHIFT);
}
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include <linux/pm_runtime.h>
+
+#include "i915_selftest.h"
+
+static struct drm_driver mock_driver = {
+ .name = "mock",
+ .driver_features = DRIVER_GEM,
+
+ .gem_close_object = i915_gem_close_object,
+ .gem_free_object_unlocked = i915_gem_free_object,
+};
+
+struct mock_object {
+ struct drm_i915_gem_object base;
+};
+
+static void release_dev(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ kfree(pdev);
+}
+
+static struct drm_i915_private *mock_gem_device(void)
+{
+ struct drm_i915_private *i915;
+ struct pci_dev *pdev;
+ int err;
+
+ i915 = kzalloc(sizeof(*i915), GFP_TEMPORARY);
+ if (!i915)
+ return NULL;
+
+ pdev = kzalloc(sizeof(*pdev), GFP_TEMPORARY);
+ if (!pdev) {
+ kfree(i915);
+ return NULL;
+ }
+
+ device_initialize(&pdev->dev);
+ pdev->dev.release = release_dev;
+ dev_set_name(&pdev->dev, "mock");
+ dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+
+ pm_runtime_dont_use_autosuspend(&pdev->dev);
+ pm_runtime_get_sync(&pdev->dev);
+ pci_set_drvdata(pdev, i915);
+
+ err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
+ if (err) {
+ pr_err("Failed to initialise mock GEM device: err=%d\n", err);
+ put_device(&pdev->dev);
+ kfree(i915);
+ return NULL;
+ }
+ i915->drm.pdev = pdev;
+ i915->drm.dev_private = i915;
+
+ mkwrite_device_info(i915)->gen = -1;
+
+ spin_lock_init(&i915->mm.object_stat_lock);
+
+ INIT_WORK(&i915->mm.free_work, __i915_gem_free_work);
+ init_llist_head(&i915->mm.free_list);
+
+ i915->objects = KMEM_CACHE(mock_object, SLAB_HWCACHE_ALIGN);
+ if (!i915->objects)
+ goto err_device;
+
+ return i915;
+
+err_device:
+ kfree(i915);
+ return NULL;
+}
+
+static void mock_device_free(struct drm_i915_private *i915)
+{
+ struct pci_dev *pdev = i915->drm.pdev;
+
+ rcu_barrier();
+ while (flush_work(&i915->mm.free_work))
+ rcu_barrier();
+
+ drm_dev_unref(&i915->drm);
+ put_device(&pdev->dev);
+}
+
+static int igt_gem_object(void *ignore)
+{
+ struct drm_i915_gem_object *obj;
+ struct drm_i915_private *i915;
+ int err = -ENOMEM;
+
+ i915 = mock_gem_device();
+ if (!i915)
+ goto out;
+
+ obj = i915_gem_object_create(i915, 4096);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ pr_err("i915_gem_object_create failed, err=%d\n", err);
+ goto out_device;
+ }
+
+ err = 0;
+ i915_gem_object_put(obj);
+out_device:
+ mock_device_free(i915);
+out:
+ return err;
+}
+
+static int igt_phys_object(void *ignore)
+{
+ struct drm_i915_gem_object *obj;
+ struct drm_i915_private *i915;
+ int err = -ENOMEM;
+
+ i915 = mock_gem_device();
+ if (!i915)
+ goto out;
+
+ obj = i915_gem_object_create(i915, 4096);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ pr_err("i915_gem_object_create failed, err=%d\n", err);
+ goto out_device;
+ }
+
+ err = -EINVAL;
+ mutex_lock(&i915->drm.struct_mutex);
+ err = i915_gem_object_attach_phys(obj, PAGE_SIZE);
+ mutex_unlock(&i915->drm.struct_mutex);
+ if (err) {
+ pr_err("i915_gem_object_attach_phys failed, err=%d\n", err);
+ goto err;
+ }
+
+ if (obj->ops != &i915_gem_phys_ops) {
+ pr_err("i915_gem_object_attach_phys did not create a phys object\n");
+ goto err;
+ }
+
+ /* Make the object work during teardown */
Not the clearest what does "work" mean? I see that the exercises the
copy to shmem path, just saying comment could be better.
+ obj->mm.dirty = true;
+
+ err = 0;
+err:
+ i915_gem_object_put(obj);
+out_device:
+ mock_device_free(i915);
+out:
+ return err;
+}
+
+int i915_gem_object_selftests(void)
+{
+ static const struct i915_subtest tests[] = {
+ SUBTEST(igt_gem_object),
+ SUBTEST(igt_phys_object),
+ };
+
+ return i915_subtests(tests, NULL);
+}
+#endif
Would it be worth shunting these "if
IS_ENABLED(CONFIG_DRM_I915_SELFTEST)" blocks into separate files and
include them from the parent?
Like in this case:
#include "i915_gem_selftests.c"
Or even:
#include "selftests/i915_gem.c"
If the include is at the end of the file it should always work. It will
be more manageable as the tests grow I think.
diff --git a/drivers/gpu/drm/i915/i915_mock_selftests.h b/drivers/gpu/drm/i915/i915_mock_selftests.h
index 9ff379b18c20..34f32f777b34 100644
--- a/drivers/gpu/drm/i915/i915_mock_selftests.h
+++ b/drivers/gpu/drm/i915/i915_mock_selftests.h
@@ -8,6 +8,7 @@
*
* Tests are executed in reverse order by igt/drv_selftest
*/
+selftest(objects, i915_gem_object_selftests)
selftest(requests, i915_gem_request_selftest)
selftest(breadcrumbs, intel_breadcrumbs_selftest)
selftest(sanitycheck, i915_mock_sanitycheck) /* keep last */
Looks clean enough and although I am not 100% familiar with some of the
core APIs used - if it works it works.
Strong suggestion to consider the move to a separate file, but in essence:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
