Re: [PATCH v12 3/6] fbmon: add videomode helpers

Started by Laurent Pinchart on Nov 22, 2012 · 2 messages from 2 authors · Last reply Nov 22, 2012

open the first message on its own page

Message details and related content
On Thursday 22 November 2012 09:53:42 Sascha Hauer wrote:
On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote:
quoted
On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote:
quoted
On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote:
quoted
Hi Steffen,
quoted
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest
solution is probably to use 64-bit computation.
You have displays with a pixelclock > 4GHz?
vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus
overflow if the clock frequency is >= ~4.3 MHz. I have displays with a
clock frequency higher than that :-)
If vm->pixelclock is in Hz, then the * 1000 above is wrong.
My bad, I though refresh was expressed in mHz. So yes, the above computation 
is wrong.

BTW it seems that the refreshrate field in struct videomode isn't used. It 
should then be removed.

I've just realized that the struct videomode fields are not documented. 
kerneldoc in include/linux/videomode.h would be a good addition.

-- 
Regards,

Laurent Pinchart
Message details and related content
On Thu, Nov 22, 2012 at 10:07:07AM +0100, Laurent Pinchart wrote:
On Thursday 22 November 2012 09:53:42 Sascha Hauer wrote:
quoted
On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote:
quoted
On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote:
quoted
On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote:
quoted
Hi Steffen,
quoted
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
This will fail if vm->pixelclock >= ((1 << 32) / 1000). The easiest
solution is probably to use 64-bit computation.
You have displays with a pixelclock > 4GHz?
vm->pixelclock is expressed in Hz. vm->pixelclock * 1000 will thus
overflow if the clock frequency is >= ~4.3 MHz. I have displays with a
clock frequency higher than that :-)
If vm->pixelclock is in Hz, then the * 1000 above is wrong.
My bad, I though refresh was expressed in mHz. So yes, the above computation 
is wrong.
Okay. I will fix that with the next version...
BTW it seems that the refreshrate field in struct videomode isn't used. It 
should then be removed.
...and remove this field.
I've just realized that the struct videomode fields are not documented. 
kerneldoc in include/linux/videomode.h would be a good addition.
Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help