On Thu, Jun 18, 2015 at 01:29:45PM +0100, Peter Antoine wrote:
> @@ -1379,6 +1380,13 @@ static int gen8_init_rcs_context(struct intel_engine_cs *ring,
> if (ret)
> return ret;
>
> + /*
> + * Failing to program the MOCS is non-fatal.The system will not
> + * run at peak performance. So generate a warning and carry on.
> + */
> + if (intel_rcs_context_init_mocs(ring, ctx) != 0)
> + DRM_ERROR("MOCS failed to program: expect performance issues.");
You said to expect display corruption as well if this failed.
Fortunately, if this fails, we have severe driver issues...
> +/**
> + * emit_mocs_l3cc_table() - emit the mocs control table
> + * @ringbuf: DRM device.
> + * @table: The values to program into the control regs.
> + *
> + * This function simply emits a MI_LOAD_REGISTER_IMM command for the
> + * given table starting at the given address. This register set is programmed
> + * in pairs.
> + *
> + * Return: Nothing.
> + */
> +static void emit_mocs_l3cc_table(struct intel_ringbuffer *ringbuf,
> + struct drm_i915_mocs_table *table) {
> + unsigned int count;
> + unsigned int i;
> + u32 value;
> + u32 filler = (table->table[0].l3cc_value & 0xffff) |
> + ((table->table[0].l3cc_value & 0xffff) << 16);
l3cc_value is only u16, & 0xffff is just noise, without & you don't need
the parantheses.
> +int intel_rcs_context_init_mocs(struct intel_engine_cs *ring,
> + struct intel_context *ctx)
> +{
> + int ret = 0;
> +
> + struct drm_i915_mocs_table t;
> + struct drm_device *dev = ring->dev;
> + struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
> +
> + if (get_mocs_settings(dev, &t)) {
> + u32 table_size;
> +
> + /*
> + * OK. For each supported ring:
> + * number of mocs entries * 2 dwords for each control_value
> + * plus number of mocs entries /2 dwords for l3cc values.
> + *
> + * Plus 1 for the load command and 1 for the NOOP per ring
> + * and the l3cc programming.
*
* With 5 rings and 63 mocs entries, this gives 715
* dwords.
> + */
> + table_size = GEN9_NUM_MOCS_RINGS *
> + ((2 * GEN9_NUM_MOCS_ENTRIES) + 2) +
> + GEN9_NUM_MOCS_ENTRIES + 2;
If you pushed the ring_begin into each function, not only would it be
easier to verify, you then don't need an explanation that starts with
"This looks like a mistake". Validation of ring_begin/ring_advance is by
review, so it has to be easy to review.
> + ret = intel_logical_ring_begin(ringbuf, ctx, table_size);
> + if (ret) {
> + DRM_DEBUG("intel_logical_ring_begin failed %d\n", ret);
> + return ret;
> + }
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
