On su, 2016-08-07 at 15:45 +0100, Chris Wilson wrote:
> @@ -530,7 +530,7 @@ int i915_error_state_to_str(struct
> drm_i915_error_state_buf *m,
> }
> }
>
> - if ((obj = error->semaphore_obj)) {
> + if ((obj = error->semaphore)) {
Hate this kind of code which is direct result of copy paste... Adding
to TODO list.
> static int gen8_rcs_signal(struct drm_i915_gem_request *req)
> @@ -2329,12 +2331,14 @@ void intel_engine_init_seqno(struct intel_engine_cs
> *engine, u32 seqno)
> if (HAS_VEBOX(dev_priv))
> I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
> }
> - if (dev_priv->semaphore_obj) {
> - struct drm_i915_gem_object *obj = dev_priv->semaphore_obj;
> + if (dev_priv->semaphore) {
> + struct drm_i915_gem_object *obj = dev_priv->semaphore->obj;
> struct page *page = i915_gem_object_get_dirty_page(obj, 0);
> void *semaphores = kmap(page);
> memset(semaphores + GEN8_SEMAPHORE_OFFSET(engine->id, 0),
> 0, I915_NUM_ENGINES * gen8_semaphore_seqno_size);
> + drm_clflush_virt_range(semaphores +
> GEN8_SEMAPHORE_OFFSET(engine->id, 0),
> + I915_NUM_ENGINES *
> gen8_semaphore_seqno_size);
Where did this hunk appear from? Did not expect based on the commit
message as there was no such thing :P
> kunmap(page);
> }
> memset(engine->semaphore.sync_seqno, 0,
> @@ -2556,36 +2560,40 @@ static int gen6_ring_flush(struct
> drm_i915_gem_request *req, u32 mode)
> static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
> struct intel_engine_cs *engine)
> {
> - struct drm_i915_gem_object *obj;
> int ret, i;
>
> if (!i915.semaphores)
> return;
>
> - if (INTEL_GEN(dev_priv) >= 8 && !dev_priv->semaphore_obj) {
> + if (INTEL_GEN(dev_priv) >= 8 && !dev_priv->semaphore) {
> + struct drm_i915_gem_object *obj;
> + struct i915_vma *vma;
> +
> obj = i915_gem_object_create(&dev_priv->drm, 4096);
> if (IS_ERR(obj)) {
> - DRM_ERROR("Failed to allocate semaphore bo. Disabling
> semaphores\n");
Silencing a DRM_ERROR, maybe into commit message too.
> i915.semaphores = 0;
> - } else {
> - i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
Right, this is traded for the drm_clflush_virt_range()? I'd add a
comment on top of the new location.
> - ret = i915_gem_object_ggtt_pin(obj, NULL,
> - 0, 0, PIN_HIGH);
> - if (ret != 0) {
> - i915_gem_object_put(obj);
> - DRM_ERROR("Failed to pin semaphore bo.
> Disabling semaphores\n");
> - i915.semaphores = 0;
> - } else {
> - dev_priv->semaphore_obj = obj;
> - }
> + return;
Goto teardown;
> }
> - }
>
> - if (!i915.semaphores)
> - return;
> + vma = i915_vma_create(obj, &dev_priv->ggtt.base, NULL);
> + if (IS_ERR(vma)) {
> + i915_gem_object_put(obj);
> + i915.semaphores = 0;
> + return;
Goto teardown.
> + }
> +
> + ret = i915_vma_pin(vma, 0, 0, PIN_GLOBAL | PIN_HIGH);
> + if (ret) {
> + i915_gem_object_put(obj);
> + i915.semaphores = 0;
> + return;
Goto teardown.
> + }
> +
> + dev_priv->semaphore = vma;
> + }
>
Above addressed;
Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
