Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 29 Mar 2022 11:13:21 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Nicolas Dufresne <nicolas.dufresne@collabora.com>
Cc:     Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        kernel@collabora.com, linux-media@vger.kernel.org,
        linux-rockchip@lists.infradead.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 19/24] media: rkvdec-h264: Add field decoding support
Message-ID: <20220329081321.GV3293@kadam>
References: <20220328195936.82552-1-nicolas.dufresne@collabora.com>
 <20220328195936.82552-20-nicolas.dufresne@collabora.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220328195936.82552-20-nicolas.dufresne@collabora.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?L9bB0qzLSULd4BIc05elqRhLA3yGhjFgIotr9l4EdwfOwm7puwt4dNkTA17t?=
 =?us-ascii?Q?2Xi5x6Y5TzF7K6llWghRVDmaEgbpmFdWHdTy2hsaNL5Ao2Uv2LdBP189YBnL?=
 =?us-ascii?Q?Y2zJklMocSnxkZvycpN/Cn13oUnfwjf4+Sywgc+VHUQCbgwmZvBQkMJhgnES?=
 =?us-ascii?Q?1enpMsdEP8vUNxhYOHXORsN+ors6aCdP8lBOFSdJuB1EHT77KYoZ8ou/68rK?=
 =?us-ascii?Q?zAoiCRlNp6IXd9RoD8RLkymYomjsMcJZMG6EU0ud/QJN3FvCG9MupJXFADj1?=
 =?us-ascii?Q?M5tvU1Ifr13F4TAeoct1mzoGD3zVbETDIHp5lHxB4q6jGlIo57Y07Qu95Eav?=
 =?us-ascii?Q?903EvPsfIXX6qtMUy1Fthu8yrjmcv+Hhlivc8xKIq9WeYE86r6GZgeCrxD3x?=
 =?us-ascii?Q?lNzi5SV2F1TJYR3pxKPScFppNdIggWEbgxccCb8R0MIChpj5wuKmrhb1RYKr?=
 =?us-ascii?Q?dSqgZ97J4PaWnvREP2RqCcJpusBoyabYp742zJ+mENiEJWDHyLKTVDXtwcF8?=
 =?us-ascii?Q?zNUsxrnVAZc6vwyWIvgoTkb5aT1uxRjIyuwNmEAaTT2q19XfURn61wD1uZAI?=
 =?us-ascii?Q?VPfvTFQZZSrifYOhRdrDwMWuL5SsKnQyrJTwPV23xSjMsXT+O6RUfXSjJ22y?=
 =?us-ascii?Q?vYu4AnA1IXI5F0Qxh5mIux7QtmTQiw23lXHI+YoUSMgATwMZ9EUhfczYvP23?=
 =?us-ascii?Q?s7BhONBX8etM2JkrGVo258rQ7v3w1tNJEbiEUH1R8xezzp+6ceThyQnPb3Uk?=
 =?us-ascii?Q?E0ol1fb/BKZeE8ziGIk8B6Toc2YzkGA2Z1WZdHKU4KEXPuLmG6pHiOZ0Gbc8?=
 =?us-ascii?Q?TnO3SFNVQyMkNbmkphsavOTAJAewO4RFwGCTQwrx6TrAFjzqDODxgt+Alu2Y?=
 =?us-ascii?Q?qG7Q6eUECEJ0979Ws6qPRG1n/txLx457QymT2AQkAtJWYYl4+dNO9N+pBQBr?=
 =?us-ascii?Q?4z7J8QnWlgFDBNuzOBPIUUSGFaeUjrg7Xj8iTuBkgrOlt9AYGoP8jIW9wUCi?=
 =?us-ascii?Q?SdRzGiWFT/1bDAaANWnGJR/d9Y0lJoHu6h/MDOAGVdzczoDuV25R8nUbHbfU?=
 =?us-ascii?Q?iPp8AW76E0lqiSw3BGrgh2r6Q27EBtOip4GQVUp+toYkKcTVGhTNOnGeeeN+?=
 =?us-ascii?Q?qGVWdU/DFiN45RVHMXNGqK/YW96FAa45pkBguugVD9xMt8b+xOpKxj0T5tHI?=
 =?us-ascii?Q?c/VBd4QPhS3poDWwGNM0EZP0uffD9GoBTUjHmrOo4jrT1rAN8eS8E+T+K+4S?=
 =?us-ascii?Q?4iqVHlb0klEIRAGYpXz/hdL7mvK9k60i7lAOnlfLjOFgpng/7DPZyWgRpO/C?=
 =?us-ascii?Q?t37fufkFmP39vaMiw59mbM/G9apIQQgmpmb+t38cVdZfY42z6dspVqvxMp5h?=
 =?us-ascii?Q?Xk6Z219Ct16Nd7XaN45Qi5slhEMuRFdSC2mPRSJNOak61AI9aj0RRlB0tCeP?=
 =?us-ascii?Q?IqGTB9snFVmP+2ZeuTdlZhD2Uzz1Ja4PTIjm7P9S0ZQXIQpKQqOiS1DNwl0W?=
 =?us-ascii?Q?L2p0s/e6+Uq437eYP7sKxsUOVuMyxfD+M0AH6cCIGqPK+6cFQqhLMTgcNi1S?=
 =?us-ascii?Q?s70sQEHzs59/PF6vF1ay29a7vjV9aKabYJMDecmFjRBJ5xr2zNfVWsoIz3/w?=
 =?us-ascii?Q?0/Vgwx0d0r21WJNWEW++b4WbNHWLpG3oFcMkfOe5RIabEhyjtt2OxhiGra4u?=
 =?us-ascii?Q?aHAe+dQECY+rjd0TaWnUYP/CG+4U1EbjYSC4z2AtjxYaf1iylq/u4M3JS96R?=
 =?us-ascii?Q?zmPSHVdE0Oyd1ol/jEa5Ic1NfTw43tk=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8c61ff2a-cde6-4058-6872-08da115c0acf
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Mar 2022 08:13:44.5655
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +1YedWUvbTkBIcShPob7pxrd+NGfkqKnzVetNrE940K/Sv/nnkTWU28HRNMjbmpfN8IM2Y/RGmxzoDvfsYV0rR7RQMPkuAhPnir8gWH9Bro=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1343
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10300 signatures=694973
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 malwarescore=0 spamscore=0
 phishscore=0 adultscore=0 mlxlogscore=999 suspectscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2203290048
