public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Dave Gordon <david.s.gordon@intel.com>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization
Date: Wed, 20 Jul 2016 17:07:14 +0100	[thread overview]
Message-ID: <578FA1B2.1050503@linux.intel.com> (raw)
In-Reply-To: <578F739D.3040302@intel.com>


On 20/07/16 13:50, Dave Gordon wrote:
> On 20/07/16 10:54, Tvrtko Ursulin wrote:
>>
>> On 19/07/16 19:38, Dave Gordon wrote:
>>> On 15/07/16 14:13, Tvrtko Ursulin wrote:
>>>>
>>>> On 29/06/16 17:00, Chris Wilson wrote:
>>>>> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 29/06/16 16:34, Chris Wilson wrote:
>>>>>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>>
>>>>>>>> Replace per-engine initialization with a common half-programatic,
>>>>>>>> half-data driven code for ease of maintenance and compactness.
>>>>>>>>
>>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> This is the biggest pill to swallow (since our 5x5 table is only
>>>>>>> sparsely populated), but it looks correct, and more importantly
>>>>>>> easier to
>>>>>>> read.
>>>>>>
>>>>>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>>>>>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>>>>>> to bits and registers respectively, and write it as a function.
>>>>>
>>>>> It's actually a very simple cyclic function based on register
>>>>> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>>>>>
>>>>> (The only real challenge is picking the direction.)
>>>>>
>>>>> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
>>>>> Author: Ben Widawsky <ben@bwidawsk.net>
>>>>> Date:   Wed Sep 14 20:32:47 2011 -0700
>>>>>
>>>>>      drm/i915: Dumb down the semaphore logic
>>>>>
>>>>>      While I think the previous code is correct, it was hard to follow
>>>>> and
>>>>>      hard to debug. Since we already have a ring abstraction, might as
>>>>> well
>>>>>      use it to handle the semaphore updates and compares.
>>>>
>>>> Doesn't seem to fit, or I just can't figure it out. Needs two functions
>>>> to get rid of the table:
>>>>
>>>> f1(0, 1) = 2
>>>> f1(0, 2) = 0
>>>> f1(0, 3) = 2
>>>> f1(1, 0) = 0
>>>> f1(1, 2) = 2
>>>> f1(1, 3) = 1
>>>> f1(2, 0) = 2
>>>> f1(2, 1) = 0
>>>> f1(2, 3) = 0
>>>> f1(3, 0) = 1
>>>> f1(3, 1) = 1
>>>> f1(3, 2) = 1
>>>>
>>>> and:
>>>>
>>>> f2(0, 1) = 1
>>>> f2(0, 2) = 0
>>>> f2(0, 3) = 1
>>>> f2(1, 0) = 0
>>>> f2(1, 2) = 1
>>>> f2(1, 3) = 2
>>>> f2(2, 0) = 1
>>>> f2(2, 1) = 0
>>>> f2(2, 3) = 0
>>>> f2(3, 0) = 2
>>>> f2(3, 1) = 2
>>>> f2(3, 2) = 2
>>>>
>>>> A weekend math puzzle for someone? :)
>>>>
>>>> Regards,
>>>> Tvrtko
>>>
>>> Here's the APL expression for (the transpose of) f2, with -1's filled in
>>> along the leading diagonal (you need ⎕io←0 so the ⍳-vectors are in
>>> origin 0)
>>>
>>>        {¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>>>
>>> ┌────────┬────────┬────────┬────────┐
>>> │¯1 0 1 2│1 ¯1 0 2│0 1 ¯1 2│1 2 0 ¯1│
>>> └────────┴────────┴────────┴────────┘
>>>
>>> or transposed back so that the first argument is the row index and the
>>> second is the column index:
>>>
>>>        ⍉↑{¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>>>
>>>  ¯1  1  0  1
>>>   0 ¯1  1  2
>>>   1  0 ¯1  0
>>>   2  2  2 ¯1
>>>
>>> http://tryapl.org/?a=%u2349%u2191%7B%AF1+%28%u2375%u2260%u23734%29%u2340%282%7C%u2375%29%u233D%28%u233D%u2363%281%3D%u2375%29%291+%u23733%7D%A8%u23734&run
>>>
>>>
>>
>>   :-C ! How to convert that to C ? :)
>>
>>> f1 is trivially derived from this by the observation that f1 is just f2
>>> with the 1's and 2's interchanged.
>>
>> Ah yes, nicely spotted.
>>
>> Regards,
>> Tvrtko
>
> Assuming you don't care about the leading diagonal (x == y), then
>
>   (⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))
>
> translates into:
>
> int f2(unsigned int x, unsigned int y)
> {
>      x -= x >= y;
>      if (y == 1)
>          x = 3 - x;
>      x += y & 1;
>      return x % 3;
> }
>
> y:x 0 1 2 3
> 0:  0 0 1 2
> 1:  1 1 0 2
> 2:  0 1 1 2
> 3:  1 2 0 0
>
> Each line of C corresponds quite closely to one operation in the APL :)
> Although, in APL we tend to leave the data unchanged while shuffling it
> around into new shapes, whereas the C below does the equivalent things
> by changing the data (noting that it's all modulo-3 arithmetic).
>
>   (⍵≠⍳4)⍀  inserts the leading diagonal, corresponding to the subtraction
>            of x >= y (which removes the leading diagonal).
>
>   ⌽⍣(1=⍵)  reverses the sequence if y==1; in C, that's the 3-x
>
>   (2|⍵)⌽   rotates the sequence by 1 if y is odd; that's the +=
>
> and the final % ensures that the result is 0-2.

I was hoping for a solution which does not include conditionals, someone 
led me to believe it is possible! :)

