Re: [Mesa-dev] [PATCH] radeonsi: fix a crash when unbinding sampler states

2019-04-08 Thread Dieter Nützel 

SOLVED it.

Thanks to both of you!

Dieter

Am 08.04.2019 21:12, schrieb James Zhu:

This patch is Acked-by: James Zhu 

On 2019-04-08 3:02 p.m., Marek Olšák wrote:


On Mon, Apr 8, 2019 at 2:45 PM James Zhu  wrote:

On 2019-04-08 2:39 p.m., Marek Olšák wrote:

On Mon, Apr 8, 2019 at 2:33 PM James Zhu  wrote:

On 2019-04-08 2:25 p.m., Marek Olšák wrote:

From: Marek Olšák 

---
src/gallium/drivers/radeonsi/si_descriptors.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c

b/src/gallium/drivers/radeonsi/si_descriptors.c

index 244ba5a7bec..ac40ed27f91 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -942,21 +942,21 @@ void si_update_ps_colorbuf0_slot(struct

si_context *sctx)

static void si_bind_sampler_states(struct pipe_context *ctx,
enum pipe_shader_type shader,
unsigned start, unsigned

count, void **states)

{
struct si_context *sctx = (struct si_context *)ctx;
struct si_samplers *samplers = >samplers[shader];
struct si_descriptors *desc =

si_sampler_and_image_descriptors(sctx, shader);

struct si_sampler_state **sstates = (struct

si_sampler_state**)states;

int i;

- if (!count || shader >= SI_NUM_SHADERS)
+ if (!count || shader >= SI_NUM_SHADERS || !sstates)


if sstates == NULL, it means we want to unbind
samplers->sampler_states
from current setting.

So I think it is better not just bypass it.

The driver never unbinds constant state objects. If sstates[i] ==
NULL, it's not unbound. sstates == NULL is a similar case.


Then we should not call unbind sampler state after compute shader
launch. Since it is doing nothing.
You are right.

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] radeonsi: fix a crash when unbinding sampler states

2019-04-08 Thread James Zhu 
This patch is Acked-by: James Zhu 

On 2019-04-08 3:02 p.m., Marek Olšák wrote:
On Mon, Apr 8, 2019 at 2:45 PM James Zhu 
mailto:jam...@amd.com>> wrote:


On 2019-04-08 2:39 p.m., Marek Olšák wrote:
On Mon, Apr 8, 2019 at 2:33 PM James Zhu 
mailto:jam...@amd.com>> wrote:

