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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3AA37C433FE for ; Sun, 6 Nov 2022 16:59:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230126AbiKFQ7d (ORCPT ); Sun, 6 Nov 2022 11:59:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36092 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230099AbiKFQ7b (ORCPT ); Sun, 6 Nov 2022 11:59:31 -0500 Received: from smtp.domeneshop.no (smtp.domeneshop.no [IPv6:2a01:5b40:0:3005::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C3DBDDEC1 for ; Sun, 6 Nov 2022 08:59:30 -0800 (PST) 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=1fHQEWgWob7nqgYb7pRzlsH6vNCFNYXfK+/x9/nB01M=; b=Yje8YQHhRml05gW1dej/dWiQQZ AqKj2i0OjO2RM1Qumjo7FoNfWRUdBC4/h2uQHli8coC+LKe7l3tUuPyfG7KqA8BEsiA+NVUYZGdcd RinbElpcQ3zEo0Ef+uc++uePxxY9puXszvLwKUx+P73tntoXkxilGwxDL1B3Co5nkaf1qLNefEJdU TbFTqkc2KcniVpob8iHII19/YNuRBUOwfVsA0XatDekK5sUsjYZNxCxDW65aky/cX6L9UkiWis18f 9uK/o/dLYZ0E8dKpnNEyK/urj4eNuLfnJnz0gFxc4QGDZbkz/3loUbjywm6lgaJZ1VZ7l87tNIpzo XoageSaA==; Received: from [2a01:799:95a:cb00:fd97:29ff:d72a:349e] (port=51993) by smtp.domeneshop.no with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1orizW-0005gi-Tq; Sun, 06 Nov 2022 17:59:26 +0100 Message-ID: Date: Sun, 6 Nov 2022 17:59:23 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH v6 16/23] drm/probe-helper: Provide a TV get_modes helper To: Mateusz Kwiatkowski , maxime@cerno.tech, 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 , Maxime Ripard , Joonas Lahtinen , Lyude Paul Cc: 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, Geert Uytterhoeven , =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= References: <20220728-rpi-analog-tv-properties-v6-0-e7792734108f@cerno.tech> <20220728-rpi-analog-tv-properties-v6-16-e7792734108f@cerno.tech> <8d0eee22-50f5-5b0a-c1e6-c5f61dd5bbcd@gmail.com> From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= In-Reply-To: <8d0eee22-50f5-5b0a-c1e6-c5f61dd5bbcd@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Den 27.10.2022 00.02, skrev Mateusz Kwiatkowski: > Hi Maxime, > > First of all, nice idea with the helper function that can be reused by different > drivers. This is neat! > > But looking at this function, it feels a bit overcomplicated. You're creating > the two modes, then checking which one is the default, then set the preferred > one and possibly reorder them. Maybe it can be simplified somehow? > > Although when I tried to refactor it myself, I ended up with something that's > not better at all. Maybe it needs to be complicated, after all :( > I also thought that the function was complicated/difficult to read, in particular the index stuff at the end, but I also failed in finding a "better" solution, just a different one ;) Noralf. My version: int drm_connector_helper_tv_get_modes(struct drm_connector *connector) { struct drm_device *dev = connector->dev; struct drm_property *tv_mode_property = dev->mode_config.tv_mode_property; struct drm_cmdline_mode *cmdline = &connector->cmdline_mode; unsigned int ntsc_modes = BIT(DRM_MODE_TV_MODE_NTSC) | BIT(DRM_MODE_TV_MODE_NTSC_443) | BIT(DRM_MODE_TV_MODE_NTSC_J) | BIT(DRM_MODE_TV_MODE_PAL_M); unsigned int pal_modes = BIT(DRM_MODE_TV_MODE_PAL) | BIT(DRM_MODE_TV_MODE_PAL_N) | BIT(DRM_MODE_TV_MODE_SECAM); unsigned int tv_modes[2] = { UINT_MAX, UINT_MAX }; unsigned int i, supported_tv_modes = 0; if (!tv_mode_property) return 0; for (i = 0; i < tv_mode_property->num_values; i++) supported_tv_modes |= BIT(tv_mode_property->values[i]); if ((supported_tv_modes & ntsc_modes) && (supported_tv_modes & pal_modes)) { uint64_t default_mode; if (drm_object_property_get_default_value(&connector->base, tv_mode_property, &default_mode)) return 0; if (cmdline->tv_mode_specified) default_mode = cmdline->tv_mode; if (BIT(default_mode) & ntsc_modes) { tv_modes[0] = DRM_MODE_TV_MODE_NTSC; tv_modes[1] = DRM_MODE_TV_MODE_PAL; } else { tv_modes[0] = DRM_MODE_TV_MODE_PAL; tv_modes[1] = DRM_MODE_TV_MODE_NTSC; } } else if (supported_tv_modes & ntsc_modes) { tv_modes[0] = DRM_MODE_TV_MODE_NTSC; } else if (supported_tv_modes & pal_modes) { tv_modes[0] = DRM_MODE_TV_MODE_PAL; } else { return 0; } for (i = 0; i < ARRAY_SIZE(tv_modes); i++) { struct drm_display_mode *mode; if (tv_modes[i] == DRM_MODE_TV_MODE_NTSC) mode = drm_mode_analog_ntsc_480i(dev); else if (tv_modes[i] == DRM_MODE_TV_MODE_PAL) mode = drm_mode_analog_pal_576i(dev); else break; if (!mode) return i; if (!i) mode->type |= DRM_MODE_TYPE_PREFERRED; drm_mode_probed_add(connector, mode); } return i; }