* [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended
@ 2010-10-28 12:34 Guennadi Liakhovetski
2010-10-28 23:17 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing Andrew Morton
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Guennadi Liakhovetski @ 2010-10-28 12:34 UTC (permalink / raw)
To: linux-fbdev
From: Erik Gilling <konkers@android.com>
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@android.com>
---
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)
+{
+ 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;
+ 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));
+ kfree(mode);
+ kfree(specs->modedb);
+ specs->modedb = m;
+ specs->modedb_len = specs->modedb_len + num;
+}
+
/*
* 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] video: add fb_edid_add_monspecs for parsing
2010-10-28 12:34 [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
@ 2010-10-28 23:17 ` Andrew Morton
2010-10-28 23:23 ` Andrew Morton
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2010-10-28 23:17 UTC (permalink / raw)
To: linux-fbdev
On Thu, 28 Oct 2010 14:34:44 +0200 (CEST)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> From: Erik Gilling <konkers@android.com>
>
> 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@android.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] video: add fb_edid_add_monspecs for parsing
2010-10-28 12:34 [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
2010-10-28 23:17 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing Andrew Morton
@ 2010-10-28 23:23 ` Andrew Morton
2010-10-28 23:45 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended edid information Paul Mundt
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2010-10-28 23:23 UTC (permalink / raw)
To: linux-fbdev
On Thu, 28 Oct 2010 14:34:44 +0200 (CEST)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> From: Erik Gilling <konkers@android.com>
>
> 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@android.com>
This patch should also be signed-off-by:you, as you were on the
delivery path.
I hit a few rejects when merging agains current mainline, the
drivers/video/sh_mobile_hdmi.c one was not completely trivial, so I ducked
it all. Redo and resend, please.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended edid information
2010-10-28 12:34 [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
2010-10-28 23:17 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing Andrew Morton
2010-10-28 23:23 ` Andrew Morton
@ 2010-10-28 23:45 ` Paul Mundt
2010-11-01 15:57 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
2010-11-01 17:42 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing Andrew Morton
4 siblings, 0 replies; 6+ messages in thread
From: Paul Mundt @ 2010-10-28 23:45 UTC (permalink / raw)
To: linux-fbdev
On Thu, Oct 28, 2010 at 04:23:12PM -0700, Andrew Morton wrote:
> On Thu, 28 Oct 2010 14:34:44 +0200 (CEST)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
>
> > From: Erik Gilling <konkers@android.com>
> >
> > 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@android.com>
>
> This patch should also be signed-off-by:you, as you were on the
> delivery path.
>
> I hit a few rejects when merging agains current mainline, the
> drivers/video/sh_mobile_hdmi.c one was not completely trivial, so I ducked
> it all. Redo and resend, please.
>
There's also a bunch of stuff in the genesis tree heading to Linus via
rmk that will probably cause more headaches. Once that stuff has merged
then this patch will be much easier to apply.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended
2010-10-28 12:34 [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
` (2 preceding siblings ...)
2010-10-28 23:45 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended edid information Paul Mundt
@ 2010-11-01 15:57 ` Guennadi Liakhovetski
2010-11-01 17:42 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing Andrew Morton
4 siblings, 0 replies; 6+ messages in thread
From: Guennadi Liakhovetski @ 2010-11-01 15:57 UTC (permalink / raw)
To: linux-fbdev
Hi Andrew
On Thu, 28 Oct 2010, Andrew Morton wrote:
> On Thu, 28 Oct 2010 14:34:44 +0200 (CEST)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
>
> > From: Erik Gilling <konkers@android.com>
Thanks for your comments. My plan is to add my S-o-b to this patch from
Erik, but otherwise leave it unchanged, because that's what the 2nd patch
in this series is for - for fixes and improvements. So, I'll address all
your comments in it, is this ok with you?
Thanks
Guennadi
> >
> > 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@android.com>
> > ---
> > 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
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] video: add fb_edid_add_monspecs for parsing
2010-10-28 12:34 [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
` (3 preceding siblings ...)
2010-11-01 15:57 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
@ 2010-11-01 17:42 ` Andrew Morton
4 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2010-11-01 17:42 UTC (permalink / raw)
To: linux-fbdev
On Mon, 1 Nov 2010 16:57:00 +0100 (CET) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> Hi Andrew
>
> On Thu, 28 Oct 2010, Andrew Morton wrote:
>
> > On Thu, 28 Oct 2010 14:34:44 +0200 (CEST)
> > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> >
> > > From: Erik Gilling <konkers@android.com>
>
> Thanks for your comments. My plan is to add my S-o-b to this patch from
> Erik, but otherwise leave it unchanged, because that's what the 2nd patch
> in this series is for - for fixes and improvements. So, I'll address all
> your comments in it, is this ok with you?
>
Yes, that's OK.
It's unusual to do things this way - normally I'd just clump Erik's
patch and your changes to it into a single commit. So we end up with
something which can be more effectively reviewed and a cleaner kernel
history.
But that's not a terribly important thing - if for some particular
reason you want to do it this way then that's OK by me.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-01 17:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-28 12:34 [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
2010-10-28 23:17 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing Andrew Morton
2010-10-28 23:23 ` Andrew Morton
2010-10-28 23:45 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended edid information Paul Mundt
2010-11-01 15:57 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing extended Guennadi Liakhovetski
2010-11-01 17:42 ` [PATCH 1/4] video: add fb_edid_add_monspecs for parsing Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.