From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 19/49] drm/i915/bdw: Populate LR contexts (somewhat) Date: Wed, 16 Apr 2014 08:04:50 +0200 Message-ID: References: <1395943218-7708-1-git-send-email-oscar.mateo@intel.com> <1395943218-7708-20-git-send-email-oscar.mateo@intel.com> <20140415160033.GA16015@jeffdesk> <20140415161033.GB16015@jeffdesk> <20140415204322.GC16015@jeffdesk> <20140415210802.GB10722@phenom.ffwll.local> <20140415223245.GD16015@jeffdesk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ie0-f180.google.com (mail-ie0-f180.google.com [209.85.223.180]) by gabe.freedesktop.org (Postfix) with ESMTP id 6C3A76E9EA for ; Tue, 15 Apr 2014 23:04:51 -0700 (PDT) Received: by mail-ie0-f180.google.com with SMTP id as1so10118965iec.11 for ; Tue, 15 Apr 2014 23:04:51 -0700 (PDT) In-Reply-To: <20140415223245.GD16015@jeffdesk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter , "Mateo Lozano, Oscar" , intel-gfx , Ben Widawsky , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Wed, Apr 16, 2014 at 12:32 AM, Jeff McGee wrote: >> > Reflecting on my initial confusion, would it be clearer to provide names for >> > each dword position in the context image, rather than using an unnamed offset >> > like CTX_R_PWR_CLK_STATE+1? Example: >> > >> > reg_state[CTX_R_PWR_CLK_STATE_ADDR] = 0x20c8 >> > reg_state[CTX_R_PWR_CLK_STATE_DATA] = 0; >> >> Usually when we emit batches in userspace (and the context is nothing else >> really) we have some OUT_BATCH macro which writes the dword and increments >> the pointer. Since MI_LOAD_REGISTER_IMM is multi-length we could add a >> OUT_BATCH_REG_WRITE(reg, value) which does both dword emissions. >> >> That should clarify a lot what's going on here. We might even completely >> drop all the offset #defines and replace them with a few comments or so. >> -Daniel >> > OK, now I get it. My mistake was in thinking the context image is just pure > state that hardware already knows how to restore. But as you say it is more > like a batch which includes the state *and* the MI_LOAD_REGISTER_IMM commands > required to restore. So in that sense I understand that the approach here to > initilize the context is much like constructing a batch. But later when we > want to update the value of a context field we have (in a later patch of this > series): > > reg_state[CTX_RING_TAIL+1] = value; > > This is a bit obscure when occurring by itself and not in the flow of > initializing the context (batch). The same will be true when we add management > of the CTX_R_PWR_CLK_STATE value dword. The approach thus far for this stuff has been to save the hidden offset value when you get around to write this dword somewhere, e.g. for relocation processing or similar. Or maybe add an assert that the contstant we #defined matches the running offset when we expect it to. Dunno really whether going with an OUT_BATCH approach really makes sense, was just an idea. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch