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

[정성찬]

Dear Roland Scheidegger

Thank you very much for your time and efforts.

First, I want to talk about the problem that I encountered.
I am currently developing a display server system using the llvmpipe driver
and the kms-dri winsys module. During the compositing process, winsys->
displaytarget_map () will be called continuously for the same resource. So
in the case of kms winsys, mmap () returns MAP_FAILED inside the
kms_sw_displaytarget_map function, and segfault terminates the process.

I also do not think this patch is perfect. As you said, the resource is
still
mapped. But in my opinion, this approach is a good way to solve the
aforementioned critical issues.

What do you think? I look forward to your reply.
Sincerely yours,

Seongchan Jeong.


2018-04-20 11:15 GMT+09:00 Roland Scheidegger <sroland at vmware.com>:

> 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.
> >      */
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180420/4f8172ac/attachment-0001.html>