[Mesa-dev] [PATCH 2/2] r600g: simplify and fix flushing and synchronization v2

[Marek Olšák]

On Fri, Jul 20, 2012 at 4:54 AM, Jerome Glisse <j.glisse at gmail.com> wrote:
> On Thu, Jul 19, 2012 at 10:25 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Fri, Jul 20, 2012 at 4:17 AM, Jerome Glisse <j.glisse at gmail.com> wrote:
>>> On Thu, Jul 19, 2012 at 10:06 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>>> I actually care a lot about lockups. Well, you are complaing about
>>>> lockups, yet you have quite obvious bugs in your hyperz code, so let's
>>>> fix them first. (I wouldn't even try and run the hyperz code in its
>>>> current state. Please don't take that personally.) Then, if the
>>>> lockups persist, we can start looking into *what* fixes them. You seem
>>>> to think that this patch helps a lot, but you don't say why. Aren't
>>>> you interested in what sequence of GPU commands helps? If I am
>>>> counting correctly, there are 7 changes in behavior in this patch. It
>>>> should be pretty easy to nail down the few that help, document them
>>>> (like /* these two lines fix a lockup with hyperz */), and discard the
>>>> rest. The documenting part is very important, so that the other
>>>> developers won't break your code accidentally.
>>>>
>>>> Marek
>>>>
>>>
>>> You haven't even try hyperz and you say i have an obvious bug, that's
>>> kind of funny, but you would not know why. I try pretty much all of
>>
>> Oh come on, I already told you about all the bugs I found in the
>> hyperz patch. You now know them too, and so does everybody else
>> reading mesa-dev.
>>
>> Marek
>>
>
> None of the issue you pointed out showed in piglit, none of them did
> have impact on things like openarena, nexuiz, doomIII, lightmark, ...
> so no issue you pointed does not cripple the hyperz patch, it's
> working quite well for many things. Before you extrapolate, yes issue
> you pointed out have impact in backward use of GL but none the less i
> addressed them and i can tell you it does help a bit with lockup.

I have no doubt that it helps with your lockups and I also have no
doubt that the piece of code that helps can be bisected. I have
mentioned 7 changes in the patch which are questionable, so the
bisection should ideally take 3 steps. After we find the change which
helps (and document it), we can discard the rest. That should give us
the same stability as this patch does, but without unnecessary code
which does cost GPU cycles (regardless of whether it is measurable on
a particular machine or not).

By the way, in draw_vbo, the emit functions should be called after
r600_need_cs_space. Otherwise the command stream may overflow.

Marek