| Message ID | 1452642006-23393-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 12, 2016 at 11:40:06PM +0000, Chris Wilson wrote: > struct drm_i915_gem_object_ops { > + const unsigned int flags; Bleh, const is redundant as the definitions should be const themselves. -Chris
On 12/01/16 23:49, Chris Wilson wrote: > On Tue, Jan 12, 2016 at 11:40:06PM +0000, Chris Wilson wrote: >> struct drm_i915_gem_object_ops { >> + const unsigned int flags; > > Bleh, const is redundant as the definitions should be const themselves. > -Chris Yeah, the const-ness attaches to the instances of and references to the structure rather than the abstract type. But it doesn't actually do any harm, so with or without the redundant 'const' it gets: Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
On Wed, Jan 13, 2016 at 11:08 AM, Dave Gordon <david.s.gordon@intel.com> wrote: > On 12/01/16 23:49, Chris Wilson wrote: >> >> On Tue, Jan 12, 2016 at 11:40:06PM +0000, Chris Wilson wrote: >>> >>> struct drm_i915_gem_object_ops { >>> + const unsigned int flags; >> >> >> Bleh, const is redundant as the definitions should be const themselves. >> -Chris > > > Yeah, the const-ness attaches to the instances of and references to the > structure rather than the abstract type. But it doesn't actually do any > harm, so with or without the redundant 'const' it gets: > > Reviewed-by: Dave Gordon <david.s.gordon@intel.com> > Fixes the crash I was seeing with relocs in a userptr bo. Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
On Sat, Jan 23, 2016 at 07:13:55AM -0000, Patchwork wrote: > == Summary == > > Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest > > Test kms_flip: > Subgroup basic-flip-vs-dpms: > pass -> DMESG-WARN (ilk-hp8440p) > Subgroup basic-flip-vs-modeset: > pass -> DMESG-WARN (ilk-hp8440p) Our dear friend, the fifo underrun on ilk: https://bugs.freedesktop.org/show_bug.cgi?id=93787 > Test kms_force_connector_basic: > Subgroup force-connector-state: > pass -> SKIP (snb-dellxps) Still undebugged whether this is a loose connector or something really fishy going on in the kernel: https://bugs.freedesktop.org/show_bug.cgi?id=93769 > > bdw-nuci7 total:140 pass:131 dwarn:0 dfail:0 fail:0 skip:9 > bdw-ultra total:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 > bsw-nuc-2 total:143 pass:119 dwarn:0 dfail:0 fail:0 skip:24 > byt-nuc total:143 pass:128 dwarn:0 dfail:0 fail:0 skip:15 > hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 > hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 > ilk-hp8440p total:143 pass:102 dwarn:3 dfail:0 fail:0 skip:38 > ivb-t430s total:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 > skl-i5k-2 total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 > snb-dellxps total:143 pass:128 dwarn:0 dfail:0 fail:0 skip:15 > snb-x220t total:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 > > Results at /archive/results/CI_IGT_test/Patchwork_1253/ > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 104bd1809936..e6ad06c5c2f5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1994,6 +1994,9 @@ enum hdmi_force_audio { #define I915_GTT_OFFSET_NONE ((u32)-1) struct drm_i915_gem_object_ops { + const unsigned int flags; +#define I915_GEM_OBJECT_HAS_STRUCT_PAGE 0x1 + /* Interface between the GEM object and its backing storage. * get_pages() is called once prior to the use of the associated set * of pages before to binding them into the GTT, and put_pages() is @@ -2009,6 +2012,7 @@ struct drm_i915_gem_object_ops { */ int (*get_pages)(struct drm_i915_gem_object *); void (*put_pages)(struct drm_i915_gem_object *); + int (*dmabuf_export)(struct drm_i915_gem_object *); void (*release)(struct drm_i915_gem_object *); }; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ddc21d4b388d..bb44bad15403 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4425,6 +4425,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, } static const struct drm_i915_gem_object_ops i915_gem_object_ops = { + .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE, .get_pages = i915_gem_object_get_pages_gtt, .put_pages = i915_gem_object_put_pages_gtt, }; @@ -5261,7 +5262,7 @@ i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, int n) struct page *page; /* Only default objects have per-page dirty tracking */ - if (WARN_ON(obj->ops != &i915_gem_object_ops)) + if (WARN_ON((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0)) return NULL; page = i915_gem_object_get_page(obj, n); diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index 19fb0bddc1cd..59e45b3a6937 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -789,9 +789,10 @@ i915_gem_userptr_dmabuf_export(struct drm_i915_gem_object *obj) } static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = { - .dmabuf_export = i915_gem_userptr_dmabuf_export, + .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE, .get_pages = i915_gem_userptr_get_pages, .put_pages = i915_gem_userptr_put_pages, + .dmabuf_export = i915_gem_userptr_dmabuf_export, .release = i915_gem_userptr_release, };
commit 033908aed5a596f6202c848c6bbc8a40fb1a8490 Author: Dave Gordon <david.s.gordon@intel.com> Date: Thu Dec 10 18:51:23 2015 +0000 drm/i915: mark GEM object pages dirty when mapped & written by the CPU introduced a check into i915_gem_object_get_dirty_pages() that returned a NULL pointer when called with a bad object, one that was not backed by shmemfs. This WARN was too strict as we can work on all struct page backed objects, and resulted in a WARN + GPF for existing userspace. In order to differentiate the various types of objects, add a new flags field to the i915_gem_object_ops struct to describe their capabilities, with the first flag being whether the object has struct pages. Reported-by: Kristian Høgsberg Kristensen <krh@bitplanet.net> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Dave Gordon <david.s.gordon@intel.com> Cc: Kristian Høgsberg Kristensen <krh@bitplanet.net> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_drv.h | 4 ++++ drivers/gpu/drm/i915/i915_gem.c | 3 ++- drivers/gpu/drm/i915/i915_gem_userptr.c | 3 ++- 3 files changed, 8 insertions(+), 2 deletions(-)