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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 817ABCAC5A5 for ; Thu, 25 Sep 2025 12:37:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=iD7D/WniQ0Zxw3Jv/99O/bSelLxFQ6gM67sSZlZ6hik=; b=zRX2bnsTICH3p3J+zegNA/1z2Q D+KIZScNSS6pSgVynzvMuQhejoPgtDdVD2AbkWqfp1ON4Ilipx8gjvF9NHVirB8G5ar2+R3SrA0ht 84X75Bl02zOc/hzw0k9X9Ysf1Ocg0B7nO+3LQDWL2KJuXOb9WVgaXYlrN0coIgpMdaSlkDKWN4VQX pAeAFwSZpWa4eSyCD2fNCgF8tWmeDhWV9IP2X2van13HtEKN4AzbOezFfq7b5RMrKkT7iaDspD3eQ Zzhvvzogy0aXElAkSp5xIwFvU/IURCzB8ENk8ewEizokkY7ucg0rJanN4b9pP7kbRJBB0MgeAZQe1 tj8WW4NQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1v1lDR-00000009240-2q5k; Thu, 25 Sep 2025 12:36:53 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1v1lDQ-0000000922g-0Lhd; Thu, 25 Sep 2025 12:36:52 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 60A8C6013A; Thu, 25 Sep 2025 12:36:51 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 054A8C4CEF0; Thu, 25 Sep 2025 12:36:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758803811; bh=IBUAnul7b8BQqt3zLS4cohInRJ5Mx15MT7PNd8Nz5R4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XkvQ1go55VUCHLla+6DHun6ac9iHj597k6xHzW2ehbWZf1qu17advzm0DOfLaJdX2 CFK1pTszoqCLJrUFasnXEbhaA+1f/RSAgXuAlCqIPBFAunLcZSLWGiS/DqtgPo1lAz vd59DiKljlaDbik78b3p69oUNn/NqxQnsAOZKEj9miVMPRGtAD7JdxnTW3ZYETd/id 3l8YdBfhuJ0+fEQ/tfJEwaZ73jeDNCtDBWibnzrzLBuVvq+35kOG7RWoaJ9NZ0Zvhk QMPN3eI4qbedD4PN9VBWseCLjfv6KabHrHPVkDmVcJZq2VK9elqC28msll8ZwXvERx rJPYbUQBQg33g== Date: Thu, 25 Sep 2025 14:36:46 +0200 From: Maxime Ripard To: Dmitry Baryshkov Cc: Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Thomas Zimmermann , David Airlie , Simona Vetter , Sandy Huang , Heiko =?utf-8?Q?St=C3=BCbner?= , Andy Yan , Chen-Yu Tsai , Samuel Holland , Dave Stevenson , =?utf-8?B?TWHDrXJh?= Canal , Raspberry Pi Kernel Maintenance , Liu Ying , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , Marijn Suijten , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, Daniel Stone Subject: Re: [PATCH v4 01/10] drm/connector: let drivers declare infoframes as unsupported Message-ID: <20250925-didactic-spiked-lobster-fefabe@penduick> References: <20250909-drm-limit-infoframes-v4-0-53fd0a65a4a2@oss.qualcomm.com> <20250909-drm-limit-infoframes-v4-1-53fd0a65a4a2@oss.qualcomm.com> <20250910-furry-singing-axolotl-9aceac@houat> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha384; protocol="application/pgp-signature"; boundary="5gspsh2yhbs7pucv" Content-Disposition: inline In-Reply-To: X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --5gspsh2yhbs7pucv Content-Type: text/plain; protected-headers=v1; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: Re: [PATCH v4 01/10] drm/connector: let drivers declare infoframes as unsupported MIME-Version: 1.0 On Wed, Sep 10, 2025 at 06:16:25PM +0300, Dmitry Baryshkov wrote: > On Wed, Sep 10, 2025 at 01:03:47PM +0200, Maxime Ripard wrote: > > On Tue, Sep 09, 2025 at 05:51:59PM +0300, Dmitry Baryshkov wrote: > > > Currently DRM framework expects that the HDMI connector driver suppor= ts > > > all infoframe types: it generates the data as required and calls into > > > the driver to program all of them, letting the driver to soft-fail if > > > the infoframe is unsupported. This has a major drawback on userspace > > > API: the framework also registers debugfs files for all Infoframe typ= es, > > > possibly surprising the users when infoframe is visible in the debugfs > > > file, but it is not visible on the wire. Drivers are also expected to > > > return success even for unsuppoted InfoFrame types. > > >=20 > > > Let drivers declare that they support only a subset of infoframes, > > > creating a more consistent interface. Make the affected drivers return > > > -EOPNOTSUPP if they are asked to program (or clear) InfoFrames which = are > > > not supported. > > >=20 > > > Acked-by: Liu Ying > > > Acked-by: Daniel Stone > > > Signed-off-by: Dmitry Baryshkov > >=20 > > Again, I still believe that it's a bad idea, goes against what the spec > > states, and the framework was meant to be. >=20 > Please correct me if I'm wrong. The specs (HDMI & CEA) define several > infoframes and whether we should be sending them. If I'm reading it > correctrly, CEA spec explicitly says 'If the Source supports the > transmission of [foo] InfoFrame..." (6.4 - AVI, 6.6 - Audio, 6.7 MPEG, > 6.9 - DRM). For other InfoFrames (6.5 SPD, 6.8 NTSC VBI) it just defines > that sending those frames is optional. SPD is indeed optional, but the HDMI spec, (HDMI 1.4b, Section 8.2.1 - Auxiliary Video information (AVI) InfoFrame) states that: A Source shall always transmit an AVI InfoFrame at least once per two Video Fields if the Source: * is ever capable of transmitting an AVI InfoFrame or, * is ever capable of transmitting YCBCR pixel encoding or, * is ever capable of transmitting any colorimetry other than the transmitted video format=E2=80=99s default colorimetry or, * is ever capable of transmitting any xvYCC or future enhanced colorimetry or, * is ever capable of transmitting any Gamut Metadata packet or, * is ever capable of transmitting any video format with multiple allowed pixel repetitions or, * is ever capable of transmitting Content Type other than =E2=80=9Cno da= ta=E2=80=9D or, * is ever capable of transmitting YCC Quantization Range. The AVI InfoFrame shall be transmitted even while such a Source is transmitting RGB and non-pixel-repeated video. When a Source is not explicitly required to transmit AVI InfoFrames, it is recommended that the Source transmit AVI InfoFrames. So it's recommended in every case, and the list kind of makes it mandatory for how Linux uses HDMI. Also, did we ever encounter some hardware that couldn't send AVI? Audio is mandatory when streaming audio, DRM when using HDR, and we don't support the others yet. So the only we support but don't have to send is SPD. > We can't even infer support for InfoFrames from the Source features. > E.g. CEA 6.6.1 defines a case when digital audio is allowed to be sent, > without sending Audio InfoFrame. HDMI 1.4 section 8.2.2 - Audio Infoframe states that: Whenever an active audio stream is being transmitted, an accurate Audio InfoFrame shall be transmitted at least once per two Video Fields. I'd say it takes precedence over CTA-861. > As we will be getting more and more features, some of the InfoFrames > or data packets will be 'good to have, but not required'. And drivers would be free to ignore those. > > So, no, sorry. That's still a no for me. Please stop sending that patch >=20 > Oops :-) >=20 > > unless we have a discussion about it and you convince me that it's > > actually something that we'd need. >=20 > My main concern is that the drivers should not opt-out of the features. > E.g. if we start supporting ISRC packets or MPEG or NTSC VBI InfoFrames > (yes, stupid examples), it should not be required to go through all the > drivers, making sure that they disable those. Instead the DRM framework > should be able to make decisions like: >=20 > - The driver supports SPD and the VSDB defines SPD, enable this > InfoFrame (BTW, this needs to be done anyway, we should not be sending > SPD if it's not defined in VSDB, if I read it correctly). >=20 > - The driver hints that the pixel data has only 10 meaninful bits of > data per component (e.g. out of 12 for DeepColor 36), the Sink has > HF-VSDB, send HF-VSIF. >=20 > - The driver has enabled 3D stereo mode, but it doesn't declare support > for HF-VSIF. Send only H14b-VSIF. >=20 > Similarly (no, I don't have these on my TODO list, these are just > examples): > - The driver defines support for NTSC VBI, register a VBI device. >=20 > - The driver defines support for ISRC packets, register ISRC-related > properties. >=20 > - The driver defines support for MPEG Source InfoFrame, provide a way > for media players to report frame type and bit rate. >=20 > - The driver provides limited support for Extended HDR DM InfoFrames, > select the correct frame type according to driver capabilities. >=20 > Without the 'supported' information we should change atomic_check() > functions to set infoframe->set to false for all unsupported InfoFrames > _and_ go through all the drivers again each time we add support for a > feature (e.g. after adding HF-VSIF support). =46rom what you described here, I think we share a similar goal and have somewhat similar concerns (thanks, btw, it wasn't obvious to me before), we just disagree on the trade-offs and ideal solution :) I agree that we need to sanity check the drivers, and I don't want to go back to the situation we had before where drivers could just ignore infoframes and take the easy way out. It should be hard, and easy to catch during review. I don't think bitflag are a solution because, to me, it kind of fails both. What if, just like the debugfs discussion, we split write_infoframe into write_avi_infoframe (mandatory), write_spd_infoframe (optional), write_audio_infoframe (checked by drm_connector_hdmi_audio_init?) and write_hdr_infoframe (checked in drmm_connector_hdmi_init if max_bpc > 8) How does that sound? Maxime --5gspsh2yhbs7pucv Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iJUEABMJAB0WIQTkHFbLp4ejekA/qfgnX84Zoj2+dgUCaNU3XQAKCRAnX84Zoj2+ duiWAYCEQHaNpq1acegbRGgnWK7idyco+oNyb5OKUdG2mSrF8iG51+HcmQ30rpWH OmNB+HsBf3AD+4uAstcClyYSies8sQ9MYWjuxdp67l9l23iryEuQM410hwJTA3iL XSaP3z9ISQ== =35Cr -----END PGP SIGNATURE----- --5gspsh2yhbs7pucv--