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 E350DCAC5B0 for ; Mon, 29 Sep 2025 13:00:19 +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=WWN09e1ExuGSiGEWOipe17yKpFWEQ3Pdou/x4FJyVyc=; b=fpnr+jDrbcvhUfpXMZ5/Rx9vvV 4L4VUzf69FrWp2XOTsV7LPLXrC/Ly8tN8sFb5brOEMkSInNPntFQ7CBlC0phB1n9naRvw+XV+pess 5hR0PhSKp519cdZZDLXZcyCxDmnpP37yoW5lY8SHePzrqsuUkQ3dSMUGPVNnA6c3tO1aO7OWg4eNB ricM0HL86K59rxv9Ke3YSCrwf4bcO6o24B62G4PWDTy8/6Eo1MCGw0bILHKS4Y4VbkphYKsvIeRYM leYGdlOZuPCeIyvzn7JoZ0fUScn9NhOalewdYgLeTlw8jAoSlALzIhPemK7F6x1nINicSLn1HrEo0 nP14e5aQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1v3DUC-00000002VqY-3HuQ; Mon, 29 Sep 2025 13:00:12 +0000 Received: from sea.source.kernel.org ([2600:3c0a:e001:78e:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1v3DUA-00000002Vpi-0dDw; Mon, 29 Sep 2025 13:00:11 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id D323340AE1; Mon, 29 Sep 2025 13:00:06 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3639AC4CEF4; Mon, 29 Sep 2025 13:00:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1759150806; bh=v14ZqLeXTqvnBbKfvaWvZF9NSLmuvJRKsZQBeOmW5Wk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jz8wwdhF/Yk6MGtZ5Kdn5W8KFDTZFtIhNpHLM8je6/OG7jVFn/717TBkjRnVxmSPy VI1Dku+c1ztJrRasrV2hmU/nNli7IsX8XRIGI5PCB9xbYnmwPj4pjsb4xFcXXvCftj IGkGfw9u6XLg7PdBvq/L4qevkPuOrYAAbXa0HjFMi/gj/RUXV860hs6msyfOP2l43H 3kPkDzyiPpaIwploKLOSx8yoBoCD4+9f+UNSRUcMaLxTKBhC7hKg4vrrlS/TvSomde e7XsEjO2ZImnkX2A/V6Ub3iFY0snOBM3YLzqDbLzkbboJAY2kJkoai/xENtnwBglub +WuX5P/Ynzd+w== Date: Mon, 29 Sep 2025 15:00:04 +0200 From: Maxime Ripard To: Dmitry Baryshkov Cc: Daniel Stone , 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 Subject: Re: [PATCH v3 00/11] drm/connector: hdmi: limit infoframes per driver capabilities Message-ID: <20250929-gregarious-worm-of-memory-c5354d@houat> References: <57ekub6uba7iee34sviadareqxv234zbmkr7avqofxes4mqnru@vgkppexnj6cb> <20250901-voracious-classy-hedgehog-ee28ef@houat> <20250910-didactic-honored-chachalaca-f233b2@houat> <20250925-fervent-merry-beagle-2baba3@penduick> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha384; protocol="application/pgp-signature"; boundary="5omahayljejanoli" Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250929_060010_237835_94F6893A X-CRM114-Status: GOOD ( 74.24 ) 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 --5omahayljejanoli Content-Type: text/plain; protected-headers=v1; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: Re: [PATCH v3 00/11] drm/connector: hdmi: limit infoframes per driver capabilities MIME-Version: 1.0 On Thu, Sep 25, 2025 at 05:16:07PM +0300, Dmitry Baryshkov wrote: > On Thu, Sep 25, 2025 at 03:13:47PM +0200, Maxime Ripard wrote: > > On Wed, Sep 10, 2025 at 06:26:56PM +0300, Dmitry Baryshkov wrote: > > > On Wed, Sep 10, 2025 at 09:30:19AM +0200, Maxime Ripard wrote: > > > > On Wed, Sep 03, 2025 at 03:03:43AM +0300, Dmitry Baryshkov wrote: > > > > > On Tue, Sep 02, 2025 at 08:06:54PM +0200, Maxime Ripard wrote: > > > > > > On Tue, Sep 02, 2025 at 06:45:44AM +0300, Dmitry Baryshkov wrot= e: > > > > > > > On Mon, Sep 01, 2025 at 09:07:02AM +0200, Maxime Ripard wrote: > > > > > > > > On Sun, Aug 31, 2025 at 01:29:13AM +0300, Dmitry Baryshkov = wrote: > > > > > > > > > On Sat, Aug 30, 2025 at 09:30:01AM +0200, Daniel Stone wr= ote: > > > > > > > > > > Hi Dmitry, > > > > > > > > > >=20 > > > > > > > > > > On Sat, 30 Aug 2025 at 02:23, Dmitry Baryshkov > > > > > > > > > > wrote: > > > > > > > > > > > It's not uncommon for the particular device to suppor= t only a subset of > > > > > > > > > > > HDMI InfoFrames. It's not a big problem for the kerne= l, since we adopted > > > > > > > > > > > a model of ignoring the unsupported Infoframes, but i= t's a bigger > > > > > > > > > > > problem for the userspace: we end up having files in = debugfs which do > > > > > > > > > > > mot match what is being sent on the wire. > > > > > > > > > > > > > > > > > > > > > > Sort that out, making sure that all interfaces are co= nsistent. > > > > > > > > > >=20 > > > > > > > > > > Thanks for the series, it's a really good cleanup. > > > > > > > > > >=20 > > > > > > > > > > I know that dw-hdmi-qp can support _any_ infoframe, by = manually > > > > > > > > > > packing it into the two GHDMI banks. So the supported s= et there is > > > > > > > > > > 'all of the currently well-known ones, plus any two oth= ers, but only > > > > > > > > > > two and not more'. I wonder if that has any effect on t= he interface > > > > > > > > > > you were thinking about for userspace? > > > > > > > > >=20 > > > > > > > > > I was mostly concerned with the existing debugfs interfac= e (as it is > > > > > > > > > also used e.g. for edid-decode, etc). > > > > > > > > >=20 > > > > > > > > > It seems "everything + 2 spare" is more or less common (A= DV7511, MSM > > > > > > > > > HDMI also have those. I don't have at hand the proper dat= asheet for > > > > > > > > > LT9611 (non-UXC one), but I think its InfoFrames are also= more or less > > > > > > > > > generic). Maybe we should change debugfs integration to = register the > > > > > > > > > file when the frame is being enabled and removing it when= it gets unset. > > > > > > > >=20 > > > > > > > > But, like, for what benefit? > > > > > > > >=20 > > > > > > > > It's a debugfs interface for userspace to consume. The curr= ent setup > > > > > > > > works fine with edid-decode already. Why should we complica= te the design > > > > > > > > that much and create fun races like "I'm running edid-decod= e in parallel > > > > > > > > to a modeset that would remove the file I just opened, what= is the file > > > > > > > > now?". > > > > > > >=20 > > > > > > > Aren't we trading that with the 'I'm running edid-decode in p= aralle with > > > > > > > to a modeset and the file suddenly becomes empty'? > > > > > >=20 > > > > > > In that case, you know what the file is going to be: empty. And= you went > > > > > > from a racy, straightforward, design to a racy, complicated, de= sign. > > > > > >=20 > > > > > > It was my question before, but I still don't really see what be= nefits it > > > > > > would have, and why we need to care about it in the core, when = it could > > > > > > be dealt with in the drivers just fine on a case by case basis. > > > > >=20 > > > > > Actually it can not: debugfs files are registered from the core, = not > > > > > from the drivers. That's why I needed all the supported_infoframes > > > > > (which later became software_infoframes). > > > >=20 > > > > That's one thing we can change then. > > > >=20 > > > > > Anyway, I'm fine with having empty files there. > > > > >=20 > > > > > > > > > Then in the long run we can add 'slots' and allocate some= of the frames > > > > > > > > > to the slots. E.g. ADV7511 would get 'software AVI', 'sof= tware SPD', > > > > > > > > > 'auto AUDIO' + 2 generic slots (and MPEG InfoFrame which = can probably be > > > > > > > > > salvaged as another generic one)). MSM HDMI would get 'so= ftware AVI', > > > > > > > > > 'software AUDIO' + 2 generic slots (+MPEG + obsucre HDMI = which I don't > > > > > > > > > want to use). Then the framework might be able to priorit= ize whether to > > > > > > > > > use generic slots for important data (as DRM HDR, HDMI) o= r less important > > > > > > > > > (SPD). > > > > > > > >=20 > > > > > > > > Why is it something for the framework to deal with? If you = want to have > > > > > > > > extra infoframes in there, just go ahead and create additio= nal debugfs > > > > > > > > files in your driver. > > > > > > > >=20 > > > > > > > > If you want to have the slot mechanism, check in your atomi= c_check that > > > > > > > > only $NUM_SLOT at most infoframes are set. > > > > > > >=20 > > > > > > > The driver can only decide that 'we have VSI, SPD and DRM Inf= oFrames > > > > > > > which is -ETOOMUCH for 2 generic slots'. The framework should= be able to > > > > > > > decide 'the device has 2 generic slots, we have HDR data, use= VSI and > > > > > > > DRM InfoFrames and disable SPD for now'. > > > > > >=20 > > > > > > I mean... the spec does? The spec says when a particular feature > > > > > > requires to send a particular infoframe. If your device cannot = support > > > > > > to have more than two "features" enabled at the same time, so b= e it. It > > > > > > something that should be checked in that driver atomic_check. > > > > >=20 > > > > > Sounds good to me. Let's have those checks in the drivers until we > > > > > actually have seveal drivers performing generic frame allocation. > > > > >=20 > > > > > > Or just don't register the SPD debugfs file, ignore it, put a c= omment > > > > > > there, and we're done too. > > > > >=20 > > > > > It's generic code. > > > > >=20 > > > > > > > But... We are not there yet and I don't have clear usecase (w= e support > > > > > > > HDR neither on ADV7511 nor on MSM HDMI, after carefully readi= ng the > > > > > > > guide I realised that ADV7511 has normal audio infoframes). M= aybe I > > > > > > > should drop all the 'auto' features, simplifying this series = and land > > > > > > > [1] for LT9611UXC as I wanted origianlly. > > > > > > >=20 > > > > > > > [1] https://lore.kernel.org/dri-devel/20250803-lt9611uxc-hdmi= -v1-2-cb9ce1793acf@oss.qualcomm.com/ > > > > > >=20 > > > > > > Looking back at that series, I think it still has value to rely= on the > > > > > > HDMI infrastructure at the very least for the atomic_check sani= tization. > > > > > >=20 > > > > > > But since you wouldn't use the generated infoframes, just skip = the > > > > > > debugfs files registration. You're not lying to userspace anymo= re, and > > > > > > you get the benefits of the HDMI framework. > > > > >=20 > > > > > We create all infoframe files for all HDMI connectors. > > > >=20 > > > > Then we can provide a debugfs_init helper to register all of them, = or > > > > only some of them, and let the drivers figure it out. > > > >=20 > > > > Worst case scenario, debugfs files will not get created, which is a= much > > > > better outcome than having to put boilerplate in every driver that = will > > > > get inconsistent over time. > > >=20 > > > debugfs_init() for each infoframe or taking some kind of bitmask? > >=20 > > I meant turning hdmi_debugfs_add and create_hdmi_*_infoframe_file into > > public helpers. That way, drivers that don't care can use the (renamed) > > hdmi_debugfs_add, and drivers with different constraints can register > > the relevant infoframes directly. >=20 > Doesn't that mean more boilerplate? I don't think it would? In the general case, it wouldn't change anything, and in special cases, then it's probably going to be different =66rom one driver to the next so there's not much we can do. > In the end, LT9611UXC is a special case, for which I'm totally fine > not to use HDMI helpers at this point: we don't control infoframes > (hopefully that can change), we don't care about the TMDS clock, no > CEC, etc. Not using the helpers sound pretty reasonable here too. > For all other usecases I'm fine with having atomic_check() unset all > unsupported infoframes and having empty files in debugfs. Then we can > evolve over the time, once we see a pattern. We had several drivers > which had very limited infoframes support, but I think this now gets > sorted over the time. I never talked about atomic_check()? You were initially concerned that the framework would expose data in debugfs that it's not using. Not registering anything in debugfs solves that, but I'm not sure we need to special case atomic_check. Worst case scenario, we're going to generate infoframes the driver will ignore. Maxime --5omahayljejanoli Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iJUEABMJAB0WIQTkHFbLp4ejekA/qfgnX84Zoj2+dgUCaNqC0wAKCRAnX84Zoj2+ diX/AYD7tGMpz//OLeUzSg3QYHAZkitZyXT00YfJmgxYpCUMSnb6h8ucgQ8Om8Vk gbiiQX0Bf2U6KVN7HpvEuga5AMtIOqqgCE/fQHox7oXRJiC3nuzlZW5e70PMzD7l uQgN/qqEkQ== =aZZH -----END PGP SIGNATURE----- --5omahayljejanoli--