[PATCH 3/6] handle detailed timing operation in xf86EdidModes.c

Ma Ling ling.ma at intel.com
Mon Dec 29 01:03:16 PST 2008


On Thu, 2008-12-25 at 10:23 +0800, Wu, Fengguang wrote: 
> On Sat, Dec 20, 2008 at 04:37:51AM +0200, Ma, Ling wrote:
> > handled detailed timing operation .e.g extract detailed timing modes from EDID and cea ext,
> > then insert mode list.
> >
> > ---
> >  hw/xfree86/ddc/xf86DDC.h         |   44 ++++++
> >  hw/xfree86/modes/xf86EdidModes.c |  278 ++++++++++++++++++++------------------
> >  2 files changed, 189 insertions(+), 133 deletions(-)
> >
> > diff --git a/hw/xfree86/ddc/xf86DDC.h b/hw/xfree86/ddc/xf86DDC.h
> > index 07411b8..0f4f65f 100644
> > --- a/hw/xfree86/ddc/xf86DDC.h
> > +++ b/hw/xfree86/ddc/xf86DDC.h
> > @@ -62,4 +62,48 @@ extern _X_EXPORT DisplayModePtr xf86DDCGetModes(int scrnIndex, xf86MonPtr DDC);
> >  extern _X_EXPORT Bool
> >  xf86MonitorIsHDMI(xf86MonPtr mon);
> >
> > +/*
> > + * Quirks to work around broken EDID data from various monitors.
> > + */
> > +typedef enum {
> > +    DDC_QUIRK_NONE = 0,
> > +    /* First detailed mode is bogus, prefer largest mode at 60hz */
> > +    DDC_QUIRK_PREFER_LARGE_60 = 1 << 0,
> > +    /* 135MHz clock is too high, drop a bit */
> > +    DDC_QUIRK_135_CLOCK_TOO_HIGH = 1 << 1,
> > +    /* Prefer the largest mode at 75 Hz */
> > +    DDC_QUIRK_PREFER_LARGE_75 = 1 << 2,
> > +    /* Convert detailed timing's horizontal from units of cm to mm */
> > +    DDC_QUIRK_DETAILED_H_IN_CM = 1 << 3,
> > +    /* Convert detailed timing's vertical from units of cm to mm */
> > +    DDC_QUIRK_DETAILED_V_IN_CM = 1 << 4,
> > +    /* Detailed timing descriptors have bogus size values, so just take the
> > +     * maximum size and use that.
> > +     */
> > +    DDC_QUIRK_DETAILED_USE_MAXIMUM_SIZE = 1 << 5,
> > +    /* Monitor forgot to set the first detailed is preferred bit. */
> > +    DDC_QUIRK_FIRST_DETAILED_PREFERRED = 1 << 6,
> > +    /* use +hsync +vsync for detailed mode */
> > +    DDC_QUIRK_DETAILED_SYNC_PP = 1 << 7,
> > +    /* Force single-link DVI bandwidth limit */
> > +    DDC_QUIRK_DVI_SINGLE_LINK = 1 << 8,
> > +} ddc_quirk_t;
> > +
> > +DisplayModePtr xf86DDCGetModes(int scrnIndex, xf86MonPtr DDC);
> > +
> > +extern Bool
> > +xf86MonitorIsHDMI(xf86MonPtr mon);
> > +
> > +typedef void (* handle_detailed_fn)(struct detailed_monitor_section *,void *);
> > +
> > +void xf86ForEachDetailedBlock(xf86MonPtr mon,
> > +                              handle_detailed_fn,
> > +                              void *data);
> > +
> > +ddc_quirk_t
> > +xf86DDCDetectQuirks(int scrnIndex, xf86MonPtr DDC, Bool verbose);
> > +
> > +void xf86DetTimingApplyQuirks(struct detailed_monitor_section *det_mon,
> > +                              ddc_quirk_t quirks, int hsize, int vsize);
> > +
> >  #endif
> > diff --git a/hw/xfree86/modes/xf86EdidModes.c b/hw/xfree86/modes/xf86EdidModes.c
> > index 1413e87..e016e43 100644
> > --- a/hw/xfree86/modes/xf86EdidModes.c
> > +++ b/hw/xfree86/modes/xf86EdidModes.c
> > @@ -45,20 +45,24 @@
> >  #include <string.h>
> >  #include <math.h>
> >
> > +void static handle_detailed_rblank(struct detailed_monitor_section *det_mon,
> 
> static void
fixed 
> 
> > +                                   void *data)
> > +{
> > +
> 
> extra blank line
fixed 
> 
> > +    if (det_mon->type == DS_RANGES)
> > +        if (det_mon->section.ranges.supported_blanking & CVT_REDUCED)
> > +            *(Bool*)data = TRUE;
> > +}
> > +
> >  static Bool
> >  xf86MonitorSupportsReducedBlanking(xf86MonPtr DDC)
> >  {
> >      /* EDID 1.4 explicitly defines RB support */
> >      if (DDC->ver.revision >= 4) {
> > -       int i;
> > -       for (i = 0; i < DET_TIMINGS; i++) {
> > -           struct detailed_monitor_section *det_mon = &DDC->det_mon[i];
> > -           if (det_mon->type == DS_RANGES)
> > -               if (det_mon->section.ranges.supported_blanking & CVT_REDUCED)
> > -                   return TRUE;
> > -       }
> > -
> > -       return FALSE;
> > +        Bool ret = FALSE;
> > +
> > +        xf86ForEachDetailedBlock(DDC, handle_detailed_rblank, &ret);
> > +        return ret;
> >      }
> >
> >      /* For anything older, assume digital means RB support. Boo. */
> > @@ -68,34 +72,6 @@ xf86MonitorSupportsReducedBlanking(xf86MonPtr DDC)
> >      return FALSE;
> >  }
> >
> > -/*
> > - * Quirks to work around broken EDID data from various monitors.
> > - */
> > -
> > -typedef enum {
> > -    DDC_QUIRK_NONE = 0,
> > -    /* First detailed mode is bogus, prefer largest mode at 60hz */
> > -    DDC_QUIRK_PREFER_LARGE_60 = 1 << 0,
> > -    /* 135MHz clock is too high, drop a bit */
> > -    DDC_QUIRK_135_CLOCK_TOO_HIGH = 1 << 1,
> > -    /* Prefer the largest mode at 75 Hz */
> > -    DDC_QUIRK_PREFER_LARGE_75 = 1 << 2,
> > -    /* Convert detailed timing's horizontal from units of cm to mm */
> > -    DDC_QUIRK_DETAILED_H_IN_CM = 1 << 3,
> > -    /* Convert detailed timing's vertical from units of cm to mm */
> > -    DDC_QUIRK_DETAILED_V_IN_CM = 1 << 4,
> > -    /* Detailed timing descriptors have bogus size values, so just take the
> > -     * maximum size and use that.
> > -     */
> > -    DDC_QUIRK_DETAILED_USE_MAXIMUM_SIZE = 1 << 5,
> > -    /* Monitor forgot to set the first detailed is preferred bit. */
> > -    DDC_QUIRK_FIRST_DETAILED_PREFERRED = 1 << 6,
> > -    /* use +hsync +vsync for detailed mode */
> > -    DDC_QUIRK_DETAILED_SYNC_PP = 1 << 7,
> > -    /* Force single-link DVI bandwidth limit */
> > -    DDC_QUIRK_DVI_SINGLE_LINK = 1 << 8,
> > -} ddc_quirk_t;
> > -
> >  static Bool quirk_prefer_large_60 (int scrnIndex, xf86MonPtr DDC)
> >  {
> >      /* Belinea 10 15 55 */
> > @@ -667,7 +643,7 @@ DDCGuessRangesFromModes(int scrnIndex, MonPtr Monitor, DisplayModePtr Modes)
> >      }
> >  }
> >
> > -static ddc_quirk_t
> > +ddc_quirk_t
> >  xf86DDCDetectQuirks(int scrnIndex, xf86MonPtr DDC, Bool verbose)
> >  {
> >      ddc_quirk_t        quirks;
> > @@ -687,6 +663,26 @@ xf86DDCDetectQuirks(int scrnIndex, xf86MonPtr DDC, Bool verbose)
> >      return quirks;
> >  }
> >
> > +void xf86DetTimingApplyQuirks(struct detailed_monitor_section *det_mon,
> > +                              ddc_quirk_t quirks,
> > +                              int hsize, int vsize)
> > +{
> > +
> 
> ditto
fixed 
> 
> > +    if (det_mon->type != DT)
> > +        return;
> > +
> > +    if (quirks & DDC_QUIRK_DETAILED_H_IN_CM)
> > +        det_mon->section.d_timings.h_size *= 10;
> > +
> > +    if (quirks & DDC_QUIRK_DETAILED_V_IN_CM)
> > +        det_mon->section.d_timings.v_size *= 10;
> > +
> > +    if (quirks & DDC_QUIRK_DETAILED_USE_MAXIMUM_SIZE) {
> > +        det_mon->section.d_timings.h_size = 10 * hsize;
> > +        det_mon->section.d_timings.v_size = 10 * vsize;
> > +    }
> > +}
> > +
> >  /**
> >   * Applies monitor-specific quirks to the decoded EDID information.
> >   *
> > @@ -700,21 +696,9 @@ xf86DDCApplyQuirks(int scrnIndex, xf86MonPtr DDC)
> >      int i;
> >
> >      for (i = 0; i < DET_TIMINGS; i++) {
> > -       struct detailed_monitor_section *det_mon = &DDC->det_mon[i];
> > -
> > -       if (det_mon->type != DT)
> > -           continue;
> > -
> > -       if (quirks & DDC_QUIRK_DETAILED_H_IN_CM)
> > -           det_mon->section.d_timings.h_size *= 10;
> > -
> > -       if (quirks & DDC_QUIRK_DETAILED_V_IN_CM)
> > -           det_mon->section.d_timings.v_size *= 10;
> > -
> > -       if (quirks & DDC_QUIRK_DETAILED_USE_MAXIMUM_SIZE) {
> > -           det_mon->section.d_timings.h_size = 10 * DDC->features.hsize;
> > -           det_mon->section.d_timings.v_size = 10 * DDC->features.vsize;
> > -       }
> > +        xf86DetTimingApplyQuirks(DDC->det_mon + i, quirks,
> > +                                 DDC->features.hsize,
> > +                                 DDC->features.vsize);
> >      }
> >  }
> >
> > @@ -759,14 +743,62 @@ xf86DDCSetPreferredRefresh(int scrnIndex, DisplayModePtr modes,
> >             best->type |= M_T_PREFERRED;
> >  }
> >
> > +struct det_modes_parameter {
> > +    xf86MonPtr DDC;
> > +    ddc_quirk_t quirks;
> > +    DisplayModePtr * Modes;
> > +    Bool rb;
> > +    Bool preferred;
> > +    int timing_level;
> > +};
> > +
> > +static void handle_detailed_modes(struct detailed_monitor_section *det_mon,
> > +                                 void *data)
> > +{
> > +    DisplayModePtr  Mode;
> > +    struct det_modes_parameter *p = (struct det_modes_parameter *)data;
> > +
> > +    xf86DetTimingApplyQuirks(det_mon,p->quirks,
> > +                             p->DDC->features.hsize,
> > +                             p->DDC->features.vsize);
> > +
> > +    switch (det_mon->type) {
> > +    case DT:
> > +
> 
> ditto
fixed 
> 
> > +        Mode = DDCModeFromDetailedTiming(p->DDC->scrnIndex,
> > +                                         &det_mon->section.d_timings,
> > +                                         p->preferred,
> > +                                         p->quirks);
> > +        p->preferred = FALSE;
> > +        *p->Modes = xf86ModesAdd(*p->Modes, Mode);
> > +        break;
> > +    case DS_STD_TIMINGS:
> > +
> 
> ditto
fixed 
> > +        Mode = DDCModesFromStandardTiming(det_mon->section.std_t,
> > +                                          p->quirks, p->timing_level,p->rb);
> > +        *p->Modes = xf86ModesAdd(*p->Modes, Mode);
> > +        break;
> > +#if XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(7,0,0,0,0)
> > +    case DS_CVT:
> > +
> 
> ditto
fixed 
> 
> > +        Mode = DDCModesFromCVT(p->DDC->scrnIndex, det_mon->section.cvt);
> > +        *p->Modes = xf86ModesAdd(*p->Modes, Mode);
> > +        break;
> > +#endif
> > +    default:
> > +        break;
> > +    }
> > +}
> > +
> > +
> >  DisplayModePtr
> >  xf86DDCGetModes(int scrnIndex, xf86MonPtr DDC)
> >  {
> > -    int                    i;
> >      DisplayModePtr  Modes = NULL, Mode;
> >      ddc_quirk_t            quirks;
> >      Bool           preferred, rb;
> >      int                    timing_level;
> > +    struct det_modes_parameter p;
> 
> p implies pointer in convention. params could be better.
> 
> >
> >      xf86DrvMsg (scrnIndex, X_INFO, "EDID vendor \"%s\", prod id %d\n",
> >                 DDC->vendor.name, DDC->vendor.prod_id);
> > @@ -785,33 +817,13 @@ xf86DDCGetModes(int scrnIndex, xf86MonPtr DDC)
> >
> >      timing_level = MonitorStandardTimingLevel(DDC);
> >
> > -    for (i = 0; i < DET_TIMINGS; i++) {
> > -       struct detailed_monitor_section *det_mon = &DDC->det_mon[i];
> > -
> > -        switch (det_mon->type) {
> > -        case DT:
> > -            Mode = DDCModeFromDetailedTiming(scrnIndex,
> > -                                             &det_mon->section.d_timings,
> > -                                            preferred,
> > -                                            quirks);
> > -           preferred = FALSE;
> > -            Modes = xf86ModesAdd(Modes, Mode);
> > -            break;
> > -        case DS_STD_TIMINGS:
> > -            Mode = DDCModesFromStandardTiming(det_mon->section.std_t,
> > -                                             quirks, timing_level, rb);
> > -            Modes = xf86ModesAdd(Modes, Mode);
> > -            break;
> > -#if XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(7,0,0,0,0)
> > -       case DS_CVT:
> > -           Mode = DDCModesFromCVT(scrnIndex, det_mon->section.cvt);
> > -           Modes = xf86ModesAdd(Modes, Mode);
> > -           break;
> > -#endif
> > -        default:
> > -            break;
> > -        }
> > -    }
> > +    p.quirks = quirks;
> > +    p.DDC = DDC;
> > +    p.Modes = &Modes;
> > +    p.rb = rb;
> > +    p.preferred = preferred;
> > +    p.timing_level = timing_level;
> > +    xf86ForEachDetailedBlock(DDC, handle_detailed_modes, &p);
> >
> >      /* Add established timings */
> >      Mode = DDCModesFromEstablished(scrnIndex, &DDC->timings1, quirks);
> > @@ -830,6 +842,57 @@ xf86DDCGetModes(int scrnIndex, xf86MonPtr DDC)
> >      return Modes;
> >  }
> >
> > +static void handle_detailed_monset(struct detailed_monitor_section *det_mon,
> > +                                   void *data)
> > +{
> > +    MonPtr Monitor = (MonPtr) data;
> > +    int clock;
> > +    int scrnIndex = ((xf86MonPtr)(Monitor->DDC))->scrnIndex;
> > +    ddc_quirk_t quirks;
> > +
> > +    quirks = xf86DDCDetectQuirks(scrnIndex, Monitor->DDC, FALSE);
> 
> Now that xf86DDCDetectQuirks() was moved into the loop, what's the
> implications of this change?
fixed, I have move xf86DDCDetectQuirks to out of the handle function 
> 
> > +    switch (det_mon->type) {
> > +    case DS_RANGES:
> > +        if (Monitor->nHsync == 0) {
> > +            if (!Monitor->nHsync)
> > +                xf86DrvMsg(scrnIndex, X_INFO,
> > +                    "Using EDID range info for horizontal sync\n");
> > +                Monitor->hsync[Monitor->nHsync].lo =
> > +                    det_mon->section.ranges.min_h;
> > +                Monitor->hsync[Monitor->nHsync].hi =
> > +                    det_mon->section.ranges.max_h;
> > +                Monitor->nHsync++;
> > +        } else {
> > +            xf86DrvMsg(scrnIndex, X_INFO,
> > +                "Using hsync ranges from config file\n");
> > +        }
> > +
> > +        if (Monitor->nVrefresh == 0) {
> > +            if (!Monitor->nVrefresh)
> > +                xf86DrvMsg(scrnIndex, X_INFO,
> > +                    "Using EDID range info for vertical refresh\n");
> > +            Monitor->vrefresh[Monitor->nVrefresh].lo =
> > +                det_mon->section.ranges.min_v;
> > +            Monitor->vrefresh[Monitor->nVrefresh].hi =
> > +                det_mon->section.ranges.max_v;
> > +            Monitor->nVrefresh++;
> > +        } else {
> > +            xf86DrvMsg(scrnIndex, X_INFO,
> > +                "Using vrefresh ranges from config file\n");
> > +        }
> > +
> > +        clock = det_mon->section.ranges.max_clock * 1000;
> > +        if (quirks & DDC_QUIRK_DVI_SINGLE_LINK)
> > +            clock = min(clock, 165000);
> > +        if (Monitor->maxPixClock == 0 && clock >Monitor->maxPixClock)
> > +            Monitor->maxPixClock = clock;
> > +
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +}
> > +
> >  /*
> >   * Fill out MonPtr with xf86MonPtr information.
> >   */
> > @@ -837,17 +900,12 @@ void
> >  xf86DDCMonitorSet(int scrnIndex, MonPtr Monitor, xf86MonPtr DDC)
> >  {
> >      DisplayModePtr Modes = NULL, Mode;
> > -    int i, clock;
> > -    Bool have_hsync = FALSE, have_vrefresh = FALSE, have_maxpixclock = FALSE;
> > -    ddc_quirk_t quirks;
> >
> >      if (!Monitor || !DDC)
> >          return;
> >
> >      Monitor->DDC = DDC;
> >
> > -    quirks = xf86DDCDetectQuirks(scrnIndex, DDC, FALSE);
> > -
> >      if (Monitor->widthmm <= 0 && Monitor->heightmm <= 0) {
> >         Monitor->widthmm = 10 * DDC->features.hsize;
> >         Monitor->heightmm = 10 * DDC->features.vsize;
> > @@ -857,54 +915,8 @@ xf86DDCMonitorSet(int scrnIndex, MonPtr Monitor, xf86MonPtr DDC)
> >
> >      Modes = xf86DDCGetModes(scrnIndex, DDC);
> >
> > -    /* Skip EDID ranges if they were specified in the config file */
> > -    have_hsync = (Monitor->nHsync != 0);
> > -    have_vrefresh = (Monitor->nVrefresh != 0);
> > -    have_maxpixclock = (Monitor->maxPixClock != 0);
> 
> Ditto. These equality conditions are also moved into the loop, the
> values they depend on may actually change. Is this the expected
> behavior?
Yes, This is a bug!

> 
> >      /* Go through the detailed monitor sections */
> > -    for (i = 0; i < DET_TIMINGS; i++) {
> > -        switch (DDC->det_mon[i].type) {
> > -        case DS_RANGES:
> > -           if (!have_hsync) {
> > -               if (!Monitor->nHsync)
> > -                   xf86DrvMsg(scrnIndex, X_INFO,
> > -                           "Using EDID range info for horizontal sync\n");
> > -               Monitor->hsync[Monitor->nHsync].lo =
> > -                   DDC->det_mon[i].section.ranges.min_h;
> > -               Monitor->hsync[Monitor->nHsync].hi =
> > -                   DDC->det_mon[i].section.ranges.max_h;
> > -               Monitor->nHsync++;
> > -           } else {
> > -               xf86DrvMsg(scrnIndex, X_INFO,
> > -                       "Using hsync ranges from config file\n");
> > -           }
> > -
> > -           if (!have_vrefresh) {
> > -               if (!Monitor->nVrefresh)
> > -                   xf86DrvMsg(scrnIndex, X_INFO,
> > -                           "Using EDID range info for vertical refresh\n");
> > -               Monitor->vrefresh[Monitor->nVrefresh].lo =
> > -                   DDC->det_mon[i].section.ranges.min_v;
> > -               Monitor->vrefresh[Monitor->nVrefresh].hi =
> > -                   DDC->det_mon[i].section.ranges.max_v;
> > -               Monitor->nVrefresh++;
> > -           } else {
> > -               xf86DrvMsg(scrnIndex, X_INFO,
> > -                       "Using vrefresh ranges from config file\n");
> > -           }
> > -
> > -           clock = DDC->det_mon[i].section.ranges.max_clock * 1000;
> > -           if (quirks & DDC_QUIRK_DVI_SINGLE_LINK)
> > -               clock = min(clock, 165000);
> > -           if (!have_maxpixclock && clock > Monitor->maxPixClock)
> > -               Monitor->maxPixClock = clock;
> > -
> > -            break;
> > -        default:
> > -            break;
> > -        }
> > -    }
> > +    xf86ForEachDetailedBlock(DDC, handle_detailed_monset, Monitor);
> >
> >      if (Modes) {
> >          /* Print Modes */
> > --
> > 1.5.4.4
> >
> >
> >




More information about the xorg mailing list