On Tue, Sep 29, 2015 at 4:54 PM, Albert Freeman <albertwdfree...@gmail.com> wrote: > On 29 September 2015 at 07:28, Albert Freeman <albertwdfree...@gmail.com> > wrote: >> On 29 September 2015 at 07:25, Albert Freeman <albertwdfree...@gmail.com> >> wrote: >>> On 28 September 2015 at 08:10, Marek Olšák <mar...@gmail.com> wrote: >>>> On Mon, Sep 28, 2015 at 7:20 AM, Albert Freeman >>>> <albertwdfree...@gmail.com> wrote: >>>>> On 28 September 2015 at 14:38, Albert Freeman <albertwdfree...@gmail.com> >>>>> wrote: >>>>>> On 28 September 2015 at 04:12, Emil Velikov <emil.l.veli...@gmail.com> >>>>>> wrote: >>>>>>> On 26 September 2015 at 00:49, Marek Olšák <mar...@gmail.com> wrote: >>>>>>>> From: Marek Olšák <marek.ol...@amd.com> >>>>>>>> >>>>>>>> The spec doesn't require it. This fixes a crash on Android. >>>>>>>> >>>>>>> Nicely spotted Marek. >>>>>>> >>>>>>> A couple of related (not supposed to be part of this patch) notes: >>>>>>> - This will just move the crash (from libEGL to i965_dri.so) for i965 >>>>>>> hardware :) >>>>>> I rediscovered this before I read the email related to this patch >>>>>> (where Marek mentions it): "Re: [Mesa-dev] Pending issues of >>>>>> lollipop-x86". >>>>>> Who (if anyone) should we CC this to? >>>>>>> - We really shouldn't be setting the flags if ctx is NULL, as per the >>>>>>> spec. >>>>>>> >>>>>>> "If no context is >>>>>>> current for the bound API, the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit >>>>>>> is ignored." >>>>>> Maybe handle this in the driver (probably state_tracker in the case of >>>>>> gallium) level. Both here and drivers? >>>>> OpenGL ClientWaitSync is very clear about SYNC_FLUSH_COMMANDS_BIT only >>>>> flushing the GL context when the fence sync is created (it doesn’t >>>>> seem to require it at the moment wait sync is called (probably to >>>>> leave room for drivers to optimise)). EGL is a lot less clear, it >>>>> could be interpreted as behaving just like GL, but to someone writing >>>>> an EGL program it could be easily interpreted as "the flush() must >>>>> occur when eglClientWaitSync is called with SYNC_FLUSH_COMMANDS_BIT". >>>>> >>>>> The current behaviour of gallium (not sure about i965) is to always >>>>> flush() when creating the sync object (hence always behave as >>>>> SYNC_FLUSH_COMMANDS_BIT is set according to GL). However >>>>> implementations are allowed to behave as if flush was not required (in >>>>> GL and I am assuming EGL). So technically the current behaviour of the >>>>> gallium dri state tracker could reduce performance (since flushing >>>>> tends to do that). It however doesn’t actually violate the EGL spec. >>>>> Perhaps some heuristics on when to flush would be a good solution? >>>> >>>> Flushing is currently required for creating fences. This is a gallium >>>> design decision as well as radeon/amdgpu kernel driver decision. It's >>>> simply not possible to put a fence anywhere in the GPU command buffer. >>>> We can only submit a command buffer to the kernel and then get a fence >>>> tracking execution of the whole command buffer. >>>> >>>> Marek >>> Thanks for the clarification. >> Also when I said "Can easily be reverted later if a better solution is >> found. :)". It seemed to me that this was an issue better resolved in >> the code outside of mesa that had the issue. > So basically: > Sorry if I upset anyone. The only thing that has changed is that I > approve of the patch a lot more than I originally did (since my > misunderstanding was cleared up). "The spec doesn't require it." in > the original patch implies an issue in the application and not the > driver. This lead me to believe that for some reason Marek had decided > that an applications incorrect behavior should be fixed in the driver, > hence my reply (I thought Marek was thinking approximately the same > thing). I understood what the patch did and its implications within > mesa, just not what it fixed (now I understand both).
No, this is not an application bug. It's really driver bug that was incorrectly evaluated as an application bug. Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev