On Mon, Aug 10, 2015 at 04:08:07PM +0300, Martin Peres wrote:
>
>
> 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).
Sorry, it was meant to be int. I missed that I had left it as bool,
hence the confusion.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre