<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 11, 2017 at 9:54 AM, Chris Wilson <span dir="ltr"><<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, May 11, 2017 at 09:13:55AM -0700, Jason Ekstrand wrote:<br>
> This doesn't actually fix any 48-bit bugs because the addresses provided<br>
> to us from the kernel are already sign-extended.  However, it still<br>
> seems like a good idea.<br>
<br>
</span>addr=1<<47-4096, delta=4096<br><div><div class="h5"></div></div></blockquote><div><br></div><div>Ugh... You're right.  I'll update the commit message with that and add a CC to stable.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
> Cc: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>><br>
> ---<br>
>  src/intel/genxml/gen_pack_<wbr>header.py | 34 ++++++++++++++++++++++++++++++<wbr>+---<br>
>  1 file changed, 31 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/intel/genxml/gen_pack_<wbr>header.py b/src/intel/genxml/gen_pack_<wbr>header.py<br>
> index 020dbe4..df71689 100644<br>
> --- a/src/intel/genxml/gen_pack_<wbr>header.py<br>
> +++ b/src/intel/genxml/gen_pack_<wbr>header.py<br>
> @@ -168,6 +168,31 @@ __gen_ufixed(float v, uint32_t start, uint32_t end, uint32_t fract_bits)<br>
>  #error #define __gen_combine_address before including this file<br>
>  #endif<br>
><br>
> +static inline uint64_t<br>
> +__gen_address(__gen_user_data *data, void *location, __gen_address_type addr,<br>
> +              uint32_t delta, uint32_t start, uint32_t end)<br>
> +{<br>
> +   __gen_validate_value(addr);<br>
> +<br>
> +   uint64_t offset = __gen_combine_address(data, location, addr, delta);<br>
> +<br>
> +   if (end >= 32) {<br>
> +      /* From the Broadwell PRM Vol. 2a, MI_LOAD_REGISTER_MEM::<wbr>MemoryAddress:<br>
> +       *<br>
> +       *    "This field specifies the address of the memory location where the<br>
> +       *    register value specified in the DWord above will read from. The<br>
> +       *    address specifies the DWord location of the data. Range =<br>
> +       *    GraphicsVirtualAddress[63:2] for a DWord register GraphicsAddress<br>
> +       *    [63:48] are ignored by the HW and assumed to be in correct<br>
> +       *    canonical form [63:48] == [47]."<br>
> +       */<br>
> +      const int shift = 63 - 47;<br>
> +      return (((int64_t)offset) << shift) >> shift;<br>
<br>
</div></div>I presume that start/end are known at compile time, so the branch here<br>
is just a figment of my imagination.<br></blockquote><div><br></div><div>Yes.  The function is declared inline and all the arguments are constants.  There is a chance GCC will decide to not inline it but I've never seen that happen.  (And, yes, I do look at objdump of these things on a reasonably regular basis.)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
There's no chance of having the shift + comments pulled in from a header?<br></blockquote><div><br></div><div>Not sure what you mean by that.  I mean, we could add a header to src/intel/common to store some of these things.  Not sure what else would go in there, but I wouldn't be opposed to starting one.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Reviewed-by: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>><span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>Thanks! <br></div></div><br></div></div>