linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).