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

[Jerome Glisse]

On Sun, Jul 22, 2012 at 8:58 PM, Marek Olšák <maraeo at gmail.com> wrote:
> 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

Again i haven't found a combination other than the outcome of the full
patch that helps more. So be my guest bisect on rv610, rv635, rv670,
rv710, rv740, rv770.

Cheers,
Jerome