* Re: [PATCH] drm/sun4i: tcon: Support keeping dclk rate upon ancestor clock changes
From: Maxime Ripard @ 2024-03-21 16:24 UTC (permalink / raw)
To: Jernej Škrabec
Cc: Frank Oltmanns, Chen-Yu Tsai, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Daniel Vetter, Samuel Holland,
Icenowy Zheng, Ondrej Jirman, dri-devel, linux-arm-kernel,
linux-sunxi, linux-kernel
In-Reply-To: <5448341.Sb9uPGUboI@jernej-laptop>
[-- Attachment #1.1: Type: text/plain, Size: 3069 bytes --]
On Thu, Mar 14, 2024 at 06:20:58PM +0100, Jernej Škrabec wrote:
> Dne četrtek, 14. marec 2024 ob 15:42:24 CET je Maxime Ripard napisal(a):
> > On Sun, Mar 10, 2024 at 02:32:29PM +0100, Frank Oltmanns wrote:
> > > Allow the dclk to reset its rate when a rate change is initiated from an
> > > ancestor clock. This makes it possible to no longer to get an exclusive
> > > lock. As a consequence, it is now possible to set new rates if
> > > necessary, e.g. when an external display is connected.
> > >
> > > The first user of this functionality is the A64 because PLL-VIDEO0 is an
> > > ancestor for both HDMI and TCON0. This allows to select an optimal rate
> > > for TCON0 as long as there is no external HDMI connection. Once a change
> > > in PLL-VIDEO0 is performed when an HDMI connection is established, TCON0
> > > can react gracefully and select an optimal rate based on this the new
> > > constraint.
> > >
> > > Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
> > > ---
> > > I would like to make the Allwinner A64's data-clock keep its rate
> > > when its ancestor's (pll-video0) rate changes. Keeping data-clock's rate
> > > is required, to let the A64 drive both an LCD and HDMI display at the
> > > same time, because both have pll-video0 as an ancestor.
> > >
> > > TCONs that use this flag store the ideal rate for their data-clock and
> > > subscribe to be notified when data-clock changes. When rate setting has
> > > finished (indicated by a POST_RATE_CHANGE event) the call back function
> > > schedules delayed work to set the data-clock's rate to the initial value
> > > after 100 ms. Using delayed work maks sure that the clock setting is
> > > finished.
> > >
> > > I've implemented this functionality as a quirk, so that it is possible
> > > to use it only for the A64.
> > >
> > > This patch supersedes [1].
> > >
> > > This work is inspired by an out-of-tree patchset [2] [3] [4].
> > > Unfortunately, the patchset uses clk_set_rate() directly in a notifier
> > > callback, which the following comment on clk_notifier_register()
> > > forbids: "The callbacks associated with the notifier must not re-enter
> > > into the clk framework by calling any top-level clk APIs." [5]
> > > Furthermore, that out-of-tree patchset no longer works since 6.6,
> > > because setting pll-mipi is now also resetting pll-video0 and therefore
> > > causes a race condition.
> >
> > Workqueues don't have an upper boundary on when they execute. As we
> > discussed multiple times, this should be solved in the clock framework
> > itself, not bypassing it.
>
> I think TCON code still needs to be touched due to clk_rate_exclusive_get()
> calls which effectively lock whole chain. You can't have both TCONs locking
> rate on A64 for this to work correctly.
>
> What was original reason for clk_rate_exclusive_get()? I forgot already.
IIRC, it was because the D-PHY and DSI controller derive from the same
clock, and we needed to make sure setting one wouldn't affect the other
one.
Maxime
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH net v2 2/2] net: dsa: mt7530: fix disabling EEE on failure on MT7531 and MT7988
From: Arınç ÜNAL via B4 Relay @ 2024-03-21 16:29 UTC (permalink / raw)
To: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, René van Dorst, Russell King,
SkyLake Huang, Heiner Kallweit
Cc: Bartel Eerdekens, mithat.guner, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek,
Arınç ÜNAL
In-Reply-To: <20240321-for-net-mt7530-fix-eee-for-mt7531-mt7988-v2-0-9af9d5041bfe@arinc9.com>
From: Arınç ÜNAL <arinc.unal@arinc9.com>
The MT7531_FORCE_EEE1G and MT7531_FORCE_EEE100 bits let the
PMCR_FORCE_EEE1G and PMCR_FORCE_EEE100 bits determine the 1G/100 EEE
abilities of the MAC. If MT7531_FORCE_EEE1G and MT7531_FORCE_EEE100 are
unset, the abilities are left to be determined by PHY auto polling.
The commit 40b5d2f15c09 ("net: dsa: mt7530: Add support for EEE features")
made it so that the PMCR_FORCE_EEE1G and PMCR_FORCE_EEE100 bits are set on
mt753x_phylink_mac_link_up(). But it did not set the MT7531_FORCE_EEE1G and
MT7531_FORCE_EEE100 bits. Because of this, EEE will be enabled on the
switch MACs by polling the PHY, regardless of the result of phy_init_eee().
Define these bits and add them to MT7531_FORCE_MODE which is being used by
the subdriver. With this, EEE will be prevented from being enabled on the
switch MACs when phy_init_eee() fails.
Fixes: 40b5d2f15c09 ("net: dsa: mt7530: Add support for EEE features")
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
drivers/net/dsa/mt7530.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 509ed5362236..5b99aeca34b4 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -299,11 +299,15 @@ enum mt7530_vlan_port_acc_frm {
#define MT7531_FORCE_DPX BIT(29)
#define MT7531_FORCE_RX_FC BIT(28)
#define MT7531_FORCE_TX_FC BIT(27)
+#define MT7531_FORCE_EEE100 BIT(26)
+#define MT7531_FORCE_EEE1G BIT(25)
#define MT7531_FORCE_MODE (MT7531_FORCE_LNK | \
MT7531_FORCE_SPD | \
MT7531_FORCE_DPX | \
MT7531_FORCE_RX_FC | \
- MT7531_FORCE_TX_FC)
+ MT7531_FORCE_TX_FC | \
+ MT7531_FORCE_EEE100 | \
+ MT7531_FORCE_EEE1G)
#define PMCR_LINK_SETTINGS_MASK (PMCR_TX_EN | PMCR_FORCE_SPEED_1000 | \
PMCR_RX_EN | PMCR_FORCE_SPEED_100 | \
PMCR_TX_FC_EN | PMCR_RX_FC_EN | \
--
2.40.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH net v2 0/2] Fix EEE support for MT7531 and MT7988 SoC switch
From: Arınç ÜNAL via B4 Relay @ 2024-03-21 16:29 UTC (permalink / raw)
To: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, René van Dorst, Russell King,
SkyLake Huang, Heiner Kallweit
Cc: Bartel Eerdekens, mithat.guner, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek,
Arınç ÜNAL, Florian Fainelli
Hi.
This patch series fixes EEE support for MT7531 and the switch on the MT7988
SoC. EEE could not be enabled on MT7531 on most boards using ethtool before
this. On MT7988 SoC switch, EEE is disabled by default but can be turned on
normally using ethtool. EEE is enabled by default on MT7530 and there's no
need to make changes on the DSA subdriver for it. This patch series
disables EEE by default on MT7531 but makes it possible to enable it using
ethtool.
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
Changes in v2:
- Delegate the patch to the net tree.
- Remove patch 3, it was revealed that it doesn't fix a bug.
- Patch 1
- Disable EEE advertisement on MT7531 by default.
- Link to v1: https://lore.kernel.org/r/20240318-for-net-mt7530-fix-eee-for-mt7531-mt7988-v1-0-3f17226344e8@arinc9.com
---
Arınç ÜNAL (2):
net: dsa: mt7530: fix enabling EEE on MT7531 switch on all boards
net: dsa: mt7530: fix disabling EEE on failure on MT7531 and MT7988
drivers/net/dsa/mt7530.c | 14 ++++++++++++++
drivers/net/dsa/mt7530.h | 7 ++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
---
base-commit: ea80e3ed09ab2c2b75724faf5484721753e92c31
change-id: 20240317-for-net-mt7530-fix-eee-for-mt7531-mt7988-a5c5453cc0e8
Best regards,
--
Arınç ÜNAL <arinc.unal@arinc9.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH net v2 1/2] net: dsa: mt7530: fix enabling EEE on MT7531 switch on all boards
From: Arınç ÜNAL via B4 Relay @ 2024-03-21 16:29 UTC (permalink / raw)
To: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, René van Dorst, Russell King,
SkyLake Huang, Heiner Kallweit
Cc: Bartel Eerdekens, mithat.guner, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek,
Arınç ÜNAL, Florian Fainelli
In-Reply-To: <20240321-for-net-mt7530-fix-eee-for-mt7531-mt7988-v2-0-9af9d5041bfe@arinc9.com>
From: Arınç ÜNAL <arinc.unal@arinc9.com>
The commit 40b5d2f15c09 ("net: dsa: mt7530: Add support for EEE features")
brought EEE support but did not enable EEE on MT7531 switch MACs. EEE is
enabled on MT7531 switch MACs either by pulling the LAN2LED0 pin low on the
board (bootstrapping), or unsetting the EEE_DIS bit on the trap register.
There are existing boards that were not designed to pull the pin low.
Therefore, unset the EEE_DIS bit on the trap register.
Unlike MT7530, the modifiable trap register won't be populated identical to
the trap status register after reset. Therefore, read from the trap status
register, modify the bits, then write to the modifiable trap register.
My testing on MT7531 shows a certain amount of traffic loss when EEE is
enabled. That said, I haven't come across a board that enables EEE. So
enable EEE on the switch MACs but disable EEE advertisement on the switch
PHYs. This way, we don't change the behaviour of the majority of the boards
that have this switch.
With this change, EEE can now be enabled using ethtool.
The disable EEE bit on the trap pertains to the LAN2LED0 pin which is
usually used to control an LED. Once the bit is unset, the pin will be low.
That will make the active low LED turn on.
The pin is controlled by the switch PHY. It seems that the PHY controls the
pin in the way that it inverts the pin state. That means depending on the
wiring of the LED connected to LAN2LED0 on the board, the LED may be on
without an active link.
Fixes: 40b5d2f15c09 ("net: dsa: mt7530: Add support for EEE features")
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
drivers/net/dsa/mt7530.c | 14 ++++++++++++++
drivers/net/dsa/mt7530.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 678b51f9cea6..6aa99b590329 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2458,6 +2458,20 @@ mt7531_setup(struct dsa_switch *ds)
/* Reset the switch through internal reset */
mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_SW_RST | SYS_CTRL_REG_RST);
+ /* Allow modifying the trap and enable Energy-Efficient Ethernet (EEE).
+ */
+ val = mt7530_read(priv, MT7531_HWTRAP);
+ val |= CHG_STRAP;
+ val &= ~EEE_DIS;
+ mt7530_write(priv, MT7530_MHWTRAP, val);
+
+ /* Disable EEE advertisement on the switch PHYs. */
+ for (i = MT753X_CTRL_PHY_ADDR;
+ i < MT753X_CTRL_PHY_ADDR + MT7530_NUM_PHYS; i++) {
+ mt7531_ind_c45_phy_write(priv, i, MDIO_MMD_AN, MDIO_AN_EEE_ADV,
+ 0);
+ }
+
if (!priv->p5_sgmii) {
mt7531_pll_setup(priv);
} else {
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index a71166e0a7fc..509ed5362236 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -457,6 +457,7 @@ enum mt7531_clk_skew {
#define XTAL_FSEL_M BIT(7)
#define PHY_EN BIT(6)
#define CHG_STRAP BIT(8)
+#define EEE_DIS BIT(4)
/* Register for hw trap modification */
#define MT7530_MHWTRAP 0x7804
--
2.40.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2 8/8] drm: zynqmp_dp: Add debugfs interface for compliance testing
From: Tomi Valkeinen @ 2024-03-21 16:31 UTC (permalink / raw)
To: Sean Anderson
Cc: Michal Simek, David Airlie, linux-kernel, Daniel Vetter,
linux-arm-kernel, Laurent Pinchart, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, dri-devel
In-Reply-To: <76831c1e-216f-430b-bacd-2d50f352e61f@linux.dev>
On 21/03/2024 18:08, Sean Anderson wrote:
> On 3/20/24 03:49, Tomi Valkeinen wrote:
>> On 20/03/2024 00:51, Sean Anderson wrote:
>>
>>> +/**
>>> + * enum test_pattern - Test patterns for test testing
>>
>> "for test testing"? =)
>>
>>> @@ -1655,6 +2321,9 @@ static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
>>> u8 status[DP_LINK_STATUS_SIZE + 2];
>>> int err;
>>> + if (READ_ONCE(dp->ignore_hpd))
>>> + return;
>>> +
>>> mutex_lock(&dp->lock);
>>> err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
>>> DP_LINK_STATUS_SIZE + 2);
>>
>> Why do you need READ/WRITE_ONCE() for ignore_hpd?
>
> It's not protected by dp->lock so we don't have to take it for
> zynqmp_dp_hpd_work_func. Although maybe we should make a version of
> zynqmp_dp_bridge_detect which assumes we already hold the lock.
Does using the macros solve some potential issue, or is it just for
documenting that this variable is accessed without lock?
Tomi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/3] Fix EEE support for MT7531 and MT7988 SoC switch
From: Florian Fainelli @ 2024-03-21 16:31 UTC (permalink / raw)
To: Arınç ÜNAL, Daniel Golle, Andrew Lunn
Cc: DENG Qingfang, Sean Wang, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, René van Dorst, Russell King,
SkyLake Huang, Heiner Kallweit, Bartel Eerdekens, mithat.guner,
erkin.bozoglu, netdev, linux-kernel, linux-arm-kernel,
linux-mediatek
In-Reply-To: <0fbe7ba2-6529-4118-b050-8ea76d28b712@arinc9.com>
On 3/21/24 09:09, Arınç ÜNAL wrote:
> On 20.03.2024 00:31, Florian Fainelli wrote:
>> On 3/19/24 13:26, Daniel Golle wrote:
>>> On Tue, Mar 19, 2024 at 08:38:03PM +0100, Andrew Lunn wrote:
>>>>> I would argue that EEE advertisement on the PHY should be enabled by
>>>>> default.
>>>>
>>>> That is an open question at the moment. For some use cases, it can add
>>>> extra delay and jitter which can cause problems. I've heard people
>>>> doing PTP don't like EEE for example.
>>>
>>> MediaTek consumer-grade hardware doesn't support PTP and hence that
>>> quite certainly won't ever be an issue with all switch ICs supported
>>> by the mt7530 driver.
>>>
>>> I'd rather first change the (configuration) default in OpenWrt (which
>>> is arguable the way most people are using this hardware), also because
>>> that will be more visible/obvious for users. Or even just make EEE
>>> configurable in the LuCI web-UI as a first step so users start playing
>>> with it.
>>>
>>> After all, I also have a hard time imagining that MediaTek disabled
>>> EEE in their downstream driver for no reason:
>>>
>>> https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/24091177a18ba7f2dd8d928a8f5b27b14df46b16
>>
>> EEE tends to be an interoperability trap and typically results in
>> unexplained link drops with different link partners which are
>> difficult to debug and root cause. It would be great to have more
>> context as to why it was disabled in the downstream tree to know what
>> we are up against, though I would not be surprised if there had been a
>> number of issues reported.
>
> I have started testing MT7531 with EEE enabled and immediately experienced
> frames that wouldn't egress the switch or improperly received on the link
> partner.
>
> SoC MAC <-EEE off-> MT7531 P6 MAC (acting as PHY)
> MT7531 P0 MAC <-EEE on -> MT7531 P0 PHY
> MT7531 P0 PHY <-EEE on -> Computer connected with twisted pair
OK, so this is intended to describe that the SoC's Ethernet MAC link to
the integrated switch did not use EEE only the user-facing ports. That
makes sense because it's all digital logic and you are not going to be
seeing much power saving from having EEE enabled between the SoC's
Ethernet MAC and CPU port of the switch, that said, however, I wonder if
this has an impact on any form of flow control within the switch that is
reacting to LPI and you need EEE to be enabled end-to-end?
>
> I've tested pinging from the SoC's CPU. Packet capturing on the twisted
> pair computer showed very few frames were being received.
>
> # ping 192.168.2.2
> PING 192.168.2.2 (192.168.2.2): 56 data bytes
> 64 bytes from 192.168.2.2: seq=36 ttl=64 time=0.486 ms
> ^C
> --- 192.168.2.2 ping statistics ---
> 64 packets transmitted, 1 packets received, 98% packet loss
> round-trip min/avg/max = 0.486/0.486/0.486 ms
>
> It seems there's less loss when frames are passed more frequently.
That would point to an issue getting in and out of LPI, do you see these
packet losses even with different LPI timeouts?
>
> # ping 192.168.2.2 -i 0.06
> PING 192.168.2.2 (192.168.2.2): 56 data bytes
> 64 bytes from 192.168.2.2: seq=5 ttl=64 time=0.285 ms
> 64 bytes from 192.168.2.2: seq=6 ttl=64 time=0.155 ms
> 64 bytes from 192.168.2.2: seq=7 ttl=64 time=0.243 ms
> 64 bytes from 192.168.2.2: seq=8 ttl=64 time=0.139 ms
> 64 bytes from 192.168.2.2: seq=9 ttl=64 time=0.224 ms
> 64 bytes from 192.168.2.2: seq=68 ttl=64 time=0.350 ms
> 64 bytes from 192.168.2.2: seq=69 ttl=64 time=0.242 ms
> 64 bytes from 192.168.2.2: seq=70 ttl=64 time=0.230 ms
> 64 bytes from 192.168.2.2: seq=71 ttl=64 time=0.242 ms
> 64 bytes from 192.168.2.2: seq=72 ttl=64 time=0.276 ms
> 64 bytes from 192.168.2.2: seq=101 ttl=64 time=0.224 ms
> 64 bytes from 192.168.2.2: seq=102 ttl=64 time=0.238 ms
> 64 bytes from 192.168.2.2: seq=103 ttl=64 time=0.240 ms
> ...
> --- 192.168.2.2 ping statistics ---
> 214 packets transmitted, 32 packets received, 85% packet loss
> round-trip min/avg/max = 0.099/0.225/0.350 ms
>
> # ping 192.168.2.2 -i 0.05
> PING 192.168.2.2 (192.168.2.2): 56 data bytes
> 64 bytes from 192.168.2.2: seq=1 ttl=64 time=0.277 ms
> 64 bytes from 192.168.2.2: seq=2 ttl=64 time=0.240 ms
> 64 bytes from 192.168.2.2: seq=3 ttl=64 time=0.133 ms
> 64 bytes from 192.168.2.2: seq=4 ttl=64 time=0.233 ms
> 64 bytes from 192.168.2.2: seq=5 ttl=64 time=0.223 ms
> 64 bytes from 192.168.2.2: seq=6 ttl=64 time=0.228 ms
> 64 bytes from 192.168.2.2: seq=7 ttl=64 time=0.236 ms
> 64 bytes from 192.168.2.2: seq=8 ttl=64 time=0.150 ms
> ...
> --- 192.168.2.2 ping statistics ---
> 41 packets transmitted, 40 packets received, 2% packet loss
> round-trip min/avg/max = 0.112/0.206/0.277 ms
>
>>
>> That said as an user, if someone has a well controlled environment,
>> they should absolutely be able to turn on EEE and see how stable it
>> holds in their environment.
>
> Looks like this is the way to go. I'm planning to submit v2 with patch 1
> as:
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 678b51f9cea6..6aa99b590329 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2458,6 +2458,20 @@ mt7531_setup(struct dsa_switch *ds)
> /* Reset the switch through internal reset */
> mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_SW_RST |
> SYS_CTRL_REG_RST);
>
> + /* Allow modifying the trap and enable Energy-Efficient Ethernet
> (EEE).
> + */
> + val = mt7530_read(priv, MT7531_HWTRAP);
> + val |= CHG_STRAP;
> + val &= ~EEE_DIS;
> + mt7530_write(priv, MT7530_MHWTRAP, val);
> +
> + /* Disable EEE advertisement on the switch PHYs. */
> + for (i = MT753X_CTRL_PHY_ADDR;
> + i < MT753X_CTRL_PHY_ADDR + MT7530_NUM_PHYS; i++) {
> + mt7531_ind_c45_phy_write(priv, i, MDIO_MMD_AN, MDIO_AN_EEE_ADV,
> + 0);
> + }
> +
> if (!priv->p5_sgmii) {
> mt7531_pll_setup(priv);
> } else {
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index a71166e0a7fc..509ed5362236 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -457,6 +457,7 @@ enum mt7531_clk_skew {
> #define XTAL_FSEL_M BIT(7)
> #define PHY_EN BIT(6)
> #define CHG_STRAP BIT(8)
> +#define EEE_DIS BIT(4)
>
> /* Register for hw trap modification */
> #define MT7530_MHWTRAP 0x7804
>
> Arınç
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v10 05/27] drm/tests: Add output bpc tests
From: Maxime Ripard @ 2024-03-21 15:29 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jonathan Corbet, Sandy Huang, Heiko Stübner, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: Hans Verkuil, Sebastian Wick, Ville Syrjälä, dri-devel,
linux-arm-kernel, linux-doc, linux-kernel, linux-media,
linux-rockchip, linux-sunxi, Maxime Ripard, Dave Stevenson
In-Reply-To: <20240321-kms-hdmi-connector-state-v10-0-e6c178361898@kernel.org>
Now that we're tracking the output bpc count in the connector state,
let's add a few tests to make sure it works as expected.
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/tests/Makefile | 1 +
.../gpu/drm/tests/drm_atomic_state_helper_test.c | 436 +++++++++++++++++++++
drivers/gpu/drm/tests/drm_connector_test.c | 140 +++++++
drivers/gpu/drm/tests/drm_kunit_edid.h | 106 +++++
4 files changed, 683 insertions(+)
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index d6183b3d7688..b29ddfd90596 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -2,10 +2,11 @@
obj-$(CONFIG_DRM_KUNIT_TEST_HELPERS) += \
drm_kunit_helpers.o
obj-$(CONFIG_DRM_KUNIT_TEST) += \
+ drm_atomic_state_helper_test.o \
drm_buddy_test.o \
drm_cmdline_parser_test.o \
drm_connector_test.o \
drm_damage_helper_test.o \
drm_dp_mst_helper_test.o \
diff --git a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
new file mode 100644
index 000000000000..5d1a1dbff433
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
@@ -0,0 +1,436 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Kunit test for drm_atomic_state_helper functions
+ */
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_atomic_uapi.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_kunit_helpers.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_probe_helper.h>
+
+#include <drm/drm_print.h>
+#include "../drm_crtc_internal.h"
+
+#include <kunit/test.h>
+
+#include "drm_kunit_edid.h"
+
+struct drm_atomic_helper_connector_hdmi_priv {
+ struct drm_device drm;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+
+ const char *current_edid;
+ size_t current_edid_len;
+};
+
+#define connector_to_priv(c) \
+ container_of_const(c, struct drm_atomic_helper_connector_hdmi_priv, connector)
+
+static struct drm_display_mode *find_preferred_mode(struct drm_connector *connector)
+{
+ struct drm_device *drm = connector->dev;
+ struct drm_display_mode *mode, *preferred;
+
+ mutex_lock(&drm->mode_config.mutex);
+ preferred = list_first_entry(&connector->modes, struct drm_display_mode, head);
+ list_for_each_entry(mode, &connector->modes, head)
+ if (mode->type & DRM_MODE_TYPE_PREFERRED)
+ preferred = mode;
+ mutex_unlock(&drm->mode_config.mutex);
+
+ return preferred;
+}
+
+static int light_up_connector(struct kunit *test,
+ struct drm_device *drm,
+ struct drm_crtc *crtc,
+ struct drm_connector *connector,
+ struct drm_display_mode *mode,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_atomic_state *state;
+ struct drm_connector_state *conn_state;
+ struct drm_crtc_state *crtc_state;
+ int ret;
+
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+ conn_state = drm_atomic_get_connector_state(state, connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, crtc);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ ret = drm_atomic_commit(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ return 0;
+}
+
+static int set_connector_edid(struct kunit *test, struct drm_connector *connector,
+ const char *edid, size_t edid_len)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv =
+ connector_to_priv(connector);
+ struct drm_device *drm = connector->dev;
+ int ret;
+
+ priv->current_edid = edid;
+ priv->current_edid_len = edid_len;
+
+ mutex_lock(&drm->mode_config.mutex);
+ ret = connector->funcs->fill_modes(connector, 4096, 4096);
+ mutex_unlock(&drm->mode_config.mutex);
+ KUNIT_ASSERT_GT(test, ret, 0);
+
+ return 0;
+}
+
+static int dummy_connector_get_modes(struct drm_connector *connector)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv =
+ connector_to_priv(connector);
+ const struct drm_edid *edid;
+ unsigned int num_modes;
+
+ edid = drm_edid_alloc(priv->current_edid, priv->current_edid_len);
+ if (!edid)
+ return -EINVAL;
+
+ drm_edid_connector_update(connector, edid);
+ num_modes = drm_edid_connector_add_modes(connector);
+
+ drm_edid_free(edid);
+
+ return num_modes;
+}
+
+static const struct drm_connector_helper_funcs dummy_connector_helper_funcs = {
+ .atomic_check = drm_atomic_helper_connector_hdmi_check,
+ .get_modes = dummy_connector_get_modes,
+};
+
+static void dummy_hdmi_connector_reset(struct drm_connector *connector)
+{
+ drm_atomic_helper_connector_reset(connector);
+ __drm_atomic_helper_connector_hdmi_reset(connector, connector->state);
+}
+
+static const struct drm_connector_funcs dummy_connector_funcs = {
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .reset = dummy_hdmi_connector_reset,
+};
+
+static
+struct drm_atomic_helper_connector_hdmi_priv *
+drm_atomic_helper_connector_hdmi_init(struct kunit *test,
+ unsigned int max_bpc)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_connector *conn;
+ struct drm_encoder *enc;
+ struct drm_device *drm;
+ struct device *dev;
+ int ret;
+
+ dev = drm_kunit_helper_alloc_device(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+
+ priv = drm_kunit_helper_alloc_drm_device(test, dev,
+ struct drm_atomic_helper_connector_hdmi_priv, drm,
+ DRIVER_MODESET | DRIVER_ATOMIC);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+ test->priv = priv;
+
+ drm = &priv->drm;
+ priv->plane = drm_kunit_helper_create_primary_plane(test, drm,
+ NULL,
+ NULL,
+ NULL, 0,
+ NULL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->plane);
+
+ priv->crtc = drm_kunit_helper_create_crtc(test, drm,
+ priv->plane, NULL,
+ NULL,
+ NULL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->crtc);
+
+ enc = &priv->encoder;
+ ret = drmm_encoder_init(drm, enc, NULL, DRM_MODE_ENCODER_TMDS, NULL);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ enc->possible_crtcs = drm_crtc_mask(priv->crtc);
+
+ conn = &priv->connector;
+ ret = drmm_connector_hdmi_init(drm, conn,
+ &dummy_connector_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ NULL,
+ max_bpc);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ drm_connector_helper_add(conn, &dummy_connector_helper_funcs);
+ drm_connector_attach_encoder(conn, enc);
+
+ drm_mode_config_reset(drm);
+
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ return priv;
+}
+
+/*
+ * Test that if we change the maximum bpc property to a different value,
+ * we trigger a mode change on the connector's CRTC, which will in turn
+ * disable/enable the connector.
+ */
+static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *old_conn_state;
+ struct drm_connector_state *new_conn_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *preferred;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test, 10);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ conn = &priv->connector;
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+ new_conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state);
+
+ new_conn_state->hdmi.output_bpc = 8;
+
+ KUNIT_ASSERT_NE(test,
+ old_conn_state->hdmi.output_bpc,
+ new_conn_state->hdmi.output_bpc);
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state);
+
+ new_conn_state = drm_atomic_get_new_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
+
+ KUNIT_ASSERT_NE(test,
+ old_conn_state->hdmi.output_bpc,
+ new_conn_state->hdmi.output_bpc);
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+ KUNIT_EXPECT_TRUE(test, crtc_state->mode_changed);
+}
+
+/*
+ * Test that if we set the output bpc property to the same value, we
+ * don't trigger a mode change on the connector's CRTC and leave the
+ * connector unaffected.
+ */
+static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *old_conn_state;
+ struct drm_connector_state *new_conn_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *preferred;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test, 10);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ conn = &priv->connector;
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+ new_conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state);
+
+ KUNIT_ASSERT_EQ(test,
+ new_conn_state->hdmi.output_bpc,
+ old_conn_state->hdmi.output_bpc);
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state);
+
+ new_conn_state = drm_atomic_get_new_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
+
+ KUNIT_EXPECT_EQ(test,
+ old_conn_state->hdmi.output_bpc,
+ new_conn_state->hdmi.output_bpc);
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+ KUNIT_EXPECT_FALSE(test, crtc_state->mode_changed);
+}
+
+static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
+ KUNIT_CASE(drm_test_check_output_bpc_crtc_mode_changed),
+ KUNIT_CASE(drm_test_check_output_bpc_crtc_mode_not_changed),
+ { }
+};
+
+static struct kunit_suite drm_atomic_helper_connector_hdmi_check_test_suite = {
+ .name = "drm_atomic_helper_connector_hdmi_check",
+ .test_cases = drm_atomic_helper_connector_hdmi_check_tests,
+};
+
+/*
+ * Test that if the connector was initialised with a maximum bpc of 8,
+ * the value of the max_bpc and max_requested_bpc properties out of
+ * reset are also set to 8, and output_bpc is set to 0 and will be
+ * filled at atomic_check time.
+ */
+static void drm_test_check_bpc_8_value(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_connector_state *conn_state;
+ struct drm_connector *conn;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test, 8);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ conn_state = conn->state;
+ KUNIT_EXPECT_EQ(test, conn_state->max_bpc, 8);
+ KUNIT_EXPECT_EQ(test, conn_state->max_requested_bpc, 8);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 0);
+}
+
+/*
+ * Test that if the connector was initialised with a maximum bpc of 10,
+ * the value of the max_bpc and max_requested_bpc properties out of
+ * reset are also set to 10, and output_bpc is set to 0 and will be
+ * filled at atomic_check time.
+ */
+static void drm_test_check_bpc_10_value(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_connector_state *conn_state;
+ struct drm_connector *conn;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test, 10);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ conn_state = conn->state;
+ KUNIT_EXPECT_EQ(test, conn_state->max_bpc, 10);
+ KUNIT_EXPECT_EQ(test, conn_state->max_requested_bpc, 10);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 0);
+}
+
+/*
+ * Test that if the connector was initialised with a maximum bpc of 12,
+ * the value of the max_bpc and max_requested_bpc properties out of
+ * reset are also set to 12, and output_bpc is set to 0 and will be
+ * filled at atomic_check time.
+ */
+static void drm_test_check_bpc_12_value(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_connector_state *conn_state;
+ struct drm_connector *conn;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test, 12);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ conn_state = conn->state;
+ KUNIT_EXPECT_EQ(test, conn_state->max_bpc, 12);
+ KUNIT_EXPECT_EQ(test, conn_state->max_requested_bpc, 12);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 0);
+}
+
+static struct kunit_case drm_atomic_helper_connector_hdmi_reset_tests[] = {
+ KUNIT_CASE(drm_test_check_bpc_8_value),
+ KUNIT_CASE(drm_test_check_bpc_10_value),
+ KUNIT_CASE(drm_test_check_bpc_12_value),
+ { }
+};
+
+static struct kunit_suite drm_atomic_helper_connector_hdmi_reset_test_suite = {
+ .name = "drm_atomic_helper_connector_hdmi_reset",
+ .test_cases = drm_atomic_helper_connector_hdmi_reset_tests,
+};
+
+kunit_test_suites(
+ &drm_atomic_helper_connector_hdmi_check_test_suite,
+ &drm_atomic_helper_connector_hdmi_reset_test_suite,
+);
+
+MODULE_AUTHOR("Maxime Ripard <mripard@kernel.org>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index 2661eb64a5cd..2519b91de95e 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -10,10 +10,12 @@
#include <drm/drm_drv.h>
#include <drm/drm_kunit_helpers.h>
#include <kunit/test.h>
+#include "../drm_crtc_internal.h"
+
struct drm_connector_init_priv {
struct drm_device drm;
struct drm_connector connector;
struct i2c_adapter ddc;
};
@@ -204,10 +206,143 @@ static void drm_test_connector_hdmi_init_null_ddc(struct kunit *test)
NULL,
8);
KUNIT_EXPECT_EQ(test, ret, 0);
}
+/*
+ * Test that the registration of a connector with an invalid maximum bpc
+ * count fails.
+ */
+static void drm_test_connector_hdmi_init_bpc_invalid(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ 9);
+ KUNIT_EXPECT_LT(test, ret, 0);
+}
+
+/*
+ * Test that the registration of a connector with a null maximum bpc
+ * count fails.
+ */
+static void drm_test_connector_hdmi_init_bpc_null(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ 0);
+ KUNIT_EXPECT_LT(test, ret, 0);
+}
+
+/*
+ * Test that the registration of a connector with a maximum bpc count of
+ * 8 succeeds, registers the max bpc property, but doesn't register the
+ * HDR output metadata one.
+ */
+static void drm_test_connector_hdmi_init_bpc_8(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ uint64_t val;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ 8);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ prop = connector->max_bpc_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+
+ ret = drm_object_property_get_value(&connector->base, prop, &val);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, val, 8);
+
+ prop = priv->drm.mode_config.hdr_output_metadata_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+}
+
+/*
+ * Test that the registration of a connector with a maximum bpc count of
+ * 10 succeeds and registers the max bpc and HDR output metadata
+ * properties.
+ */
+static void drm_test_connector_hdmi_init_bpc_10(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ uint64_t val;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ 10);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ prop = connector->max_bpc_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+
+ ret = drm_object_property_get_value(&connector->base, prop, &val);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, val, 10);
+
+ prop = priv->drm.mode_config.hdr_output_metadata_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+}
+
+/*
+ * Test that the registration of a connector with a maximum bpc count of
+ * 12 succeeds and registers the max bpc and HDR output metadata
+ * properties.
+ */
+static void drm_test_connector_hdmi_init_bpc_12(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ uint64_t val;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ 12);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ prop = connector->max_bpc_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+
+ ret = drm_object_property_get_value(&connector->base, prop, &val);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, val, 12);
+
+ prop = priv->drm.mode_config.hdr_output_metadata_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+}
+
/*
* Test that the registration of an HDMI connector with an HDMI
* connector type succeeds.
*/
static void drm_test_connector_hdmi_init_type_valid(struct kunit *test)
@@ -282,10 +417,15 @@ KUNIT_ARRAY_PARAM(drm_connector_hdmi_init_type_invalid,
drm_connector_hdmi_init_type_invalid_tests,
drm_connector_hdmi_init_type_desc);
static struct kunit_case drmm_connector_hdmi_init_tests[] = {
KUNIT_CASE(drm_test_connector_hdmi_init_valid),
+ KUNIT_CASE(drm_test_connector_hdmi_init_bpc_8),
+ KUNIT_CASE(drm_test_connector_hdmi_init_bpc_10),
+ KUNIT_CASE(drm_test_connector_hdmi_init_bpc_12),
+ KUNIT_CASE(drm_test_connector_hdmi_init_bpc_invalid),
+ KUNIT_CASE(drm_test_connector_hdmi_init_bpc_null),
KUNIT_CASE(drm_test_connector_hdmi_init_null_ddc),
KUNIT_CASE_PARAM(drm_test_connector_hdmi_init_type_valid,
drm_connector_hdmi_init_type_valid_gen_params),
KUNIT_CASE_PARAM(drm_test_connector_hdmi_init_type_invalid,
drm_connector_hdmi_init_type_invalid_gen_params),
diff --git a/drivers/gpu/drm/tests/drm_kunit_edid.h b/drivers/gpu/drm/tests/drm_kunit_edid.h
new file mode 100644
index 000000000000..0366dd29c820
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_kunit_edid.h
@@ -0,0 +1,106 @@
+#ifndef DRM_KUNIT_EDID_H_
+#define DRM_KUNIT_EDID_H_
+
+/*
+ * edid-decode (hex):
+ *
+ * 00 ff ff ff ff ff ff 00 31 d8 2a 00 00 00 00 00
+ * 00 21 01 03 81 a0 5a 78 02 00 00 00 00 00 00 00
+ * 00 00 00 20 00 00 01 01 01 01 01 01 01 01 01 01
+ * 01 01 01 01 01 01 02 3a 80 18 71 38 2d 40 58 2c
+ * 45 00 40 84 63 00 00 1e 00 00 00 fc 00 54 65 73
+ * 74 20 45 44 49 44 0a 20 20 20 00 00 00 fd 00 32
+ * 46 1e 46 0f 00 0a 20 20 20 20 20 20 00 00 00 10
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 92
+ *
+ * 02 03 1b 81 e3 05 00 20 41 10 e2 00 4a 6d 03 0c
+ * 00 12 34 00 28 20 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 d0
+ *
+ * ----------------
+ *
+ * Block 0, Base EDID:
+ * EDID Structure Version & Revision: 1.3
+ * Vendor & Product Identification:
+ * Manufacturer: LNX
+ * Model: 42
+ * Made in: 2023
+ * Basic Display Parameters & Features:
+ * Digital display
+ * DFP 1.x compatible TMDS
+ * Maximum image size: 160 cm x 90 cm
+ * Gamma: 2.20
+ * Monochrome or grayscale display
+ * First detailed timing is the preferred timing
+ * Color Characteristics:
+ * Red : 0.0000, 0.0000
+ * Green: 0.0000, 0.0000
+ * Blue : 0.0000, 0.0000
+ * White: 0.0000, 0.0000
+ * Established Timings I & II:
+ * DMT 0x04: 640x480 59.940476 Hz 4:3 31.469 kHz 25.175000 MHz
+ * Standard Timings: none
+ * Detailed Timing Descriptors:
+ * DTD 1: 1920x1080 60.000000 Hz 16:9 67.500 kHz 148.500000 MHz (1600 mm x 900 mm)
+ * Hfront 88 Hsync 44 Hback 148 Hpol P
+ * Vfront 4 Vsync 5 Vback 36 Vpol P
+ * Display Product Name: 'Test EDID'
+ * Display Range Limits:
+ * Monitor ranges (GTF): 50-70 Hz V, 30-70 kHz H, max dotclock 150 MHz
+ * Dummy Descriptor:
+ * Extension blocks: 1
+ * Checksum: 0x92
+ *
+ * ----------------
+ *
+ * Block 1, CTA-861 Extension Block:
+ * Revision: 3
+ * Underscans IT Video Formats by default
+ * Native detailed modes: 1
+ * Colorimetry Data Block:
+ * sRGB
+ * Video Data Block:
+ * VIC 16: 1920x1080 60.000000 Hz 16:9 67.500 kHz 148.500000 MHz
+ * Video Capability Data Block:
+ * YCbCr quantization: No Data
+ * RGB quantization: Selectable (via AVI Q)
+ * PT scan behavior: No Data
+ * IT scan behavior: Always Underscanned
+ * CE scan behavior: Always Underscanned
+ * Vendor-Specific Data Block (HDMI), OUI 00-0C-03:
+ * Source physical address: 1.2.3.4
+ * Maximum TMDS clock: 200 MHz
+ * Extended HDMI video details:
+ * Checksum: 0xd0 Unused space in Extension Block: 100 bytes
+ */
+static const unsigned char test_edid_hdmi_1080p_rgb_max_200mhz[] = {
+ 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
+ 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
+ 0x00, 0x00, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+ 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3a, 0x80, 0x18, 0x71, 0x38,
+ 0x2d, 0x40, 0x58, 0x2c, 0x45, 0x00, 0x40, 0x84, 0x63, 0x00, 0x00, 0x1e,
+ 0x00, 0x00, 0x00, 0xfc, 0x00, 0x54, 0x65, 0x73, 0x74, 0x20, 0x45, 0x44,
+ 0x49, 0x44, 0x0a, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32,
+ 0x46, 0x00, 0x00, 0xc4, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+ 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x41, 0x02, 0x03, 0x1b, 0x81,
+ 0xe3, 0x05, 0x00, 0x20, 0x41, 0x10, 0xe2, 0x00, 0x4a, 0x6d, 0x03, 0x0c,
+ 0x00, 0x12, 0x34, 0x00, 0x28, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0xd0
+};
+
+#endif // DRM_KUNIT_EDID_H_
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v10 06/27] drm/connector: hdmi: Add support for output format
From: Maxime Ripard @ 2024-03-21 15:29 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jonathan Corbet, Sandy Huang, Heiko Stübner, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: Hans Verkuil, Sebastian Wick, Ville Syrjälä, dri-devel,
linux-arm-kernel, linux-doc, linux-kernel, linux-media,
linux-rockchip, linux-sunxi, Maxime Ripard, Dave Stevenson
In-Reply-To: <20240321-kms-hdmi-connector-state-v10-0-e6c178361898@kernel.org>
Just like BPC, we'll add support for automatic selection of the output
format for HDMI connectors.
Let's add the needed defaults and fields for now.
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/drm_atomic.c | 2 ++
drivers/gpu/drm/drm_atomic_state_helper.c | 3 ++-
drivers/gpu/drm/drm_connector.c | 31 ++++++++++++++++++++++
.../gpu/drm/tests/drm_atomic_state_helper_test.c | 22 +++++++++++----
drivers/gpu/drm/tests/drm_connector_test.c | 9 +++++++
include/drm/drm_connector.h | 20 ++++++++++++++
6 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 4e11cfb4518b..8730137baa86 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1144,10 +1144,12 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
drm_printf(p, "\tcolorspace=%s\n", drm_get_colorspace_name(state->colorspace));
if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
drm_printf(p, "\toutput_bpc=%u\n", state->hdmi.output_bpc);
+ drm_printf(p, "\toutput_format=%s\n",
+ drm_hdmi_connector_get_output_format_name(state->hdmi.output_format));
}
if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
if (state->writeback_job && state->writeback_job->fb)
drm_printf(p, "\tfb=%d\n", state->writeback_job->fb->base.id);
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index c56ca1b08228..3a9ef45509e1 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -658,11 +658,12 @@ int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
struct drm_connector_state *old_conn_state =
drm_atomic_get_old_connector_state(state, connector);
struct drm_connector_state *new_conn_state =
drm_atomic_get_new_connector_state(state, connector);
- if (old_conn_state->hdmi.output_bpc != new_conn_state->hdmi.output_bpc) {
+ if (old_conn_state->hdmi.output_bpc != new_conn_state->hdmi.output_bpc ||
+ old_conn_state->hdmi.output_format != new_conn_state->hdmi.output_format) {
struct drm_crtc *crtc = new_conn_state->crtc;
struct drm_crtc_state *crtc_state;
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state))
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da51a2bcb978..b629c8e990f4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -457,10 +457,11 @@ EXPORT_SYMBOL(drmm_connector_init);
* @dev: DRM device
* @connector: A pointer to the HDMI connector to init
* @funcs: callbacks for this connector
* @connector_type: user visible type of the connector
* @ddc: optional pointer to the associated ddc adapter
+ * @supported_formats: Bitmask of @hdmi_colorspace listing supported output formats
* @max_bpc: Maximum bits per char the HDMI connector supports
*
* Initialises a preallocated HDMI connector. Connectors can be
* subclassed as part of driver connector objects.
*
@@ -475,25 +476,31 @@ EXPORT_SYMBOL(drmm_connector_init);
int drmm_connector_hdmi_init(struct drm_device *dev,
struct drm_connector *connector,
const struct drm_connector_funcs *funcs,
int connector_type,
struct i2c_adapter *ddc,
+ unsigned long supported_formats,
unsigned int max_bpc)
{
int ret;
if (!(connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_HDMIB))
return -EINVAL;
+ if (!supported_formats || !(supported_formats & BIT(HDMI_COLORSPACE_RGB)))
+ return -EINVAL;
+
if (!(max_bpc == 8 || max_bpc == 10 || max_bpc == 12))
return -EINVAL;
ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
if (ret)
return ret;
+ connector->hdmi.supported_formats = supported_formats;
+
/*
* drm_connector_attach_max_bpc_property() requires the
* connector to have a state.
*/
if (connector->funcs->reset)
@@ -1199,10 +1206,34 @@ static const u32 dp_colorspaces =
BIT(DRM_MODE_COLORIMETRY_SYCC_601) |
BIT(DRM_MODE_COLORIMETRY_OPYCC_601) |
BIT(DRM_MODE_COLORIMETRY_BT2020_CYCC) |
BIT(DRM_MODE_COLORIMETRY_BT2020_YCC);
+static const char * const output_format_str[] = {
+ [HDMI_COLORSPACE_RGB] = "RGB",
+ [HDMI_COLORSPACE_YUV420] = "YUV 4:2:0",
+ [HDMI_COLORSPACE_YUV422] = "YUV 4:2:2",
+ [HDMI_COLORSPACE_YUV444] = "YUV 4:4:4",
+};
+
+/*
+ * drm_hdmi_connector_get_output_format_name() - Return a string for HDMI connector output format
+ * @fmt: Output format to compute name of
+ *
+ * Returns: the name of the output format, or NULL if the type is not
+ * valid.
+ */
+const char *
+drm_hdmi_connector_get_output_format_name(enum hdmi_colorspace fmt)
+{
+ if (fmt >= ARRAY_SIZE(output_format_str))
+ return NULL;
+
+ return output_format_str[fmt];
+}
+EXPORT_SYMBOL(drm_hdmi_connector_get_output_format_name);
+
/**
* DOC: standard connector properties
*
* DRM connectors have a few standardized properties:
*
diff --git a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
index 5d1a1dbff433..a129ea5f014c 100644
--- a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
@@ -145,10 +145,11 @@ static const struct drm_connector_funcs dummy_connector_funcs = {
};
static
struct drm_atomic_helper_connector_hdmi_priv *
drm_atomic_helper_connector_hdmi_init(struct kunit *test,
+ unsigned int formats,
unsigned int max_bpc)
{
struct drm_atomic_helper_connector_hdmi_priv *priv;
struct drm_connector *conn;
struct drm_encoder *enc;
@@ -188,10 +189,11 @@ drm_atomic_helper_connector_hdmi_init(struct kunit *test,
conn = &priv->connector;
ret = drmm_connector_hdmi_init(drm, conn,
&dummy_connector_funcs,
DRM_MODE_CONNECTOR_HDMIA,
NULL,
+ formats,
max_bpc);
KUNIT_ASSERT_EQ(test, ret, 0);
drm_connector_helper_add(conn, &dummy_connector_helper_funcs);
drm_connector_attach_encoder(conn, enc);
@@ -223,11 +225,13 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
struct drm_connector *conn;
struct drm_device *drm;
struct drm_crtc *crtc;
int ret;
- priv = drm_atomic_helper_connector_hdmi_init(test, 10);
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 10);
KUNIT_ASSERT_NOT_NULL(test, priv);
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
@@ -290,11 +294,13 @@ static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
struct drm_connector *conn;
struct drm_device *drm;
struct drm_crtc *crtc;
int ret;
- priv = drm_atomic_helper_connector_hdmi_init(test, 10);
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 10);
KUNIT_ASSERT_NOT_NULL(test, priv);
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
@@ -359,11 +365,13 @@ static void drm_test_check_bpc_8_value(struct kunit *test)
{
struct drm_atomic_helper_connector_hdmi_priv *priv;
struct drm_connector_state *conn_state;
struct drm_connector *conn;
- priv = drm_atomic_helper_connector_hdmi_init(test, 8);
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
KUNIT_ASSERT_NOT_NULL(test, priv);
conn = &priv->connector;
conn_state = conn->state;
KUNIT_EXPECT_EQ(test, conn_state->max_bpc, 8);
@@ -381,11 +389,13 @@ static void drm_test_check_bpc_10_value(struct kunit *test)
{
struct drm_atomic_helper_connector_hdmi_priv *priv;
struct drm_connector_state *conn_state;
struct drm_connector *conn;
- priv = drm_atomic_helper_connector_hdmi_init(test, 10);
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 10);
KUNIT_ASSERT_NOT_NULL(test, priv);
conn = &priv->connector;
conn_state = conn->state;
KUNIT_EXPECT_EQ(test, conn_state->max_bpc, 10);
@@ -403,11 +413,13 @@ static void drm_test_check_bpc_12_value(struct kunit *test)
{
struct drm_atomic_helper_connector_hdmi_priv *priv;
struct drm_connector_state *conn_state;
struct drm_connector *conn;
- priv = drm_atomic_helper_connector_hdmi_init(test, 12);
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 12);
KUNIT_ASSERT_NOT_NULL(test, priv);
conn = &priv->connector;
conn_state = conn->state;
KUNIT_EXPECT_EQ(test, conn_state->max_bpc, 12);
diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index 2519b91de95e..9589867bdf7c 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -185,10 +185,11 @@ static void drm_test_connector_hdmi_init_valid(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
}
/*
@@ -202,10 +203,11 @@ static void drm_test_connector_hdmi_init_null_ddc(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
DRM_MODE_CONNECTOR_HDMIA,
NULL,
+ BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
}
/*
@@ -219,10 +221,11 @@ static void drm_test_connector_hdmi_init_bpc_invalid(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
9);
KUNIT_EXPECT_LT(test, ret, 0);
}
/*
@@ -236,10 +239,11 @@ static void drm_test_connector_hdmi_init_bpc_null(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
0);
KUNIT_EXPECT_LT(test, ret, 0);
}
/*
@@ -257,10 +261,11 @@ static void drm_test_connector_hdmi_init_bpc_8(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, connector,
&dummy_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
prop = connector->max_bpc_property;
KUNIT_ASSERT_NOT_NULL(test, prop);
@@ -290,10 +295,11 @@ static void drm_test_connector_hdmi_init_bpc_10(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, connector,
&dummy_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
10);
KUNIT_EXPECT_EQ(test, ret, 0);
prop = connector->max_bpc_property;
KUNIT_ASSERT_NOT_NULL(test, prop);
@@ -323,10 +329,11 @@ static void drm_test_connector_hdmi_init_bpc_12(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, connector,
&dummy_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
12);
KUNIT_EXPECT_EQ(test, ret, 0);
prop = connector->max_bpc_property;
KUNIT_ASSERT_NOT_NULL(test, prop);
@@ -353,10 +360,11 @@ static void drm_test_connector_hdmi_init_type_valid(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
connector_type,
&priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
}
static const unsigned int drm_connector_hdmi_init_type_valid_tests[] = {
@@ -385,10 +393,11 @@ static void drm_test_connector_hdmi_init_type_invalid(struct kunit *test)
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
connector_type,
&priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_LT(test, ret, 0);
}
static const unsigned int drm_connector_hdmi_init_type_invalid_tests[] = {
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index d4d2ae15bc1e..29883e6f8e50 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -366,10 +366,13 @@ enum drm_panel_orientation {
DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP,
DRM_MODE_PANEL_ORIENTATION_LEFT_UP,
DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
};
+const char *
+drm_hdmi_connector_get_output_format_name(enum hdmi_colorspace fmt);
+
/**
* struct drm_monitor_range_info - Panel's Monitor range in EDID for
* &drm_display_info
*
* This struct is used to store a frequency range supported by panel
@@ -1039,10 +1042,15 @@ struct drm_connector_state {
struct {
/**
* @output_bpc: Bits per color channel to output.
*/
unsigned int output_bpc;
+
+ /**
+ * @output_format: Pixel format to output in.
+ */
+ enum hdmi_colorspace output_format;
} hdmi;
};
/**
* struct drm_connector_funcs - control connectors on a given device
@@ -1900,10 +1908,21 @@ struct drm_connector {
*/
struct llist_node free_node;
/** @hdr_sink_metadata: HDR Metadata Information read from sink */
struct hdr_sink_metadata hdr_sink_metadata;
+
+ /**
+ * @hdmi: HDMI-related variable and properties.
+ */
+ struct {
+ /**
+ * @supported_formats: Bitmask of @hdmi_colorspace
+ * supported by the controller.
+ */
+ unsigned long supported_formats;
+ } hdmi;
};
#define obj_to_connector(x) container_of(x, struct drm_connector, base)
int drm_connector_init(struct drm_device *dev,
@@ -1923,10 +1942,11 @@ int drmm_connector_init(struct drm_device *dev,
int drmm_connector_hdmi_init(struct drm_device *dev,
struct drm_connector *connector,
const struct drm_connector_funcs *funcs,
int connector_type,
struct i2c_adapter *ddc,
+ unsigned long supported_formats,
unsigned int max_bpc);
void drm_connector_attach_edid_property(struct drm_connector *connector);
int drm_connector_register(struct drm_connector *connector);
void drm_connector_unregister(struct drm_connector *connector);
int drm_connector_attach_encoder(struct drm_connector *connector,
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v10 12/27] drm/connector: hdmi: Add custom hook to filter TMDS character rate
From: Maxime Ripard @ 2024-03-21 15:29 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jonathan Corbet, Sandy Huang, Heiko Stübner, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: Hans Verkuil, Sebastian Wick, Ville Syrjälä, dri-devel,
linux-arm-kernel, linux-doc, linux-kernel, linux-media,
linux-rockchip, linux-sunxi, Maxime Ripard, Dave Stevenson
In-Reply-To: <20240321-kms-hdmi-connector-state-v10-0-e6c178361898@kernel.org>
Most of the HDMI controllers have an upper TMDS character rate limit
they can't exceed. On "embedded"-grade display controllers, it will
typically be lower than what high-grade monitors can provide these days,
so drivers will filter the TMDS character rate based on the controller
capabilities.
To make that easier to handle for drivers, let's provide an optional
hook to be implemented by drivers so they can tell the HDMI controller
helpers if a given TMDS character rate is reachable for them or not.
This will then be useful to figure out the best format and bpc count for
a given mode.
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/drm_atomic_state_helper.c | 9 +++++++
drivers/gpu/drm/drm_connector.c | 4 +++
.../gpu/drm/tests/drm_atomic_state_helper_test.c | 4 +++
drivers/gpu/drm/tests/drm_connector_test.c | 14 ++++++++++
include/drm/drm_connector.h | 30 ++++++++++++++++++++++
5 files changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 7258178bc385..4c58e03aa82f 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -665,15 +665,24 @@ connector_state_get_mode(const struct drm_connector_state *conn_state)
static enum drm_mode_status
hdmi_clock_valid(const struct drm_connector *connector,
const struct drm_display_mode *mode,
unsigned long long clock)
{
+ const struct drm_connector_hdmi_funcs *funcs = connector->hdmi.funcs;
const struct drm_display_info *info = &connector->display_info;
if (info->max_tmds_clock && clock > info->max_tmds_clock * 1000)
return MODE_CLOCK_HIGH;
+ if (funcs && funcs->tmds_char_rate_valid) {
+ enum drm_mode_status status;
+
+ status = funcs->tmds_char_rate_valid(connector, mode, clock);
+ if (status != MODE_OK)
+ return status;
+ }
+
return MODE_OK;
}
static int
hdmi_compute_clock(const struct drm_connector *connector,
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 8cc1332f11c2..591d2d500f61 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -455,10 +455,11 @@ EXPORT_SYMBOL(drmm_connector_init);
/**
* drmm_connector_hdmi_init - Init a preallocated HDMI connector
* @dev: DRM device
* @connector: A pointer to the HDMI connector to init
* @funcs: callbacks for this connector
+ * @hdmi_funcs: HDMI-related callbacks for this connector
* @connector_type: user visible type of the connector
* @ddc: optional pointer to the associated ddc adapter
* @supported_formats: Bitmask of @hdmi_colorspace listing supported output formats
* @max_bpc: Maximum bits per char the HDMI connector supports
*
@@ -474,10 +475,11 @@ EXPORT_SYMBOL(drmm_connector_init);
* Zero on success, error code on failure.
*/
int drmm_connector_hdmi_init(struct drm_device *dev,
struct drm_connector *connector,
const struct drm_connector_funcs *funcs,
+ const struct drm_connector_hdmi_funcs *hdmi_funcs,
int connector_type,
struct i2c_adapter *ddc,
unsigned long supported_formats,
unsigned int max_bpc)
{
@@ -510,10 +512,12 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
connector->max_bpc = max_bpc;
if (max_bpc > 8)
drm_connector_attach_hdr_output_metadata_property(connector);
+ connector->hdmi.funcs = hdmi_funcs;
+
return 0;
}
EXPORT_SYMBOL(drmm_connector_hdmi_init);
/**
diff --git a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
index dfed45b250a5..989661ad3aee 100644
--- a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
@@ -108,10 +108,13 @@ static int set_connector_edid(struct kunit *test, struct drm_connector *connecto
KUNIT_ASSERT_GT(test, ret, 0);
return 0;
}
+static const struct drm_connector_hdmi_funcs dummy_connector_hdmi_funcs = {
+};
+
static int dummy_connector_get_modes(struct drm_connector *connector)
{
struct drm_atomic_helper_connector_hdmi_priv *priv =
connector_to_priv(connector);
const struct drm_edid *edid;
@@ -190,10 +193,11 @@ drm_atomic_helper_connector_hdmi_init(struct kunit *test,
enc->possible_crtcs = drm_crtc_mask(priv->crtc);
conn = &priv->connector;
ret = drmm_connector_hdmi_init(drm, conn,
&dummy_connector_funcs,
+ &dummy_connector_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
NULL,
formats,
max_bpc);
KUNIT_ASSERT_EQ(test, ret, 0);
diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index 4d1ac744a844..ac4b8d00e992 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -20,10 +20,13 @@ struct drm_connector_init_priv {
struct drm_device drm;
struct drm_connector connector;
struct i2c_adapter ddc;
};
+static const struct drm_connector_hdmi_funcs dummy_hdmi_funcs = {
+};
+
static const struct drm_connector_funcs dummy_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.reset = drm_atomic_helper_connector_reset,
};
@@ -185,10 +188,11 @@ static void drm_test_connector_hdmi_init_valid(struct kunit *test)
struct drm_connector_init_priv *priv = test->priv;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
@@ -203,10 +207,11 @@ static void drm_test_connector_hdmi_init_null_ddc(struct kunit *test)
struct drm_connector_init_priv *priv = test->priv;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
NULL,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
@@ -221,10 +226,11 @@ static void drm_test_connector_hdmi_init_bpc_invalid(struct kunit *test)
struct drm_connector_init_priv *priv = test->priv;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
9);
KUNIT_EXPECT_LT(test, ret, 0);
@@ -239,10 +245,11 @@ static void drm_test_connector_hdmi_init_bpc_null(struct kunit *test)
struct drm_connector_init_priv *priv = test->priv;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
0);
KUNIT_EXPECT_LT(test, ret, 0);
@@ -261,10 +268,11 @@ static void drm_test_connector_hdmi_init_bpc_8(struct kunit *test)
uint64_t val;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
@@ -295,10 +303,11 @@ static void drm_test_connector_hdmi_init_bpc_10(struct kunit *test)
uint64_t val;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
10);
KUNIT_EXPECT_EQ(test, ret, 0);
@@ -329,10 +338,11 @@ static void drm_test_connector_hdmi_init_bpc_12(struct kunit *test)
uint64_t val;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
12);
KUNIT_EXPECT_EQ(test, ret, 0);
@@ -359,10 +369,11 @@ static void drm_test_connector_hdmi_init_formats_empty(struct kunit *test)
struct drm_connector_init_priv *priv = test->priv;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
0,
8);
KUNIT_EXPECT_LT(test, ret, 0);
@@ -377,10 +388,11 @@ static void drm_test_connector_hdmi_init_formats_no_rgb(struct kunit *test)
struct drm_connector_init_priv *priv = test->priv;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_YUV422),
8);
KUNIT_EXPECT_LT(test, ret, 0);
@@ -396,10 +408,11 @@ static void drm_test_connector_hdmi_init_type_valid(struct kunit *test)
unsigned int connector_type = *(unsigned int *)test->param_value;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
connector_type,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
@@ -429,10 +442,11 @@ static void drm_test_connector_hdmi_init_type_invalid(struct kunit *test)
unsigned int connector_type = *(unsigned int *)test->param_value;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
&dummy_funcs,
+ &dummy_hdmi_funcs,
connector_type,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_LT(test, ret, 0);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index e3917ea93986..8cda902934cd 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1056,10 +1056,34 @@ struct drm_connector_state {
*/
unsigned long long tmds_char_rate;
} hdmi;
};
+/**
+ * struct drm_connector_hdmi_funcs - drm_hdmi_connector control functions
+ */
+struct drm_connector_hdmi_funcs {
+ /**
+ * @tmds_char_rate_valid:
+ *
+ * This callback is invoked at atomic_check time to figure out
+ * whether a particular TMDS character rate is supported by the
+ * driver.
+ *
+ * The @tmds_char_rate_valid callback is optional.
+ *
+ * Returns:
+ *
+ * Either &drm_mode_status.MODE_OK or one of the failure reasons
+ * in &enum drm_mode_status.
+ */
+ enum drm_mode_status
+ (*tmds_char_rate_valid)(const struct drm_connector *connector,
+ const struct drm_display_mode *mode,
+ unsigned long long tmds_rate);
+};
+
/**
* struct drm_connector_funcs - control connectors on a given device
*
* Each CRTC may have one or more connectors attached to it. The functions
* below allow the core DRM code to control connectors, enumerate available modes,
@@ -1924,10 +1948,15 @@ struct drm_connector {
/**
* @supported_formats: Bitmask of @hdmi_colorspace
* supported by the controller.
*/
unsigned long supported_formats;
+
+ /**
+ * @funcs: HDMI connector Control Functions
+ */
+ const struct drm_connector_hdmi_funcs *funcs;
} hdmi;
};
#define obj_to_connector(x) container_of(x, struct drm_connector, base)
@@ -1946,10 +1975,11 @@ int drmm_connector_init(struct drm_device *dev,
int connector_type,
struct i2c_adapter *ddc);
int drmm_connector_hdmi_init(struct drm_device *dev,
struct drm_connector *connector,
const struct drm_connector_funcs *funcs,
+ const struct drm_connector_hdmi_funcs *hdmi_funcs,
int connector_type,
struct i2c_adapter *ddc,
unsigned long supported_formats,
unsigned int max_bpc);
void drm_connector_attach_edid_property(struct drm_connector *connector);
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2 8/8] drm: zynqmp_dp: Add debugfs interface for compliance testing
From: Sean Anderson @ 2024-03-21 16:35 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Michal Simek, David Airlie, linux-kernel, Daniel Vetter,
linux-arm-kernel, Laurent Pinchart, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, dri-devel
In-Reply-To: <aa559c02-b8df-41f8-9439-d7182ac55ffe@ideasonboard.com>
On 3/21/24 12:31, Tomi Valkeinen wrote:
> On 21/03/2024 18:08, Sean Anderson wrote:
>> On 3/20/24 03:49, Tomi Valkeinen wrote:
>>> On 20/03/2024 00:51, Sean Anderson wrote:
>>>
>>>> +/**
>>>> + * enum test_pattern - Test patterns for test testing
>>>
>>> "for test testing"? =)
>>>
>>>> @@ -1655,6 +2321,9 @@ static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
>>>> u8 status[DP_LINK_STATUS_SIZE + 2];
>>>> int err;
>>>> + if (READ_ONCE(dp->ignore_hpd))
>>>> + return;
>>>> +
>>>> mutex_lock(&dp->lock);
>>>> err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
>>>> DP_LINK_STATUS_SIZE + 2);
>>>
>>> Why do you need READ/WRITE_ONCE() for ignore_hpd?
>>
>> It's not protected by dp->lock so we don't have to take it for
>> zynqmp_dp_hpd_work_func. Although maybe we should make a version of
>> zynqmp_dp_bridge_detect which assumes we already hold the lock.
>
> Does using the macros solve some potential issue, or is it just for documenting that this variable is accessed without lock?
Without this the compiler is free to issue multiple loads for this
variable, which could be incorrect.
--Sean
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/4] coresight: Add support for multiple output ports on the funnel
From: Suzuki K Poulose @ 2024-03-21 16:41 UTC (permalink / raw)
To: Tao Zhang, Mathieu Poirier, Alexander Shishkin, Konrad Dybcio,
Mike Leach, Rob Herring, Krzysztof Kozlowski
Cc: Jinlong Mao, Leo Yan, Greg Kroah-Hartman, coresight,
linux-arm-kernel, linux-kernel, devicetree, Tingwei Zhang,
Yuanfang Zhang, Trilok Soni, Song Chai, linux-arm-msm, andersson
In-Reply-To: <1711009927-17873-3-git-send-email-quic_taozha@quicinc.com>
On 21/03/2024 08:32, Tao Zhang wrote:
> Funnel devices are now capable of supporting multiple-inputs and
> multiple-outputs configuration with in built hardware filtering
> for TPDM devices. Add software support to this function. Output
> port is selected according to the source in the trace path.
>
> The source of the input port on funnels will be marked in the
> device tree.
> e.g.
> tpdm@xxxxxxx {
> ... ... ... ...
> };
>
> funnel_XXX: funnel@xxxxxxx {
> ... ... ... ...
> out-ports {
> ... ... ... ...
> port@x {
> ... ... ... ...
> label = "xxxxxxx.tpdm"; <-- To label the source
> }; corresponding to the output
> ... ... ... ... connection "port@x". And this
> }; is a hardware static connections.
> ... ... ... ... Here needs to refer to hardware
> }; design.
>
> Then driver will parse the source label marked in the device tree, and
> save it to the coresight path. When the function needs to know the
> source label, it could obtain it from coresight path parameter. Finally,
> the output port knows which source it corresponds to, and it also knows
> which input port it corresponds to.
Why do we need labels ? We have connection information for all devices
(both in and out), so, why do we need this label to find a device ?
And also, I thought TPDM is a source device, why does a funnel output
port link to a source ?
Are these funnels programmable ? Or, are they static ? If they are
static, do these need to be described in the DT ? If they are simply
acting as a "LINK" (or HWFIFO ?)
Suzuki
>
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
> .../hwtracing/coresight/coresight-platform.c | 5 ++
> include/linux/coresight.h | 2 +
> 3 files changed, 75 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 5dde597403b3..b1b5e6d9ec7a 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -113,15 +113,63 @@ struct coresight_device *coresight_get_percpu_sink(int cpu)
> }
> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>
> +static struct coresight_device *coresight_get_source(struct list_head *path)
> +{
> + struct coresight_device *csdev;
> +
> + if (!path)
> + return NULL;
> +
> + csdev = list_first_entry(path, struct coresight_node, link)->csdev;
> + if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
> + return NULL;
> +
> + return csdev;
> +}
> +
> +/**
> + * coresight_source_filter - checks whether the connection matches the source
> + * of path if connection is binded to specific source.
> + * @path: The list of devices
> + * @conn: The connection of one outport
> + *
> + * Return zero if the connection doesn't have a source binded or source of the
> + * path matches the source binds to connection.
> + */
> +static int coresight_source_filter(struct list_head *path,
> + struct coresight_connection *conn)
> +{
> + int ret = 0;
> + struct coresight_device *source = NULL;
> +
> + if (conn->source_label == NULL)
> + return ret;
> +
> + source = coresight_get_source(path);
> + if (source == NULL)
> + return ret;
> +
> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> + conn->source_label))
> + ret = 0;
> + else
> + ret = -1;
> +
> + return ret;
> +}
> +
> static struct coresight_connection *
> coresight_find_out_connection(struct coresight_device *src_dev,
> - struct coresight_device *dest_dev)
> + struct coresight_device *dest_dev,
> + struct list_head *path)
> {
> int i;
> struct coresight_connection *conn;
>
> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> conn = src_dev->pdata->out_conns[i];
> + if (coresight_source_filter(path, conn))
> + continue;
> if (conn->dest_dev == dest_dev)
> return conn;
> }
> @@ -312,7 +360,8 @@ static void coresight_disable_sink(struct coresight_device *csdev)
>
> static int coresight_enable_link(struct coresight_device *csdev,
> struct coresight_device *parent,
> - struct coresight_device *child)
> + struct coresight_device *child,
> + struct list_head *path)
> {
> int ret = 0;
> int link_subtype;
> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
> if (!parent || !child)
> return -EINVAL;
>
> - inconn = coresight_find_out_connection(parent, csdev);
> - outconn = coresight_find_out_connection(csdev, child);
> + inconn = coresight_find_out_connection(parent, csdev, path);
> + outconn = coresight_find_out_connection(csdev, child, path);
> link_subtype = csdev->subtype.link_subtype;
>
> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && IS_ERR(inconn))
> @@ -341,7 +390,8 @@ static int coresight_enable_link(struct coresight_device *csdev,
>
> static void coresight_disable_link(struct coresight_device *csdev,
> struct coresight_device *parent,
> - struct coresight_device *child)
> + struct coresight_device *child,
> + struct list_head *path)
> {
> int i;
> int link_subtype;
> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct coresight_device *csdev,
> if (!parent || !child)
> return;
>
> - inconn = coresight_find_out_connection(parent, csdev);
> - outconn = coresight_find_out_connection(csdev, child);
> + inconn = coresight_find_out_connection(parent, csdev, path);
> + outconn = coresight_find_out_connection(csdev, child, path);
> link_subtype = csdev->subtype.link_subtype;
>
> if (link_ops(csdev)->disable) {
> @@ -507,7 +557,7 @@ static void coresight_disable_path_from(struct list_head *path,
> case CORESIGHT_DEV_TYPE_LINK:
> parent = list_prev_entry(nd, link)->csdev;
> child = list_next_entry(nd, link)->csdev;
> - coresight_disable_link(csdev, parent, child);
> + coresight_disable_link(csdev, parent, child, path);
> break;
> default:
> break;
> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path, enum cs_mode mode,
> case CORESIGHT_DEV_TYPE_LINK:
> parent = list_prev_entry(nd, link)->csdev;
> child = list_next_entry(nd, link)->csdev;
> - ret = coresight_enable_link(csdev, parent, child);
> + ret = coresight_enable_link(csdev, parent, child, path);
> if (ret)
> goto err;
> break;
> @@ -802,7 +852,8 @@ static void coresight_drop_device(struct coresight_device *csdev)
> */
> static int _coresight_build_path(struct coresight_device *csdev,
> struct coresight_device *sink,
> - struct list_head *path)
> + struct list_head *path,
> + struct coresight_device *source)
> {
> int i, ret;
> bool found = false;
> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct coresight_device *csdev,
>
> if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) &&
> sink == per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev))) {
> - if (_coresight_build_path(sink, sink, path) == 0) {
> + if (_coresight_build_path(sink, sink, path, source) == 0) {
> found = true;
> goto out;
> }
> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct coresight_device *csdev,
> struct coresight_device *child_dev;
>
> child_dev = csdev->pdata->out_conns[i]->dest_dev;
> + if (csdev->pdata->out_conns[i]->source_label &&
> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> + csdev->pdata->out_conns[i]->source_label))
> + continue;
> if (child_dev &&
> - _coresight_build_path(child_dev, sink, path) == 0) {
> + _coresight_build_path(child_dev, sink, path, source) == 0) {
> found = true;
> break;
> }
> @@ -871,7 +926,7 @@ struct list_head *coresight_build_path(struct coresight_device *source,
>
> INIT_LIST_HEAD(path);
>
> - rc = _coresight_build_path(source, sink, path);
> + rc = _coresight_build_path(source, sink, path, source);
> if (rc) {
> kfree(path);
> return ERR_PTR(rc);
> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> index 9d550f5697fa..f553fb20966d 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct device *dev,
> struct fwnode_handle *rdev_fwnode;
> struct coresight_connection conn = {};
> struct coresight_connection *new_conn;
> + const char *label;
>
> do {
> /* Parse the local port details */
> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct device *dev,
> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> conn.dest_port = rendpoint.port;
>
> + conn.source_label = NULL;
> + if (!of_property_read_string(ep, "label", &label))
> + conn.source_label = label;
> +
> new_conn = coresight_add_out_conn(dev, pdata, &conn);
> if (IS_ERR_VALUE(new_conn)) {
> fwnode_handle_put(conn.dest_fwnode);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index e8b6e388218c..a9c06ef9bbb2 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -167,6 +167,7 @@ struct coresight_desc {
> * struct coresight_connection - representation of a single connection
> * @src_port: a connection's output port number.
> * @dest_port: destination's input port number @src_port is connected to.
> + * @source_label: source component's label.
> * @dest_fwnode: destination component's fwnode handle.
> * @dest_dev: a @coresight_device representation of the component
> connected to @src_port. NULL until the device is created
> @@ -195,6 +196,7 @@ struct coresight_desc {
> struct coresight_connection {
> int src_port;
> int dest_port;
> + const char *source_label;
> struct fwnode_handle *dest_fwnode;
> struct coresight_device *dest_dev;
> struct coresight_sysfs_link *link;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v10 14/27] drm/connector: hdmi: Compute bpc and format automatically
From: Maxime Ripard @ 2024-03-21 15:29 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jonathan Corbet, Sandy Huang, Heiko Stübner, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: Hans Verkuil, Sebastian Wick, Ville Syrjälä, dri-devel,
linux-arm-kernel, linux-doc, linux-kernel, linux-media,
linux-rockchip, linux-sunxi, Maxime Ripard
In-Reply-To: <20240321-kms-hdmi-connector-state-v10-0-e6c178361898@kernel.org>
Now that we have all the infrastructure needed, we can add some code
that will, for a given connector state and mode, compute the best output
format and bpc.
The algorithm is equivalent to the one already found in i915 and vc4.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/drm_atomic_state_helper.c | 196 ++++++++++++++++++++-
.../gpu/drm/tests/drm_atomic_state_helper_test.c | 25 ++-
2 files changed, 209 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 4c58e03aa82f..50f1789dce15 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -29,10 +29,11 @@
#include <drm/drm_blend.h>
#include <drm/drm_bridge.h>
#include <drm/drm_connector.h>
#include <drm/drm_crtc.h>
#include <drm/drm_device.h>
+#include <drm/drm_edid.h>
#include <drm/drm_framebuffer.h>
#include <drm/drm_plane.h>
#include <drm/drm_print.h>
#include <drm/drm_vblank.h>
#include <drm/drm_writeback.h>
@@ -660,10 +661,110 @@ connector_state_get_mode(const struct drm_connector_state *conn_state)
return NULL;
return &crtc_state->mode;
}
+static bool
+sink_supports_format_bpc(const struct drm_connector *connector,
+ const struct drm_display_info *info,
+ const struct drm_display_mode *mode,
+ unsigned int format, unsigned int bpc)
+{
+ struct drm_device *dev = connector->dev;
+ u8 vic = drm_match_cea_mode(mode);
+
+ /*
+ * CTA-861-F, section 5.4 - Color Coding & Quantization states
+ * that the bpc must be 8, 10, 12 or 16 except for the default
+ * 640x480 VIC1 where the value must be 8.
+ *
+ * The definition of default here is ambiguous but the spec
+ * refers to VIC1 being the default timing in several occasions
+ * so our understanding is that for the default timing (ie,
+ * VIC1), the bpc must be 8.
+ */
+ if (vic == 1 && bpc != 8) {
+ drm_dbg_kms(dev, "VIC1 requires a bpc of 8, got %u\n", bpc);
+ return false;
+ }
+
+ if (!info->is_hdmi &&
+ (format != HDMI_COLORSPACE_RGB || bpc != 8)) {
+ drm_dbg_kms(dev, "DVI Monitors require an RGB output at 8 bpc\n");
+ return false;
+ }
+
+ if (!(connector->hdmi.supported_formats & BIT(format))) {
+ drm_dbg_kms(dev, "%s format unsupported by the connector.\n",
+ drm_hdmi_connector_get_output_format_name(format));
+ return false;
+ }
+
+ switch (format) {
+ case HDMI_COLORSPACE_RGB:
+ drm_dbg_kms(dev, "RGB Format, checking the constraints.\n");
+
+ if (!(info->color_formats & DRM_COLOR_FORMAT_RGB444))
+ return false;
+
+ if (bpc == 10 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30)) {
+ drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
+ return false;
+ }
+
+ if (bpc == 12 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36)) {
+ drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
+ return false;
+ }
+
+ drm_dbg_kms(dev, "RGB format supported in that configuration.\n");
+
+ return true;
+
+ case HDMI_COLORSPACE_YUV422:
+ drm_dbg_kms(dev, "YUV422 format, checking the constraints.\n");
+
+ if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR422)) {
+ drm_dbg_kms(dev, "Sink doesn't support YUV422.\n");
+ return false;
+ }
+
+ if (bpc != 12) {
+ drm_dbg_kms(dev, "YUV422 only supports 12 bpc.\n");
+ return false;
+ }
+
+ drm_dbg_kms(dev, "YUV422 format supported in that configuration.\n");
+
+ return true;
+
+ case HDMI_COLORSPACE_YUV444:
+ drm_dbg_kms(dev, "YUV444 format, checking the constraints.\n");
+
+ if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR444)) {
+ drm_dbg_kms(dev, "Sink doesn't support YUV444.\n");
+ return false;
+ }
+
+ if (bpc == 10 && !(info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_30)) {
+ drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
+ return false;
+ }
+
+ if (bpc == 12 && !(info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_36)) {
+ drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
+ return false;
+ }
+
+ drm_dbg_kms(dev, "YUV444 format supported in that configuration.\n");
+
+ return true;
+ }
+
+ return false;
+}
+
static enum drm_mode_status
hdmi_clock_valid(const struct drm_connector *connector,
const struct drm_display_mode *mode,
unsigned long long clock)
{
@@ -704,10 +805,101 @@ hdmi_compute_clock(const struct drm_connector *connector,
conn_state->hdmi.tmds_char_rate = clock;
return 0;
}
+static bool
+hdmi_try_format_bpc(const struct drm_connector *connector,
+ struct drm_connector_state *conn_state,
+ const struct drm_display_mode *mode,
+ unsigned int bpc, enum hdmi_colorspace fmt)
+{
+ const struct drm_display_info *info = &connector->display_info;
+ struct drm_device *dev = connector->dev;
+ int ret;
+
+ drm_dbg_kms(dev, "Trying %s output format\n",
+ drm_hdmi_connector_get_output_format_name(fmt));
+
+ if (!sink_supports_format_bpc(connector, info, mode, fmt, bpc)) {
+ drm_dbg_kms(dev, "%s output format not supported with %u bpc\n",
+ drm_hdmi_connector_get_output_format_name(fmt),
+ bpc);
+ return false;
+ }
+
+ ret = hdmi_compute_clock(connector, conn_state, mode, bpc, fmt);
+ if (ret) {
+ drm_dbg_kms(dev, "Couldn't compute clock for %s output format and %u bpc\n",
+ drm_hdmi_connector_get_output_format_name(fmt),
+ bpc);
+ return false;
+ }
+
+ drm_dbg_kms(dev, "%s output format supported with %u (TMDS char rate: %llu Hz)\n",
+ drm_hdmi_connector_get_output_format_name(fmt),
+ bpc, conn_state->hdmi.tmds_char_rate);
+
+ return true;
+}
+
+static int
+hdmi_compute_format(const struct drm_connector *connector,
+ struct drm_connector_state *conn_state,
+ const struct drm_display_mode *mode,
+ unsigned int bpc)
+{
+ struct drm_device *dev = connector->dev;
+
+ /*
+ * TODO: Add support for YCbCr420 output for HDMI 2.0 capable
+ * devices, for modes that only support YCbCr420.
+ */
+ if (hdmi_try_format_bpc(connector, conn_state, mode, bpc, HDMI_COLORSPACE_RGB)) {
+ conn_state->hdmi.output_format = HDMI_COLORSPACE_RGB;
+ return 0;
+ }
+
+ drm_dbg_kms(dev, "Failed. No Format Supported for that bpc count.\n");
+
+ return -EINVAL;
+}
+
+static int
+hdmi_compute_config(const struct drm_connector *connector,
+ struct drm_connector_state *conn_state,
+ const struct drm_display_mode *mode)
+{
+ struct drm_device *dev = connector->dev;
+ unsigned int max_bpc = clamp_t(unsigned int,
+ conn_state->max_bpc,
+ 8, connector->max_bpc);
+ unsigned int bpc;
+ int ret;
+
+ for (bpc = max_bpc; bpc >= 8; bpc -= 2) {
+ drm_dbg_kms(dev, "Trying with a %d bpc output\n", bpc);
+
+ ret = hdmi_compute_format(connector, conn_state, mode, bpc);
+ if (ret)
+ continue;
+
+ conn_state->hdmi.output_bpc = bpc;
+
+ drm_dbg_kms(dev,
+ "Mode %ux%u @ %uHz: Found configuration: bpc: %u, fmt: %s, clock: %llu\n",
+ mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode),
+ conn_state->hdmi.output_bpc,
+ drm_hdmi_connector_get_output_format_name(conn_state->hdmi.output_format),
+ conn_state->hdmi.tmds_char_rate);
+
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
/**
* drm_atomic_helper_connector_hdmi_check() - Helper to check HDMI connector atomic state
* @connector: DRM Connector
* @state: the DRM State object
*
@@ -727,13 +919,11 @@ int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
drm_atomic_get_new_connector_state(state, connector);
const struct drm_display_mode *mode =
connector_state_get_mode(new_conn_state);
int ret;
- ret = hdmi_compute_clock(connector, new_conn_state, mode,
- new_conn_state->hdmi.output_bpc,
- new_conn_state->hdmi.output_format);
+ ret = hdmi_compute_config(connector, new_conn_state, mode);
if (ret)
return ret;
if (old_conn_state->hdmi.output_bpc != new_conn_state->hdmi.output_bpc ||
old_conn_state->hdmi.output_format != new_conn_state->hdmi.output_format) {
diff --git a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
index 5a8750153510..f010fde0eb69 100644
--- a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
@@ -68,13 +68,10 @@ static int light_up_connector(struct kunit *test,
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
conn_state = drm_atomic_get_connector_state(state, connector);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
- conn_state->hdmi.output_bpc = connector->max_bpc;
- conn_state->hdmi.output_format = HDMI_COLORSPACE_RGB;
-
ret = drm_atomic_set_crtc_for_connector(conn_state, crtc);
KUNIT_EXPECT_EQ(test, ret, 0);
crtc_state = drm_atomic_get_crtc_state(state, crtc);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
@@ -249,14 +246,19 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
priv = drm_atomic_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
10);
KUNIT_ASSERT_NOT_NULL(test, priv);
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
- conn = &priv->connector;
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
drm = &priv->drm;
crtc = priv->crtc;
@@ -270,15 +272,15 @@ static void drm_test_check_output_bpc_crtc_mode_changed(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
old_conn_state = drm_atomic_get_old_connector_state(state, conn);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state);
- new_conn_state->hdmi.output_bpc = 8;
+ new_conn_state->max_requested_bpc = 8;
KUNIT_ASSERT_NE(test,
- old_conn_state->hdmi.output_bpc,
- new_conn_state->hdmi.output_bpc);
+ old_conn_state->max_requested_bpc,
+ new_conn_state->max_requested_bpc);
ret = drm_atomic_check_only(state);
KUNIT_ASSERT_EQ(test, ret, 0);
old_conn_state = drm_atomic_get_old_connector_state(state, conn);
@@ -318,14 +320,19 @@ static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
priv = drm_atomic_helper_connector_hdmi_init(test,
BIT(HDMI_COLORSPACE_RGB),
10);
KUNIT_ASSERT_NOT_NULL(test, priv);
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
ctx = drm_kunit_helper_acquire_ctx_alloc(test);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
- conn = &priv->connector;
preferred = find_preferred_mode(conn);
KUNIT_ASSERT_NOT_NULL(test, preferred);
drm = &priv->drm;
crtc = priv->crtc;
@@ -668,11 +675,11 @@ static void drm_test_check_format_value(struct kunit *test)
8);
KUNIT_ASSERT_NOT_NULL(test, priv);
conn = &priv->connector;
conn_state = conn->state;
- KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, 0);
}
/*
* Test that the value of the output format property out of reset is set
* to 0, and will be computed at atomic_check time.
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v10 17/27] drm/tests: Add tests for Broadcast RGB property
From: Maxime Ripard @ 2024-03-21 15:29 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jonathan Corbet, Sandy Huang, Heiko Stübner, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: Hans Verkuil, Sebastian Wick, Ville Syrjälä, dri-devel,
linux-arm-kernel, linux-doc, linux-kernel, linux-media,
linux-rockchip, linux-sunxi, Maxime Ripard, Dave Stevenson
In-Reply-To: <20240321-kms-hdmi-connector-state-v10-0-e6c178361898@kernel.org>
This had a bunch of kunit tests to make sure our code to handle the
Broadcast RGB property behaves properly.
This requires bringing a bit of infrastructure to create mock HDMI
connectors, with custom EDIDs.
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
.../gpu/drm/tests/drm_atomic_state_helper_test.c | 151 +++++++++++++++++++++
drivers/gpu/drm/tests/drm_connector_test.c | 116 ++++++++++++++++
2 files changed, 267 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
index da8a1840ff47..9f500a0bf8a8 100644
--- a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
@@ -222,10 +222,138 @@ drm_atomic_helper_connector_hdmi_init(struct kunit *test,
KUNIT_ASSERT_EQ(test, ret, 0);
return priv;
}
+/*
+ * Test that if we change the RGB quantization property to a different
+ * value, we trigger a mode change on the connector's CRTC, which will
+ * in turn disable/enable the connector.
+ */
+static void drm_test_check_broadcast_rgb_crtc_mode_changed(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *old_conn_state;
+ struct drm_connector_state *new_conn_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *preferred;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ conn = &priv->connector;
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+ new_conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state);
+
+ new_conn_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_FULL;
+
+ KUNIT_ASSERT_NE(test,
+ old_conn_state->hdmi.broadcast_rgb,
+ new_conn_state->hdmi.broadcast_rgb);
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ new_conn_state = drm_atomic_get_new_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
+ KUNIT_EXPECT_EQ(test, new_conn_state->hdmi.broadcast_rgb, DRM_HDMI_BROADCAST_RGB_FULL);
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+ KUNIT_EXPECT_TRUE(test, crtc_state->mode_changed);
+}
+
+/*
+ * Test that if we set the RGB quantization property to the same value,
+ * we don't trigger a mode change on the connector's CRTC and leave the
+ * connector unaffected.
+ */
+static void drm_test_check_broadcast_rgb_crtc_mode_not_changed(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *old_conn_state;
+ struct drm_connector_state *new_conn_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *preferred;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ conn = &priv->connector;
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, drm, ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+ new_conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state);
+
+ new_conn_state->hdmi.broadcast_rgb = old_conn_state->hdmi.broadcast_rgb;
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, old_conn_state);
+
+ new_conn_state = drm_atomic_get_new_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new_conn_state);
+
+ KUNIT_EXPECT_EQ(test,
+ old_conn_state->hdmi.broadcast_rgb,
+ new_conn_state->hdmi.broadcast_rgb);
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+ KUNIT_EXPECT_FALSE(test, crtc_state->mode_changed);
+}
+
/*
* Test that if we change the maximum bpc property to a different value,
* we trigger a mode change on the connector's CRTC, which will in turn
* disable/enable the connector.
*/
@@ -1060,10 +1188,12 @@ static void drm_test_check_output_bpc_format_display_8bpc_only(struct kunit *tes
KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
}
static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
+ KUNIT_CASE(drm_test_check_broadcast_rgb_crtc_mode_changed),
+ KUNIT_CASE(drm_test_check_broadcast_rgb_crtc_mode_not_changed),
KUNIT_CASE(drm_test_check_hdmi_funcs_reject_rate),
KUNIT_CASE(drm_test_check_max_tmds_rate_bpc_fallback),
KUNIT_CASE(drm_test_check_max_tmds_rate_format_fallback),
KUNIT_CASE(drm_test_check_output_bpc_crtc_mode_changed),
KUNIT_CASE(drm_test_check_output_bpc_crtc_mode_not_changed),
@@ -1091,10 +1221,30 @@ static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
static struct kunit_suite drm_atomic_helper_connector_hdmi_check_test_suite = {
.name = "drm_atomic_helper_connector_hdmi_check",
.test_cases = drm_atomic_helper_connector_hdmi_check_tests,
};
+/*
+ * Test that the value of the Broadcast RGB property out of reset is set
+ * to auto.
+ */
+static void drm_test_check_broadcast_rgb_value(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_connector_state *conn_state;
+ struct drm_connector *conn;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ conn_state = conn->state;
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.broadcast_rgb, DRM_HDMI_BROADCAST_RGB_AUTO);
+}
+
/*
* Test that if the connector was initialised with a maximum bpc of 8,
* the value of the max_bpc and max_requested_bpc properties out of
* reset are also set to 8, and output_bpc is set to 0 and will be
* filled at atomic_check time.
@@ -1208,10 +1358,11 @@ static void drm_test_check_tmds_char_value(struct kunit *test)
conn_state = conn->state;
KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate, 0);
}
static struct kunit_case drm_atomic_helper_connector_hdmi_reset_tests[] = {
+ KUNIT_CASE(drm_test_check_broadcast_rgb_value),
KUNIT_CASE(drm_test_check_bpc_8_value),
KUNIT_CASE(drm_test_check_bpc_10_value),
KUNIT_CASE(drm_test_check_bpc_12_value),
KUNIT_CASE(drm_test_check_format_value),
KUNIT_CASE(drm_test_check_tmds_char_value),
diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index ac4b8d00e992..d5320a089887 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -562,10 +562,67 @@ static struct kunit_case drm_get_tv_mode_from_name_tests[] = {
static struct kunit_suite drm_get_tv_mode_from_name_test_suite = {
.name = "drm_get_tv_mode_from_name",
.test_cases = drm_get_tv_mode_from_name_tests,
};
+struct drm_hdmi_connector_get_broadcast_rgb_name_test {
+ unsigned int kind;
+ const char *expected_name;
+};
+
+#define BROADCAST_RGB_TEST(_kind, _name) \
+ { \
+ .kind = _kind, \
+ .expected_name = _name, \
+ }
+
+static void drm_test_drm_hdmi_connector_get_broadcast_rgb_name(struct kunit *test)
+{
+ const struct drm_hdmi_connector_get_broadcast_rgb_name_test *params =
+ test->param_value;
+
+ KUNIT_EXPECT_STREQ(test,
+ drm_hdmi_connector_get_broadcast_rgb_name(params->kind),
+ params->expected_name);
+}
+
+static const
+struct drm_hdmi_connector_get_broadcast_rgb_name_test
+drm_hdmi_connector_get_broadcast_rgb_name_valid_tests[] = {
+ BROADCAST_RGB_TEST(DRM_HDMI_BROADCAST_RGB_AUTO, "Automatic"),
+ BROADCAST_RGB_TEST(DRM_HDMI_BROADCAST_RGB_FULL, "Full"),
+ BROADCAST_RGB_TEST(DRM_HDMI_BROADCAST_RGB_LIMITED, "Limited 16:235"),
+};
+
+static void
+drm_hdmi_connector_get_broadcast_rgb_name_valid_desc(const struct drm_hdmi_connector_get_broadcast_rgb_name_test *t,
+ char *desc)
+{
+ sprintf(desc, "%s", t->expected_name);
+}
+
+KUNIT_ARRAY_PARAM(drm_hdmi_connector_get_broadcast_rgb_name_valid,
+ drm_hdmi_connector_get_broadcast_rgb_name_valid_tests,
+ drm_hdmi_connector_get_broadcast_rgb_name_valid_desc);
+
+static void drm_test_drm_hdmi_connector_get_broadcast_rgb_name_invalid(struct kunit *test)
+{
+ KUNIT_EXPECT_NULL(test, drm_hdmi_connector_get_broadcast_rgb_name(3));
+};
+
+static struct kunit_case drm_hdmi_connector_get_broadcast_rgb_name_tests[] = {
+ KUNIT_CASE_PARAM(drm_test_drm_hdmi_connector_get_broadcast_rgb_name,
+ drm_hdmi_connector_get_broadcast_rgb_name_valid_gen_params),
+ KUNIT_CASE(drm_test_drm_hdmi_connector_get_broadcast_rgb_name_invalid),
+ { }
+};
+
+static struct kunit_suite drm_hdmi_connector_get_broadcast_rgb_name_test_suite = {
+ .name = "drm_hdmi_connector_get_broadcast_rgb_name",
+ .test_cases = drm_hdmi_connector_get_broadcast_rgb_name_tests,
+};
+
struct drm_hdmi_connector_get_output_format_name_test {
unsigned int kind;
const char *expected_name;
};
@@ -620,10 +677,67 @@ static struct kunit_case drm_hdmi_connector_get_output_format_name_tests[] = {
static struct kunit_suite drm_hdmi_connector_get_output_format_name_test_suite = {
.name = "drm_hdmi_connector_get_output_format_name",
.test_cases = drm_hdmi_connector_get_output_format_name_tests,
};
+static void drm_test_drm_connector_attach_broadcast_rgb_property(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ int ret;
+
+ ret = drmm_connector_init(&priv->drm, connector,
+ &dummy_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = drm_connector_attach_broadcast_rgb_property(connector);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ prop = connector->broadcast_rgb_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+}
+
+static void drm_test_drm_connector_attach_broadcast_rgb_property_hdmi_connector(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = &priv->connector;
+ struct drm_property *prop;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ ret = drm_connector_attach_broadcast_rgb_property(connector);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ prop = connector->broadcast_rgb_property;
+ KUNIT_ASSERT_NOT_NULL(test, prop);
+ KUNIT_EXPECT_NOT_NULL(test, drm_mode_obj_find_prop_id(&connector->base, prop->base.id));
+}
+
+static struct kunit_case drm_connector_attach_broadcast_rgb_property_tests[] = {
+ KUNIT_CASE(drm_test_drm_connector_attach_broadcast_rgb_property),
+ KUNIT_CASE(drm_test_drm_connector_attach_broadcast_rgb_property_hdmi_connector),
+ { }
+};
+
+static struct kunit_suite drm_connector_attach_broadcast_rgb_property_test_suite = {
+ .name = "drm_connector_attach_broadcast_rgb_property",
+ .init = drm_test_connector_init,
+ .test_cases = drm_connector_attach_broadcast_rgb_property_tests,
+};
+
/*
* Test that for a given mode, with 8bpc and an RGB output the TMDS
* character rate is equal to the mode pixel clock.
*/
static void drm_test_drm_connector_hdmi_compute_mode_clock_rgb(struct kunit *test)
@@ -943,12 +1057,14 @@ static struct kunit_suite drm_connector_hdmi_compute_mode_clock_test_suite = {
};
kunit_test_suites(
&drmm_connector_hdmi_init_test_suite,
&drmm_connector_init_test_suite,
+ &drm_connector_attach_broadcast_rgb_property_test_suite,
&drm_connector_hdmi_compute_mode_clock_test_suite,
&drm_get_tv_mode_from_name_test_suite,
+ &drm_hdmi_connector_get_broadcast_rgb_name_test_suite,
&drm_hdmi_connector_get_output_format_name_test_suite
);
MODULE_AUTHOR("Maxime Ripard <maxime@cerno.tech>");
MODULE_LICENSE("GPL");
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v10 15/27] drm/tests: Add HDMI connector bpc and format tests
From: Maxime Ripard @ 2024-03-21 15:29 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jonathan Corbet, Sandy Huang, Heiko Stübner, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: Hans Verkuil, Sebastian Wick, Ville Syrjälä, dri-devel,
linux-arm-kernel, linux-doc, linux-kernel, linux-media,
linux-rockchip, linux-sunxi, Maxime Ripard
In-Reply-To: <20240321-kms-hdmi-connector-state-v10-0-e6c178361898@kernel.org>
The previous patch added the bpc and format an HDMI connector needs to
be set up with for a given connector state.
Let's add a few tests to make sure it works as expected.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
.../gpu/drm/tests/drm_atomic_state_helper_test.c | 507 +++++++++++++++++++++
drivers/gpu/drm/tests/drm_kunit_edid.h | 160 +++++++
2 files changed, 667 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
index f010fde0eb69..da8a1840ff47 100644
--- a/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_atomic_state_helper_test.c
@@ -368,10 +368,60 @@ static void drm_test_check_output_bpc_crtc_mode_not_changed(struct kunit *test)
crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
KUNIT_EXPECT_FALSE(test, crtc_state->mode_changed);
}
+/*
+ * Test that if we have an HDMI connector but a !HDMI display, we always
+ * output RGB with 8 bpc.
+ */
+static void drm_test_check_output_bpc_dvi(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *conn_state;
+ struct drm_display_info *info;
+ struct drm_display_mode *preferred;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB) |
+ BIT(HDMI_COLORSPACE_YUV422) |
+ BIT(HDMI_COLORSPACE_YUV444),
+ 12);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_dvi_1080p,
+ ARRAY_SIZE(test_edid_dvi_1080p));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ info = &conn->display_info;
+ KUNIT_ASSERT_FALSE(test, info->is_hdmi);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ conn_state = conn->state;
+ KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+}
+
/*
* Test that when doing a commit which would use RGB 8bpc, the TMDS
* clock rate stored in the connector state is equal to the mode clock
*/
static void drm_test_check_tmds_char_rate_rgb_8bpc(struct kunit *test)
@@ -560,14 +610,471 @@ static void drm_test_check_hdmi_funcs_reject_rate(struct kunit *test)
ret = drm_atomic_check_only(state);
KUNIT_EXPECT_LT(test, ret, 0);
}
+/*
+ * Test that if:
+ * - We have an HDMI connector supporting RGB only
+ * - The chosen mode has a TMDS character rate higher than the display
+ * supports in RGB/12bpc
+ * - The chosen mode has a TMDS character rate lower than the display
+ * supports in RGB/10bpc.
+ *
+ * Then we will pick the latter, and the computed TMDS character rate
+ * will be equal to 1.25 times the mode pixel clock.
+ */
+static void drm_test_check_max_tmds_rate_bpc_fallback(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *conn_state;
+ struct drm_display_info *info;
+ struct drm_display_mode *preferred;
+ unsigned long long rate;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 12);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ info = &conn->display_info;
+ KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+ KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+ KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
+
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
+ KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
+
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 10, HDMI_COLORSPACE_RGB);
+ KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ conn_state = conn->state;
+ KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 10);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.tmds_char_rate, preferred->clock * 1250);
+}
+
+/*
+ * Test that if:
+ * - We have an HDMI connector supporting both RGB and YUV422 and up to
+ * 12 bpc
+ * - The chosen mode has a TMDS character rate higher than the display
+ * supports in RGB/12bpc but lower than the display supports in
+ * RGB/10bpc
+ * - The chosen mode has a TMDS character rate lower than the display
+ * supports in YUV422/12bpc.
+ *
+ * Then we will prefer to keep the RGB format with a lower bpc over
+ * picking YUV422.
+ */
+static void drm_test_check_max_tmds_rate_format_fallback(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *conn_state;
+ struct drm_display_info *info;
+ struct drm_display_mode *preferred;
+ unsigned long long rate;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB) |
+ BIT(HDMI_COLORSPACE_YUV422) |
+ BIT(HDMI_COLORSPACE_YUV444),
+ 12);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ info = &conn->display_info;
+ KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+ KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+ KUNIT_ASSERT_FALSE(test, preferred->flags & DRM_MODE_FLAG_DBLCLK);
+
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 10, HDMI_COLORSPACE_RGB);
+ KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
+ KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
+
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
+ KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ conn_state = conn->state;
+ KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 10);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+}
+
+/*
+ * Test that if a driver and screen supports RGB and YUV formats, and we
+ * try to set the VIC 1 mode, we end up with 8bpc RGB even if we could
+ * have had a higher bpc.
+ */
+static void drm_test_check_output_bpc_format_vic_1(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *conn_state;
+ struct drm_display_info *info;
+ struct drm_display_mode *mode;
+ unsigned long long rate;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB) |
+ BIT(HDMI_COLORSPACE_YUV422) |
+ BIT(HDMI_COLORSPACE_YUV444),
+ 12);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ info = &conn->display_info;
+ KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+ KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ mode = drm_display_mode_from_cea_vic(drm, 1);
+ KUNIT_ASSERT_NOT_NULL(test, mode);
+
+ /*
+ * NOTE: We can't use drm_connector_hdmi_compute_mode_clock()
+ * here because we're trying to get the rate of an invalid
+ * configuration.
+ *
+ * Thus, we have to calculate the rate by hand.
+ */
+ rate = mode->clock * 1500;
+ KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, mode, ctx);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ conn_state = conn->state;
+ KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+}
+
+/*
+ * Test that if a driver supports only RGB but the screen also supports
+ * YUV formats, we only end up with an RGB format.
+ */
+static void drm_test_check_output_bpc_format_driver_rgb_only(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *conn_state;
+ struct drm_display_info *info;
+ struct drm_display_mode *preferred;
+ unsigned long long rate;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 12);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_200mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ info = &conn->display_info;
+ KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+ KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ /*
+ * We're making sure that YUV422 would be the preferred option
+ * here: we're always favouring higher bpc, we can't have RGB
+ * because the TMDS character rate exceeds the maximum supported
+ * by the display, and YUV422 works for that display.
+ *
+ * But since the driver only supports RGB, we should fallback to
+ * a lower bpc with RGB.
+ */
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
+ KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
+
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
+ KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ conn_state = conn->state;
+ KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+ KUNIT_EXPECT_LT(test, conn_state->hdmi.output_bpc, 12);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+}
+
+/*
+ * Test that if a screen supports only RGB but the driver also supports
+ * YUV formats, we only end up with an RGB format.
+ */
+static void drm_test_check_output_bpc_format_display_rgb_only(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *conn_state;
+ struct drm_display_info *info;
+ struct drm_display_mode *preferred;
+ unsigned long long rate;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB) |
+ BIT(HDMI_COLORSPACE_YUV422) |
+ BIT(HDMI_COLORSPACE_YUV444),
+ 12);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_200mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ info = &conn->display_info;
+ KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+ KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ /*
+ * We're making sure that YUV422 would be the preferred option
+ * here: we're always favouring higher bpc, we can't have RGB
+ * because the TMDS character rate exceeds the maximum supported
+ * by the display, and YUV422 works for that display.
+ *
+ * But since the display only supports RGB, we should fallback to
+ * a lower bpc with RGB.
+ */
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
+ KUNIT_ASSERT_GT(test, rate, info->max_tmds_clock * 1000);
+
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_YUV422);
+ KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ conn_state = conn->state;
+ KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+ KUNIT_EXPECT_LT(test, conn_state->hdmi.output_bpc, 12);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+}
+
+/*
+ * Test that if a display supports higher bpc but the driver only
+ * supports 8 bpc, we only end up with 8 bpc even if we could have had a
+ * higher bpc.
+ */
+static void drm_test_check_output_bpc_format_driver_8bpc_only(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *conn_state;
+ struct drm_display_info *info;
+ struct drm_display_mode *preferred;
+ unsigned long long rate;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ info = &conn->display_info;
+ KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+ KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ /*
+ * We're making sure that we have headroom on the TMDS character
+ * clock to actually use 12bpc.
+ */
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
+ KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ conn_state = conn->state;
+ KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+}
+
+/*
+ * Test that if a driver supports higher bpc but the display only
+ * supports 8 bpc, we only end up with 8 bpc even if we could have had a
+ * higher bpc.
+ */
+static void drm_test_check_output_bpc_format_display_8bpc_only(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_modeset_acquire_ctx *ctx;
+ struct drm_connector_state *conn_state;
+ struct drm_display_info *info;
+ struct drm_display_mode *preferred;
+ unsigned long long rate;
+ struct drm_connector *conn;
+ struct drm_device *drm;
+ struct drm_crtc *crtc;
+ int ret;
+
+ priv = drm_atomic_helper_connector_hdmi_init(test,
+ BIT(HDMI_COLORSPACE_RGB) |
+ BIT(HDMI_COLORSPACE_YUV422) |
+ BIT(HDMI_COLORSPACE_YUV444),
+ 12);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ ret = set_connector_edid(test, conn,
+ test_edid_hdmi_1080p_rgb_max_340mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_max_340mhz));
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ info = &conn->display_info;
+ KUNIT_ASSERT_TRUE(test, info->is_hdmi);
+ KUNIT_ASSERT_GT(test, info->max_tmds_clock, 0);
+
+ ctx = drm_kunit_helper_acquire_ctx_alloc(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+ preferred = find_preferred_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+ /*
+ * We're making sure that we have headroom on the TMDS character
+ * clock to actually use 12bpc.
+ */
+ rate = drm_connector_hdmi_compute_mode_clock(preferred, 12, HDMI_COLORSPACE_RGB);
+ KUNIT_ASSERT_LT(test, rate, info->max_tmds_clock * 1000);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, preferred, ctx);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ conn_state = conn->state;
+ KUNIT_ASSERT_NOT_NULL(test, conn_state);
+
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, HDMI_COLORSPACE_RGB);
+}
+
static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
KUNIT_CASE(drm_test_check_hdmi_funcs_reject_rate),
+ KUNIT_CASE(drm_test_check_max_tmds_rate_bpc_fallback),
+ KUNIT_CASE(drm_test_check_max_tmds_rate_format_fallback),
KUNIT_CASE(drm_test_check_output_bpc_crtc_mode_changed),
KUNIT_CASE(drm_test_check_output_bpc_crtc_mode_not_changed),
+ KUNIT_CASE(drm_test_check_output_bpc_dvi),
+ KUNIT_CASE(drm_test_check_output_bpc_format_vic_1),
+ KUNIT_CASE(drm_test_check_output_bpc_format_display_8bpc_only),
+ KUNIT_CASE(drm_test_check_output_bpc_format_display_rgb_only),
+ KUNIT_CASE(drm_test_check_output_bpc_format_driver_8bpc_only),
+ KUNIT_CASE(drm_test_check_output_bpc_format_driver_rgb_only),
KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_8bpc),
KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_10bpc),
KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_12bpc),
/*
* TODO: We should have tests to check that a change in the
diff --git a/drivers/gpu/drm/tests/drm_kunit_edid.h b/drivers/gpu/drm/tests/drm_kunit_edid.h
index ed051d356d5e..cda410440aca 100644
--- a/drivers/gpu/drm/tests/drm_kunit_edid.h
+++ b/drivers/gpu/drm/tests/drm_kunit_edid.h
@@ -1,8 +1,66 @@
#ifndef DRM_KUNIT_EDID_H_
#define DRM_KUNIT_EDID_H_
+/*
+ * edid-decode (hex):
+ *
+ * 00 ff ff ff ff ff ff 00 31 d8 2a 00 00 00 00 00
+ * 00 21 01 03 81 a0 5a 78 0a 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 01 01 01 01 01 01 01 01 01 01
+ * 01 01 01 01 01 01 02 3a 80 18 71 38 2d 40 58 2c
+ * 45 00 40 84 63 00 00 1e 00 00 00 fc 00 54 65 73
+ * 74 20 45 44 49 44 0a 20 20 20 00 00 00 fd 00 32
+ * 46 1e 46 0f 00 0a 20 20 20 20 20 20 00 00 00 10
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ab
+ *
+ * ----------------
+ *
+ * Block 0, Base EDID:
+ * EDID Structure Version & Revision: 1.3
+ * Vendor & Product Identification:
+ * Manufacturer: LNX
+ * Model: 42
+ * Made in: 2023
+ * Basic Display Parameters & Features:
+ * Digital display
+ * DFP 1.x compatible TMDS
+ * Maximum image size: 160 cm x 90 cm
+ * Gamma: 2.20
+ * RGB color display
+ * First detailed timing is the preferred timing
+ * Color Characteristics:
+ * Red : 0.0000, 0.0000
+ * Green: 0.0000, 0.0000
+ * Blue : 0.0000, 0.0000
+ * White: 0.0000, 0.0000
+ * Established Timings I & II: none
+ * Standard Timings: none
+ * Detailed Timing Descriptors:
+ * DTD 1: 1920x1080 60.000000 Hz 16:9 67.500 kHz 148.500000 MHz (1600 mm x 900 mm)
+ * Hfront 88 Hsync 44 Hback 148 Hpol P
+ * Vfront 4 Vsync 5 Vback 36 Vpol P
+ * Display Product Name: 'Test EDID'
+ * Display Range Limits:
+ * Monitor ranges (GTF): 50-70 Hz V, 30-70 kHz H, max dotclock 150 MHz
+ * Dummy Descriptor:
+ * Checksum: 0xab
+ */
+static const unsigned char test_edid_dvi_1080p[] = {
+ 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
+ 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+ 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3a, 0x80, 0x18, 0x71, 0x38,
+ 0x2d, 0x40, 0x58, 0x2c, 0x45, 0x00, 0x40, 0x84, 0x63, 0x00, 0x00, 0x1e,
+ 0x00, 0x00, 0x00, 0xfc, 0x00, 0x54, 0x65, 0x73, 0x74, 0x20, 0x45, 0x44,
+ 0x49, 0x44, 0x0a, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32,
+ 0x46, 0x1e, 0x46, 0x0f, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+ 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xab
+};
+
/*
* edid-decode (hex):
*
* 00 ff ff ff ff ff ff 00 31 d8 2a 00 00 00 00 00
* 00 21 01 03 81 a0 5a 78 02 00 00 00 00 00 00 00
@@ -101,10 +159,112 @@ static const unsigned char test_edid_hdmi_1080p_rgb_max_200mhz[] = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0xd0
};
+/*
+ * edid-decode (hex):
+ *
+ * 00 ff ff ff ff ff ff 00 31 d8 2a 00 00 00 00 00
+ * 00 21 01 03 81 a0 5a 78 02 00 00 00 00 00 00 00
+ * 00 00 00 20 00 00 01 01 01 01 01 01 01 01 01 01
+ * 01 01 01 01 01 01 02 3a 80 18 71 38 2d 40 58 2c
+ * 45 00 40 84 63 00 00 1e 00 00 00 fc 00 54 65 73
+ * 74 20 45 44 49 44 0a 20 20 20 00 00 00 fd 00 32
+ * 46 1e 46 0f 00 0a 20 20 20 20 20 20 00 00 00 10
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 92
+ *
+ * 02 03 1b 81 e3 05 00 20 41 10 e2 00 4a 6d 03 0c
+ * 00 12 34 00 28 20 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 d0
+ *
+ * ----------------
+ *
+ * Block 0, Base EDID:
+ * EDID Structure Version & Revision: 1.3
+ * Vendor & Product Identification:
+ * Manufacturer: LNX
+ * Model: 42
+ * Made in: 2023
+ * Basic Display Parameters & Features:
+ * Digital display
+ * DFP 1.x compatible TMDS
+ * Maximum image size: 160 cm x 90 cm
+ * Gamma: 2.20
+ * Monochrome or grayscale display
+ * First detailed timing is the preferred timing
+ * Color Characteristics:
+ * Red : 0.0000, 0.0000
+ * Green: 0.0000, 0.0000
+ * Blue : 0.0000, 0.0000
+ * White: 0.0000, 0.0000
+ * Established Timings I & II:
+ * DMT 0x04: 640x480 59.940476 Hz 4:3 31.469 kHz 25.175000 MHz
+ * Standard Timings: none
+ * Detailed Timing Descriptors:
+ * DTD 1: 1920x1080 60.000000 Hz 16:9 67.500 kHz 148.500000 MHz (1600 mm x 900 mm)
+ * Hfront 88 Hsync 44 Hback 148 Hpol P
+ * Vfront 4 Vsync 5 Vback 36 Vpol P
+ * Display Product Name: 'Test EDID'
+ * Display Range Limits:
+ * Monitor ranges (GTF): 50-70 Hz V, 30-70 kHz H, max dotclock 150 MHz
+ * Dummy Descriptor:
+ * Extension blocks: 1
+ * Checksum: 0x92
+ *
+ * ----------------
+ *
+ * Block 1, CTA-861 Extension Block:
+ * Revision: 3
+ * Underscans IT Video Formats by default
+ * Native detailed modes: 1
+ * Colorimetry Data Block:
+ * sRGB
+ * Video Data Block:
+ * VIC 16: 1920x1080 60.000000 Hz 16:9 67.500 kHz 148.500000 MHz
+ * Video Capability Data Block:
+ * YCbCr quantization: No Data
+ * RGB quantization: Selectable (via AVI Q)
+ * PT scan behavior: No Data
+ * IT scan behavior: Always Underscanned
+ * CE scan behavior: Always Underscanned
+ * Vendor-Specific Data Block (HDMI), OUI 00-0C-03:
+ * Source physical address: 1.2.3.4
+ * Maximum TMDS clock: 340 MHz
+ * Extended HDMI video details:
+ * Checksum: 0xd0 Unused space in Extension Block: 100 bytes
+ */
+static const unsigned char test_edid_hdmi_1080p_rgb_max_340mhz[] = {
+ 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
+ 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
+ 0x00, 0x00, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+ 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3a, 0x80, 0x18, 0x71, 0x38,
+ 0x2d, 0x40, 0x58, 0x2c, 0x45, 0x00, 0x40, 0x84, 0x63, 0x00, 0x00, 0x1e,
+ 0x00, 0x00, 0x00, 0xfc, 0x00, 0x54, 0x65, 0x73, 0x74, 0x20, 0x45, 0x44,
+ 0x49, 0x44, 0x0a, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32,
+ 0x46, 0x00, 0x00, 0xc4, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+ 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x41, 0x02, 0x03, 0x1b, 0x81,
+ 0xe3, 0x05, 0x00, 0x20, 0x41, 0x10, 0xe2, 0x00, 0x4a, 0x6d, 0x03, 0x0c,
+ 0x00, 0x12, 0x34, 0x00, 0x44, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0xd0
+};
+
/*
* edid-decode (hex):
*
* 00 ff ff ff ff ff ff 00 31 d8 2a 00 00 00 00 00
* 00 21 01 03 81 a0 5a 78 1a 00 00 00 00 00 00 00
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 0/6] KVM: arm64: Hide unsupported MPAM from the guest
From: James Morse @ 2024-03-21 16:57 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm
Cc: Marc Zyngier, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Jing Zhang, James Morse
'lo
Sorry skipped a cycle, I got busy with the resctrl end of this, and it took
me a while to convince myself doing this can still work if we need to
prevent migration to incompatible hardware.
The change since v2 is to (mostly) ignore the value provided by user-space
as the simplest thing to do. The value is ignored when it matches the
sanitised hardware value - as that is the value exposed by the bug. But
otherwise is checked normally. This allows the value to be used to prevent
migration on some hypothetical MPAM==2 hardware if that is incompatible. If
it is compatible, the writeable-id-registers can be used. Sufficiently hairy
that I added a test.
~
This series fixes up a long standing bug where MPAM was accidentally exposed
to a guest, but the feature was not otherwise trapped or context switched.
This could result in KVM warning about unexpected traps, and injecting an
undef into the guest contradicting the ID registers.
This would prevent an MPAM aware kernel from booting - fortunately, there
aren't any of those.
Ideally, we'd take the MPAM feature away from the ID registers, but that
would leave existing guests unable to migrate to a newer kernel. Instead,
just ignore that field when it matches the hardware. KVM wasn't going to
expose MPAM anyway. The guest will not see MPAM in the id registers.
This series includes the head.S and KVM changes to enable/disable traps. If
MPAM is neither enabled nor emulated by EL3 firmware, these system register
accesses will trap to EL3.
If your kernel doesn't boot, and the problem bisects here - please update
your firmware. MPAM has been supported by trusted firmware since v1.6 in
2018. (also noted on patch 2).
This series is based on Linus' commit 23956900041d and can be retrieved from:
https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git
mpam/kvm_mpam_fix/v3
Sorry for the mid-merge window base, I'm away for a few weeks - this should
rebase trivially onto rc1.
Thanks
James
[v1] https://lore.kernel.org/linux-arm-kernel/20200925160102.118858-1-james.morse@arm.com/
[v2] https://lore.kernel.org/linux-arm-kernel/20231207150804.3425468-1-james.morse@arm.com/
James Morse (6):
arm64: head.S: Initialise MPAM EL2 registers and disable traps
arm64: cpufeature: discover CPU support for MPAM
KVM: arm64: Fix missing traps of guest accesses to the MPAM registers
KVM: arm64: Disable MPAM visibility by default and ignore VMM writes
KVM: arm64: selftests: Move the bulky macro invocation to a helper
KVM: arm64: selftests: Test ID_AA64PFR0.MPAM isn't completely ignored
.../arch/arm64/cpu-feature-registers.rst | 2 +
arch/arm64/Kconfig | 19 +++-
arch/arm64/include/asm/cpu.h | 1 +
arch/arm64/include/asm/cpufeature.h | 13 +++
arch/arm64/include/asm/el2_setup.h | 16 +++
arch/arm64/include/asm/kvm_arm.h | 1 +
arch/arm64/include/asm/mpam.h | 75 +++++++++++++
arch/arm64/include/asm/sysreg.h | 8 ++
arch/arm64/kernel/Makefile | 2 +
arch/arm64/kernel/cpufeature.c | 104 ++++++++++++++++++
arch/arm64/kernel/cpuinfo.c | 4 +
arch/arm64/kernel/image-vars.h | 5 +
arch/arm64/kernel/mpam.c | 8 ++
arch/arm64/kvm/hyp/include/hyp/switch.h | 32 ++++++
arch/arm64/kvm/sys_regs.c | 43 +++++++-
arch/arm64/tools/cpucaps | 1 +
arch/arm64/tools/sysreg | 32 ++++++
.../selftests/kvm/aarch64/set_id_regs.c | 64 ++++++++++-
18 files changed, 424 insertions(+), 6 deletions(-)
create mode 100644 arch/arm64/include/asm/mpam.h
create mode 100644 arch/arm64/kernel/mpam.c
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 1/6] arm64: head.S: Initialise MPAM EL2 registers and disable traps
From: James Morse @ 2024-03-21 16:57 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm
Cc: Marc Zyngier, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Jing Zhang, James Morse
In-Reply-To: <20240321165728.31907-1-james.morse@arm.com>
Add code to head.S's el2_setup to detect MPAM and disable any EL2 traps.
This register resets to an unknown value, setting it to the default
parititons/pmg before we enable the MMU is the best thing to do.
Kexec/kdump will depend on this if the previous kernel left the CPU
configured with a restrictive configuration.
If linux is booted at the highest implemented exception level el2_setup
will clear the enable bit, disabling MPAM.
This code can't be enabled until a subsequent patch adds the Kconfig
and cpufeature boiler plate.
Signed-off-by: James Morse <james.morse@arm.com>
---
arch/arm64/include/asm/el2_setup.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index b7afaa026842..1e2181820a0a 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -208,6 +208,21 @@
msr spsr_el2, x0
.endm
+.macro __init_el2_mpam
+#ifdef CONFIG_ARM64_MPAM
+ /* Memory Partioning And Monitoring: disable EL2 traps */
+ mrs x1, id_aa64pfr0_el1
+ ubfx x0, x1, #ID_AA64PFR0_EL1_MPAM_SHIFT, #4
+ cbz x0, .Lskip_mpam_\@ // skip if no MPAM
+ msr_s SYS_MPAM2_EL2, xzr // use the default partition
+ // and disable lower traps
+ mrs_s x0, SYS_MPAMIDR_EL1
+ tbz x0, #17, .Lskip_mpam_\@ // skip if no MPAMHCR reg
+ msr_s SYS_MPAMHCR_EL2, xzr // clear TRAP_MPAMIDR_EL1 -> EL2
+.Lskip_mpam_\@:
+#endif /* CONFIG_ARM64_MPAM */
+.endm
+
/**
* Initialize EL2 registers to sane values. This should be called early on all
* cores that were booted in EL2. Note that everything gets initialised as
@@ -225,6 +240,7 @@
__init_el2_stage2
__init_el2_gicv3
__init_el2_hstr
+ __init_el2_mpam
__init_el2_nvhe_idregs
__init_el2_cptr
__init_el2_fgt
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 4/6] KVM: arm64: Disable MPAM visibility by default and ignore VMM writes
From: James Morse @ 2024-03-21 16:57 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm
Cc: Marc Zyngier, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Jing Zhang, James Morse
In-Reply-To: <20240321165728.31907-1-james.morse@arm.com>
commit 011e5f5bf529f ("arm64/cpufeature: Add remaining feature bits in
ID_AA64PFR0 register") exposed the MPAM field of AA64PFR0_EL1 to guests,
but didn't add trap handling. A previous patch supplied the missing trap
handling.
Existing VMs that have the MPAM field of ID_AA64PFR0_EL1 set need to
be migratable, but there is little point enabling the MPAM CPU
interface on new VMs until there is something a guest can do with it.
Clear the MPAM field from the guest's ID_AA64PFR0_EL1 and on hardware
that supports MPAM, politely ignore the VMMs attempts to set this bit.
Guests expossed to this bug have the sanitised value of the MPAM field,
so only the correct value needs to be ignored. This means the field
can continue to be used to block migration to incompatible hardware
(between MPAM=1 and MPAM=5), and the VMM can't rely on the field
being ignored.
Signed-off-by: James Morse <james.morse@arm.com>
---
arch/arm64/kvm/sys_regs.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index d6afb21849de..56d70a90c965 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1685,6 +1685,13 @@ static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
val &= ~ID_AA64PFR0_EL1_AMU_MASK;
+ /*
+ * MPAM is disabled by default as KVM also needs a set of PARTID to
+ * program the MPAMVPMx_EL2 PARTID remapping registers with. But some
+ * older kernels let the guest see the ID bit.
+ */
+ val &= ~ID_AA64PFR0_EL1_MPAM_MASK;
+
return val;
}
@@ -1795,6 +1802,29 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu,
return set_id_reg(vcpu, rd, val);
}
+static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
+ const struct sys_reg_desc *rd, u64 user_val)
+{
+ u64 hw_val = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
+ u64 mpam_mask = ID_AA64PFR0_EL1_MPAM_MASK;
+
+ /*
+ * Commit 011e5f5bf529f ("arm64/cpufeature: Add remaining feature bits
+ * in ID_AA64PFR0 register") exposed the MPAM field of AA64PFR0_EL1 to
+ * guests, but didn't add trap handling. KVM doesn't support MPAM and
+ * always returns an UNDEF for these registers. The guest must see 0
+ * for this field.
+ *
+ * But KVM must also accept values from user-space that were provided
+ * by KVM. On CPUs that support MPAM, permit user-space to write
+ * the santisied value to ID_AA64PFR0_EL1.MPAM, but ignore this field.
+ */
+ if ((hw_val & mpam_mask) == (user_val & mpam_mask))
+ user_val &= ~ID_AA64PFR0_EL1_MPAM_MASK;
+
+ return set_id_reg(vcpu, rd, user_val);
+}
+
/*
* cpufeature ID register user accessors
*
@@ -2291,7 +2321,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_ID_AA64PFR0_EL1),
.access = access_id_reg,
.get_user = get_id_reg,
- .set_user = set_id_reg,
+ .set_user = set_id_aa64pfr0_el1,
.reset = read_sanitised_id_aa64pfr0_el1,
.val = ~(ID_AA64PFR0_EL1_AMU |
ID_AA64PFR0_EL1_MPAM |
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 5/6] KVM: arm64: selftests: Move the bulky macro invocation to a helper
From: James Morse @ 2024-03-21 16:57 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm
Cc: Marc Zyngier, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Jing Zhang, James Morse
In-Reply-To: <20240321165728.31907-1-james.morse@arm.com>
KVM_ARM_FEATURE_ID_RANGE_IDX() takes a whole bunch of arguments, that
are typically going to be extracted from a single value.
Before adding a second copy of this, pull it out into a helper.
Signed-off-by: James Morse <james.morse@arm.com>
---
tools/testing/selftests/kvm/aarch64/set_id_regs.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
index 16e2338686c1..b42a0dc19852 100644
--- a/tools/testing/selftests/kvm/aarch64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
@@ -374,6 +374,13 @@ static void test_reg_set_fail(struct kvm_vcpu *vcpu, uint64_t reg,
TEST_ASSERT_EQ(val, old_val);
}
+static int sys_reg_to_idx(uint32_t reg_id)
+{
+ return KVM_ARM_FEATURE_ID_RANGE_IDX(sys_reg_Op0(reg_id), sys_reg_Op1(reg_id),
+ sys_reg_CRn(reg_id), sys_reg_CRm(reg_id),
+ sys_reg_Op2(reg_id));
+}
+
static void test_user_set_reg(struct kvm_vcpu *vcpu, bool aarch64_only)
{
uint64_t masks[KVM_ARM_FEATURE_ID_RANGE_SIZE];
@@ -398,9 +405,7 @@ static void test_user_set_reg(struct kvm_vcpu *vcpu, bool aarch64_only)
int idx;
/* Get the index to masks array for the idreg */
- idx = KVM_ARM_FEATURE_ID_RANGE_IDX(sys_reg_Op0(reg_id), sys_reg_Op1(reg_id),
- sys_reg_CRn(reg_id), sys_reg_CRm(reg_id),
- sys_reg_Op2(reg_id));
+ idx = sys_reg_to_idx(reg_id);
for (int j = 0; ftr_bits[j].type != FTR_END; j++) {
/* Skip aarch32 reg on aarch64 only system, since they are RAZ/WI. */
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 2/6] arm64: cpufeature: discover CPU support for MPAM
From: James Morse @ 2024-03-21 16:57 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm
Cc: Marc Zyngier, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Jing Zhang, James Morse
In-Reply-To: <20240321165728.31907-1-james.morse@arm.com>
ARMv8.4 adds support for 'Memory Partitioning And Monitoring' (MPAM)
which describes an interface to cache and bandwidth controls wherever
they appear in the system.
Add support to detect MPAM. Like SVE, MPAM has an extra id register that
describes some more properties, including the virtualisation support,
which is optional. Detect this separately so we can detect
mismatched/insane systems, but still use MPAM on the host even if the
virtualisation support is missing.
MPAM needs enabling at the highest implemented exception level, otherwise
the register accesses trap. The 'enabled' flag is accessible to lower
exception levels, but its in a register that traps when MPAM isn't enabled.
The cpufeature 'matches' hook is extended to test this on one of the
CPUs, so that firmware can emulate MPAM as disabled if it is reserved
for use by secure world.
Secondary CPUs that appear late could trip cpufeature's 'lower safe'
behaviour after the MPAM properties have been advertised to user-space.
Add a verify call to ensure late secondaries match the existing CPUs.
(If you have a boot failure that bisects here its likely your CPUs
advertise MPAM in the id registers, but firmware failed to either enable
or MPAM, or emulate the trap as if it were disabled)
Signed-off-by: James Morse <james.morse@arm.com>
---
.../arch/arm64/cpu-feature-registers.rst | 2 +
arch/arm64/Kconfig | 19 +++-
arch/arm64/include/asm/cpu.h | 1 +
arch/arm64/include/asm/cpufeature.h | 13 +++
arch/arm64/include/asm/mpam.h | 75 +++++++++++++
arch/arm64/include/asm/sysreg.h | 8 ++
arch/arm64/kernel/Makefile | 2 +
arch/arm64/kernel/cpufeature.c | 104 ++++++++++++++++++
arch/arm64/kernel/cpuinfo.c | 4 +
arch/arm64/kernel/mpam.c | 8 ++
arch/arm64/tools/cpucaps | 1 +
arch/arm64/tools/sysreg | 32 ++++++
12 files changed, 268 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/mpam.h
create mode 100644 arch/arm64/kernel/mpam.c
diff --git a/Documentation/arch/arm64/cpu-feature-registers.rst b/Documentation/arch/arm64/cpu-feature-registers.rst
index 44f9bd78539d..253e9743de2f 100644
--- a/Documentation/arch/arm64/cpu-feature-registers.rst
+++ b/Documentation/arch/arm64/cpu-feature-registers.rst
@@ -152,6 +152,8 @@ infrastructure:
+------------------------------+---------+---------+
| DIT | [51-48] | y |
+------------------------------+---------+---------+
+ | MPAM | [43-40] | n |
+ +------------------------------+---------+---------+
| SVE | [35-32] | y |
+------------------------------+---------+---------+
| GIC | [27-24] | n |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 77e05d4959f2..a7b01adbd3df 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1977,7 +1977,24 @@ config ARM64_TLB_RANGE
The feature introduces new assembly instructions, and they were
support when binutils >= 2.30.
-endmenu # "ARMv8.4 architectural features"
+config ARM64_MPAM
+ bool "Enable support for MPAM"
+ help
+ Memory Partitioning and Monitoring is an optional extension
+ that allows the CPUs to mark load and store transactions with
+ labels for partition-id and performance-monitoring-group.
+ System components, such as the caches, can use the partition-id
+ to apply a performance policy. MPAM monitors can use the
+ partition-id and performance-monitoring-group to measure the
+ cache occupancy or data throughput.
+
+ Use of this extension requires CPU support, support in the
+ memory system components (MSC), and a description from firmware
+ of where the MSC are in the address space.
+
+ MPAM is exposed to user-space via the resctrl pseudo filesystem.
+
+endmenu
menu "ARMv8.5 architectural features"
diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index 9b73fd0cd721..81e4157f92b7 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -46,6 +46,7 @@ struct cpuinfo_arm64 {
u64 reg_revidr;
u64 reg_gmid;
u64 reg_smidr;
+ u64 reg_mpamidr;
u64 reg_id_aa64dfr0;
u64 reg_id_aa64dfr1;
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 8b904a757bd3..c7006fb6a30a 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -612,6 +612,13 @@ static inline bool id_aa64pfr1_sme(u64 pfr1)
return val > 0;
}
+static inline bool id_aa64pfr0_mpam(u64 pfr0)
+{
+ u32 val = cpuid_feature_extract_unsigned_field(pfr0, ID_AA64PFR0_EL1_MPAM_SHIFT);
+
+ return val > 0;
+}
+
static inline bool id_aa64pfr1_mte(u64 pfr1)
{
u32 val = cpuid_feature_extract_unsigned_field(pfr1, ID_AA64PFR1_EL1_MTE_SHIFT);
@@ -832,6 +839,12 @@ static inline bool system_supports_lpa2(void)
return cpus_have_final_cap(ARM64_HAS_LPA2);
}
+static inline bool cpus_support_mpam(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_MPAM) &&
+ cpus_have_final_cap(ARM64_MPAM);
+}
+
int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
diff --git a/arch/arm64/include/asm/mpam.h b/arch/arm64/include/asm/mpam.h
new file mode 100644
index 000000000000..bae662107082
--- /dev/null
+++ b/arch/arm64/include/asm/mpam.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (C) 2024 Arm Ltd. */
+
+#ifndef __ASM__MPAM_H
+#define __ASM__MPAM_H
+
+#include <linux/bitops.h>
+#include <linux/init.h>
+#include <linux/jump_label.h>
+
+#include <asm/cpucaps.h>
+#include <asm/cpufeature.h>
+#include <asm/sysreg.h>
+
+/* CPU Registers */
+#define MPAM_SYSREG_EN BIT_ULL(63)
+#define MPAM_SYSREG_TRAP_IDR BIT_ULL(58)
+#define MPAM_SYSREG_TRAP_MPAM0_EL1 BIT_ULL(49)
+#define MPAM_SYSREG_TRAP_MPAM1_EL1 BIT_ULL(48)
+#define MPAM_SYSREG_PMG_D GENMASK(47, 40)
+#define MPAM_SYSREG_PMG_I GENMASK(39, 32)
+#define MPAM_SYSREG_PARTID_D GENMASK(31, 16)
+#define MPAM_SYSREG_PARTID_I GENMASK(15, 0)
+
+#define MPAMIDR_PMG_MAX GENMASK(40, 32)
+#define MPAMIDR_PMG_MAX_SHIFT 32
+#define MPAMIDR_PMG_MAX_LEN 8
+#define MPAMIDR_VPMR_MAX GENMASK(20, 18)
+#define MPAMIDR_VPMR_MAX_SHIFT 18
+#define MPAMIDR_VPMR_MAX_LEN 3
+#define MPAMIDR_HAS_HCR BIT(17)
+#define MPAMIDR_HAS_HCR_SHIFT 17
+#define MPAMIDR_PARTID_MAX GENMASK(15, 0)
+#define MPAMIDR_PARTID_MAX_SHIFT 0
+#define MPAMIDR_PARTID_MAX_LEN 15
+
+#define MPAMHCR_EL0_VPMEN BIT_ULL(0)
+#define MPAMHCR_EL1_VPMEN BIT_ULL(1)
+#define MPAMHCR_GSTAPP_PLK BIT_ULL(8)
+#define MPAMHCR_TRAP_MPAMIDR BIT_ULL(31)
+
+/* Properties of the VPM registers */
+#define MPAM_VPM_NUM_REGS 8
+#define MPAM_VPM_PARTID_LEN 16
+#define MPAM_VPM_PARTID_MASK 0xffff
+#define MPAM_VPM_REG_LEN 64
+#define MPAM_VPM_PARTIDS_PER_REG (MPAM_VPM_REG_LEN / MPAM_VPM_PARTID_LEN)
+#define MPAM_VPM_MAX_PARTID (MPAM_VPM_NUM_REGS * MPAM_VPM_PARTIDS_PER_REG)
+
+DECLARE_STATIC_KEY_FALSE(arm64_mpam_has_hcr);
+
+/* check whether all CPUs have MPAM support */
+static inline bool mpam_cpus_have_feature(void)
+{
+ if (IS_ENABLED(CONFIG_ARM64_MPAM))
+ return cpus_have_final_cap(ARM64_MPAM);
+ return false;
+}
+
+/* check whether all CPUs have MPAM virtualisation support */
+static inline bool mpam_cpus_have_mpam_hcr(void)
+{
+ if (IS_ENABLED(CONFIG_ARM64_MPAM))
+ return static_branch_unlikely(&arm64_mpam_has_hcr);
+ return false;
+}
+
+/* enable MPAM virtualisation support */
+static inline void __init __enable_mpam_hcr(void)
+{
+ if (IS_ENABLED(CONFIG_ARM64_MPAM))
+ static_branch_enable(&arm64_mpam_has_hcr);
+}
+
+#endif /* __ASM__MPAM_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 9e8999592f3a..dc34681ff4ef 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -535,6 +535,13 @@
#define SYS_MPAMVPM6_EL2 __SYS__MPAMVPMx_EL2(6)
#define SYS_MPAMVPM7_EL2 __SYS__MPAMVPMx_EL2(7)
+#define SYS_MPAMHCR_EL2 sys_reg(3, 4, 10, 4, 0)
+#define SYS_MPAMVPMV_EL2 sys_reg(3, 4, 10, 4, 1)
+#define SYS_MPAM2_EL2 sys_reg(3, 4, 10, 5, 0)
+
+#define __VPMn_op2(n) ((n) & 0x7)
+#define SYS_MPAM_VPMn_EL2(n) sys_reg(3, 4, 10, 6, __VPMn_op2(n))
+
#define SYS_VBAR_EL2 sys_reg(3, 4, 12, 0, 0)
#define SYS_RVBAR_EL2 sys_reg(3, 4, 12, 0, 1)
#define SYS_RMR_EL2 sys_reg(3, 4, 12, 0, 2)
@@ -622,6 +629,7 @@
#define SYS_PMSCR_EL12 sys_reg(3, 5, 9, 9, 0)
#define SYS_MAIR_EL12 sys_reg(3, 5, 10, 2, 0)
#define SYS_AMAIR_EL12 sys_reg(3, 5, 10, 3, 0)
+#define SYS_MPAM1_EL12 sys_reg(3, 5, 10, 5, 0)
#define SYS_VBAR_EL12 sys_reg(3, 5, 12, 0, 0)
#define SYS_CONTEXTIDR_EL12 sys_reg(3, 5, 13, 0, 1)
#define SYS_SCXTNUM_EL12 sys_reg(3, 5, 13, 0, 7)
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 763824963ed1..c790c67d3bc9 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -64,10 +64,12 @@ obj-$(CONFIG_KEXEC_CORE) += machine_kexec.o relocate_kernel.o \
obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o kexec_image.o
obj-$(CONFIG_ARM64_RELOC_TEST) += arm64-reloc-test.o
arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
+
obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
obj-$(CONFIG_VMCORE_INFO) += vmcore_info.o
obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
+obj-$(CONFIG_ARM64_MPAM) += mpam.o
obj-$(CONFIG_ARM64_MTE) += mte.o
obj-y += vdso-wrap.o
obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 56583677c1f2..0d9216395d85 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -84,6 +84,7 @@
#include <asm/insn.h>
#include <asm/kvm_host.h>
#include <asm/mmu_context.h>
+#include <asm/mpam.h>
#include <asm/mte.h>
#include <asm/processor.h>
#include <asm/smp.h>
@@ -681,6 +682,18 @@ static const struct arm64_ftr_bits ftr_id_dfr1[] = {
ARM64_FTR_END,
};
+static const struct arm64_ftr_bits ftr_mpamidr[] = {
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
+ MPAMIDR_PMG_MAX_SHIFT, MPAMIDR_PMG_MAX_LEN, 0), /* PMG_MAX */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
+ MPAMIDR_VPMR_MAX_SHIFT, MPAMIDR_VPMR_MAX_LEN, 0), /* VPMR_MAX */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
+ MPAMIDR_HAS_HCR_SHIFT, 1, 0), /* HAS_HCR */
+ ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
+ MPAMIDR_PARTID_MAX_SHIFT, MPAMIDR_PARTID_MAX_LEN, 0), /* PARTID_MAX */
+ ARM64_FTR_END,
+};
+
/*
* Common ftr bits for a 32bit register with all hidden, strict
* attributes, with 4bit feature fields and a default safe value of
@@ -801,6 +814,9 @@ static const struct __ftr_reg_entry {
ARM64_FTR_REG(SYS_ID_AA64MMFR3_EL1, ftr_id_aa64mmfr3),
ARM64_FTR_REG(SYS_ID_AA64MMFR4_EL1, ftr_id_aa64mmfr4),
+ /* Op1 = 0, CRn = 10, CRm = 4 */
+ ARM64_FTR_REG(SYS_MPAMIDR_EL1, ftr_mpamidr),
+
/* Op1 = 1, CRn = 0, CRm = 0 */
ARM64_FTR_REG(SYS_GMID_EL1, ftr_gmid),
@@ -1160,6 +1176,9 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info)
cpacr_restore(cpacr);
}
+ if (id_aa64pfr0_mpam(info->reg_id_aa64pfr0))
+ init_cpu_ftr_reg(SYS_MPAMIDR_EL1, info->reg_mpamidr);
+
if (id_aa64pfr1_mte(info->reg_id_aa64pfr1))
init_cpu_ftr_reg(SYS_GMID_EL1, info->reg_gmid);
}
@@ -1416,6 +1435,11 @@ void update_cpu_features(int cpu,
cpacr_restore(cpacr);
}
+ if (id_aa64pfr0_mpam(info->reg_id_aa64pfr0)) {
+ taint |= check_update_ftr_reg(SYS_MPAMIDR_EL1, cpu,
+ info->reg_mpamidr, boot->reg_mpamidr);
+ }
+
/*
* The kernel uses the LDGM/STGM instructions and the number of tags
* they read/write depends on the GMID_EL1.BS field. Check that the
@@ -2358,6 +2382,42 @@ cpucap_panic_on_conflict(const struct arm64_cpu_capabilities *cap)
return !!(cap->type & ARM64_CPUCAP_PANIC_ON_CONFLICT);
}
+static bool __maybe_unused
+test_has_mpam(const struct arm64_cpu_capabilities *entry, int scope)
+{
+ if (!has_cpuid_feature(entry, scope))
+ return false;
+
+ /* Check firmware actually enabled MPAM on this cpu. */
+ return (read_sysreg_s(SYS_MPAM1_EL1) & MPAM_SYSREG_EN);
+}
+
+static void __maybe_unused
+cpu_enable_mpam(const struct arm64_cpu_capabilities *entry)
+{
+ /*
+ * Access by the kernel (at EL1) should use the reserved PARTID
+ * which is configured unrestricted. This avoids priority-inversion
+ * where latency sensitive tasks have to wait for a task that has
+ * been throttled to release the lock.
+ */
+ write_sysreg_s(0, SYS_MPAM1_EL1);
+
+ /* Until resctrl is supported, user-space should be unrestricted too. */
+ write_sysreg_s(0, SYS_MPAM0_EL1);
+}
+
+static void mpam_extra_caps(void)
+{
+ u64 idr = read_sanitised_ftr_reg(SYS_MPAMIDR_EL1);
+
+ if (!IS_ENABLED(CONFIG_ARM64_MPAM))
+ return;
+
+ if (idr & MPAMIDR_HAS_HCR)
+ __enable_mpam_hcr();
+}
+
static const struct arm64_cpu_capabilities arm64_features[] = {
{
.capability = ARM64_ALWAYS_BOOT,
@@ -2852,6 +2912,15 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
ARM64_CPUID_FIELDS(ID_AA64MMFR0_EL1, TGRAN16, 52_BIT)
#endif
#endif
+#endif
+#ifdef CONFIG_ARM64_MPAM
+ {
+ .desc = "Memory Partitioning And Monitoring",
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .capability = ARM64_MPAM,
+ .matches = test_has_mpam,
+ .cpu_enable = cpu_enable_mpam,
+ ARM64_CPUID_FIELDS(ID_AA64PFR0_EL1, MPAM, 1)
},
#endif
{
@@ -3364,6 +3433,37 @@ static void verify_hyp_capabilities(void)
}
}
+static void verify_mpam_capabilities(void)
+{
+ u64 cpu_idr = read_cpuid(ID_AA64PFR0_EL1);
+ u64 sys_idr = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
+ u16 cpu_partid_max, cpu_pmg_max, sys_partid_max, sys_pmg_max;
+
+ if (FIELD_GET(ID_AA64PFR0_EL1_MPAM_MASK, cpu_idr) !=
+ FIELD_GET(ID_AA64PFR0_EL1_MPAM_MASK, sys_idr)) {
+ pr_crit("CPU%d: MPAM version mismatch\n", smp_processor_id());
+ cpu_die_early();
+ }
+
+ cpu_idr = read_cpuid(MPAMIDR_EL1);
+ sys_idr = read_sanitised_ftr_reg(SYS_MPAMIDR_EL1);
+ if (FIELD_GET(MPAMIDR_EL1_HAS_HCR, cpu_idr) !=
+ FIELD_GET(MPAMIDR_EL1_HAS_HCR, sys_idr)) {
+ pr_crit("CPU%d: Missing MPAM HCR\n", smp_processor_id());
+ cpu_die_early();
+ }
+
+ cpu_partid_max = FIELD_GET(MPAMIDR_EL1_PARTID_MAX, cpu_idr);
+ cpu_pmg_max = FIELD_GET(MPAMIDR_EL1_PMG_MAX, cpu_idr);
+ sys_partid_max = FIELD_GET(MPAMIDR_EL1_PARTID_MAX, sys_idr);
+ sys_pmg_max = FIELD_GET(MPAMIDR_EL1_PMG_MAX, sys_idr);
+ if ((cpu_partid_max < sys_partid_max) || (cpu_pmg_max < sys_pmg_max)) {
+ pr_crit("CPU%d: MPAM PARTID/PMG max values are mismatched\n",
+ smp_processor_id());
+ cpu_die_early();
+ }
+}
+
/*
* Run through the enabled system capabilities and enable() it on this CPU.
* The capabilities were decided based on the available CPUs at the boot time.
@@ -3390,6 +3490,9 @@ static void verify_local_cpu_capabilities(void)
if (is_hyp_mode_available())
verify_hyp_capabilities();
+
+ if (cpus_support_mpam())
+ verify_mpam_capabilities();
}
void check_local_cpu_capabilities(void)
@@ -3557,6 +3660,7 @@ void __init setup_user_features(void)
}
minsigstksz_setup();
+ mpam_extra_caps();
}
static int enable_mismatched_32bit_el0(unsigned int cpu)
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 09eeaa24d456..4a4cfe496bb6 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -477,6 +477,10 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
if (id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))
__cpuinfo_store_cpu_32bit(&info->aarch32);
+ if (IS_ENABLED(CONFIG_ARM64_MPAM) &&
+ id_aa64pfr0_mpam(info->reg_id_aa64pfr0))
+ info->reg_mpamidr = read_cpuid(MPAMIDR_EL1);
+
cpuinfo_detect_icache_policy(info);
}
diff --git a/arch/arm64/kernel/mpam.c b/arch/arm64/kernel/mpam.c
new file mode 100644
index 000000000000..0c49bafc46bf
--- /dev/null
+++ b/arch/arm64/kernel/mpam.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2024 Arm Ltd. */
+
+#include <asm/mpam.h>
+
+#include <linux/jump_label.h>
+
+DEFINE_STATIC_KEY_FALSE(arm64_mpam_has_hcr);
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 62b2838a231a..2685cfa8376c 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -59,6 +59,7 @@ HW_DBM
KVM_HVHE
KVM_PROTECTED_MODE
MISMATCHED_CACHE_TYPE
+MPAM
MTE
MTE_ASYMM
SME
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index a4c1dd4741a4..8bc1ce34587f 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2911,6 +2911,22 @@ Res0 1
Field 0 EN
EndSysreg
+Sysreg MPAMIDR_EL1 3 0 10 4 4
+Res0 63:62
+Field 61 HAS_SDEFLT
+Field 60 HAS_FORCE_NS
+Field 59 SP4
+Field 58 HAS_TIDR
+Field 57 HAS_ALTSP
+Res0 56:40
+Field 39:32 PMG_MAX
+Res0 31:21
+Field 20:18 VPMR_MAX
+Field 17 HAS_HCR
+Res0 16
+Field 15:0 PARTID_MAX
+EndSysreg
+
Sysreg LORID_EL1 3 0 10 4 7
Res0 63:24
Field 23:16 LD
@@ -2918,6 +2934,22 @@ Res0 15:8
Field 7:0 LR
EndSysreg
+Sysreg MPAM1_EL1 3 0 10 5 0
+Res0 63:48
+Field 47:40 PMG_D
+Field 39:32 PMG_I
+Field 31:16 PARTID_D
+Field 15:0 PARTID_I
+EndSysreg
+
+Sysreg MPAM0_EL1 3 0 10 5 1
+Res0 63:48
+Field 47:40 PMG_D
+Field 39:32 PMG_I
+Field 31:16 PARTID_D
+Field 15:0 PARTID_I
+EndSysreg
+
Sysreg ISR_EL1 3 0 12 1 0
Res0 63:11
Field 10 IS
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 3/6] KVM: arm64: Fix missing traps of guest accesses to the MPAM registers
From: James Morse @ 2024-03-21 16:57 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm
Cc: Marc Zyngier, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Jing Zhang, James Morse,
Anshuman Khandual
In-Reply-To: <20240321165728.31907-1-james.morse@arm.com>
commit 011e5f5bf529f ("arm64/cpufeature: Add remaining feature bits in
ID_AA64PFR0 register") exposed the MPAM field of AA64PFR0_EL1 to guests,
but didn't add trap handling.
If you are unlucky, this results in an MPAM aware guest being delivered
an undef during boot. The host prints:
| kvm [97]: Unsupported guest sys_reg access at: ffff800080024c64 [00000005]
| { Op0( 3), Op1( 0), CRn(10), CRm( 5), Op2( 0), func_read },
Which results in:
| Internal error: Oops - Undefined instruction: 0000000002000000...
| Modules linked in:
| CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.6.0-rc7-00559 #14
| Hardware name: linux,dummy-virt (DT)
| pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : test_has_mpam+0x18/0x30
| lr : test_has_mpam+0x10/0x30
| sp : ffff80008000bd90
...
| Call trace:
| test_has_mpam+0x18/0x30
| update_cpu_capabilities+0x7c/0x11c
| setup_cpu_features+0x14/0xd8
| smp_cpus_done+0x24/0xb8
| smp_init+0x7c/0x8c
| kernel_init_freeable+0xf8/0x280
| kernel_init+0x24/0x1e0
| ret_from_fork+0x10/0x20
| Code: 910003fd 97ffffde 72001c00 54000080 (d538a500)
| ---[ end trace 0000000000000000 ]---
| Kernel panic - not syncing: Attempted to kill init! exitcode=...
Add the support to enable the traps, and handle the three guest accessible
registers by injecting an UNDEF. This stops KVM from spamming the host
log, but doesn't yet hide the feature from the id registers.
With MPAM v1.0 we can trap the MPAMIDR_EL1 register only if
ARM64_HAS_MPAM_HCR, with v1.1 an additional MPAM2_EL2.TIDR bit traps
MPAMIDR_EL1 on platforms that don't have MPAMHCR_EL2. Enable one of
these if either is supported. If neither is supported, the guest can
discover that the CPU has MPAM support, and how many PARTID etc the
host has ... but it can't influence anything, so its harmless.
Fixes: 011e5f5bf529f ("arm64/cpufeature: Add remaining feature bits in ID_AA64PFR0 register")
CC: Anshuman Khandual <anshuman.khandual@arm.com>
Link: https://lore.kernel.org/linux-arm-kernel/20200925160102.118858-1-james.morse@arm.com/
Signed-off-by: James Morse <james.morse@arm.com>
---
arch/arm64/include/asm/kvm_arm.h | 1 +
arch/arm64/include/asm/mpam.h | 4 ++--
arch/arm64/kernel/image-vars.h | 5 ++++
arch/arm64/kvm/hyp/include/hyp/switch.h | 32 +++++++++++++++++++++++++
arch/arm64/kvm/sys_regs.c | 11 +++++++++
5 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index e01bb5ca13b7..532007e2f860 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -104,6 +104,7 @@
#define HCRX_GUEST_FLAGS (HCRX_EL2_SMPME | HCRX_EL2_TCR2En)
#define HCRX_HOST_FLAGS (HCRX_EL2_MSCEn | HCRX_EL2_TCR2En | HCRX_EL2_EnFPM)
+#define MPAMHCR_HOST_FLAGS 0
/* TCR_EL2 Registers bits */
#define TCR_EL2_DS (1UL << 32)
diff --git a/arch/arm64/include/asm/mpam.h b/arch/arm64/include/asm/mpam.h
index bae662107082..72735d8a217b 100644
--- a/arch/arm64/include/asm/mpam.h
+++ b/arch/arm64/include/asm/mpam.h
@@ -50,7 +50,7 @@
DECLARE_STATIC_KEY_FALSE(arm64_mpam_has_hcr);
/* check whether all CPUs have MPAM support */
-static inline bool mpam_cpus_have_feature(void)
+static __always_inline bool mpam_cpus_have_feature(void)
{
if (IS_ENABLED(CONFIG_ARM64_MPAM))
return cpus_have_final_cap(ARM64_MPAM);
@@ -58,7 +58,7 @@ static inline bool mpam_cpus_have_feature(void)
}
/* check whether all CPUs have MPAM virtualisation support */
-static inline bool mpam_cpus_have_mpam_hcr(void)
+static __always_inline bool mpam_cpus_have_mpam_hcr(void)
{
if (IS_ENABLED(CONFIG_ARM64_MPAM))
return static_branch_unlikely(&arm64_mpam_has_hcr);
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index ba4f8f7d6a91..42c7eca708e7 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -101,6 +101,11 @@ KVM_NVHE_ALIAS(nvhe_hyp_panic_handler);
/* Vectors installed by hyp-init on reset HVC. */
KVM_NVHE_ALIAS(__hyp_stub_vectors);
+/* Additional static keys for cpufeatures */
+#ifdef CONFIG_ARM64_MPAM
+KVM_NVHE_ALIAS(arm64_mpam_has_hcr);
+#endif
+
/* Static keys which are set if a vGIC trap should be handled in hyp. */
KVM_NVHE_ALIAS(vgic_v2_cpuif_trap);
KVM_NVHE_ALIAS(vgic_v3_cpuif_trap);
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index e3fcf8c4d5b4..44bc446859a2 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -27,6 +27,7 @@
#include <asm/kvm_hyp.h>
#include <asm/kvm_mmu.h>
#include <asm/kvm_nested.h>
+#include <asm/mpam.h>
#include <asm/fpsimd.h>
#include <asm/debug-monitors.h>
#include <asm/processor.h>
@@ -210,6 +211,35 @@ static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu)
__deactivate_fgt(hctxt, vcpu, kvm, HAFGRTR_EL2);
}
+static inline void __activate_traps_mpam(struct kvm_vcpu *vcpu)
+{
+ u64 r = MPAM_SYSREG_TRAP_MPAM0_EL1 | MPAM_SYSREG_TRAP_MPAM1_EL1;
+
+ if (!mpam_cpus_have_feature())
+ return;
+
+ /* trap guest access to MPAMIDR_EL1 */
+ if (mpam_cpus_have_mpam_hcr()) {
+ write_sysreg_s(MPAMHCR_TRAP_MPAMIDR, SYS_MPAMHCR_EL2);
+ } else {
+ /* From v1.1 TIDR can trap MPAMIDR, set it unconditionally */
+ r |= MPAM_SYSREG_TRAP_IDR;
+ }
+
+ write_sysreg_s(r, SYS_MPAM2_EL2);
+}
+
+static inline void __deactivate_traps_mpam(void)
+{
+ if (!mpam_cpus_have_feature())
+ return;
+
+ write_sysreg_s(0, SYS_MPAM2_EL2);
+
+ if (mpam_cpus_have_mpam_hcr())
+ write_sysreg_s(MPAMHCR_HOST_FLAGS, SYS_MPAMHCR_EL2);
+}
+
static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
{
/* Trap on AArch32 cp15 c15 (impdef sysregs) accesses (EL1 or EL0) */
@@ -250,6 +280,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
}
__activate_traps_hfgxtr(vcpu);
+ __activate_traps_mpam(vcpu);
}
static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
@@ -269,6 +300,7 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
write_sysreg_s(HCRX_HOST_FLAGS, SYS_HCRX_EL2);
__deactivate_traps_hfgxtr(vcpu);
+ __deactivate_traps_mpam();
}
static inline void ___activate_traps(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c9f4f387155f..d6afb21849de 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -537,6 +537,14 @@ static bool trap_oslar_el1(struct kvm_vcpu *vcpu,
return true;
}
+static bool trap_mpam(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+ const struct sys_reg_desc *r)
+{
+ kvm_inject_undefined(vcpu);
+
+ return false;
+}
+
static bool trap_oslsr_el1(struct kvm_vcpu *vcpu,
struct sys_reg_params *p,
const struct sys_reg_desc *r)
@@ -2429,8 +2437,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_LOREA_EL1), trap_loregion },
{ SYS_DESC(SYS_LORN_EL1), trap_loregion },
{ SYS_DESC(SYS_LORC_EL1), trap_loregion },
+ { SYS_DESC(SYS_MPAMIDR_EL1), trap_mpam },
{ SYS_DESC(SYS_LORID_EL1), trap_loregion },
+ { SYS_DESC(SYS_MPAM1_EL1), trap_mpam },
+ { SYS_DESC(SYS_MPAM0_EL1), trap_mpam },
{ SYS_DESC(SYS_VBAR_EL1), access_rw, reset_val, VBAR_EL1, 0 },
{ SYS_DESC(SYS_DISR_EL1), NULL, reset_val, DISR_EL1, 0 },
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 6/6] KVM: arm64: selftests: Test ID_AA64PFR0.MPAM isn't completely ignored
From: James Morse @ 2024-03-21 16:57 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm
Cc: Marc Zyngier, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Jing Zhang, James Morse
In-Reply-To: <20240321165728.31907-1-james.morse@arm.com>
The ID_AA64PFR0.MPAM bit was previously accidentally exposed to guests,
and is ignored by KVM. KVM will always present the guest with 0 here,
and trap the MPAM system registers to inject an undef.
But, this vaulue is still needed to prevent migration when the value
is incompatible with the target hardware. Add a kvm unit test to try
and write multiple values to ID_AA64PFR0.MPAM. Only the hardware value
previously exposed should be ignored, all other values should be
rejected.
Signed-off-by: James Morse <james.morse@arm.com>
---
.../selftests/kvm/aarch64/set_id_regs.c | 53 ++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
index b42a0dc19852..fbb600111969 100644
--- a/tools/testing/selftests/kvm/aarch64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
@@ -426,6 +426,56 @@ static void test_user_set_reg(struct kvm_vcpu *vcpu, bool aarch64_only)
}
}
+#define MPAM_IDREG_TEST 1
+static void test_user_set_mpam_reg(struct kvm_vcpu *vcpu)
+{
+ uint64_t masks[KVM_ARM_FEATURE_ID_RANGE_SIZE];
+ struct reg_mask_range range = {
+ .addr = (__u64)masks,
+ };
+ uint64_t val, ftr_mask;
+ int idx, err;
+
+ /*
+ * If ID_AA64PFR0.MPAM is _not_ officially modifiable and is zero,
+ * check that if it can be set to 1, (i.e. it is supported by the
+ * hardware), that it can't be set to other values.
+ */
+
+ /* Get writable masks for feature ID registers */
+ memset(range.reserved, 0, sizeof(range.reserved));
+ vm_ioctl(vcpu->vm, KVM_ARM_GET_REG_WRITABLE_MASKS, &range);
+
+ /* Writeable? Nothing to test! */
+ idx = sys_reg_to_idx(SYS_ID_AA64PFR0_EL1);
+ ftr_mask = ID_AA64PFR0_EL1_MPAM_MASK;
+ if ((masks[idx] & ftr_mask) == ftr_mask) {
+ ksft_test_result_skip("ID_AA64PFR0.MPAM is officially writable, nothing to test\n");
+ return;
+ }
+
+ /* Get the id register value */
+ vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), &val);
+
+ /* Try to set MPAM=1 */
+ val &= ~GENMASK_ULL(44, 40);
+ val |= FIELD_PREP(GENMASK_ULL(44, 40), 1);
+ err = __vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), val);
+ if (err) {
+ ksft_test_result_skip("ID_AA64PFR0.MPAM is not writable, nothing to test\n");
+ return;
+ }
+
+ /* Try to set MPAM=2 */
+ val &= ~GENMASK_ULL(43, 40);
+ val |= FIELD_PREP(GENMASK_ULL(43, 40), 2);
+ err = __vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), val);
+ if (err == -EPERM)
+ ksft_test_result_pass("ID_AA64PFR0_EL1.MPAM not arbitrarily modifiable\n");
+ else
+ ksft_test_result_fail("ID_AA64PFR0_EL1.MPAM value should not be ignored\n");
+}
+
static void test_guest_reg_read(struct kvm_vcpu *vcpu)
{
bool done = false;
@@ -477,12 +527,13 @@ int main(void)
ARRAY_SIZE(ftr_id_aa64isar2_el1) + ARRAY_SIZE(ftr_id_aa64pfr0_el1) +
ARRAY_SIZE(ftr_id_aa64mmfr0_el1) + ARRAY_SIZE(ftr_id_aa64mmfr1_el1) +
ARRAY_SIZE(ftr_id_aa64mmfr2_el1) + ARRAY_SIZE(ftr_id_aa64zfr0_el1) -
- ARRAY_SIZE(test_regs);
+ ARRAY_SIZE(test_regs) + MPAM_IDREG_TEST;
ksft_set_plan(ftr_cnt);
test_user_set_reg(vcpu, aarch64_only);
test_guest_reg_read(vcpu);
+ test_user_set_mpam_reg(vcpu);
kvm_vm_free(vm);
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 1/2] dt-bindings: thermal: convert st,stih407-thermal to DT schema
From: Daniel Lezcano @ 2024-03-21 17:16 UTC (permalink / raw)
To: Raphael Gallais-Pou, Rafael J. Wysocki, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Patrice Chotard,
Lee Jones
Cc: linux-pm, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20240320-thermal-v3-1-700296694c4a@gmail.com>
On 20/03/2024 22:33, Raphael Gallais-Pou wrote:
> 'st,passive_colling_temp' does not appear in the device-tree, 'reg' and
> '#thermal-sensor-cells' are also missing in the device description.
>
> Convert st,stih407-thermal binding to DT schema format in order to clean
> unused 'st,passive_cooling_temp' and add missing properties.
>
> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
> ---
> Changes in v3:
> - Specify const value for '#thermal-sensor-cells'
> - Add 'maxItems' for 'interrupts' property
> - Change commit log accordingly
>
> Changes in v2:
> - Change commit log to use imperative
> - Drop description
> - Drop 'clocks' description
> - Add 'reg' property
> - Add '#thermal-sensor-cells'
> - Make node name generic in example
> - Fix YAML style
> ---
> .../bindings/thermal/st,stih407-thermal.yaml | 58 ++++++++++++++++++++++
> .../devicetree/bindings/thermal/st-thermal.txt | 32 ------------
> 2 files changed, 58 insertions(+), 32 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/st,stih407-thermal.yaml b/Documentation/devicetree/bindings/thermal/st,stih407-thermal.yaml
> new file mode 100644
> index 000000000000..9f6fc5c95c55
> --- /dev/null
Applied, thanks
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 5/8] drm: zynqmp_dp: Don't retrain the link in our IRQ
From: Tomi Valkeinen @ 2024-03-21 17:25 UTC (permalink / raw)
To: Sean Anderson
Cc: Michal Simek, David Airlie, linux-kernel, Daniel Vetter,
linux-arm-kernel, Laurent Pinchart, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, dri-devel
In-Reply-To: <53b2df23-d5ea-498b-a501-b64f753c0074@linux.dev>
On 21/03/2024 17:52, Sean Anderson wrote:
> On 3/20/24 02:53, Tomi Valkeinen wrote:
>> On 20/03/2024 00:51, Sean Anderson wrote:
>>> Retraining the link can take a while, and might involve waiting for
>>> DPCD reads/writes to complete. This is inappropriate for an IRQ handler.
>>> Just schedule this work for later completion. This is racy, but will be
>>> fixed in the next commit.
>>
>> You should add the locks first, and use them here, rather than first
>> adding a buggy commit and fixing it in the next one.
>
> I didn't think I could add the locks first since I only noticed the IRQ
> was threaded right before sending out this series. So yeah, we could add
> locking, add the workqueue, and then unthread the IRQ.
>
>>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>>> ---
>>> Actually, on second look this IRQ is threaded. So why do we have a
>>> workqueue for HPD events? Maybe we should make it unthreaded?
>>
>> Indeed, there's not much work being done in the IRQ handler. I don't know why it's threaded.
>>
>> We could move the queued work to be inside the threaded irq handler,
>> but with a quick look, the HPD work has lines like "msleep(100)" (and
>> that's inside a for loop...), which is probably not a good thing to do
>> even in threaded irq handler.
>>
>> Although I'm not sure if that code is good to have anywhere. Why do we
>> even have such code in the HPD work path... We already got the HPD
>> interrupt. What does "It takes some delay (ex, 100 ~ 500 msec) to get
>> the HPD signal with some monitors" even mean...
>
> The documentation for this bit is
>
> | HPD_STATE 0 ro 0x0 Contains the raw state of the HPD pin on the DisplayPort connector.
>
> So I think the idea is to perform some debouncing.
Hmm, it just looks a bit odd to me. It can sleep for a second. And the
wording "It takes some delay (ex, 100 ~ 500 msec) to get the HPD signal
with some monitors" makes it sound like some kind of a hack...
The docs mention debounce once:
https://docs.amd.com/r/en-US/pg299-v-dp-txss1/Hot-Plug-Detection
But it's not immediately obvious what the SW must do and what's done by
the HW. Debounce is not mentioned later, e.g. in the HPD Event Handling.
But if debounce is needed, wouldn't it be perhaps in a few milliseconds,
instead of hundreds of milliseconds...
zynqmp_dp_bridge_detect() is used for drm_bridge_funcs.detect(), and if
the cable is not connected, it'll sleep for 1 second (probably more)
until returning not connected. It just doesn't sound correct to me.
Well, it's not part of this patch as such, but related to the amount of
time we spend in the interrupt handler (and also the detect()).
>> Would it be possible to clean up the work funcs a bit (I haven't
>> looked a the new work func yet), to remove the worst extra sleeps, and
>> just do all that inside the threaded irq handler?
>
> Probably not, since a HPD IRQ results in link retraining, which can take a while.
But is it any different if you have a workqueue? Isn't a threaded
interrupt handler basically the same thing?
Probably at least the zynqmp_dp_hpd_work_func() could be done in the
threaded irq just fine, if the insane 1s sleep can be dropped.
>> Do we need to handle interrupts while either delayed work is being done?
>
> Probably not.
>
>> If we do need a delayed work, would just one work be enough which
>> handles both HPD_EVENT and HPD_IRQ, instead of two?
>
> Maybe, but then we need to determine which pending events we need to
> handle. I think since we have only two events it will be easier to just
> have separate workqueues.
The less concurrency, the better...Which is why it would be nice to do
it all in the threaded irq.
Tomi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 0/0] (proposed?) Add ACPI binding to Rockchip RK3xxx I2C bus
From: Shimrra Shai @ 2024-03-21 17:34 UTC (permalink / raw)
To: linux-kernel
Cc: Shimrra Shai, linux-rockchip, linux-arm-kernel, heiko,
max.schwarz
Hi! I have been contributing to a firmware project for Rockchip RK3588-
based board using the TianoCore EDK2 firmware development kit, at
https://github.com/edk2-porting/edk2-rk3588. I am seeking to get it
possible to seamlessly boot mainline kernels using it on these platforms,
especially given how that mainline support for this platform has progressed
far since its debut. Since this is not a firmware from a board vendor, I'm
actually not really sure how to best interface or integrate its development
with the Linux kernel community. Particularly the question of Device Tree
Blob (DTB) vs. ACPI binding for configuration bindings, for which previous
discussions have always seemed to center around situations where the
firmware was not controllable by the kernel developer, but that is not the
case with this situation here and now. However, I've found, particularly
given the firmware has an HDMI driver that the currnet kernel does not,
that I am able to get a quicker path to mainline boot on this platform
going through the ACPI route, which inspires the following patch.
In particular, I found one small issue that obstructs immediate direct
booting of the off-the-shelf downloaded 6.8.x kernel series on my platform
(Firefly ITX-3588J, with no mainline device tree blob support) which is
that the I2C bus lacks code to configure it in the ACPI boot mode. The
firmware came with ACPI bindings for it, nonetheless, and so this patch
acts to enable and document those bindings for future use.
Signed-off-by: Shimrra Shai <shimmyshai00@gmail.com>
---
Documentation/firmware-guide/acpi/dsd/soc/general.rst | 32 ++++++++
Documentation/firmware-guide/acpi/dsd/soc/soc-i2c.rst | 50 ++++++++++++
drivers/i2c/busses/i2c-rk3x.c | 105 +++++++++++++++++++++++---
3 files changed, 177 insertions(+), 10 deletions(-)
diff --git a/Documentation/firmware-guide/acpi/dsd/soc/general.rst b/Documentation/firmware-guide/acpi/dsd/soc/general.rst
new file mode 100644
index 000000000..d21ccad2d
--- /dev/null
+++ b/Documentation/firmware-guide/acpi/dsd/soc/general.rst
@@ -0,0 +1,32 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==========================================
+System-on-Chip (SoC) facility descriptions
+==========================================
+
+These documents describe how to create ACPI profiles for devices compsed into
+system-on-chip (SoC) architectures. Currently, we only describe the options
+availed in the _DSD block for select platforms; other considerations are not
+yet applied. The inspiration for these documents are based around the advent
+(as of March 2024) of hefty, non-Apple ARM SoC systems such as the Rockchip
+RK3588 and newer chips that have desktop-like performance and thus are prime
+candidates for a UEFI-based desktop-like boot system with the goal being to
+deliver the same user-friendly ease of loading operating systems as on the
+Intel x86 sphere. Open-source UEFI-based firmware engines, such as TianoCore
+[1], mean it is possible on such platforms for the open-source developer to
+control both firmware and kernel simultaneously, which is not the case for
+the situation with Intel-based PC boards where boards are provided with pre-
+baked, vendor-selected and opaque firmwares.
+
+The description of ACPI usage here is not meant to suggest that ACPI replace
+Deviec Tree altogether for such SoCs; rather, we recognize that given they
+often will have a variety of applications that may include embedded usage where
+that more hard-wired boot loader setups such as U-Boot still shine, the
+maintenance of ACPI and DTB-based configuration options should be in parallel,
+and it may be possible for the same firmware to deploy both options.
+
+References
+==========
+
+[1] EDK2-RK3588 port of TianoCore EDK2 firmware.
+ https://github.com/edk2-porting/edk2-rk3588
diff --git a/Documentation/firmware-guide/acpi/dsd/soc/soc-i2c.rst b/Documentation/firmware-guide/acpi/dsd/soc/soc-i2c.rst
new file mode 100644
index 000000000..47ff69db4
--- /dev/null
+++ b/Documentation/firmware-guide/acpi/dsd/soc/soc-i2c.rst
@@ -0,0 +1,50 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================
+I2C buses on SoCs
+=================
+
+This document describes the ACPI _DSD parameters currently being employed for
+Inter-Integrated Circuit (I2C) buses in the Linux kernel. It is based off the
+conventions used in the Rockchip RK3588 firmware project [1], which is also the
+first SoC documented here.
+
+General considerations
+======================
+
+For general use, we recommend indicating I2C busses in the ACPI firmware table
+in the following manner. First, they should be named I2Cx, where "x" is the bus
+index, and that index should also be used for the _UID component, e.g. on
+Rockchip RK3588 (see below), we use:
+
+ Device (I2Cx) {
+ Name (_HID, "RKCP3001")
+ Name (_CID, "PRP0001")
+ Name (_UID, x)
+ Name (_CCA, 0)
+
+ ...
+ }
+
+Interrupts should be specified in the usual ACPI manner. Parameters specific to
+the Rockchip and I2C devices are indicated in the _DSD block as given below.
+The parameters are a curated selection from the Device Tree Blob (DTB)
+representation.
+
+_DSD parameters for different SoCs
+==================================
+
+Rockchip RK3588
+---------------
+
+The following parameters are accepted for the I2C on Rockchip RK3588.
+
+- i2c,clk-rate Describe the pclk rate for the I2C bus, in Hz.
+- rockchip,bclk Describe the bclk rate for the I2C bus, in Hz.
+
+References
+==========
+
+[1] EDK2-RK3588 port of TianoCore EDK2 firmware.
+ https://github.com/edk2-porting/edk2-rk3588
+
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 086fdf262..5a5e1d551 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -19,6 +19,9 @@
#include <linux/of_irq.h>
#include <linux/spinlock.h>
#include <linux/clk.h>
+#ifdef CONFIG_ACPI
+#include <linux/clk-provider.h>
+#endif
#include <linux/wait.h>
#include <linux/mfd/syscon.h>
#include <linux/regmap.h>
@@ -1235,6 +1238,15 @@ static const struct of_device_id rk3x_i2c_match[] = {
};
MODULE_DEVICE_TABLE(of, rk3x_i2c_match);
+#ifdef CONFIG_ACPI
+/* for RK3588 and at least when loaded with EDK2-RK3588 Tianocore firmware */
+static const struct acpi_device_id rk3x_i2c_acpi_match[] = {
+ { .id = "RKCP3001", .driver_data = (kernel_ulong_t)&rk3399_soc_data },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, rk3x_i2c_acpi_match);
+#endif
+
static int rk3x_i2c_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -1243,6 +1255,10 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
int ret = 0;
int bus_nr;
u32 value;
+#ifdef CONFIG_ACPI
+ u64 value64;
+ char clk_name[20];
+#endif
int irq;
unsigned long clk_rate;
@@ -1250,8 +1266,12 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
if (!i2c)
return -ENOMEM;
- match = of_match_node(rk3x_i2c_match, np);
- i2c->soc_data = match->data;
+ if (acpi_disabled) {
+ match = of_match_node(rk3x_i2c_match, np);
+ i2c->soc_data = match->data;
+ } else {
+ i2c->soc_data = device_get_match_data(&pdev->dev);
+ }
/* use common interface to get I2C timing properties */
i2c_parse_fw_timings(&pdev->dev, &i2c->t, true);
@@ -1266,6 +1286,9 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
i2c->dev = &pdev->dev;
+ if (!acpi_disabled)
+ ACPI_COMPANION_SET(&i2c->adap.dev, ACPI_COMPANION(&pdev->dev));
+
spin_lock_init(&i2c->lock);
init_waitqueue_head(&i2c->wait);
@@ -1273,8 +1296,25 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
if (IS_ERR(i2c->regs))
return PTR_ERR(i2c->regs);
- /* Try to set the I2C adapter number from dt */
- bus_nr = of_alias_get_id(np, "i2c");
+ if (acpi_disabled) {
+ /* Try to set the I2C adapter number from dt */
+ bus_nr = of_alias_get_id(np, "i2c");
+ } else {
+ ret = acpi_dev_uid_to_integer(ACPI_COMPANION(&pdev->dev),
+ &value64);
+ if (ret) {
+ dev_err(&pdev->dev, "Cannot retrieve UID\n");
+ return ret;
+ }
+
+ if (value64 < INT_MAX) {
+ bus_nr = (int) value64;
+ } else {
+ /* shouldn't happen!!! */
+ dev_err(&pdev->dev, "Too big UID\n");
+ return -EINVAL;
+ }
+ }
/*
* Switch to new interface if the SoC also offers the old one.
@@ -1325,13 +1365,58 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, i2c);
- if (i2c->soc_data->calc_timings == rk3x_i2c_v0_calc_timings) {
- /* Only one clock to use for bus clock and peripheral clock */
- i2c->clk = devm_clk_get(&pdev->dev, NULL);
- i2c->pclk = i2c->clk;
+ if (acpi_disabled) {
+ if (i2c->soc_data->calc_timings == rk3x_i2c_v0_calc_timings) {
+ /* Only one clock to use for bus clock and peripheral
+ * clock
+ */
+ i2c->clk = devm_clk_get(&pdev->dev, NULL);
+ i2c->pclk = i2c->clk;
+ } else {
+ i2c->clk = devm_clk_get(&pdev->dev, "i2c");
+ i2c->pclk = devm_clk_get(&pdev->dev, "pclk");
+ }
} else {
- i2c->clk = devm_clk_get(&pdev->dev, "i2c");
- i2c->pclk = devm_clk_get(&pdev->dev, "pclk");
+ if (i2c->soc_data->calc_timings != rk3x_i2c_v0_calc_timings) {
+ u32 bclkrate = 0;
+ u32 pclkrate = 0;
+
+ device_property_read_u32(&pdev->dev, "rockchip,bclk",
+ &bclkrate);
+ device_property_read_u32(&pdev->dev, "i2c,clk-rate",
+ &pclkrate);
+
+ if (bclkrate != 0) {
+ sprintf(clk_name, "rockchip,i2c-clk.%d", bus_nr);
+ i2c->clk = clk_register_fixed_rate(&pdev->dev,
+ clk_name,
+ NULL,
+ 0, bclkrate);
+ dev_dbg(&pdev->dev,
+ "registered i2c clk at %u Hz\n",
+ bclkrate);
+ }
+
+ if (pclkrate != 0) {
+ sprintf(clk_name, "rockchip,i2c-pclk.%d",
+ bus_nr);
+ i2c->pclk = clk_register_fixed_rate(&pdev->dev,
+ clk_name,
+ NULL, 0,
+ pclkrate);
+ dev_dbg(&pdev->dev,
+ "registered i2c pclk at %u Hz\n",
+ pclkrate);
+ }
+ } else {
+ /* NB: currently not expected w/UEFI firmware given
+ * these are not super performant RK3xxx
+ */
+ dev_err(&pdev->dev,
+ "ACPI not supported for this RK3xxx\n");
+
+ return -EINVAL;
+ }
}
if (IS_ERR(i2c->clk))
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v5 14/27] iommu/arm-smmu-v3: Make changing domains be hitless for ATS
From: Jason Gunthorpe @ 2024-03-21 17:32 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Nicolin Chen,
patches, Shameerali Kolothum Thodi
In-Reply-To: <CAKHBV26dSY+Gy-x9HTWVCcNMyAkB1g+Y+dv-Lg+-furpu9bnSw@mail.gmail.com>
On Thu, Mar 21, 2024 at 10:53:20PM +0800, Michael Shavit wrote:
> On Thu, Mar 21, 2024 at 9:28 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Thu, Mar 21, 2024 at 08:26:43PM +0800, Michael Shavit wrote:
> > > Overall I think the patch works, but it took me a while to really
> > > digest the big picture. Just to make sure I fully understand:
> > >
> > > We're trying to satisfy the following invariants for correctness:
> > > 1. Devices cannot get translations from a domain that was detached
> > > after arm_smmu_attach_dev() returns.
> >
> > Yes, regardless of ATS this is the API requirement.
> >
> > > 2. Devices cannot get cached translations from a domain after
> > > arm_smmu_atc_inv_domain() returns, regardless of whether the domain is
> > > simultaneously being attached/detached from a device.
> >
> > There is no simultaneously here since the group lock is held by the
> > core code.
>
> I meant a call to arm_smmu_attach_dev concurrent with
> arm_smmu_atc_inv_domain (through tlb_flush_all).
Oh, yes that is the concurrency that drives alot of this organization
> > Yes, we can't just disable ATS due to the PASIDs. A SVA PASID might be
> > present and disabling ATS to change the RID domain would completely
> > wreck it. Today this scenario is prevented by sva_enable, which is
> > removed in following patches.
>
> What do you mean by wrecking? We might slow it down to a crawl but we
> wouldn't be corrupting or destroying anything by disabling ATS while
> SVA is running would we?
> Oh does disabling ATS abort all transactions rather than making it transparent?
As you deduced, without ATS a non-present page will return an abort
back to the device instead of a non-present ATS reply which should
then trigger PRI.
IOW the SVA domain can no longer handle page faults and is effectively
non-functioning if ATS is disabled.
Jason
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox