Re: [PATCH] drm/i915: Bug fixes to ring 'head' updating

Daniel Vetter
2014-11-24T09:35:50Z

On Tue, Nov 18, 2014 at 9:02 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Nov 03, 2014 at 01:29:04PM +0000, Dave Gordon wrote:
>> Fixes to both the LRC and the legacy ringbuffer code to correctly
>> calculate and update the available space in a ring.
>>
>> The logical ring code was updating the software ring 'head' value
>> by reading the hardware 'HEAD' register. In LRC mode, this is not
>> valid as the hardware is not necessarily executing the same context
>> that is being processed by the software. Thus reading the h/w HEAD
>> could put an unrelated (undefined, effectively random) value into
>> the s/w 'head' -- A Bad Thing for the free space calculations.
>>
>> In addition, the old code could update a ringbuffer's 'head' value
>> from the 'last_retired_head' even when the latter hadn't been recently
>> updated and therefore had a value of -1; this would also confuse the
>> freespace calculations. Now, we consume 'last_retired_head' in just
>> one place, ensuring that this confusion does not arise.
>>
>> Change-Id: Id7ce9096ed100a2882c68a54206f30b6c87e92fa
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>
> Is there an igt testcase which readily reproduces this? Or can we have
> one?

Ping. Bugfixes really should have either a Testcase: line in the
commit message indicating the igt used to reproduce/validate the fix
or an explanation why testing with an igt case is not possible.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch