From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 131E8C433DB for ; Mon, 22 Feb 2021 20:17:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CC85564E07 for ; Mon, 22 Feb 2021 20:17:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233405AbhBVURC (ORCPT ); Mon, 22 Feb 2021 15:17:02 -0500 Received: from bhuna.collabora.co.uk ([46.235.227.227]:33426 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229996AbhBVUQ6 (ORCPT ); Mon, 22 Feb 2021 15:16:58 -0500 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nicolas) with ESMTPSA id 72CF91F44ADB Message-ID: Subject: Re: [PATCH v2 4/9] media: uapi: Add a control for HANTRO driver From: Nicolas Dufresne To: Benjamin Gaignard , John Cox Cc: ezequiel@collabora.com, p.zabel@pengutronix.de, mchehab@kernel.org, robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, gregkh@linuxfoundation.org, mripard@kernel.org, paul.kocialkowski@bootlin.com, wens@csie.org, jernej.skrabec@siol.net, peng.fan@nxp.com, hverkuil-cisco@xs4all.nl, dan.carpenter@oracle.com, linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel@collabora.com Date: Mon, 22 Feb 2021 15:16:02 -0500 In-Reply-To: <6364fa8a-db6a-af43-3660-7f0a7a3e0b79@collabora.com> References: <20210218191844.297869-1-benjamin.gaignard@collabora.com> <20210218191844.297869-5-benjamin.gaignard@collabora.com> <9ql73glgbnjaqqsp8ulqenae5n82kfk0o3@4ax.com> <6364fa8a-db6a-af43-3660-7f0a7a3e0b79@collabora.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3 (3.38.3-1.fc33) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le lundi 22 février 2021 à 17:28 +0100, Benjamin Gaignard a écrit : > > Le 22/02/2021 à 17:16, John Cox a écrit : > > > The HEVC HANTRO driver needs to know the number of bits to skip at > > > the beginning of the slice header. > > > That is a hardware specific requirement so create a dedicated control > > > that this purpose. > > > > > > Signed-off-by: Benjamin Gaignard > > > --- > > > include/uapi/linux/hantro-v4l2-controls.h | 20 ++++++++++++++++++++ > > > include/uapi/linux/v4l2-controls.h        |  5 +++++ > > > 2 files changed, 25 insertions(+) > > > create mode 100644 include/uapi/linux/hantro-v4l2-controls.h > > > > > > diff --git a/include/uapi/linux/hantro-v4l2-controls.h > > > b/include/uapi/linux/hantro-v4l2-controls.h > > > new file mode 100644 > > > index 000000000000..30b1999b7af3 > > > --- /dev/null > > > +++ b/include/uapi/linux/hantro-v4l2-controls.h > > > @@ -0,0 +1,20 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > > > + > > > +#ifndef __UAPI_HANTRO_V4L2_CONYTROLS_H__ > > > +#define __UAPI_HANTRO_V4L2_CONYTROLS_H__ > > > + > > > +#include > > > +#include > > > + > > > +#define > > > V4L2_CID_HANTRO_HEVC_EXTRA_DECODE_PARAMS       (V4L2_CID_USER_HANTRO_BASE > > > + 0) > > > + > > > +/** > > > + * struct hantro_hevc_extra_decode_params - extra decode parameters for > > > hantro driver > > > + * @hevc_hdr_skip_lenght:      header first bits offset > > > + */ > > > +struct hantro_hevc_extra_decode_params { > > > +       __u32   hevc_hdr_skip_lenght; > > > +       __u8    padding[4]; > > > +}; > > Can you clarify how hevc_hdr_skip_length differs from > > v4l2_ctrl_hevc_slice_params.data_bit_offset?  At first sight they would > > appear to be very similar. > > hevc_hdr_skip_length is the difference between the start positions of 2 nals. > v4l2_ctrl_hevc_slice_params.data_bit_offset is the offset of the data in the > nal. More precisely (and doc should reflect this please), the Hantro skip is the distance in bits for data in the slice_segment_header() header syntax after 'slice_type'. The slice_segment_header_extension_length and bytes seems excluded from reading the reference code, we don't know if this is because this extended header is simply not supported or if it's being parsed by the HW. I believe we need to clarify this before merging the control. If dependent_slice_segment_flag is set, then this offsets seems to be 0. This fact also needs further investigation since this flag, as you mention, isn't passed and is needed to decide whether to parse or not the slice_type. Benjamin, I think we need some HW fact checking for all these, and finally better documentation. > > > Benjamin > > > > > Regards > > > > John Cox > > > > > +#endif > > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2- > > > controls.h > > > index 039c0d7add1b..ced7486c7f46 100644 > > > --- a/include/uapi/linux/v4l2-controls.h > > > +++ b/include/uapi/linux/v4l2-controls.h > > > @@ -209,6 +209,11 @@ enum v4l2_colorfx { > > >   * We reserve 128 controls for this driver. > > >   */ > > > #define V4L2_CID_USER_CCS_BASE                  (V4L2_CID_USER_BASE + > > > 0x10f0) > > > +/* > > > + * The base for HANTRO driver controls. > > > + * We reserve 32 controls for this driver. > > > + */ > > > +#define V4L2_CID_USER_HANTRO_BASE              (V4L2_CID_USER_BASE + > > > 0x1170) > > > > > > /* MPEG-class control IDs */ > > > /* The MPEG controls are applicable to all codec controls >