[PATCH] drm/amd/display: Fix 64-bit division, yet again

[PATCH] drm/amd/display: Fix 64-bit division, yet again

[Harry Wentland]

Also make the code somewhat more readable.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index b00b1df71f3e..2026ef34b11b 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -210,6 +210,8 @@ bool mod_freesync_add_stream(struct mod_freesync *mod_freesync,
 	struct core_freesync *core_freesync = NULL;
 	int persistent_freesync_enable = 0;
 	struct persistent_data_flag flag;
+	unsigned int nom_refresh_rate_micro_hz;
+	unsigned long long temp;
 
 	if (mod_freesync == NULL)
 		return false;
@@ -262,11 +264,16 @@ bool mod_freesync_add_stream(struct mod_freesync *mod_freesync,
 					enable_for_video = false;
 		}
 
-		unsigned int nom_refresh_rate_micro_hz = (unsigned int)
-				(((unsigned long long) core_stream->public.timing.pix_clk_khz) * 1000ULL * 1000ULL * 1000ULL
-				/ core_stream->public.timing.h_total / core_stream->public.timing.v_total);
+		temp = core_stream->public.timing.pix_clk_khz;
+		temp *= 1000ULL * 1000ULL * 1000ULL;
+		temp = div_u64(temp, core_stream->public.timing.h_total);
+		temp = div_u64(temp, core_stream->public.timing.v_total);
 
-		if (caps->supported && nom_refresh_rate_micro_hz >= caps->min_refresh_in_micro_hz && nom_refresh_rate_micro_hz <= caps->max_refresh_in_micro_hz)
+		nom_refresh_rate_micro_hz = (unsigned int) temp;
+
+		if (caps->supported &&
+		    nom_refresh_rate_micro_hz >= caps->min_refresh_in_micro_hz &&
+		    nom_refresh_rate_micro_hz <= caps->max_refresh_in_micro_hz)
 			core_stream->public.ignore_msa_timing_param = 1;
 
 		core_freesync->num_entities++;
-- 
2.9.3

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Fix 64-bit division, yet again

[Emil Velikov]

On 14 February 2017 at 20:36, Harry Wentland <harry.wentland@amd.com> wrote:
> Also make the code somewhat more readable.
>
I'd suggest reaching to the team to integrate scripts/checkpatch.pl in
the pre-commit hook.

It will help you improve the coding standard and, as you mentioned, it
"make[s] the code somewhat more readable".

-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Fix 64-bit division, yet again

[Harry Wentland]

On 2017-02-15 06:44 AM, Emil Velikov wrote:
> On 14 February 2017 at 20:36, Harry Wentland <harry.wentland@amd.com> wrote:
>> Also make the code somewhat more readable.
>>
> I'd suggest reaching to the team to integrate scripts/checkpatch.pl in
> the pre-commit hook.
>
> It will help you improve the coding standard and, as you mentioned, it
> "make[s] the code somewhat more readable".
>

Thanks, Emil. We actually have that hooked up at pre-submission in our 
internal repo but don't block submission on warnings and allow a couple 
people to push anyways despite errors because we seem to get some 
unnecessary errors on complex macros that can't really be handled 
differently.

Need to think on how to improve that. It's a pain being the bad guy 
constantly and people don't always see the importance of good coding and 
commit msg style.

Harry

> -Emil
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Fix 64-bit division, yet again

[Emil Velikov]

On 15 February 2017 at 14:47, Harry Wentland <harry.wentland@amd.com> wrote:
> On 2017-02-15 06:44 AM, Emil Velikov wrote:
>>
>> On 14 February 2017 at 20:36, Harry Wentland <harry.wentland@amd.com>
>> wrote:
>>>
>>> Also make the code somewhat more readable.
>>>
>> I'd suggest reaching to the team to integrate scripts/checkpatch.pl in
>> the pre-commit hook.
>>
>> It will help you improve the coding standard and, as you mentioned, it
>> "make[s] the code somewhat more readable".
>>
>
> Thanks, Emil. We actually have that hooked up at pre-submission in our
> internal repo but don't block submission on warnings and allow a couple
> people to push anyways despite errors because we seem to get some
> unnecessary errors on complex macros that can't really be handled
> differently.
>
Some ideas:
 - nobody is exempt - everyone has to go through the checks/hooks
 - can not push if there are more than X warnings and Y errors
 - whitelist patches that include changes to the problematic macros
Btw, do you recall which macros are responsible?

> Need to think on how to improve that. It's a pain being the bad guy
> constantly and people don't always see the importance of good coding and
> commit msg style.
>
One possible solution is to check that the commit message is at least
A% of the diffstat - refusing to commit + sending a bunch of links
about why you want good commit messages.

Afaict all of the above can be automated, thus nobody will be an
exception to the rule. With time all that can go away, but until then
it will save you/Alex a) time and b) being the bad guys.
It will take a few rounds to tweaking the X/Y/A numbers though ;-)

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx