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

[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 <maraeo at gmail.com> 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
> <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
> >> >
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170616/37d67b44/attachment.html>