Re: [PATCH 04/17] drm/i915: Wrap around the tail offset before setting ring->tail

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

 



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




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

  Powered by Linux