Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-06-16 Thread Marc Di Luzio
Hi,

>From what I'm aware this is the progress on the fixes:

TotalWarhammer and HitmanPro should now have a fix in shipping.
MadMax, DeusExMD and DirtRally have the fix in a patch making it's way
through our internal testing.

We would still very much prefer to not have a special case for our games
within compiled code in gallium trunk, the case in r300_chipset.c doesn't
appear analogous.

We'll be pushing up the priority for those remaining patches internally. If
however the consensus is to get this fixed ASAP then we'd rather the code
path be data controlled by drirc or at least an environment variable we (or
users) can set in our launch scripts in the interim weeks.

Cheers,

-
Marc Di Luzio
Linux Group Lead @ Feral Interactive Ltd.

On 16 June 2017 at 12:45, Marek Olšák  wrote:

> Hi,
>
> Feral's games still enable primitive restart for all draw calls.
>
> FYI, I will push this patch on Monday if there is no other feedback.
> Some other points:
> - This is not the first occurrence of private app lists in drivers.
> r300 also has an app list in r300_chipset.c.
> - The list of Feral's games needing this workaround was indeed
> complete at the time of writing the patch.
>
> Marek
>
>
> On Tue, Apr 25, 2017 at 11:26 AM, Marc Di Luzio
>  wrote:
> >> Thanks. Do you plan to update the games not to enable primitive
> >> restart for non-strip primitives?
> >
> > I won't be able to give a decent time frame yet, but yes I'll make sure
> it's
> > on our schedule.
> >
> > -
> > Marc Di Luzio
> > Linux Group Lead @ Feral Interactive Ltd.
> >
> > On 25 April 2017 at 10:15, Marek Olšák  wrote:
> >>
> >> On Tue, Apr 25, 2017 at 11:09 AM, Marc Di Luzio
> >>  wrote:
> >> > Hi Marek,
> >> >
> >> > I agree with Ken here.
> >> >
> >> > For what it's worth, the list of our titles that use primitive restart
> >> > here
> >> > is likely the full list. DXMD was the first as far as I know - see
> >> >
> >> > https://cgit.freedesktop.org/mesa/mesa/commit/?id=
> e33f31d61f5e9019f8b0bac0378dfb8fd1147421.
> >> > It also appears to be an app side issue so we will patch as needed.
> >> >
> >> > In the future let us know first, in pretty much all cases we'd prefer
> to
> >> > make the change on our side instead of adding game specific hacks in
> >> > Mesa.
> >>
> >> Thanks. Do you plan to update the games not to enable primitive
> >> restart for non-strip primitives?
> >>
> >> Marek
> >>
> >> >
> >> > Cheers,
> >> >
> >> >
> >> > -
> >> > Marc Di Luzio
> >> > Linux Group Lead @ Feral Interactive Ltd.
> >> >
> >> > On 24 April 2017 at 23:26, Kenneth Graunke 
> >> > wrote:
> >> >>
> >> >> On Monday, April 24, 2017 6:22:41 AM PDT Marek Olšák wrote:
> >> >> > From: Marek Olšák 
> >> >> >
> >> >> > ---
> >> >> >  src/gallium/drivers/radeonsi/si_pipe.c   | 20 +
> >> >> >  src/gallium/drivers/radeonsi/si_pipe.h   |  1 +
> >> >> >  src/gallium/drivers/radeonsi/si_state_draw.c | 45
> >> >> > 
> >> >> >  3 files changed, 54 insertions(+), 12 deletions(-)
> >> >> >
> >> >> > diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
> >> >> > b/src/gallium/drivers/radeonsi/si_pipe.c
> >> >> > index 1a83564..53a8201 100644
> >> >> > --- a/src/gallium/drivers/radeonsi/si_pipe.c
> >> >> > +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> >> >> > @@ -29,20 +29,29 @@
> >> >> >  #include "radeon/radeon_uvd.h"
> >> >> >  #include "util/u_memory.h"
> >> >> >  #include "util/u_suballoc.h"
> >> >> >  #include "util/u_tests.h"
> >> >> >  #include "vl/vl_decoder.h"
> >> >> >  #include "../ddebug/dd_util.h"
> >> >> >
> >> >> >  #define SI_LLVM_DEFAULT_FEATURES \
> >> >> >   "+DumpCode,+vgpr-spilling,-fp32-denormals,-xnack"
> >> >> >
> >> >> > +/* DX10/11 apply primitive restart to strip primitive types only.
> */
> >> >> > +static const char *apps_with_prim_restart_dx_behavior[] = {
> >> >> > + "DeusExMD",
> >> >> > + "DirtRally",
> >> >> > + "HitmanPro",
> >> >> > + "MadMax",
> >> >> > + "TotalWarhammer",
> >> >> > +};
> >> >> > +
> >> >>
> >> >> Hi Marek,
> >> >>
> >> >> You seem to be adding driver workarounds for an incomplete list of
> >> >> Feral
> >> >> Interactive's titles.  Presumably, if you're going to go this route,
> >> >> you
> >> >> may need to add more of them.  Or, perhaps this is something they can
> >> >> fix in their translator layer, so they only enable it when they want
> >> >> it?
> >> >>
> >> >> I've copied Marc and Alex from Feral in case they want to weigh in.
> >> >>
> >> >> --Ken
> >> >
> >> >
> >
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-06-16 Thread Marek Olšák
Hi,

Feral's games still enable primitive restart for all draw calls.

FYI, I will push this patch on Monday if there is no other feedback.
Some other points:
- This is not the first occurrence of private app lists in drivers.
r300 also has an app list in r300_chipset.c.
- The list of Feral's games needing this workaround was indeed
complete at the time of writing the patch.

Marek


On Tue, Apr 25, 2017 at 11:26 AM, Marc Di Luzio
 wrote:
>> Thanks. Do you plan to update the games not to enable primitive
>> restart for non-strip primitives?
>
> I won't be able to give a decent time frame yet, but yes I'll make sure it's
> on our schedule.
>
> -
> Marc Di Luzio
> Linux Group Lead @ Feral Interactive Ltd.
>
> On 25 April 2017 at 10:15, Marek Olšák  wrote:
>>
>> On Tue, Apr 25, 2017 at 11:09 AM, Marc Di Luzio
>>  wrote:
>> > Hi Marek,
>> >
>> > I agree with Ken here.
>> >
>> > For what it's worth, the list of our titles that use primitive restart
>> > here
>> > is likely the full list. DXMD was the first as far as I know - see
>> >
>> > https://cgit.freedesktop.org/mesa/mesa/commit/?id=e33f31d61f5e9019f8b0bac0378dfb8fd1147421.
>> > It also appears to be an app side issue so we will patch as needed.
>> >
>> > In the future let us know first, in pretty much all cases we'd prefer to
>> > make the change on our side instead of adding game specific hacks in
>> > Mesa.
>>
>> Thanks. Do you plan to update the games not to enable primitive
>> restart for non-strip primitives?
>>
>> Marek
>>
>> >
>> > Cheers,
>> >
>> >
>> > -
>> > Marc Di Luzio
>> > Linux Group Lead @ Feral Interactive Ltd.
>> >
>> > On 24 April 2017 at 23:26, Kenneth Graunke 
>> > wrote:
>> >>
>> >> On Monday, April 24, 2017 6:22:41 AM PDT Marek Olšák wrote:
>> >> > From: Marek Olšák 
>> >> >
>> >> > ---
>> >> >  src/gallium/drivers/radeonsi/si_pipe.c   | 20 +
>> >> >  src/gallium/drivers/radeonsi/si_pipe.h   |  1 +
>> >> >  src/gallium/drivers/radeonsi/si_state_draw.c | 45
>> >> > 
>> >> >  3 files changed, 54 insertions(+), 12 deletions(-)
>> >> >
>> >> > diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
>> >> > b/src/gallium/drivers/radeonsi/si_pipe.c
>> >> > index 1a83564..53a8201 100644
>> >> > --- a/src/gallium/drivers/radeonsi/si_pipe.c
>> >> > +++ b/src/gallium/drivers/radeonsi/si_pipe.c
>> >> > @@ -29,20 +29,29 @@
>> >> >  #include "radeon/radeon_uvd.h"
>> >> >  #include "util/u_memory.h"
>> >> >  #include "util/u_suballoc.h"
>> >> >  #include "util/u_tests.h"
>> >> >  #include "vl/vl_decoder.h"
>> >> >  #include "../ddebug/dd_util.h"
>> >> >
>> >> >  #define SI_LLVM_DEFAULT_FEATURES \
>> >> >   "+DumpCode,+vgpr-spilling,-fp32-denormals,-xnack"
>> >> >
>> >> > +/* DX10/11 apply primitive restart to strip primitive types only. */
>> >> > +static const char *apps_with_prim_restart_dx_behavior[] = {
>> >> > + "DeusExMD",
>> >> > + "DirtRally",
>> >> > + "HitmanPro",
>> >> > + "MadMax",
>> >> > + "TotalWarhammer",
>> >> > +};
>> >> > +
>> >>
>> >> Hi Marek,
>> >>
>> >> You seem to be adding driver workarounds for an incomplete list of
>> >> Feral
>> >> Interactive's titles.  Presumably, if you're going to go this route,
>> >> you
>> >> may need to add more of them.  Or, perhaps this is something they can
>> >> fix in their translator layer, so they only enable it when they want
>> >> it?
>> >>
>> >> I've copied Marc and Alex from Feral in case they want to weigh in.
>> >>
>> >> --Ken
>> >
>> >
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-25 Thread Marc Di Luzio
> Thanks. Do you plan to update the games not to enable primitive
> restart for non-strip primitives?

