[Mesa-dev] [RFC PATCH] mesa/st/cb_clear: in st_Clear also validate the render state (needed by virgl)

[Ilia Mirkin]

On Thu, May 3, 2018 at 12:29 PM, Gert Wollny <gert.wollny at collabora.com> wrote:
> Am Donnerstag, den 03.05.2018, 11:43 -0400 schrieb Ilia Mirkin:
>> You're supposed to keep track of the bound state (usually in the
>> context). After your clear() implementation is done, you have to undo
>> all the state you've messed up on your "hardware" (or mark it dirty
>> for revalidation).
>
> Thanks for you answer, but I think I didn't explain the issue properly.
>
>
> The problem is not the state after the clear(), it is what is happening
> before.  To explain the specific case GL_RASTERIZER_DISCARD.
>
> In the tests I mentioned you see following for the standalone test
>
> - setup contest and buffers
> - glClear(...)
> - glEnable(GL_RASTERIZER_DISCARD)
> - ...
> - glDrawArrays(...)
> - glDisable(GL_RASTERIZER_DISCARD)
> - check result
> - Tear down buffers and context
>
> On a normal host (r600 in my case) I see state updates before clear
> (initial setup) and a state update initialized by glDrawArrays.
> The state update related to the glDisable is not really transmitted to
> the hardware since it is not needed.
>
> On virgl the first state update isn't send to the host, but since the
> test is run stand-alone, and GL_RASTERIZER_DISCARD is disabled by
> default all is fine, i.e. clear() works as expected and the glDisable
> is of no consequence.
>
> Now, if you run the test suite batched, you get
>
> - setup contest and buffers
> - FOREACH rasterizer_discard_test
> -   glClear(...)
> -   glEnable(GL_RASTERIZER_DISCARD)
> -   ...
> -   glDrawArrays(...)
> -   glDisable(GL_RASTERIZER_DISCARD)
> -   check result
> - ENDFOR
> - Tear down buffers and context
>
> Here, at each beginning of the loop the "normal" host sends a state
> update to the hardware before clear is actually executed, because the
> blitter validates the render state. Hence
> glDisable(GL_RASTERIZER_DISCARD) goes into effect in consecutive runs
> and the clear() operation actually clears the buffers.
>
> On virgl the current code path for virgl_clear doesn't validate the
> render state, i.e. it doesn't do a blit like drivers that directly talk
> to the hardware and that validates the render state before the actual
> drawing happens. Virgl it just sends the command indicating clear from
> the guest running in Qemu to the host, without validating the state
> first. Hence the host doesn't update the hardware state, because it
> doesn't know that glDisable(GL_RASTERIZER_DISCARD) was called in the
> guest, and the blit, that will eventually be executed on the host is
> still executed as if GL_RASTERIZER_DISCARD were enabled.

virgl_clear implements a gallium API call which is meant to clear the
surface. It sounds like virglrenderer does not properly implement that
call. Either workaround your buggy "hardware" (i.e. virglrenderer) by
throwing in an explicit "no, really, do this" call, or fix
virglrenderer to do the clear irrespective of what the last-set
rasterizer discard state might be. I'd recommend the latter (i.e.
fixing this in virglrenderer).

  -ilia