<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jun 14, 2018 at 11:42 AM, Keith Packard <span dir="ltr"><<a href="mailto:keithp@keithp.com" target="_blank">keithp@keithp.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
<br>
>> +   if (instance->enabled_extensions.<wbr>KHR_display) {<br>
>> +      master_fd = open(path, O_RDWR | O_CLOEXEC);<br>
>><br>
><br>
> Is this supposed to be opening primary_path instead?<br>
<br>
</span>Yes, and this section of code needs to occur before anv_init_wsi.<br>
<br>
I appear to have skipped testing this path on ANV and only tested it on<br>
RADV -- RADV has the code in the right order. Thanks for catching this;<br>
sorry I messed up and didn't test it.<br>
<span class=""><br>
> This could just be<br>
><br>
> if (anv_gem_get_param(master_fd, I915_PARAM_CHIPSET_ID) == 0) {<br>
>    close(master_fd);<br>
>    master_fd = -1;<br>
> }<br>
><br>
> No need to type out all that IOCTL stuff.<br>
<br>
</span>Thanks, that's lots shorter (RADV doesn't appear to have a similar helper).<br>
<br>
Here's an amendment to the proposed patch which fixes the bug and<br>
switches to the simpler detection method.<br></blockquote><div><br></div><div>Looks good to me.  With that,</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div></div></div></div>