I won't be able to give a decent time frame yet, but yes I'll make sure
it's on our schedule.

-
Marc Di Luzio
Linux Group Lead @ Feral Interactive Ltd.

On 25 April 2017 at 10:15, Marek Olšák  wrote:

> On Tue, Apr 25, 2017 at 11:09 AM, Marc Di Luzio
>  wrote:
> > Hi Marek,
> >
> > I agree with Ken here.
> >
> > For what it's worth, the list of our titles that use primitive restart
> here
> > is likely the full list. DXMD was the first as far as I know - see
> > https://cgit.freedesktop.org/mesa/mesa/commit/?id=
> e33f31d61f5e9019f8b0bac0378dfb8fd1147421.
> > It also appears to be an app side issue so we will patch as needed.
> >
> > In the future let us know first, in pretty much all cases we'd prefer to
> > make the change on our side instead of adding game specific hacks in
> Mesa.
>
> Thanks. Do you plan to update the games not to enable primitive
> restart for non-strip primitives?
>
> Marek
>
> >
> > Cheers,
> >
> >
> > -
> > Marc Di Luzio
> > Linux Group Lead @ Feral Interactive Ltd.
> >
> > On 24 April 2017 at 23:26, Kenneth Graunke 
> wrote:
> >>
> >> On Monday, April 24, 2017 6:22:41 AM PDT Marek Olšák wrote:
> >> > From: Marek Olšák 
> >> >
> >> > ---
> >> >  src/gallium/drivers/radeonsi/si_pipe.c   | 20 +
> >> >  src/gallium/drivers/radeonsi/si_pipe.h   |  1 +
> >> >  src/gallium/drivers/radeonsi/si_state_draw.c | 45
> >> > 
> >> >  3 files changed, 54 insertions(+), 12 deletions(-)
> >> >
> >> > diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
> >> > b/src/gallium/drivers/radeonsi/si_pipe.c
> >> > index 1a83564..53a8201 100644
> >> > --- a/src/gallium/drivers/radeonsi/si_pipe.c
> >> > +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> >> > @@ -29,20 +29,29 @@
> >> >  #include "radeon/radeon_uvd.h"
> >> >  #include "util/u_memory.h"
> >> >  #include "util/u_suballoc.h"
> >> >  #include "util/u_tests.h"
> >> >  #include "vl/vl_decoder.h"
> >> >  #include "../ddebug/dd_util.h"
> >> >
> >> >  #define SI_LLVM_DEFAULT_FEATURES \
> >> >   "+DumpCode,+vgpr-spilling,-fp32-denormals,-xnack"
> >> >
> >> > +/* DX10/11 apply primitive restart to strip primitive types only. */
> >> > +static const char *apps_with_prim_restart_dx_behavior[] = {
> >> > + "DeusExMD",
> >> > + "DirtRally",
> >> > + "HitmanPro",
> >> > + "MadMax",
> >> > + "TotalWarhammer",
> >> > +};
> >> > +
> >>
> >> Hi Marek,
> >>
> >> You seem to be adding driver workarounds for an incomplete list of Feral
> >> Interactive's titles.  Presumably, if you're going to go this route, you
> >> may need to add more of them.  Or, perhaps this is something they can
> >> fix in their translator layer, so they only enable it when they want it?
> >>
> >> I've copied Marc and Alex from Feral in case they want to weigh in.
> >>
> >> --Ken
> >
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-25 Thread Marek Olšák
On Tue, Apr 25, 2017 at 11:09 AM, Marc Di Luzio
 wrote:
> Hi Marek,
>
> I agree with Ken here.
>
> For what it's worth, the list of our titles that use primitive restart here
> is likely the full list. DXMD was the first as far as I know - see
> https://cgit.freedesktop.org/mesa/mesa/commit/?id=e33f31d61f5e9019f8b0bac0378dfb8fd1147421.
> It also appears to be an app side issue so we will patch as needed.
>
> In the future let us know first, in pretty much all cases we'd prefer to
> make the change on our side instead of adding game specific hacks in Mesa.

Thanks. Do you plan to update the games not to enable primitive
restart for non-strip primitives?

Marek

>
> Cheers,
>
>
> -
> Marc Di Luzio
> Linux Group Lead @ Feral Interactive Ltd.
>
> On 24 April 2017 at 23:26, Kenneth Graunke  wrote:
>>
>> On Monday, April 24, 2017 6:22:41 AM PDT Marek Olšák wrote:
>> > From: Marek Olšák 
>> >
>> > ---
>> >  src/gallium/drivers/radeonsi/si_pipe.c   | 20 +
>> >  src/gallium/drivers/radeonsi/si_pipe.h   |  1 +
>> >  src/gallium/drivers/radeonsi/si_state_draw.c | 45
>> > 
>> >  3 files changed, 54 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
>> > b/src/gallium/drivers/radeonsi/si_pipe.c
>> > index 1a83564..53a8201 100644
>> > --- a/src/gallium/drivers/radeonsi/si_pipe.c
>> > +++ b/src/gallium/drivers/radeonsi/si_pipe.c
>> > @@ -29,20 +29,29 @@
>> >  #include "radeon/radeon_uvd.h"
>> >  #include "util/u_memory.h"
>> >  #include "util/u_suballoc.h"
>> >  #include "util/u_tests.h"
>> >  #include "vl/vl_decoder.h"
>> >  #include "../ddebug/dd_util.h"
>> >
>> >  #define SI_LLVM_DEFAULT_FEATURES \
>> >   "+DumpCode,+vgpr-spilling,-fp32-denormals,-xnack"
>> >
>> > +/* DX10/11 apply primitive restart to strip primitive types only. */
>> > +static const char *apps_with_prim_restart_dx_behavior[] = {
>> > + "DeusExMD",
>> > + "DirtRally",
>> > + "HitmanPro",
>> > + "MadMax",
>> > + "TotalWarhammer",
>> > +};
>> > +
>>
>> Hi Marek,
>>
>> You seem to be adding driver workarounds for an incomplete list of Feral
>> Interactive's titles.  Presumably, if you're going to go this route, you
>> may need to add more of them.  Or, perhaps this is something they can
>> fix in their translator layer, so they only enable it when they want it?
>>
>> I've copied Marc and Alex from Feral in case they want to weigh in.
>>
>> --Ken
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-25 Thread Marc Di Luzio
Hi Marek,

I agree with Ken here.

For what it's worth, the list of our titles that use primitive restart here
is likely the full list. DXMD was the first as far as I know - see
https://cgit.freedesktop.org/mesa/mesa/commit/?id=
e33f31d61f5e9019f8b0bac0378dfb8fd1147421. It also appears to be an app side
issue so we will patch as needed.

In the future let us know first, in pretty much all cases we'd prefer to
make the change on our side instead of adding game specific hacks in Mesa.

Cheers,


-
Marc Di Luzio
Linux Group Lead @ Feral Interactive Ltd.

On 24 April 2017 at 23:26, Kenneth Graunke  wrote:

