On Mon, Oct 19, 2015 at 11:55 AM, Connor Abbott <cwabbo...@gmail.com> wrote:
> On Mon, Oct 19, 2015 at 2:29 PM, Matt Turner <matts...@gmail.com> wrote:
>> On Mon, Oct 19, 2015 at 7:45 AM, Emil Velikov <emil.l.veli...@gmail.com>
>> wrote:
>>> We're about to reuse get_timestamp() for the nir_intrinsic_shader_clock.
>>> The in the latter the generalisation does not apply, so move the smear()
>>> where needed. This also makes the function analogous to the vec4 one.
>>>
>>> Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com>
>>> ---
>>> src/mesa/drivers/dri/i965/brw_fs.cpp | 30 +++++++++++++++++++++---------
>>> 1 file changed, 21 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> index a2fd441..81a6a29 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> @@ -521,6 +521,14 @@ fs_visitor::get_timestamp(const fs_builder &bld)
>>> */
>>> bld.group(4, 0).exec_all().MOV(dst, ts);
>>>
>>> + return dst;
>>> +}
>>> +
>>> +void
>>> +fs_visitor::emit_shader_time_begin()
>>> +{
>>> + shader_start_time = get_timestamp(bld.annotate("shader time start"));
>>> +
>>> /* The caller wants the low 32 bits of the timestamp. Since it's
>>> running
>>> * at the GPU clock rate of ~1.2ghz, it will roll over every ~3 seconds,
>>> * which is plenty of time for our purposes. It is identical across the
>>> @@ -531,15 +539,7 @@ fs_visitor::get_timestamp(const fs_builder &bld)
>>> * else that might disrupt timing) by setting smear to 2 and checking if
>>> * that field is != 0.
>>> */
>>> - dst.set_smear(0);
>>> -
>>> - return dst;
>>> -}
>>> -
>>> -void
>>> -fs_visitor::emit_shader_time_begin()
>>> -{
>>> - shader_start_time = get_timestamp(bld.annotate("shader time start"));
>>> + shader_start_time.set_smear(0);
>>> }
>>>
>>> void
>>> @@ -553,6 +553,18 @@ fs_visitor::emit_shader_time_end()
>>>
>>> fs_reg shader_end_time = get_timestamp(ibld);
>>>
>>> + /* The caller wants the low 32 bits of the timestamp. Since it's
>>> running
>>> + * at the GPU clock rate of ~1.2ghz, it will roll over every ~3 seconds,
>>> + * which is plenty of time for our purposes. It is identical across the
>>> + * EUs, but since it's tracking GPU core speed it will increment at a
>>> + * varying rate as render P-states change.
>>> + *
>>> + * The caller could also check if render P-states have changed (or
>>> anything
>>> + * else that might disrupt timing) by setting smear to 2 and checking if
>>> + * that field is != 0.
>>> + */
>>
>> I wouldn't bother copying the comment. It's immediately above this
>> function and very visible if reading this code.
>
> Wouldn't it be better to leave the comment here, replacing "The
> caller" with "we" and delete it from get_timestamp though? Especially
> since get_timestamp() is going to be used from the shader_clock
> implementation which actually wants the high 32 bits too.
Yep, that seems like a good plan.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
