[Mesa-dev] r600/sb loop issue

[Vadim Girlin]

On 12/16/2014 05:44 AM, Dave Airlie wrote:
> On 16 December 2014 at 08:59, Vadim Girlin <vadimgirlin at gmail.com> wrote:
>> On 12/16/2014 01:30 AM, Dave Airlie wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> New patch is attached, the only difference is in the sb_sched.cpp (it
>>>>>> disables copy coalescing for some "unsafe" cases, so it may leave more
>>>>>> MOVs
>>>>>> than previously, but I don't think there will be any noticeable effect
>>>>>> on
>>>>>> performance).
>>>>>>
>>>>>> So far I don't see any problems with it, but I don't have many GL apps
>>>>>> on
>>>>>> the test machine. At least lightsmark and unigine demos work for me.
>>>>>>
>>>>>
>>>>> Based on my limited understanding of the code:
>>>>>
>>>>> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>>>>
>>>>
>>>>
>>>> Alex, thanks for the review, I understand you wanted it to get into mesa
>>>> release, but it really needs careful testing with more apps, so far I
>>>> hoped
>>>> Dave would do it as long as he's looking into these issues anyway. In
>>>> theory
>>>> I can also install steam on the test machine and some games, it just
>>>> needs
>>>> the time and I'm not sure if I'll find it, so far my main job is
>>>> sufficient
>>>> to make me pretty tired.
>>>>
>>>> Current scheduler in SB is very fragile after adding handling for all
>>>> special cases discovered during initial debugging etc, I said since the
>>>> very
>>>> beginning that I'd like to rewrite it, if only I had time. So any change
>>>> like this can potentially break some apps even if piglit passes, and I'm
>>>> not
>>>> ready to take responsibility for that if I commit it myself, I just don't
>>>> have time to deal with all possible consequences on all supported chips.
>>>>
>>>> If you think it's ok, just push this patch (it requires revert of the
>>>> previous Dave's commit 7b0067d2). I'm really sorry that I can't do more
>>>> to
>>>> help with it.
>>>
>>>
>>> Myself and Glenn are looking at it, Glenn noticed a piglit regression
>>> from this yesterday, I'll reproduce today and take a look.
>>
>>
>> Hi, Dave & Glenn,
>>
>> Thanks for looking into it. FWIW, when I worked on it I've ran piglit's
>> quick tests and didn't see any regressions on evergreen (juniper 5750).
>> There were some failed tests in some piglit runs, but AFAIU they were just
>> random.
>
> Turns out we had a pre-existing fail that we noticed, not a regression.
>
> I'm going to push this, since its better than what is there, we can
> see if some public testing notices any big issues also.

Thanks, Dave. I'm really sorry that I can't pay as much attention to 
that code as I'd like, and I really appreciate your and Glenn's efforts 
for maintaining it.

(In case if someone thinks it's my fault, I must remind, I warned that I 
won't be able to support it even before it was merged. So please don't 
blame me :) ).