> On Monday, April 24, 2017 6:22:41 AM PDT Marek Olšák wrote:
> > From: Marek Olšák 
> >
> > ---
> >  src/gallium/drivers/radeonsi/si_pipe.c   | 20 +
> >  src/gallium/drivers/radeonsi/si_pipe.h   |  1 +
> >  src/gallium/drivers/radeonsi/si_state_draw.c | 45
> 
> >  3 files changed, 54 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
> b/src/gallium/drivers/radeonsi/si_pipe.c
> > index 1a83564..53a8201 100644
> > --- a/src/gallium/drivers/radeonsi/si_pipe.c
> > +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> > @@ -29,20 +29,29 @@
> >  #include "radeon/radeon_uvd.h"
> >  #include "util/u_memory.h"
> >  #include "util/u_suballoc.h"
> >  #include "util/u_tests.h"
> >  #include "vl/vl_decoder.h"
> >  #include "../ddebug/dd_util.h"
> >
> >  #define SI_LLVM_DEFAULT_FEATURES \
> >   "+DumpCode,+vgpr-spilling,-fp32-denormals,-xnack"
> >
> > +/* DX10/11 apply primitive restart to strip primitive types only. */
> > +static const char *apps_with_prim_restart_dx_behavior[] = {
> > + "DeusExMD",
> > + "DirtRally",
> > + "HitmanPro",
> > + "MadMax",
> > + "TotalWarhammer",
> > +};
> > +
>
> Hi Marek,
>
> You seem to be adding driver workarounds for an incomplete list of Feral
> Interactive's titles.  Presumably, if you're going to go this route, you
> may need to add more of them.  Or, perhaps this is something they can
> fix in their translator layer, so they only enable it when they want it?
>
> I've copied Marc and Alex from Feral in case they want to weigh in.
>
> --Ken
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-24 Thread Michel Dänzer
On 25/04/17 01:43 AM, Nicolai Hähnle wrote:
> On 24.04.2017 18:32, Marek Olšák wrote:
>> On Mon, Apr 24, 2017 at 5:34 PM, Nicolai Hähnle 
>> wrote:
>>> On 24.04.2017 15:22, Marek Olšák wrote:

 From: Marek Olšák 
>>>
>>>
>>> I don't like it. This kind of app-specific override is what drirc was
>>> meant
>>> to provide. Having separate places for it is confusing.
>>
>> The question is: would other drivers want this code in st_draw_vbo?
>> For threaded gallium, we shouldn't put more stuff into st_draw_vbo.
>>
>> Alternatively, it can be a flag for pipe_screen::context_create to
>> enable this behavior in radeonsi and keep the list in drirc.
> 
> Is there a particular reason we can't let radeonsi (and other gallium
> drivers) access drirc directly?

Yeah, something like that would be a better approach. This patch is too
hackish.


-- 
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 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-24 Thread Kenneth Graunke
On Monday, April 24, 2017 6:22:41 AM PDT Marek Olšák wrote:
> From: Marek Olšák 
> 
> ---
>  src/gallium/drivers/radeonsi/si_pipe.c   | 20 +
>  src/gallium/drivers/radeonsi/si_pipe.h   |  1 +
>  src/gallium/drivers/radeonsi/si_state_draw.c | 45 
> 
>  3 files changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index 1a83564..53a8201 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -29,20 +29,29 @@
>  #include "radeon/radeon_uvd.h"
>  #include "util/u_memory.h"
>  #include "util/u_suballoc.h"
>  #include "util/u_tests.h"
>  #include "vl/vl_decoder.h"
>  #include "../ddebug/dd_util.h"
>  
>  #define SI_LLVM_DEFAULT_FEATURES \
>   "+DumpCode,+vgpr-spilling,-fp32-denormals,-xnack"
>  
> +/* DX10/11 apply primitive restart to strip primitive types only. */
> +static const char *apps_with_prim_restart_dx_behavior[] = {
> + "DeusExMD",
> + "DirtRally",
> + "HitmanPro",
> + "MadMax",
> + "TotalWarhammer",
> +};
> +

Hi Marek,

You seem to be adding driver workarounds for an incomplete list of Feral
Interactive's titles.  Presumably, if you're going to go this route, you
may need to add more of them.  Or, perhaps this is something they can
fix in their translator layer, so they only enable it when they want it?

I've copied Marc and Alex from Feral in case they want to weigh in.

--Ken


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-24 Thread Samuel Pitoiset



