Re: [PATCH] drm/i915: Update ring space correctly on lrc context reset

Chris Wilson
2015-10-23T10:51:54Z

On Fri, Oct 23, 2015 at 01:39:24PM +0300, Mika Kuoppala wrote:
> Jani Nikula <jani.nikula@linux.intel.com> writes:
>
> > On Wed, 02 Sep 2015, Arun Siluvery <arun.siluvery@linux.intel.com> wrote:
> >> On 20/08/2015 16:27, Chris Wilson wrote:
> >>> On Thu, Aug 20, 2015 at 05:34:59PM +0300, Mika Kuoppala wrote:
> >>>> If we leave the last_retired_head to pre-reset value, we might
> >>>> end up in a situation where intel_ring_space() returns wrong
> >>>> value on next hardware init.
> >>>
> >>> http://patchwork.freedesktop.org/patch/46612/
> >>> and earlier
> >>> -Chris
> >>>
> >> Hi Chris,
> >>
> >> I see the warning even with below batch,
> >>
> >> [Intel-gfx] [PATCH 50/70] drm/i915: The argument for postfix is redundant
> >> http://patchwork.freedesktop.org/patch/46601/
> >>
> >> the following patch need to be updated as it uses olr,
> >> [Intel-gfx] [PATCH 51/70] drm/i915: Record the position of the start of
> >> the request
> >> http://patchwork.freedesktop.org/patch/46612/
> >>
> >> Do we need some of the previous patches in series as well?
> >>
> >> This patch is fixing the issue in the current code, do you think we can
> >> get this in its current state?
> >
> > Is this patch still valid?
> >
>
> My understanding is that Chris wants more throughout
> revamp of the code so that we have a common ringbuffer init.
> And then go further and remove the special reset handling in
> execlist case.
>
> But argument for this patch is that it fixes a bug in current
> nightly and it makes the legacy ring init and execlist ring init
> identical how they set/reset the ring space. Until we gain
> the generic ringbuffer init code.

My argument is that we can progress the former with a simple generic patch
here that is agnostic of the legacy/execlists duplication.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre