[Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

[Samuel Pitoiset]

On 06/06/2016 12:08 AM, Emil Velikov wrote:
> On 5 June 2016 at 23:00, Samuel Pitoiset <samuel.pitoiset at gmail.com> wrote:
>>
>>
>> On 06/05/2016 11:50 PM, Emil Velikov wrote:
>>>
>>> On 5 June 2016 at 22:36, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>
>>>> On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov <emil.l.velikov at gmail.com>
>>>> wrote:
>>>>>
>>>>> On 5 June 2016 at 22:17, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>>>
>>>>>> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov <emil.l.velikov at gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> On 5 June 2016 at 22:13, Emil Velikov <emil.l.velikov at gmail.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> On 5 June 2016 at 17:56, Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> We should not call nouveau_bufctx_reset() inside a validate
>>>>>>>>> function.
>>>>>>>>
>>>>>>>> This seems to contradict the changes introduced in nvc0_compute.c.
>>>>>>>> Worth explaining a bit better the dos and don'ts ?
>>>>>>>>
>>>>>>> As this is already in master, can you please provide a more
>>>>>>> elaborate/correct summary for -stable ?
>>>>>>
>>>>>>
>>>>>> I think it's fine as is.
>>>>>>
>>>>>> Do: reset bufctx when setting dirty bit
>>>>>> Don't: reset bufctx in validate logic, since it's "too late" by then.
>>>>>> (Not strictly wrong, but just should do it earlier.)
>>>>>
>>>>>
>>>>> So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
>>>>> what a confusing name it has ;-)
>>>>
>>>>
>>>> It validates compute. And it invalidates (and clears) the 3d bin.
>>>>
>>> So one can reset_bufctx(3d) from the compute validate and vice-versa.
>>> While doing reset_bufctx(foo) from foo validate is a bad idea ?
>>> Shouldn't one just say so in the commit message ?
>>
>>
>> Because the common practice is to clear foo bins at the same place where the
>> dirty_3d |= foo is updated, this makes sense. :)
>>
> Yet the commit message does not say that, right ? It says "We should
> not call nouveau_bufctx_reset() inside a validate function.", while
> the patch does the complete opposite - it adds a call to
> nouveau_bufctx_reset() inside a validate function.
>
> All I'm asking is for the commit message to reflect the code change or
> vice-versa. I hope I'm not being unreasonable ?

You are reasonable. I assume something like "nvc0: clear out surfaces 
bins at the right place" (and explain why in the description) is more 
close to what the code does.

>
> Thanks
> Emil
>