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.8 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 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 9A580C43460 for ; Wed, 5 May 2021 21:01:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6D05861222 for ; Wed, 5 May 2021 21:01:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235589AbhEEVCj (ORCPT ); Wed, 5 May 2021 17:02:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41418 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235097AbhEEVCi (ORCPT ); Wed, 5 May 2021 17:02:38 -0400 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 63799C061574; Wed, 5 May 2021 14:01:41 -0700 (PDT) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: ezequiel) with ESMTPSA id D21551F4322A Message-ID: Subject: Re: [PATCH v10 6/9] media: uapi: Add a control for HANTRO driver From: Ezequiel Garcia To: Hans Verkuil , Benjamin Gaignard , p.zabel@pengutronix.de, mchehab@kernel.org, robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com, lee.jones@linaro.org, gregkh@linuxfoundation.org, mripard@kernel.org, paul.kocialkowski@bootlin.com, wens@csie.org, jernej.skrabec@siol.net, emil.l.velikov@gmail.com Cc: kernel@pengutronix.de, linux-imx@nxp.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, devel@driverdev.osuosl.org, kernel@collabora.com, cphealy@gmail.com Date: Wed, 05 May 2021 18:01:25 -0300 In-Reply-To: References: <20210420121046.181889-1-benjamin.gaignard@collabora.com> <20210420121046.181889-7-benjamin.gaignard@collabora.com> Organization: Collabora Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.2-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On Wed, 2021-05-05 at 16:55 +0200, Hans Verkuil wrote: > On 20/04/2021 14:10, Benjamin Gaignard wrote: > > 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 > > for this purpose. > > > > Signed-off-by: Benjamin Gaignard > > --- > >  .../userspace-api/media/drivers/hantro.rst    | 19 +++++++++++++++++++ > >  .../userspace-api/media/drivers/index.rst     |  1 + > >  include/media/hevc-ctrls.h                    | 13 +++++++++++++ > >  3 files changed, 33 insertions(+) > >  create mode 100644 Documentation/userspace-api/media/drivers/hantro.rst > > > > diff --git a/Documentation/userspace-api/media/drivers/hantro.rst b/Documentation/userspace-api/media/drivers/hantro.rst > > new file mode 100644 > > index 000000000000..cd9754b4e005 > > --- /dev/null > > +++ b/Documentation/userspace-api/media/drivers/hantro.rst > > @@ -0,0 +1,19 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > +Hantro video decoder driver > > +=========================== > > + > > +The Hantro video decoder driver implements the following driver-specific controls: > > + > > +``V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP (integer)`` > > +    Specifies to Hantro HEVC video decoder driver the number of data (in bits) to > > +    skip in the slice segment header. > > +    If non-IDR, the bits to be skipped go from syntax element "pic_output_flag" > > +    to before syntax element "slice_temporal_mvp_enabled_flag". > > +    If IDR, the skipped bits are just "pic_output_flag" > > +    (separate_colour_plane_flag is not supported). > > I'm not very keen on this. Without this information the video data cannot be > decoded, or will it just be suboptimal? > > The problem is that a generic decoder would have to know that the HW is a hantro, Applications can just query which controls are exposed by a video device, and if this control is found, then it means it needs to be set. > and then call this control. If they don't (and are testing on non-hantro HW), then > it won't work, thus defeating the purpose of the HW independent decoder API. > > Since hantro is widely used, and if there is no other way to do this beside explitely > setting this control, then perhaps this should be part of the standard HEVC API. > Non-hantro drivers that do not need this can just skip it. > The decision to move it out of the HEVC API is not really to avoid setting it. In the end, most/all applications will end up required to set this