[Mesa-dev] [PATCH 0/8] nouveau: add support for vaapi

[Julien Isorce]

Hi,

Thx for the comments. I forgot to mention that there is nothing really new
in these patches.
All the low level code relative to nouveau was already there.
The patches are almost "just" moving code in order to call nouveau_vp3_bsp
and nvc0_decoder_bsp
multiple times between each begin/end frame. But any refactoring is a
source of regressions so the "just" may be not that trivial :)

I made sure that vdpau was still working. Actually it was more than that,
without vdpau I think I would not have been able to do this refactoring. I
also compared that for one begin/end frame the nouveau_bo buffer contains
the same data for vdpau and vaapi. I dumped them for the same stream and
several seconds and there were all the same.

Would it be ok if I squash the patches like this (4 patches instead of 8):

*A: Common to nvc0 and nv98:*
nouveau: extract memcpy loop from nouveau_vp3_bsp
nouveau: remove nouveau_vp3_bsp to use begin/next/end

*B: Only for nvc0 but same split can be applied to nv98:*
+nvc0(-nouveau): split nvc0_decoder_bsp in begin/next/end
+nvc0(-nouveau): remove nvc0_decoder_bsp and use begin/next/end instead
+nvc0(-nouveau): preserve content buffer when calling nvc0_decoder_bsp_next
nvc0: implement pipe_video_codec::begin_frame/end_frame

*C: Common to nvc0 and nv98:*
nouveau: fix chunk decoding by updating number of slices

*D: Common to nvc0 and nv98:*
build: enable st/va with nouveau driver

I can still squash A and B, no pb. Let me know.
Also should I use "--in-reply-to" or should I submit a new patch set since
some will be squashed ?

Cheers
Julien


On 27 August 2015 at 22:36, Emil Velikov <emil.l.velikov at gmail.com> wrote:

> On 27 August 2015 at 18:35, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> > On Thu, Aug 27, 2015 at 1:11 PM, Emil Velikov <emil.l.velikov at gmail.com>
> wrote:
> >> HI Julien,
> >>
> >> On 27 August 2015 at 15:15, Julien Isorce <j.isorce at samsung.com> wrote:
> >>> Currently nouveau does not support chunk decoding
> >>> which is required to support st/va
> >>>
> >>> The following patches refactor nouveau_vp3_bsp
> >>> and nvc0_decoder_bsp in order to implement
> >>> pipe_video_codec::begin_frame/decode_bitstream/end_frame.
> >>> So that decode_bitstream can be call multiple times
> >>> between each begin/end.
> >>>
> >>> TODO: apply same logic for nv98 but I do not have the
> >>> material to test it.
> >>>
> >>> https://bugs.freedesktop.org/show_bug.cgi?id=89969
> >>>
> >>> Julien Isorce (8):
> >>>   nouveau: extract memcpy loop from nouveau_vp3_bsp
> >>>   nouveau: remove nouveau_vp3_bsp to use begin/next/end
> >>>   nouveau: split nvc0_decoder_bsp in begin/next/end
> >>>   nouveau: preserve content buffer when calling nvc0_decoder_bsp_next
> >>>   nouveau: remove nvc0_decoder_bsp and use begin/next/end instead
> >>>   nvc0: implement pipe_video_codec::begin_frame/end_frame
> >>>   nouveau: fix chunk decoding by updating number of slices
> >>>   build: enable st/va with nouveau driver
> >>>
> >> Glad to hear that you've got vaapi working. Did you check that vdpau
> >> did not regress ?
> >>
> >> I'm wondering if it's going to be better if you gradually re-factor
> >> the existing code. The current "1. add 'new' implementation (incl.
> >> variables we don't want/need until halfway through), 2. remove 'old'
> >> implementation" approach does not seem too appealing imho.
> >>
> >> Others might disagree with my suggestion, though :)
> >
> > I wholeheartedly agree... the current patch series makes it very
> > difficult to verify that you're doing stuff "right" since I have to
> > look at the "add" and "remove" patches at once. Also your "preserve
> > content buffer" thing should probably be folded into the previous
> > patch, since that's integral to what those functions do. I realize
> > that in the end it'll end up creating just one or two mega-patches,
> > but I'd rather have that than the functionality scattered all over.
> >
> Actually it seems that one can get away without big patches.
> Two begin/next/end combos = 6 patches, a cleanup/fix or two plus the
> enable patch.
>
> > You don't have to worry about nv98_video if you don't want to, I can
> > take care of fixing that up afterwards, but do try to leave as much
> > functionality in nouveau_vp3 as possible. (Perhaps you've done that, I
> > haven't looked in great detail yet.)
> >
> From a quick look all the nouveau_vp3 code is still there.
>
> -Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150901/c926ea65/attachment.html>