[Libva] [PATCH ] Fix inconsistent surface dimension during allocating surface buffer object

[ykzhao]

On Tue, 2012-09-18 at 03:55 -0600, Xiang, Haihao wrote:
> 
> > -----Original Message-----
> > From: libva-bounces+haihao.xiang=intel.com at lists.freedesktop.org
> > [mailto:libva-bounces+haihao.xiang=intel.com at lists.freedesktop.org] On
> > Behalf Of ykzhao
> > Sent: Tuesday, September 18, 2012 9:34 AM
> > To: Gwenole Beauchesne
> > Cc: libva at lists.freedesktop.org
> > Subject: Re: [Libva] [PATCH ] Fix inconsistent surface dimension during
> > allocating surface buffer object
> > 
> > On Mon, 2012-09-17 at 21:59 -0600, Gwenole Beauchesne wrote:
> > > Hi Yakui,
> > >
> > > 2012/9/18  <yakui.zhao at intel.com>:
> > >
> > > > When one surface is created, the height/width is aligned to 16 pixels.
> > > > But when trying to allocate the buffer object for it, the width is
> > > > aligned to 128 and height is aligned to 32. If the surface is mapped
> > > > and accessed before allocating the buffer object, the incorrect
> > > > dimension is returned and and the incorrect content is written.
> > >
> > > What branch are you using? I think that was the case since the
> > > beginning, something was probably changed from then, because...
> > 
> > This is for the master branch.
> > And this issue doesn't exist on the staging branch as it will try to create the
> > buffer object when trying to create the surfaces. In such case the dimension
> > parameter is updated as expected. But on the master branch the allocation of
> > buffer object is deferred for the surfaces. So this issue appears.
> 
> We can cherry-pick the fix from the staging branch. If so, we can use the right width/height
> no matter the surface is tiled or not.
> 

Yes. This is one solution. I will look at the staging branch and add the
corresponding code on the master branch. 

> > 
> > >
> > > > @@ -506,8 +506,8 @@ i965_CreateSurfaces(VADriverContextP ctx,
> > > >          obj_surface->orig_width = width;
> > > >          obj_surface->orig_height = height;
> > > >
> > > > -        obj_surface->width = ALIGN(width, 16);
> > > > -        obj_surface->height = ALIGN(height, 16);
> > > > +        obj_surface->width = ALIGN(width, 128);
> > > > +        obj_surface->height = ALIGN(height, 32);
> > > >          obj_surface->flags = SURFACE_REFERENCED;
> > > >          obj_surface->fourcc = 0;
> > > >          obj_surface->bo = NULL;
> > >
> > > ... this actually is an SNB/IVB requirement IIRC. Oh, the change was
> > > indeed brought by f7eedfab. Could you please create a patch that
> > > re-instates f7eedfab hunks for obj_surface->{width,height}? i.e. with
> > > the SNB/IVB check, and mentioning this is a regression fix?
> > 
> > Yes. The issue is related with the commit id you mentioned. It is also ok to add
> > the corresponding hunks for SNB/IVB.
> > 
> > But I prefer to use the 128 alignment for width and 32 alignment for height
> > when creating the surfaces, which is the requirement for SNB/IVB and later
> > chips. Although it allocates more memory for the chip before SNB, it will break
> > nothing. Is this OK to you?
> > 
> > thanks
> > 
> > 
> > >
> > > Thanks,
> > > Gwenole.
> > 
> > 
> > _______________________________________________
> > Libva mailing list
> > Libva at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/libva