On 04/24/2017 06:32 PM, Marek Olšák wrote:

On Mon, Apr 24, 2017 at 5:34 PM, Nicolai Hähnle  wrote:

On 24.04.2017 15:22, Marek Olšák wrote:


From: Marek Olšák 



I don't like it. This kind of app-specific override is what drirc was meant
to provide. Having separate places for it is confusing.


The question is: would other drivers want this code in st_draw_vbo?
For threaded gallium, we shouldn't put more stuff into st_draw_vbo.

Alternatively, it can be a flag for pipe_screen::context_create to
enable this behavior in radeonsi and keep the list in drirc.


I do agree with Nicolai, it would be nice to avoid adding app-specific 
workarounds directly in the driver.


Maybe you can add a new function in pipe_screen, like 
set_context_options() with a new flag or something? There are many 
get_XXX() functions there, maybe it makes sense to add a set_something().




Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-24 Thread Marek Olšák
On Mon, Apr 24, 2017 at 6:43 PM, Nicolai Hähnle  wrote:
> On 24.04.2017 18:32, Marek Olšák wrote:
>>
>> On Mon, Apr 24, 2017 at 5:34 PM, Nicolai Hähnle 
>> wrote:
>>>
>>> On 24.04.2017 15:22, Marek Olšák wrote:


 From: Marek Olšák 
>>>
>>>
>>>
>>> I don't like it. This kind of app-specific override is what drirc was
>>> meant
>>> to provide. Having separate places for it is confusing.
>>
>>
>> The question is: would other drivers want this code in st_draw_vbo?
>> For threaded gallium, we shouldn't put more stuff into st_draw_vbo.
>>
>> Alternatively, it can be a flag for pipe_screen::context_create to
>> enable this behavior in radeonsi and keep the list in drirc.
>
>
> Is there a particular reason we can't let radeonsi (and other gallium
> drivers) access drirc directly?

Windows I guess. st/dri provides drirc and is Linux-only.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-24 Thread Nicolai Hähnle

On 24.04.2017 18:32, Marek Olšák wrote:

On Mon, Apr 24, 2017 at 5:34 PM, Nicolai Hähnle  wrote:

On 24.04.2017 15:22, Marek Olšák wrote:


From: Marek Olšák 



I don't like it. This kind of app-specific override is what drirc was meant
to provide. Having separate places for it is confusing.


The question is: would other drivers want this code in st_draw_vbo?
For threaded gallium, we shouldn't put more stuff into st_draw_vbo.

Alternatively, it can be a flag for pipe_screen::context_create to
enable this behavior in radeonsi and keep the list in drirc.


Is there a particular reason we can't let radeonsi (and other gallium 
drivers) access drirc directly?


Cheers,
Nicolai



Marek




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-24 Thread Marek Olšák
On Mon, Apr 24, 2017 at 5:34 PM, Nicolai Hähnle  wrote:
> On 24.04.2017 15:22, Marek Olšák wrote:
>>
>> From: Marek Olšák 
>
>
> I don't like it. This kind of app-specific override is what drirc was meant
> to provide. Having separate places for it is confusing.

The question is: would other drivers want this code in st_draw_vbo?
For threaded gallium, we shouldn't put more stuff into st_draw_vbo.

Alternatively, it can be a flag for pipe_screen::context_create to
enable this behavior in radeonsi and keep the list in drirc.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-24 Thread Nicolai Hähnle

On 24.04.2017 15:22, Marek Olšák wrote:

From: Marek Olšák 


I don't like it. This kind of app-specific override is what drirc was 
meant to provide. Having separate places for it is confusing.


Cheers,
Nicolai



---
 src/gallium/drivers/radeonsi/si_pipe.c   | 20 +
 src/gallium/drivers/radeonsi/si_pipe.h   |  1 +
 src/gallium/drivers/radeonsi/si_state_draw.c | 45 
 3 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c
index 1a83564..53a8201 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -29,20 +29,29 @@
 #include "radeon/radeon_uvd.h"
 #include "util/u_memory.h"
 #include "util/u_suballoc.h"
 #include "util/u_tests.h"
 #include "vl/vl_decoder.h"
 #include "../ddebug/dd_util.h"

 #define SI_LLVM_DEFAULT_FEATURES \
