OK, actually struct winsys_handle is an obscure structure for virgl
driver so we can't access whandle->stride here...
So maybe just leave this CL as it is?
On Wed, Jul 17, 2019 at 1:12 PM Chia-I Wu <olva...@gmail.com> wrote:
>
> On Wed, Jul 17, 2019 at 12:45 PM Lepton Wu <lep...@chromium.org> wrote:
> > metadata->stride[0] is calculated from
> > util_format_get_stride(pt->format, pt->width0);
> > So basically you are asking to check if
> > util_format_get_stride(pt->format, pt->width0) == whandle->stride
> > Should this be something done by framework?
> The framework does not know that is how virgl decides the stride for a
> resource. That is also not the case for most drivers.
>
> The framework asks virgl to import a buffer with the specified stride.
> virgl should either accept it, or reject it if virgl does not want to
> handle the unexpected stride. Not that I believe that is going to
> happen, but still...
>
> >
> > On Wed, Jul 17, 2019 at 12:25 PM Chia-I Wu <olva...@gmail.com> wrote:
> > >
> > > On Wed, Jul 17, 2019 at 11:44 AM Lepton Wu <lep...@chromium.org> wrote:
> > > >
> > > > On Wed, Jul 17, 2019 at 11:26 AM Chia-I Wu <olva...@gmail.com> wrote:
> > > > >
> > > > > On Wed, Jul 17, 2019 at 10:14 AM Erik Faye-Lund
> > > > > <erik.faye-l...@collabora.com> wrote:
> > > > > >
> > > > > > On Wed, 2019-07-17 at 10:02 -0700, Lepton Wu wrote:
> > > > > > > The set of meta data was removed by commit 8083464. It broke lots
> > > > > > > of
> > > > > > > dEQP tests when running with pbuffer surface type.
> > > > > > >
> > > > > > > Fixes: 80834640137 ("virgl: remove dead code")
> > > > > > > Signed-off-by: Lepton Wu <lep...@chromium.org>
> > > > > > > ---
> > > > > > > src/gallium/drivers/virgl/virgl_resource.c | 1 +
> > > > > > > 1 file changed, 1 insertion(+)
> > > > > > >
> > > > > > > diff --git a/src/gallium/drivers/virgl/virgl_resource.c
> > > > > > > b/src/gallium/drivers/virgl/virgl_resource.c
> > > > > > > index c22a78a4731..909deb774c7 100644
> > > > > > > --- a/src/gallium/drivers/virgl/virgl_resource.c
> > > > > > > +++ b/src/gallium/drivers/virgl/virgl_resource.c
> > > > > > > @@ -515,6 +515,7 @@ static struct pipe_resource
> > > > > > > *virgl_resource_from_handle(struct pipe_screen *scre
> > > > > > > res->u.b = *templ;
> > > > > > > res->u.b.screen = &vs->base;
> > > > > > > pipe_reference_init(&res->u.b.reference, 1);
> > > > > > > + virgl_resource_layout(&res->u.b, &res->metadata);
> > > > > There was a similar MR for this
> > > > >
> > > > > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/965
> > > > >
> > > > > Can you add a check to make sure the stride is compatible?
> > > > I think this kind of check should be in "framework" side instead of
> > > > inside virgl driver.
> > > > The check what you are said is basically to check if stride info n
> > > > whandle is comptabile
> > > > with value in pipe_resource, I think if we need this check, we should
> > > > put it in dri2_allocate_textures
> > > > and dri2_create_image_from_winsys? and that should be another CL?
> > > The framework does not know the stride of a pipe resource.
> > >
> > > > >
> > > > > if (res->metadata->stride[0] != whandle->stride) reject the whandle;
> > > > >
> > > > > > >
> > > > > > > res->hw_res = vs->vws->resource_create_from_handle(vs->vws,
> > > > > > > whandle);
> > > > > > > if (!res->hw_res) {
> > > > > >
> > > > > > Whoops! Good catch, sorry for the mess!
> > > > > >
> > > > > > Reviewed-by: Erik Faye-Lund <erik.faye-l...@collabora.com>
> > > > > >
> > > > > > _______________________________________________
> > > > > > mesa-dev mailing list
> > > > > > mesa-dev@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
