From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Subject: Re: [PATCH 1/6] drm/i915: Reinitialize default context
	after reset
Date: Tue, 07 Oct 2014 17:54:54 +0300
Message-ID: <87eguk0w1t.fsf@gaia.fi.intel.com>
References: <1411052315-22979-1-git-send-email-mika.kuoppala@intel.com>
 <1411052315-22979-2-git-send-email-mika.kuoppala@intel.com>
 <20140919174615.GA2491@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 mga11.intel.com (mga11.intel.com [192.55.52.93])
 by gabe.freedesktop.org (Postfix) with ESMTP id B51BB6E114
 for <intel-gfx@lists.freedesktop.org>; Tue,  7 Oct 2014 07:56:06 -0700 (PDT)
In-Reply-To: <20140919174615.GA2491@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>
List-Id: intel-gfx@lists.freedesktop.org

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

Hi Bradley,

> [snip]
>
> On Thu, Sep 18, 2014 at 07:58:30AM -0700, Mika Kuoppala wrote:
>> @@ -577,7 +596,7 @@ static int do_switch(struct intel_engine_cs *ring,
>>  		vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level, GLOBAL_BIND);
>>  	}
>>  
>> -	if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
>> +	if (!to->initialized || i915_gem_context_is_default(to))
>>  		hw_flags |= MI_RESTORE_INHIBIT;
>>  
>>  	ret = mi_set_context(ring, to, hw_flags);
>> @@ -618,26 +637,19 @@ static int do_switch(struct intel_engine_cs *ring,
>>  		/* obj is kept alive until the next request by its active ref */
>>  		i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
>>  		i915_gem_context_unreference(from);
>> -	}
>>  
>> -	uninitialized = !to->legacy_hw_ctx.initialized && from == NULL;
>> -	to->legacy_hw_ctx.initialized = true;
>> +		/* We inherit the state from the previous context */
>> +		to->initialized = true;
>> +	}
>
> Regarding these two hunks, I may have asked this question before but if
> so I've forgotten the answer. Why do we want to set MI_RESTORE_INHIBIT
> when switching to the default context? Why do we want to inherit state
> from the previous context?
>
> I assumed that when switching to an initialized context we would always
> want to restore its last state.
>

I would have been pondering the same questions. Perhaps it has something
to do with the fact that default context should not assume anything
about its previous state and thus userspace needs to initialize
everything fully. And also to avoid the context switch performance
hit on restoring. Ben might know answer.

My plan is to experiment with making master copy of the first fully
initialized state and use it as a read-only bo to initialize all
subsequent contexts from. Atleast with the code, we could measure
the perf impact.

-Mika