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 B536DC4332F for ; Sun, 16 Oct 2022 18:54:04 +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-Transfer-Encoding:Content-Type:Reply-To:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:References:Cc:To: Subject:MIME-Version:Date:Message-ID:From:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=9UB88Gh9M2z+6JBqxYRjrCQRu96blWUHe+kFH77Cw4c=; b=wlSVkk2UseUG7Y pPIoiML1gkCVz2agWPlIoaYyw+bXgAzNxh/cz56jUkz+HkVeFZsdMnHWFSQu0k1aXWoSwKIKEazkQ pn7SYaOuuC56Lhep0hpd3BC3LSehhtS2QopZ2yBsHGFSxPU60tCD8cBwvuaiARkzxBW4JA9X939LE /S8KlJZ7wyQ//ay333KR9G0VNr6sj9QwWrNVM6yL/oagAlLZpLSQOrA4OyIGfx9cV95HsY7cVvFzv skWbHfNs42VGNOqSkL/SK6gXfUvZ4VYqVwoGBVGjA5A0PjQmmsYMiyBAMGsuT1l3EgkyR/Mdat49f Xy/a7RcH36QAw0FXw7DQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ok8kX-003E1V-VE; Sun, 16 Oct 2022 18:52:38 +0000 Received: from mail-lj1-x231.google.com ([2a00:1450:4864:20::231]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ok8kU-003Dwu-SM for linux-arm-kernel@lists.infradead.org; Sun, 16 Oct 2022 18:52:36 +0000 Received: by mail-lj1-x231.google.com with SMTP id b18so11604182ljr.13 for ; Sun, 16 Oct 2022 11:52:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:reply-to:subject:user-agent:mime-version:date:message-id:from :from:to:cc:subject:date:message-id:reply-to; bh=L86zD4BE93sh72AeYReVIs01pAGXwZm6n1nhGapyqcU=; b=jkI8DFEtgd0iCPaVQhi7Teq9cyV9bjBA6KsEnb7GUTna6b0BCJ/q3e9KvJTJf+/pgH bOyBG8HxOgeA28keCz7WDK6bQaw7GOkXH38myfIhMm2tpSxgrZdyI6XUM9ar6yQrC5Ms sCyq/IboT6MblZ6+kbcRoePAU15ZfWyjW9NaE7+PyxSjlHZmcd0O57VpL4B6zvy9BJQ4 yDvNX8fmFfeQ+q4Z+cyH8RJrVTMfh4jdZXQoTrGFsps4dRmokcopF3y1cpyx6H1ooDEE 2UdfpED3Y926o9lCJrnqalYZX9xpISArgJbvfGj4LmoZspT7d+8owSUqeiZ4q0h0HBmW Cgcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:reply-to:subject:user-agent:mime-version:date:message-id:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=L86zD4BE93sh72AeYReVIs01pAGXwZm6n1nhGapyqcU=; b=0HokhIpeDt1rgkbrxTGgXY9pDYMUuB423WasTWVsmIWPFjFbx21ufLk6vyZ/pS0D5f ZxJkYujPWGpWDs48CgKJ3x5ElgWQ24EZS3hXVkRGGis0dhRZCqZUhxUmqTNYJDKa/NKx AS0rcPQ4XrBLVUsP81i+7QsrL59bXcJnxPM1acTAo3u7MqDAHPY91rBXwFBLRwH4DAfZ r8v0HINSIfSuOOY1RbzaVCnf8OPXPWiYZXr4R7V1B6YJ6jFwXjDG255FtfkqItlt1le1 Z/aWucIr9x8JW1sCi77B7xNQ5z4LuUaiCUO6pUX3x3Ivyvq+2VjScWq4qXgNxOiClC1P ydGA== X-Gm-Message-State: ACrzQf28yQFh1m6ciBBTlYE7H7sHAI84Ua8gji2Xrh4gdgOiFXWCnZFu 9nelaqBa07maR9DLQRpu6sY= X-Google-Smtp-Source: AMsMyM7Jz5dVg4MMw9epFOAljLEqm7fvWmI8tG8aPvCSkAkxqmjN6SH6pvd/9hhDRCoi+A0Wf3pFPw== X-Received: by 2002:a2e:2e05:0:b0:26f:c234:3335 with SMTP id u5-20020a2e2e05000000b0026fc2343335mr2768156lju.76.1665946349247; Sun, 16 Oct 2022 11:52:29 -0700 (PDT) Received: from ?IPV6:2a02:a31a:a240:1700:64bb:87df:aad7:a9f0? ([2a02:a31a:a240:1700:64bb:87df:aad7:a9f0]) by smtp.googlemail.com with ESMTPSA id q8-20020a056512210800b0049d3614463dsm1143112lfr.77.2022.10.16.11.52.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 16 Oct 2022 11:52:28 -0700 (PDT) From: Mateusz Kwiatkowski X-Google-Original-From: Mateusz Kwiatkowski Message-ID: Date: Sun, 16 Oct 2022 20:52:26 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.3.3 Subject: Re: [PATCH v5 20/22] drm/vc4: vec: Convert to the new TV mode property To: 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 , =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= , 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 References: <20220728-rpi-analog-tv-properties-v5-0-d841cc64fe4b@cerno.tech> <20220728-rpi-analog-tv-properties-v5-20-d841cc64fe4b@cerno.tech> Content-Language: pl In-Reply-To: <20220728-rpi-analog-tv-properties-v5-20-d841cc64fe4b@cerno.tech> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221016_115234_988296_1381E257 X-CRM114-Status: GOOD ( 22.00 ) 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: , Reply-To: kfyatek+publicgit@gmail.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Maxime, > static int vc4_vec_connector_get_modes(struct drm_connector *connector) > { > - struct drm_connector_state *state = connector->state; > struct drm_display_mode *mode; > > - mode = drm_mode_duplicate(connector->dev, > - vc4_vec_tv_modes[state->tv.legacy_mode].mode); > + mode = drm_mode_analog_ntsc_480i(connector->dev); > if (!mode) { > DRM_ERROR("Failed to create a new display mode\n"); > return -ENOMEM; > } > > + mode->type |= DRM_MODE_TYPE_PREFERRED; > drm_mode_probed_add(connector, mode); > > - return 1; > + mode = drm_mode_analog_pal_576i(connector->dev); > + if (!mode) { > + DRM_ERROR("Failed to create a new display mode\n"); > + return -ENOMEM; > + } > + > + drm_mode_probed_add(connector, mode); > + > + return 2; > +} Referencing those previous discussions: - https://lore.kernel.org/dri-devel/0255f7c6-0484-6456-350d-cf24f3fee5d6@tronnes.org/ - https://lore.kernel.org/dri-devel/c8f8015a-75da-afa8-ca7f-b2b134cacd16@gmail.com/ Unconditionally setting the 480i mode as DRM_MODE_TYPE_PREFERRED causes Xorg (at least on current Raspberry Pi OS) to display garbage when video=Composite1:PAL is specified on the command line, so I'm afraid this won't do. As I see it, there are three viable solutions for this issue: a) Somehow query the video= command line option from this function, and set DRM_MODE_TYPE_PREFERRED appropriately. This would break the abstraction provided by global DRM code, but should work fine. b) Modify drm_helper_probe_add_cmdline_mode() so that it sets DRM_MODE_TYPE_PREFERRED in addition to DRM_MODE_TYPE_USERDEF. This seems pretty robust, but affects the entire DRM subsystem, which may break userspace in different ways. - Maybe this could be mitigated by adding some additional conditions, e.g. setting the PREFERRED flag only if no modes are already flagged as such and/or only if the cmdline mode is a named one (~= analog TV mode) c) Forcing userspace (Xorg / Raspberry Pi OS) to get fixed and honor the USERDEF flag. Either way, hardcoding 480i as PREFERRED does not seem right. Note: this also applies to the sun4i version (patch 22/22). > @@ -366,13 +472,16 @@ static void vc4_vec_encoder_enable(struct drm_encoder *encoder, > struct drm_connector *connector = &vec->connector; > struct drm_connector_state *conn_state = > drm_atomic_get_new_connector_state(state, connector); > - const struct vc4_vec_tv_mode *tv_mode = > - &vc4_vec_tv_modes[conn_state->tv.legacy_mode]; > + const struct vc4_vec_tv_mode *tv_mode; > int idx, ret; > > if (!drm_dev_enter(drm, &idx)) > return; > > + tv_mode = vc4_vec_tv_mode_lookup(conn_state->tv.mode); > + if (!tv_mode) > + goto err_dev_exit; > + > ret = pm_runtime_get_sync(&vec->pdev->dev); > if (ret < 0) { > DRM_ERROR("Failed to retain power domain: %d\n", ret); If this (!tv_mode) condition is somehow triggered, the power management goes somewhat crazy. vc4_vec_encoder_enable() cannot return an error, so when vc4_vec_encoder_disable() is eventually called after a failed enable, it hangs in pm_runtime_put() for quite a bit. At least I think that's what's happening. Anyway, to solve this, I'd propose this thing below: Best regards, Mateusz Kwiatkowski _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel