[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