<div dir="ltr"><div>Dear Roland Scheidegger</div><div><br></div><div>Thank you very much for your time and efforts.</div><div><br></div><div>First, I want to talk about the problem that I encountered.</div><div>I am currently developing a display server system using the llvmpipe driver</div><div>and the kms-dri winsys module. During the compositing process, winsys-></div><div>displaytarget_map () will be called continuously for the same resource. So </div><div>in the case of kms winsys, mmap () returns MAP_FAILED inside the </div><div>kms_sw_displaytarget_map function, and segfault terminates the process.</div><div><br></div><div>I also do not think this patch is perfect. As you said, the resource is still </div><div>mapped. But in my opinion, this approach is a good way to solve the </div><div>aforementioned critical issues.</div><div><br></div><div>What do you think? I look forward to your reply.</div><div>Sincerely yours,</div><div><br></div><div>Seongchan Jeong.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2018-04-20 11:15 GMT+09:00 Roland Scheidegger <span dir="ltr"><<a href="mailto:sroland@vmware.com" target="_blank">sroland@vmware.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">Am 19.04.2018 um 08:04 schrieb Seongchan Jeong:<br>
> The lp_setup_set_fragment_sampler_<wbr>views function can be called<br>
> when the texture module is enabled. However, mapping can be<br>
> performed several times for one display target texture, but<br>
> unmapping does not proceed. So some logic have been added to<br>
> unmap the display target texture to prevent additional mappings<br>
> when the texture is already mapped.<br>
> ---<br>
>  src/gallium/drivers/llvmpipe/<wbr>lp_setup.c   | 9 +++++++++<br>
>  src/gallium/drivers/llvmpipe/<wbr>lp_texture.h | 2 ++<br>
>  2 files changed, 11 insertions(+)<br>
> <br>
> diff --git a/src/gallium/drivers/<wbr>llvmpipe/lp_setup.c b/src/gallium/drivers/<wbr>llvmpipe/lp_setup.c<br>
> index c157323133..71ceafe2b7 100644<br>
> --- a/src/gallium/drivers/<wbr>llvmpipe/lp_setup.c<br>
> +++ b/src/gallium/drivers/<wbr>llvmpipe/lp_setup.c<br>
> @@ -907,6 +907,13 @@ lp_setup_set_fragment_sampler_<wbr>views(struct lp_setup_context *setup,<br>
>               */<br>
>              struct llvmpipe_screen *screen = llvmpipe_screen(res->screen);<br>
>              struct sw_winsys *winsys = screen->winsys;<br>
> +<br>
> +            /* unmap the texture which is already mapped */<br>
> +            if(lp_tex->mapped){<br>
> +                winsys->displaytarget_unmap(<wbr>winsys, lp_tex->dt);<br>
> +                lp_tex->mapped = false;<br>
> +            }<br>
> +<br>
>              jit_tex->base = winsys->displaytarget_map(<wbr>winsys, lp_tex->dt,<br>
>                                                           PIPE_TRANSFER_READ);<br>
>              jit_tex->row_stride[0] = lp_tex->row_stride[0];<br>
> @@ -917,6 +924,8 @@ lp_setup_set_fragment_sampler_<wbr>views(struct lp_setup_context *setup,<br>
>              jit_tex->depth = res->depth0;<br>
>              jit_tex->first_level = jit_tex->last_level = 0;<br>
>              assert(jit_tex->base);<br>
> +<br>
> +            lp_tex->mapped = true;<br>
<br>
</div></div>I am not quite convinced this is the right fix.<br>
Clearly the code right now isn't right, and pretty much relies on the<br>
winsys->displaytarget_map() being a no-op there just giving the mapping<br>
without any side effects.<br>
The problem with this fix is it still would be kept mapped in the end<br>
after sampling (and, it can and probably will be mapped elsewhere too<br>
still).<br>
<br>
Do you hit any specific bug with the code as-is?<br>
<span class="HOEnZb"><font color="#888888"><br>
Roland<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
>           }<br>
>        }<br>
>        else {<br>
> diff --git a/src/gallium/drivers/<wbr>llvmpipe/lp_texture.h b/src/gallium/drivers/<wbr>llvmpipe/lp_texture.h<br>
> index 3d315bb9a7..9e39d31eb3 100644<br>
> --- a/src/gallium/drivers/<wbr>llvmpipe/lp_texture.h<br>
> +++ b/src/gallium/drivers/<wbr>llvmpipe/lp_texture.h<br>
> @@ -75,6 +75,8 @@ struct llvmpipe_resource<br>
>      */<br>
>     struct sw_displaytarget *dt;<br>
>  <br>
> +   boolean mapped; /** status of displaytarget, map or unmap */<br>
> +<br>
>     /**<br>
>      * Malloc'ed data for regular textures, or a mapping to dt above.<br>
>      */<br>
> <br>
<br>
</div></div></blockquote></div><br></div>