Re: [PATCH v1] drm/i915: Add IOCTL Param to control data port coherency.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux