<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 19, 2019 at 4:29 AM Erik Faye-Lund <<a href="mailto:erik.faye-lund@collabora.com">erik.faye-lund@collabora.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, 2019-03-18 at 14:44 -0700, Lepton Wu wrote:<br>
> virgl render complains about "Illegal resource" when running<br>
> dEQP-EGL.functional.color_clears.single_context.gles2.rgb888_window,<br>
> the reason is that a zero bind value was given for temp resource.<br>
> <br>
> Signed-off-by: Lepton Wu <<a href="mailto:lepton@chromium.org" target="_blank">lepton@chromium.org</a>><br>
> ---<br>
>  src/gallium/drivers/virgl/virgl_texture.c | 1 +<br>
>  1 file changed, 1 insertion(+)<br>
> <br>
> diff --git a/src/gallium/drivers/virgl/virgl_texture.c<br>
> b/src/gallium/drivers/virgl/virgl_texture.c<br>
> index 231319899e0..563dbacba7e 100644<br>
> --- a/src/gallium/drivers/virgl/virgl_texture.c<br>
> +++ b/src/gallium/drivers/virgl/virgl_texture.c<br>
> @@ -66,6 +66,7 @@ static void<br>
> virgl_init_temp_resource_from_box(struct pipe_resource *res,<br>
>                                                unsigned level,<br>
> unsigned flags)<br>
>  {<br>
>     memset(res, 0, sizeof(*res));<br>
> +   res->bind = orig->bind;<br>
>     res->format = orig->format;<br>
>     res->width0 = box->width;<br>
>     res->height0 = box->height;<br>
<br>
I have a similar-ish patch for the same issue in a branch I'll be<br>
sending out soon:<br>
<br>
<a href="https://gitlab.freedesktop.org/kusma/mesa/commit/6c19b6b98025a1be31eabdb559709b18eecdbafa#note_132855" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/kusma/mesa/commit/6c19b6b98025a1be31eabdb559709b18eecdbafa#note_132855</a><br>
<br>
Now, as Dave pointed out, there might be some more cases missing in my<br>
patch. I also tried your approach, and it works for me. But I'm not<br>
entirely sure it's the right one; for instance I don't think we'd ever<br>
want to carry flags like PIPE_BIND_DISPLAY_TARGET and<br>
PIPE_BIND_BLENDABLE forward.<br>
<br>
Perhaps the right thing is to do something like:<br>
<br>
res->bind = orig->bind & (PIPE_BIND_DEPTH_STENCIL |<br>
                          PIPE_BIND_RENDER_TARGET);<br>
<br>
But I'm not sure if that's enough; what if we get a surface with<br>
PIPE_BIND_SHADER_IMAGE set? We probably still want to use<br>
PIPE_BIND_RENDER_TARGET for the blit then...<br></blockquote><div>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 </div><div>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.</div></div></div>