[Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
They cause regressions on little endian. Fixes: 172bfdaa9e ("r300g: add support for PIPE_FORMAT_x8R8G8B8_*") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98869 Signed-off-by: Grazvydas Ignotas --- no commit access src/gallium/drivers/r300/r300_texture.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/gallium/drivers/r300/r300_texture.c b/src/gallium/drivers/r300/r300_texture.c index fbac07a..929c3fe 100644 --- a/src/gallium/drivers/r300/r300_texture.c +++ b/src/gallium/drivers/r300/r300_texture.c @@ -47,6 +47,11 @@ */ static enum pipe_format r300_unbyteswap_array_format(enum pipe_format format) { +/* FIXME: Disabled on little endian because of a reported regression: + * https://bugs.freedesktop.org/show_bug.cgi?id=98869 */ +if (PIPE_ENDIAN_NATIVE != PIPE_ENDIAN_BIG) +return format; + /* Only BGRA array formats are supported for simplicity of * the implementation. */ switch (format) { -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
Hi. I'm the user affected by this bug. I can do more testing if needed. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
On 11/02/17 08:01 AM, Grazvydas Ignotas wrote: > They cause regressions on little endian. > > Fixes: 172bfdaa9e ("r300g: add support for PIPE_FORMAT_x8R8G8B8_*") > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98869 > Signed-off-by: Grazvydas Ignotas > --- > no commit access > > src/gallium/drivers/r300/r300_texture.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/src/gallium/drivers/r300/r300_texture.c > b/src/gallium/drivers/r300/r300_texture.c > index fbac07a..929c3fe 100644 > --- a/src/gallium/drivers/r300/r300_texture.c > +++ b/src/gallium/drivers/r300/r300_texture.c > @@ -47,6 +47,11 @@ > */ > static enum pipe_format r300_unbyteswap_array_format(enum pipe_format format) > { > +/* FIXME: Disabled on little endian because of a reported regression: > + * https://bugs.freedesktop.org/show_bug.cgi?id=98869 */ > +if (PIPE_ENDIAN_NATIVE != PIPE_ENDIAN_BIG) > +return format; Is there any reason to believe that whatever issue this avoids couldn't happen on big endian hosts as well? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
On 13/02/17 05:17 PM, Michel Dänzer wrote: > On 11/02/17 08:01 AM, Grazvydas Ignotas wrote: >> They cause regressions on little endian. >> >> Fixes: 172bfdaa9e ("r300g: add support for PIPE_FORMAT_x8R8G8B8_*") >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98869 >> Signed-off-by: Grazvydas Ignotas >> --- >> no commit access >> >> src/gallium/drivers/r300/r300_texture.c | 5 + >> 1 file changed, 5 insertions(+) >> >> diff --git a/src/gallium/drivers/r300/r300_texture.c >> b/src/gallium/drivers/r300/r300_texture.c >> index fbac07a..929c3fe 100644 >> --- a/src/gallium/drivers/r300/r300_texture.c >> +++ b/src/gallium/drivers/r300/r300_texture.c >> @@ -47,6 +47,11 @@ >> */ >> static enum pipe_format r300_unbyteswap_array_format(enum pipe_format >> format) >> { >> +/* FIXME: Disabled on little endian because of a reported regression: >> + * https://bugs.freedesktop.org/show_bug.cgi?id=98869 */ >> +if (PIPE_ENDIAN_NATIVE != PIPE_ENDIAN_BIG) >> +return format; > > Is there any reason to believe that whatever issue this avoids couldn't > happen on big endian hosts as well? More to the point, this seems to disable part of the logic needed for supporting PIPE_FORMAT_x8R8G8B8_* on little endian, but leaves those formats advertised as supported. Did you confirm that there are no piglit gpu profile regressions with this patch? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
On Mon, Feb 13, 2017 at 10:22 AM, Michel Dänzer wrote: > On 13/02/17 05:17 PM, Michel Dänzer wrote: >> On 11/02/17 08:01 AM, Grazvydas Ignotas wrote: >>> They cause regressions on little endian. >>> >>> Fixes: 172bfdaa9e ("r300g: add support for PIPE_FORMAT_x8R8G8B8_*") >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98869 >>> Signed-off-by: Grazvydas Ignotas >>> --- >>> no commit access >>> >>> src/gallium/drivers/r300/r300_texture.c | 5 + >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/src/gallium/drivers/r300/r300_texture.c >>> b/src/gallium/drivers/r300/r300_texture.c >>> index fbac07a..929c3fe 100644 >>> --- a/src/gallium/drivers/r300/r300_texture.c >>> +++ b/src/gallium/drivers/r300/r300_texture.c >>> @@ -47,6 +47,11 @@ >>> */ >>> static enum pipe_format r300_unbyteswap_array_format(enum pipe_format >>> format) >>> { >>> +/* FIXME: Disabled on little endian because of a reported regression: >>> + * https://bugs.freedesktop.org/show_bug.cgi?id=98869 */ >>> +if (PIPE_ENDIAN_NATIVE != PIPE_ENDIAN_BIG) >>> +return format; >> >> Is there any reason to believe that whatever issue this avoids couldn't >> happen on big endian hosts as well? I don't know... > More to the point, this seems to disable part of the logic needed for > supporting PIPE_FORMAT_x8R8G8B8_* on little endian, but leaves those > formats advertised as supported. Well it just reverts to an earlier working state before Marek's patch (172bfdaa9e) tor LE. The patch in question which added support for those formats hasn't done anything specific related to advertising them, and r300_is_*_supported() end up indirectly calling r300_unbyteswap_array_format(), so it looks like nothing specific is needed for advertisement? > Did you confirm that there are no > piglit gpu profile regressions with this patch? I don't have the hardware and can't test, I just have confirmation from a user that the regression gets fixed. Gražvydas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
I'd like some evidence that the bug is caused by r300g and not some common code. Marek On Feb 13, 2017 12:10 PM, "Grazvydas Ignotas" wrote: > On Mon, Feb 13, 2017 at 10:22 AM, Michel Dänzer > wrote: > > On 13/02/17 05:17 PM, Michel Dänzer wrote: > >> On 11/02/17 08:01 AM, Grazvydas Ignotas wrote: > >>> They cause regressions on little endian. > >>> > >>> Fixes: 172bfdaa9e ("r300g: add support for PIPE_FORMAT_x8R8G8B8_*") > >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98869 > >>> Signed-off-by: Grazvydas Ignotas > >>> --- > >>> no commit access > >>> > >>> src/gallium/drivers/r300/r300_texture.c | 5 + > >>> 1 file changed, 5 insertions(+) > >>> > >>> diff --git a/src/gallium/drivers/r300/r300_texture.c > b/src/gallium/drivers/r300/r300_texture.c > >>> index fbac07a..929c3fe 100644 > >>> --- a/src/gallium/drivers/r300/r300_texture.c > >>> +++ b/src/gallium/drivers/r300/r300_texture.c > >>> @@ -47,6 +47,11 @@ > >>> */ > >>> static enum pipe_format r300_unbyteswap_array_format(enum > pipe_format format) > >>> { > >>> +/* FIXME: Disabled on little endian because of a reported > regression: > >>> + * https://bugs.freedesktop.org/show_bug.cgi?id=98869 */ > >>> +if (PIPE_ENDIAN_NATIVE != PIPE_ENDIAN_BIG) > >>> +return format; > >> > >> Is there any reason to believe that whatever issue this avoids couldn't > >> happen on big endian hosts as well? > > I don't know... > > > More to the point, this seems to disable part of the logic needed for > > supporting PIPE_FORMAT_x8R8G8B8_* on little endian, but leaves those > > formats advertised as supported. > > Well it just reverts to an earlier working state before Marek's patch > (172bfdaa9e) tor LE. > The patch in question which added support for those formats hasn't > done anything specific related to advertising them, and > r300_is_*_supported() end up indirectly calling > r300_unbyteswap_array_format(), so it looks like nothing specific is > needed for advertisement? > > > Did you confirm that there are no > > piglit gpu profile regressions with this patch? > > I don't have the hardware and can't test, I just have confirmation > from a user that the regression gets fixed. > > Gražvydas > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
I've changed my mind. The patch can be merged if nobody disagrees. Marek On Feb 13, 2017 2:10 PM, "Marek Olšák" wrote: > I'd like some evidence that the bug is caused by r300g and not some common > code. > > Marek > > On Feb 13, 2017 12:10 PM, "Grazvydas Ignotas" wrote: > >> On Mon, Feb 13, 2017 at 10:22 AM, Michel Dänzer >> wrote: >> > On 13/02/17 05:17 PM, Michel Dänzer wrote: >> >> On 11/02/17 08:01 AM, Grazvydas Ignotas wrote: >> >>> They cause regressions on little endian. >> >>> >> >>> Fixes: 172bfdaa9e ("r300g: add support for PIPE_FORMAT_x8R8G8B8_*") >> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98869 >> >>> Signed-off-by: Grazvydas Ignotas >> >>> --- >> >>> no commit access >> >>> >> >>> src/gallium/drivers/r300/r300_texture.c | 5 + >> >>> 1 file changed, 5 insertions(+) >> >>> >> >>> diff --git a/src/gallium/drivers/r300/r300_texture.c >> b/src/gallium/drivers/r300/r300_texture.c >> >>> index fbac07a..929c3fe 100644 >> >>> --- a/src/gallium/drivers/r300/r300_texture.c >> >>> +++ b/src/gallium/drivers/r300/r300_texture.c >> >>> @@ -47,6 +47,11 @@ >> >>> */ >> >>> static enum pipe_format r300_unbyteswap_array_format(enum >> pipe_format format) >> >>> { >> >>> +/* FIXME: Disabled on little endian because of a reported >> regression: >> >>> + * https://bugs.freedesktop.org/show_bug.cgi?id=98869 */ >> >>> +if (PIPE_ENDIAN_NATIVE != PIPE_ENDIAN_BIG) >> >>> +return format; >> >> >> >> Is there any reason to believe that whatever issue this avoids couldn't >> >> happen on big endian hosts as well? >> >> I don't know... >> >> > More to the point, this seems to disable part of the logic needed for >> > supporting PIPE_FORMAT_x8R8G8B8_* on little endian, but leaves those >> > formats advertised as supported. >> >> Well it just reverts to an earlier working state before Marek's patch >> (172bfdaa9e) tor LE. >> The patch in question which added support for those formats hasn't >> done anything specific related to advertising them, and >> r300_is_*_supported() end up indirectly calling >> r300_unbyteswap_array_format(), so it looks like nothing specific is >> needed for advertisement? >> >> > Did you confirm that there are no >> > piglit gpu profile regressions with this patch? >> >> I don't have the hardware and can't test, I just have confirmation >> from a user that the regression gets fixed. >> >> Gražvydas >> > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
On 14/02/17 08:25 PM, Marek Olšák wrote: > I've changed my mind. The patch can be merged if nobody disagrees. It would be nice to better understand what exactly the problem is. It seems unlikely that it's actually an endianness specific problem in the driver code, more likely an endianness specific problem in state tracker code, or maybe a non-endianness-specific problem in the driver code (which may or may not be possible to hit on big endian hosts as well). In other words, I suspect this isn't a fix for the actual problem, but just a workaround. That said, if you guys are happy with that, I'm not holding it up. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
On Tue, Feb 14, 2017 at 10:27 PM, Michel Dänzer wrote: > On 14/02/17 08:25 PM, Marek Olšák wrote: >> I've changed my mind. The patch can be merged if nobody disagrees. > > It would be nice to better understand what exactly the problem is. It > seems unlikely that it's actually an endianness specific problem in the > driver code, more likely an endianness specific problem in state tracker > code, or maybe a non-endianness-specific problem in the driver code > (which may or may not be possible to hit on big endian hosts as well). > > In other words, I suspect this isn't a fix for the actual problem, but > just a workaround. That said, if you guys are happy with that, I'm not > holding it up. I tend to agree, but OTOH, r300 is pretty old and BE systems are pretty few and far between these days. Neither of which are conducive to spending much time on them. Someone with an interest in BE systems really needs to step up and fix up endian handling in mesa for real. Alex ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
So, as there are no better solutions in sight, can somebody please push this patch? Gražvydas On Wed, Feb 15, 2017 at 5:33 AM, Alex Deucher wrote: > On Tue, Feb 14, 2017 at 10:27 PM, Michel Dänzer wrote: >> On 14/02/17 08:25 PM, Marek Olšák wrote: >>> I've changed my mind. The patch can be merged if nobody disagrees. >> >> It would be nice to better understand what exactly the problem is. It >> seems unlikely that it's actually an endianness specific problem in the >> driver code, more likely an endianness specific problem in state tracker >> code, or maybe a non-endianness-specific problem in the driver code >> (which may or may not be possible to hit on big endian hosts as well). >> >> In other words, I suspect this isn't a fix for the actual problem, but >> just a workaround. That said, if you guys are happy with that, I'm not >> holding it up. > > I tend to agree, but OTOH, r300 is pretty old and BE systems are > pretty few and far between these days. Neither of which are conducive > to spending much time on them. Someone with an interest in BE systems > really needs to step up and fix up endian handling in mesa for real. > > Alex ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian
Pushed. Marek On Mon, Feb 20, 2017 at 11:40 PM, Grazvydas Ignotas wrote: > So, as there are no better solutions in sight, can somebody please > push this patch? > > Gražvydas > > On Wed, Feb 15, 2017 at 5:33 AM, Alex Deucher wrote: >> On Tue, Feb 14, 2017 at 10:27 PM, Michel Dänzer wrote: >>> On 14/02/17 08:25 PM, Marek Olšák wrote: I've changed my mind. The patch can be merged if nobody disagrees. >>> >>> It would be nice to better understand what exactly the problem is. It >>> seems unlikely that it's actually an endianness specific problem in the >>> driver code, more likely an endianness specific problem in state tracker >>> code, or maybe a non-endianness-specific problem in the driver code >>> (which may or may not be possible to hit on big endian hosts as well). >>> >>> In other words, I suspect this isn't a fix for the actual problem, but >>> just a workaround. That said, if you guys are happy with that, I'm not >>> holding it up. >> >> I tend to agree, but OTOH, r300 is pretty old and BE systems are >> pretty few and far between these days. Neither of which are conducive >> to spending much time on them. Someone with an interest in BE systems >> really needs to step up and fix up endian handling in mesa for real. >> >> Alex ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev