From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) (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 F1676258A for ; Tue, 14 Jan 2025 17:50:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736877029; cv=none; b=Z2aBQgLkVAr+IWAsyCjMLc5n7IlQsQkgJgzI9mQ8pLG2KftgiSuwBZy3CRPsYdxqgEbrMl4nOnfJTgTdTcrOeSBjyiPCga2UkY9/gM3BbE49lJw8NIW+JqnTtgJ4nBgPP33oy2TgshWc6viZM93qm4UsClLTL/ekHlhKIoxDT4c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736877029; c=relaxed/simple; bh=uzwTioenZg5xf7H0vQHKLQdODMzc7DOYT5bZwjqjYWU=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=gdKzBcXoLg8jJhyYA8iclvAnPhCW6296uqF3rDngp24mmb/J9/BHreiFzuanZrUNVgezaEWw6CK8MwTb1n7Nx486cUMyPmwkWInU37KAK6HLP7wHL50MEUIf6LKyjhqYgg6yWSmEoChXR1eVv6efiEYC4hyuojaxqnbzprrSuvM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b=I8XFZjoI; arc=none smtp.client-ip=209.85.221.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b="I8XFZjoI" Received: by mail-wr1-f41.google.com with SMTP id ffacd0b85a97d-3862d6d5765so3111618f8f.3 for ; Tue, 14 Jan 2025 09:50:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1736877025; x=1737481825; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:user-agent :references:in-reply-to:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=r7kyhS+AN7mALzi6gxFmcRFltHcX9PkTUE9xxZswSTo=; b=I8XFZjoItrHbQ1kezhXcKw/mkvx0ui/2soKJ7q3dnShNnfzujBYdBG3V+Lls9djnlZ x+eWjTsqbaQ/MXjQfwAJvVKnUTWF4SabF61DSJQ5rJEjO8QScZRR80o7+F72gRYpJ2E/ a7eRwJobjo9kbr8WUSGAeFROfNxczmPsR8ecrC2+poe9Kjcnu3FVua0Rx6JQEFGXqBYu pv/RSsZMOogZsSWlC4vpWsHDAMs+C0HCuL7aPINiazZEdXzi4S8ozwWKiEy9lSKc6U+0 wYhu6flNXVWiMuo+YA8GooJKT1YM/0MtHMcV6ahc9io030mANyLNNGW6vbvGm11PONZ3 evQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736877025; x=1737481825; h=content-transfer-encoding:mime-version:message-id:date:user-agent :references:in-reply-to:subject:cc:to:from:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=r7kyhS+AN7mALzi6gxFmcRFltHcX9PkTUE9xxZswSTo=; b=pJcknfps6cVoWHSMnOeh6AAfFmuz8WlwnE1Gt+jOcCqPPn9Oztmm1vS6XRxiwxlP6R h5K2kpFJMrUX7wyp2T9ZgNv6K9jR/hl2EypMOKdWJ5HbSohB9ZTxPEkyxwiJe1T8skbI gE5TqWfkyBt9vzIKD0ADm09n+HYm+tzzj5h5AFaPrMfce2xkBRJ+MMyz8f4Lrg1pMvPR 09vFSeb+C6ZPMAieVD3+5brK3BjL1PH3KQC3bUU+KiOkYdu821eRQPV/CfSps79GhkFd ZPBE4RY0hwdN2Es6Vcz4aWY6WYjQLR5ptpWcXU3F8MdgJSTIJKQ9gxiP2FjO8dQgxXGu JyGw== X-Forwarded-Encrypted: i=1; AJvYcCV3YeZkXqooPe+l0I5T97V5IjmdPS6MvDJv8r2+uJ425R28Y1e8Ylnk0eB/UA8DTwvnE7YrYKMU+VUANyk=@vger.kernel.org X-Gm-Message-State: AOJu0YzaFMtZ0/CIGvL5fzNvZNijjj5VhNaNvUJDUpqbwWkDX/SQdK+h qId5iW3nLCa5jC2oYw4j6njc4nbinlGEL/BZJBhIqa+n8mo0TL+bIaW825unDi4= X-Gm-Gg: ASbGncsmEpPYatWBGWb9gYLuUTUJQewNLikWe0wE/8zcYlMEn2ZT0VBnWoQz/TfGR9W bYvtKlh8tUsfksFKAohFmgl16+sMlIfXt+plXSq6TDEknsYH6oliLxtO+rmsQ2uuVW6l8si5R/0 BwURLwdJP9BrljyGn0rySP+Gnx4Pq+PqZzmiMsu3dMDHc/+bGUF4wgrIJdPOF6yNnU8Up5/um7P JDonagYQ7q3XfFCLHWIfjeXfJzfapaChSVlRRy+Zk/X0s/lgdvb+Bsx X-Google-Smtp-Source: AGHT+IGniaS5vKpw5UThOug4qTnk81tkkU2MITchcBy/hQw5sKSXoGOKno5PQ4PZG23+YM4eJ9Ctww== X-Received: by 2002:a05:6000:1547:b0:386:391e:bc75 with SMTP id ffacd0b85a97d-38a872debb2mr24343563f8f.16.1736877025045; Tue, 14 Jan 2025 09:50:25 -0800 (PST) Received: from localhost ([2a01:e0a:3c5:5fb1:317c:3d93:b7d4:96cd]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a8e4c23dcsm15192221f8f.101.2025.01.14.09.50.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jan 2025 09:50:24 -0800 (PST) From: Jerome Brunet To: Martin Blumenstingl Cc: ao.xu@amlogic.com, Neil Armstrong , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Kevin Hilman , dri-devel@lists.freedesktop.org, linux-amlogic@lists.infradead.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 00/11] Subject: [PATCH 00/11] Add DRM support for Amlogic S4 In-Reply-To: (Martin Blumenstingl's message of "Sun, 12 Jan 2025 23:44:52 +0100") References: <20250110-drm-s4-v1-0-cbc2d5edaae8@amlogic.com> User-Agent: mu4e 1.12.7; emacs 29.4 Date: Tue, 14 Jan 2025 18:50:23 +0100 Message-ID: <1jwmex5lpc.fsf@starbuckisacylon.baylibre.com> 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: quoted-printable On Sun 12 Jan 2025 at 23:44, Martin Blumenstingl wrote: > Hello, > > On Fri, Jan 10, 2025 at 6:39=E2=80=AFAM Ao Xu via B4 Relay > wrote: >> >> This patch series adds DRM support for the Amlogic S4-series SoCs. >> Compared to the Amlogic G12-series, the S4-series introduces the followi= ng changes: > Thanks for your patches. With this mail I'll try to summarize my > understanding of the situation with the drm/meson driver as I'm not > sure how familiar you are with previous discussions. > >> 1 The S4-series splits the HIU into three separate components: `sys_ctrl= `, `pwr_ctrl`, and `clk_ctrl`. >> As a result, VENC and VCLK drivers are updated with S4-specific compat= ible strings to accommodate these changes. > Jerome has already commented on patch 3/11 that this mixes in too many > IP blocks into one driver. > This is not a new situation, the existing code is doing exactly the same. > > Jerome has previously sent a series which started "an effort to remove > the use HHI syscon" [0] from the drm/meson hdmi driver. > In the same mail he further notes: "[the patchset] stops short of > making any controversial DT changes. To decouple the HDMI > phy driver and main DRM driver, allowing the PHY to get hold of its > registers, I believe a DT ABI break is inevitable. Ideally the > register region of the PHY within the HHI should provided, not the > whole HHI. That's pain for another day ..." > > For now I'm assuming you're familiar with device-tree ABI. > If not then please let us know so we can elaborate further on this. > > My own notes for meson_dw_hdmi.c are: > - we should not program HHI_HDMI_CLK_CNTL directly but go through CCF > (common clock framework) instead (we should already have the driver > for this in place) > - we should not program HHI_MEM_PD_REG0 directly but go through the > genpd/pmdomain framework (we should already have the driver for this > in place) > - for the HDMI PHY registers: on Meson8/8b/8m2 (those were 32-bit SoCs > in case you're not familiar with them, they predate GXBB/GXL/...) I > wrote a PHY driver (which is already upstream: > drivers/phy/amlogic/phy-meson8-hdmi-tx.c) as that made most sense to > me > > Then there's meson_venc.c which has two writes to HHI_GCLK_MPEG2. > I think those should go through CCF instead of directly accessing this re= gister. > > Also there's the VDAC registers in meson_encoder_cvbs.c: > I think Neil suggested at one point to make it a PHY driver. I even > started working on this (if you're curious: see [0] and [1]). > DT ABI backwards compatibility is also a concern here. > > And finally there's the video clock tree programming in meson_vclk.c. > My understanding here is that video PLL settings are very sensitive > and require fine-tuning according to the desired output clock. > Since it's a bunch of clocks I'd say that direct programming of the > clock registers should be avoided and we need to go through CCF as > well. > But to me those register values are all magic (as I am not aware of > any documentation that's available to me which describes how to > determine the correct PLL register values - otherside the standard > en/m/n/frac/lock and reset bits). > > I'm not saying that all of my thoughts are correct and immediately > need to be put into code. > Think of them more as an explanation to Jerome's reaction. > > I think what we need next is a discussion on how to move forward with > device-tree ABI for new SoCs. > Neil, Jerome: I'd like to hear your feedback on this. I completely agree with your description of the problem, that and Krzysztof's remark on patch 6. This is not new with this series indeed, so it does not introduce new problems really but it compounds the existing ones. Addressing those issues, if we want to, will get more difficult as more support is added for sure. > >> 2 The S4-series secures access to HDMITX DWC and TOP registers, >> requiring modifications to the driver to handle this new access method. Regmap buses are made for those cases where the registers are the same, but accessed differently. There is an example in the patchset referenced by Martin, to handle GXL and G12 diff. > This info should go into patch 1/11 to explain why the g12a compatible > string cannot be used as fallback. > > > Best regards, > Martin > > > [0] https://github.com/xdarklight/linux/commit/36e698edc25dc698a0d57b729a= 7a4587fafc0987 > [1] https://github.com/xdarklight/linux/commit/824b792fdbd2d3c0b71b21e110= 5eca0aaad8daa6 --=20 Jerome