From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 933F944A403 for ; Thu, 13 Aug 2026 10:13:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786616032; cv=none; b=aTiabywOGoB+HSRsGropk1swAjoUuskc3GAXYt583x7oFFdsNbMOjVhbP2NGCToFQVSFHDrIWBs3hwAyi4FVoBw4RsDTFYyrWMeUc6KWp7/au1/wAlg5OpzPbaAROYZyWU7h1KSBrailODVbhk/JDoLXBm2R20iD9MsJnzxOqCI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786616032; c=relaxed/simple; bh=MKERt3x2NRVCX4spT42n6ewQANE3be4K+htRTaPMNHg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=l67DcJtpTRK5gThfpKG7nFxjV/i2AHDeHqexasxvEuwk4FmfA+3uITwAIM2gmsi33wLQRAuWv3GrG9sVgmvrBmh9HG4JV/o5iTiL2Y5LvmAqd8yYB4D2UNiHP0XIrKnDL0hqGrgsd9cSbJ7PRSAiGOGQy0cu91aV4haCRf5ZuCg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=pvpmJlI4; arc=none smtp.client-ip=209.85.221.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="pvpmJlI4" Received: by mail-wr1-f49.google.com with SMTP id ffacd0b85a97d-47c2ae992beso143724f8f.2 for ; Thu, 13 Aug 2026 03:13:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786616023; x=1787220823; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to:content-type; bh=OvZxXgWwlw9mSCW44p+jCE5q4lBZ+vVfSoXZPbAhS0M=; b=pvpmJlI4qMl8If5+2L7U29hamTXWe5D7mVvdBZgRpDMa/Zu4gZhz3sH4R2HLGQZmv8 4xx8rEPCX2WmdWuNu7lN20FNx5X8jdDFTKn4awAHYHmnr5EXXN6knDATMtmBqVSqv31f SfhKai6sS4csozvMYr592TqSvBttI6UfI9E/PfHLJGZ+VbPUX2ciTZhSoZ34hFFFVBeV norhdq7rk7y5FMCkNzEVv+4Qy3Ec8333qflC8qtrqjLUfhN512EYBukgDAEyrUf91e/e FzfukcKU9uyu8UfOTfXYP5eOAUHXpGLgucE+0FKM8gPsxQftMDlOTuTZBcbJ78qnK5me G8EQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786616023; x=1787220823; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=OvZxXgWwlw9mSCW44p+jCE5q4lBZ+vVfSoXZPbAhS0M=; b=gZFZDype047bTGecVJfryl6grfCPuc0A3q4LaEeLWoxEhSERrldZ67xF7IcTjLnnpm zQeIg4KT10j7/0P9bmOOsGZZXpKzQyH9ckV0awDPBG/HFQZIlWIH/JqxqGwE4ZEXOEQx 5TZ7kpf94NX0zg98+YOW6Mr6Y2Pwt/T3wWu0AGNqxdR+6rNecNJmuspFpdnXyzzdd458 2iSOeOBKiE06OwguWEFFVp1ZBTBlnAOv9y9YwOtRW6DCRv3qy6aOJ24EPToWO02hD+d9 K8cQRKlRMOcxNGAprxbfdNrjijSCR9LDTrzfTP4TAogDwL3iLzZKvOCB1FuEq5p0fC1N wlcQ== X-Forwarded-Encrypted: i=1; AHgh+Rp9WaKGaFIQpYKmhResnZmABBfXcUT6XWgjqnQ9IiQYHiFWBwkJIxWw8oIOlMYgUMij9l1RV37nB5datMY=@vger.kernel.org X-Gm-Message-State: AOJu0Yw9B2w3/bzkREhKqtZo1zSh0ePdomrVY70p8G9EKpgW1hEaHueG 5tU1okC5I23DXsUcbmx6TfwKhw6wWX51PZ0nVpn9xqmBaHQU8NWPKY7m5GH4weQk X-Gm-Gg: AR+sD110anq298APm+1LFsRMfu1xPGX0Yfcqh5bRACiMWwrdi7OI9HKaDsC4zA/aVJ8 ehjOZP4k+o8jhTh/fjHbJuN7xwtkP9L21CiFnnqXEQD87ml65kUykQBrAlO3i4vzs0Jn0oIHHSD qUjw89JsJ9h8clYSJOktddTGEtYBBrNpo1YxFA6QirUTjdDIGo6iROc0II7lTIVa7MwKTSUl0nB R7sdI9yRfTzg6U46FzQYT22TjbeBa0BfA+UkN3Fi499S/GTlSsNp5XAmG1cYgnPPzMB8fsAl68A 9XkWoI0gu5KOK7ULiOJuIP7zu0MSJZztzUo43PsWW2NMCJrXZCx2PJb7AamlottF1VnFL+AhW1J 2pdQ1oceAadDMmapYdabmQKvwybCdavYRo7fGgdgwE+VNkajhFu+kcQ1dLOGprOztyS4w0Af0FU qOChxgxbaoUrcKB0NCiiwoc8gWOZhopJAhtAXlTxxmMjDzVepCZ1BhyAshkkPBi9e2TBin+JqQm d/dMUUuruaDBWgir+7PIpuO6B11o2B/bvs6rTwGDmEAA3grIvGWOyGJ91LD8s130IRUe/xZW9JU NMkx X-Received: by 2002:a05:600c:b93:b0:499:59da:12b9 with SMTP id 5b1f17b1804b1-499825cefdcmr25741845e9.0.1786616022989; Thu, 13 Aug 2026 03:13:42 -0700 (PDT) Received: from OrangePi5-Plus.BB-HOME (20014C4E1B911C009377EE8DF6D0A1C2.dsl.pool.telekom.hu. [2001:4c4e:1b91:1c00:9377:ee8d:f6d0:a1c2]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49981abd6desm59541945e9.0.2026.08.13.03.13.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Aug 2026 03:13:42 -0700 (PDT) From: Igor Paunovic To: Sandy Huang , Heiko Stuebner , Andy Yan Cc: Igor Paunovic , Cristian Ciocaltea , Sebastian Reichel , Chaoyi Chen , Alexey Charkov , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] drm/rockchip: vop2: Scale the AXI clock to the bandwidth the mode needs Date: Thu, 13 Aug 2026 12:13:00 +0200 Message-ID: <20260813101307.10945-1-royalnet026@gmail.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260813100027.349761F000E9@smtp.kernel.org> References: <20260813094614.9072-1-royalnet026@gmail.com> <20260813100027.349761F000E9@smtp.kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review bot found three things on v2 and all three are right. I am answering rather than sending a v3 straight away, because the fix for the two High ones is a single change that touches a file shared by every Rockchip SoC, and I would rather ask about that than guess. Both High findings come from the same shortcut. v2 keeps the requirement in a global atomic state object, which I still think is the right container, but it applies the rate from vop2_crtc_atomic_enable() and _disable() rather than from the commit tail: - Out of order commits. Two non-blocking commits on different CRTCs share only the private object, and nothing orders them, so a commit that took its snapshot before another CRTC raised the rate can land after it and lower it again. - Multi-CRTC disable. atomic_disable() runs once per CRTC, and the first one already sees a state in which every participating CRTC is off, so the rate drops while the others are still scanning out and waiting for dsp_hold_completion. vc4 solves both of these for its core clock, and what I did was take half of that pattern instead of all of it: - vc4_atomic_commit_setup() records a pending commit per channel in the private state and the next commit waits on it with drm_crtc_commit_wait(). That is the ordering v2 has no equivalent of. - vc4_atomic_commit_tail() holds max(old, new) for the length of the commit and only drops to the new rate after drm_atomic_helper_wait_for_flip_done(). That is exactly the window the second finding describes. Hence the question. Doing the same in rockchip means adding both .atomic_commit_setup and .atomic_commit_tail to rockchip_mode_config_helpers in rockchip_drm_fb.c, which today carries only .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm and is shared by every SoC this driver supports, VOP as well as VOP2. The commit tail would be a thin wrapper around the rpm helper with the clock work on either side of it, and both hooks would do nothing on anything that is not RK3588. Is that acceptable, or would you rather this stayed inside vop2 in some other shape? I am happy to write it either way, but I would rather find that out before than after. The Medium finding needs no discussion: if drm_atomic_private_obj_init() fails, the jump to err_crtcs does not undo rockchip_rgb_init(). It is also new in this patch, since before it nothing after rockchip_rgb_init() could fail, so it is mine and it will be fixed in the next version whatever shape the rest takes. Igor