X-Proofpoint-GUID: 98bHWn-LOBivGNocLYwv77zlpgQQo_fX
X-Proofpoint-ORIG-GUID: 98bHWn-LOBivGNocLYwv77zlpgQQo_fX
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Mar 28, 2022 at 03:59:31PM -0400, Nicolas Dufresne wrote:
> @@ -738,23 +735,26 @@ static void lookup_ref_buf_idx(struct rkvdec_ctx *ctx,
>  		struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
>  		int buf_idx = -1;
>  
> -		if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE)
> +		if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE) {
>  			buf_idx = vb2_find_timestamp(cap_q,
>  						     dpb[i].reference_ts, 0);
> +			if (buf_idx < 0)
> +				pr_debug("No buffer for reference_ts %llu",
> +					 dpb[i].reference_ts);

pr_debug() is too quiet.  Make it pr_err().  Set buf_idx to zero instead
leaving it as an error code.

> +		}
>  
>  		run->ref_buf_idx[i] = buf_idx;
>  	}
>  }
>  
>  static void assemble_hw_rps(struct rkvdec_ctx *ctx,
> +			    struct v4l2_h264_reflist_builder *builder,
>  			    struct rkvdec_h264_run *run)
>  {
>  	const struct v4l2_ctrl_h264_decode_params *dec_params = run->decode_params;
>  	const struct v4l2_h264_dpb_entry *dpb = dec_params->dpb;
>  	struct rkvdec_h264_ctx *h264_ctx = ctx->priv;
> -	const struct v4l2_ctrl_h264_sps *sps = run->sps;
>  	struct rkvdec_h264_priv_tbl *priv_tbl = h264_ctx->priv_tbl.cpu;
> -	u32 max_frame_num = 1 << (sps->log2_max_frame_num_minus4 + 4);
>  
>  	u32 *hw_rps = priv_tbl->rps;
>  	u32 i, j;
> @@ -772,37 +772,36 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
>  		if (!(dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
>  			continue;
>  
> -		if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM ||
> -		    dpb[i].frame_num <= dec_params->frame_num) {
> -			p[i] = dpb[i].frame_num;
> -			continue;
> -		}
> -
> -		p[i] = dpb[i].frame_num - max_frame_num;
> +		p[i] = builder->refs[i].frame_num;
>  	}
>  
>  	for (j = 0; j < RKVDEC_NUM_REFLIST; j++) {
> -		for (i = 0; i < h264_ctx->reflists.num_valid; i++) {
> -			u8 dpb_valid = run->ref_buf_idx[i] >= 0;
> -			u8 idx = 0;
> +		for (i = 0; i < builder->num_valid; i++) {
> +			struct v4l2_h264_reference *ref;
> +			u8 dpb_valid;
> +			u8 bottom;

These would be better as type bool.

regards,
dan carpenter

>  
>  			switch (j) {
>  			case 0:
> -				idx = h264_ctx->reflists.p[i].index;
> +				ref = &h264_ctx->reflists.p[i];
>  				break;
>  			case 1:
> -				idx = h264_ctx->reflists.b0[i].index;
> +				ref = &h264_ctx->reflists.b0[i];
>  				break;
>  			case 2:
> -				idx = h264_ctx->reflists.b1[i].index;
> +				ref = &h264_ctx->reflists.b1[i];
>  				break;
>  			}
>  
> -			if (idx >= ARRAY_SIZE(dec_params->dpb))
> +			if (WARN_ON(ref->index >= ARRAY_SIZE(dec_params->dpb)))
>  				continue;
>  
> +			dpb_valid = run->ref_buf_idx[ref->index] >= 0;
> +			bottom = ref->fields == V4L2_H264_BOTTOM_FIELD_REF;
> +
>  			set_ps_field(hw_rps, DPB_INFO(i, j),
> -				     idx | dpb_valid << 4);
> +				     ref->index | dpb_valid << 4);
> +			set_ps_field(hw_rps, BOTTOM_FLAG(i, j), bottom);
>  		}
>  	}
>  }