From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from wnew1-smtp.messagingengine.com (wnew1-smtp.messagingengine.com [64.147.123.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9401C186A for ; Thu, 27 Oct 2022 11:58:32 +0000 (UTC) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.west.internal (Postfix) with ESMTP id 9AD912B066FE; Thu, 27 Oct 2022 07:58:27 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 27 Oct 2022 07:58:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm3; t=1666871907; x=1666879107; bh=QyApW52GGW OGnMQtvgaWWVm3IOEOZXDwE9lLItGdiLY=; b=uF//YRP5AA3BkLmQtb+sTcWpgH hk2HTy6BCO6QZu5obOUiI2weB0GqkqU7EYohbQTg1XdfrOGCYkVJi+p3vOUopU6I JTXI5dP21OTXqJRlGU/wvVC5PBPw4hYaL2AFhxYgnFTDv36LLn4rA7ytqFXb3zAk bt+FmImAVzuTI2sHRcVQRCv/XjSppxGquUVmshtTICQWRRE3m9az2fp8SqKScgPK TUaI0dfU29NYrX6H/9nIbVVx2ljADhJWJ5B6VvyFLDHVXCDe2q5n3d2htKFfJmAI ItI5qfAu65peUdI4fxjyHl+8YesnAPhZ0KQiGuVQ6WsmAbm8bItIXBigIHYA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1666871907; x=1666879107; bh=QyApW52GGWOGnMQtvgaWWVm3IOEO ZXDwE9lLItGdiLY=; b=X5KOnyeXpFpd9UUPBhTooylt8Bn5VnVl9lyj1OWBqdOT 3eMOeTk76m/uJpIUSNpx/gKUJuCsu4oMVW+2w5uu53DfcAi2dyotu5NiSwMA7zF2 kDpC9DrK+wT9QK/xPGF7J879cPUWSeP+ovy4NIXMMqQrF0uuI+vuw03DlVAz7TQS pt3dbB5A7FyhUe6nG3yGWQ9spnihyotU/dXSS9/Fo8475Urre+4o7TjnCZnzYvrs r73sucWl8RuMrSovYvdUOJfNWjeLek2qCEnfjkR9btgZ9nKZXlS5g5npQPNqoM5K h/RVHWs+mq0A+Pr7hQTRzfRHGu5MGs5r3H83s45SOQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrtdeggdeggecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnheptdelkeejiedufedvhfeiffetlefhiedvleeigfeiuefghfdvleeiffejieeg teejnecuffhomhgrihhnpehkvghrnhgvlhdrohhrghenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Feedback-ID: i8771445c:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 27 Oct 2022 07:58:24 -0400 (EDT) Date: Thu, 27 Oct 2022 13:58:22 +0200 From: Maxime Ripard To: Mateusz Kwiatkowski Cc: Karol Herbst , Emma Anholt , Ben Skeggs , Chen-Yu Tsai , Rodrigo Vivi , Maarten Lankhorst , Jani Nikula , Daniel Vetter , Thomas Zimmermann , Tvrtko Ursulin , Samuel Holland , Jernej Skrabec , David Airlie , Joonas Lahtinen , Lyude Paul , linux-sunxi@lists.linux.dev, intel-gfx@lists.freedesktop.org, Phil Elwell , linux-arm-kernel@lists.infradead.org, nouveau@lists.freedesktop.org, Hans de Goede , Dom Cobley , dri-devel@lists.freedesktop.org, Dave Stevenson , linux-kernel@vger.kernel.org, Noralf =?utf-8?Q?Tr=C3=B8nnes?= , Geert Uytterhoeven Subject: Re: [PATCH v6 22/23] drm/vc4: vec: Add support for more analog TV standards Message-ID: <20221027115822.5vd3fqlcpy4gfq5v@houat> References: <20220728-rpi-analog-tv-properties-v6-0-e7792734108f@cerno.tech> <20220728-rpi-analog-tv-properties-v6-22-e7792734108f@cerno.tech> Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ysfbxxbxsmqctfb3" Content-Disposition: inline In-Reply-To: --ysfbxxbxsmqctfb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Thu, Oct 27, 2022 at 12:32:50AM +0200, Mateusz Kwiatkowski wrote: > I've seen that you've incorporated my PAL60 patch. Thanks! >=20 > I still yet need to test your v6 changes, but looking at this code with j= ust my > mental static analysis, it seems to me that the vc4_vec_encoder_atomic_ch= eck() > should have the tv_mode validation. I should've added it to the PAL60 pat= ch, > but it somehow slipped my mind then. >=20 > Anyway, I mentioned it previously here: > https://lore.kernel.org/dri-devel/0f2beec2-ae8e-5579-f0b6-a73d9dae1af4@gm= ail.com/ >=20 > It would look something like this, inside vc4_vec_encoder_atomic_check(): >=20 > + const struct vc4_vec_tv_mode *tv_mode =3D > + vc4_vec_tv_mode_lookup(conn_state->tv.mode); > + > + if (!tv_mode) > + return -EINVAL; >=20 > Without this, it's possible to set e.g. 480i mode and SECAM, which will f= ail - > but with the current version it will only fail in vc4_vec_encoder_enable(= ), > which cannot return an error, and in my experience that causes a rather l= engthy > lockup. ACK, I'll add it. > But, like I said, I still need to actually test that with this version. > > Anyway, I was also thinking about adding support for the more "exotic" > non-standard modes. NTSC-50 is, unfortunately, impossible with VEC, but > PAL-N-60 and PAL-M-50 should work. The necessary vc4_vec_tv_modes entries= would > look something like: >=20 > @@ -325,12 +325,28 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_mode= s[] =3D { > .config0 =3D VEC_CONFIG0_PAL_M_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-M-50 */ > + .mode =3D DRM_MODE_TV_MODE_PAL, > + .expected_htotal =3D 864, > + .config0 =3D VEC_CONFIG0_PAL_BDGHI_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21e6efe3, > + }, > { > .mode =3D DRM_MODE_TV_MODE_PAL_N, > .expected_htotal =3D 864, > .config0 =3D VEC_CONFIG0_PAL_N_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-N-60 */ > + .mode =3D DRM_MODE_TV_MODE_PAL_N, > + .expected_htotal =3D 858, > + .config0 =3D VEC_CONFIG0_PAL_M_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21f69446, > + }, > { > .mode =3D DRM_MODE_TV_MODE_SECAM, > .expected_htotal =3D 864, >=20 > I'm not sure if we actually want to add that. The two arguments for doing= so > I can think of is 1. it should work, so "why not", 2. it means that more = modes > will result in _some_ kind of a valid signal, rather than erroring out, w= hich > is always a plus in my book. I can also think of a hypothetical use case,= like > someone in South America with an old PAL-N-only set that would neverthele= ss > still sync at 60 Hz (perhaps with the help of messing with vertical hold = knob), > who would like to play retro games at 60 Hz in color. >=20 > But on the other hand, I admit that this scenario is likely a stretch and= the > number of people who would actually use it is probably close to the prove= rbial > two ;) So it's your call, I'm just leaving those settings here just in ca= se. This series is already pretty massive and is difficult to merge, so I'd rather avoid to add new stuff in at every version. The changes look easy enough to be a follow-up patch, so I'd prefer to do it that way. Maxime --ysfbxxbxsmqctfb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCY1pyXgAKCRDj7w1vZxhR xUhMAP0XpvpKwMyQUT0qv5zm8wU1Oj1FqnHhCXeuVVO8jcNQEQEA3oyfkhXsxwGB 0XwSRrhGDf0kHcwS9XXqBCQFEgMYgww= =04yJ -----END PGP SIGNATURE----- --ysfbxxbxsmqctfb3-- 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 E3470FA3742 for ; Thu, 27 Oct 2022 11:58:38 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 037EF10E06E; Thu, 27 Oct 2022 11:58:38 +0000 (UTC) Received: from wnew1-smtp.messagingengine.com (wnew1-smtp.messagingengine.com [64.147.123.26]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1FE3D10E06E; Thu, 27 Oct 2022 11:58:33 +0000 (UTC) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.west.internal (Postfix) with ESMTP id 9AD912B066FE; Thu, 27 Oct 2022 07:58:27 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 27 Oct 2022 07:58:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm3; t=1666871907; x=1666879107; bh=QyApW52GGW OGnMQtvgaWWVm3IOEOZXDwE9lLItGdiLY=; b=uF//YRP5AA3BkLmQtb+sTcWpgH hk2HTy6BCO6QZu5obOUiI2weB0GqkqU7EYohbQTg1XdfrOGCYkVJi+p3vOUopU6I JTXI5dP21OTXqJRlGU/wvVC5PBPw4hYaL2AFhxYgnFTDv36LLn4rA7ytqFXb3zAk bt+FmImAVzuTI2sHRcVQRCv/XjSppxGquUVmshtTICQWRRE3m9az2fp8SqKScgPK TUaI0dfU29NYrX6H/9nIbVVx2ljADhJWJ5B6VvyFLDHVXCDe2q5n3d2htKFfJmAI ItI5qfAu65peUdI4fxjyHl+8YesnAPhZ0KQiGuVQ6WsmAbm8bItIXBigIHYA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1666871907; x=1666879107; bh=QyApW52GGWOGnMQtvgaWWVm3IOEO ZXDwE9lLItGdiLY=; b=X5KOnyeXpFpd9UUPBhTooylt8Bn5VnVl9lyj1OWBqdOT 3eMOeTk76m/uJpIUSNpx/gKUJuCsu4oMVW+2w5uu53DfcAi2dyotu5NiSwMA7zF2 kDpC9DrK+wT9QK/xPGF7J879cPUWSeP+ovy4NIXMMqQrF0uuI+vuw03DlVAz7TQS pt3dbB5A7FyhUe6nG3yGWQ9spnihyotU/dXSS9/Fo8475Urre+4o7TjnCZnzYvrs r73sucWl8RuMrSovYvdUOJfNWjeLek2qCEnfjkR9btgZ9nKZXlS5g5npQPNqoM5K h/RVHWs+mq0A+Pr7hQTRzfRHGu5MGs5r3H83s45SOQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrtdeggdeggecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnheptdelkeejiedufedvhfeiffetlefhiedvleeigfeiuefghfdvleeiffejieeg teejnecuffhomhgrihhnpehkvghrnhgvlhdrohhrghenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Feedback-ID: i8771445c:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 27 Oct 2022 07:58:24 -0400 (EDT) Date: Thu, 27 Oct 2022 13:58:22 +0200 From: Maxime Ripard To: Mateusz Kwiatkowski Message-ID: <20221027115822.5vd3fqlcpy4gfq5v@houat> References: <20220728-rpi-analog-tv-properties-v6-0-e7792734108f@cerno.tech> <20220728-rpi-analog-tv-properties-v6-22-e7792734108f@cerno.tech> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ysfbxxbxsmqctfb3" Content-Disposition: inline In-Reply-To: Subject: Re: [Nouveau] [PATCH v6 22/23] drm/vc4: vec: Add support for more analog TV standards X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , nouveau@lists.freedesktop.org, Joonas Lahtinen , dri-devel@lists.freedesktop.org, Phil Elwell , Emma Anholt , Samuel Holland , Jernej Skrabec , Chen-Yu Tsai , Geert Uytterhoeven , Ben Skeggs , linux-sunxi@lists.linux.dev, Daniel Vetter , intel-gfx@lists.freedesktop.org, Maarten Lankhorst , Jani Nikula , Hans de Goede , Rodrigo Vivi , linux-arm-kernel@lists.infradead.org, Tvrtko Ursulin , Dom Cobley , linux-kernel@vger.kernel.org, Noralf =?utf-8?Q?Tr=C3=B8nnes?= Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" --ysfbxxbxsmqctfb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Thu, Oct 27, 2022 at 12:32:50AM +0200, Mateusz Kwiatkowski wrote: > I've seen that you've incorporated my PAL60 patch. Thanks! >=20 > I still yet need to test your v6 changes, but looking at this code with j= ust my > mental static analysis, it seems to me that the vc4_vec_encoder_atomic_ch= eck() > should have the tv_mode validation. I should've added it to the PAL60 pat= ch, > but it somehow slipped my mind then. >=20 > Anyway, I mentioned it previously here: > https://lore.kernel.org/dri-devel/0f2beec2-ae8e-5579-f0b6-a73d9dae1af4@gm= ail.com/ >=20 > It would look something like this, inside vc4_vec_encoder_atomic_check(): >=20 > + const struct vc4_vec_tv_mode *tv_mode =3D > + vc4_vec_tv_mode_lookup(conn_state->tv.mode); > + > + if (!tv_mode) > + return -EINVAL; >=20 > Without this, it's possible to set e.g. 480i mode and SECAM, which will f= ail - > but with the current version it will only fail in vc4_vec_encoder_enable(= ), > which cannot return an error, and in my experience that causes a rather l= engthy > lockup. ACK, I'll add it. > But, like I said, I still need to actually test that with this version. > > Anyway, I was also thinking about adding support for the more "exotic" > non-standard modes. NTSC-50 is, unfortunately, impossible with VEC, but > PAL-N-60 and PAL-M-50 should work. The necessary vc4_vec_tv_modes entries= would > look something like: >=20 > @@ -325,12 +325,28 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_mode= s[] =3D { > .config0 =3D VEC_CONFIG0_PAL_M_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-M-50 */ > + .mode =3D DRM_MODE_TV_MODE_PAL, > + .expected_htotal =3D 864, > + .config0 =3D VEC_CONFIG0_PAL_BDGHI_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21e6efe3, > + }, > { > .mode =3D DRM_MODE_TV_MODE_PAL_N, > .expected_htotal =3D 864, > .config0 =3D VEC_CONFIG0_PAL_N_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-N-60 */ > + .mode =3D DRM_MODE_TV_MODE_PAL_N, > + .expected_htotal =3D 858, > + .config0 =3D VEC_CONFIG0_PAL_M_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21f69446, > + }, > { > .mode =3D DRM_MODE_TV_MODE_SECAM, > .expected_htotal =3D 864, >=20 > I'm not sure if we actually want to add that. The two arguments for doing= so > I can think of is 1. it should work, so "why not", 2. it means that more = modes > will result in _some_ kind of a valid signal, rather than erroring out, w= hich > is always a plus in my book. I can also think of a hypothetical use case,= like > someone in South America with an old PAL-N-only set that would neverthele= ss > still sync at 60 Hz (perhaps with the help of messing with vertical hold = knob), > who would like to play retro games at 60 Hz in color. >=20 > But on the other hand, I admit that this scenario is likely a stretch and= the > number of people who would actually use it is probably close to the prove= rbial > two ;) So it's your call, I'm just leaving those settings here just in ca= se. This series is already pretty massive and is difficult to merge, so I'd rather avoid to add new stuff in at every version. The changes look easy enough to be a follow-up patch, so I'd prefer to do it that way. Maxime --ysfbxxbxsmqctfb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCY1pyXgAKCRDj7w1vZxhR xUhMAP0XpvpKwMyQUT0qv5zm8wU1Oj1FqnHhCXeuVVO8jcNQEQEA3oyfkhXsxwGB 0XwSRrhGDf0kHcwS9XXqBCQFEgMYgww= =04yJ -----END PGP SIGNATURE----- --ysfbxxbxsmqctfb3-- 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 7A427FA3742 for ; Thu, 27 Oct 2022 11:59:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To: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=faABykB7SkyDbf0RcgYtUlAWT1WP7yeM1LCxPw7I7sg=; b=xPc/5j8njGP5fJnvJPLpUORI5v No0HQNVpamMbKzPrn3cSsbCyi6/denuX0cJJp0N3yEf9+AzhC/XRHIAhELn2e/3l82ajDbTd4nXDZ XR5VMwrSByjPMSqZbZXeKJ3gqRV5MekpRVq09HwIbfEbUnK+5mklNTdZPHPMsjOHe5H7RgYQRRXYo e2hHmpTv3q17RpjAROgMPx2Zs1vnhYdghv5Tr+mN8LiPEH82y6fTlFTWxRnjJcCUWZ7rRtmIcut1S eptM+D+DAUhEECp06lNwa6Q31/bLlBOQFcYq15G1vNT8/Vka83+k6XbOluD4INRxoQgEaK0zGUhWo KkY89ULw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oo1Wu-00D6W2-IA; Thu, 27 Oct 2022 11:58:36 +0000 Received: from wnew1-smtp.messagingengine.com ([64.147.123.26]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oo1Wr-00D6VA-KH for linux-arm-kernel@lists.infradead.org; Thu, 27 Oct 2022 11:58:35 +0000 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.west.internal (Postfix) with ESMTP id 9AD912B066FE; Thu, 27 Oct 2022 07:58:27 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 27 Oct 2022 07:58:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm3; t=1666871907; x=1666879107; bh=QyApW52GGW OGnMQtvgaWWVm3IOEOZXDwE9lLItGdiLY=; b=uF//YRP5AA3BkLmQtb+sTcWpgH hk2HTy6BCO6QZu5obOUiI2weB0GqkqU7EYohbQTg1XdfrOGCYkVJi+p3vOUopU6I JTXI5dP21OTXqJRlGU/wvVC5PBPw4hYaL2AFhxYgnFTDv36LLn4rA7ytqFXb3zAk bt+FmImAVzuTI2sHRcVQRCv/XjSppxGquUVmshtTICQWRRE3m9az2fp8SqKScgPK TUaI0dfU29NYrX6H/9nIbVVx2ljADhJWJ5B6VvyFLDHVXCDe2q5n3d2htKFfJmAI ItI5qfAu65peUdI4fxjyHl+8YesnAPhZ0KQiGuVQ6WsmAbm8bItIXBigIHYA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1666871907; x=1666879107; bh=QyApW52GGWOGnMQtvgaWWVm3IOEO ZXDwE9lLItGdiLY=; b=X5KOnyeXpFpd9UUPBhTooylt8Bn5VnVl9lyj1OWBqdOT 3eMOeTk76m/uJpIUSNpx/gKUJuCsu4oMVW+2w5uu53DfcAi2dyotu5NiSwMA7zF2 kDpC9DrK+wT9QK/xPGF7J879cPUWSeP+ovy4NIXMMqQrF0uuI+vuw03DlVAz7TQS pt3dbB5A7FyhUe6nG3yGWQ9spnihyotU/dXSS9/Fo8475Urre+4o7TjnCZnzYvrs r73sucWl8RuMrSovYvdUOJfNWjeLek2qCEnfjkR9btgZ9nKZXlS5g5npQPNqoM5K h/RVHWs+mq0A+Pr7hQTRzfRHGu5MGs5r3H83s45SOQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrtdeggdeggecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnheptdelkeejiedufedvhfeiffetlefhiedvleeigfeiuefghfdvleeiffejieeg teejnecuffhomhgrihhnpehkvghrnhgvlhdrohhrghenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Feedback-ID: i8771445c:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 27 Oct 2022 07:58:24 -0400 (EDT) Date: Thu, 27 Oct 2022 13:58:22 +0200 From: Maxime Ripard To: Mateusz Kwiatkowski Cc: Karol Herbst , Emma Anholt , Ben Skeggs , Chen-Yu Tsai , Rodrigo Vivi , Maarten Lankhorst , Jani Nikula , Daniel Vetter , Thomas Zimmermann , Tvrtko Ursulin , Samuel Holland , Jernej Skrabec , David Airlie , Joonas Lahtinen , Lyude Paul , linux-sunxi@lists.linux.dev, intel-gfx@lists.freedesktop.org, Phil Elwell , linux-arm-kernel@lists.infradead.org, nouveau@lists.freedesktop.org, Hans de Goede , Dom Cobley , dri-devel@lists.freedesktop.org, Dave Stevenson , linux-kernel@vger.kernel.org, Noralf =?utf-8?Q?Tr=C3=B8nnes?= , Geert Uytterhoeven Subject: Re: [PATCH v6 22/23] drm/vc4: vec: Add support for more analog TV standards Message-ID: <20221027115822.5vd3fqlcpy4gfq5v@houat> References: <20220728-rpi-analog-tv-properties-v6-0-e7792734108f@cerno.tech> <20220728-rpi-analog-tv-properties-v6-22-e7792734108f@cerno.tech> MIME-Version: 1.0 In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221027_045833_894983_36EEEC9F X-CRM114-Status: GOOD ( 28.77 ) 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: , Content-Type: multipart/mixed; boundary="===============0028534935366774540==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============0028534935366774540== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ysfbxxbxsmqctfb3" Content-Disposition: inline --ysfbxxbxsmqctfb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Thu, Oct 27, 2022 at 12:32:50AM +0200, Mateusz Kwiatkowski wrote: > I've seen that you've incorporated my PAL60 patch. Thanks! >=20 > I still yet need to test your v6 changes, but looking at this code with j= ust my > mental static analysis, it seems to me that the vc4_vec_encoder_atomic_ch= eck() > should have the tv_mode validation. I should've added it to the PAL60 pat= ch, > but it somehow slipped my mind then. >=20 > Anyway, I mentioned it previously here: > https://lore.kernel.org/dri-devel/0f2beec2-ae8e-5579-f0b6-a73d9dae1af4@gm= ail.com/ >=20 > It would look something like this, inside vc4_vec_encoder_atomic_check(): >=20 > + const struct vc4_vec_tv_mode *tv_mode =3D > + vc4_vec_tv_mode_lookup(conn_state->tv.mode); > + > + if (!tv_mode) > + return -EINVAL; >=20 > Without this, it's possible to set e.g. 480i mode and SECAM, which will f= ail - > but with the current version it will only fail in vc4_vec_encoder_enable(= ), > which cannot return an error, and in my experience that causes a rather l= engthy > lockup. ACK, I'll add it. > But, like I said, I still need to actually test that with this version. > > Anyway, I was also thinking about adding support for the more "exotic" > non-standard modes. NTSC-50 is, unfortunately, impossible with VEC, but > PAL-N-60 and PAL-M-50 should work. The necessary vc4_vec_tv_modes entries= would > look something like: >=20 > @@ -325,12 +325,28 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_mode= s[] =3D { > .config0 =3D VEC_CONFIG0_PAL_M_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-M-50 */ > + .mode =3D DRM_MODE_TV_MODE_PAL, > + .expected_htotal =3D 864, > + .config0 =3D VEC_CONFIG0_PAL_BDGHI_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21e6efe3, > + }, > { > .mode =3D DRM_MODE_TV_MODE_PAL_N, > .expected_htotal =3D 864, > .config0 =3D VEC_CONFIG0_PAL_N_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-N-60 */ > + .mode =3D DRM_MODE_TV_MODE_PAL_N, > + .expected_htotal =3D 858, > + .config0 =3D VEC_CONFIG0_PAL_M_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21f69446, > + }, > { > .mode =3D DRM_MODE_TV_MODE_SECAM, > .expected_htotal =3D 864, >=20 > I'm not sure if we actually want to add that. The two arguments for doing= so > I can think of is 1. it should work, so "why not", 2. it means that more = modes > will result in _some_ kind of a valid signal, rather than erroring out, w= hich > is always a plus in my book. I can also think of a hypothetical use case,= like > someone in South America with an old PAL-N-only set that would neverthele= ss > still sync at 60 Hz (perhaps with the help of messing with vertical hold = knob), > who would like to play retro games at 60 Hz in color. >=20 > But on the other hand, I admit that this scenario is likely a stretch and= the > number of people who would actually use it is probably close to the prove= rbial > two ;) So it's your call, I'm just leaving those settings here just in ca= se. This series is already pretty massive and is difficult to merge, so I'd rather avoid to add new stuff in at every version. The changes look easy enough to be a follow-up patch, so I'd prefer to do it that way. Maxime --ysfbxxbxsmqctfb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCY1pyXgAKCRDj7w1vZxhR xUhMAP0XpvpKwMyQUT0qv5zm8wU1Oj1FqnHhCXeuVVO8jcNQEQEA3oyfkhXsxwGB 0XwSRrhGDf0kHcwS9XXqBCQFEgMYgww= =04yJ -----END PGP SIGNATURE----- --ysfbxxbxsmqctfb3-- --===============0028534935366774540== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============0028534935366774540==-- 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 D08A0FA3743 for ; Thu, 27 Oct 2022 11:58:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 676E710E5D3; Thu, 27 Oct 2022 11:58:38 +0000 (UTC) Received: from wnew1-smtp.messagingengine.com (wnew1-smtp.messagingengine.com [64.147.123.26]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1FE3D10E06E; Thu, 27 Oct 2022 11:58:33 +0000 (UTC) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.west.internal (Postfix) with ESMTP id 9AD912B066FE; Thu, 27 Oct 2022 07:58:27 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 27 Oct 2022 07:58:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm3; t=1666871907; x=1666879107; bh=QyApW52GGW OGnMQtvgaWWVm3IOEOZXDwE9lLItGdiLY=; b=uF//YRP5AA3BkLmQtb+sTcWpgH hk2HTy6BCO6QZu5obOUiI2weB0GqkqU7EYohbQTg1XdfrOGCYkVJi+p3vOUopU6I JTXI5dP21OTXqJRlGU/wvVC5PBPw4hYaL2AFhxYgnFTDv36LLn4rA7ytqFXb3zAk bt+FmImAVzuTI2sHRcVQRCv/XjSppxGquUVmshtTICQWRRE3m9az2fp8SqKScgPK TUaI0dfU29NYrX6H/9nIbVVx2ljADhJWJ5B6VvyFLDHVXCDe2q5n3d2htKFfJmAI ItI5qfAu65peUdI4fxjyHl+8YesnAPhZ0KQiGuVQ6WsmAbm8bItIXBigIHYA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1666871907; x=1666879107; bh=QyApW52GGWOGnMQtvgaWWVm3IOEO ZXDwE9lLItGdiLY=; b=X5KOnyeXpFpd9UUPBhTooylt8Bn5VnVl9lyj1OWBqdOT 3eMOeTk76m/uJpIUSNpx/gKUJuCsu4oMVW+2w5uu53DfcAi2dyotu5NiSwMA7zF2 kDpC9DrK+wT9QK/xPGF7J879cPUWSeP+ovy4NIXMMqQrF0uuI+vuw03DlVAz7TQS pt3dbB5A7FyhUe6nG3yGWQ9spnihyotU/dXSS9/Fo8475Urre+4o7TjnCZnzYvrs r73sucWl8RuMrSovYvdUOJfNWjeLek2qCEnfjkR9btgZ9nKZXlS5g5npQPNqoM5K h/RVHWs+mq0A+Pr7hQTRzfRHGu5MGs5r3H83s45SOQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrtdeggdeggecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnheptdelkeejiedufedvhfeiffetlefhiedvleeigfeiuefghfdvleeiffejieeg teejnecuffhomhgrihhnpehkvghrnhgvlhdrohhrghenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Feedback-ID: i8771445c:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 27 Oct 2022 07:58:24 -0400 (EDT) Date: Thu, 27 Oct 2022 13:58:22 +0200 From: Maxime Ripard To: Mateusz Kwiatkowski Subject: Re: [PATCH v6 22/23] drm/vc4: vec: Add support for more analog TV standards Message-ID: <20221027115822.5vd3fqlcpy4gfq5v@houat> References: <20220728-rpi-analog-tv-properties-v6-0-e7792734108f@cerno.tech> <20220728-rpi-analog-tv-properties-v6-22-e7792734108f@cerno.tech> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ysfbxxbxsmqctfb3" Content-Disposition: inline In-Reply-To: X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Karol Herbst , David Airlie , nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org, Phil Elwell , Emma Anholt , Samuel Holland , Jernej Skrabec , Chen-Yu Tsai , Geert Uytterhoeven , Ben Skeggs , linux-sunxi@lists.linux.dev, intel-gfx@lists.freedesktop.org, Hans de Goede , Rodrigo Vivi , linux-arm-kernel@lists.infradead.org, Tvrtko Ursulin , Dom Cobley , Dave Stevenson , linux-kernel@vger.kernel.org, Noralf =?utf-8?Q?Tr=C3=B8nnes?= , Thomas Zimmermann Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" --ysfbxxbxsmqctfb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Thu, Oct 27, 2022 at 12:32:50AM +0200, Mateusz Kwiatkowski wrote: > I've seen that you've incorporated my PAL60 patch. Thanks! >=20 > I still yet need to test your v6 changes, but looking at this code with j= ust my > mental static analysis, it seems to me that the vc4_vec_encoder_atomic_ch= eck() > should have the tv_mode validation. I should've added it to the PAL60 pat= ch, > but it somehow slipped my mind then. >=20 > Anyway, I mentioned it previously here: > https://lore.kernel.org/dri-devel/0f2beec2-ae8e-5579-f0b6-a73d9dae1af4@gm= ail.com/ >=20 > It would look something like this, inside vc4_vec_encoder_atomic_check(): >=20 > + const struct vc4_vec_tv_mode *tv_mode =3D > + vc4_vec_tv_mode_lookup(conn_state->tv.mode); > + > + if (!tv_mode) > + return -EINVAL; >=20 > Without this, it's possible to set e.g. 480i mode and SECAM, which will f= ail - > but with the current version it will only fail in vc4_vec_encoder_enable(= ), > which cannot return an error, and in my experience that causes a rather l= engthy > lockup. ACK, I'll add it. > But, like I said, I still need to actually test that with this version. > > Anyway, I was also thinking about adding support for the more "exotic" > non-standard modes. NTSC-50 is, unfortunately, impossible with VEC, but > PAL-N-60 and PAL-M-50 should work. The necessary vc4_vec_tv_modes entries= would > look something like: >=20 > @@ -325,12 +325,28 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_mode= s[] =3D { > .config0 =3D VEC_CONFIG0_PAL_M_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-M-50 */ > + .mode =3D DRM_MODE_TV_MODE_PAL, > + .expected_htotal =3D 864, > + .config0 =3D VEC_CONFIG0_PAL_BDGHI_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21e6efe3, > + }, > { > .mode =3D DRM_MODE_TV_MODE_PAL_N, > .expected_htotal =3D 864, > .config0 =3D VEC_CONFIG0_PAL_N_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-N-60 */ > + .mode =3D DRM_MODE_TV_MODE_PAL_N, > + .expected_htotal =3D 858, > + .config0 =3D VEC_CONFIG0_PAL_M_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21f69446, > + }, > { > .mode =3D DRM_MODE_TV_MODE_SECAM, > .expected_htotal =3D 864, >=20 > I'm not sure if we actually want to add that. The two arguments for doing= so > I can think of is 1. it should work, so "why not", 2. it means that more = modes > will result in _some_ kind of a valid signal, rather than erroring out, w= hich > is always a plus in my book. I can also think of a hypothetical use case,= like > someone in South America with an old PAL-N-only set that would neverthele= ss > still sync at 60 Hz (perhaps with the help of messing with vertical hold = knob), > who would like to play retro games at 60 Hz in color. >=20 > But on the other hand, I admit that this scenario is likely a stretch and= the > number of people who would actually use it is probably close to the prove= rbial > two ;) So it's your call, I'm just leaving those settings here just in ca= se. This series is already pretty massive and is difficult to merge, so I'd rather avoid to add new stuff in at every version. The changes look easy enough to be a follow-up patch, so I'd prefer to do it that way. Maxime --ysfbxxbxsmqctfb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCY1pyXgAKCRDj7w1vZxhR xUhMAP0XpvpKwMyQUT0qv5zm8wU1Oj1FqnHhCXeuVVO8jcNQEQEA3oyfkhXsxwGB 0XwSRrhGDf0kHcwS9XXqBCQFEgMYgww= =04yJ -----END PGP SIGNATURE----- --ysfbxxbxsmqctfb3-- 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 333B2ECAAA1 for ; Thu, 27 Oct 2022 11:58:46 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 44E9210E5D8; Thu, 27 Oct 2022 11:58:39 +0000 (UTC) Received: from wnew1-smtp.messagingengine.com (wnew1-smtp.messagingengine.com [64.147.123.26]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1FE3D10E06E; Thu, 27 Oct 2022 11:58:33 +0000 (UTC) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.west.internal (Postfix) with ESMTP id 9AD912B066FE; Thu, 27 Oct 2022 07:58:27 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 27 Oct 2022 07:58:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm3; t=1666871907; x=1666879107; bh=QyApW52GGW OGnMQtvgaWWVm3IOEOZXDwE9lLItGdiLY=; b=uF//YRP5AA3BkLmQtb+sTcWpgH hk2HTy6BCO6QZu5obOUiI2weB0GqkqU7EYohbQTg1XdfrOGCYkVJi+p3vOUopU6I JTXI5dP21OTXqJRlGU/wvVC5PBPw4hYaL2AFhxYgnFTDv36LLn4rA7ytqFXb3zAk bt+FmImAVzuTI2sHRcVQRCv/XjSppxGquUVmshtTICQWRRE3m9az2fp8SqKScgPK TUaI0dfU29NYrX6H/9nIbVVx2ljADhJWJ5B6VvyFLDHVXCDe2q5n3d2htKFfJmAI ItI5qfAu65peUdI4fxjyHl+8YesnAPhZ0KQiGuVQ6WsmAbm8bItIXBigIHYA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1666871907; x=1666879107; bh=QyApW52GGWOGnMQtvgaWWVm3IOEO ZXDwE9lLItGdiLY=; b=X5KOnyeXpFpd9UUPBhTooylt8Bn5VnVl9lyj1OWBqdOT 3eMOeTk76m/uJpIUSNpx/gKUJuCsu4oMVW+2w5uu53DfcAi2dyotu5NiSwMA7zF2 kDpC9DrK+wT9QK/xPGF7J879cPUWSeP+ovy4NIXMMqQrF0uuI+vuw03DlVAz7TQS pt3dbB5A7FyhUe6nG3yGWQ9spnihyotU/dXSS9/Fo8475Urre+4o7TjnCZnzYvrs r73sucWl8RuMrSovYvdUOJfNWjeLek2qCEnfjkR9btgZ9nKZXlS5g5npQPNqoM5K h/RVHWs+mq0A+Pr7hQTRzfRHGu5MGs5r3H83s45SOQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrtdeggdeggecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnheptdelkeejiedufedvhfeiffetlefhiedvleeigfeiuefghfdvleeiffejieeg teejnecuffhomhgrihhnpehkvghrnhgvlhdrohhrghenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Feedback-ID: i8771445c:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 27 Oct 2022 07:58:24 -0400 (EDT) Date: Thu, 27 Oct 2022 13:58:22 +0200 From: Maxime Ripard To: Mateusz Kwiatkowski Message-ID: <20221027115822.5vd3fqlcpy4gfq5v@houat> References: <20220728-rpi-analog-tv-properties-v6-0-e7792734108f@cerno.tech> <20220728-rpi-analog-tv-properties-v6-22-e7792734108f@cerno.tech> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ysfbxxbxsmqctfb3" Content-Disposition: inline In-Reply-To: Subject: Re: [Intel-gfx] [PATCH v6 22/23] drm/vc4: vec: Add support for more analog TV standards X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Karol Herbst , David Airlie , nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org, Phil Elwell , Emma Anholt , Samuel Holland , Jernej Skrabec , Chen-Yu Tsai , Geert Uytterhoeven , Ben Skeggs , linux-sunxi@lists.linux.dev, Daniel Vetter , intel-gfx@lists.freedesktop.org, Hans de Goede , Rodrigo Vivi , linux-arm-kernel@lists.infradead.org, Dom Cobley , Dave Stevenson , linux-kernel@vger.kernel.org, Noralf =?utf-8?Q?Tr=C3=B8nnes?= , Thomas Zimmermann Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" --ysfbxxbxsmqctfb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Thu, Oct 27, 2022 at 12:32:50AM +0200, Mateusz Kwiatkowski wrote: > I've seen that you've incorporated my PAL60 patch. Thanks! >=20 > I still yet need to test your v6 changes, but looking at this code with j= ust my > mental static analysis, it seems to me that the vc4_vec_encoder_atomic_ch= eck() > should have the tv_mode validation. I should've added it to the PAL60 pat= ch, > but it somehow slipped my mind then. >=20 > Anyway, I mentioned it previously here: > https://lore.kernel.org/dri-devel/0f2beec2-ae8e-5579-f0b6-a73d9dae1af4@gm= ail.com/ >=20 > It would look something like this, inside vc4_vec_encoder_atomic_check(): >=20 > + const struct vc4_vec_tv_mode *tv_mode =3D > + vc4_vec_tv_mode_lookup(conn_state->tv.mode); > + > + if (!tv_mode) > + return -EINVAL; >=20 > Without this, it's possible to set e.g. 480i mode and SECAM, which will f= ail - > but with the current version it will only fail in vc4_vec_encoder_enable(= ), > which cannot return an error, and in my experience that causes a rather l= engthy > lockup. ACK, I'll add it. > But, like I said, I still need to actually test that with this version. > > Anyway, I was also thinking about adding support for the more "exotic" > non-standard modes. NTSC-50 is, unfortunately, impossible with VEC, but > PAL-N-60 and PAL-M-50 should work. The necessary vc4_vec_tv_modes entries= would > look something like: >=20 > @@ -325,12 +325,28 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_mode= s[] =3D { > .config0 =3D VEC_CONFIG0_PAL_M_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-M-50 */ > + .mode =3D DRM_MODE_TV_MODE_PAL, > + .expected_htotal =3D 864, > + .config0 =3D VEC_CONFIG0_PAL_BDGHI_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21e6efe3, > + }, > { > .mode =3D DRM_MODE_TV_MODE_PAL_N, > .expected_htotal =3D 864, > .config0 =3D VEC_CONFIG0_PAL_N_STD, > .config1 =3D VEC_CONFIG1_C_CVBS_CVBS, > }, > + { > + /* PAL-N-60 */ > + .mode =3D DRM_MODE_TV_MODE_PAL_N, > + .expected_htotal =3D 858, > + .config0 =3D VEC_CONFIG0_PAL_M_STD, > + .config1 =3D VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, > + .custom_freq =3D 0x21f69446, > + }, > { > .mode =3D DRM_MODE_TV_MODE_SECAM, > .expected_htotal =3D 864, >=20 > I'm not sure if we actually want to add that. The two arguments for doing= so > I can think of is 1. it should work, so "why not", 2. it means that more = modes > will result in _some_ kind of a valid signal, rather than erroring out, w= hich > is always a plus in my book. I can also think of a hypothetical use case,= like > someone in South America with an old PAL-N-only set that would neverthele= ss > still sync at 60 Hz (perhaps with the help of messing with vertical hold = knob), > who would like to play retro games at 60 Hz in color. >=20 > But on the other hand, I admit that this scenario is likely a stretch and= the > number of people who would actually use it is probably close to the prove= rbial > two ;) So it's your call, I'm just leaving those settings here just in ca= se. This series is already pretty massive and is difficult to merge, so I'd rather avoid to add new stuff in at every version. The changes look easy enough to be a follow-up patch, so I'd prefer to do it that way. Maxime --ysfbxxbxsmqctfb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCY1pyXgAKCRDj7w1vZxhR xUhMAP0XpvpKwMyQUT0qv5zm8wU1Oj1FqnHhCXeuVVO8jcNQEQEA3oyfkhXsxwGB 0XwSRrhGDf0kHcwS9XXqBCQFEgMYgww= =04yJ -----END PGP SIGNATURE----- --ysfbxxbxsmqctfb3--