public inbox for linux-rockchip@lists.infradead.org
 help / color / mirror / Atom feed
From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
To: Hans Verkuil <hverkuil@xs4all.nl>,
	Tiffany Lin <tiffany.lin@mediatek.com>,
	 Andrew-CT Chen <andrew-ct.chen@mediatek.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Matthias Brugger <matthias.bgg@gmail.com>
Cc: kernel@collabora.com, linux-kernel@vger.kernel.org,
	 linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
	 linux-staging@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	 linux-mediatek@lists.infradead.org
Subject: Re: [PATCH v3 05/24] media: h264: Use v4l2_h264_reference for reflist
Date: Fri, 22 Apr 2022 10:44:15 -0400	[thread overview]
Message-ID: <0df420cc2cf2f564260010c78c8307f83833e685.camel@collabora.com> (raw)
In-Reply-To: <0717023f-dffd-5891-53fd-e866dc319265@xs4all.nl>

Le vendredi 22 avril 2022 à 08:59 +0200, Hans Verkuil a écrit :
> On 05/04/2022 22:44, Nicolas Dufresne wrote:
> > In preparation for adding field decoding support, convert the byte arrays
> > for reflist into array of struct v4l2_h264_reference. That struct will
> > allow us to mark which field of the reference picture is being referenced.
> > 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> > ---
> >  .../mediatek/vcodec/vdec/vdec_h264_req_if.c   | 17 +++-
> >  drivers/media/v4l2-core/v4l2-h264.c           | 33 ++++---
> >  .../staging/media/hantro/hantro_g1_h264_dec.c | 38 +++----
> >  drivers/staging/media/hantro/hantro_hw.h      |  6 +-
> >  .../media/hantro/rockchip_vpu2_hw_h264_dec.c  | 98 +++++++++----------
> >  drivers/staging/media/rkvdec/rkvdec-h264.c    | 12 +--
> >  include/media/v4l2-h264.h                     | 19 ++--
> >  7 files changed, 116 insertions(+), 107 deletions(-)
> > 
> > diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
> > index 43542de11e9c..72c599e05a47 100644
> > --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
> > +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
> > @@ -442,9 +442,16 @@ static void update_dpb(const struct v4l2_ctrl_h264_decode_params *dec_param,
> >  /*
> >   * The firmware expects unused reflist entries to have the value 0x20.
> >   */
> > -static void fixup_ref_list(u8 *ref_list, size_t num_valid)
> > +static void get_ref_list(u8 *ref_list, struct v4l2_h264_reflist_builder *b)
> >  {
> > -	memset(&ref_list[num_valid], 0x20, 32 - num_valid);
> > +	u32 i;
> > +
> > +	/* FIXME mark the reference parity */
> 
> It's not clear how serious this FIXME is. Either it should be fixed (here or
> later in the series), or explain why this isn't fixed yet. Presumably because
> it is not a problem now, but it might be later when some other feature is
> enabled.

I will improve this comment. I have spoken to Yunfei, and his firmware have no
interlacing support (despite the reference list size being 32 entry, so
interlaced ready). Though he said this is something he'd like to work in the
future, after this patchset have been merged of course. Sot he purpose of the
FIXME was really to ease locating were the field parity information is being
lost.

> 
> > +	for (i = 0; i < b->num_valid; i++)
> > +		ref_list[i] = b->index;
> > +
> > +	for (; i < 32; i++)
> > +		ref_list[i] = 0x20;
> 
> How about:
> 
> 	for (i = 0; i < 32; i++)
> 		ref_list[i] = i < b->num_valid ? b->index : 0x20;
> 
> Or:
> 
> 	memset(ref_list, b->index, b->num_valid);
> 	memset(ref_list + b->num_valid, 0x20, 32 - b->num_valid);
> 
> Either approach is better IMHO than using two for-loops.

Thanks, I have no strong opinion, though I think I'll use the second, with weak
rationale that is looks more like the original style.

> 
> Regards,
> 
> 	Hans
> 
> >  }
> >  
> >  static void get_vdec_decode_parameters(struct vdec_h264_slice_inst *inst)
> > @@ -478,9 +485,9 @@ static void get_vdec_decode_parameters(struct vdec_h264_slice_inst *inst)
> >  	v4l2_h264_build_p_ref_list(&reflist_builder, p0_reflist);
> >  	v4l2_h264_build_b_ref_lists(&reflist_builder, b0_reflist, b1_reflist);
> >  	/* Adapt the built lists to the firmware's expectations */
> > -	fixup_ref_list(p0_reflist, reflist_builder.num_valid);
> > -	fixup_ref_list(b0_reflist, reflist_builder.num_valid);
> > -	fixup_ref_list(b1_reflist, reflist_builder.num_valid);
> > +	get_ref_list(p0_reflist, reflist_builder);
> > +	get_ref_list(b0_reflist, reflist_builder);
> > +	get_ref_list(b1_reflist, reflist_builder);
> >  
> >  	memcpy(&inst->vsi_ctx.h264_slice_params, slice_param,
> >  	       sizeof(inst->vsi_ctx.h264_slice_params));
> > diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
> > index ac47519a9fbe..afbfcf78efe4 100644
> > --- a/drivers/media/v4l2-core/v4l2-h264.c
> > +++ b/drivers/media/v4l2-core/v4l2-h264.c
> > @@ -75,12 +75,12 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
> >  			pic_order_count = dpb[i].top_field_order_cnt;
> >  
> >  		b->refs[i].pic_order_count = pic_order_count;
> > -		b->unordered_reflist[b->num_valid] = i;
> > +		b->unordered_reflist[b->num_valid].index = i;
> >  		b->num_valid++;
> >  	}
> >  
> >  	for (i = b->num_valid; i < ARRAY_SIZE(b->unordered_reflist); i++)
> > -		b->unordered_reflist[i] = i;
> > +		b->unordered_reflist[i].index = i;
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_h264_init_reflist_builder);
> >  
> > @@ -90,8 +90,8 @@ static int v4l2_h264_p_ref_list_cmp(const void *ptra, const void *ptrb,
> >  	const struct v4l2_h264_reflist_builder *builder = data;
> >  	u8 idxa, idxb;
> >  
> > -	idxa = *((u8 *)ptra);
> > -	idxb = *((u8 *)ptrb);
> > +	idxa = ((struct v4l2_h264_reference *)ptra)->index;
> > +	idxb = ((struct v4l2_h264_reference *)ptrb)->index;
> >  
> >  	if (WARN_ON(idxa >= V4L2_H264_NUM_DPB_ENTRIES ||
> >  		    idxb >= V4L2_H264_NUM_DPB_ENTRIES))
> > @@ -125,8 +125,8 @@ static int v4l2_h264_b0_ref_list_cmp(const void *ptra, const void *ptrb,
> >  	s32 poca, pocb;
> >  	u8 idxa, idxb;
> >  
> > -	idxa = *((u8 *)ptra);
> > -	idxb = *((u8 *)ptrb);
> > +	idxa = ((struct v4l2_h264_reference *)ptra)->index;
> > +	idxb = ((struct v4l2_h264_reference *)ptrb)->index;
> >  
> >  	if (WARN_ON(idxa >= V4L2_H264_NUM_DPB_ENTRIES ||
> >  		    idxb >= V4L2_H264_NUM_DPB_ENTRIES))
> > @@ -170,8 +170,8 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
> >  	s32 poca, pocb;
> >  	u8 idxa, idxb;
> >  
> > -	idxa = *((u8 *)ptra);
> > -	idxb = *((u8 *)ptrb);
> > +	idxa = ((struct v4l2_h264_reference *)ptra)->index;
> > +	idxb = ((struct v4l2_h264_reference *)ptrb)->index;
> >  
> >  	if (WARN_ON(idxa >= V4L2_H264_NUM_DPB_ENTRIES ||
> >  		    idxb >= V4L2_H264_NUM_DPB_ENTRIES))
> > @@ -212,8 +212,8 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
> >   * v4l2_h264_build_p_ref_list() - Build the P reference list
> >   *
> >   * @builder: reference list builder context
> > - * @reflist: 16-bytes array used to store the P reference list. Each entry
> > - *	     is an index in the DPB
> > + * @reflist: 16 sized array used to store the P reference list. Each entry
> > + *	     is a v4l2_h264_reference structure
> >   *
> >   * This functions builds the P reference lists. This procedure is describe in
> >   * section '8.2.4 Decoding process for reference picture lists construction'
> > @@ -222,7 +222,7 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
> >   */
> >  void
> >  v4l2_h264_build_p_ref_list(const struct v4l2_h264_reflist_builder *builder,
> > -			   u8 *reflist)
> > +			   struct v4l2_h264_reference *reflist)
> >  {
> >  	memcpy(reflist, builder->unordered_reflist,
> >  	       sizeof(builder->unordered_reflist[0]) * builder->num_valid);
> > @@ -235,10 +235,10 @@ EXPORT_SYMBOL_GPL(v4l2_h264_build_p_ref_list);
> >   * v4l2_h264_build_b_ref_lists() - Build the B0/B1 reference lists
> >   *
> >   * @builder: reference list builder context
> > - * @b0_reflist: 16-bytes array used to store the B0 reference list. Each entry
> > - *		is an index in the DPB
> > - * @b1_reflist: 16-bytes array used to store the B1 reference list. Each entry
> > - *		is an index in the DPB
> > + * @b0_reflist: 16 sized array used to store the B0 reference list. Each entry
> > + *		is a v4l2_h264_reference structure
> > + * @b1_reflist: 16 sized array used to store the B1 reference list. Each entry
> > + *		is a v4l2_h264_reference structure
> >   *
> >   * This functions builds the B0/B1 reference lists. This procedure is described
> >   * in section '8.2.4 Decoding process for reference picture lists construction'
> > @@ -247,7 +247,8 @@ EXPORT_SYMBOL_GPL(v4l2_h264_build_p_ref_list);
> >   */
> >  void
> >  v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
> > -			    u8 *b0_reflist, u8 *b1_reflist)
> > +			    struct v4l2_h264_reference *b0_reflist,
> > +			    struct v4l2_h264_reference *b1_reflist)
> >  {
> >  	memcpy(b0_reflist, builder->unordered_reflist,
> >  	       sizeof(builder->unordered_reflist[0]) * builder->num_valid);
> > diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > index f49dbfb8a843..9de7f05eff2a 100644
> > --- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > +++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > @@ -126,7 +126,7 @@ static void set_params(struct hantro_ctx *ctx, struct vb2_v4l2_buffer *src_buf)
> >  
> >  static void set_ref(struct hantro_ctx *ctx)
> >  {
> > -	const u8 *b0_reflist, *b1_reflist, *p_reflist;
> > +	const struct v4l2_h264_reference *b0_reflist, *b1_reflist, *p_reflist;
> >  	struct hantro_dev *vpu = ctx->dev;
> >  	int reg_num;
> >  	u32 reg;
> > @@ -157,12 +157,12 @@ static void set_ref(struct hantro_ctx *ctx)
> >  	 */
> >  	reg_num = 0;
> >  	for (i = 0; i < 15; i += 3) {
> > -		reg = G1_REG_BD_REF_PIC_BINIT_RLIST_F0(b0_reflist[i]) |
> > -		      G1_REG_BD_REF_PIC_BINIT_RLIST_F1(b0_reflist[i + 1]) |
> > -		      G1_REG_BD_REF_PIC_BINIT_RLIST_F2(b0_reflist[i + 2]) |
> > -		      G1_REG_BD_REF_PIC_BINIT_RLIST_B0(b1_reflist[i]) |
> > -		      G1_REG_BD_REF_PIC_BINIT_RLIST_B1(b1_reflist[i + 1]) |
> > -		      G1_REG_BD_REF_PIC_BINIT_RLIST_B2(b1_reflist[i + 2]);
> > +		reg = G1_REG_BD_REF_PIC_BINIT_RLIST_F0(b0_reflist[i].index) |
> > +		      G1_REG_BD_REF_PIC_BINIT_RLIST_F1(b0_reflist[i + 1].index) |
> > +		      G1_REG_BD_REF_PIC_BINIT_RLIST_F2(b0_reflist[i + 2].index) |
> > +		      G1_REG_BD_REF_PIC_BINIT_RLIST_B0(b1_reflist[i].index) |
> > +		      G1_REG_BD_REF_PIC_BINIT_RLIST_B1(b1_reflist[i + 1].index) |
> > +		      G1_REG_BD_REF_PIC_BINIT_RLIST_B2(b1_reflist[i + 2].index);
> >  		vdpu_write_relaxed(vpu, reg, G1_REG_BD_REF_PIC(reg_num++));
> >  	}
> >  
> > @@ -171,12 +171,12 @@ static void set_ref(struct hantro_ctx *ctx)
> >  	 * of forward and backward reference picture lists and first 4 entries
> >  	 * of P forward picture list.
> >  	 */
> > -	reg = G1_REG_BD_P_REF_PIC_BINIT_RLIST_F15(b0_reflist[15]) |
> > -	      G1_REG_BD_P_REF_PIC_BINIT_RLIST_B15(b1_reflist[15]) |
> > -	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(p_reflist[0]) |
> > -	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F1(p_reflist[1]) |
> > -	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F2(p_reflist[2]) |
> > -	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F3(p_reflist[3]);
> > +	reg = G1_REG_BD_P_REF_PIC_BINIT_RLIST_F15(b0_reflist[15].index) |
> > +	      G1_REG_BD_P_REF_PIC_BINIT_RLIST_B15(b1_reflist[15].index) |
> > +	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(p_reflist[0].index) |
> > +	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F1(p_reflist[1].index) |
> > +	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F2(p_reflist[2].index) |
> > +	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F3(p_reflist[3].index);
> >  	vdpu_write_relaxed(vpu, reg, G1_REG_BD_P_REF_PIC);
> >  
> >  	/*
> > @@ -185,12 +185,12 @@ static void set_ref(struct hantro_ctx *ctx)
> >  	 */
> >  	reg_num = 0;
> >  	for (i = 4; i < HANTRO_H264_DPB_SIZE; i += 6) {
> > -		reg = G1_REG_FWD_PIC_PINIT_RLIST_F0(p_reflist[i]) |
> > -		      G1_REG_FWD_PIC_PINIT_RLIST_F1(p_reflist[i + 1]) |
> > -		      G1_REG_FWD_PIC_PINIT_RLIST_F2(p_reflist[i + 2]) |
> > -		      G1_REG_FWD_PIC_PINIT_RLIST_F3(p_reflist[i + 3]) |
> > -		      G1_REG_FWD_PIC_PINIT_RLIST_F4(p_reflist[i + 4]) |
> > -		      G1_REG_FWD_PIC_PINIT_RLIST_F5(p_reflist[i + 5]);
> > +		reg = G1_REG_FWD_PIC_PINIT_RLIST_F0(p_reflist[i].index) |
> > +		      G1_REG_FWD_PIC_PINIT_RLIST_F1(p_reflist[i + 1].index) |
> > +		      G1_REG_FWD_PIC_PINIT_RLIST_F2(p_reflist[i + 2].index) |
> > +		      G1_REG_FWD_PIC_PINIT_RLIST_F3(p_reflist[i + 3].index) |
> > +		      G1_REG_FWD_PIC_PINIT_RLIST_F4(p_reflist[i + 4].index) |
> > +		      G1_REG_FWD_PIC_PINIT_RLIST_F5(p_reflist[i + 5].index);
> >  		vdpu_write_relaxed(vpu, reg, G1_REG_FWD_PIC(reg_num++));
> >  	}
> >  
> > diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> > index ed018e293ba0..2bc6b8f088f5 100644
> > --- a/drivers/staging/media/hantro/hantro_hw.h
> > +++ b/drivers/staging/media/hantro/hantro_hw.h
> > @@ -69,9 +69,9 @@ struct hantro_h264_dec_ctrls {
> >   * @b1:		B1 reflist
> >   */
> >  struct hantro_h264_dec_reflists {
> > -	u8 p[HANTRO_H264_DPB_SIZE];
> > -	u8 b0[HANTRO_H264_DPB_SIZE];
> > -	u8 b1[HANTRO_H264_DPB_SIZE];
> > +	struct v4l2_h264_reference p[HANTRO_H264_DPB_SIZE];
> > +	struct v4l2_h264_reference b0[HANTRO_H264_DPB_SIZE];
> > +	struct v4l2_h264_reference b1[HANTRO_H264_DPB_SIZE];
> >  };
> >  
> >  /**
> > diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c b/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c
> > index 64a6330475eb..46c1a83bcc4e 100644
> > --- a/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c
> > +++ b/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c
> > @@ -298,7 +298,7 @@ static void set_params(struct hantro_ctx *ctx, struct vb2_v4l2_buffer *src_buf)
> >  
> >  static void set_ref(struct hantro_ctx *ctx)
> >  {
> > -	const u8 *b0_reflist, *b1_reflist, *p_reflist;
> > +	const struct v4l2_h264_reference *b0_reflist, *b1_reflist, *p_reflist;
> >  	struct hantro_dev *vpu = ctx->dev;
> >  	u32 reg;
> >  	int i;
> > @@ -307,20 +307,20 @@ static void set_ref(struct hantro_ctx *ctx)
> >  	b1_reflist = ctx->h264_dec.reflists.b1;
> >  	p_reflist = ctx->h264_dec.reflists.p;
> >  
> > -	reg = VDPU_REG_PINIT_RLIST_F9(p_reflist[9]) |
> > -	      VDPU_REG_PINIT_RLIST_F8(p_reflist[8]) |
> > -	      VDPU_REG_PINIT_RLIST_F7(p_reflist[7]) |
> > -	      VDPU_REG_PINIT_RLIST_F6(p_reflist[6]) |
> > -	      VDPU_REG_PINIT_RLIST_F5(p_reflist[5]) |
> > -	      VDPU_REG_PINIT_RLIST_F4(p_reflist[4]);
> > +	reg = VDPU_REG_PINIT_RLIST_F9(p_reflist[9].index) |
> > +	      VDPU_REG_PINIT_RLIST_F8(p_reflist[8].index) |
> > +	      VDPU_REG_PINIT_RLIST_F7(p_reflist[7].index) |
> > +	      VDPU_REG_PINIT_RLIST_F6(p_reflist[6].index) |
> > +	      VDPU_REG_PINIT_RLIST_F5(p_reflist[5].index) |
> > +	      VDPU_REG_PINIT_RLIST_F4(p_reflist[4].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(74));
> >  
> > -	reg = VDPU_REG_PINIT_RLIST_F15(p_reflist[15]) |
> > -	      VDPU_REG_PINIT_RLIST_F14(p_reflist[14]) |
> > -	      VDPU_REG_PINIT_RLIST_F13(p_reflist[13]) |
> > -	      VDPU_REG_PINIT_RLIST_F12(p_reflist[12]) |
> > -	      VDPU_REG_PINIT_RLIST_F11(p_reflist[11]) |
> > -	      VDPU_REG_PINIT_RLIST_F10(p_reflist[10]);
> > +	reg = VDPU_REG_PINIT_RLIST_F15(p_reflist[15].index) |
> > +	      VDPU_REG_PINIT_RLIST_F14(p_reflist[14].index) |
> > +	      VDPU_REG_PINIT_RLIST_F13(p_reflist[13].index) |
> > +	      VDPU_REG_PINIT_RLIST_F12(p_reflist[12].index) |
> > +	      VDPU_REG_PINIT_RLIST_F11(p_reflist[11].index) |
> > +	      VDPU_REG_PINIT_RLIST_F10(p_reflist[10].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(75));
> >  
> >  	reg = VDPU_REG_REFER1_NBR(hantro_h264_get_ref_nbr(ctx, 1)) |
> > @@ -355,54 +355,54 @@ static void set_ref(struct hantro_ctx *ctx)
> >  	      VDPU_REG_REFER14_NBR(hantro_h264_get_ref_nbr(ctx, 14));
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(83));
> >  
> > -	reg = VDPU_REG_BINIT_RLIST_F5(b0_reflist[5]) |
> > -	      VDPU_REG_BINIT_RLIST_F4(b0_reflist[4]) |
> > -	      VDPU_REG_BINIT_RLIST_F3(b0_reflist[3]) |
> > -	      VDPU_REG_BINIT_RLIST_F2(b0_reflist[2]) |
> > -	      VDPU_REG_BINIT_RLIST_F1(b0_reflist[1]) |
> > -	      VDPU_REG_BINIT_RLIST_F0(b0_reflist[0]);
> > +	reg = VDPU_REG_BINIT_RLIST_F5(b0_reflist[5].index) |
> > +	      VDPU_REG_BINIT_RLIST_F4(b0_reflist[4].index) |
> > +	      VDPU_REG_BINIT_RLIST_F3(b0_reflist[3].index) |
> > +	      VDPU_REG_BINIT_RLIST_F2(b0_reflist[2].index) |
> > +	      VDPU_REG_BINIT_RLIST_F1(b0_reflist[1].index) |
> > +	      VDPU_REG_BINIT_RLIST_F0(b0_reflist[0].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(100));
> >  
> > -	reg = VDPU_REG_BINIT_RLIST_F11(b0_reflist[11]) |
> > -	      VDPU_REG_BINIT_RLIST_F10(b0_reflist[10]) |
> > -	      VDPU_REG_BINIT_RLIST_F9(b0_reflist[9]) |
> > -	      VDPU_REG_BINIT_RLIST_F8(b0_reflist[8]) |
> > -	      VDPU_REG_BINIT_RLIST_F7(b0_reflist[7]) |
> > -	      VDPU_REG_BINIT_RLIST_F6(b0_reflist[6]);
> > +	reg = VDPU_REG_BINIT_RLIST_F11(b0_reflist[11].index) |
> > +	      VDPU_REG_BINIT_RLIST_F10(b0_reflist[10].index) |
> > +	      VDPU_REG_BINIT_RLIST_F9(b0_reflist[9].index) |
> > +	      VDPU_REG_BINIT_RLIST_F8(b0_reflist[8].index) |
> > +	      VDPU_REG_BINIT_RLIST_F7(b0_reflist[7].index) |
> > +	      VDPU_REG_BINIT_RLIST_F6(b0_reflist[6].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(101));
> >  
> > -	reg = VDPU_REG_BINIT_RLIST_F15(b0_reflist[15]) |
> > -	      VDPU_REG_BINIT_RLIST_F14(b0_reflist[14]) |
> > -	      VDPU_REG_BINIT_RLIST_F13(b0_reflist[13]) |
> > -	      VDPU_REG_BINIT_RLIST_F12(b0_reflist[12]);
> > +	reg = VDPU_REG_BINIT_RLIST_F15(b0_reflist[15].index) |
> > +	      VDPU_REG_BINIT_RLIST_F14(b0_reflist[14].index) |
> > +	      VDPU_REG_BINIT_RLIST_F13(b0_reflist[13].index) |
> > +	      VDPU_REG_BINIT_RLIST_F12(b0_reflist[12].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(102));
> >  
> > -	reg = VDPU_REG_BINIT_RLIST_B5(b1_reflist[5]) |
> > -	      VDPU_REG_BINIT_RLIST_B4(b1_reflist[4]) |
> > -	      VDPU_REG_BINIT_RLIST_B3(b1_reflist[3]) |
> > -	      VDPU_REG_BINIT_RLIST_B2(b1_reflist[2]) |
> > -	      VDPU_REG_BINIT_RLIST_B1(b1_reflist[1]) |
> > -	      VDPU_REG_BINIT_RLIST_B0(b1_reflist[0]);
> > +	reg = VDPU_REG_BINIT_RLIST_B5(b1_reflist[5].index) |
> > +	      VDPU_REG_BINIT_RLIST_B4(b1_reflist[4].index) |
> > +	      VDPU_REG_BINIT_RLIST_B3(b1_reflist[3].index) |
> > +	      VDPU_REG_BINIT_RLIST_B2(b1_reflist[2].index) |
> > +	      VDPU_REG_BINIT_RLIST_B1(b1_reflist[1].index) |
> > +	      VDPU_REG_BINIT_RLIST_B0(b1_reflist[0].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(103));
> >  
> > -	reg = VDPU_REG_BINIT_RLIST_B11(b1_reflist[11]) |
> > -	      VDPU_REG_BINIT_RLIST_B10(b1_reflist[10]) |
> > -	      VDPU_REG_BINIT_RLIST_B9(b1_reflist[9]) |
> > -	      VDPU_REG_BINIT_RLIST_B8(b1_reflist[8]) |
> > -	      VDPU_REG_BINIT_RLIST_B7(b1_reflist[7]) |
> > -	      VDPU_REG_BINIT_RLIST_B6(b1_reflist[6]);
> > +	reg = VDPU_REG_BINIT_RLIST_B11(b1_reflist[11].index) |
> > +	      VDPU_REG_BINIT_RLIST_B10(b1_reflist[10].index) |
> > +	      VDPU_REG_BINIT_RLIST_B9(b1_reflist[9].index) |
> > +	      VDPU_REG_BINIT_RLIST_B8(b1_reflist[8].index) |
> > +	      VDPU_REG_BINIT_RLIST_B7(b1_reflist[7].index) |
> > +	      VDPU_REG_BINIT_RLIST_B6(b1_reflist[6].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(104));
> >  
> > -	reg = VDPU_REG_BINIT_RLIST_B15(b1_reflist[15]) |
> > -	      VDPU_REG_BINIT_RLIST_B14(b1_reflist[14]) |
> > -	      VDPU_REG_BINIT_RLIST_B13(b1_reflist[13]) |
> > -	      VDPU_REG_BINIT_RLIST_B12(b1_reflist[12]);
> > +	reg = VDPU_REG_BINIT_RLIST_B15(b1_reflist[15].index) |
> > +	      VDPU_REG_BINIT_RLIST_B14(b1_reflist[14].index) |
> > +	      VDPU_REG_BINIT_RLIST_B13(b1_reflist[13].index) |
> > +	      VDPU_REG_BINIT_RLIST_B12(b1_reflist[12].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(105));
> >  
> > -	reg = VDPU_REG_PINIT_RLIST_F3(p_reflist[3]) |
> > -	      VDPU_REG_PINIT_RLIST_F2(p_reflist[2]) |
> > -	      VDPU_REG_PINIT_RLIST_F1(p_reflist[1]) |
> > -	      VDPU_REG_PINIT_RLIST_F0(p_reflist[0]);
> > +	reg = VDPU_REG_PINIT_RLIST_F3(p_reflist[3].index) |
> > +	      VDPU_REG_PINIT_RLIST_F2(p_reflist[2].index) |
> > +	      VDPU_REG_PINIT_RLIST_F1(p_reflist[1].index) |
> > +	      VDPU_REG_PINIT_RLIST_F0(p_reflist[0].index);
> >  	vdpu_write_relaxed(vpu, reg, VDPU_SWREG(106));
> >  
> >  	reg = VDPU_REG_REFER_LTERM_E(ctx->h264_dec.dpb_longterm);
> > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > index 951e19231da2..3c7f3d87fab4 100644
> > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > @@ -100,9 +100,9 @@ struct rkvdec_h264_priv_tbl {
> >  #define RKVDEC_H264_DPB_SIZE 16
> >  
> >  struct rkvdec_h264_reflists {
> > -	u8 p[RKVDEC_H264_DPB_SIZE];
> > -	u8 b0[RKVDEC_H264_DPB_SIZE];
> > -	u8 b1[RKVDEC_H264_DPB_SIZE];
> > +	struct v4l2_h264_reference p[RKVDEC_H264_DPB_SIZE];
> > +	struct v4l2_h264_reference b0[RKVDEC_H264_DPB_SIZE];
> > +	struct v4l2_h264_reference b1[RKVDEC_H264_DPB_SIZE];
> >  	u8 num_valid;
> >  };
> >  
> > @@ -767,13 +767,13 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
> >  
> >  			switch (j) {
> >  			case 0:
> > -				idx = h264_ctx->reflists.p[i];
> > +				idx = h264_ctx->reflists.p[i].index;
> >  				break;
> >  			case 1:
> > -				idx = h264_ctx->reflists.b0[i];
> > +				idx = h264_ctx->reflists.b0[i].index;
> >  				break;
> >  			case 2:
> > -				idx = h264_ctx->reflists.b1[i];
> > +				idx = h264_ctx->reflists.b1[i].index;
> >  				break;
> >  			}
> >  
> > diff --git a/include/media/v4l2-h264.h b/include/media/v4l2-h264.h
> > index 4b1c71c935e0..ef9a894e3c32 100644
> > --- a/include/media/v4l2-h264.h
> > +++ b/include/media/v4l2-h264.h
> > @@ -37,7 +37,7 @@ struct v4l2_h264_reflist_builder {
> >  		u16 longterm : 1;
> >  	} refs[V4L2_H264_NUM_DPB_ENTRIES];
> >  	s32 cur_pic_order_count;
> > -	u8 unordered_reflist[V4L2_H264_NUM_DPB_ENTRIES];
> > +	struct v4l2_h264_reference unordered_reflist[V4L2_H264_NUM_DPB_ENTRIES];
> >  	u8 num_valid;
> >  };
> >  
> > @@ -51,10 +51,10 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
> >   * v4l2_h264_build_b_ref_lists() - Build the B0/B1 reference lists
> >   *
> >   * @builder: reference list builder context
> > - * @b0_reflist: 16-bytes array used to store the B0 reference list. Each entry
> > - *		is an index in the DPB
> > - * @b1_reflist: 16-bytes array used to store the B1 reference list. Each entry
> > - *		is an index in the DPB
> > + * @b0_reflist: 16 sized array used to store the B0 reference list. Each entry
> > + *		is a v4l2_h264_reference structure
> > + * @b1_reflist: 16 sized array used to store the B1 reference list. Each entry
> > + *		is a v4l2_h264_reference structure
> >   *
> >   * This functions builds the B0/B1 reference lists. This procedure is described
> >   * in section '8.2.4 Decoding process for reference picture lists construction'
> > @@ -63,14 +63,15 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
> >   */
> >  void
> >  v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
> > -			    u8 *b0_reflist, u8 *b1_reflist);
> > +			    struct v4l2_h264_reference *b0_reflist,
> > +			    struct v4l2_h264_reference *b1_reflist);
> >  
> >  /**
> >   * v4l2_h264_build_p_ref_list() - Build the P reference list
> >   *
> >   * @builder: reference list builder context
> > - * @reflist: 16-bytes array used to store the P reference list. Each entry
> > - *	     is an index in the DPB
> > + * @reflist: 16 sized array used to store the P reference list. Each entry
> > + *	     is a v4l2_h264_reference structure
> >   *
> >   * This functions builds the P reference lists. This procedure is describe in
> >   * section '8.2.4 Decoding process for reference picture lists construction'
> > @@ -79,6 +80,6 @@ v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
> >   */
> >  void
> >  v4l2_h264_build_p_ref_list(const struct v4l2_h264_reflist_builder *builder,
> > -			   u8 *reflist);
> > +			   struct v4l2_h264_reference *reflist);
> >  
> >  #endif /* _MEDIA_V4L2_H264_H */
> 


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  reply	other threads:[~2022-04-22 14:44 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20220405204426.259074-1-nicolas.dufresne@collabora.com>
2022-04-05 20:44 ` [PATCH v3 05/24] media: h264: Use v4l2_h264_reference for reflist Nicolas Dufresne
2022-04-22  5:42   ` Hans Verkuil
2022-04-22 15:23     ` Nicolas Dufresne
2022-04-22  6:59   ` Hans Verkuil
2022-04-22 14:44     ` Nicolas Dufresne [this message]
2022-04-05 20:44 ` [PATCH v3 06/24] media: h264: Increase reference lists size to 32 Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 12/24] media: rkvdec: Stop overclocking the decoder Nicolas Dufresne
2022-04-22  7:33   ` Hans Verkuil
2022-04-05 20:44 ` [PATCH v3 13/24] media: rkvdec: h264: Fix dpb_valid implementation Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 14/24] media: rkvdec: h264: Fix bit depth wrap in pps packet Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 15/24] media: rkvdec: Move H264 SPS validation in rkvdec-h264 Nicolas Dufresne
2022-04-06  5:17   ` Ezequiel Garcia
2022-04-22  7:36   ` Hans Verkuil
2022-04-05 20:44 ` [PATCH v3 16/24] media: rkvdec: h264: Validate and use pic width and height in mbs Nicolas Dufresne
2022-04-06 13:10   ` Nicolas Dufresne
2022-04-06 13:12   ` Ezequiel Garcia
2022-04-22  7:39   ` Hans Verkuil
2022-04-05 20:44 ` [PATCH v3 17/24] media: rkvdec: h264: Fix reference frame_num wrap for second field Nicolas Dufresne
2022-04-22  7:43   ` Hans Verkuil
2022-04-25 18:55     ` Nicolas Dufresne
2022-04-25 19:00       ` Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 18/24] media: rkvdec: Ensure decoded resolution fit coded resolution Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 19/24] media: rkvdec-h264: Add field decoding support Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 20/24] media: rkvdec: Enable capture buffer holding for H264 Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 21/24] media: hantro: Stop using H.264 parameter pic_num Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 22/24] media: hantro: h264: Make dpb entry management more robust Nicolas Dufresne
2022-04-05 20:44 ` [PATCH v3 23/24] media: hantro: Add H.264 field decoding support Nicolas Dufresne
2022-04-22  7:49   ` Hans Verkuil
2022-04-05 20:44 ` [PATCH v3 24/24] media: hantro: Enable HOLD_CAPTURE_BUF for H.264 Nicolas Dufresne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0df420cc2cf2f564260010c78c8307f83833e685.camel@collabora.com \
    --to=nicolas.dufresne@collabora.com \
    --cc=andrew-ct.chen@mediatek.com \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=gregkh@linuxfoundation.org \
    --cc=heiko@sntech.de \
    --cc=hverkuil@xs4all.nl \
    --cc=kernel@collabora.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=matthias.bgg@gmail.com \
    --cc=mchehab@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=tiffany.lin@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox