Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian

[Marek Olšák]

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

Re: [Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian

[Grazvydas Ignotas]

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

[Alex Deucher]

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

[Michel Dänzer]

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

[Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian

[cosiekvfj]

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

[Marek Olšák]

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

[Marek Olšák]

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

[Grazvydas Ignotas]

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

[Michel Dänzer]

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

[Michel Dänzer]

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

[Mesa-dev] [PATCH] r300g: only allow byteswapped formats on big endian

[Grazvydas Ignotas]

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