Menu
▾
▴
mesa3d-dev
|
From: Dave A. <ai...@gm...> - 2010-04-04 10:31:16
|
Hey, So I was trying to fix tfp test on r300g, and ran into an issue with dri st I think. So the way TFP works we get dri2_set_tex_buffer, which then validates the attachment, but ignores the format passed in. So r300g picks up the kernel buffer from the handle and sets up the texture + texture state without the format information. Once we've validated, we call ctx->st->teximage and can give it a different format however at no point does r300g get any place to change the texture format and update its internal state. I'm not sure if either r300g should delay setting up its internal state for emission until later or whether we need to enhance the st interface. The main issue with we get a TFP with a B8G8R8X8 but the visual is B8G8R8A8 which triggers this. Dave. |
|
From: Dave A. <ai...@gm...> - 2010-04-04 21:35:21
Attachments:
0001-dri-st-add-hacky-tfp-format-override.patch
|
On Sun, Apr 4, 2010 at 8:31 PM, Dave Airlie <ai...@gm...> wrote: > Hey, > > So I was trying to fix tfp test on r300g, and ran into an issue with > dri st I think. > > So the way TFP works we get dri2_set_tex_buffer, which then validates the > attachment, but ignores the format passed in. So r300g picks up the kernel > buffer from the handle and sets up the texture + texture state without > the format > information. > > Once we've validated, we call ctx->st->teximage and can give it a different > format however at no point does r300g get any place to change the texture format > and update its internal state. > > I'm not sure if either r300g should delay setting up its internal > state for emission > until later or whether we need to enhance the st interface. > > The main issue with we get a TFP with a B8G8R8X8 but the visual is B8G8R8A8 > which triggers this. > Here's a hacky patch to demonstrate the issue, though it doesn't fix the problem I'm seeing with the test, just one less thing wrong. Dave. |
|
From: Dave A. <ai...@gm...> - 2010-04-04 22:42:52
Attachments:
0001-dri-st-add-hacky-tfp-format-override-v2.patch
|
> > Here's a hacky patch to demonstrate the issue, though it doesn't fix the problem > I'm seeing with the test, just one less thing wrong. > Here's a second patch that actually fixes the piglit tfp test for me on r300g. Dave. |
|
From: Chia-I Wu <ol...@gm...> - 2010-04-05 02:49:21
|
On Sun, Apr 4, 2010 at 6:31 PM, Dave Airlie <ai...@gm...> wrote: > Hey, > > So I was trying to fix tfp test on r300g, and ran into an issue with > dri st I think. > > So the way TFP works we get dri2_set_tex_buffer, which then validates the > attachment, but ignores the format passed in. So r300g picks up the kernel > buffer from the handle and sets up the texture + texture state without > the format > information. > > Once we've validated, we call ctx->st->teximage and can give it a different > format however at no point does r300g get any place to change the texture format > and update its internal state. > > I'm not sure if either r300g should delay setting up its internal > state for emission > until later or whether we need to enhance the st interface. > > The main issue with we get a TFP with a B8G8R8X8 but the visual is B8G8R8A8 > which triggers this. The attached patch fixes tfp test for me (with i915g). Could you see if r300g passes the test with the patch? The teximage callback has an internal_format parameter that specifies how the pipe texture should be treated. It can differ from the format of the texture. It seems to suffice for TFP. I was lazy enough not to use it :( -- ol...@Lu... |
|
From: Dave A. <ai...@gm...> - 2010-04-05 04:00:34
|
On Mon, Apr 5, 2010 at 12:49 PM, Chia-I Wu <ol...@gm...> wrote: > On Sun, Apr 4, 2010 at 6:31 PM, Dave Airlie <ai...@gm...> wrote: >> Hey, >> >> So I was trying to fix tfp test on r300g, and ran into an issue with >> dri st I think. >> >> So the way TFP works we get dri2_set_tex_buffer, which then validates the >> attachment, but ignores the format passed in. So r300g picks up the kernel >> buffer from the handle and sets up the texture + texture state without >> the format >> information. >> >> Once we've validated, we call ctx->st->teximage and can give it a different >> format however at no point does r300g get any place to change the texture format >> and update its internal state. >> >> I'm not sure if either r300g should delay setting up its internal >> state for emission >> until later or whether we need to enhance the st interface. >> >> The main issue with we get a TFP with a B8G8R8X8 but the visual is B8G8R8A8 >> which triggers this. > The attached patch fixes tfp test for me (with i915g). Could you see if r300g > passes the test with the patch? > > The teximage callback has an internal_format parameter that specifies how the > pipe texture should be treated. It can differ from the format of the texture. > It seems to suffice for TFP. I was lazy enough not to use it :( > That was my first attempt also, however it fails as the texture is already created, and in r300g we already have worked out the hw state assuming the texture format won't change. This seems to be what gallium expects. So in this case you end up recreating a new texture at finalise because the formats don't match and you lose the pixmap. Dave. |
|
From: Chia-I Wu <ol...@gm...> - 2010-04-05 04:37:06
|
On Mon, Apr 5, 2010 at 12:00 PM, Dave Airlie <ai...@gm...> wrote: >> The attached patch fixes tfp test for me (with i915g). Could you see if r300g >> passes the test with the patch? >> The teximage callback has an internal_format parameter that specifies how the >> pipe texture should be treated. It can differ from the format of the texture. >> It seems to suffice for TFP. I was lazy enough not to use it :( > That was my first attempt also, however it fails as the texture is > already created, and > in r300g we already have worked out the hw state assuming the texture > format won't > change. This seems to be what gallium expects. So in this case you end > up recreating > a new texture at finalise because the formats don't match and you lose > the pixmap. r300g does not see the texture before st_finalize_texture, right? It seems to me that the patch should give the correct behavior but a (really bad) unnecessary texture copy in st_finalize_texture. Could we avoid the copy by solely changing the sampler view? -- ol...@Lu... |
|
From: Dave A. <ai...@gm...> - 2010-04-05 05:41:42
|
On Mon, Apr 5, 2010 at 2:37 PM, Chia-I Wu <ol...@gm...> wrote: > On Mon, Apr 5, 2010 at 12:00 PM, Dave Airlie <ai...@gm...> wrote: >>> The attached patch fixes tfp test for me (with i915g). Could you see if r300g >>> passes the test with the patch? >>> The teximage callback has an internal_format parameter that specifies how the >>> pipe texture should be treated. It can differ from the format of the texture. >>> It seems to suffice for TFP. I was lazy enough not to use it :( >> That was my first attempt also, however it fails as the texture is >> already created, and >> in r300g we already have worked out the hw state assuming the texture >> format won't >> change. This seems to be what gallium expects. So in this case you end >> up recreating >> a new texture at finalise because the formats don't match and you lose >> the pixmap. > r300g does not see the texture before st_finalize_texture, right? It seems to > me that the patch should give the correct behavior but a (really bad) > unnecessary texture copy in st_finalize_texture. Could we avoid the copy by > solely changing the sampler view? It sees the texture via dri_st_framebuffer_validate_att(drawable->stfb, ST_ATTACHMENT_FRONT_LEFT) which causes the texture to be creates from the dri2 buffer handle, The thing is we need the exact buffer object we get from the X server, we can't copy it to another texture later, as it destroys the shared texture and just seems to create another private one. Dave. |
|
From: Chia-I Wu <ol...@gm...> - 2010-04-05 07:25:00
|
On Mon, Apr 5, 2010 at 1:41 PM, Dave Airlie <ai...@gm...> wrote: > On Mon, Apr 5, 2010 at 2:37 PM, Chia-I Wu <ol...@gm...> wrote: >> On Mon, Apr 5, 2010 at 12:00 PM, Dave Airlie <ai...@gm...> wrote: >>>> The attached patch fixes tfp test for me (with i915g). Could you see if r300g >>>> passes the test with the patch? >>>> The teximage callback has an internal_format parameter that specifies how the >>>> pipe texture should be treated. It can differ from the format of the texture. >>>> It seems to suffice for TFP. I was lazy enough not to use it :( >>> That was my first attempt also, however it fails as the texture is >>> already created, and >>> in r300g we already have worked out the hw state assuming the texture >>> format won't >>> change. This seems to be what gallium expects. So in this case you end >>> up recreating >>> a new texture at finalise because the formats don't match and you lose >>> the pixmap. >> r300g does not see the texture before st_finalize_texture, right? It seems to >> me that the patch should give the correct behavior but a (really bad) >> unnecessary texture copy in st_finalize_texture. Could we avoid the copy by >> solely changing the sampler view? > It sees the texture via > dri_st_framebuffer_validate_att(drawable->stfb, > ST_ATTACHMENT_FRONT_LEFT) > which causes the texture to be creates from the dri2 buffer handle, > The thing is we need the exact buffer object we get from the X server, > we can't copy it to another texture later, as it destroys the shared texture > and just seems to create another private one. Suppose a PIPE_FORMAT_B8G8R8A8_UNORM GLXPixmap is created both for rendering, and t-f-p texturing with GLX_TEXTURE_FORMAT_RGB_EXT. The front left buffer of the pixmap should be a single PIPE_FORMAT_B8G8R8A8_UNORM pipe_texture. When glXBindTexImageEXT is called, st/mesa should "view" the pipe_texture as if there is no alpha channel. It is hacky for st/dri to create another pipe_texture from the same dri2 buffer handle with a different format and pass the new pipe_texture to st/mesa. To achieve the "viewing" thing efficiently, st/mesa should create a PIPE_FORMAT_B8G8R8X8_UNORM sampler view for the pipe_texture (I suppose this is what a sampler view for? No?). This is contrary to performing an implicit copy on the pipe_texture only to ignore the alpha channel, which is what st_finalize_texture does. IIRC, tfp allows an implementation to perform an implicit copy upon binding. Both behaviors are correct in this sense, but the latter is obviously undesirable. -- ol...@Lu... |
|
From: Dave A. <ai...@gm...> - 2010-04-05 07:38:21
|
On Mon, Apr 5, 2010 at 5:24 PM, Chia-I Wu <ol...@gm...> wrote: > On Mon, Apr 5, 2010 at 1:41 PM, Dave Airlie <ai...@gm...> wrote: >> On Mon, Apr 5, 2010 at 2:37 PM, Chia-I Wu <ol...@gm...> wrote: >>> On Mon, Apr 5, 2010 at 12:00 PM, Dave Airlie <ai...@gm...> wrote: >>>>> The attached patch fixes tfp test for me (with i915g). Could you see if r300g >>>>> passes the test with the patch? >>>>> The teximage callback has an internal_format parameter that specifies how the >>>>> pipe texture should be treated. It can differ from the format of the texture. >>>>> It seems to suffice for TFP. I was lazy enough not to use it :( >>>> That was my first attempt also, however it fails as the texture is >>>> already created, and >>>> in r300g we already have worked out the hw state assuming the texture >>>> format won't >>>> change. This seems to be what gallium expects. So in this case you end >>>> up recreating >>>> a new texture at finalise because the formats don't match and you lose >>>> the pixmap. >>> r300g does not see the texture before st_finalize_texture, right? It seems to >>> me that the patch should give the correct behavior but a (really bad) >>> unnecessary texture copy in st_finalize_texture. Could we avoid the copy by >>> solely changing the sampler view? >> It sees the texture via >> dri_st_framebuffer_validate_att(drawable->stfb, >> ST_ATTACHMENT_FRONT_LEFT) >> which causes the texture to be creates from the dri2 buffer handle, >> The thing is we need the exact buffer object we get from the X server, >> we can't copy it to another texture later, as it destroys the shared texture >> and just seems to create another private one. > Suppose a PIPE_FORMAT_B8G8R8A8_UNORM GLXPixmap is created both for rendering, > and t-f-p texturing with GLX_TEXTURE_FORMAT_RGB_EXT. The front left buffer of > the pixmap should be a single PIPE_FORMAT_B8G8R8A8_UNORM pipe_texture. When > glXBindTexImageEXT is called, st/mesa should "view" the pipe_texture as if > there is no alpha channel. It is hacky for st/dri to create another > pipe_texture from the same dri2 buffer handle with a different format and pass > the new pipe_texture to st/mesa. > > To achieve the "viewing" thing efficiently, st/mesa should create a > PIPE_FORMAT_B8G8R8X8_UNORM sampler view for the pipe_texture (I suppose this is > what a sampler view for? No?). This is contrary to performing an implicit > copy on the pipe_texture only to ignore the alpha channel, which is what > st_finalize_texture does. IIRC, tfp allows an implementation to perform an > implicit copy upon binding. Both behaviors are correct in this sense, but the > latter is obviously undesirable. > Yes a sampler view seems the sanest. The latter behaviour might be correct but doesn't work here, at least with the equiv patch I tried originally the test still failed, so it would be correct if it worked though slower. Dave. |
|
From: Chia-I Wu <ol...@gm...> - 2010-04-05 08:06:58
|
On Mon, Apr 5, 2010 at 3:38 PM, Dave Airlie <ai...@gm...> wrote: > On Mon, Apr 5, 2010 at 5:24 PM, Chia-I Wu <ol...@gm...> wrote: >> On Mon, Apr 5, 2010 at 1:41 PM, Dave Airlie <ai...@gm...> wrote: >>> On Mon, Apr 5, 2010 at 2:37 PM, Chia-I Wu <ol...@gm...> wrote: >>>> On Mon, Apr 5, 2010 at 12:00 PM, Dave Airlie <ai...@gm...> wrote: >>>>>> The attached patch fixes tfp test for me (with i915g). Could you see if r300g >>>>>> passes the test with the patch? >>>>>> The teximage callback has an internal_format parameter that specifies how the >>>>>> pipe texture should be treated. It can differ from the format of the texture. >>>>>> It seems to suffice for TFP. I was lazy enough not to use it :( >>>>> That was my first attempt also, however it fails as the texture is >>>>> already created, and >>>>> in r300g we already have worked out the hw state assuming the texture >>>>> format won't >>>>> change. This seems to be what gallium expects. So in this case you end >>>>> up recreating >>>>> a new texture at finalise because the formats don't match and you lose >>>>> the pixmap. >>>> r300g does not see the texture before st_finalize_texture, right? It seems to >>>> me that the patch should give the correct behavior but a (really bad) >>>> unnecessary texture copy in st_finalize_texture. Could we avoid the copy by >>>> solely changing the sampler view? >>> It sees the texture via >>> dri_st_framebuffer_validate_att(drawable->stfb, >>> ST_ATTACHMENT_FRONT_LEFT) >>> which causes the texture to be creates from the dri2 buffer handle, >>> The thing is we need the exact buffer object we get from the X server, >>> we can't copy it to another texture later, as it destroys the shared texture >>> and just seems to create another private one. >> Suppose a PIPE_FORMAT_B8G8R8A8_UNORM GLXPixmap is created both for rendering, >> and t-f-p texturing with GLX_TEXTURE_FORMAT_RGB_EXT. The front left buffer of >> the pixmap should be a single PIPE_FORMAT_B8G8R8A8_UNORM pipe_texture. When >> glXBindTexImageEXT is called, st/mesa should "view" the pipe_texture as if >> there is no alpha channel. It is hacky for st/dri to create another >> pipe_texture from the same dri2 buffer handle with a different format and pass >> the new pipe_texture to st/mesa. >> >> To achieve the "viewing" thing efficiently, st/mesa should create a >> PIPE_FORMAT_B8G8R8X8_UNORM sampler view for the pipe_texture (I suppose this is >> what a sampler view for? No?). This is contrary to performing an implicit >> copy on the pipe_texture only to ignore the alpha channel, which is what >> st_finalize_texture does. IIRC, tfp allows an implementation to perform an >> implicit copy upon binding. Both behaviors are correct in this sense, but the >> latter is obviously undesirable. >> > > Yes a sampler view seems the sanest. The latter behaviour might be > correct but doesn't > work here, at least with the equiv patch I tried originally the test > still failed, so it would be > correct if it worked though slower. The patch I sent earlier works here with i915g. I am not sure where it goes wrong with r300g. Do you mind have a look into it? -- ol...@Lu... |
|
From: Dave A. <ai...@gm...> - 2010-04-05 08:21:02
|
On Mon, Apr 5, 2010 at 6:06 PM, Chia-I Wu <ol...@gm...> wrote: > On Mon, Apr 5, 2010 at 3:38 PM, Dave Airlie <ai...@gm...> wrote: >> On Mon, Apr 5, 2010 at 5:24 PM, Chia-I Wu <ol...@gm...> wrote: >>> On Mon, Apr 5, 2010 at 1:41 PM, Dave Airlie <ai...@gm...> wrote: >>>> On Mon, Apr 5, 2010 at 2:37 PM, Chia-I Wu <ol...@gm...> wrote: >>>>> On Mon, Apr 5, 2010 at 12:00 PM, Dave Airlie <ai...@gm...> wrote: >>>>>>> The attached patch fixes tfp test for me (with i915g). Could you see if r300g >>>>>>> passes the test with the patch? >>>>>>> The teximage callback has an internal_format parameter that specifies how the >>>>>>> pipe texture should be treated. It can differ from the format of the texture. >>>>>>> It seems to suffice for TFP. I was lazy enough not to use it :( >>>>>> That was my first attempt also, however it fails as the texture is >>>>>> already created, and >>>>>> in r300g we already have worked out the hw state assuming the texture >>>>>> format won't >>>>>> change. This seems to be what gallium expects. So in this case you end >>>>>> up recreating >>>>>> a new texture at finalise because the formats don't match and you lose >>>>>> the pixmap. >>>>> r300g does not see the texture before st_finalize_texture, right? It seems to >>>>> me that the patch should give the correct behavior but a (really bad) >>>>> unnecessary texture copy in st_finalize_texture. Could we avoid the copy by >>>>> solely changing the sampler view? >>>> It sees the texture via >>>> dri_st_framebuffer_validate_att(drawable->stfb, >>>> ST_ATTACHMENT_FRONT_LEFT) >>>> which causes the texture to be creates from the dri2 buffer handle, >>>> The thing is we need the exact buffer object we get from the X server, >>>> we can't copy it to another texture later, as it destroys the shared texture >>>> and just seems to create another private one. >>> Suppose a PIPE_FORMAT_B8G8R8A8_UNORM GLXPixmap is created both for rendering, >>> and t-f-p texturing with GLX_TEXTURE_FORMAT_RGB_EXT. The front left buffer of >>> the pixmap should be a single PIPE_FORMAT_B8G8R8A8_UNORM pipe_texture. When >>> glXBindTexImageEXT is called, st/mesa should "view" the pipe_texture as if >>> there is no alpha channel. It is hacky for st/dri to create another >>> pipe_texture from the same dri2 buffer handle with a different format and pass >>> the new pipe_texture to st/mesa. >>> >>> To achieve the "viewing" thing efficiently, st/mesa should create a >>> PIPE_FORMAT_B8G8R8X8_UNORM sampler view for the pipe_texture (I suppose this is >>> what a sampler view for? No?). This is contrary to performing an implicit >>> copy on the pipe_texture only to ignore the alpha channel, which is what >>> st_finalize_texture does. IIRC, tfp allows an implementation to perform an >>> implicit copy upon binding. Both behaviors are correct in this sense, but the >>> latter is obviously undesirable. >>> >> >> Yes a sampler view seems the sanest. The latter behaviour might be >> correct but doesn't >> work here, at least with the equiv patch I tried originally the test >> still failed, so it would be >> correct if it worked though slower. > The patch I sent earlier works here with i915g. I am not sure where it goes > wrong with r300g. Do you mind have a look into it? Yeah I suspect we have an issue in r300g alright, we use a stride override when we get the texture from TFP, and this might not be propogated properly when we copy it later. Dave. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X