From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Subject: Re: [PATCH v3] drm/i915: Add null state batch to active
	list
Date: Thu, 22 May 2014 16:34:30 +0300
Message-ID: <87lhturl7d.fsf@gaia.fi.intel.com>
References: <20140521130602.GA12436@nuc-i3427.alporthouse.com>
 <1400680976-19107-1-git-send-email-mika.kuoppala@intel.com>
 <20140521152906.GA2828@bdvolkin-ubuntu-desktop>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces@lists.freedesktop.org>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by gabe.freedesktop.org (Postfix) with ESMTP id 326E86E42B
 for <intel-gfx@lists.freedesktop.org>; Thu, 22 May 2014 06:35:52 -0700 (PDT)
In-Reply-To: <20140521152906.GA2828@bdvolkin-ubuntu-desktop>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
To: "Volkin, Bradley D" <bradley.d.volkin@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, "miku@iki.fi" <miku@iki.fi>
List-Id: intel-gfx@lists.freedesktop.org

"Volkin, Bradley D" <bradley.d.volkin@intel.com> writes:

> On Wed, May 21, 2014 at 07:02:56AM -0700, Mika Kuoppala wrote:
>> +	if (ring->id == RCS && !to->is_initialized && from == NULL) {
>> +		ret = i915_gem_render_state_init(ring);
>> +		if (ret)
>> +			DRM_ERROR("init render state: %d\n", ret);
>> +	}
>
> Apologies if this has already been discussed, but why do we have the
> 'from == NULL' check? Shouldn't we initialize all uninitialized RCS
> contexts? Otherwise I thought we'll inherit whatever state 'from' left
> behind.
>
> The hw state should be valid in either case (and so I expect would fix
> the rc6 issue either way), it's just the difference between initializing
> every context to a specific valid state or initializing every context to
> _some_ valid state. The commit message on the first render state patch
> seemed to indicate the former while the implementation looks like the
> latter. Just want to understand which we intended.

It seems that the intentions changed and I forgot to update the
commit message. For now this is just to push some state and hoping
that we can get in/out from rc6 without symptomps. 

The idea that we would restore/initialize to a specific (golden) state for
each new context makes me think that we would get rid of some transient 
bugs we are seeing. As how I understand things are now, app might
inherit some parts from previous ctx and then have lacking
initialization by itself and then see a hang..sometimes.

I guess the main opponent here is the performance implications.
And I lack the experience from user/application side to estimate the impact.

I hope that other devs with more experience on this topic will join
the discussion.

Thanks,
-Mika