| Message ID | 1353426896-6045-4-git-send-email-s.trumtrar@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Steffen, I am trying to add DT support for da8xx-fb driver on top of your patches. Encountered below build error. Sorry for reporting it late. On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote: > Add a function to convert from the generic videomode to a fb_videomode. > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de> > Acked-by: Thierry Reding <thierry.reding@avionic-design.de> > Tested-by: Thierry Reding <thierry.reding@avionic-design.de> > Tested-by: Philipp Zabel <p.zabel@pengutronix.de> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/video/fbmon.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/fb.h | 6 ++++++ > 2 files changed, 52 insertions(+) > > diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c > index cef6557..c1939a6 100644 > --- a/drivers/video/fbmon.c > +++ b/drivers/video/fbmon.c > @@ -31,6 +31,7 @@ > #include <linux/pci.h> > #include <linux/slab.h> > #include <video/edid.h> > +#include <linux/videomode.h> > #ifdef CONFIG_PPC_OF > #include <asm/prom.h> > #include <asm/pci-bridge.h> > @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf > kfree(timings); > return err; > } > + > +#if IS_ENABLED(CONFIG_VIDEOMODE) > +int fb_videomode_from_videomode(const struct videomode *vm, > + struct fb_videomode *fbmode) > +{ > + unsigned int htotal, vtotal; > + > + fbmode->xres = vm->hactive; > + fbmode->left_margin = vm->hback_porch; > + fbmode->right_margin = vm->hfront_porch; > + fbmode->hsync_len = vm->hsync_len; > + > + fbmode->yres = vm->vactive; > + fbmode->upper_margin = vm->vback_porch; > + fbmode->lower_margin = vm->vfront_porch; > + fbmode->vsync_len = vm->vsync_len; > + > + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000); > + > + fbmode->sync = 0; > + fbmode->vmode = 0; > + if (vm->hah) > + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT; > + if (vm->vah) > + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT; > + if (vm->interlaced) > + fbmode->vmode |= FB_VMODE_INTERLACED; > + if (vm->doublescan) > + fbmode->vmode |= FB_VMODE_DOUBLE; > + if (vm->de) > + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT; "FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting build error on this. Please let me know if I am missing something. Thanks, Prakash > + fbmode->flag = 0; > + > + 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); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode); > +#endif > + > + > #else > int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var) > { > diff --git a/include/linux/fb.h b/include/linux/fb.h > index c7a9571..920cbe3 100644 > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -14,6 +14,7 @@ > #include <linux/backlight.h> > #include <linux/slab.h> > #include <asm/io.h> > +#include <linux/videomode.h> > > struct vm_area_struct; > struct fb_info; > @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb); > extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb); > extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter); > > +#if IS_ENABLED(CONFIG_VIDEOMODE) > +extern int fb_videomode_from_videomode(const struct videomode *vm, > + struct fb_videomode *fbmode); > +#endif > + > /* drivers/video/modedb.c */ > #define VESA_MODEDB_SIZE 34 > extern void fb_var_to_videomode(struct fb_videomode *mode, > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Yes, Even I got the same build error. later I fixed it by including "#include <linux/mxsfb.h>" Best Wishes, Leela Krishna. On Wed, Nov 21, 2012 at 3:39 PM, Manjunathappa, Prakash <prakash.pm@ti.com> wrote: > Hi Steffen, > > I am trying to add DT support for da8xx-fb driver on top of your patches. > Encountered below build error. Sorry for reporting it late. > > On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote: >> Add a function to convert from the generic videomode to a fb_videomode. >> >> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> >> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de> >> Acked-by: Thierry Reding <thierry.reding@avionic-design.de> >> Tested-by: Thierry Reding <thierry.reding@avionic-design.de> >> Tested-by: Philipp Zabel <p.zabel@pengutronix.de> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> --- >> drivers/video/fbmon.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/fb.h | 6 ++++++ >> 2 files changed, 52 insertions(+) >> >> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c >> index cef6557..c1939a6 100644 >> --- a/drivers/video/fbmon.c >> +++ b/drivers/video/fbmon.c >> @@ -31,6 +31,7 @@ >> #include <linux/pci.h> >> #include <linux/slab.h> >> #include <video/edid.h> >> +#include <linux/videomode.h> >> #ifdef CONFIG_PPC_OF >> #include <asm/prom.h> >> #include <asm/pci-bridge.h> >> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf >> kfree(timings); >> return err; >> } >> + >> +#if IS_ENABLED(CONFIG_VIDEOMODE) >> +int fb_videomode_from_videomode(const struct videomode *vm, >> + struct fb_videomode *fbmode) >> +{ >> + unsigned int htotal, vtotal; >> + >> + fbmode->xres = vm->hactive; >> + fbmode->left_margin = vm->hback_porch; >> + fbmode->right_margin = vm->hfront_porch; >> + fbmode->hsync_len = vm->hsync_len; >> + >> + fbmode->yres = vm->vactive; >> + fbmode->upper_margin = vm->vback_porch; >> + fbmode->lower_margin = vm->vfront_porch; >> + fbmode->vsync_len = vm->vsync_len; >> + >> + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000); >> + >> + fbmode->sync = 0; >> + fbmode->vmode = 0; >> + if (vm->hah) >> + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT; >> + if (vm->vah) >> + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT; >> + if (vm->interlaced) >> + fbmode->vmode |= FB_VMODE_INTERLACED; >> + if (vm->doublescan) >> + fbmode->vmode |= FB_VMODE_DOUBLE; >> + if (vm->de) >> + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT; > > "FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting > build error on this. Please let me know if I am missing something. > > Thanks, > Prakash > >> + fbmode->flag = 0; >> + >> + 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); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode); >> +#endif >> + >> + >> #else >> int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var) >> { >> diff --git a/include/linux/fb.h b/include/linux/fb.h >> index c7a9571..920cbe3 100644 >> --- a/include/linux/fb.h >> +++ b/include/linux/fb.h >> @@ -14,6 +14,7 @@ >> #include <linux/backlight.h> >> #include <linux/slab.h> >> #include <asm/io.h> >> +#include <linux/videomode.h> >> >> struct vm_area_struct; >> struct fb_info; >> @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb); >> extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb); >> extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter); >> >> +#if IS_ENABLED(CONFIG_VIDEOMODE) >> +extern int fb_videomode_from_videomode(const struct videomode *vm, >> + struct fb_videomode *fbmode); >> +#endif >> + >> /* drivers/video/modedb.c */ >> #define VESA_MODEDB_SIZE 34 >> extern void fb_var_to_videomode(struct fb_videomode *mode, >> -- >> 1.7.10.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > _______________________________________________ > devicetree-discuss mailing list > devicetree-discuss@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss
Hi! On Wed, Nov 21, 2012 at 04:39:01PM +0530, Leela Krishna Amudala wrote: > Yes, > Even I got the same build error. > later I fixed it by including "#include <linux/mxsfb.h>" > > Best Wishes, > Leela Krishna. > > On Wed, Nov 21, 2012 at 3:39 PM, Manjunathappa, Prakash > <prakash.pm@ti.com> wrote: > > Hi Steffen, > > > > I am trying to add DT support for da8xx-fb driver on top of your patches. > > Encountered below build error. Sorry for reporting it late. > > > > On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote: > >> Add a function to convert from the generic videomode to a fb_videomode. > >> > >> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > >> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de> > >> Acked-by: Thierry Reding <thierry.reding@avionic-design.de> > >> Tested-by: Thierry Reding <thierry.reding@avionic-design.de> > >> Tested-by: Philipp Zabel <p.zabel@pengutronix.de> > >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> --- > >> drivers/video/fbmon.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > >> include/linux/fb.h | 6 ++++++ > >> 2 files changed, 52 insertions(+) > >> > >> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c > >> index cef6557..c1939a6 100644 > >> --- a/drivers/video/fbmon.c > >> +++ b/drivers/video/fbmon.c > >> @@ -31,6 +31,7 @@ > >> #include <linux/pci.h> > >> #include <linux/slab.h> > >> #include <video/edid.h> > >> +#include <linux/videomode.h> > >> #ifdef CONFIG_PPC_OF > >> #include <asm/prom.h> > >> #include <asm/pci-bridge.h> > >> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf > >> kfree(timings); > >> return err; > >> } > >> + > >> +#if IS_ENABLED(CONFIG_VIDEOMODE) > >> +int fb_videomode_from_videomode(const struct videomode *vm, > >> + struct fb_videomode *fbmode) > >> +{ > >> + unsigned int htotal, vtotal; > >> + > >> + fbmode->xres = vm->hactive; > >> + fbmode->left_margin = vm->hback_porch; > >> + fbmode->right_margin = vm->hfront_porch; > >> + fbmode->hsync_len = vm->hsync_len; > >> + > >> + fbmode->yres = vm->vactive; > >> + fbmode->upper_margin = vm->vback_porch; > >> + fbmode->lower_margin = vm->vfront_porch; > >> + fbmode->vsync_len = vm->vsync_len; > >> + > >> + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000); > >> + > >> + fbmode->sync = 0; > >> + fbmode->vmode = 0; > >> + if (vm->hah) > >> + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT; > >> + if (vm->vah) > >> + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT; > >> + if (vm->interlaced) > >> + fbmode->vmode |= FB_VMODE_INTERLACED; > >> + if (vm->doublescan) > >> + fbmode->vmode |= FB_VMODE_DOUBLE; > >> + if (vm->de) > >> + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT; > > > > "FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting > > build error on this. Please let me know if I am missing something. > > > > Thanks, > > Prakash > > I compile-tested the series and didn't have that error. But obviously I should have. As this is a mxsfs flag, I will throw it out. Regards, Steffen
On 2012-11-20 17:54, Steffen Trumtrar wrote: > diff --git a/include/linux/fb.h b/include/linux/fb.h > index c7a9571..920cbe3 100644 > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -14,6 +14,7 @@ > #include <linux/backlight.h> > #include <linux/slab.h> > #include <asm/io.h> > +#include <linux/videomode.h> No need for this, just add "struct xxx;". > struct vm_area_struct; > struct fb_info; > @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb); > extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb); > extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter); > > +#if IS_ENABLED(CONFIG_VIDEOMODE) > +extern int fb_videomode_from_videomode(const struct videomode *vm, > + struct fb_videomode *fbmode); > +#endif > + > /* drivers/video/modedb.c */ > #define VESA_MODEDB_SIZE 34 > extern void fb_var_to_videomode(struct fb_videomode *mode, >
Hi Steffen, Sorry, I've just found another small bug below. On Tuesday 20 November 2012 16:54:53 Steffen Trumtrar wrote: > Add a function to convert from the generic videomode to a fb_videomode. > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de> > Acked-by: Thierry Reding <thierry.reding@avionic-design.de> > Tested-by: Thierry Reding <thierry.reding@avionic-design.de> > Tested-by: Philipp Zabel <p.zabel@pengutronix.de> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/video/fbmon.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/fb.h | 6 ++++++ > 2 files changed, 52 insertions(+) > > diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c > index cef6557..c1939a6 100644 > --- a/drivers/video/fbmon.c > +++ b/drivers/video/fbmon.c > @@ -31,6 +31,7 @@ > #include <linux/pci.h> > #include <linux/slab.h> > #include <video/edid.h> > +#include <linux/videomode.h> > #ifdef CONFIG_PPC_OF > #include <asm/prom.h> > #include <asm/pci-bridge.h> > @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct > fb_var_screeninfo *var, struct fb_inf kfree(timings); > return err; > } > + > +#if IS_ENABLED(CONFIG_VIDEOMODE) > +int fb_videomode_from_videomode(const struct videomode *vm, > + struct fb_videomode *fbmode) > +{ > + unsigned int htotal, vtotal; > + > + fbmode->xres = vm->hactive; > + fbmode->left_margin = vm->hback_porch; > + fbmode->right_margin = vm->hfront_porch; > + fbmode->hsync_len = vm->hsync_len; > + > + fbmode->yres = vm->vactive; > + fbmode->upper_margin = vm->vback_porch; > + fbmode->lower_margin = vm->vfront_porch; > + fbmode->vsync_len = vm->vsync_len; > + > + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000); > + > + fbmode->sync = 0; > + fbmode->vmode = 0; > + if (vm->hah) > + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT; > + if (vm->vah) > + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT; > + if (vm->interlaced) > + fbmode->vmode |= FB_VMODE_INTERLACED; > + if (vm->doublescan) > + fbmode->vmode |= FB_VMODE_DOUBLE; > + if (vm->de) > + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT; > + fbmode->flag = 0; > + > + 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. > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode); > +#endif > + > + > #else > int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var) > { > diff --git a/include/linux/fb.h b/include/linux/fb.h > index c7a9571..920cbe3 100644 > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -14,6 +14,7 @@ > #include <linux/backlight.h> > #include <linux/slab.h> > #include <asm/io.h> > +#include <linux/videomode.h> > > struct vm_area_struct; > struct fb_info; > @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode > *modedb); extern int fb_find_mode_cvt(struct fb_videomode *mode, int > margins, int rb); extern unsigned char *fb_ddc_read(struct i2c_adapter > *adapter); > > +#if IS_ENABLED(CONFIG_VIDEOMODE) > +extern int fb_videomode_from_videomode(const struct videomode *vm, > + struct fb_videomode *fbmode); > +#endif > + > /* drivers/video/modedb.c */ > #define VESA_MODEDB_SIZE 34 > extern void fb_var_to_videomode(struct fb_videomode *mode,
Hi Laurent, On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > Hi Steffen, > > > + > > + 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? Sascha
Hi Sascha, On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote: > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > > Hi Steffen, > > > > > + > > > + 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 :-)
On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote: > Hi Sascha, > > On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote: > > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > > > Hi Steffen, > > > > > > > + > > > > + 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. Sascha
On Thursday 22 November 2012 09:53:42 Sascha Hauer wrote: > On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote: > > On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote: > > > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > > > > Hi Steffen, > > > > > > > > > + > > > > > + 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.
On Thu, Nov 22, 2012 at 10:07:07AM +0100, Laurent Pinchart wrote: > On Thursday 22 November 2012 09:53:42 Sascha Hauer wrote: > > On Thu, Nov 22, 2012 at 09:50:10AM +0100, Laurent Pinchart wrote: > > > On Thursday 22 November 2012 07:20:00 Sascha Hauer wrote: > > > > On Wed, Nov 21, 2012 at 11:02:44PM +0100, Laurent Pinchart wrote: > > > > > Hi Steffen, > > > > > > > > > > > + > > > > > > + 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
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c index cef6557..c1939a6 100644 --- a/drivers/video/fbmon.c +++ b/drivers/video/fbmon.c @@ -31,6 +31,7 @@ #include <linux/pci.h> #include <linux/slab.h> #include <video/edid.h> +#include <linux/videomode.h> #ifdef CONFIG_PPC_OF #include <asm/prom.h> #include <asm/pci-bridge.h> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf kfree(timings); return err; } + +#if IS_ENABLED(CONFIG_VIDEOMODE) +int fb_videomode_from_videomode(const struct videomode *vm, + struct fb_videomode *fbmode) +{ + unsigned int htotal, vtotal; + + fbmode->xres = vm->hactive; + fbmode->left_margin = vm->hback_porch; + fbmode->right_margin = vm->hfront_porch; + fbmode->hsync_len = vm->hsync_len; + + fbmode->yres = vm->vactive; + fbmode->upper_margin = vm->vback_porch; + fbmode->lower_margin = vm->vfront_porch; + fbmode->vsync_len = vm->vsync_len; + + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000); + + fbmode->sync = 0; + fbmode->vmode = 0; + if (vm->hah) + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT; + if (vm->vah) + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT; + if (vm->interlaced) + fbmode->vmode |= FB_VMODE_INTERLACED; + if (vm->doublescan) + fbmode->vmode |= FB_VMODE_DOUBLE; + if (vm->de) + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT; + fbmode->flag = 0; + + 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); + + return 0; +} +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode); +#endif + + #else int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var) { diff --git a/include/linux/fb.h b/include/linux/fb.h index c7a9571..920cbe3 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -14,6 +14,7 @@ #include <linux/backlight.h> #include <linux/slab.h> #include <asm/io.h> +#include <linux/videomode.h> struct vm_area_struct; struct fb_info; @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb); extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb); extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter); +#if IS_ENABLED(CONFIG_VIDEOMODE) +extern int fb_videomode_from_videomode(const struct videomode *vm, + struct fb_videomode *fbmode); +#endif + /* drivers/video/modedb.c */ #define VESA_MODEDB_SIZE 34 extern void fb_var_to_videomode(struct fb_videomode *mode,