"+DumpCode,+vgpr-spilling,-fp32-denormals,-xnack"

+/* DX10/11 apply primitive restart to strip primitive types only. */
+static const char *apps_with_prim_restart_dx_behavior[] = {
+   "DeusExMD",
+   "DirtRally",
+   "HitmanPro",
+   "MadMax",
+   "TotalWarhammer",
+};
+
 /*
  * pipe_context
  */
 static void si_destroy_context(struct pipe_context *context)
 {
struct si_context *sctx = (struct si_context *)context;
int i;

/* Unreference the framebuffer normally to disable related logic
 * properly.
@@ -306,20 +315,31 @@ static struct pipe_context *si_create_context(struct 
pipe_screen *screen,
 *
 * The recommended value is 4 per CU at most. Higher numbers don't
 * bring much benefit, but they still occupy chip resources (think
 * async compute). I've seen ~2% performance difference between 4 and 
32.
 */
sctx->scratch_waves = MAX2(32 * sscreen->b.info.num_good_compute_units,
   max_threads_per_block / 64);

sctx->tm = si_create_llvm_target_machine(sscreen);

+   /* Process the app list. */
+   char process_name[128];
+   if (os_get_process_name(process_name, sizeof(process_name))) {
+   for (i = 0; i < ARRAY_SIZE(apps_with_prim_restart_dx_behavior); 
i++) {
+   if (strcmp(process_name, 
apps_with_prim_restart_dx_behavior[i]) == 0) {
+   sctx->use_prim_restart_dx_behavior = true;
+   break;
+   }
+   }
+   }
+
return >b.b;
 fail:
fprintf(stderr, "radeonsi: Failed to create a context.\n");
si_destroy_context(>b.b);
return NULL;
 }

 /*
  * pipe_screen
  */
