[Mesa-dev] [PATCH v5 1/4] nouveau: split nouveau_vp3_bsp in begin/next/end
Julien Isorce
julien.isorce at gmail.com
Wed Dec 23 01:20:03 PST 2015
On 17 December 2015 at 04:02, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Wed, Dec 16, 2015 at 9:39 AM, Julien Isorce <j.isorce at samsung.com>
> wrote:
> > It allows to call nouveau_vp3_bsp_next multiple times
> > between one begin/end.
> >
> > It is required to support st/va.
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=89969
> >
> > Signed-off-by: Julien Isorce <j.isorce at samsung.com>
> > ---
> > src/gallium/drivers/nouveau/nouveau_vp3_video.h | 16 +++-
> > .../drivers/nouveau/nouveau_vp3_video_bsp.c | 96
> ++++++++++++++--------
> > src/gallium/drivers/nouveau/nv50/nv98_video_bsp.c | 5 +-
> > src/gallium/drivers/nouveau/nvc0/nvc0_video_bsp.c | 5 +-
> > 4 files changed, 78 insertions(+), 44 deletions(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/nouveau_vp3_video.h
> b/src/gallium/drivers/nouveau/nouveau_vp3_video.h
> > index 58df5ee..9db8c63 100644
> > --- a/src/gallium/drivers/nouveau/nouveau_vp3_video.h
> > +++ b/src/gallium/drivers/nouveau/nouveau_vp3_video.h
> > @@ -114,6 +114,10 @@ struct nouveau_vp3_decoder {
> > unsigned fence_seq, fw_sizes, last_frame_num, tmp_stride, ref_stride;
> >
> > unsigned bsp_idx, vp_idx, ppp_idx;
> > +
> > + /* End of the bsp bo where new data should be appended between one
> begin/end
> > + * frame. */
> > + char *bsp_ptr;
> > };
> >
> > struct comm {
> > @@ -208,11 +212,15 @@ nouveau_vp3_load_firmware(struct
> nouveau_vp3_decoder *dec,
> > enum pipe_video_profile profile,
> > unsigned chipset);
> >
> > +void
> > +nouveau_vp3_bsp_begin(struct nouveau_vp3_decoder *dec);
> > +
> > +void
> > +nouveau_vp3_bsp_next(struct nouveau_vp3_decoder *dec, unsigned
> num_buffers,
> > + const void *const *data, const unsigned
> *num_bytes);
> > +
> > uint32_t
> > -nouveau_vp3_bsp(struct nouveau_vp3_decoder *dec, union pipe_desc desc,
> > - struct nouveau_vp3_video_buffer *target,
> > - unsigned comm_seq, unsigned num_buffers,
> > - const void *const *data, const unsigned *num_bytes);
> > +nouveau_vp3_bsp_end(struct nouveau_vp3_decoder *dec, union pipe_desc
> desc);
> >
> > void
> > nouveau_vp3_vp_caps(struct nouveau_vp3_decoder *dec, union pipe_desc
> desc,
> > diff --git a/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
> b/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
> > index 692772e..2c15955 100644
> > --- a/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
> > +++ b/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
> > @@ -230,20 +230,61 @@ nouveau_vp3_fill_picparm_h264_bsp(struct
> nouveau_vp3_decoder *dec,
> > return caps | 3;
> > }
> >
> > +void
> > +nouveau_vp3_bsp_begin(struct nouveau_vp3_decoder *dec)
> > +{
> > + uint32_t comm_seq = dec->fence_seq;
> > + struct nouveau_bo *bsp_bo = dec->bsp_bo[comm_seq %
> NOUVEAU_VP3_VIDEO_QDEPTH];
> > + struct strparm_bsp *str_bsp = NULL;
> > +
> > + dec->bsp_ptr = bsp_bo->map;
> > +
> > + dec->bsp_ptr += 0x100;
> > +
> > + str_bsp = (struct strparm_bsp *)dec->bsp_ptr;
> > + memset(str_bsp, 0, 0x80);
> > + str_bsp->w0[0] = 16;
>
> This field is, ostensibly, the number of bytes in the buffer. This 16
> should get added when you actually add the end sequence. Here it
> should get initialized to 0.
>
All right I'll move that to _end. It will also simplify
nouveau_vp3_bsp_next.
>
> > + str_bsp->w1[0] = 0x1;
> > + dec->bsp_ptr += 0x100;
> > + /* Reserved for picparm_vp */
> > + dec->bsp_ptr += 0x300;
> > + /* Reserved for comm */
> > + #if !NOUVEAU_VP3_DEBUG_FENCE
>
> I'm surprised this compiles... I thought macro processor stuff had to
> be on column 0. Either way, nowhere do we put spaces before the #.
>
> git grep -P '^\s+#' -- *.c\*
>
> just a handful of results.
>
Ack :)
>
> > + memset(dec->bsp_ptr, 0, 0x200);
> > + #endif
> > + dec->bsp_ptr += 0x200;
> > +}
> > +
> > +void
> > +nouveau_vp3_bsp_next(struct nouveau_vp3_decoder *dec, unsigned
> num_buffers,
> > + const void *const *data, const unsigned *num_bytes)
> > +{
> > + uint32_t comm_seq = dec->fence_seq;
> > + struct nouveau_bo *bsp_bo = dec->bsp_bo[comm_seq %
> NOUVEAU_VP3_VIDEO_QDEPTH];
> > + char *bsp_origin = bsp_bo->map;
> > + struct strparm_bsp *str_bsp = NULL;
> > + int i = 0;
> > +
> > + bsp_origin += 0x100;
> > + str_bsp = (struct strparm_bsp *)bsp_origin;
>
> Seems easier to write this as (struct strparm_bsp *)(bsp_bo->map + 0x100)
>
> Technically void ptr arithmetic is "bad", but in practice it's pretty
> well defined, and nouveau is only built with well-behaved compilers.
>
Ack but I'll remove it since I'll update str_bsp->w0[0] in _end.
>
> > +
> > + for (i = 0; i < num_buffers; ++i) {
>
> assert that we don't overflow the bo... something like
>
> assert(bsp_bo->map + bsp_bo->size <= dec->bsp_ptr + num_bytes[i])
>
Ack
>
> > + memcpy(dec->bsp_ptr, data[i], num_bytes[i]);
> > + dec->bsp_ptr += num_bytes[i];
> > + str_bsp->w0[0] += num_bytes[i];
> > + }
> > +}
> > +
> > uint32_t
> > -nouveau_vp3_bsp(struct nouveau_vp3_decoder *dec, union pipe_desc desc,
> > - struct nouveau_vp3_video_buffer *target,
> > - unsigned comm_seq, unsigned num_buffers,
> > - const void *const *data, const unsigned *num_bytes)
> > +nouveau_vp3_bsp_end(struct nouveau_vp3_decoder *dec, union pipe_desc
> desc)
> > {
> > - enum pipe_video_format codec =
> u_reduce_video_profile(dec->base.profile);
> > + uint32_t comm_seq = dec->fence_seq;
> > struct nouveau_bo *bsp_bo = dec->bsp_bo[comm_seq %
> NOUVEAU_VP3_VIDEO_QDEPTH];
> > - char *bsp;
> > - uint32_t endmarker, caps;
> > - struct strparm_bsp *str_bsp;
> > - int i;
> > + enum pipe_video_format codec =
> u_reduce_video_profile(dec->base.profile);
> > + uint32_t caps = 0;
> > + uint32_t endmarker = 0;
> > + char *bsp = bsp_bo->map;
> >
> > - bsp = bsp_bo->map;
> > /*
> > * 0x000..0x100: picparm_bsp
> > * 0x200..0x500: picparm_vp
> > @@ -277,34 +318,17 @@ nouveau_vp3_bsp(struct nouveau_vp3_decoder *dec,
> union pipe_desc desc,
> > caps |= 1 << 17; // enable watchdog
> > caps |= 0 << 18; // do not report error to VP, so it can continue
> decoding what we have
> > caps |= 0 << 19; // if enabled, use crypto crap?
> > - bsp += 0x100;
> > -
> > - str_bsp = (struct strparm_bsp *)bsp;
> > - memset(str_bsp, 0, 0x80);
> > - str_bsp->w0[0] = 16;
> > - str_bsp->w1[0] = 0x1;
> > - bsp += 0x100;
> > - /* Reserved for picparm_vp */
> > - bsp += 0x300;
> > - /* Reserved for comm */
> > -#if !NOUVEAU_VP3_DEBUG_FENCE
> > - memset(bsp, 0, 0x200);
> > -#endif
> > - bsp += 0x200;
> > - for (i = 0; i < num_buffers; ++i) {
> > - memcpy(bsp, data[i], num_bytes[i]);
> > - bsp += num_bytes[i];
> > - str_bsp->w0[0] += num_bytes[i];
> > - }
> >
> > /* Append end sequence */
> > - *(uint32_t *)bsp = endmarker;
> > - bsp += 4;
> > - *(uint32_t *)bsp = 0x00000000;
> > - bsp += 4;
> > - *(uint32_t *)bsp = endmarker;
> > - bsp += 4;
> > - *(uint32_t *)bsp = 0x00000000;
> > + *(uint32_t *)dec->bsp_ptr = endmarker;
> > + dec->bsp_ptr += 4;
> > + *(uint32_t *)dec->bsp_ptr = 0x00000000;
> > + dec->bsp_ptr += 4;
> > + *(uint32_t *)dec->bsp_ptr = endmarker;
> > + dec->bsp_ptr += 4;
> > + *(uint32_t *)dec->bsp_ptr = 0x00000000;
>
> And then here also do w0[0] += 16
>
Ack
>
> > +
> > + dec->bsp_ptr = NULL;
> >
> > return caps;
> > }
> > diff --git a/src/gallium/drivers/nouveau/nv50/nv98_video_bsp.c
> b/src/gallium/drivers/nouveau/nv50/nv98_video_bsp.c
> > index dbde1bf..a0c36c3 100644
> > --- a/src/gallium/drivers/nouveau/nv50/nv98_video_bsp.c
> > +++ b/src/gallium/drivers/nouveau/nv50/nv98_video_bsp.c
> > @@ -106,8 +106,9 @@ nv98_decoder_bsp(struct nouveau_vp3_decoder *dec,
> union pipe_desc desc,
> > return -1;
> > }
> >
> > - caps = nouveau_vp3_bsp(dec, desc, target, comm_seq,
> > - num_buffers, data, num_bytes);
> > + nouveau_vp3_bsp_begin(dec);
> > + nouveau_vp3_bsp_next(dec, num_buffers, data, num_bytes);
> > + caps = nouveau_vp3_bsp_end(dec, desc);
> >
> > nouveau_vp3_vp_caps(dec, desc, target, comm_seq, vp_caps, is_ref,
> refs);
> >
> > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_video_bsp.c
> b/src/gallium/drivers/nouveau/nvc0/nvc0_video_bsp.c
> > index 4392f62..b0ee4a4 100644
> > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_video_bsp.c
> > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_video_bsp.c
> > @@ -114,8 +114,9 @@ nvc0_decoder_bsp(struct nouveau_vp3_decoder *dec,
> union pipe_desc desc,
> > return -1;
> > }
> >
> > - caps = nouveau_vp3_bsp(dec, desc, target, comm_seq,
> > - num_buffers, data, num_bytes);
> > + nouveau_vp3_bsp_begin(dec);
> > + nouveau_vp3_bsp_next(dec, num_buffers, data, num_bytes);
> > + caps = nouveau_vp3_bsp_end(dec, desc);
> >
> > nouveau_vp3_vp_caps(dec, desc, target, comm_seq, vp_caps, is_ref,
> refs);
> >
> > --
> > 1.9.1
> >
> _______________________________________________
> 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/20151223/a87cd1f7/attachment.html>
More information about the mesa-dev
mailing list