Re: [PATCH] fbdev: add a function to parse further EDID Detailed

[Guennadi Liakhovetski <g.liakhovetski@gmx.de>]

Hi Erik

On Fri, 20 Aug 2010, Erik Gilling wrote:

> Guennadi,
>     Not sure how you're using the edid but you can get all 4 DTDs by
> calling fb_edid_to_monspecs() then calling fb_videomode_to_var() on
> each of the entries in monspecs.modedb.  This is still lacking as
> modern monitors/tvs can have multiple EDID blocks.  My recent patch
> [1] is a proposal for addressing that.

Thanks for your reply, yes, I did switch to using fb_edid_to_monspecs(), 
which does just what I need. However, I would indeed need the extra modes 
from extended EDID blocks, so, your patch would come in handy. What's its 
status? There have been no comments and it's not in next. I would imagine, 
that at least an EXPORT_SYNBOL() and a dummy version for when 
CONFIG_FB_MODE_HELPERS is not set are missing in your patch, but otherwise 
- are there any big objections? I.e., you just need to apply something 
like the diff below. With it applied:

Tested-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Andrew, would you be taking it after the above issues are corrected?

As a TODO: are you planning to add parsing of SVDs to your E-EDID code?

Thanks
Guennadi

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 7a77170..5ba2234 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1345,6 +1345,9 @@ int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 {
 }
+void fb_edid_add_monspecs(unsigned char *edid, struct fb_monspecs *specs)
+{
+}
 void fb_destroy_modedb(struct fb_videomode *modedb)
 {
 }
@@ -1452,6 +1455,7 @@ EXPORT_SYMBOL(fb_firmware_edid);
 
 EXPORT_SYMBOL(fb_parse_edid);
 EXPORT_SYMBOL(fb_edid_to_monspecs);
+EXPORT_SYMBOL(fb_edid_add_monspecs);
 EXPORT_SYMBOL(fb_get_mode);
 EXPORT_SYMBOL(fb_validate_mode);
 EXPORT_SYMBOL(fb_destroy_modedb);