diff --git a/src/gallium/drivers/radeonsi/si_pipe.h 
b/src/gallium/drivers/radeonsi/si_pipe.h
index ea61e1e..1edcfbc 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.h
+++ b/src/gallium/drivers/radeonsi/si_pipe.h
@@ -236,20 +236,21 @@ struct si_context {

struct radeon_winsys_cs *ce_ib;
struct radeon_winsys_cs *ce_preamble_ib;
boolce_need_synchronization;
struct u_suballocator   *ce_suballocator;

struct si_shader_ctx_state  fixed_func_tcs_shader;
LLVMTargetMachineReftm; /* only non-threaded compilation */
boolgfx_flush_in_progress;
boolcompute_is_busy;
+   booluse_prim_restart_dx_behavior;

/* Atoms (direct states). */
union si_state_atomsatoms;
unsigneddirty_atoms; /* mask */
/* PM4 states (precomputed immutable states) */
unsigneddirty_states;
union si_state  queued;
union si_state  emitted;

/* Atom declarations. */
diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c 
b/src/gallium/drivers/radeonsi/si_state_draw.c
index e6a9ee0..319160e 100644
--- a/src/gallium/drivers/radeonsi/si_state_draw.c
+++ b/src/gallium/drivers/radeonsi/si_state_draw.c
@@ -445,42 +445,43 @@ void si_init_ia_multi_vgt_param_table(struct si_context 
*sctx)
key.u.tcs_tes_uses_prim_id = tess_uses_primid;
key.u.uses_gs = uses_gs;

sctx->ia_multi_vgt_param[key.index] =
si_get_init_multi_vgt_param(sctx->screen, );
}
 }

 static unsigned si_get_ia_multi_vgt_param(struct si_context *sctx,
  const struct pipe_draw_info *info,
- unsigned num_patches)
+ unsigned num_patches,
+ bool primitive_restart)
 {
union si_vgt_param_key key = sctx->ia_multi_vgt_param_key;
unsigned primgroup_size;
unsigned ia_multi_vgt_param;


[Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list

2017-04-24 Thread Marek Olšák
From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_pipe.c   | 20 +
 src/gallium/drivers/radeonsi/si_pipe.h   |  1 +
 src/gallium/drivers/radeonsi/si_state_draw.c | 45 
 3 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c
index 1a83564..53a8201 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -29,20 +29,29 @@
 #include "radeon/radeon_uvd.h"
 #include "util/u_memory.h"
 #include "util/u_suballoc.h"
 #include "util/u_tests.h"
 #include "vl/vl_decoder.h"
 #include "../ddebug/dd_util.h"
 
 #define SI_LLVM_DEFAULT_FEATURES \
"+DumpCode,+vgpr-spilling,-fp32-denormals,-xnack"
 
+/* DX10/11 apply primitive restart to strip primitive types only. */
+static const char *apps_with_prim_restart_dx_behavior[] = {
+   "DeusExMD",
+   "DirtRally",
+   "HitmanPro",
+   "MadMax",
+   "TotalWarhammer",
+};
+
 /*
  * pipe_context
  */
 static void si_destroy_context(struct pipe_context *context)
 {
struct si_context *sctx = (struct si_context *)context;
int i;
 
/* Unreference the framebuffer normally to disable related logic
 * properly.
@@ -306,20 +315,31 @@ static struct pipe_context *si_create_context(struct 
pipe_screen *screen,
 *
 * The recommended value is 4 per CU at most. Higher numbers don't
 * bring much benefit, but they still occupy chip resources (think
 * async compute). I've seen ~2% performance difference between 4 and 
32.
 */
sctx->scratch_waves = MAX2(32 * sscreen->b.info.num_good_compute_units,
   max_threads_per_block / 64);
 
sctx->tm = si_create_llvm_target_machine(sscreen);
 
+   /* Process the app list. */
+   char process_name[128];
+   if (os_get_process_name(process_name, sizeof(process_name))) {
+   for (i = 0; i < ARRAY_SIZE(apps_with_prim_restart_dx_behavior); 
i++) {
+   if (strcmp(process_name, 
apps_with_prim_restart_dx_behavior[i]) == 0) {
+   sctx->use_prim_restart_dx_behavior = true;
+   break;
+   }
+   }
+   }
+
return >b.b;
 fail:
fprintf(stderr, "radeonsi: Failed to create a context.\n");
si_destroy_context(>b.b);
return NULL;
 }
 
 /*
  * pipe_screen
  */
diff --git a/src/gallium/drivers/radeonsi/si_pipe.h 
b/src/gallium/drivers/radeonsi/si_pipe.h
index ea61e1e..1edcfbc 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.h
+++ b/src/gallium/drivers/radeonsi/si_pipe.h
@@ -236,20 +236,21 @@ struct si_context {
 
struct radeon_winsys_cs *ce_ib;
struct radeon_winsys_cs *ce_preamble_ib;
boolce_need_synchronization;
struct u_suballocator   *ce_suballocator;
 
struct si_shader_ctx_state  fixed_func_tcs_shader;
LLVMTargetMachineReftm; /* only non-threaded compilation */
boolgfx_flush_in_progress;
boolcompute_is_busy;
+   booluse_prim_restart_dx_behavior;
 
/* Atoms (direct states). */
union si_state_atomsatoms;
unsigneddirty_atoms; /* mask */
/* PM4 states (precomputed immutable states) */
unsigneddirty_states;
union si_state  queued;
union si_state  emitted;
 
/* Atom declarations. */
diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c 
b/src/gallium/drivers/radeonsi/si_state_draw.c
index e6a9ee0..319160e 100644
--- a/src/gallium/drivers/radeonsi/si_state_draw.c
+++ b/src/gallium/drivers/radeonsi/si_state_draw.c
@@ -445,42 +445,43 @@ void si_init_ia_multi_vgt_param_table(struct si_context 
*sctx)
key.u.tcs_tes_uses_prim_id = tess_uses_primid;
key.u.uses_gs = uses_gs;
 
sctx->ia_multi_vgt_param[key.index] =
si_get_init_multi_vgt_param(sctx->screen, );
}
 }
 
 static unsigned si_get_ia_multi_vgt_param(struct si_context *sctx,
  const struct pipe_draw_info *info,
- unsigned num_patches)
+ unsigned num_patches,
+ bool primitive_restart)
 {
union si_vgt_param_key key = sctx->ia_multi_vgt_param_key;
unsigned primgroup_size;
unsigned ia_multi_vgt_param;
 
if (sctx->tes_shader.cso) {
primgroup_size = num_patches; /* must be a multiple of 
NUM_PATCHES */
} else if (sctx->gs_shader.cso) {