Re: [Mesa-dev] [PATCH 2/2] radeonsi: disable primitive restart for non-strip prims based on app list
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šákwrote: > 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
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 Luziowrote: >> 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
> 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šákwrote: > 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
On Tue, Apr 25, 2017 at 11:09 AM, Marc Di Luziowrote: > 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
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 Graunkewrote: > 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
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
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
On 04/24/2017 06:32 PM, Marek Olšák wrote: On Mon, Apr 24, 2017 at 5:34 PM, Nicolai Hähnlewrote: 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
On Mon, Apr 24, 2017 at 6:43 PM, Nicolai Hähnlewrote: > 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
On 24.04.2017 18:32, Marek Olšák wrote: On Mon, Apr 24, 2017 at 5:34 PM, Nicolai Hähnlewrote: 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
On Mon, Apr 24, 2017 at 5:34 PM, Nicolai Hähnlewrote: > 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
On 24.04.2017 15:22, Marek Olšák wrote: From: Marek OlšákI 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
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) {