Re: [Mesa-dev] [PATCH] virgl: Set bind when creating temp resource.
On Mon, 2019-04-01 at 12:43 -0700, Lepton Wu wrote: > > > On Tue, Mar 19, 2019 at 4:29 AM Erik Faye-Lund < > erik.faye-l...@collabora.com> wrote: > > On Mon, 2019-03-18 at 14:44 -0700, Lepton Wu wrote: > > > virgl render complains about "Illegal resource" when running > > > dEQP- > > EGL.functional.color_clears.single_context.gles2.rgb888_window, > > > the reason is that a zero bind value was given for temp resource. > > > > > > Signed-off-by: Lepton Wu > > > --- > > > src/gallium/drivers/virgl/virgl_texture.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/src/gallium/drivers/virgl/virgl_texture.c > > > b/src/gallium/drivers/virgl/virgl_texture.c > > > index 231319899e0..563dbacba7e 100644 > > > --- a/src/gallium/drivers/virgl/virgl_texture.c > > > +++ b/src/gallium/drivers/virgl/virgl_texture.c > > > @@ -66,6 +66,7 @@ static void > > > virgl_init_temp_resource_from_box(struct pipe_resource *res, > > >unsigned level, > > > unsigned flags) > > > { > > > memset(res, 0, sizeof(*res)); > > > + res->bind = orig->bind; > > > res->format = orig->format; > > > res->width0 = box->width; > > > res->height0 = box->height; > > > > I have a similar-ish patch for the same issue in a branch I'll be > > sending out soon: > > > > https://gitlab.freedesktop.org/kusma/mesa/commit/6c19b6b98025a1be31eabdb559709b18eecdbafa#note_132855 > > > > Now, as Dave pointed out, there might be some more cases missing in > > my > > patch. I also tried your approach, and it works for me. But I'm not > > entirely sure it's the right one; for instance I don't think we'd > > ever > > want to carry flags like PIPE_BIND_DISPLAY_TARGET and > > PIPE_BIND_BLENDABLE forward. > > > > Perhaps the right thing is to do something like: > > > > res->bind = orig->bind & (PIPE_BIND_DEPTH_STENCIL | > > PIPE_BIND_RENDER_TARGET); > > > > But I'm not sure if that's enough; what if we get a surface with > > PIPE_BIND_SHADER_IMAGE set? We probably still want to use > > PIPE_BIND_RENDER_TARGET for the blit then... > > How about this, let's deal with PIPE_BIND_DEPTH_STENCIL > and PIPE_BIND_RENDER_TARGET first in this patch, also add a warning > here > if we see any other bind so later people can fix things based on > real cases. Then at least we fix something in this patch. Sounds good to me. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] virgl: Set bind when creating temp resource.
On Tue, Mar 19, 2019 at 4:29 AM Erik Faye-Lund wrote: > On Mon, 2019-03-18 at 14:44 -0700, Lepton Wu wrote: > > virgl render complains about "Illegal resource" when running > > dEQP-EGL.functional.color_clears.single_context.gles2.rgb888_window, > > the reason is that a zero bind value was given for temp resource. > > > > Signed-off-by: Lepton Wu > > --- > > src/gallium/drivers/virgl/virgl_texture.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/src/gallium/drivers/virgl/virgl_texture.c > > b/src/gallium/drivers/virgl/virgl_texture.c > > index 231319899e0..563dbacba7e 100644 > > --- a/src/gallium/drivers/virgl/virgl_texture.c > > +++ b/src/gallium/drivers/virgl/virgl_texture.c > > @@ -66,6 +66,7 @@ static void > > virgl_init_temp_resource_from_box(struct pipe_resource *res, > >unsigned level, > > unsigned flags) > > { > > memset(res, 0, sizeof(*res)); > > + res->bind = orig->bind; > > res->format = orig->format; > > res->width0 = box->width; > > res->height0 = box->height; > > I have a similar-ish patch for the same issue in a branch I'll be > sending out soon: > > > https://gitlab.freedesktop.org/kusma/mesa/commit/6c19b6b98025a1be31eabdb559709b18eecdbafa#note_132855 > > Now, as Dave pointed out, there might be some more cases missing in my > patch. I also tried your approach, and it works for me. But I'm not > entirely sure it's the right one; for instance I don't think we'd ever > want to carry flags like PIPE_BIND_DISPLAY_TARGET and > PIPE_BIND_BLENDABLE forward. > > Perhaps the right thing is to do something like: > > res->bind = orig->bind & (PIPE_BIND_DEPTH_STENCIL | > PIPE_BIND_RENDER_TARGET); > > But I'm not sure if that's enough; what if we get a surface with > PIPE_BIND_SHADER_IMAGE set? We probably still want to use > PIPE_BIND_RENDER_TARGET for the blit then... > How about this, let's deal with PIPE_BIND_DEPTH_STENCIL and PIPE_BIND_RENDER_TARGET first in this patch, also add a warning here if we see any other bind so later people can fix things based on real cases. Then at least we fix something in this patch. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] virgl: Set bind when creating temp resource.
On Mon, 2019-03-18 at 14:44 -0700, Lepton Wu wrote: > virgl render complains about "Illegal resource" when running > dEQP-EGL.functional.color_clears.single_context.gles2.rgb888_window, > the reason is that a zero bind value was given for temp resource. > > Signed-off-by: Lepton Wu > --- > src/gallium/drivers/virgl/virgl_texture.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/gallium/drivers/virgl/virgl_texture.c > b/src/gallium/drivers/virgl/virgl_texture.c > index 231319899e0..563dbacba7e 100644 > --- a/src/gallium/drivers/virgl/virgl_texture.c > +++ b/src/gallium/drivers/virgl/virgl_texture.c > @@ -66,6 +66,7 @@ static void > virgl_init_temp_resource_from_box(struct pipe_resource *res, >unsigned level, > unsigned flags) > { > memset(res, 0, sizeof(*res)); > + res->bind = orig->bind; > res->format = orig->format; > res->width0 = box->width; > res->height0 = box->height; I have a similar-ish patch for the same issue in a branch I'll be sending out soon: https://gitlab.freedesktop.org/kusma/mesa/commit/6c19b6b98025a1be31eabdb559709b18eecdbafa#note_132855 Now, as Dave pointed out, there might be some more cases missing in my patch. I also tried your approach, and it works for me. But I'm not entirely sure it's the right one; for instance I don't think we'd ever want to carry flags like PIPE_BIND_DISPLAY_TARGET and PIPE_BIND_BLENDABLE forward. Perhaps the right thing is to do something like: res->bind = orig->bind & (PIPE_BIND_DEPTH_STENCIL | PIPE_BIND_RENDER_TARGET); But I'm not sure if that's enough; what if we get a surface with PIPE_BIND_SHADER_IMAGE set? We probably still want to use PIPE_BIND_RENDER_TARGET for the blit then... ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] virgl: Set bind when creating temp resource.
virgl render complains about "Illegal resource" when running dEQP-EGL.functional.color_clears.single_context.gles2.rgb888_window, the reason is that a zero bind value was given for temp resource. Signed-off-by: Lepton Wu --- src/gallium/drivers/virgl/virgl_texture.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/drivers/virgl/virgl_texture.c b/src/gallium/drivers/virgl/virgl_texture.c index 231319899e0..563dbacba7e 100644 --- a/src/gallium/drivers/virgl/virgl_texture.c +++ b/src/gallium/drivers/virgl/virgl_texture.c @@ -66,6 +66,7 @@ static void virgl_init_temp_resource_from_box(struct pipe_resource *res, unsigned level, unsigned flags) { memset(res, 0, sizeof(*res)); + res->bind = orig->bind; res->format = orig->format; res->width0 = box->width; res->height0 = box->height; -- 2.21.0.225.g810b269d1ac-goog ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev