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

[Gert Wollny]

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. 

Now, calling st_validate_state(st, ST_PIPELINE_RENDER) before
virgl_clear is called solves the problem, but I see that this might not
be thebest solution, because then all drivers that actually use the
blitter will validate the render state a second time without need. 

Best, 
Gert 



> 
> See for example
> https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nouve
> au/nvc0/nvc0_surface.c#n1122
> which is how a bunch of state is restored after a blit which uses the
> 3d engine.
> 
> Cheers,
> 
>   -ilia
> 
> 
> On Thu, May 3, 2018 at 11:24 AM, Gert Wollny <gert.wollny at collabora.c
> om> wrote:
> > A number of  CTS tests from the dEQP-
> > GLES3.functional.rasterizer_discard.* subset
> > fail when the tests are run in a batch, because the
> > GL_RASTERIZER_DISCARD state
> > is enabled after running glClear and the disabled state issed after
> > a number of draw
> > commands is not properly send to the host.
> > 
> > This happens because in virgl the render state is not validated
> > (and updated on the host)
> > when clear is run. With this patch  the render state is explicitely
> > updated in st_Clear
> > thereby fixing these failures.
> > 
> > Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> > 
> > ---
> > The reason why I send this as a RFC is that I'm well aware of the
> > fact that the drivers
> > that use the blitter to execute a clear do already implicitely call
> > st_validate_state for
> > ST_PIPELINE_RENDER because the blitter does so.  Unfortunetely, I
> > haven't found yet how I
> > can issue this call from virgl_clear, because I don't see how I can
> > access the rasterer
> > state from there, but maybe someone has a pointer for me?
> > 
> > Many thanks,
> > Gert
> > 
> > PS: I don't have mesa-git commit rights.
> > 
> > ---
> >  src/mesa/state_tracker/st_cb_clear.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/mesa/state_tracker/st_cb_clear.c
> > b/src/mesa/state_tracker/st_cb_clear.c
> > index fbc577a370..8f8295ea6c 100644
> > --- a/src/mesa/state_tracker/st_cb_clear.c
> > +++ b/src/mesa/state_tracker/st_cb_clear.c
> > @@ -377,6 +377,7 @@ st_Clear(struct gl_context *ctx, GLbitfield
> > mask)
> > 
> >     /* This makes sure the pipe has the latest scissor, etc values
> > */
> >     st_validate_state(st, ST_PIPELINE_CLEAR);
> > +   st_validate_state(st, ST_PIPELINE_RENDER);
> > 
> > 
> >     if (mask & BUFFER_BITS_COLOR) {
> >        for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++)
> > {
> > --
> > 2.16.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev