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

[Adam Jackson]

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).

> 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.

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.

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg/attachments/20090211/c3ee4e91/attachment.pgp>