> 
> Cheers,
>     Erik
> 
> [1] http://marc.info/?l=linux-fbdev&m\x128208740412602&w=2
> 
> On Thu, Aug 19, 2010 at 11:48 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > Currently a function is available to parse the first EDID Detailed Timing
> > Descriptor (DTD) block. However, EDID can contain up to four DTDs. Add a
> > function to parse further DTDs.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  drivers/video/fbmon.c |   74 ++++++++++++++++++++++++++++++++++++------------
> >  include/linux/fb.h    |    2 +
> >  2 files changed, 57 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> > index 563a98b..03de586 100644
> > --- a/drivers/video/fbmon.c
> > +++ b/drivers/video/fbmon.c
> > @@ -866,6 +866,29 @@ static void get_monspecs(unsigned char *edid, struct fb_monspecs *specs)
> >        }
> >  }
> >
> > +static void fb_edid_to_var(unsigned char *block, struct fb_var_screeninfo *var)
> > +{
> > +       var->xres = var->xres_virtual = H_ACTIVE;
> > +       var->yres = var->yres_virtual = V_ACTIVE;
> > +       var->height = var->width = 0;
> > +       var->right_margin = H_SYNC_OFFSET;
> > +       var->left_margin = (H_ACTIVE + H_BLANKING) -
> > +               (H_ACTIVE + H_SYNC_OFFSET + H_SYNC_WIDTH);
> > +       var->upper_margin = V_BLANKING - V_SYNC_OFFSET -
> > +               V_SYNC_WIDTH;
> > +       var->lower_margin = V_SYNC_OFFSET;
> > +       var->hsync_len = H_SYNC_WIDTH;
> > +       var->vsync_len = V_SYNC_WIDTH;
> > +       var->pixclock = PIXEL_CLOCK;
> > +       var->pixclock /= 1000;
> > +       var->pixclock = KHZ2PICOS(var->pixclock);
> > +
> > +       if (HSYNC_POSITIVE)
> > +               var->sync |= FB_SYNC_HOR_HIGH_ACT;
> > +       if (VSYNC_POSITIVE)
> > +               var->sync |= FB_SYNC_VERT_HIGH_ACT;
> > +}
> > +
> >  int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
> >  {
> >        int i;
> > @@ -884,31 +907,38 @@ int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
> >
> >        for (i = 0; i < 4; i++, block += DETAILED_TIMING_DESCRIPTION_SIZE) {
> >                if (edid_is_timing_block(block)) {
> > -                       var->xres = var->xres_virtual = H_ACTIVE;
> > -                       var->yres = var->yres_virtual = V_ACTIVE;
> > -                       var->height = var->width = 0;
> > -                       var->right_margin = H_SYNC_OFFSET;
> > -                       var->left_margin = (H_ACTIVE + H_BLANKING) -
> > -                               (H_ACTIVE + H_SYNC_OFFSET + H_SYNC_WIDTH);
> > -                       var->upper_margin = V_BLANKING - V_SYNC_OFFSET -
> > -                               V_SYNC_WIDTH;
> > -                       var->lower_margin = V_SYNC_OFFSET;
> > -                       var->hsync_len = H_SYNC_WIDTH;
> > -                       var->vsync_len = V_SYNC_WIDTH;
> > -                       var->pixclock = PIXEL_CLOCK;
> > -                       var->pixclock /= 1000;
> > -                       var->pixclock = KHZ2PICOS(var->pixclock);
> > -
> > -                       if (HSYNC_POSITIVE)
> > -                               var->sync |= FB_SYNC_HOR_HIGH_ACT;
> > -                       if (VSYNC_POSITIVE)
> > -                               var->sync |= FB_SYNC_VERT_HIGH_ACT;
> > +                       fb_edid_to_var(block, var);
> >                        return 0;
> >                }
> >        }
> >        return 1;
> >  }
> >
> > +int fb_parse_edid_index(unsigned char *edid, struct fb_var_screeninfo *var,
> > +                       unsigned int idx)
> > +{
> > +       unsigned char *block;
> > +
> > +       if (edid = NULL || var = NULL || idx > 3)
> > +               return 1;
> > +
> > +       if (!(edid_checksum(edid)))
> > +               return 1;
> > +
> > +       if (!(edid_check_header(edid)))
> > +               return 1;
> > +
> > +       block = edid + DETAILED_TIMING_DESCRIPTIONS_START +
> > +               idx * DETAILED_TIMING_DESCRIPTION_SIZE;
> > +
> > +       if (edid_is_timing_block(block)) {
> > +               fb_edid_to_var(block, var);
> > +               return 0;
> > +       }
> > +
> > +       return 1;
> > +}
> > +
> >  void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
> >  {
> >        unsigned char *block;
> > @@ -1285,6 +1315,11 @@ int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
> >  {
> >        return 1;
> >  }
> > +int fb_parse_edid_index(unsigned char *edid, struct fb_var_screeninfo *var,
> > +                       unsigned int idx)
> > +{
> > +       return 1;
> > +}
> >  void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
> >  {
> >        specs = NULL;
> > @@ -1395,6 +1430,7 @@ const unsigned char *fb_firmware_edid(struct device *device)
> >  EXPORT_SYMBOL(fb_firmware_edid);
> >
> >  EXPORT_SYMBOL(fb_parse_edid);
> > +EXPORT_SYMBOL(fb_parse_edid_index);
> >  EXPORT_SYMBOL(fb_edid_to_monspecs);
> >  EXPORT_SYMBOL(fb_get_mode);
> >  EXPORT_SYMBOL(fb_validate_mode);
> > diff --git a/include/linux/fb.h b/include/linux/fb.h
> > index f0268de..3649c47 100644
> > --- a/include/linux/fb.h
> > +++ b/include/linux/fb.h
> > @@ -1083,6 +1083,8 @@ extern int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var,
> >  extern int fb_validate_mode(const struct fb_var_screeninfo *var,
> >                            struct fb_info *info);
> >  extern int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var);
> > +extern int fb_parse_edid_index(unsigned char *edid, struct fb_var_screeninfo *var,
> > +                              unsigned int idx);
> >  extern const unsigned char *fb_firmware_edid(struct device *device);
> >  extern void fb_edid_to_monspecs(unsigned char *edid,
> >                                struct fb_monspecs *specs);
> > --
> > 1.7.2
> >
> > --
> > 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
> >
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/