[Mesa-dev] [PATCH v2 1/4] nouveau: split nouveau_vp3_bsp in begin/next/end
Ilia Mirkin
imirkin at alum.mit.edu
Fri Sep 4 13:00:41 PDT 2015
On Fri, Sep 4, 2015 at 12:43 PM, 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 | 17 ++-
> .../drivers/nouveau/nouveau_vp3_video_bsp.c | 166 ++++++++++++---------
> src/gallium/drivers/nouveau/nv50/nv98_video_bsp.c | 5 +-
> src/gallium/drivers/nouveau/nvc0/nvc0_video_bsp.c | 5 +-
> 4 files changed, 115 insertions(+), 78 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nouveau_vp3_video.h b/src/gallium/drivers/nouveau/nouveau_vp3_video.h
> index 33e3bef..d7c2438 100644
> --- a/src/gallium/drivers/nouveau/nouveau_vp3_video.h
> +++ b/src/gallium/drivers/nouveau/nouveau_vp3_video.h
> @@ -114,6 +114,11 @@ struct nouveau_vp3_decoder {
> unsigned fence_seq, fw_sizes, last_frame_num, tmp_stride, ref_stride;
>
> unsigned bsp_idx, vp_idx, ppp_idx;
> +
> + // To manage chunk decoding.
Please don't use C++ comments in C code. [Looks like the old code was
breaking this rule too, but... not a good excuse.]
> + char *bsp_ptr;
This is more like the end of the bsp bo where new data should be
appended, yes? Also worth noting that this is only valid between begin
and end frame, since we normally switch bo's on frames, at which point
the pointer/etc will be all wrong.
> + unsigned bsp_size;
> + unsigned int nb_slices;
The purpose of these two fields is not immediately apparent to me.
Perhaps they should be added in a later patch?
> };
>
> struct comm {
> @@ -208,11 +213,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 6d968c1..70e88ba 100644
> --- a/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
> +++ b/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
> @@ -230,81 +230,107 @@ nouveau_vp3_fill_picparm_h264_bsp(struct nouveau_vp3_decoder *dec,
> return caps | 3;
> }
>
> -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)
> +void
> +nouveau_vp3_bsp_begin(struct nouveau_vp3_decoder *dec)
> {
> - enum pipe_video_format codec = u_reduce_video_profile(dec->base.profile);
> - 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;
> + uint32_t comm_seq = dec->fence_seq;
Looks like you set the indent to 4 spaces? Mesa uses 3-space indents.
Please fix all the patches, I believe they should become much shorter
and more readable as a result.
> + struct nouveau_bo *bsp_bo = dec->bsp_bo[comm_seq % NOUVEAU_VP3_VIDEO_QDEPTH];
> + struct strparm_bsp *str_bsp = NULL;
>
> - bsp = bsp_bo->map;
> - /*
> - * 0x000..0x100: picparm_bsp
> - * 0x200..0x500: picparm_vp
> - * 0x500..0x700: comm
> - * 0x700..onward: raw bitstream
> - */
> + dec->bsp_ptr = bsp_bo->map;
> + dec->bsp_size = NOUVEAU_VP3_BSP_RESERVED_SIZE;
> + dec->nb_slices = 0;
>
> - switch (codec){
> - case PIPE_VIDEO_FORMAT_MPEG12:
> - endmarker = 0xb7010000;
> - caps = nouveau_vp3_fill_picparm_mpeg12_bsp(dec, desc.mpeg12, bsp);
> - break;
> - case PIPE_VIDEO_FORMAT_MPEG4:
> - endmarker = 0xb1010000;
> - caps = nouveau_vp3_fill_picparm_mpeg4_bsp(dec, desc.mpeg4, bsp);
> - break;
> - case PIPE_VIDEO_FORMAT_VC1: {
> - endmarker = 0x0a010000;
> - caps = nouveau_vp3_fill_picparm_vc1_bsp(dec, desc.vc1, bsp);
> - break;
> - }
> - case PIPE_VIDEO_FORMAT_MPEG4_AVC: {
> - endmarker = 0x0b010000;
> - caps = nouveau_vp3_fill_picparm_h264_bsp(dec, desc.h264, bsp);
> - break;
> - }
> - default: assert(0); return -1;
> - }
> + dec->bsp_ptr += 0x100;
>
> - caps |= 0 << 16; // reset struct comm if flag is set
> - 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 *)dec->bsp_ptr;
> + memset(str_bsp, 0, 0x80);
> + str_bsp->w0[0] = 16;
> + 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
> + memset(dec->bsp_ptr, 0, 0x200);
> + #endif
> + dec->bsp_ptr += 0x200;
> +}
>
> - 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];
> - }
> +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;
> +
> + ++dec->nb_slices;
> +
> + bsp_origin += 0x100;
> + str_bsp = (struct strparm_bsp *)bsp_origin;
> +
> + for (i = 0; i < num_buffers; ++i) {
> + 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_end(struct nouveau_vp3_decoder *dec, union pipe_desc desc)
> +{
> + uint32_t comm_seq = dec->fence_seq;
> + struct nouveau_bo *bsp_bo = dec->bsp_bo[comm_seq % NOUVEAU_VP3_VIDEO_QDEPTH];
> + 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;
> +
> + /*
> + * 0x000..0x100: picparm_bsp
> + * 0x200..0x500: picparm_vp
> + * 0x500..0x700: comm
> + * 0x700..onward: raw bitstream
> + */
> +
> + switch (codec){
> + case PIPE_VIDEO_FORMAT_MPEG12:
> + endmarker = 0xb7010000;
> + caps = nouveau_vp3_fill_picparm_mpeg12_bsp(dec, desc.mpeg12, bsp);
> + break;
> + case PIPE_VIDEO_FORMAT_MPEG4:
> + endmarker = 0xb1010000;
> + caps = nouveau_vp3_fill_picparm_mpeg4_bsp(dec, desc.mpeg4, bsp);
> + break;
> + case PIPE_VIDEO_FORMAT_VC1: {
> + endmarker = 0x0a010000;
> + caps = nouveau_vp3_fill_picparm_vc1_bsp(dec, desc.vc1, bsp);
> + break;
> + }
> + case PIPE_VIDEO_FORMAT_MPEG4_AVC: {
> + endmarker = 0x0b010000;
> + caps = nouveau_vp3_fill_picparm_h264_bsp(dec, desc.h264, bsp);
> + break;
> + }
> + default: assert(0); return -1;
> + }
> +
> + caps |= 0 << 16; // reset struct comm if flag is set
> + 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?
>
> - /* 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;
> + /* Append end sequence */
> + *(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;
>
> - return caps;
> + 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 6058c22..ff38acf 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 9139bc1..385af20 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
More information about the mesa-dev
mailing list