Re: [Dri-devel] MESA_ycbcr_texture for MGA, Radeon, i810, and Rage 128

[Ian R. <id...@us...>]

Keith Whitwell wrote:
> Ian Romanick wrote:
> 
>> This patch adds support for MESA_ycbcr_texture for MGA, Radeon, i810, 
>> and Rage 128.  Since I do not have access to either i810 or Rage 128 
>> hardware, I have not been able to test the patches for those drivers. 
>> Could someone with access to that hardware please test them?  I 
>> suspect that they should be fine modulo byte-order issues in the 
>> texture format.
>>
>> I have also modified Keith's yuvsquare test to render both the YUV and 
>> RGB version of the texture.  This helped me discover a problem with 
>> the byte-ordering in the original version of the MGA patch. :)
>>
>> If I don't hear any feedback on the i810 and/or Rage 128 drivers in 
>> the next day or so, I will commit the patches without adding 
>> "GL_MESA_ycbcr_texture" to the extension string (i.e., I'll add the 
>> code to support it, but I won't enable the extension).  If it's okay 
>> with Keith, I'll go ahead and commit the changes to yuvsquare.c ASAP.
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: lib/GL/mesa/src/drv//i810/i810context.c
>> ===================================================================
>> RCS file: /cvsroot/dri/xc/xc/lib/GL/mesa/src/drv/i810/i810context.c,v
>> retrieving revision 1.15
>> diff -u -d -r1.15 i810context.c
>> --- lib/GL/mesa/src/drv//i810/i810context.c    19 May 2003 17:24:04 
>> -0000    1.15
>> +++ lib/GL/mesa/src/drv//i810/i810context.c    21 May 2003 15:54:07 -0000
>> @@ -98,6 +98,7 @@
>>     "GL_EXT_texture_env_add",
>>     "GL_EXT_texture_lod_bias",
>>     "GL_IBM_texture_mirrored_repeat",
>> +   "GL_MESA_ycbcr_texture",
>>     "GL_SGIS_generate_mipmap",
>>     "GL_SGIS_texture_border_clamp",
>>     "GL_SGIS_texture_edge_clamp",
>> Index: lib/GL/mesa/src/drv//i810/i810texstate.c
>> ===================================================================
>> RCS file: /cvsroot/dri/xc/xc/lib/GL/mesa/src/drv/i810/i810texstate.c,v
>> retrieving revision 1.6
>> diff -u -d -r1.6 i810texstate.c
>> --- lib/GL/mesa/src/drv//i810/i810texstate.c    25 Nov 2002 19:57:58 
>> -0000    1.6
>> +++ lib/GL/mesa/src/drv//i810/i810texstate.c    21 May 2003 15:54:07 
>> -0000
>> @@ -69,6 +69,15 @@
>>        textureFormat = MI1_FMT_8CI | MI1_PF_8CI_ARGB4444;
>>        t->texelBytes = 1;
>>        break;
>> +   case GL_YCBCR_MESA:
>> +      t->texelBytes = 2;
>> +      textureFormat = MI1_FMT_422 | MI1_PF_422_YCRCB;
>> +      break;
>> +   case GL_YCBCR_REV_MESA:
>> +      t->texelBytes = 2;
>> +      textureFormat = MI1_FMT_422 | MI1_PF_422_YCRCB_SWAP_Y;
>> +      break;
>> +       
> 
> 
> On the i830, I ended up with this:
> 
>    case MESA_FORMAT_YCBCR_REV:
>       t->texelBytes = 2;
>       textureFormat = (MAPSURF_422 | MT_422_YCRCB_NORMAL |
>                TM0S1_COLORSPACE_CONVERSION);
>       break;
> 
>    case MESA_FORMAT_YCBCR:
>       t->texelBytes = 2;
>       textureFormat = (MAPSURF_422 | MT_422_YCRCB_SWAPY
>                 | TM0S1_COLORSPACE_CONVERSION
>      );
>       break;
> 
> ie.  the 'REV' case is unswapped, and the 'MESA_FORMAT_YCBCR' case is 
> SWAPY.
> 
> Don't know if that's really right, but it is what worked.

That sounds like what I ended up doing on MGA.  It turns out that the 
i810 part of the patch is totally hosed.  For some reason, i810 doesn't 
use the ChooseTexFormat function like every other driver.  I just lets 
Mesa pick an internal format and does the conversion on upload.  This is 
the way the MGA worked before the texmem conversion. :(

I don't have any i810 or i815 hardware or I'd fix it & convert it to use 
the texmem interface...