[Mesa-dev] [PATCH 07/70] i965: Combine the multiple pipelined register detection into one round-trip

[Martin Peres]

On 10/08/15 16:00, Chris Wilson wrote:
> On Mon, Aug 10, 2015 at 03:17:26PM +0300, Martin Peres wrote:
>>> +static int
>>> +intel_detect_pipelined_oacontrol(struct intel_screen *screen,
>>> +                                 struct detect_pipelined_register *detect)
>>>   {
>>>      if (screen->devinfo->gen < 6 || screen->devinfo->gen >= 8)
>>> -      return false;
>>> +      return 0;
>>>      /* Set "Select Context ID" to a particular address (which is likely not a
>>>       * context), but leave all counting disabled.  This should be harmless.
>>>       */
>>> -   return intel_detect_pipelined_register(screen,
>>> -                                          OACONTROL,
>>> -                                          0x31337000,
>>> -                                          true);
>>> +   detect->reg = OACONTROL;
>>> +   detect->expected_value = 0x31337000;
>>> +   detect->result = HW_HAS_PIPELINED_OACONTROL;
>>> +   detect->reset = true;
>>> +   return 1;
>>> +}
>>> +
>>> +static void
>>> +intel_detect_pipelined_register_access(struct intel_screen *screen)
>>> +{
>>> +   struct detect_pipelined_register regs[2], *r =regs;
>>> +
>>> +   /* Combine the multiple register access validation into a single
>>> +    * round trip through the kernel + GPU.
>>> +    */
>>> +   r += intel_detect_pipelined_so(screen, r);
>>> +   r += intel_detect_pipelined_oacontrol(screen, r);
>> Not a fan of this construct. How about changing the return types of
>> the detect functions to int?.
> Do you mean
> 	if (intel_detect_pipelined_so(screen, r))
> 		r++;
> or
> 	int index = 0;
> 	index += intel_detect_pipelined_so(screen, &r[index]);
> ?
> -Chris

Sorry, I meant:

static BOOL intel_detect_pipelined_so() --> static INT intel_detect_pipelined_so()

Booleans are supposed to be booleans, not integers. The fact that true 
and false map to 0 and 1 are an implementation detail (which should 
never ever be different for plenty of reasons, but we are talking about 
semantics here).