From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.domeneshop.no (smtp.domeneshop.no [194.63.252.55]) (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 7860B10F7 for ; Mon, 17 Oct 2022 10:21:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tronnes.org ; s=ds202112; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Y4j7gBOaq+2LeHWeErehnoB7+9tH84EPBKVdDAWDliE=; b=AoVlrX2Ik0FBhZTaqHAgAgXDq3 sKgykY/zMoWZvYv/wwlTjx33qQU6S8eKEpaorbc+ZIhkKjRpUwTgiBYGb6GkwlSu4Wf+spNboAz/N 1Q9Dn9Hw7vEb7gaGtICHUG1djD+wwVRu5sx2XYOn1bKxdAxEnkXah3X9mi1EmN69vpTES5V5h/np9 deEPLmHaxO1ek7B34SPfN4PmegGOTgOOPdlNFiiADkTT593XYyHN1rj/FmfJbUK8F0ZanLdzV6zOe s46j/eFnXBiSwk6jlRYsduk/ylik9IZzGhZFnxWyZzcYnLDDP9Vk2J6ri4eobHTZgRvvlWgLKtxwV 7DsvpzmQ==; Received: from 211.81-166-168.customer.lyse.net ([81.166.168.211]:59672 helo=[192.168.10.61]) by smtp.domeneshop.no with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1okNFL-0001vj-HG; Mon, 17 Oct 2022 12:21:23 +0200 Message-ID: <769d2f6e-7fe6-30da-06d8-3c2e9fb9df34@tronnes.org> Date: Mon, 17 Oct 2022 12:21:14 +0200 Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.3.3 Subject: Re: [PATCH v5 13/22] drm/modes: Introduce the tv_mode property as a command-line option To: kfyatek+publicgit@gmail.com, Maxime Ripard , Karol Herbst , Jani Nikula , Tvrtko Ursulin , Daniel Vetter , Maarten Lankhorst , David Airlie , Joonas Lahtinen , Lyude Paul , Maxime Ripard , Emma Anholt , Chen-Yu Tsai , Samuel Holland , Ben Skeggs , Thomas Zimmermann , Rodrigo Vivi , Jernej Skrabec Cc: Dom Cobley , linux-sunxi@lists.linux.dev, Dave Stevenson , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, nouveau@lists.freedesktop.org, Geert Uytterhoeven , linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org, Hans de Goede , Phil Elwell , =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= References: <20220728-rpi-analog-tv-properties-v5-0-d841cc64fe4b@cerno.tech> <20220728-rpi-analog-tv-properties-v5-13-d841cc64fe4b@cerno.tech> From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Den 16.10.2022 19.51, skrev Mateusz Kwiatkowski: > Hi Maxime, Noralf & everyone, > > I'd like to address Noralf here in particular, and refer to these discussions > from the past: > > - https://lore.kernel.org/linux-arm-kernel/2f607c7d-6da1-c8df-1c02-8dd344a92343@gmail.com/ > - https://lore.kernel.org/linux-arm-kernel/9e76a508-f469-a54d-ecd7-b5868ca99af4@tronnes.org/ > >> @@ -2230,20 +2256,22 @@ struct drm_named_mode { >> unsigned int xres; >> unsigned int yres; >> unsigned int flags; >> + unsigned int tv_mode; >> }; > > I saw that you (Noralf) opposed my suggestion about the DRM_MODE_TV_MODE_NONE > enum value in enum drm drm_connector_tv_mode. I get your argumentation, and I'm > not gonna argue, but I still don't like the fact that struct drm_named_mode now > includes a field that is only relevant for analog TV modes, has no "none" value, > and yet the type is supposed to be generic enough to be usable for other types > of outputs as well. > > It's true that it can just be ignored (as Maxime mentioned in his response to > my e-mail linked above), and now the value of 0 corresponds to > DRM_MODE_TV_MODE_NTSC, which is a rather sane default, but it still feels messy > to me. > > I'm not gonna force my opinion here, but I wanted to bring your attention to > this issue, maybe you have some other solution in mind for this problem. Or if > you don't see that as a problem at all, that's fine, too. > I hadn't looked at this patch in detail before, but you're right this, together with drm_atomic_helper_connector_tv_reset(), will overwrite tv.mode unconditionally regardless of tv_mode being present in video= or not. We need a tv_mode_specified flag like we have for bpp and refresh. Noralf.