Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Quoting Mika Kuoppala (2018-06-11 11:16:14)
>> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> > index 409f499c0a45..7970ecb199e2 100644
>> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> > @@ -496,6 +496,10 @@ static int init_ring_common(struct intel_engine_cs *engine)
>> > DRM_DEBUG_DRIVER("%s initialization failed [head=%08x], fudging\n",
>> > engine->name, I915_READ_HEAD(engine));
>> >
>> > + /* Check that the ring offsets point within the ring! */
>> > + GEM_BUG_ON(!intel_ring_offset_valid(ring, ring->head));
>> > + GEM_BUG_ON(!intel_ring_offset_valid(ring, ring->tail));
>> > +
>> > intel_ring_update_space(ring);
>> > I915_WRITE_HEAD(engine, ring->head);
>> > I915_WRITE_TAIL(engine, ring->tail);
>> > @@ -1064,6 +1068,7 @@ int intel_ring_pin(struct intel_ring *ring,
>> >
>> > void intel_ring_reset(struct intel_ring *ring, u32 tail)
>> > {
>> > + tail = intel_ring_wrap(ring, tail);
>>
>> I am pondering this wrap here and it's usefulness. Where
>> could we ever get a tail which is not valid? From corrupted
>> context?
>
> It's just being defensive. We could make this
> GEM_BUG_ON(!offset_valid()) instead?
>
> Your choice?
That would be best to assert it.
For fancyness, you could do a ring_offset_valid check from
assert_ring_tail_valid as it is a subset and share
some of the checks.
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
