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

[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
<mdiluzio at feralinteractive.com> 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 <maraeo at gmail.com> wrote:
>>
>> On Tue, Apr 25, 2017 at 11:09 AM, Marc Di Luzio
>> <mdiluzio at feralinteractive.com> 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 <kenneth at whitecape.org>
>> > wrote:
>> >>
>> >> On Monday, April 24, 2017 6:22:41 AM PDT Marek Olšák wrote:
>> >> > From: Marek Olšák <marek.olsak at amd.com>
>> >> >
>> >> > ---
>> >> >  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
>> >
>> >
>
>