On 2019-04-08 2:25 p.m., Marek Olšák wrote:
> From: Marek Olšák mailto:marek.ol...@amd.com>>
>
> ---
>   src/gallium/drivers/radeonsi/si_descriptors.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
> b/src/gallium/drivers/radeonsi/si_descriptors.c
> index 244ba5a7bec..ac40ed27f91 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -942,21 +942,21 @@ void si_update_ps_colorbuf0_slot(struct si_context 
> *sctx)
>   static void si_bind_sampler_states(struct pipe_context *ctx,
>  enum pipe_shader_type shader,
>  unsigned start, unsigned count, void 
> **states)
>   {
>   struct si_context *sctx = (struct si_context *)ctx;
>   struct si_samplers *samplers = >samplers[shader];
>   struct si_descriptors *desc = si_sampler_and_image_descriptors(sctx, 
> shader);
>   struct si_sampler_state **sstates = (struct si_sampler_state**)states;
>   int i;
>
> - if (!count || shader >= SI_NUM_SHADERS)
> + if (!count || shader >= SI_NUM_SHADERS || !sstates)

if sstates == NULL, it means we want to unbind samplers->sampler_states
from current setting.

So I think it is better not just bypass it.

The driver never unbinds constant state objects. If sstates[i] == NULL, it's 
not unbound. sstates == NULL is a similar case.

Then we should not call unbind sampler state after compute shader launch. Since 
it is doing nothing.

You are right.

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

Re: [Mesa-dev] [PATCH] radeonsi: fix a crash when unbinding sampler states

2019-04-08 Thread Marek Olšák 
On Mon, Apr 8, 2019 at 2:45 PM James Zhu  wrote:

>
> On 2019-04-08 2:39 p.m., Marek Olšák wrote:
>
> On Mon, Apr 8, 2019 at 2:33 PM James Zhu  wrote:
>
>>
>> On 2019-04-08 2:25 p.m., Marek Olšák wrote:
>> > From: Marek Olšák 
>> >
>> > ---
>> >   src/gallium/drivers/radeonsi/si_descriptors.c | 2 +-
>> >   1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c
>> b/src/gallium/drivers/radeonsi/si_descriptors.c
>> > index 244ba5a7bec..ac40ed27f91 100644
>> > --- a/src/gallium/drivers/radeonsi/si_descriptors.c
>> > +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
>> > @@ -942,21 +942,21 @@ void si_update_ps_colorbuf0_slot(struct
>> si_context *sctx)
>> >   static void si_bind_sampler_states(struct pipe_context *ctx,
>> >  enum pipe_shader_type shader,
>> >  unsigned start, unsigned count,
>> void **states)
>> >   {
>> >   struct si_context *sctx = (struct si_context *)ctx;
>> >   struct si_samplers *samplers = >samplers[shader];
>> >   struct si_descriptors *desc =
>> si_sampler_and_image_descriptors(sctx, shader);
>> >   struct si_sampler_state **sstates = (struct
>> si_sampler_state**)states;
>> >   int i;
>> >
>> > - if (!count || shader >= SI_NUM_SHADERS)
>> > + if (!count || shader >= SI_NUM_SHADERS || !sstates)
>>
>> if sstates == NULL, it means we want to unbind samplers->sampler_states
>> from current setting.
>>
>> So I think it is better not just bypass it.
>>
>
> The driver never unbinds constant state objects. If sstates[i] == NULL,
> it's not unbound. sstates == NULL is a similar case.
>
> Then we should not call unbind sampler state after compute shader launch.
> Since it is doing nothing.
>
You are right.

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

Re: [Mesa-dev] [PATCH] radeonsi: fix a crash when unbinding sampler states

2019-04-08 Thread James Zhu 

On 2019-04-08 2:39 p.m., Marek Olšák wrote:
On Mon, Apr 8, 2019 at 2:33 PM James Zhu 
mailto:jam...@amd.com>> wrote:

On 2019-04-08 2:25 p.m., Marek Olšák wrote:
> From: Marek Olšák mailto:marek.ol...@amd.com>>
>
> ---
>   src/gallium/drivers/radeonsi/si_descriptors.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
> b/src/gallium/drivers/radeonsi/si_descriptors.c
> index 244ba5a7bec..ac40ed27f91 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -942,21 +942,21 @@ void si_update_ps_colorbuf0_slot(struct si_context 
> *sctx)
>   static void si_bind_sampler_states(struct pipe_context *ctx,
>  enum pipe_shader_type shader,
>  unsigned start, unsigned count, void 
> **states)
>   {
>   struct si_context *sctx = (struct si_context *)ctx;
>   struct si_samplers *samplers = >samplers[shader];
>   struct si_descriptors *desc = si_sampler_and_image_descriptors(sctx, 
> shader);
>   struct si_sampler_state **sstates = (struct si_sampler_state**)states;
>   int i;
>
> - if (!count || shader >= SI_NUM_SHADERS)
> + if (!count || shader >= SI_NUM_SHADERS || !sstates)

if sstates == NULL, it means we want to unbind samplers->sampler_states
from current setting.

So I think it is better not just bypass it.

The driver never unbinds constant state objects. If sstates[i] == NULL, it's 
not unbound. sstates == NULL is a similar case.

Then we should not call unbind sampler state after compute shader launch. Since 
it is doing nothing.

James

It's not a standard behavior, but the driver has been doing it for a very long 
time.

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

Re: [Mesa-dev] [PATCH] radeonsi: fix a crash when unbinding sampler states

2019-04-08 Thread Marek Olšák 
On Mon, Apr 8, 2019 at 2:33 PM James Zhu  wrote:

>
> On 2019-04-08 2:25 p.m., Marek Olšák wrote:
> > From: Marek Olšák 
> >
> > ---
> >   src/gallium/drivers/radeonsi/si_descriptors.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c
> b/src/gallium/drivers/radeonsi/si_descriptors.c
> > index 244ba5a7bec..ac40ed27f91 100644
> > --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> > +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> > @@ -942,21 +942,21 @@ void si_update_ps_colorbuf0_slot(struct si_context
> *sctx)
> >   static void si_bind_sampler_states(struct pipe_context *ctx,
> >  enum pipe_shader_type shader,
> >  unsigned start, unsigned count,
> void **states)
> >   {
> >   struct si_context *sctx = (struct si_context *)ctx;
> >   struct si_samplers *samplers = >samplers[shader];
> >   struct si_descriptors *desc =
> si_sampler_and_image_descriptors(sctx, shader);
> >   struct si_sampler_state **sstates = (struct
> si_sampler_state**)states;
> >   int i;
> >
> > - if (!count || shader >= SI_NUM_SHADERS)
> > + if (!count || shader >= SI_NUM_SHADERS || !sstates)
>
> if sstates == NULL, it means we want to unbind samplers->sampler_states
> from current setting.
>
> So I think it is better not just bypass it.
>

The driver never unbinds constant state objects. If sstates[i] == NULL,
it's not unbound. sstates == NULL is a similar case.

It's not a standard behavior, but the driver has been doing it for a very
long time.

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

Re: [Mesa-dev] [PATCH] radeonsi: fix a crash when unbinding sampler states

2019-04-08 Thread James Zhu 

On 2019-04-08 2:25 p.m., Marek Olšák wrote:
> From: Marek Olšák 
>
> ---
>   src/gallium/drivers/radeonsi/si_descriptors.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
> b/src/gallium/drivers/radeonsi/si_descriptors.c
> index 244ba5a7bec..ac40ed27f91 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -942,21 +942,21 @@ void si_update_ps_colorbuf0_slot(struct si_context 
> *sctx)
>   static void si_bind_sampler_states(struct pipe_context *ctx,
>  enum pipe_shader_type shader,
>  unsigned start, unsigned count, void 
> **states)
>   {
>   struct si_context *sctx = (struct si_context *)ctx;
>   struct si_samplers *samplers = >samplers[shader];
>   struct si_descriptors *desc = si_sampler_and_image_descriptors(sctx, 
> shader);
>   struct si_sampler_state **sstates = (struct si_sampler_state**)states;
>   int i;
>   
> - if (!count || shader >= SI_NUM_SHADERS)
> + if (!count || shader >= SI_NUM_SHADERS || !sstates)

if sstates == NULL, it means we want to unbind samplers->sampler_states 
from current setting.

So I think it is better not just bypass it.

James

>   return;
>   
>   for (i = 0; i < count; i++) {
>   unsigned slot = start + i;
>   unsigned desc_slot = si_get_sampler_slot(slot);
>   
>   if (!sstates[i] ||
>   sstates[i] == samplers->sampler_states[slot])
>   continue;
>   
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radeonsi: fix a crash when unbinding sampler states

2019-04-08 Thread Marek Olšák 
From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_descriptors.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index 244ba5a7bec..ac40ed27f91 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -942,21 +942,21 @@ void si_update_ps_colorbuf0_slot(struct si_context *sctx)
 static void si_bind_sampler_states(struct pipe_context *ctx,
enum pipe_shader_type shader,
unsigned start, unsigned count, void 
**states)
 {
struct si_context *sctx = (struct si_context *)ctx;
struct si_samplers *samplers = >samplers[shader];
struct si_descriptors *desc = si_sampler_and_image_descriptors(sctx, 
shader);
struct si_sampler_state **sstates = (struct si_sampler_state**)states;
int i;
 
-   if (!count || shader >= SI_NUM_SHADERS)
+   if (!count || shader >= SI_NUM_SHADERS || !sstates)
return;
 
for (i = 0; i < count; i++) {
unsigned slot = start + i;
unsigned desc_slot = si_get_sampler_slot(slot);
 
if (!sstates[i] ||
sstates[i] == samplers->sampler_states[slot])
continue;
 
-- 
2.17.1

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