On Thu, 28 Oct 2010 14:34:44 +0200 (CEST) Guennadi Liakhovetski <g.liakhovetski@xxxxxx> wrote: > From: Erik Gilling <konkers@xxxxxxxxxxx> > > Modern monitors/tvs have more extended EDID information blocks which can > contain extra detailed modes. This adds a fb_edid_add_monspecs function > which drivers can use to parse those additions blocks. > > Signed-off-by: Erik Gilling <konkers@xxxxxxxxxxx> > --- > drivers/video/fbmon.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/fb.h | 2 + > 2 files changed, 59 insertions(+), 0 deletions(-) > > diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c > index 563a98b..a0b5a93 100644 > --- a/drivers/video/fbmon.c > +++ b/drivers/video/fbmon.c > @@ -973,6 +973,63 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs) > DPRINTK("========================================\n"); > } > > +void fb_edid_add_monspecs(unsigned char *edid, struct fb_monspecs *specs) Please document it. This is a general rule when implementing library-style functions. > +{ > + unsigned char *block; > + struct fb_videomode *mode, *m; > + int num = 0, i, first = 1; > + > + if (edid == NULL) > + return; > + > + if (!edid_checksum(edid)) > + return; > + > + if (edid[0] != 0x2) > + return; > + > + mode = kzalloc(50 * sizeof(struct fb_videomode), GFP_KERNEL); > + if (mode == NULL) > + return; > + > + block = edid + edid[0x2]; > + > + DPRINTK(" Extended Detailed Timings\n"); > + > + for (i = 0; i < (128 - edid[0x2]) / DETAILED_TIMING_DESCRIPTION_SIZE; It should validate edid[2] first, methinks. If it's some crazy value then this loop will run off into the weeds. > + i++, block += DETAILED_TIMING_DESCRIPTION_SIZE) { > + if (!(block[0] == 0x00 && block[1] == 0x00)) { > + get_detailed_timing(block, &mode[num]); > + if (first) { > + mode[num].flag |= FB_MODE_IS_FIRST; > + first = 0; > + } > + num++; > + } > + } > + > + /* Yikes, EDID data is totally useless */ > + if (!num) { > + kfree(mode); > + return; > + } > + > + m = kzalloc((specs->modedb_len + num) * > + sizeof(struct fb_videomode), GFP_KERNEL); > + > + if (!m) { > + kfree(mode); > + return; > + } > + > + memmove(m, specs->modedb, specs->modedb_len * sizeof(struct fb_videomode)); > + memmove(m + specs->modedb_len, mode, num * sizeof(struct fb_videomode)); It's unusual to use memmove() for a copy which cannot overlap - memcpy() would be more typical. memmove() works though. > + kfree(mode); > + kfree(specs->modedb); > + specs->modedb = m; > + specs->modedb_len = specs->modedb_len + num; > +} Without an EXPORT_SYMBOL(), other modules cannot use this function? > /* > * VESA Generalized Timing Formula (GTF) > */ > diff --git a/include/linux/fb.h b/include/linux/fb.h > index f0268de..3fc99cb 100644 > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -1086,6 +1086,8 @@ extern int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var); > extern const unsigned char *fb_firmware_edid(struct device *device); > extern void fb_edid_to_monspecs(unsigned char *edid, > struct fb_monspecs *specs); > +extern void fb_edid_add_monspecs(unsigned char *edid, > + struct fb_monspecs *specs); > 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); > -- > 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html