* [PATCH 1/2] [media] cx88: don't declare restart_video_queue if not used
@ 2015-06-10 20:59 Mauro Carvalho Chehab
2015-06-10 20:59 ` [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64 Mauro Carvalho Chehab
0 siblings, 1 reply; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-10 20:59 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil
While compiled on alpha, got this error:
drivers/media/pci/cx88/cx88-video.c:415:12: warning: 'restart_video_queue' defined but not used [-Wunused-function]
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
diff --git a/drivers/media/pci/cx88/cx88-video.c b/drivers/media/pci/cx88/cx88-video.c
index cebb07d87617..400e5caefd58 100644
--- a/drivers/media/pci/cx88/cx88-video.c
+++ b/drivers/media/pci/cx88/cx88-video.c
@@ -410,7 +410,6 @@ static int stop_video_dma(struct cx8800_dev *dev)
cx_clear(MO_VID_INTMSK, 0x0f0011);
return 0;
}
-#endif
static int restart_video_queue(struct cx8800_dev *dev,
struct cx88_dmaqueue *q)
@@ -426,6 +425,7 @@ static int restart_video_queue(struct cx8800_dev *dev,
}
return 0;
}
+#endif
/* ------------------------------------------------------------------ */
--
2.4.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
2015-06-10 20:59 [PATCH 1/2] [media] cx88: don't declare restart_video_queue if not used Mauro Carvalho Chehab
@ 2015-06-10 20:59 ` Mauro Carvalho Chehab
2015-06-11 9:26 ` Fabien DESSENNE
0 siblings, 1 reply; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-10 20:59 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Fabien Dessenne
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 <mchehab@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 1000000
+
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;
- do_div(avg_time_us, request->nb_req);
-
- avg_fps = 1000000;
- min_fps = 1000000;
- max_fps = 1000000;
- last_fps = 1000000;
- 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);
+
+ 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);
+
+ 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
2015-06-10 20:59 ` [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64 Mauro Carvalho Chehab
@ 2015-06-11 9:26 ` Fabien DESSENNE
2015-06-11 10:37 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 5+ messages in thread
From: Fabien DESSENNE @ 2015-06-11 9:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List; +Cc: Mauro Carvalho Chehab
Hi Mauro,
Please check my comments below.
> -----Original Message-----
> From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> owner@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 <mchehab@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 1000000
> +
> 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 = 1000000;
> - min_fps = 1000000;
> - max_fps = 1000000;
> - last_fps = 1000000;
> - 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 majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
2015-06-11 9:26 ` Fabien DESSENNE
@ 2015-06-11 10:37 ` Mauro Carvalho Chehab
2015-06-11 12:27 ` Fabien DESSENNE
0 siblings, 1 reply; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-06-11 10:37 UTC (permalink / raw)
To: Fabien DESSENNE; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Hi Fabien,
Em Thu, 11 Jun 2015 11:26:22 +0200
Fabien DESSENNE <fabien.dessenne@st.com> escreveu:
> Hi Mauro,
>
> Please check my comments below.
>
> > -----Original Message-----
> > From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> > owner@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 <mchehab@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 1000000
> > +
> > 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 = 1000000;
> > - min_fps = 1000000;
> > - max_fps = 1000000;
> > - last_fps = 1000000;
> > - 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
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 <mchehab@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..18282a0f80c9 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 1000000
+
static int bdisp_dbg_perf(struct seq_file *s, void *data)
{
struct bdisp_dev *bdisp = s->private;
@@ -584,17 +586,26 @@ static int bdisp_dbg_perf(struct seq_file *s, void *data)
return 0;
}
- avg_time_us = bdisp->dbg.tot_duration;
- do_div(avg_time_us, request->nb_req);
-
- avg_fps = 1000000;
- min_fps = 1000000;
- max_fps = 1000000;
- last_fps = 1000000;
- 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);
+ 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;
+
+ 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",
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64
2015-06-11 10:37 ` Mauro Carvalho Chehab
@ 2015-06-11 12:27 ` Fabien DESSENNE
0 siblings, 0 replies; 5+ messages in thread
From: Fabien DESSENNE @ 2015-06-11 12:27 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Mauro Carvalho Chehab
Acked-by: Fabien Dessenne <fabien.dessenne@st.com>
> -----Original Message-----
> From: Mauro Carvalho Chehab [mailto:mchehab@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.dessenne@st.com> escreveu:
>
> > Hi Mauro,
> >
> > Please check my comments below.
> >
> > > -----Original Message-----
> > > From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> > > owner@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 <mchehab@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 1000000
> > > +
> > > 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 = 1000000;
> > > - min_fps = 1000000;
> > > - max_fps = 1000000;
> > > - last_fps = 1000000;
> > > - 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
> 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 <mchehab@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..18282a0f80c9 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 1000000
> +
> static int bdisp_dbg_perf(struct seq_file *s, void *data) {
> struct bdisp_dev *bdisp = s->private;
> @@ -584,17 +586,26 @@ static int bdisp_dbg_perf(struct seq_file *s, void
> *data)
> return 0;
> }
>
> - avg_time_us = bdisp->dbg.tot_duration;
> - do_div(avg_time_us, request->nb_req);
> -
> - avg_fps = 1000000;
> - min_fps = 1000000;
> - max_fps = 1000000;
> - last_fps = 1000000;
> - 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);
> + 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;
> +
> + 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",
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-11 12:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-10 20:59 [PATCH 1/2] [media] cx88: don't declare restart_video_queue if not used Mauro Carvalho Chehab
2015-06-10 20:59 ` [PATCH 2/2] [media] bdisp-debug: don't try to divide by s64 Mauro Carvalho Chehab
2015-06-11 9:26 ` Fabien DESSENNE
2015-06-11 10:37 ` Mauro Carvalho Chehab
2015-06-11 12:27 ` Fabien DESSENNE
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).