* [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