[Mesa-dev] [PATCH] llvmpipe : Fixed an issue where the display target texture was mapped multiple times.

[Roland Scheidegger]

Am 19.04.2018 um 08:04 schrieb Seongchan Jeong:
> The lp_setup_set_fragment_sampler_views function can be called
> when the texture module is enabled. However, mapping can be
> performed several times for one display target texture, but
> unmapping does not proceed. So some logic have been added to
> unmap the display target texture to prevent additional mappings
> when the texture is already mapped.
> ---
>  src/gallium/drivers/llvmpipe/lp_setup.c   | 9 +++++++++
>  src/gallium/drivers/llvmpipe/lp_texture.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
> index c157323133..71ceafe2b7 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> @@ -907,6 +907,13 @@ lp_setup_set_fragment_sampler_views(struct lp_setup_context *setup,
>               */
>              struct llvmpipe_screen *screen = llvmpipe_screen(res->screen);
>              struct sw_winsys *winsys = screen->winsys;
> +
> +            /* unmap the texture which is already mapped */
> +            if(lp_tex->mapped){
> +                winsys->displaytarget_unmap(winsys, lp_tex->dt);
> +                lp_tex->mapped = false;
> +            }
> +
>              jit_tex->base = winsys->displaytarget_map(winsys, lp_tex->dt,
>                                                           PIPE_TRANSFER_READ);
>              jit_tex->row_stride[0] = lp_tex->row_stride[0];
> @@ -917,6 +924,8 @@ lp_setup_set_fragment_sampler_views(struct lp_setup_context *setup,
>              jit_tex->depth = res->depth0;
>              jit_tex->first_level = jit_tex->last_level = 0;
>              assert(jit_tex->base);
> +
> +            lp_tex->mapped = true;

I am not quite convinced this is the right fix.
Clearly the code right now isn't right, and pretty much relies on the
winsys->displaytarget_map() being a no-op there just giving the mapping
without any side effects.
The problem with this fix is it still would be kept mapped in the end
after sampling (and, it can and probably will be mapped elsewhere too
still).

Do you hit any specific bug with the code as-is?

Roland


>           }
>        }
>        else {
> diff --git a/src/gallium/drivers/llvmpipe/lp_texture.h b/src/gallium/drivers/llvmpipe/lp_texture.h
> index 3d315bb9a7..9e39d31eb3 100644
> --- a/src/gallium/drivers/llvmpipe/lp_texture.h
> +++ b/src/gallium/drivers/llvmpipe/lp_texture.h
> @@ -75,6 +75,8 @@ struct llvmpipe_resource
>      */
>     struct sw_displaytarget *dt;
>  
> +   boolean mapped; /** status of displaytarget, map or unmap */
> +
>     /**
>      * Malloc'ed data for regular textures, or a mapping to dt above.
>      */
>