Re: [PATCH xf86-video-intel v6] sna: Added AYUV format support for textured and sprite video adapters.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Nov 08, 2018 at 05:17:13PM +0000, Chris Wilson wrote:
> Quoting Stanislav Lisovskiy (2018-11-02 10:06:03)
> > v2: Renamed DRM_FORMAT_XYUV to DRM_FORMAT_XYUV8888.
> >     Added comment about AYUV byte ordering in Gstreamer.
> > 
> > v3: Removed sna_composite_op flags related change to the separate patch.
> > 
> > v4: Fixed review comments, done code refactoring
> > 
> > v5: Fixed following review comments:
> >     - Fixed comment in shader code for ayuv kernel.
> >     - Fixed naming to VIDEO_AYUV_BT601/BT709 for ayuv kernels.
> >     - Removed duplicate gen9_kernel parameter, left from previous patches
> >     - Added colorspace handling for new AYUV kernel
> >     - Fixed naming of sna_copy_packed_data_ayuv to sna_copy_ayuv_data
> >     - Started using standard bswap_32 function for byte swapping in sna_copy_ayuv_data
> >     - Removed redundant code in sna_copy_ayuv_data so that it looks more neat
> >     - Fixed XVIMAGE_AYUV structure initialization to contain proper byte sequence for GST
> >     - Fixed bogus comment about subsampling for DRM_FORMAT_XYUV8888
> >     - Fixed AYUV advertisement for all platforms
> >     - Removed unnecessary RGB888 declaration.
> > 
> > v6:
> >     - Fixed surface format not to use alpha as supposed
> >     - Now doing byte swapping always during copy
> >     - Changed hack, required for GST to work to be at one place
> >     - Fixed invalid sampling values for XVIMAGE_AYUV
> >     - Fixed sprite format checking order and images_ayuv definition.
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> Ville, happy?

Almost.

> 
> > +       if (reverse_bytes) {

This bool bugs me, replied separately.

> > +               /*
> > +                * Have to reverse bytes order, because the only
> > +                * player which supports AYUV format currently is
> > +                * Gstreamer and it supports in bad way, even though
> > +                * spec says MSB:AYUV, we get the bytes opposite way.
> > +                */
> 
> This worries me. Is there no room for negotiation with Gstreamer so they
> use the format the kernel and HW expects. Though I presume they chose
> their layout for good reason (some TV probably expects it in host
> order.)

Either that or no one ever actually tested it.

One might hope that the component_order[] in the XvImage can be used to
tell the standard AYUV and gst AYUV apart. But probably no one even
looks at that thing. Also not sure how that would work w.r.t. the
fourcc if both needed to be supported.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx