Re: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
Hi Fabien,
Em Thu, 11 Jun 2015 11:26:22 +0200
Fabien DESSENNE fabien.desse...@st.com escreveu:
Hi Mauro,
Please check my comments below.
-Original Message-
From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Mauro Carvalho Chehab
Sent: mercredi 10 juin 2015 22:59
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab; Mauro Carvalho Chehab; Fabien DESSENNE
Subject: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
There are several warnings there, on some architectures, related to dividing
a s32 by a s64 value:
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: comparison
of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: right shift
count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning: comparison
of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning: right shift
count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning: passing
argument 1 of '__div64_32' from incompatible pointer type CC [M]
drivers/media/tuners/mt2060.o
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning: comparison
of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning: right shift
count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning: comparison
of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning: right shift
count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
That doesn't make much sense. What the driver is actually trying to do is to
divide one second by a value. So, check the range before dividing. That
warrants the right result and will remove the warnings on non-64 bits archs.
Also fixes this warning:
drivers/media/platform/sti/bdisp/bdisp-debug.c:588: warning: comparison
of distinct pointer types lacks a cast
by using div64_s64() instead of calling do_div() directly.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/platform/sti/bdisp/bdisp-debug.c
b/drivers/media/platform/sti/bdisp/bdisp-debug.c
index 7c3a632746ba..3f6f411aafdd 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-debug.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-debug.c
@@ -572,6 +572,8 @@ static int bdisp_dbg_regs(struct seq_file *s, void
*data)
return 0;
}
+#define SECOND 100
+
static int bdisp_dbg_perf(struct seq_file *s, void *data) {
struct bdisp_dev *bdisp = s-private;
@@ -585,16 +587,27 @@ static int bdisp_dbg_perf(struct seq_file *s, void
*data)
}
avg_time_us = bdisp-dbg.tot_duration;
When using div64_s64 the above line can be deleted, see my next comment.
- do_div(avg_time_us, request-nb_req);
-
- avg_fps = 100;
- min_fps = 100;
- max_fps = 100;
- last_fps = 100;
- do_div(avg_fps, avg_time_us);
- do_div(min_fps, bdisp-dbg.min_duration);
- do_div(max_fps, bdisp-dbg.max_duration);
- do_div(last_fps, bdisp-dbg.last_duration);
+ div64_s64(avg_time_us, request-nb_req);
The operation result is returned by div64_s64(different from do_div that
updates the 1st parameter).
The expected syntax is:
avg_time_us = div64_s64(bdisp-dbg.tot_duration, request-nb_req);
+
+ if (avg_time_us SECOND)
+ avg_fps = 0;
+ else
+ avg_fps = SECOND / (s32)avg_time_us;
+
+ if (bdisp-dbg.min_duration SECOND)
+ min_fps = 0;
+ else
+ min_fps = SECOND / (s32)bdisp-dbg.min_duration);
It probably builds better without the last unexpected parenthesis ;)
Gah, a left-over... I did a first version using a different syntax.
See version 2 below.
+
+ if (bdisp-dbg.max_duration SECOND)
+ max_fps = 0;
+ else
+ max_fps = SECOND / (s32)bdisp-dbg.max_duration;
+
+ if (bdisp-dbg.last_duration SECOND)
+ last_fps = 0;
+ else
+ last_fps = SECOND / (s32)bdisp-dbg.last_duration;
seq_printf(s, HW processing (%d requests):\n, request-nb_req);
seq_printf(s, Average: %5lld us (%3d fps)\n,
--
2.4.2
[PATCHv2] [media] bdisp-debug: don't try to divide by s64
There are several warnings there, on some architectures, related
to dividing a s32 by a s64 value:
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: comparison of
distinct pointer types lacks a cast
RE: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
Hi Mauro,
Please check my comments below.
-Original Message-
From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Mauro Carvalho Chehab
Sent: mercredi 10 juin 2015 22:59
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab; Mauro Carvalho Chehab; Fabien DESSENNE
Subject: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
There are several warnings there, on some architectures, related to dividing
a s32 by a s64 value:
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: comparison
of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: right shift
count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning: comparison
of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning: right shift
count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning: passing
argument 1 of '__div64_32' from incompatible pointer type CC [M]
drivers/media/tuners/mt2060.o
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning: comparison
of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning: right shift
count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning: comparison
of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning: right shift
count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
That doesn't make much sense. What the driver is actually trying to do is to
divide one second by a value. So, check the range before dividing. That
warrants the right result and will remove the warnings on non-64 bits archs.
Also fixes this warning:
drivers/media/platform/sti/bdisp/bdisp-debug.c:588: warning: comparison
of distinct pointer types lacks a cast
by using div64_s64() instead of calling do_div() directly.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/platform/sti/bdisp/bdisp-debug.c
b/drivers/media/platform/sti/bdisp/bdisp-debug.c
index 7c3a632746ba..3f6f411aafdd 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-debug.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-debug.c
@@ -572,6 +572,8 @@ static int bdisp_dbg_regs(struct seq_file *s, void
*data)
return 0;
}
+#define SECOND 100
+
static int bdisp_dbg_perf(struct seq_file *s, void *data) {
struct bdisp_dev *bdisp = s-private;
@@ -585,16 +587,27 @@ static int bdisp_dbg_perf(struct seq_file *s, void
*data)
}
avg_time_us = bdisp-dbg.tot_duration;
When using div64_s64 the above line can be deleted, see my next comment.
- do_div(avg_time_us, request-nb_req);
-
- avg_fps = 100;
- min_fps = 100;
- max_fps = 100;
- last_fps = 100;
- do_div(avg_fps, avg_time_us);
- do_div(min_fps, bdisp-dbg.min_duration);
- do_div(max_fps, bdisp-dbg.max_duration);
- do_div(last_fps, bdisp-dbg.last_duration);
+ div64_s64(avg_time_us, request-nb_req);
The operation result is returned by div64_s64(different from do_div that
updates the 1st parameter).
The expected syntax is:
avg_time_us = div64_s64(bdisp-dbg.tot_duration, request-nb_req);
+
+ if (avg_time_us SECOND)
+ avg_fps = 0;
+ else
+ avg_fps = SECOND / (s32)avg_time_us;
+
+ if (bdisp-dbg.min_duration SECOND)
+ min_fps = 0;
+ else
+ min_fps = SECOND / (s32)bdisp-dbg.min_duration);
It probably builds better without the last unexpected parenthesis ;)
+
+ if (bdisp-dbg.max_duration SECOND)
+ max_fps = 0;
+ else
+ max_fps = SECOND / (s32)bdisp-dbg.max_duration;
+
+ if (bdisp-dbg.last_duration SECOND)
+ last_fps = 0;
+ else
+ last_fps = SECOND / (s32)bdisp-dbg.last_duration;
seq_printf(s, HW processing (%d requests):\n, request-nb_req);
seq_printf(s, Average: %5lld us (%3d fps)\n,
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-media in the
body of a message to majord...@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
Acked-by: Fabien Dessenne fabien.desse...@st.com
-Original Message-
From: Mauro Carvalho Chehab [mailto:mche...@osg.samsung.com]
Sent: jeudi 11 juin 2015 12:37
To: Fabien DESSENNE
Cc: Linux Media Mailing List; Mauro Carvalho Chehab
Subject: Re: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
Hi Fabien,
Em Thu, 11 Jun 2015 11:26:22 +0200
Fabien DESSENNE fabien.desse...@st.com escreveu:
Hi Mauro,
Please check my comments below.
-Original Message-
From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Mauro Carvalho Chehab
Sent: mercredi 10 juin 2015 22:59
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab; Mauro Carvalho Chehab; Fabien DESSENNE
Subject: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
There are several warnings there, on some architectures, related to
dividing a s32 by a s64 value:
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning:
comparison of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: right
shift count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:594: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning:
comparison of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning: right
shift count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:595: warning: passing
argument 1 of '__div64_32' from incompatible pointer type CC [M]
drivers/media/tuners/mt2060.o
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning:
comparison of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning: right
shift count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:596: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning:
comparison of distinct pointer types lacks a cast
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning: right
shift count = width of type
drivers/media/platform/sti/bdisp/bdisp-debug.c:597: warning: passing
argument 1 of '__div64_32' from incompatible pointer type
That doesn't make much sense. What the driver is actually trying to
do is to divide one second by a value. So, check the range before
dividing. That warrants the right result and will remove the warnings on
non-64 bits archs.
Also fixes this warning:
drivers/media/platform/sti/bdisp/bdisp-debug.c:588: warning:
comparison of distinct pointer types lacks a cast
by using div64_s64() instead of calling do_div() directly.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/platform/sti/bdisp/bdisp-debug.c
b/drivers/media/platform/sti/bdisp/bdisp-debug.c
index 7c3a632746ba..3f6f411aafdd 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-debug.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-debug.c
@@ -572,6 +572,8 @@ static int bdisp_dbg_regs(struct seq_file *s,
void
*data)
return 0;
}
+#define SECOND 100
+
static int bdisp_dbg_perf(struct seq_file *s, void *data) {
struct bdisp_dev *bdisp = s-private; @@ -585,16 +587,27 @@ static
int bdisp_dbg_perf(struct seq_file *s, void
*data)
}
avg_time_us = bdisp-dbg.tot_duration;
When using div64_s64 the above line can be deleted, see my next
comment.
- do_div(avg_time_us, request-nb_req);
-
- avg_fps = 100;
- min_fps = 100;
- max_fps = 100;
- last_fps = 100;
- do_div(avg_fps, avg_time_us);
- do_div(min_fps, bdisp-dbg.min_duration);
- do_div(max_fps, bdisp-dbg.max_duration);
- do_div(last_fps, bdisp-dbg.last_duration);
+ div64_s64(avg_time_us, request-nb_req);
The operation result is returned by div64_s64(different from do_div that
updates the 1st parameter).
The expected syntax is:
avg_time_us = div64_s64(bdisp-dbg.tot_duration, request-nb_req);
+
+ if (avg_time_us SECOND)
+ avg_fps = 0;
+ else
+ avg_fps = SECOND / (s32)avg_time_us;
+
+ if (bdisp-dbg.min_duration SECOND)
+ min_fps = 0;
+ else
+ min_fps = SECOND / (s32)bdisp-dbg.min_duration);
It probably builds better without the last unexpected parenthesis ;)
Gah, a left-over... I did a first version using a different syntax.
See version 2 below.
+
+ if (bdisp-dbg.max_duration SECOND)
+ max_fps = 0;
+ else
+ max_fps = SECOND / (s32)bdisp-dbg.max_duration;
+
+ if (bdisp-dbg.last_duration SECOND)
+ last_fps = 0;
+ else
+ last_fps = SECOND / (s32)bdisp-dbg.last_duration;
seq_printf(s, HW processing (%d requests):\n