But thanks, your transformation really works. I've sent a patch 
implementing it to trybot for now.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-07-20 16:07 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-29 15:09 [PATCH 01/13] drm/i915: Consolidate write_tail vfunc initializer Tvrtko Ursulin
2016-06-29 15:09 ` [PATCH 02/13] drm/i915: Consolidate add_request vfunc Tvrtko Ursulin
2016-06-29 15:09 ` [PATCH 03/13] drm/i915: Consolidate seqno_barrier vfunc Tvrtko Ursulin
2016-06-29 15:09 ` [PATCH 04/13] drm/i915: Consolidate get and put irq vfuncs Tvrtko Ursulin
2016-06-29 15:09 ` [PATCH 05/13] drm/i915: Consolidate get/set_seqno Tvrtko Ursulin
2016-06-29 15:09 ` [PATCH 06/13] drm/i915: Consolidate init_hw vfunc Tvrtko Ursulin
2016-06-29 15:09 ` [PATCH 07/13] drm/i915: Consolidate dispatch_execbuffer vfunc Tvrtko Ursulin
2016-06-29 16:40   ` [PATCH v3] " Tvrtko Ursulin
2016-06-30 15:12     ` Chris Wilson
2016-06-29 15:09 ` [PATCH 08/13] drm/i915: Consolidate semaphore vfuncs init Tvrtko Ursulin
2016-06-29 15:09 ` [PATCH 09/13] drm/i915: Move semaphore object creation into intel_ring_init_semaphores Tvrtko Ursulin
2016-06-29 15:30   ` Chris Wilson
2016-06-29 15:09 ` [PATCH 10/13] drm/i915: Compact Gen8 semaphore initialization Tvrtko Ursulin
2016-06-29 15:31   ` Chris Wilson
2016-06-29 15:09 ` [PATCH 11/13] drm/i915: Compact gen8_ring_sync Tvrtko Ursulin
2016-06-29 15:33   ` Chris Wilson
2016-06-29 15:09 ` [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization Tvrtko Ursulin
2016-06-29 15:34   ` Chris Wilson
2016-06-29 15:41     ` Tvrtko Ursulin
2016-06-29 16:00       ` Chris Wilson
2016-06-29 16:14         ` Tvrtko Ursulin
2016-06-29 16:24           ` Chris Wilson
2016-06-29 16:34             ` Tvrtko Ursulin
2016-06-29 16:43               ` Chris Wilson
2016-07-15 13:13         ` Tvrtko Ursulin
2016-07-19 18:38           ` Dave Gordon
2016-07-20  9:54             ` Tvrtko Ursulin
2016-07-20 12:50               ` Dave Gordon
2016-07-20 16:07                 ` Tvrtko Ursulin [this message]
2016-07-20 17:08                   ` Dave Gordon
2016-06-29 15:09 ` [PATCH 13/13] drm/i915: Trim some if-else braces Tvrtko Ursulin
2016-06-29 15:35   ` Chris Wilson
2016-06-29 16:06 ` ✗ Ro.CI.BAT: failure for series starting with [01/13] drm/i915: Consolidate write_tail vfunc initializer Patchwork
2016-06-30  5:20 ` ✗ Ro.CI.BAT: warning for series starting with [01/13] drm/i915: Consolidate write_tail vfunc initializer (rev2) Patchwork
2016-06-30  8:44   ` Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=578FA1B2.1050503@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=Intel-gfx@lists.freedesktop.org \
    --cc=chris@chris-wilson.co.uk \
    --cc=david.s.gordon@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox