On Thu, Jun 19, 2014 at 02:00:24PM +0000, Mateo Lozano, Oscar wrote: > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel > > Vetter > > Sent: Wednesday, June 18, 2014 8:51 PM > > To: Mateo Lozano, Oscar > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Subject: Re: [PATCH 1/8] drm/i915: Get rid of the synchronous flag > > in switch_mm > > > > On Wed, Jun 18, 2014 at 05:15:35PM +0100, oscar.mateo@xxxxxxxxx wrote: > > > From: Oscar Mateo <oscar.mateo@xxxxxxxxx> > > > > > > As a first step, split switch_mm into two different functions: > > > sync_switch_mm (currently used by ppgtt_enable) and switch_mm (used by > > > the outside world). > > > > > > Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> > > > > I guess that jira task description was a bit unclear - removing the bool > > synchronous argument isn't the goal itself. The idea was to rework the gpu > > reset sequence so that it matches the same sequence we use at driver load > > and so can use the same code. This patch removes the bool synchronous, but > > the logical split with two different ways to load ppgtt is still there. > > No, I realized I didn´t really do anything, but I saw the possibility of saving one special casing in Execlists and I jumped for it, forgetting about the JIRA task (psychological fixation, I guess) > Anyway, I was going to ask for more details on the JIRA... > > > The reason for aiming as hard as possible to use the exact same code for > > driver load, gpu reset and runtime pm/system resume is that we've simply > > seen too many bugs due to slight variations and unintended omissions. > > > > If for some really odd reason this is impossible (e.g. when the gpu needs a > > special dance when coming out of reset) then we need a giant comment > > explaining the magic. But without such a clear reason the goal should always > > be to have just 1 single autorative piece of code for a given state transition. > > Ok. How about this: on ring init, I manually set the Aliasing PPGTT and > the default context object synchronously via MMIO writes (the Aliasing > PPGTT via the ppgtt->sync_switch_mm() and the default context directly > using the CCID). I assume that, whenever we are in ring init, the GPU is > sufficiently idle to manually frob all the required registers. That way, > I get rid of i915_gem_context_enable() completely and I don´t have to do > any special casing at all. Ring commands (PPGTT switch and > MI_SET_CONTEXT) are then left completely for the normal do_switch(). > What do you think? Can't we do the ring_init loading the same way as a normal context switch? That's kinda my dream land thing. Of course if the initial loading is sufficiently differnt then we have a problem. So essentially do_switch would check whether the ppgtt changes and load that as part of the context. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
