From: Ian R. <id...@us...> - 2003-05-21 16:07:27
Attachments:
ycbcr.patch
yuvsquare.patch
|
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. |
From: Denis O. K. <do...@di...> - 2003-05-21 17:37:25
|
Quoting Ian Romanick (id...@us...): > 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. Hi, I've committed your patch to the embedded-2-branch and the test application is running successfully ;) -- Best regards, Denis Oliver Kropp .------------------------------------------. | DirectFB - Hardware accelerated graphics | | http://www.directfb.org/ | "------------------------------------------" Convergence GmbH |
From: Keith W. <ke...@tu...> - 2003-05-21 20:51:01
|
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. Keith |
From: Ian R. <id...@us...> - 2003-05-21 20:57:02
|
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... |
From: Keith W. <ke...@tu...> - 2003-05-21 23:02:32
|
Ian Romanick wrote: > 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. :( It still works in the mesa 3.x way, where mesa's copy of the texture was always stored in rgba8888 format. > I don't have any i810 or i815 hardware or I'd fix it & convert it to use > the texmem interface... I've got it, but it's a way down my list of things to do. Keith |
From: Ian R. <id...@us...> - 2003-05-21 20:58:38
|
Andreas Stenglein wrote: > well... > but I thought those textures are supported > on old Radeon hardware only > with radeon.o kernelmodules 1.6 and above. > (After r200-branch to trunk merge) > > Or is this issue not an issue as it wouldnt > really "hurt"? I don't know of anything in the DRM that should make any difference WRT YUV texture support. Keith or Michel might know if there is any issue, though. Ideas? |
From: Leif D. <lde...@re...> - 2003-05-21 21:26:42
|
On Wed, 21 May 2003, Ian Romanick wrote: > Andreas Stenglein wrote: > > well... > > but I thought those textures are supported > > on old Radeon hardware only > > with radeon.o kernelmodules 1.6 and above. > > (After r200-branch to trunk merge) > > > > Or is this issue not an issue as it wouldnt > > really "hurt"? > > I don't know of anything in the DRM that should make any difference WRT > YUV texture support. Keith or Michel might know if there is any issue, > though. Ideas? Andreas is correct. It's becuase the new formats weren't added to the switch in the texture blit function until 1.6, which means older versions will produce an "invalid texture format" error. This issue is also present with the r128 patch. Rage 128 needs to have the datatype defines added to r128_drv.h and it needs to check for the new formats in the switch in the blit dispatch function (r128_state.c). With that addition, the r128 patch works. Also, the r100 driver (like r200) needs two defines added to radeon_texstate.c to remap the Mesa format names to the real names: #define RADEON_TXFORMAT_YCBCR RADEON_TXFORMAT_YVYU422 #define RADEON_TXFORMAT_YCBCR_REV RADEON_TXFORMAT_VYUY422 I was just working on a patch with some of these changes. Also, mach64 supports this as well, so I can add it in the branch. -- Leif Delgass http://www.retinalburn.net |
From: Leif D. <lde...@re...> - 2003-05-21 22:09:09
|
On Wed, 21 May 2003, Leif Delgass wrote: > Also, the r100 driver (like r200) needs two defines added to > radeon_texstate.c to remap the Mesa format names to the real names: > > #define RADEON_TXFORMAT_YCBCR RADEON_TXFORMAT_YVYU422 > #define RADEON_TXFORMAT_YCBCR_REV RADEON_TXFORMAT_VYUY422 Whoops. Disregard this last comment, I copied the patch out from the email by hand and didn't notice that this was already there. -- Leif Delgass http://www.retinalburn.net |
From: Ian R. <id...@us...> - 2003-05-21 22:41:43
Attachments:
ycbcr.patch
|
Leif Delgass wrote: > On Wed, 21 May 2003, Ian Romanick wrote: > > >>Andreas Stenglein wrote: >> >>>well... >>>but I thought those textures are supported >>>on old Radeon hardware only >>>with radeon.o kernelmodules 1.6 and above. >>>(After r200-branch to trunk merge) >>> >>>Or is this issue not an issue as it wouldnt >>>really "hurt"? >> >>I don't know of anything in the DRM that should make any difference WRT >>YUV texture support. Keith or Michel might know if there is any issue, >>though. Ideas? > > Andreas is correct. It's becuase the new formats weren't added to the > switch in the texture blit function until 1.6, which means older > versions will produce an "invalid texture format" error. This issue is > also present with the r128 patch. Rage 128 needs to have the datatype > defines added to r128_drv.h and it needs to check for the new formats > in the switch in the blit dispatch function (r128_state.c). With that > addition, the r128 patch works. That's annoying. Does this patch fix the Rage 128 issues? I also added the check to the r100 driver for the DRM version and the "fix" to the i810 driver. > Also, the r100 driver (like r200) needs two defines added to > radeon_texstate.c to remap the Mesa format names to the real names: > > #define RADEON_TXFORMAT_YCBCR RADEON_TXFORMAT_YVYU422 > #define RADEON_TXFORMAT_YCBCR_REV RADEON_TXFORMAT_VYUY422 D'oh. I *thought* that was in the patch that I sent out. > I was just working on a patch with some of these changes. Also, mach64 > supports this as well, so I can add it in the branch. > |
From: Michel <mi...@da...> - 2003-05-21 23:00:09
|
On Wed, 2003-05-21 at 23:25, Leif Delgass wrote: > On Wed, 21 May 2003, Ian Romanick wrote: > > > Andreas Stenglein wrote: > > > well... > > > but I thought those textures are supported > > > on old Radeon hardware only > > > with radeon.o kernelmodules 1.6 and above. > > > (After r200-branch to trunk merge) > > > > > > Or is this issue not an issue as it wouldnt > > > really "hurt"? > > > > I don't know of anything in the DRM that should make any difference WRT > > YUV texture support. Keith or Michel might know if there is any issue, > > though. Ideas? > > Andreas is correct. It's becuase the new formats weren't added to the > switch in the texture blit function until 1.6, which means older > versions will produce an "invalid texture format" error. But AFAICT, the r200 and radeon drivers only ever use RADEON_TXFORMAT_I8 anymore in uploadSubImage(). -- Earthling Michel Dänzer \ Debian (powerpc), XFree86 and DRI developer Software libre enthusiast \ http://svcs.affero.net/rm.php?r=daenzer |
From: Leif D. <lde...@re...> - 2003-05-21 22:59:21
|
On 22 May 2003, Michel D=E4nzer wrote: > On Wed, 2003-05-21 at 23:25, Leif Delgass wrote: > > On Wed, 21 May 2003, Ian Romanick wrote: > > = > > > Andreas Stenglein wrote: > > > > well... > > > > but I thought those textures are supported > > > > on old Radeon hardware only > > > > with radeon.o kernelmodules 1.6 and above. > > > > (After r200-branch to trunk merge) > > > > = > > > > Or is this issue not an issue as it wouldnt > > > > really =22hurt=22? > > > = > > > I don't know of anything in the DRM that should make any difference= WRT = > > > YUV texture support. Keith or Michel might know if there is any is= sue, = > > > though. Ideas? > > = > > Andreas is correct. It's becuase the new formats weren't added to th= e = > > switch in the texture blit function until 1.6, which means older = > > versions will produce an =22invalid texture format=22 error. > = > But AFAICT, the r200 and radeon drivers only ever use RADEON_TXFORMAT_I= 8 > anymore in uploadSubImage(). Right, I forgot about that. I was incorrectly extrapolating from r128, which produces an invalid blit format error without the drm patch. -- = Leif Delgass = http://www.retinalburn.net |