[Libva] [libva-intel-driver][PATCH] CSC: Remove average logic when saving NV12 surface on IVB+

Xiang, Haihao haihao.xiang at intel.com
Mon Apr 25 02:48:37 UTC 2016


> On 11 April 2016 at 09:40, Xiang, Haihao <haihao.xiang at intel.com>
> wrote:
> > 
> > > Hi Haihao,
> > > 
> > > On 7 April 2016 at 17:28, Xiang, Haihao <haihao.xiang at intel.com>
> > > wrote:
> > > > This fixes the issue mentioned in https://bugs.freedesktop.org/
> > > > show
> > > > _bug.cgi?id=94845 on
> > > > IVB+(except KBL)
> > > > 
> > > From a quick diff it seems that the post processing
> > > shaders/assembly
> > > is identical between gen7 and gen8, correct ?
> > 
> > There are a few differences between gen7 and gen8 shaders, so we
> > don't
> > want to mix up the shaders for gen7 and gen8.
> > 
> Imho that's not a reason to duplicate 90% of the shaders: from a
> quick
> look the diff is the following
> 
>  - whitespace and/or comments
>  - missing the odd <1>
>  - some strange ones like the following (no particular reason why one
> cannot use the same for both gens ?)
> 
> -       mac   (16) acc0.0<1>:f            fBUFFER_U(0,
> 0)<8;8,1>        -0.344f
> +       mac   (8) acc0.0<1>:f             fBUFFER_U(0,
> 0)<8;8,1>        -0.344f
> +       mac   (8) acc1.0<1>:f             fBUFFER_U(1,
> 0)<8;8,1>        -0.344f
> 
> 
>  - And the most significant one "Gen7 AVS WA Only for YUV packed
> surfaces, NV12 and Y-channel only for Planar surfaces."
> Seems to be an identical (?) hunk copied across 12 files.
> 
> So one can just factor the last one into a separate file for gen7
> (and
> ones that I might have missed), correct ? After all the beauty of
> good
> software design/engineering is the way we rework these to better
> reuse
> existing code ;-)

I agree with your point. However this patch mainly focuses on bug fix
instead of cleaning up code. If possible, could you help us to clean
these code ?

Thanks
Haihao

> 
> > 
> > > Obviously there are a few missing <1> (afaict they do not made
> > > any
> > > difference) and a few missing files for gen8 but everything else
> > > is
> > > just white space.
> > > As such shouldn't one consolidate things ? Earlier gen do share a
> > > fair
> > > bit, and if I have to wild guess I'd say things don't differ that
> > > much
> > > between them and gen7+.
> > 
> > Yes, adding <1> is just for readable. I will remove it from the
> > patch.
> > 
> Glad to hear. Thanks
> 
> Emil


More information about the Libva mailing list