[PATCH 8/9] handle cea-ext data block and insert mode into mode list.

[Ma Ling]

Sorry, please use this one to replace last email.

Thanks
Ma Ling
On Thu, 2009-02-12 at 03:11 +0800, Adam Jackson wrote:
> On Wed, 2009-02-11 at 17:17 +0800, Ma Ling wrote:
> 
> > +void xf86FetchDataBlock(xf86MonPtr mon,
> > +                        int ext_type, int data_type,
> > +                        void *data)
> > +{
> > +    int i;
> > +    Uchar *ext;
> > +
> > +    if (mon == NULL)
> > +	return;
> > +
> > +    for (i = 0; i < mon->no_sections; i++) {
> > +	ext = mon->rawData + EDID1_LEN * (i + 1);
> > +	if (ext[EXT_TAG]  == ext_type)
> > +	    break;
> > +    }
> > +    /* return if don't find corresponding extension type */
> > +    if (i == mon->no_sections)
> > +	return;
> > +
> > +    switch (ext[EXT_TAG]){
> > +    case CEA_EXT:
> > +	extract_cea_data_block(ext, data_type, data);
> > +	break;
> > +    case VTB_EXT:
> > +    case DI_EXT:
> > +    case LS_EXT:
> > +    case MI_EXT:
> > +	break;
> > +    }
> > +}
> 
> This doesn't seem like the right interface.  VTB, for example, lets you
> have multiple detailed timing blocks, but this interface would only let
> you ask for one.  Strictly speaking it's legal in CEA too, but I don't
> think anyone's been stupid enough to encode their CEA block that way.
> 
> Also, returns void, but has a single void * outparameter?  Ew.
> 
> Either make this an iterator like ForEachDetailedBlock, or lose the
> ext_type and data_type and just grab the CEA video descriptor directly
> (and return it, rather than outparam'ing it).
> 
ok, I will chose the similar approach like xf86ForEachDetailedBlock.
> > diff --git a/hw/xfree86/modes/xf86EdidModes.c b/hw/xfree86/modes/xf86EdidModes.c
> > index 97c40bf..4760316 100644
> > --- a/hw/xfree86/modes/xf86EdidModes.c
> > +++ b/hw/xfree86/modes/xf86EdidModes.c
> > @@ -741,6 +741,104 @@ xf86DDCSetPreferredRefresh(int scrnIndex, DisplayModePtr modes,
> >  	    best->type |= M_T_PREFERRED;
> >  }
> >  
> > +#define CEA_VIDEO_MODES_NUM  64
> > +static const DisplayModeRec CEAVideoModes[CEA_VIDEO_MODES_NUM] = {
> 
> This timing list actually brings up an unpleasant topic.  How are
> interlaced modes supposed to be represented inside the server?  Detailed
> blocks seem to mostly be written such that 1080i is described as 1920
> wide, 540 high, with the interlace bit set; ie, that height is field
> height.  The mode list you've got here, and the standard mode list
> already in the server, give height as frame height.
> 
I notice xf86GetDefaultModes function will search
xf86DefaultModes table which also contain interlaced mode, but whether
duplicate interlaced mode line or not depends on interalceAllowed flag
set by driver, So can we implement the similar function like that to
handle CEA short video descriptor, and  Mode structure always use 1080
as VDisplay?
> The language in the EDID spec describes detailed timing numbers as being
> about the addressable image size of the mode, ie, frame height.  So
> probably we just need to correct detailed timings on the way in.
> 
> > +static DisplayModePtr
> > +DDCModesFromCeaExtension(int scrnIndex, xf86MonPtr MonPtr)
> 
> CEA should be all-capitals for consistency.
OK I will fix it!
> 

> - ajax