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

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