On 2018-06-21 09:05, Chris Wilson wrote:
Quoting Tomasz Lis (2018-06-20 16:03:07)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 33bc914..c69dc26 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -258,6 +258,57 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
ce->lrc_desc = desc;
}
+static int emit_set_data_port_coherency(struct i915_request *req, bool enable)
+{
+ u32 *cs;
+ i915_reg_t reg;
+
+ GEM_BUG_ON(req->engine->class != RENDER_CLASS);
+ GEM_BUG_ON(INTEL_GEN(req->i915) < 9);
+
+ cs = intel_ring_begin(req, 4);
+ if (IS_ERR(cs))
+ return PTR_ERR(cs);
+
+ if (INTEL_GEN(req->i915) >= 10)
+ reg = CNL_HDC_CHICKEN0;
+ else
+ reg = HDC_CHICKEN0;
+
+ /* FIXME: this feature may be unuseable on CNL; If this checks to be
+ * true, we should enodev for CNL. */
+ *cs++ = MI_LOAD_REGISTER_IMM(1);
+ *cs++ = i915_mmio_reg_offset(reg);
+ /* Enabling coherency means disabling the bit which forces it off */
+ if (enable)
+ *cs++ = _MASKED_BIT_DISABLE(HDC_FORCE_NON_COHERENT);
+ else
+ *cs++ = _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT);
+ *cs++ = MI_NOOP;
+
+ intel_ring_advance(req, cs);
+
+ return 0;
+}
There's nothing specific to the logical ringbuffer context here afaics.
It could have just been done inside the single
i915_gem_context_set_data_port_coherency(). Also makes it clearer that
i915_gem_context_set_data_port_coherency needs struct_mutex.
cmd = HDC_FORCE_NON_COHERENT << 16;
if (!coherent)
cmd |= HDC_FORCE_NON_COHERENT;
*cs++ = cmd;
Does that read any clearer?
Sorry, I don't think I follow.
Should I move the code out of logical ringbuffer context (intel_lrc.c)?
Should I merge the emit_set_data_port_coherency() with
intel_lr_context_modify_data_port_coherency()?
Should I lock a mutex while adding the request?
-Tomasz
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 1593194..214e291 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -104,4 +104,8 @@ struct i915_gem_context;
void intel_lr_context_resume(struct drm_i915_private *dev_priv);
+int
+intel_lr_context_modify_data_port_coherency(struct i915_gem_context *ctx,
+ bool enable);
+
#endif /* _INTEL_LRC_H_ */
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 7f5634c..fab072f 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1453,6 +1453,7 @@ struct drm_i915_gem_context_param {
#define I915_CONTEXT_PARAM_NO_ERROR_CAPTURE 0x4
#define I915_CONTEXT_PARAM_BANNABLE 0x5
#define I915_CONTEXT_PARAM_PRIORITY 0x6
+#define I915_CONTEXT_PARAM_COHERENCY 0x7
DATAPORT_COHERENCY
There are many different caches.
There should be some commentary around here telling userspace what the
contract is.
Will do.
#define I915_CONTEXT_MAX_USER_PRIORITY 1023 /* inclusive */
#define I915_CONTEXT_DEFAULT_PRIORITY 0
#define I915_CONTEXT_MIN_USER_PRIORITY -1023 /* inclusive */
COHERENCY has MAX/MIN_USER_PRIORITY, interesting. I thought it was just
a boolean.
-Chris
I did not noticed the structure of defines here; will move the new define.
-Tomasz
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
