On Wed, Aug 17, 2016 at 12:09 PM, Nicolai Hähnle <[email protected]> wrote: > On 17.08.2016 12:04, Marek Olšák wrote: >> >> On Wed, Aug 17, 2016 at 11:41 AM, Nicolai Hähnle <[email protected]> >> wrote: >>> >>> On 11.08.2016 18:16, Marek Olšák wrote: >>>> >>>> >>>> From: Marek Olšák <[email protected]> >>>> >>>> This is just a workaround. The problem is described in the code. >>>> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96541 >>>> --- >>>> src/gallium/drivers/radeon/r600_pipe_common.h | 2 +- >>>> src/gallium/drivers/radeon/r600_texture.c | 35 >>>> ++++++++++++++++++++++----- >>>> src/gallium/drivers/radeonsi/si_blit.c | 12 +++------ >>>> src/gallium/drivers/radeonsi/si_descriptors.c | 2 +- >>>> 4 files changed, 35 insertions(+), 16 deletions(-) >>>> >>>> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h >>>> b/src/gallium/drivers/radeon/r600_pipe_common.h >>>> index e4002f9..5375044 100644 >>>> --- a/src/gallium/drivers/radeon/r600_pipe_common.h >>>> +++ b/src/gallium/drivers/radeon/r600_pipe_common.h >>>> @@ -764,21 +764,21 @@ void vi_separate_dcc_stop_query(struct >>>> pipe_context >>>> *ctx, >>>> void vi_separate_dcc_process_and_reset_stats(struct pipe_context *ctx, >>>> struct r600_texture *tex); >>>> void vi_dcc_clear_level(struct r600_common_context *rctx, >>>> struct r600_texture *rtex, >>>> unsigned level, unsigned clear_value); >>>> void evergreen_do_fast_color_clear(struct r600_common_context *rctx, >>>> struct pipe_framebuffer_state *fb, >>>> struct r600_atom *fb_state, >>>> unsigned *buffers, unsigned >>>> *dirty_cbufs, >>>> const union pipe_color_union *color); >>>> -bool r600_texture_disable_dcc(struct r600_common_screen *rscreen, >>>> +bool r600_texture_disable_dcc(struct r600_common_context *rctx, >>>> struct r600_texture *rtex); >>>> void r600_init_screen_texture_functions(struct r600_common_screen >>>> *rscreen); >>>> void r600_init_context_texture_functions(struct r600_common_context >>>> *rctx); >>>> >>>> /* r600_viewport.c */ >>>> void evergreen_apply_scissor_bug_workaround(struct r600_common_context >>>> *rctx, >>>> struct pipe_scissor_state >>>> *scissor); >>>> void r600_set_scissor_enable(struct r600_common_context *rctx, bool >>>> enable); >>>> void r600_update_vs_writes_viewport_index(struct r600_common_context >>>> *rctx, >>>> struct tgsi_shader_info >>>> *info); >>>> diff --git a/src/gallium/drivers/radeon/r600_texture.c >>>> b/src/gallium/drivers/radeon/r600_texture.c >>>> index ddfa135..78dd177 100644 >>>> --- a/src/gallium/drivers/radeon/r600_texture.c >>>> +++ b/src/gallium/drivers/radeon/r600_texture.c >>>> @@ -393,34 +393,55 @@ static bool r600_texture_discard_dcc(struct >>>> r600_common_screen *rscreen, >>>> assert(rtex->dcc_separate_buffer == NULL); >>>> >>>> /* Disable DCC. */ >>>> rtex->dcc_offset = 0; >>>> >>>> /* Notify all contexts about the change. */ >>>> r600_dirty_all_framebuffer_states(rscreen); >>>> return true; >>>> } >>>> >>>> -bool r600_texture_disable_dcc(struct r600_common_screen *rscreen, >>>> +/** >>>> + * Disable DCC for the texture. (first decompress, then discard >>>> metadata). >>>> + * >>>> + * Unresolved multi-context synchronization issue: >>>> + * >>>> + * If context 1 disables DCC and context 2 has queued commands that >>>> write >>>> + * to the texture via CB with DCC enabled, and the order of operations >>>> is >>>> + * as follows: >>>> + * context 2 queues draw calls rendering to the texture, but doesn't >>>> flush >>>> + * context 1 disables DCC and flushes >>>> + * context 1 & 2 reset descriptors and FB state >>>> + * context 2 flushes (new compressed tiles written by the draw calls) >>>> + * context 1 & 2 read garbage, because DCC is disabled, yet there are >>>> + * compressed tiled >>> >>> >>> >>> Should this case lead to defined result according to the spec? The reason >>> for context 1 disabling DCC is that it in some sense "reads" from the >>> texture -- via a blit, get_handle, or whatever. But without any >>> synchronization, those reads should be undefined anyway, because context >>> 1 >>> doesn't know whether context 2 has completed. >>> >>> Granted, the _type_ of undefined behavior is perhaps unusual, because >>> some >>> people might expect either the results from before context 2's draws or >>> from >>> after context 2's draws, not some random DCC-related garbage, but should >>> we >>> care? >>> >>> Or maybe I'm missing some other situation? >> >> >> You may be right. Maybe it's really undefined from the GL point of view. >> >> The problem here was that the driver used aux_context from the screen >> and it wasn't (and couldn't be) synchronized properly with the main >> context because of DCC. All single-context applications can get the >> corruption because aux_context is hidden to them. > > > Right. So the code change itself looks fine to me. The only question is > about the comment. I think it should be reworded slightly that this > multi-context behavior by applications should probably be undefined anyway?
Yes, I'll reword it that it's only between the current context and aux_context. Marek _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
