From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lionel Landwerlin Subject: Re: [PATCH 1/6] drm/i915: store all subslice masks Date: Thu, 11 Jan 2018 16:00:08 +0000 Message-ID: References: <20171218153520.14181-1-lionel.g.landwerlin@intel.com> <20171218153520.14181-2-lionel.g.landwerlin@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0361474560==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id 05BC96E7BF for ; Thu, 11 Jan 2018 16:00:10 +0000 (UTC) In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Tvrtko Ursulin , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============0361474560== Content-Type: multipart/alternative; boundary="------------69E6F9A5201B789634711EC1" Content-Language: en-US This is a multi-part message in MIME format. --------------69E6F9A5201B789634711EC1 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 11/01/18 11:12, Tvrtko Ursulin wrote: >> */ >> +    eu_en = ~I915_READ(GEN8_EU_DISABLE0); >> +    for (ss = 0; ss < sseu->max_subslices; ss++) >> +        sseu->eu_mask[ss]     = (eu_en >> (8 * ss)) & eu_mask; >> +    /* Slice1 */ >> +    sseu->eu_mask[sseu->max_subslices]         = (eu_en >> 24) & >> eu_mask; >> +    eu_en = ~I915_READ(GEN8_EU_DISABLE1); >> +    sseu->eu_mask[sseu->max_subslices + 1]     = eu_en & eu_mask; > > I suggest a helper to index into sse->eu_mask, like > sseu->eu_mask[_eu_mask_idx(slice, subslice)] or something, so it is > more readable what is happening here. Or even: > >  _eu_mask(sseu, slice, subslice) = mask; > > Doable? I am not 100% I picked up exactly on the layout of the eu_mask > array.. each element is one subslice? Consecutive for slice0, > subslice0..N, slice1... sliceN ? Oops, I misread that part. I thought you wanted a helper for accessing the registers. I'll add a helper for what you would like. --------------69E6F9A5201B789634711EC1 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
On 11/01/18 11:12, Tvrtko Ursulin wrote:
*/
+    eu_en = ~I915_READ(GEN8_EU_DISABLE0);
+    for (ss = 0; ss < sseu->max_subslices; ss++)
+        sseu->eu_mask[ss]     = (eu_en >> (8 * ss)) & eu_mask;
+    /* Slice1 */
+    sseu->eu_mask[sseu->max_subslices]         = (eu_en >> 24) & eu_mask;
+    eu_en = ~I915_READ(GEN8_EU_DISABLE1);
+    sseu->eu_mask[sseu->max_subslices + 1]     = eu_en & eu_mask;

I suggest a helper to index into sse->eu_mask, like sseu->eu_mask[_eu_mask_idx(slice, subslice)] or something, so it is more readable what is happening here. Or even:

 _eu_mask(sseu, slice, subslice) = mask;

Doable? I am not 100% I picked up exactly on the layout of the eu_mask array.. each element is one subslice? Consecutive for slice0, subslice0..N, slice1... sliceN ?
Oops, I misread that part.
I thought you wanted a helper for accessing the registers.
I'll add a helper for what you would like.
--------------69E6F9A5201B789634711EC1-- --===============0361474560== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============0361474560==--