* [PATCH v2 2/3] clk: samsung: gs101: propagate PERIC1 USI SPI clock rate
From: Tudor Ambarus @ 2024-03-26 17:28 UTC (permalink / raw)
To: peter.griffin, krzysztof.kozlowski
Cc: alim.akhtar, linux-arm-kernel, linux-samsung-soc, linux-kernel,
andre.draszik, willmcvicker, kernel-team, s.nawrocki, cw00.choi,
mturquette, sboyd, semen.protsenko, linux-clk, jaewon02.kim,
Tudor Ambarus
In-Reply-To: <20240326172813.801470-1-tudor.ambarus@linaro.org>
When SPI transfer is being prepared, the spi-s3c64xx driver will call
clk_set_rate() to change the rate of SPI source clock (IPCLK). But IPCLK
is a gate (leaf) clock, so it must propagate the rate change up the
clock tree, so that corresponding MUX/DIV clocks can actually change
their values. Add CLK_SET_RATE_PARENT flag to corresponding clocks for
all USI instances in GS101 PERIC1: USI{0, 9, 10, 11, 12, 13}. This change
involves the following clocks:
PERIC1 USI*:
Clock Div range MUX Selection
-------------------------------------------------------------------
gout_peric1_peric1_top0_ipclk_* - -
dout_peric1_usi*_usi /1..16 -
mout_peric1_usi*_usi_user - {24.5 MHz, 400 MHz}
With input clock of 400 MHz this scheme provides the following IPCLK
rate range, for each USI block:
PERIC1 USI*: 1.5 MHz ... 400 MHz
Accounting for internal /4 divider in SPI blocks, and because the max
SPI frequency is limited at 50 MHz, it gives us next SPI SCK rates:
PERIC1 USI_SPI*: 384 KHz ... 49.9 MHz
Which shall be fine for the applications of the SPI bus.
Note that with this we allow the reparenting of the MUX_USIx clocks to
OSCCLK. Each instance of the USI IP has its own MUX_USI clock, thus the
reparenting of a MUX_USI clock corresponds to a single instance of the
USI IP. The datasheet mentions OSCCLK just in the low-power mode
context, but the downstream driver reparents too the MUX_USI clocks to
OSCCLK. Follow the downstream driver and do the same.
Fixes: 63b4bd1259d9 ("clk: samsung: gs101: add support for cmu_peric1")
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/clk/samsung/clk-gs101.c | 90 ++++++++++++++++++---------------
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index d065e343a85d..ddf2d57eed68 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -3230,47 +3230,53 @@ static const struct samsung_mux_clock peric1_mux_clks[] __initconst = {
MUX(CLK_MOUT_PERIC1_I3C_USER,
"mout_peric1_i3c_user", mout_peric1_nonbususer_p,
PLL_CON0_MUX_CLKCMU_PERIC1_I3C_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI0_USI_USER,
- "mout_peric1_usi0_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI0_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI10_USI_USER,
- "mout_peric1_usi10_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI10_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI11_USI_USER,
- "mout_peric1_usi11_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI11_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI12_USI_USER,
- "mout_peric1_usi12_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI12_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI13_USI_USER,
- "mout_peric1_usi13_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI13_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI9_USI_USER,
- "mout_peric1_usi9_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI9_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI0_USI_USER,
+ "mout_peric1_usi0_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI0_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI10_USI_USER,
+ "mout_peric1_usi10_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI10_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI11_USI_USER,
+ "mout_peric1_usi11_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI11_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI12_USI_USER,
+ "mout_peric1_usi12_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI12_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI13_USI_USER,
+ "mout_peric1_usi13_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI13_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI9_USI_USER,
+ "mout_peric1_usi9_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI9_USI_USER, 4, 1),
};
static const struct samsung_div_clock peric1_div_clks[] __initconst = {
DIV(CLK_DOUT_PERIC1_I3C, "dout_peric1_i3c", "mout_peric1_i3c_user",
CLK_CON_DIV_DIV_CLK_PERIC1_I3C, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI0_USI,
- "dout_peric1_usi0_usi", "mout_peric1_usi0_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI0_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI10_USI,
- "dout_peric1_usi10_usi", "mout_peric1_usi10_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI10_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI11_USI,
- "dout_peric1_usi11_usi", "mout_peric1_usi11_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI11_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI12_USI,
- "dout_peric1_usi12_usi", "mout_peric1_usi12_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI12_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI13_USI,
- "dout_peric1_usi13_usi", "mout_peric1_usi13_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI13_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI9_USI,
- "dout_peric1_usi9_usi", "mout_peric1_usi9_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI9_USI, 0, 4),
+ DIV_F(CLK_DOUT_PERIC1_USI0_USI,
+ "dout_peric1_usi0_usi", "mout_peric1_usi0_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI0_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI10_USI,
+ "dout_peric1_usi10_usi", "mout_peric1_usi10_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI10_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI11_USI,
+ "dout_peric1_usi11_usi", "mout_peric1_usi11_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI11_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI12_USI,
+ "dout_peric1_usi12_usi", "mout_peric1_usi12_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI12_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI13_USI,
+ "dout_peric1_usi13_usi", "mout_peric1_usi13_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI13_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI9_USI,
+ "dout_peric1_usi9_usi", "mout_peric1_usi9_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI9_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
};
static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
@@ -3305,27 +3311,27 @@ static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_1,
"gout_peric1_peric1_top0_ipclk_1", "dout_peric1_usi0_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_1,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_2,
"gout_peric1_peric1_top0_ipclk_2", "dout_peric1_usi9_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_2,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_3,
"gout_peric1_peric1_top0_ipclk_3", "dout_peric1_usi10_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_3,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_4,
"gout_peric1_peric1_top0_ipclk_4", "dout_peric1_usi11_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_4,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_5,
"gout_peric1_peric1_top0_ipclk_5", "dout_peric1_usi12_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_5,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_6,
"gout_peric1_peric1_top0_ipclk_6", "dout_peric1_usi13_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_6,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_8,
"gout_peric1_peric1_top0_ipclk_8", "dout_peric1_i3c",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_8,
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
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 v2 0/3] clk: samsung: introduce nMUX to reparent MUX clocks
From: Tudor Ambarus @ 2024-03-26 17:28 UTC (permalink / raw)
To: peter.griffin, krzysztof.kozlowski
Cc: alim.akhtar, linux-arm-kernel, linux-samsung-soc, linux-kernel,
andre.draszik, willmcvicker, kernel-team, s.nawrocki, cw00.choi,
mturquette, sboyd, semen.protsenko, linux-clk, jaewon02.kim,
Tudor Ambarus
All samsung MUX clocks that are defined with MUX() set the
CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents MUXes to be
reparented during clk_set_rate().
Introduce nMUX() for MUX clocks that can be reparented. nMUX is used
in GS101 to reparent the USI MUX to OSCCLK on low SPI clock rates.
Each instance of the USI IP in GS101 has its own MUX_USI clock, thus
the reparenting of a MUX_USI clock corresponds to a single instance
of the USI IP. We allow the reparenting of the MUX_USIx clocks to
OSCCLK. The datasheet mentions OSCCLK just in the low-power mode
context, but the downstream driver reparents too the MUX_USI clocks
to OSCCLK. Follow the downstream driver and do the same.
Tested with USI6 and USI13 SPI.
Find discussion on MUX reparenting to OSCCLK at:
https://lore.kernel.org/linux-samsung-soc/d508dfc1-bc28-4470-92aa-cf71915966f4@linaro.org/
v2:
- reword commit messages
- drop exynos850 patch on Sam's request
v1: https://lore.kernel.org/linux-samsung-soc/20240229122021.1901785-1-tudor.ambarus@linaro.org/
Tudor Ambarus (3):
clk: samsung: introduce nMUX for MUX clks that can reparented
clk: samsung: gs101: propagate PERIC1 USI SPI clock rate
clk: samsung: gs101: propagate PERIC0 USI SPI clock rate
drivers/clk/samsung/clk-gs101.c | 225 +++++++++++++++++---------------
drivers/clk/samsung/clk.h | 20 +++
2 files changed, 140 insertions(+), 105 deletions(-)
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] dt-bindings: display: rockchip: add missing #sound-dai-cells to dw-hdmi
From: Heiko Stuebner @ 2024-03-26 17:28 UTC (permalink / raw)
To: hjc, andy.yan, robh, krzysztof.kozlowski+dt, conor+dt
Cc: dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Heiko Stuebner
The #sound-dai-cells DT property is required to describe link between
the HDMI IP block and the SoC's audio subsystem.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
.../devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
index af638b6c0d21..3768df80ca7a 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
@@ -124,6 +124,9 @@ properties:
description:
phandle to the GRF to mux vopl/vopb.
+ "#sound-dai-cells":
+ const: 0
+
required:
- compatible
- reg
--
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: [RFC PATCH v1 3/4] mm/memory: Use ptep_get_lockless_norecency() for orig_pte
From: Ryan Roberts @ 2024-03-26 17:27 UTC (permalink / raw)
To: David Hildenbrand, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <e0bdbd5e-a098-422a-90af-9cf07ce378a4@redhat.com>
On 26/03/2024 17:02, David Hildenbrand wrote:
> On 15.02.24 13:17, Ryan Roberts wrote:
>> Let's convert handle_pte_fault()'s use of ptep_get_lockless() to
>> ptep_get_lockless_norecency() to save orig_pte.
>>
>> There are a number of places that follow this model:
>>
>> orig_pte = ptep_get_lockless(ptep)
>> ...
>> <lock>
>> if (!pte_same(orig_pte, ptep_get(ptep)))
>> // RACE!
>> ...
>> <unlock>
>>
>> So we need to be careful to convert all of those to use
>> pte_same_norecency() so that the access and dirty bits are excluded from
>> the comparison.
>>
>> Additionally there are a couple of places that genuinely rely on the
>> access and dirty bits of orig_pte, but with some careful refactoring, we
>> can use ptep_get() once we are holding the lock to achieve equivalent
>> logic.
>
> We really should document that changed behavior somewhere where it can be easily
> found: that orig_pte might have incomplete/stale accessed/dirty information.
I could add it to the orig_pte definition in the `struct vm_fault`?
>
>
>> @@ -5343,7 +5356,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>> vmf->address, &vmf->ptl);
>> if (unlikely(!vmf->pte))
>> return 0;
>> - vmf->orig_pte = ptep_get_lockless(vmf->pte);
>> + vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte);
>> vmf->flags |= FAULT_FLAG_ORIG_PTE_VALID;
>>
>> if (pte_none(vmf->orig_pte)) {
>> @@ -5363,7 +5376,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>>
>> spin_lock(vmf->ptl);
>> entry = vmf->orig_pte;
>> - if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
>> + if (unlikely(!pte_same_norecency(ptep_get(vmf->pte), entry))) {
>> update_mmu_tlb(vmf->vma, vmf->address, vmf->pte);
>> goto unlock;
>
> I was wondering about the following:
>
> Assume the PTE is not dirty.
>
> Thread 1 does
Sorry not sure what threads have to do with this? How is the vmf shared between
threads? What have I misunderstood...
>
> vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte)
> /* not dirty */
>
> /* Now, thread 2 ends up setting the PTE dirty under PT lock. */
>
> spin_lock(vmf->ptl);
> entry = vmf->orig_pte;
> if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
> ...
> }
> ...
> entry = pte_mkyoung(entry);
Do you mean pte_mkdirty() here? You're talking about dirty everywhere else.
> if (ptep_set_access_flags(vmf->vma, ...)
> ...
> pte_unmap_unlock(vmf->pte, vmf->ptl);
>
>
> Generic ptep_set_access_flags() will do another pte_same() check and realize
> "hey, there was a change!" let's update the PTE!
>
> set_pte_at(vma->vm_mm, address, ptep, entry);
This is called from the generic ptep_set_access_flags() in your example, right?
>
> would overwrite the dirty bit set by thread 2.
I'm not really sure what you are getting at... Is your concern that there is a
race where the page could become dirty in the meantime and it now gets lost? I
think that's why arm64 overrides ptep_set_access_flags(); since the hw can
update access/dirty we have to deal with the races.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: (subset) [PATCH v4 0/7] Add support for the internal RK3308 audio codec
From: Heiko Stuebner @ 2024-03-26 17:25 UTC (permalink / raw)
To: Jaroslav Kysela, Krzysztof Kozlowski, Liam Girdwood,
Luca Ceresoli, Philipp Zabel, Nicolas Frattaroli, Conor Dooley,
Rob Herring, Mark Brown, Takashi Iwai
Cc: Heiko Stuebner, linux-arm-kernel, devicetree, Rob Herring,
linux-rockchip, linux-kernel, linux-sound, alsa-devel,
Thomas Petazzoni
In-Reply-To: <20240305-rk3308-audio-codec-v4-0-312acdbe628f@bootlin.com>
On Tue, 05 Mar 2024 15:36:27 +0100, Luca Ceresoli wrote:
> This series adds a driver for the internal audio codec of the Rockchip
> RK3308 SoC, along with some related patches. This codec is internally
> connected to the I2S peripherals on the same chip, and it has some
> peculiarities arising from that interconnection.
>
> For proper bidirectional operation with the internal codec at any possible
> combination of sampling rates, the I2S peripheral needs two clock sources
> (tx and rx), while connection with an external codec commonly needs only
> one.
>
> [...]
Applied, thanks!
[5/7] arm64: defconfig: enable Rockchip RK3308 internal audio codec driver
commit: 9fdd7b45da18b84d5e7d5a6b8b4b0167910f2d13
[6/7] arm64: dts: rockchip: add i2s_8ch_2 and i2s_8ch_3
commit: b5ffc424360eaced41f405f0e38bcabe61fecf39
[7/7] arm64: dts: rockchip: add the internal audio codec
commit: 30d72458624bb1ba7bab1c7a1d5f4c42f512010c
Best regards,
--
Heiko Stuebner <heiko@sntech.de>
_______________________________________________
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/4] arm64: dts: rockchip: add wolfvision pf5 mainboard
From: Heiko Stuebner @ 2024-03-26 17:25 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Michael Riesch, Conor Dooley
Cc: Heiko Stuebner, linux-kernel, linux-arm-kernel, linux-rockchip,
devicetree
In-Reply-To: <20240325-feature-wolfvision-pf5-v1-0-5725445f792a@wolfvision.net>
On Mon, 25 Mar 2024 15:22:30 +0100, Michael Riesch wrote:
> Habidere,
>
> This series adds the device tree for the WolfVision PF5 mainboard, which
> serves as base for recent WolfVision products. It features the Rockchip
> RK3568 and can be extended with several different extension boards.
>
> The WolfVision PF5 IO Expander is one example of such an extension board.
> The corresponding device tree overlay is also included in this series.
>
> [...]
Applied, thanks!
[1/4] dt-bindings: add wolfvision vendor prefix
commit: 37a6d5864fb8226c97eefa59a79de57571e1fee8
[2/4] dt-bindings: arm: rockchip: add wolfvision pf5 mainboard
commit: 6be2ad17acb76c47e544864467680a7b738d7eb8
[3/4] arm64: dts: rockchip: add wolfvision pf5 mainboard
commit: 0be29f76633a447e15fc58066ea47688974e68d9
[4/4] arm64: dts: rockchip: add wolfvision pf5 io expander board
commit: 28799a7734a0e1a38d01b511ca08d33d8cf91b6c
Best regards,
--
Heiko Stuebner <heiko@sntech.de>
_______________________________________________
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] iommu/arm-smmu-v3: Fix access for STE.SHCFG
From: Will Deacon @ 2024-03-26 17:23 UTC (permalink / raw)
To: robin.murphy, linux-arm-kernel, iommu, Mostafa Saleh
Cc: catalin.marinas, kernel-team, Will Deacon, linux-kernel, joro,
nicolinc, mshavit, Jason Gunthorpe
In-Reply-To: <20240323134658.464743-1-smostafa@google.com>
On Sat, 23 Mar 2024 13:46:58 +0000, Mostafa Saleh wrote:
> STE attributes(NSCFG, PRIVCFG, INSTCFG) use value 0 for "Use Icomming",
> for some reason SHCFG doesn't follow that, and it is defined as "0b01".
>
> Currently the driver sets SHCFG to Use Incoming for stage-2 and bypass
> domains.
>
> However according to the User Manual (ARM IHI 0070 F.b):
> When SMMU_IDR1.ATTR_TYPES_OVR == 0, this field is RES0 and the
> incoming Shareability attribute is used.
>
> [...]
Applied to will (for-joerg/arm-smmu/fixes), thanks!
[1/1] iommu/arm-smmu-v3: Fix access for STE.SHCFG
https://git.kernel.org/will/c/ec9098d6bffe
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
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] iommu/arm-smmu-v3: Add cpu_to_le64() around STRTAB_STE_0_V
From: Will Deacon @ 2024-03-26 17:23 UTC (permalink / raw)
To: iommu, Joerg Roedel, linux-arm-kernel, Jason Gunthorpe
Cc: catalin.marinas, kernel-team, Will Deacon, kernel test robot,
Michael Shavit, patches, Robin Murphy
In-Reply-To: <0-v1-98b23ebb0c84+9f-smmu_cputole_jgg@nvidia.com>
On Mon, 04 Mar 2024 15:50:08 -0400, Jason Gunthorpe wrote:
> STRTAB_STE_0_V is a CPU value, it needs conversion for sparse to be clean.
>
> The missing annotation was a mistake introduced by splitting the ops out
> from the STE writer.
>
>
Applied to will (for-joerg/arm-smmu/fixes), thanks!
[1/1] iommu/arm-smmu-v3: Add cpu_to_le64() around STRTAB_STE_0_V
https://git.kernel.org/will/c/0493e739ccc6
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
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 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Laurent Pinchart @ 2024-03-26 17:18 UTC (permalink / raw)
To: Stefan Wahren
Cc: Krzysztof Kozlowski, devicetree, linux-rpi-kernel,
linux-arm-kernel, linux-gpio, Dave Stevenson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski
In-Reply-To: <45242028-edf7-49fc-80bf-be9eb242b4cd@gmx.net>
On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> opinion about this]
>
> [drop Emma Anholt old address since she is not involved anymore]
>
> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> > On 26/03/2024 01:49, Laurent Pinchart wrote:
> >> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >> and, as a result, also needs to specify #address-cells and #size-cells.
> >> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >> limitations"), but the DT bindings haven't been updated, resulting in
> >> validation errors:
> >>
> >> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >> from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>
> >> Fix this by adding the properties to the bindings.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Children do not perform any IO on their own, because everything is
> > handled by parent. It is really odd to see dma-ranges without ranges.
> > Referenced commits might be also wrong.
Comunication with the firmware goes through a mailbox interface, which
uses DMA transfers. See for instance
rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
{
u32 message = MBOX_MSG(chan, data);
int ret;
WARN_ON(data & 0xf);
mutex_lock(&transaction_lock);
reinit_completion(&fw->c);
ret = mbox_send_message(fw->chan, &message);
if (ret >= 0) {
if (wait_for_completion_timeout(&fw->c, HZ)) {
ret = 0;
} else {
ret = -ETIMEDOUT;
WARN_ONCE(1, "Firmware transaction timeout");
}
} else {
dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
}
mutex_unlock(&transaction_lock);
return ret;
}
int rpi_firmware_property_list(struct rpi_firmware *fw,
void *data, size_t tag_size)
{
size_t size = tag_size + 12;
u32 *buf;
dma_addr_t bus_addr;
int ret;
/* Packets are processed a dword at a time. */
if (size & 3)
return -EINVAL;
buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
GFP_ATOMIC);
if (!buf)
return -ENOMEM;
/* The firmware will error out without parsing in this case. */
WARN_ON(size >= 1024 * 1024);
buf[0] = size;
buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
memcpy(&buf[2], data, tag_size);
buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
wmb();
ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
rmb();
memcpy(data, &buf[2], tag_size);
if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
/*
* The tag name here might not be the one causing the
* error, if there were multiple tags in the request.
* But single-tag is the most common, so go with it.
*/
dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
buf[2], buf[1]);
ret = -EINVAL;
}
dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
return ret;
}
fw->cl.dev is the device for the firmware child node. That may be where
the problem comes from, shouldn't we use the mailbox device for DMA
mapping ?
--
Regards,
Laurent Pinchart
_______________________________________________
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 1/3] remoteproc: Add Arm remoteproc driver
From: Abdellatif El Khlifi @ 2024-03-26 17:14 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Abdellatif El Khlifi, Sudeep Holla, Bjorn Andersson, Rob Herring,
Liviu Dudau, Lorenzo Pieralisi, Krzysztof Kozlowski, Conor Dooley,
Drew.Reed, Adam.Johnston, linux-arm-kernel, devicetree,
linux-kernel, linux-remoteproc
In-Reply-To: <CANLsYkwOrtXxObL5MKf30OrUYB_uT=DnGEXUtfjH503r_LyMQA@mail.gmail.com>
Hi Mathieu,
> > > > > > > > > > This is an initial patchset for allowing to turn on and off the remote processor.
> > > > > > > > > > The FW is already loaded before the Corstone-1000 SoC is powered on and this
> > > > > > > > > > is done through the FPGA board bootloader in case of the FPGA target. Or by the Corstone-1000 FVP model
> > > > > > > > > > (emulator).
> > > > > > > > > >
> > > > > > > > > >From the above I take it that booting with a preloaded firmware is a
> > > > > > > > > scenario that needs to be supported and not just a temporary stage.
> > > > > > > >
> > > > > > > > The current status of the Corstone-1000 SoC requires that there is
> > > > > > > > a preloaded firmware for the external core. Preloading is done externally
> > > > > > > > either through the FPGA bootloader or the emulator (FVP) before powering
> > > > > > > > on the SoC.
> > > > > > > >
> > > > > > >
> > > > > > > Ok
> > > > > > >
> > > > > > > > Corstone-1000 will be upgraded in a way that the A core running Linux is able
> > > > > > > > to share memory with the remote core and also being able to access the remote
> > > > > > > > core memory so Linux can copy the firmware to. This HW changes are still
> > > > > > > > This is why this patchset is relying on a preloaded firmware. And it's the step 1
> > > > > > > > of adding remoteproc support for Corstone.
> > > > > > > >
> > > > > > >
> > > > > > > Ok, so there is a HW problem where A core and M core can't see each other's
> > > > > > > memory, preventing the A core from copying the firmware image to the proper
> > > > > > > location.
> > > > > > >
> > > > > > > When the HW is fixed, will there be a need to support scenarios where the
> > > > > > > firmware image has been preloaded into memory?
> > > > > >
> > > > > > No, this scenario won't apply when we get the HW upgrade. No need for an
> > > > > > external entity anymore. The firmware(s) will all be files in the linux filesystem.
> > > > > >
> > > > >
> > > > > Very well. I am willing to continue with this driver but it does so little that
> > > > > I wonder if it wouldn't simply be better to move forward with upstreaming when
> > > > > the HW is fixed. The choice is yours.
> > > > >
> > > >
> > > > I think Robin has raised few points that need clarification. I think it was
> > > > done as part of DT binding patch. I share those concerns and I wanted to
> > > > reaching to the same concerns by starting the questions I asked on corstone
> > > > device tree changes.
> > > >
> > >
> > > I also agree with Robin's point of view. Proceeding with an initial
> > > driver with minimal functionality doesn't preclude having complete
> > > bindings. But that said and as I pointed out, it might be better to
> > > wait for the HW to be fixed before moving forward.
> >
> > We checked with the HW teams. The missing features will be implemented but
> > this will take time.
> >
> > The foundation driver as it is right now is still valuable for people wanting to
> > know how to power control Corstone external systems in a future proof manner
> > (even in the incomplete state). We prefer to address all the review comments
> > made so it can be merged. This includes making the DT binding as complete as
> > possible as you advised. Then, once the HW is ready, I'll implement the comms
> > and the FW reload part. Is that OK please ?
> >
>
> I'm in agreement with that plan as long as we agree the current
> preloaded heuristic is temporary and is not a valid long term
> scenario.
Yes, that's the plan, no problem.
Cheers,
Abdellatif
_______________________________________________
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 4/4] kprobes: Remove core dependency on modules
From: Jarkko Sakkinen @ 2024-03-26 17:13 UTC (permalink / raw)
To: Mark Rutland, linux-kernel
Cc: agordeev, anil.s.keshavamurthy, aou, bp, catalin.marinas,
dave.hansen, davem, gor, hca, jcalvinowens, linux-arm-kernel,
mhiramat, mingo, mpe, naveen.n.rao, palmer, paul.walmsley, tglx,
will
In-Reply-To: <20240326163624.3253157-5-mark.rutland@arm.com>
On Tue Mar 26, 2024 at 6:36 PM EET, Mark Rutland wrote:
> From: Jarkko Sakkinen <jarkko@kernel.org>
>
> Tracing with kprobes while running a monolithic kernel is currently
> impossible because KPROBES depends on MODULES. While this dependency is
> necessary when KPROBES_USE_MODULE_ALLOC=y, all the other module-specific
> code only exist to handle the case when MODULES=y, and can be hidden
> behind ifdeffery.
>
> Add the necessary ifdeffery, and remove the dependency on MODULES=N when
> KPROBES_USE_MODULE_ALLOC=n.
>
> Currently this allows kprobes to be used when CONFIG_MODULES=n on arm64
> and riscv, and other architectures can enable support by implementing
> their own kprobes_alloc_insn_page() and kprobes_free_insn_page() which
> do not depend on MODULES.
>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> Link: https://lore.kernel.org/all/20240326012102.27438-1-jarkko@kernel.org/
> [Mark: Remove execmem changes, depend on !KPROBES_USE_MODULE_ALLOC]
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> ---
> arch/Kconfig | 2 +-
> kernel/kprobes.c | 12 +++++++++++-
> kernel/trace/trace_kprobe.c | 15 +++++++++++++--
> 3 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 85bb59f7b8c07..cf43de9ffb5b9 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -52,7 +52,7 @@ config GENERIC_ENTRY
>
> config KPROBES
> bool "Kprobes"
> - depends on MODULES
> + depends on MODULES || !KPROBES_USE_MODULE_ALLOC
> depends on HAVE_KPROBES
> select KALLSYMS
> select TASKS_RCU if PREEMPTION
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index fa2ee4e59eca2..7c2f0b504cdcb 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1582,6 +1582,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
> goto out;
> }
>
> +#ifdef CONFIG_MODULES
> /* Check if 'p' is probing a module. */
> *probed_mod = __module_text_address((unsigned long) p->addr);
> if (*probed_mod) {
> @@ -1605,6 +1606,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
> ret = -ENOENT;
> }
> }
> +#endif
This can be scoped a bit more (see v7 of my patch set).
> +
> out:
> preempt_enable();
> jump_label_unlock();
> @@ -2484,6 +2487,7 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
> return 0;
> }
>
> +#ifdef CONFIG_MODULES
> /* Remove all symbols in given area from kprobe blacklist */
> static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
> {
> @@ -2501,6 +2505,7 @@ static void kprobe_remove_ksym_blacklist(unsigned long entry)
> {
> kprobe_remove_area_blacklist(entry, entry + 1);
> }
> +#endif /* CONFIG_MODULES */
>
> int __weak arch_kprobe_get_kallsym(unsigned int *symnum, unsigned long *value,
> char *type, char *sym)
> @@ -2566,6 +2571,7 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
> return ret ? : arch_populate_kprobe_blacklist();
> }
>
> +#ifdef CONFIG_MODULES
> static void add_module_kprobe_blacklist(struct module *mod)
> {
> unsigned long start, end;
> @@ -2662,6 +2668,9 @@ static int kprobes_module_callback(struct notifier_block *nb,
> mutex_unlock(&kprobe_mutex);
> return NOTIFY_DONE;
> }
> +#else
> +#define kprobes_module_callback (NULL)
> +#endif /* CONFIG_MODULES */
>
> static struct notifier_block kprobe_module_nb = {
> .notifier_call = kprobes_module_callback,
> @@ -2726,7 +2735,8 @@ static int __init init_kprobes(void)
> err = arch_init_kprobes();
> if (!err)
> err = register_die_notifier(&kprobe_exceptions_nb);
> - if (!err)
> +
> + if (!err && IS_ENABLED(CONFIG_MODULES))
> err = register_module_notifier(&kprobe_module_nb);
>
> kprobes_initialized = (err == 0);
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 14099cc17fc9e..c509ba776e679 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -111,6 +111,7 @@ static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
> return strncmp(module_name(mod), name, len) == 0 && name[len] == ':';
> }
>
> +#ifdef CONFIG_MODULES
> static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
> {
> char *p;
> @@ -129,6 +130,9 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
>
> return ret;
> }
> +#else
> +#define trace_kprobe_module_exist(tk) false /* aka a module never exists */
> +#endif /* CONFIG_MODULES */
>
> static bool trace_kprobe_is_busy(struct dyn_event *ev)
> {
> @@ -670,6 +674,7 @@ static int register_trace_kprobe(struct trace_kprobe *tk)
> return ret;
> }
>
> +#ifdef CONFIG_MODULES
> /* Module notifier call back, checking event on the module */
> static int trace_kprobe_module_callback(struct notifier_block *nb,
> unsigned long val, void *data)
> @@ -699,6 +704,9 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
>
> return NOTIFY_DONE;
> }
> +#else
> +#define trace_kprobe_module_callback (NULL)
> +#endif /* CONFIG_MODULES */
The last two CONFIG_MODULES sections could be combined. This was also in
v7.
>
> static struct notifier_block trace_kprobe_module_nb = {
> .notifier_call = trace_kprobe_module_callback,
> @@ -1933,8 +1941,11 @@ static __init int init_kprobe_trace_early(void)
> if (ret)
> return ret;
>
> - if (register_module_notifier(&trace_kprobe_module_nb))
> - return -EINVAL;
> + if (IS_ENABLED(CONFIG_MODULES)) {
> + ret = register_module_notifier(&trace_kprobe_module_nb);
> + if (ret)
> + return -EINVAL;
> + }
>
> return 0;
> }
Other than lgtm.
BR, Jarkko
_______________________________________________
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 1/5] dt-bindings: iio: dac: ti,dac5571: Add DAC081C081 support
From: Jonathan Cameron @ 2024-03-26 17:13 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Jonathan Cameron, devicetree, imx, linux-arm-kernel,
Trevor Zaharichuk, Greg Lytle, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sean Nyekjaer, linux-iio
In-Reply-To: <20240326165043.GB28895@pendragon.ideasonboard.com>
On Tue, 26 Mar 2024 18:50:43 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> On Tue, Mar 26, 2024 at 03:29:27PM +0000, Jonathan Cameron wrote:
> > On Mon, 25 Mar 2024 22:56:41 +0200 Laurent Pinchart wrote:
> > > On Mon, Mar 25, 2024 at 08:48:57PM +0000, Jonathan Cameron wrote:
> > > > On Mon, 25 Mar 2024 22:32:41 +0200 Laurent Pinchart wrote:
> > > >
> > > > > The DAC081C081 is a TI DAC whose software interface is compatible with
> > > > > the DAC5571. It is the 8-bit version of the DAC121C081, already
> > > > > supported by the DAC5571 bindings. Extends the bindings to support this
> > > > > chip.
> > > > >
> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > Hi Laurent,
> > > >
> > > > Given it's a part number where no one is going to guess it is compatible
> > > > with the DAC5571 and that we don't have a history of fallback compatibles
> > > > I'm fine with this change, but just wanted to ask is a fallback compatible
> > > > useful to you to run with older kernels?
> > > >
> > > > I should have noticed when Peter added the dac121c081. If we add a fallback
> > > > should do that one as well.
> > >
> > > I've indeed noticed that there should have been a fallback for
> > > dac121c081, but didn't stop to ponder why that wasn't the case, and just
> > > went along with the flow :-) I agree a fallback could be useful, which
> > > would then allow dropping patch 2/5 from this series (*). I can do so if
> > > you prefer.
> > >
> > > * This is not entirely true. While the DAC1081C081 is largely compatible
> > > with the DAC5573, they have different values for one of the power-down
> > > resistors (2.5kΩ instead of 1kΩ if I recall correctly). To be completely
> > > accurate, the driver should report that. We could still use the fallback
> > > compatible, reporting the wrong power-down resistor value.
> >
> > Hmm - Would anyone really care about that value being wrong?
>
> I don't have enough expertise with IIO to be sure, but my guess is that
> nobody would.
>
> > I think perhaps that's just about significant enough that maybe a fallback
> > compatible doesn't make sense here.
>
> Then let's keep it simple and just merge this patch as-is ? :-)
Makes sense. I'm on wrong computer at the moment but will pick it up at
weekend if not before.
Jonathan
>
> > > > > ---
> > > > > Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > index 79da0323c327..e59db861e2eb 100644
> > > > > --- a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > @@ -21,6 +21,7 @@ properties:
> > > > > - ti,dac5573
> > > > > - ti,dac6573
> > > > > - ti,dac7573
> > > > > + - ti,dac081c081
> > > > > - ti,dac121c081
> > > > >
> > > > > reg:
>
_______________________________________________
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] kprobes/treewide: Add kprobes_ prefix to insn alloc/free functions
From: Jarkko Sakkinen @ 2024-03-26 17:11 UTC (permalink / raw)
To: Mark Rutland, linux-kernel
Cc: agordeev, anil.s.keshavamurthy, aou, bp, catalin.marinas,
dave.hansen, davem, gor, hca, jcalvinowens, linux-arm-kernel,
mhiramat, mingo, mpe, naveen.n.rao, palmer, paul.walmsley, tglx,
will
In-Reply-To: <20240326163624.3253157-3-mark.rutland@arm.com>
On Tue Mar 26, 2024 at 6:36 PM EET, Mark Rutland wrote:
> The alloc_(opt)insn_page() and free_(opt)insn_page() functions are
> specific to KPROBES, but their name makes them sound more generic than
> they are.
>
> Given them a 'kprobes_' prefix to make it clear that they're part of
> kprobes.
>
> This was generated automatically with:
>
> sed -i 's/alloc_insn_page/kprobes_alloc_insn_page/' $(git grep -l 'alloc_insn_page')
> sed -i 's/free_insn_page/kprobes_free_insn_page/' $(git grep -l 'free_insn_page')
> sed -i 's/alloc_optinsn_page/kprobes_alloc_optinsn_page/' $(git grep -l 'alloc_optinsn_page')
> sed -i 's/free_optinsn_page/kprobes_free_optinsn_page/' $(git grep -l 'free_optinsn_page')
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> ---
> arch/arm64/kernel/probes/kprobes.c | 2 +-
> arch/powerpc/kernel/kprobes.c | 2 +-
> arch/powerpc/kernel/optprobes.c | 4 ++--
> arch/riscv/kernel/probes/kprobes.c | 2 +-
> arch/s390/kernel/kprobes.c | 2 +-
> arch/x86/kernel/kprobes/core.c | 2 +-
> include/linux/kprobes.h | 6 +++---
> kernel/kprobes.c | 20 ++++++++++----------
> 8 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 327855a11df2f..4b6ab7b1fa211 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -129,7 +129,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
> return 0;
> }
>
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
> {
> return __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END,
> GFP_KERNEL, PAGE_KERNEL_ROX, VM_FLUSH_RESET_PERMS,
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index bbca90a5e2ec0..0b297718d5de6 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -126,7 +126,7 @@ kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offse
> return (kprobe_opcode_t *)(addr + offset);
> }
>
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
> {
> void *page;
>
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index 004fae2044a3e..0ddbda217073f 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -27,7 +27,7 @@
>
> static bool insn_page_in_use;
>
> -void *alloc_optinsn_page(void)
> +void *kprobes_alloc_optinsn_page(void)
> {
> if (insn_page_in_use)
> return NULL;
> @@ -35,7 +35,7 @@ void *alloc_optinsn_page(void)
> return &optinsn_slot;
> }
>
> -void free_optinsn_page(void *page)
> +void kprobes_free_optinsn_page(void *page)
> {
> insn_page_in_use = false;
> }
> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
> index 2f08c14a933d0..75201ce721057 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -105,7 +105,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
> }
>
> #ifdef CONFIG_MMU
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
> {
> return __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END,
> GFP_KERNEL, PAGE_KERNEL_READ_EXEC,
> diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
> index f0cf20d4b3c58..91ca4d501d4ef 100644
> --- a/arch/s390/kernel/kprobes.c
> +++ b/arch/s390/kernel/kprobes.c
> @@ -34,7 +34,7 @@ struct kretprobe_blackpoint kretprobe_blacklist[] = { };
>
> static int insn_page_in_use;
>
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
> {
> void *page;
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index d0e49bd7c6f3f..7f01bbbfa9e2a 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -491,7 +491,7 @@ static int prepare_singlestep(kprobe_opcode_t *buf, struct kprobe *p,
> }
>
> /* Make page to RO mode when allocate it */
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
> {
> void *page;
>
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 0ff44d6633e33..ad4b561100f9e 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -430,10 +430,10 @@ int enable_kprobe(struct kprobe *kp);
>
> void dump_kprobe(struct kprobe *kp);
>
> -void *alloc_insn_page(void);
> +void *kprobes_alloc_insn_page(void);
>
> -void *alloc_optinsn_page(void);
> -void free_optinsn_page(void *page);
> +void *kprobes_alloc_optinsn_page(void);
> +void kprobes_free_optinsn_page(void *page);
>
> int kprobe_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
> char *sym);
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 9d9095e817928..35adf56430c9b 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -110,7 +110,7 @@ enum kprobe_slot_state {
> SLOT_USED = 2,
> };
>
> -void __weak *alloc_insn_page(void)
> +void __weak *kprobes_alloc_insn_page(void)
> {
> /*
> * Use module_alloc() so this page is within +/- 2GB of where the
> @@ -121,15 +121,15 @@ void __weak *alloc_insn_page(void)
> return module_alloc(PAGE_SIZE);
> }
>
> -static void free_insn_page(void *page)
> +static void kprobes_free_insn_page(void *page)
> {
> module_memfree(page);
> }
>
> struct kprobe_insn_cache kprobe_insn_slots = {
> .mutex = __MUTEX_INITIALIZER(kprobe_insn_slots.mutex),
> - .alloc = alloc_insn_page,
> - .free = free_insn_page,
> + .alloc = kprobes_alloc_insn_page,
> + .free = kprobes_free_insn_page,
> .sym = KPROBE_INSN_PAGE_SYM,
> .pages = LIST_HEAD_INIT(kprobe_insn_slots.pages),
> .insn_size = MAX_INSN_SIZE,
> @@ -333,21 +333,21 @@ int kprobe_cache_get_kallsym(struct kprobe_insn_cache *c, unsigned int *symnum,
> }
>
> #ifdef CONFIG_OPTPROBES
> -void __weak *alloc_optinsn_page(void)
> +void __weak *kprobes_alloc_optinsn_page(void)
> {
> - return alloc_insn_page();
> + return kprobes_alloc_insn_page();
> }
>
> -void __weak free_optinsn_page(void *page)
> +void __weak kprobes_free_optinsn_page(void *page)
> {
> - free_insn_page(page);
> + kprobes_free_insn_page(page);
> }
>
> /* For optimized_kprobe buffer */
> struct kprobe_insn_cache kprobe_optinsn_slots = {
> .mutex = __MUTEX_INITIALIZER(kprobe_optinsn_slots.mutex),
> - .alloc = alloc_optinsn_page,
> - .free = free_optinsn_page,
> + .alloc = kprobes_alloc_optinsn_page,
> + .free = kprobes_free_optinsn_page,
> .sym = KPROBE_OPTINSN_PAGE_SYM,
> .pages = LIST_HEAD_INIT(kprobe_optinsn_slots.pages),
> /* .insn_size is initialized later */
OK, great, I'll test this.
BR, Jarkko
_______________________________________________
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 v6 04/16] dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on QCA6390
From: Johan Hovold @ 2024-03-26 17:05 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Kalle Valo, Marcel Holtmann, Luiz Augusto von Dentz,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Liam Girdwood, Mark Brown, Catalin Marinas,
Will Deacon, Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven,
Arnd Bergmann, Neil Armstrong, Marek Szyprowski, Alex Elder,
Srini Kandagatla, Greg Kroah-Hartman, Abel Vesa,
Manivannan Sadhasivam, Lukas Wunner, Dmitry Baryshkov,
linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
Bartosz Golaszewski, ath11k
In-Reply-To: <CAMRc=Me5ef_kFDz0SyGZb4S+2Ma4i=Fek_tzwj+bYD4DGSV4mA@mail.gmail.com>
On Tue, Mar 26, 2024 at 05:32:55PM +0100, Bartosz Golaszewski wrote:
> On Tue, Mar 26, 2024 at 4:12 PM Kalle Valo <kvalo@kernel.org> wrote:
> > >> Adding also Johan and ath11k list. For example, I don't know what's the
> > >> plan with Lenovo X13s, will it use this framework? I guess in theory we
> > >> could have devices which use qcom,ath11k-calibration-variant from DT but
> > >> not any of these supply properties?
> > >
> > > Good point. I will receive the X13s in a month from now. I do plan on
> > > upstreaming correct support for WLAN and BT for it as well.
> > >
> > > I guess we can always relax the requirements once a valid use-case appears?
> >
> > I think we have such cases already now:
> >
> > $ git grep ath11k-calibration-variant -- arch
> > arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts: qcom,ath11k-calibration-variant = "Fairphone_5";
> > arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts: qcom,ath11k-calibration-variant = "LE_X13S";
> >
> > But please do check that. I'm no DT expert :)
>
> You're thinking about making the required: field depend on the value
> of qcom,ath11k-calibration-variant? Am I getting this right?
No, I think Kalle is worried about requiring the supply properties for
certain PCI device ids, in case we have existing or future devicetrees
with those ids that did not specify them or that need not specify them
(e.g. any PC modules).
Currently we only have the X13s controller in mainline being described
by a PCIe endpoint node in DT, but it has a different id ("pci17cb,1103"
instead of "pci17cb,1101" which you are adding here).
The Fairphone controller is apparently not a PCI device at all.
Johan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 0/4] Reduce cost of ptep_get_lockless on arm64
From: David Hildenbrand @ 2024-03-26 17:04 UTC (permalink / raw)
To: Ryan Roberts, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <374d8500-4625-4bff-a934-77b5f34cf2ec@arm.com>
>>>>
>>>> Likely, we just want to read "the real deal" on both sides of the pte_same()
>>>> handling.
>>>
>>> Sorry I'm not sure I understand? You mean read the full pte including
>>> access/dirty? That's the same as dropping the patch, right? Of course if we do
>>> that, we still have to keep pte_get_lockless() around for this case. In an ideal
>>> world we would convert everything over to ptep_get_lockless_norecency() and
>>> delete ptep_get_lockless() to remove the ugliness from arm64.
>>
>> Yes, agreed. Patch #3 does not look too crazy and it wouldn't really affect any
>> architecture.
>>
>> I do wonder if pte_same_norecency() should be defined per architecture and the
>> default would be pte_same(). So we could avoid the mkold etc on all other
>> architectures.
>
> Wouldn't that break it's semantics? The "norecency" of
> ptep_get_lockless_norecency() means "recency information in the returned pte may
> be incorrect". But the "norecency" of pte_same_norecency() means "ignore the
> access and dirty bits when you do the comparison".
My idea was that ptep_get_lockless_norecency() would return the actual
result on these architectures. So e.g., on x86, there would be no actual
change in generated code.
But yes, the documentation of these functions would have to be improved.
Now I wonder if ptep_get_lockless_norecency() should actively clear
dirty/accessed bits to more easily find any actual issues where the bits
still matter ...
>
> I think you could only do the optimization you describe if you required that
> pte_same_norecency() would only be given values returned by
> ptep_get_lockless_norecency() (or ptep_get_norecency()). Even then, its not
> quite the same; if a page is accessed between gets one will return true and the
> other false.
Right.
--
Cheers,
David / dhildenb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 3/4] mm/memory: Use ptep_get_lockless_norecency() for orig_pte
From: David Hildenbrand @ 2024-03-26 17:02 UTC (permalink / raw)
To: Ryan Roberts, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <20240215121756.2734131-4-ryan.roberts@arm.com>
On 15.02.24 13:17, Ryan Roberts wrote:
> Let's convert handle_pte_fault()'s use of ptep_get_lockless() to
> ptep_get_lockless_norecency() to save orig_pte.
>
> There are a number of places that follow this model:
>
> orig_pte = ptep_get_lockless(ptep)
> ...
> <lock>
> if (!pte_same(orig_pte, ptep_get(ptep)))
> // RACE!
> ...
> <unlock>
>
> So we need to be careful to convert all of those to use
> pte_same_norecency() so that the access and dirty bits are excluded from
> the comparison.
>
> Additionally there are a couple of places that genuinely rely on the
> access and dirty bits of orig_pte, but with some careful refactoring, we
> can use ptep_get() once we are holding the lock to achieve equivalent
> logic.
We really should document that changed behavior somewhere where it can
be easily found: that orig_pte might have incomplete/stale
accessed/dirty information.
> @@ -5343,7 +5356,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
> vmf->address, &vmf->ptl);
> if (unlikely(!vmf->pte))
> return 0;
> - vmf->orig_pte = ptep_get_lockless(vmf->pte);
> + vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte);
> vmf->flags |= FAULT_FLAG_ORIG_PTE_VALID;
>
> if (pte_none(vmf->orig_pte)) {
> @@ -5363,7 +5376,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>
> spin_lock(vmf->ptl);
> entry = vmf->orig_pte;
> - if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
> + if (unlikely(!pte_same_norecency(ptep_get(vmf->pte), entry))) {
> update_mmu_tlb(vmf->vma, vmf->address, vmf->pte);
> goto unlock;
I was wondering about the following:
Assume the PTE is not dirty.
Thread 1 does
vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte)
/* not dirty */
/* Now, thread 2 ends up setting the PTE dirty under PT lock. */
spin_lock(vmf->ptl);
entry = vmf->orig_pte;
if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
...
}
...
entry = pte_mkyoung(entry);
if (ptep_set_access_flags(vmf->vma, ...)
...
pte_unmap_unlock(vmf->pte, vmf->ptl);
Generic ptep_set_access_flags() will do another pte_same() check and
realize "hey, there was a change!" let's update the PTE!
set_pte_at(vma->vm_mm, address, ptep, entry);
would overwrite the dirty bit set by thread 2.
--
Cheers,
David / dhildenb
_______________________________________________
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] ARM: dts: exynos4212-tab3: limit usable memory range
From: Krzysztof Kozlowski @ 2024-03-26 16:54 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
Artur Weber
Cc: devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel,
~postmarketos/upstreaming
In-Reply-To: <20240217-tab3-limit-usable-memory-range-v1-1-49cc9c86a5cc@gmail.com>
On Sat, 17 Feb 2024 20:02:47 +0100, Artur Weber wrote:
> The stock bootloader on the Samsung Galaxy Tab 3 8.0 provides an
> incorrect available memory range over ATAG_MEM. Limit the usable
> memory in the DTS to prevent it from doing so, without having to
> disable ATAG support.
>
>
Applied, thanks!
[1/1] ARM: dts: exynos4212-tab3: limit usable memory range
https://git.kernel.org/krzk/linux/c/7bff1d35c1294c011b0269b8eaeb8f930df386fe
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 0/4] Reduce cost of ptep_get_lockless on arm64
From: Ryan Roberts @ 2024-03-26 16:53 UTC (permalink / raw)
To: David Hildenbrand, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <de143212-49ce-4c30-8bfa-4c0ff613f107@redhat.com>
On 26/03/2024 16:34, David Hildenbrand wrote:
> On 26.03.24 17:31, Ryan Roberts wrote:
>> On 26/03/2024 16:17, David Hildenbrand wrote:
>>> On 15.02.24 13:17, Ryan Roberts wrote:
>>>> This is an RFC for a series that aims to reduce the cost and complexity of
>>>> ptep_get_lockless() for arm64 when supporting transparent contpte mappings [1].
>>>> The approach came from discussion with Mark and David [2].
>>>>
>>>> It introduces a new helper, ptep_get_lockless_norecency(), which allows the
>>>> access and dirty bits in the returned pte to be incorrect. This relaxation
>>>> permits arm64's implementation to just read the single target pte, and avoids
>>>> having to iterate over the full contpte block to gather the access and dirty
>>>> bits, for the contpte case.
>>>>
>>>> It turns out that none of the call sites using ptep_get_lockless() require
>>>> accurate access and dirty bit information, so we can also convert those sites.
>>>> Although a couple of places need care (see patches 2 and 3).
>>>>
>>>> Arguably patch 3 is a bit fragile, given the wide accessibility of
>>>> vmf->orig_pte. So it might make sense to drop this patch and stick to using
>>>> ptep_get_lockless() in the page fault path. I'm keen to hear opinions.
>>>
>>> Yes. Especially as we have these pte_same() checks that might just fail now
>>> because of wrong accessed/dirty bits?
>>
>> Which pte_same() checks are you referring to? I've changed them all to
>> pte_same_norecency() which ignores the access/dirty bits when doing the
>> comparison.
>
> I'm reading the patches just now. So I stumbled over that just after I wrote
> that, so I was missing that part from the description here.
>
>>
>>>
>>> Likely, we just want to read "the real deal" on both sides of the pte_same()
>>> handling.
>>
>> Sorry I'm not sure I understand? You mean read the full pte including
>> access/dirty? That's the same as dropping the patch, right? Of course if we do
>> that, we still have to keep pte_get_lockless() around for this case. In an ideal
>> world we would convert everything over to ptep_get_lockless_norecency() and
>> delete ptep_get_lockless() to remove the ugliness from arm64.
>
> Yes, agreed. Patch #3 does not look too crazy and it wouldn't really affect any
> architecture.
>
> I do wonder if pte_same_norecency() should be defined per architecture and the
> default would be pte_same(). So we could avoid the mkold etc on all other
> architectures.
Wouldn't that break it's semantics? The "norecency" of
ptep_get_lockless_norecency() means "recency information in the returned pte may
be incorrect". But the "norecency" of pte_same_norecency() means "ignore the
access and dirty bits when you do the comparison".
I think you could only do the optimization you describe if you required that
pte_same_norecency() would only be given values returned by
ptep_get_lockless_norecency() (or ptep_get_norecency()). Even then, its not
quite the same; if a page is accessed between gets one will return true and the
other false.
_______________________________________________
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 1/5] dt-bindings: iio: dac: ti,dac5571: Add DAC081C081 support
From: Laurent Pinchart @ 2024-03-26 16:50 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jonathan Cameron, devicetree, imx, linux-arm-kernel,
Trevor Zaharichuk, Greg Lytle, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sean Nyekjaer, linux-iio
In-Reply-To: <20240326152927.00006229@Huawei.com>
On Tue, Mar 26, 2024 at 03:29:27PM +0000, Jonathan Cameron wrote:
> On Mon, 25 Mar 2024 22:56:41 +0200 Laurent Pinchart wrote:
> > On Mon, Mar 25, 2024 at 08:48:57PM +0000, Jonathan Cameron wrote:
> > > On Mon, 25 Mar 2024 22:32:41 +0200 Laurent Pinchart wrote:
> > >
> > > > The DAC081C081 is a TI DAC whose software interface is compatible with
> > > > the DAC5571. It is the 8-bit version of the DAC121C081, already
> > > > supported by the DAC5571 bindings. Extends the bindings to support this
> > > > chip.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > Hi Laurent,
> > >
> > > Given it's a part number where no one is going to guess it is compatible
> > > with the DAC5571 and that we don't have a history of fallback compatibles
> > > I'm fine with this change, but just wanted to ask is a fallback compatible
> > > useful to you to run with older kernels?
> > >
> > > I should have noticed when Peter added the dac121c081. If we add a fallback
> > > should do that one as well.
> >
> > I've indeed noticed that there should have been a fallback for
> > dac121c081, but didn't stop to ponder why that wasn't the case, and just
> > went along with the flow :-) I agree a fallback could be useful, which
> > would then allow dropping patch 2/5 from this series (*). I can do so if
> > you prefer.
> >
> > * This is not entirely true. While the DAC1081C081 is largely compatible
> > with the DAC5573, they have different values for one of the power-down
> > resistors (2.5kΩ instead of 1kΩ if I recall correctly). To be completely
> > accurate, the driver should report that. We could still use the fallback
> > compatible, reporting the wrong power-down resistor value.
>
> Hmm - Would anyone really care about that value being wrong?
I don't have enough expertise with IIO to be sure, but my guess is that
nobody would.
> I think perhaps that's just about significant enough that maybe a fallback
> compatible doesn't make sense here.
Then let's keep it simple and just merge this patch as-is ? :-)
> > > > ---
> > > > Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > index 79da0323c327..e59db861e2eb 100644
> > > > --- a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > @@ -21,6 +21,7 @@ properties:
> > > > - ti,dac5573
> > > > - ti,dac6573
> > > > - ti,dac7573
> > > > + - ti,dac081c081
> > > > - ti,dac121c081
> > > >
> > > > reg:
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 2/4] mm/gup: Use ptep_get_lockless_norecency()
From: Ryan Roberts @ 2024-03-26 16:48 UTC (permalink / raw)
To: David Hildenbrand, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <5d80c368-7ce7-4a44-9cd7-aee3e1c9182b@redhat.com>
On 26/03/2024 16:30, David Hildenbrand wrote:
> On 15.02.24 13:17, Ryan Roberts wrote:
>> Gup needs to read ptes locklessly, so it uses ptep_get_lockless().
>> However, the returned access and dirty bits are unimportant so let's
>> switch over to ptep_get_lockless_norecency().
>>
>> The wrinkle is that gup needs to check that the pte hasn't changed once
>> it has pinned the folio following this model:
>>
>> pte = ptep_get_lockless_norecency(ptep)
>> ...
>> if (!pte_same(pte, ptep_get_lockless(ptep)))
>> // RACE!
>> ...
>>
>> And now that pte may not contain correct access and dirty information,
>> the pte_same() comparison could spuriously fail. So let's introduce a
>> new pte_same_norecency() helper which will ignore the access and dirty
>> bits when doing the comparison.
>>
>> Note that previously, ptep_get() was being used for the comparison; this
>> is technically incorrect because the PTL is not held. I've also
>> converted the comparison to use the preferred pmd_same() helper instead
>> of doing a raw value comparison.
>>
>> As a side-effect, this new approach removes the possibility of
>> concurrent read/write to the page causing a spurious fast gup failure,
>> because the access and dirty bits are no longer used in the comparison.
>>
>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
>> ---
>
> [...]
>
>> #ifndef __HAVE_ARCH_PTE_UNUSED
>> /*
>> * Some architectures provide facilities to virtualization guests
>> diff --git a/mm/gup.c b/mm/gup.c
>> index df83182ec72d..0f96d0a5ec09 100644
>> --- a/mm/gup.c
>> +++ b/mm/gup.c
>> @@ -2576,7 +2576,7 @@ static int gup_pte_range(pmd_t pmd, pmd_t *pmdp,
>> unsigned long addr,
>> if (!ptep)
>> return 0;
>> do {
>> - pte_t pte = ptep_get_lockless(ptep);
>> + pte_t pte = ptep_get_lockless_norecency(ptep);
>> struct page *page;
>> struct folio *folio;
>>
>> @@ -2617,8 +2617,9 @@ static int gup_pte_range(pmd_t pmd, pmd_t *pmdp,
>> unsigned long addr,
>> goto pte_unmap;
>> }
>>
>> - if (unlikely(pmd_val(pmd) != pmd_val(*pmdp)) ||
>> - unlikely(pte_val(pte) != pte_val(ptep_get(ptep)))) {
>> + if (unlikely(!pmd_same(pmd, *pmdp)) ||
>> + unlikely(!pte_same_norecency(pte,
>> + ptep_get_lockless_norecency(ptep)))) {
>> gup_put_folio(folio, 1, flags);
>> goto pte_unmap;
>
> We pass the pte into pte_access_permitted(). It would be good to mention that
> you checked all implementations.
TBH, I hadn't; I decided that since the "inaccurate access/dirty bits" was only
possible on arm64, then only arm64's implementation needed checking. But given
your comment, I just had a quick look at all impls. I didn't spot any problems
where any impl needs the access/dirty bits. I'll add this to the commit log.
>
> Acked-by: David Hildenbrand <david@redhat.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 v11 27/28] drm/rockchip: inno_hdmi: Switch to HDMI connector
From: Maxime Ripard @ 2024-03-26 15:40 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: <20240326-kms-hdmi-connector-state-v11-0-c5680ffcf261@kernel.org>
The new HDMI connector infrastructure allows to remove some boilerplate,
especially to generate infoframes. Let's switch to it.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/rockchip/Kconfig | 1 +
drivers/gpu/drm/rockchip/inno_hdmi.c | 143 +++++++++++++----------------------
2 files changed, 53 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 04300e5c6d14..82e04cebd108 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -72,10 +72,11 @@ config ROCKCHIP_DW_MIPI_DSI
enable MIPI DSI on RK3288 or RK3399 based SoC, you should
select this option.
config ROCKCHIP_INNO_HDMI
bool "Rockchip specific extensions for Innosilicon HDMI"
+ depends on DRM_HDMI_STATE_HELPER
help
This selects support for Rockchip SoC specific extensions
for the Innosilicon HDMI driver. If you want to enable
HDMI on RK3036 based SoC, you should select this option.
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 1d2261643743..d59947679042 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -65,13 +65,11 @@ struct inno_hdmi {
const struct inno_hdmi_variant *variant;
};
struct inno_hdmi_connector_state {
struct drm_connector_state base;
- unsigned int enc_out_format;
unsigned int colorimetry;
- bool rgb_limited_range;
};
static struct inno_hdmi *encoder_to_inno_hdmi(struct drm_encoder *encoder)
{
struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
@@ -255,90 +253,53 @@ static void inno_hdmi_reset(struct inno_hdmi *hdmi)
hdmi_modb(hdmi, HDMI_SYS_CTRL, msk, val);
inno_hdmi_standby(hdmi);
}
-static void inno_hdmi_disable_frame(struct inno_hdmi *hdmi,
- enum hdmi_infoframe_type type)
+static int inno_hdmi_disable_frame(struct drm_connector *connector,
+ enum hdmi_infoframe_type type)
{
- struct drm_connector *connector = &hdmi->connector;
-
- if (type != HDMI_INFOFRAME_TYPE_AVI) {
- drm_err(connector->dev,
- "Unsupported infoframe type: %u\n", type);
- return;
- }
-
- hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_BUF_INDEX, INFOFRAME_AVI);
-}
-
-static int inno_hdmi_upload_frame(struct inno_hdmi *hdmi,
- union hdmi_infoframe *frame, enum hdmi_infoframe_type type)
-{
- struct drm_connector *connector = &hdmi->connector;
- u8 packed_frame[HDMI_MAXIMUM_INFO_FRAME_SIZE];
- ssize_t rc, i;
+ struct inno_hdmi *hdmi = connector_to_inno_hdmi(connector);
if (type != HDMI_INFOFRAME_TYPE_AVI) {
drm_err(connector->dev,
"Unsupported infoframe type: %u\n", type);
return 0;
}
- inno_hdmi_disable_frame(hdmi, type);
+ hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_BUF_INDEX, INFOFRAME_AVI);
- rc = hdmi_infoframe_pack(frame, packed_frame,
- sizeof(packed_frame));
- if (rc < 0)
- return rc;
+ return 0;
+}
- for (i = 0; i < rc; i++)
+static int inno_hdmi_upload_frame(struct drm_connector *connector,
+ enum hdmi_infoframe_type type,
+ const u8 *buffer, size_t len)
+{
+ struct inno_hdmi *hdmi = connector_to_inno_hdmi(connector);
+ u8 packed_frame[HDMI_MAXIMUM_INFO_FRAME_SIZE];
+ ssize_t i;
+
+ if (type != HDMI_INFOFRAME_TYPE_AVI) {
+ drm_err(connector->dev,
+ "Unsupported infoframe type: %u\n", type);
+ return 0;
+ }
+
+ inno_hdmi_disable_frame(connector, type);
+
+ for (i = 0; i < len; i++)
hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_ADDR + i,
packed_frame[i]);
return 0;
}
-static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
- struct drm_display_mode *mode)
-{
- struct drm_connector *connector = &hdmi->connector;
- struct drm_connector_state *conn_state = connector->state;
- struct inno_hdmi_connector_state *inno_conn_state =
- to_inno_hdmi_conn_state(conn_state);
- union hdmi_infoframe frame;
- int rc;
-
- rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
- &hdmi->connector,
- mode);
- if (rc) {
- inno_hdmi_disable_frame(hdmi, HDMI_INFOFRAME_TYPE_AVI);
- return rc;
- }
-
- if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV444)
- frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
- else if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV422)
- frame.avi.colorspace = HDMI_COLORSPACE_YUV422;
- else
- frame.avi.colorspace = HDMI_COLORSPACE_RGB;
-
- if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_RGB) {
- drm_hdmi_avi_infoframe_quant_range(&frame.avi,
- connector, mode,
- inno_conn_state->rgb_limited_range ?
- HDMI_QUANTIZATION_RANGE_LIMITED :
- HDMI_QUANTIZATION_RANGE_FULL);
- } else {
- frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
- frame.avi.ycc_quantization_range =
- HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
- }
-
- return inno_hdmi_upload_frame(hdmi, &frame, HDMI_INFOFRAME_TYPE_AVI);
-}
+static const struct drm_connector_hdmi_funcs inno_hdmi_hdmi_connector_funcs = {
+ .clear_infoframe = inno_hdmi_disable_frame,
+ .write_infoframe = inno_hdmi_upload_frame,
+};
static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
{
struct drm_connector *connector = &hdmi->connector;
struct drm_connector_state *conn_state = connector->state;
@@ -359,12 +320,12 @@ static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
value = v_VIDEO_INPUT_BITS(VIDEO_INPUT_8BITS) |
v_VIDEO_OUTPUT_COLOR(0) |
v_VIDEO_INPUT_CSP(0);
hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL2, value);
- if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_RGB) {
- if (inno_conn_state->rgb_limited_range) {
+ if (conn_state->hdmi.output_format == HDMI_COLORSPACE_RGB) {
+ if (!conn_state->hdmi.is_full_range) {
csc_mode = CSC_RGB_0_255_TO_RGB_16_235_8BIT;
auto_csc = AUTO_CSC_DISABLE;
c0_c2_change = C0_C2_CHANGE_DISABLE;
csc_enable = v_CSC_ENABLE;
@@ -378,18 +339,18 @@ static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
return 0;
}
} else {
if (inno_conn_state->colorimetry == HDMI_COLORIMETRY_ITU_601) {
- if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV444) {
+ if (conn_state->hdmi.output_format == HDMI_COLORSPACE_YUV444) {
csc_mode = CSC_RGB_0_255_TO_ITU601_16_235_8BIT;
auto_csc = AUTO_CSC_DISABLE;
c0_c2_change = C0_C2_CHANGE_DISABLE;
csc_enable = v_CSC_ENABLE;
}
} else {
- if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV444) {
+ if (conn_state->hdmi.output_format == HDMI_COLORSPACE_YUV444) {
csc_mode = CSC_RGB_0_255_TO_ITU709_16_235_8BIT;
auto_csc = AUTO_CSC_DISABLE;
c0_c2_change = C0_C2_CHANGE_DISABLE;
csc_enable = v_CSC_ENABLE;
}
@@ -460,14 +421,16 @@ static int inno_hdmi_config_video_timing(struct inno_hdmi *hdmi,
return 0;
}
static int inno_hdmi_setup(struct inno_hdmi *hdmi,
- struct drm_display_mode *mode)
+ struct drm_crtc_state *new_crtc_state,
+ struct drm_connector_state *new_conn_state)
{
- struct drm_display_info *display = &hdmi->connector.display_info;
- unsigned long mpixelclock = mode->clock * 1000;
+ struct drm_connector *connector = &hdmi->connector;
+ struct drm_display_info *display = &connector->display_info;
+ struct drm_display_mode *mode = &new_crtc_state->adjusted_mode;
/* Mute video and audio output */
hdmi_modb(hdmi, HDMI_AV_MUTE, m_AUDIO_MUTE | m_VIDEO_BLACK,
v_AUDIO_MUTE(1) | v_VIDEO_MUTE(1));
@@ -477,26 +440,26 @@ static int inno_hdmi_setup(struct inno_hdmi *hdmi,
inno_hdmi_config_video_timing(hdmi, mode);
inno_hdmi_config_video_csc(hdmi);
- if (display->is_hdmi)
- inno_hdmi_config_video_avi(hdmi, mode);
+ drm_atomic_helper_connector_hdmi_update_infoframes(connector,
+ new_conn_state->state);
/*
* When IP controller have configured to an accurate video
* timing, then the TMDS clock source would be switched to
* DCLK_LCDC, so we need to init the TMDS rate to mode pixel
* clock rate, and reconfigure the DDC clock.
*/
- inno_hdmi_i2c_init(hdmi, mpixelclock);
+ inno_hdmi_i2c_init(hdmi, new_conn_state->hdmi.tmds_char_rate);
/* Unmute video and audio output */
hdmi_modb(hdmi, HDMI_AV_MUTE, m_AUDIO_MUTE | m_VIDEO_BLACK,
v_AUDIO_MUTE(0) | v_VIDEO_MUTE(0));
- inno_hdmi_power_up(hdmi, mpixelclock);
+ inno_hdmi_power_up(hdmi, new_conn_state->hdmi.tmds_char_rate);
return 0;
}
static enum drm_mode_status inno_hdmi_display_mode_valid(struct inno_hdmi *hdmi,
@@ -544,11 +507,11 @@ static void inno_hdmi_encoder_enable(struct drm_encoder *encoder,
crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
if (WARN_ON(!crtc_state))
return;
- inno_hdmi_setup(hdmi, &crtc_state->adjusted_mode);
+ inno_hdmi_setup(hdmi, crtc_state, conn_state);
}
static void inno_hdmi_encoder_disable(struct drm_encoder *encoder,
struct drm_atomic_state *state)
{
@@ -561,11 +524,10 @@ static int
inno_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
- struct inno_hdmi *hdmi = encoder_to_inno_hdmi(encoder);
struct drm_display_mode *mode = &crtc_state->adjusted_mode;
u8 vic = drm_match_cea_mode(mode);
struct inno_hdmi_connector_state *inno_conn_state =
to_inno_hdmi_conn_state(conn_state);
@@ -578,16 +540,11 @@ inno_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
vic == 17 || vic == 18)
inno_conn_state->colorimetry = HDMI_COLORIMETRY_ITU_601;
else
inno_conn_state->colorimetry = HDMI_COLORIMETRY_ITU_709;
- inno_conn_state->enc_out_format = HDMI_COLORSPACE_RGB;
- inno_conn_state->rgb_limited_range =
- drm_default_rgb_quant_range(mode) == HDMI_QUANTIZATION_RANGE_LIMITED;
-
- return inno_hdmi_display_mode_valid(hdmi,
- &crtc_state->adjusted_mode) == MODE_OK ? 0 : -EINVAL;
+ return 0;
}
static struct drm_encoder_helper_funcs inno_hdmi_encoder_helper_funcs = {
.atomic_check = inno_hdmi_encoder_atomic_check,
.atomic_enable = inno_hdmi_encoder_enable,
@@ -660,14 +617,13 @@ static void inno_hdmi_connector_reset(struct drm_connector *connector)
inno_conn_state = kzalloc(sizeof(*inno_conn_state), GFP_KERNEL);
if (!inno_conn_state)
return;
__drm_atomic_helper_connector_reset(connector, &inno_conn_state->base);
+ __drm_atomic_helper_connector_hdmi_reset(connector, connector->state);
inno_conn_state->colorimetry = HDMI_COLORIMETRY_ITU_709;
- inno_conn_state->enc_out_format = HDMI_COLORSPACE_RGB;
- inno_conn_state->rgb_limited_range = false;
}
static struct drm_connector_state *
inno_hdmi_connector_duplicate_state(struct drm_connector *connector)
{
@@ -696,10 +652,11 @@ static const struct drm_connector_funcs inno_hdmi_connector_funcs = {
.atomic_duplicate_state = inno_hdmi_connector_duplicate_state,
.atomic_destroy_state = inno_hdmi_connector_destroy_state,
};
static struct drm_connector_helper_funcs inno_hdmi_connector_helper_funcs = {
+ .atomic_check = drm_atomic_helper_connector_hdmi_check,
.get_modes = inno_hdmi_connector_get_modes,
.mode_valid = inno_hdmi_connector_mode_valid,
};
static int inno_hdmi_register(struct drm_device *drm, struct inno_hdmi *hdmi)
@@ -723,14 +680,18 @@ static int inno_hdmi_register(struct drm_device *drm, struct inno_hdmi *hdmi)
hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
drm_connector_helper_add(&hdmi->connector,
&inno_hdmi_connector_helper_funcs);
- drm_connector_init_with_ddc(drm, &hdmi->connector,
- &inno_hdmi_connector_funcs,
- DRM_MODE_CONNECTOR_HDMIA,
- hdmi->ddc);
+ drmm_connector_hdmi_init(drm, &hdmi->connector,
+ "Rockchip", "Inno HDMI",
+ &inno_hdmi_connector_funcs,
+ &inno_hdmi_hdmi_connector_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ hdmi->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
drm_connector_attach_encoder(&hdmi->connector, encoder);
return 0;
}
--
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 v11 22/28] drm/tests: Add infoframes test
From: Maxime Ripard @ 2024-03-26 15:40 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: <20240326-kms-hdmi-connector-state-v11-0-c5680ffcf261@kernel.org>
The previous patch added the generation of the infoframes matching an
HDMI connector state. Let's add a few tests to make sure it works as
expected.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/tests/drm_connector_test.c | 219 +++++++++++++++++++++++++++++
1 file changed, 219 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index 9129cee56975..9260aa378421 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -219,10 +219,221 @@ static void drm_test_connector_hdmi_init_null_ddc(struct kunit *test)
BIT(HDMI_COLORSPACE_RGB),
8);
KUNIT_EXPECT_EQ(test, ret, 0);
}
+/*
+ * Test that the registration of an HDMI connector with a NULL vendor
+ * fails.
+ */
+static void drm_test_connector_hdmi_init_null_vendor(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ NULL, "Product",
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_LT(test, ret, 0);
+}
+
+/*
+ * Test that the registration of an HDMI connector with a NULL product
+ * fails.
+ */
+static void drm_test_connector_hdmi_init_null_product(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ "Vendor", NULL,
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_LT(test, ret, 0);
+}
+
+/*
+ * Test that the registration of a connector with a valid, shorter than
+ * the max length, product name succeeds, and is stored padded with 0.
+ */
+static void drm_test_connector_hdmi_init_product_valid(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ const unsigned char expected_product[DRM_CONNECTOR_HDMI_PRODUCT_LEN] = {
+ 'P', 'r', 'o', 'd',
+ };
+ const char *product_name = "Prod";
+ int ret;
+
+ KUNIT_ASSERT_LT(test, strlen(product_name), DRM_CONNECTOR_HDMI_PRODUCT_LEN);
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ "Vendor", product_name,
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_MEMEQ(test,
+ priv->connector.hdmi.product,
+ expected_product,
+ sizeof(priv->connector.hdmi.product));
+}
+
+/*
+ * Test that the registration of a connector with a valid, at max
+ * length, product name succeeds, and is stored padded without any
+ * trailing \0.
+ */
+static void drm_test_connector_hdmi_init_product_length_exact(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ const unsigned char expected_product[DRM_CONNECTOR_HDMI_PRODUCT_LEN] = {
+ 'P', 'r', 'o', 'd', 'u', 'c', 't',
+ 'P', 'r', 'o', 'd', 'u', 'c', 't',
+ 'P', 'r',
+ };
+ const char *product_name = "ProductProductPr";
+ int ret;
+
+ KUNIT_ASSERT_EQ(test, strlen(product_name), DRM_CONNECTOR_HDMI_PRODUCT_LEN);
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ "Vendor", product_name,
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_MEMEQ(test,
+ priv->connector.hdmi.product,
+ expected_product,
+ sizeof(priv->connector.hdmi.product));
+}
+
+/*
+ * Test that the registration of a connector with a product name larger
+ * than the maximum length fails.
+ */
+static void drm_test_connector_hdmi_init_product_length_too_long(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ const char *product_name = "ProductProductProduct";
+ int ret;
+
+ KUNIT_ASSERT_GT(test, strlen(product_name), DRM_CONNECTOR_HDMI_PRODUCT_LEN);
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ "Vendor", product_name,
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_LT(test, ret, 0);
+}
+
+/*
+ * Test that the registration of a connector with a vendor name smaller
+ * than the maximum length succeeds, and is stored padded with zeros.
+ */
+static void drm_test_connector_hdmi_init_vendor_valid(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ const char expected_vendor[DRM_CONNECTOR_HDMI_VENDOR_LEN] = {
+ 'V', 'e', 'n', 'd',
+ };
+ const char *vendor_name = "Vend";
+ int ret;
+
+ KUNIT_ASSERT_LT(test, strlen(vendor_name), DRM_CONNECTOR_HDMI_VENDOR_LEN);
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ vendor_name, "Product",
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_MEMEQ(test,
+ priv->connector.hdmi.vendor,
+ expected_vendor,
+ sizeof(priv->connector.hdmi.vendor));
+}
+
+/*
+ * Test that the registration of a connector with a vendor name at the
+ * maximum length succeeds, and is stored padded without the trailing
+ * zero.
+ */
+static void drm_test_connector_hdmi_init_vendor_length_exact(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ const char expected_vendor[DRM_CONNECTOR_HDMI_VENDOR_LEN] = {
+ 'V', 'e', 'n', 'd', 'o', 'r',
+ 'V', 'e',
+ };
+ const char *vendor_name = "VendorVe";
+ int ret;
+
+ KUNIT_ASSERT_EQ(test, strlen(vendor_name), DRM_CONNECTOR_HDMI_VENDOR_LEN);
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ vendor_name, "Product",
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_MEMEQ(test,
+ priv->connector.hdmi.vendor,
+ expected_vendor,
+ sizeof(priv->connector.hdmi.vendor));
+}
+
+/*
+ * Test that the registration of a connector with a vendor name larger
+ * than the maximum length fails.
+ */
+static void drm_test_connector_hdmi_init_vendor_length_too_long(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ const char *vendor_name = "VendorVendor";
+ int ret;
+
+ KUNIT_ASSERT_GT(test, strlen(vendor_name), DRM_CONNECTOR_HDMI_VENDOR_LEN);
+
+ ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ vendor_name, "Product",
+ &dummy_funcs,
+ &dummy_hdmi_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ &priv->ddc,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
+ KUNIT_EXPECT_LT(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)
@@ -499,10 +710,18 @@ static struct kunit_case drmm_connector_hdmi_init_tests[] = {
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_formats_empty),
KUNIT_CASE(drm_test_connector_hdmi_init_formats_no_rgb),
KUNIT_CASE(drm_test_connector_hdmi_init_null_ddc),
+ KUNIT_CASE(drm_test_connector_hdmi_init_null_product),
+ KUNIT_CASE(drm_test_connector_hdmi_init_null_vendor),
+ KUNIT_CASE(drm_test_connector_hdmi_init_product_length_exact),
+ KUNIT_CASE(drm_test_connector_hdmi_init_product_length_too_long),
+ KUNIT_CASE(drm_test_connector_hdmi_init_product_valid),
+ KUNIT_CASE(drm_test_connector_hdmi_init_vendor_length_exact),
+ KUNIT_CASE(drm_test_connector_hdmi_init_vendor_length_too_long),
+ KUNIT_CASE(drm_test_connector_hdmi_init_vendor_valid),
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),
{ }
--
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 v11 21/28] drm/connector: hdmi: Add Infoframes generation
From: Maxime Ripard @ 2024-03-26 15:40 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: <20240326-kms-hdmi-connector-state-v11-0-c5680ffcf261@kernel.org>
Infoframes in KMS is usually handled by a bunch of low-level helpers
that require quite some boilerplate for drivers. This leads to
discrepancies with how drivers generate them, and which are actually
sent.
Now that we have everything needed to generate them in the HDMI
connector state, we can generate them in our common logic so that
drivers can simply reuse what we precomputed.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/display/drm_hdmi_state_helper.c | 336 +++++++++++++++++++++
drivers/gpu/drm/drm_connector.c | 14 +
drivers/gpu/drm/tests/drm_connector_test.c | 12 +
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 1 +
include/drm/display/drm_hdmi_state_helper.h | 7 +
include/drm/drm_connector.h | 109 +++++++
6 files changed, 479 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index e693fe51abf5..0fc97edac9ae 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -311,10 +311,148 @@ hdmi_compute_config(const struct drm_connector *connector,
}
return -EINVAL;
}
+static int hdmi_generate_avi_infoframe(const struct drm_connector *connector,
+ struct drm_connector_state *conn_state)
+{
+ const struct drm_display_mode *mode =
+ connector_state_get_mode(conn_state);
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &conn_state->hdmi.infoframes.avi;
+ struct hdmi_avi_infoframe *frame =
+ &infoframe->data.avi;
+ bool is_full_range = conn_state->hdmi.is_full_range;
+ enum hdmi_quantization_range rgb_quant_range =
+ is_full_range ? HDMI_QUANTIZATION_RANGE_FULL : HDMI_QUANTIZATION_RANGE_LIMITED;
+ int ret;
+
+ ret = drm_hdmi_avi_infoframe_from_display_mode(frame, connector, mode);
+ if (ret)
+ return ret;
+
+ frame->colorspace = conn_state->hdmi.output_format;
+
+ /*
+ * FIXME: drm_hdmi_avi_infoframe_quant_range() doesn't handle
+ * YUV formats at all at the moment, so if we ever support YUV
+ * formats this needs to be revised.
+ */
+ drm_hdmi_avi_infoframe_quant_range(frame, connector, mode, rgb_quant_range);
+ drm_hdmi_avi_infoframe_colorimetry(frame, conn_state);
+ drm_hdmi_avi_infoframe_bars(frame, conn_state);
+
+ infoframe->set = true;
+
+ return 0;
+}
+
+static int hdmi_generate_spd_infoframe(const struct drm_connector *connector,
+ struct drm_connector_state *conn_state)
+{
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &conn_state->hdmi.infoframes.spd;
+ struct hdmi_spd_infoframe *frame =
+ &infoframe->data.spd;
+ int ret;
+
+ ret = hdmi_spd_infoframe_init(frame,
+ connector->hdmi.vendor,
+ connector->hdmi.product);
+ if (ret)
+ return ret;
+
+ frame->sdi = HDMI_SPD_SDI_PC;
+
+ infoframe->set = true;
+
+ return 0;
+}
+
+static int hdmi_generate_hdr_infoframe(const struct drm_connector *connector,
+ struct drm_connector_state *conn_state)
+{
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &conn_state->hdmi.infoframes.hdr_drm;
+ struct hdmi_drm_infoframe *frame =
+ &infoframe->data.drm;
+ int ret;
+
+ if (connector->max_bpc < 10)
+ return 0;
+
+ if (!conn_state->hdr_output_metadata)
+ return 0;
+
+ ret = drm_hdmi_infoframe_set_hdr_metadata(frame, conn_state);
+ if (ret)
+ return ret;
+
+ infoframe->set = true;
+
+ return 0;
+}
+
+static int hdmi_generate_hdmi_vendor_infoframe(const struct drm_connector *connector,
+ struct drm_connector_state *conn_state)
+{
+ const struct drm_display_info *info = &connector->display_info;
+ const struct drm_display_mode *mode =
+ connector_state_get_mode(conn_state);
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &conn_state->hdmi.infoframes.hdmi;
+ struct hdmi_vendor_infoframe *frame =
+ &infoframe->data.vendor.hdmi;
+ int ret;
+
+ if (!info->has_hdmi_infoframe)
+ return 0;
+
+ ret = drm_hdmi_vendor_infoframe_from_display_mode(frame, connector, mode);
+ if (ret)
+ return ret;
+
+ infoframe->set = true;
+
+ return 0;
+}
+
+static int
+hdmi_generate_infoframes(const struct drm_connector *connector,
+ struct drm_connector_state *conn_state)
+{
+ const struct drm_display_info *info = &connector->display_info;
+ int ret;
+
+ if (!info->is_hdmi)
+ return 0;
+
+ ret = hdmi_generate_avi_infoframe(connector, conn_state);
+ if (ret)
+ return ret;
+
+ ret = hdmi_generate_spd_infoframe(connector, conn_state);
+ if (ret)
+ return ret;
+
+ /*
+ * Audio Infoframes will be generated by ALSA, and updated by
+ * drm_atomic_helper_connector_hdmi_update_audio_infoframe().
+ */
+
+ ret = hdmi_generate_hdr_infoframe(connector, conn_state);
+ if (ret)
+ return ret;
+
+ ret = hdmi_generate_hdmi_vendor_infoframe(connector, conn_state);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
/**
* drm_atomic_helper_connector_hdmi_check() - Helper to check HDMI connector atomic state
* @connector: DRM Connector
* @state: the DRM State object
*
@@ -340,10 +478,14 @@ int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
ret = hdmi_compute_config(connector, new_conn_state, mode);
if (ret)
return ret;
+ ret = hdmi_generate_infoframes(connector, new_conn_state);
+ if (ret)
+ return ret;
+
if (old_conn_state->hdmi.broadcast_rgb != new_conn_state->hdmi.broadcast_rgb ||
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;
@@ -356,5 +498,199 @@ int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
}
return 0;
}
EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_check);
+
+#define HDMI_MAX_INFOFRAME_SIZE 29
+
+static int clear_device_infoframe(struct drm_connector *connector,
+ enum hdmi_infoframe_type type)
+{
+ const struct drm_connector_hdmi_funcs *funcs = connector->hdmi.funcs;
+ struct drm_device *dev = connector->dev;
+ int ret;
+
+ drm_dbg_kms(dev, "Clearing infoframe type 0x%x\n", type);
+
+ if (!funcs || !funcs->clear_infoframe) {
+ drm_dbg_kms(dev, "Function not implemented, bailing.\n");
+ return 0;
+ }
+
+ ret = funcs->clear_infoframe(connector, type);
+ if (ret) {
+ drm_dbg_kms(dev, "Call failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int clear_infoframe(struct drm_connector *connector,
+ struct drm_connector_hdmi_infoframe *old_frame)
+{
+ int ret;
+
+ ret = clear_device_infoframe(connector, old_frame->data.any.type);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int write_device_infoframe(struct drm_connector *connector,
+ union hdmi_infoframe *frame)
+{
+ const struct drm_connector_hdmi_funcs *funcs = connector->hdmi.funcs;
+ struct drm_device *dev = connector->dev;
+ u8 buffer[HDMI_MAX_INFOFRAME_SIZE];
+ int ret;
+ int len;
+
+ drm_dbg_kms(dev, "Writing infoframe type %x\n", frame->any.type);
+
+ if (!funcs || !funcs->write_infoframe) {
+ drm_dbg_kms(dev, "Function not implemented, bailing.\n");
+ return -ENOSYS;
+ }
+
+ len = hdmi_infoframe_pack(frame, buffer, sizeof(buffer));
+ if (len < 0)
+ return len;
+
+ ret = funcs->write_infoframe(connector, frame->any.type, buffer, len);
+ if (ret) {
+ drm_dbg_kms(dev, "Call failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int write_infoframe(struct drm_connector *connector,
+ struct drm_connector_hdmi_infoframe *new_frame)
+{
+ int ret;
+
+ ret = write_device_infoframe(connector, &new_frame->data);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int write_or_clear_infoframe(struct drm_connector *connector,
+ struct drm_connector_hdmi_infoframe *old_frame,
+ struct drm_connector_hdmi_infoframe *new_frame)
+{
+ if (new_frame->set)
+ return write_infoframe(connector, new_frame);
+
+ if (old_frame->set && !new_frame->set)
+ return clear_infoframe(connector, old_frame);
+
+ return 0;
+}
+
+/**
+ * drm_atomic_helper_connector_hdmi_update_infoframes - Update the Infoframes
+ * @connector: A pointer to the HDMI connector
+ * @state: The HDMI connector state to generate the infoframe from
+ *
+ * This function is meant for HDMI connector drivers to write their
+ * infoframes. It will typically be used in a
+ * @drm_connector_helper_funcs.atomic_enable implementation.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_atomic_helper_connector_hdmi_update_infoframes(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ 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);
+ struct drm_display_info *info = &connector->display_info;
+ int ret;
+
+ if (!info->is_hdmi)
+ return 0;
+
+ mutex_lock(&connector->hdmi.infoframes.lock);
+
+ ret = write_or_clear_infoframe(connector,
+ &old_conn_state->hdmi.infoframes.avi,
+ &new_conn_state->hdmi.infoframes.avi);
+ if (ret)
+ goto out;
+
+ if (connector->hdmi.infoframes.audio.set) {
+ ret = write_infoframe(connector,
+ &connector->hdmi.infoframes.audio);
+ if (ret)
+ goto out;
+ }
+
+ ret = write_or_clear_infoframe(connector,
+ &old_conn_state->hdmi.infoframes.hdr_drm,
+ &new_conn_state->hdmi.infoframes.hdr_drm);
+ if (ret)
+ goto out;
+
+ ret = write_or_clear_infoframe(connector,
+ &old_conn_state->hdmi.infoframes.spd,
+ &new_conn_state->hdmi.infoframes.spd);
+ if (ret)
+ goto out;
+
+ if (info->has_hdmi_infoframe) {
+ ret = write_or_clear_infoframe(connector,
+ &old_conn_state->hdmi.infoframes.hdmi,
+ &new_conn_state->hdmi.infoframes.hdmi);
+ if (ret)
+ goto out;
+ }
+
+out:
+ mutex_unlock(&connector->hdmi.infoframes.lock);
+ return ret;
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_update_infoframes);
+
+/**
+ * drm_atomic_helper_connector_hdmi_update_audio_infoframe - Update the Audio Infoframe
+ * @connector: A pointer to the HDMI connector
+ * @frame: A pointer to the audio infoframe to write
+ *
+ * This function is meant for HDMI connector drivers to update their
+ * audio infoframe. It will typically be used in one of the ALSA hooks
+ * (most likely prepare).
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int
+drm_atomic_helper_connector_hdmi_update_audio_infoframe(struct drm_connector *connector,
+ struct hdmi_audio_infoframe *frame)
+{
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &connector->hdmi.infoframes.audio;
+ struct drm_display_info *info = &connector->display_info;
+ int ret;
+
+ if (!info->is_hdmi)
+ return 0;
+
+ mutex_lock(&connector->hdmi.infoframes.lock);
+
+ memcpy(&infoframe->data, frame, sizeof(infoframe->data));
+ infoframe->set = true;
+
+ ret = write_infoframe(connector, infoframe);
+
+ mutex_unlock(&connector->hdmi.infoframes.lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_update_audio_infoframe);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index bdd3361ccc73..7237e8cf8c58 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -276,10 +276,11 @@ static int __drm_connector_init(struct drm_device *dev,
INIT_LIST_HEAD(&connector->global_connector_list_entry);
INIT_LIST_HEAD(&connector->probed_modes);
INIT_LIST_HEAD(&connector->modes);
mutex_init(&connector->mutex);
mutex_init(&connector->edid_override_mutex);
+ mutex_init(&connector->hdmi.infoframes.lock);
connector->edid_blob_ptr = NULL;
connector->epoch_counter = 0;
connector->tile_blob_ptr = NULL;
connector->status = connector_status_unknown;
connector->display_info.panel_orientation =
@@ -454,10 +455,12 @@ 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
+ * @vendor: HDMI Controller Vendor name
+ * @product: HDMI Controller Product name
* @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
@@ -474,19 +477,27 @@ EXPORT_SYMBOL(drmm_connector_init);
* Returns:
* Zero on success, error code on failure.
*/
int drmm_connector_hdmi_init(struct drm_device *dev,
struct drm_connector *connector,
+ const char *vendor, const char *product,
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)
{
int ret;
+ if (!vendor || !product)
+ return -EINVAL;
+
+ if ((strlen(vendor) > DRM_CONNECTOR_HDMI_VENDOR_LEN) ||
+ (strlen(product) > DRM_CONNECTOR_HDMI_PRODUCT_LEN))
+ return -EINVAL;
+
if (!(connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_HDMIB))
return -EINVAL;
if (!supported_formats || !(supported_formats & BIT(HDMI_COLORSPACE_RGB)))
@@ -498,10 +509,12 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
if (ret)
return ret;
connector->hdmi.supported_formats = supported_formats;
+ strtomem_pad(connector->hdmi.vendor, vendor, 0);
+ strtomem_pad(connector->hdmi.product, product, 0);
/*
* drm_connector_attach_max_bpc_property() requires the
* connector to have a state.
*/
@@ -650,10 +663,11 @@ void drm_connector_cleanup(struct drm_connector *connector)
WARN_ON(connector->state && !connector->funcs->atomic_destroy_state);
if (connector->state && connector->funcs->atomic_destroy_state)
connector->funcs->atomic_destroy_state(connector,
connector->state);
+ mutex_destroy(&connector->hdmi.infoframes.lock);
mutex_destroy(&connector->mutex);
memset(connector, 0, sizeof(*connector));
if (dev->registered)
diff --git a/drivers/gpu/drm/tests/drm_connector_test.c b/drivers/gpu/drm/tests/drm_connector_test.c
index 1d11f802c36d..9129cee56975 100644
--- a/drivers/gpu/drm/tests/drm_connector_test.c
+++ b/drivers/gpu/drm/tests/drm_connector_test.c
@@ -189,10 +189,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,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
@@ -208,10 +209,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,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
NULL,
BIT(HDMI_COLORSPACE_RGB),
@@ -227,10 +229,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,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
@@ -246,10 +249,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,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
@@ -269,10 +273,11 @@ static void drm_test_connector_hdmi_init_bpc_8(struct kunit *test)
struct drm_property *prop;
uint64_t val;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
@@ -304,10 +309,11 @@ static void drm_test_connector_hdmi_init_bpc_10(struct kunit *test)
struct drm_property *prop;
uint64_t val;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
@@ -339,10 +345,11 @@ static void drm_test_connector_hdmi_init_bpc_12(struct kunit *test)
struct drm_property *prop;
uint64_t val;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
@@ -370,10 +377,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,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
0,
@@ -389,10 +397,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,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_YUV422),
@@ -409,10 +418,11 @@ static void drm_test_connector_hdmi_init_type_valid(struct kunit *test)
struct drm_connector_init_priv *priv = test->priv;
unsigned int connector_type = *(unsigned int *)test->param_value;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
connector_type,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
@@ -443,10 +453,11 @@ static void drm_test_connector_hdmi_init_type_invalid(struct kunit *test)
struct drm_connector_init_priv *priv = test->priv;
unsigned int connector_type = *(unsigned int *)test->param_value;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, &priv->connector,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
connector_type,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
@@ -708,10 +719,11 @@ static void drm_test_drm_connector_attach_broadcast_rgb_property_hdmi_connector(
struct drm_connector *connector = &priv->connector;
struct drm_property *prop;
int ret;
ret = drmm_connector_hdmi_init(&priv->drm, connector,
+ "Vendor", "Product",
&dummy_funcs,
&dummy_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
&priv->ddc,
BIT(HDMI_COLORSPACE_RGB),
diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index 84c081b99582..9d42a327c576 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -204,10 +204,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,
+ "Vendor", "Product",
&dummy_connector_funcs,
&dummy_connector_hdmi_funcs,
DRM_MODE_CONNECTOR_HDMIA,
NULL,
formats,
diff --git a/include/drm/display/drm_hdmi_state_helper.h b/include/drm/display/drm_hdmi_state_helper.h
index 6021983e2602..fbf86ff9cdfb 100644
--- a/include/drm/display/drm_hdmi_state_helper.h
+++ b/include/drm/display/drm_hdmi_state_helper.h
@@ -4,13 +4,20 @@
#define DRM_HDMI_STATE_HELPER_H_
struct drm_atomic_state;
struct drm_connector;
struct drm_connector_state;
+struct hdmi_audio_infoframe;
void __drm_atomic_helper_connector_hdmi_reset(struct drm_connector *connector,
struct drm_connector_state *new_conn_state);
int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
struct drm_atomic_state *state);
+int drm_atomic_helper_connector_hdmi_update_audio_infoframe(struct drm_connector *connector,
+ struct hdmi_audio_infoframe *frame);
+int drm_atomic_helper_connector_hdmi_update_infoframes(struct drm_connector *connector,
+ struct drm_atomic_state *state);
+
+
#endif // DRM_HDMI_STATE_HELPER_H_
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index f5e960e89de3..5321da490a0c 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -912,10 +912,25 @@ struct drm_tv_connector_state {
unsigned int overscan;
unsigned int saturation;
unsigned int hue;
};
+/**
+ * struct drm_connector_hdmi_infoframe - HDMI Infoframe container
+ */
+struct drm_connector_hdmi_infoframe {
+ /**
+ * @data: HDMI Infoframe structure
+ */
+ union hdmi_infoframe data;
+
+ /**
+ * @set: Is the content of @data valid?
+ */
+ bool set;
+};
+
/**
* struct drm_connector_state - mutable connector state
*/
struct drm_connector_state {
/** @connector: backpointer to the connector */
@@ -1068,10 +1083,39 @@ struct drm_connector_state {
* @broadcast_rgb: Connector property to pass the
* Broadcast RGB selection value.
*/
enum drm_hdmi_broadcast_rgb broadcast_rgb;
+ /**
+ * @infoframes: HDMI Infoframes matching that state
+ */
+ struct {
+ /**
+ * @avi: AVI Infoframes structure matching our
+ * state.
+ */
+ struct drm_connector_hdmi_infoframe avi;
+
+ /**
+ * @hdr_drm: DRM (Dynamic Range and Mastering)
+ * Infoframes structure matching our state.
+ */
+ struct drm_connector_hdmi_infoframe hdr_drm;
+
+ /**
+ * @spd: SPD Infoframes structure matching our
+ * state.
+ */
+ struct drm_connector_hdmi_infoframe spd;
+
+ /**
+ * @vendor: HDMI Vendor Infoframes structure
+ * matching our state.
+ */
+ struct drm_connector_hdmi_infoframe hdmi;
+ } infoframes;
+
/**
* @is_full_range: Is the output supposed to use a full
* RGB Quantization Range or not?
*/
bool is_full_range;
@@ -1113,10 +1157,45 @@ struct drm_connector_hdmi_funcs {
*/
enum drm_mode_status
(*tmds_char_rate_valid)(const struct drm_connector *connector,
const struct drm_display_mode *mode,
unsigned long long tmds_rate);
+
+ /**
+ * @clear_infoframe:
+ *
+ * This callback is invoked through
+ * @drm_atomic_helper_connector_hdmi_update_infoframes during a
+ * commit to clear the infoframes into the hardware. It will be
+ * called multiple times, once for every disabled infoframe
+ * type.
+ *
+ * The @clear_infoframe callback is optional.
+ *
+ * Returns:
+ * 0 on success, a negative error code otherwise
+ */
+ int (*clear_infoframe)(struct drm_connector *connector,
+ enum hdmi_infoframe_type type);
+
+ /**
+ * @write_infoframe:
+ *
+ * This callback is invoked through
+ * @drm_atomic_helper_connector_hdmi_update_infoframes during a
+ * commit to program the infoframes into the hardware. It will
+ * be called multiple times, once for every updated infoframe
+ * type.
+ *
+ * The @write_infoframe callback is mandatory.
+ *
+ * Returns:
+ * 0 on success, a negative error code otherwise
+ */
+ int (*write_infoframe)(struct drm_connector *connector,
+ enum hdmi_infoframe_type type,
+ const u8 *buffer, size_t len);
};
/**
* struct drm_connector_funcs - control connectors on a given device
*
@@ -1984,20 +2063,49 @@ struct drm_connector {
/**
* @hdmi: HDMI-related variable and properties.
*/
struct {
+#define DRM_CONNECTOR_HDMI_VENDOR_LEN 8
+ /**
+ * @vendor: HDMI Controller Vendor Name
+ */
+ unsigned char vendor[DRM_CONNECTOR_HDMI_VENDOR_LEN] __nonstring;
+
+#define DRM_CONNECTOR_HDMI_PRODUCT_LEN 16
+ /**
+ * @product: HDMI Controller Product Name
+ */
+ unsigned char product[DRM_CONNECTOR_HDMI_PRODUCT_LEN] __nonstring;
+
/**
* @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;
+
+ /**
+ * @infoframes: Current Infoframes output by the connector
+ */
+ struct {
+ /**
+ * @lock: Mutex protecting against concurrent access to
+ * the infoframes, most notably between KMS and ALSA.
+ */
+ struct mutex lock;
+
+ /**
+ * @audio: Current Audio Infoframes structure. Protected
+ * by @lock.
+ */
+ struct drm_connector_hdmi_infoframe audio;
+ } infoframes;
} hdmi;
};
#define obj_to_connector(x) container_of(x, struct drm_connector, base)
@@ -2015,10 +2123,11 @@ int drmm_connector_init(struct drm_device *dev,
const struct drm_connector_funcs *funcs,
int connector_type,
struct i2c_adapter *ddc);
int drmm_connector_hdmi_init(struct drm_device *dev,
struct drm_connector *connector,
+ const char *vendor, const char *product,
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,
--
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 v11 20/28] drm/tests: Add RGB Quantization tests
From: Maxime Ripard @ 2024-03-26 15:40 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: <20240326-kms-hdmi-connector-state-v11-0-c5680ffcf261@kernel.org>
The previous commit added the infrastructure to the connector state to
track what RGB Quantization should be used in a given state for an HDMI
connector.
Let's add some kunit 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/drm_hdmi_state_helper_test.c | 355 +++++++++++++++++++++
1 file changed, 355 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index ff9a882201eb..84c081b99582 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -353,10 +353,354 @@ static void drm_test_check_broadcast_rgb_crtc_mode_not_changed(struct kunit *tes
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 for an HDMI connector, with an HDMI monitor, if the
+ * Broadcast RGB property is set to auto with a mode that isn't the
+ * VIC-1 mode, we will get a limited RGB Quantization Range.
+ */
+static void drm_test_check_broadcast_rgb_auto_cea_mode(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_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);
+
+ conn = &priv->connector;
+ KUNIT_ASSERT_TRUE(test, conn->display_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);
+ KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
+
+ 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);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ KUNIT_ASSERT_EQ(test,
+ conn_state->hdmi.broadcast_rgb,
+ DRM_HDMI_BROADCAST_RGB_AUTO);
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ KUNIT_EXPECT_FALSE(test, conn_state->hdmi.is_full_range);
+}
+
+/*
+ * Test that for an HDMI connector, with an HDMI monitor, if the
+ * Broadcast RGB property is set to auto with a VIC-1 mode, we will get
+ * a full RGB Quantization Range.
+ */
+static void drm_test_check_broadcast_rgb_auto_cea_mode_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_atomic_state *state;
+ struct drm_display_mode *mode;
+ 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);
+
+ drm = &priv->drm;
+ conn = &priv->connector;
+ KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
+
+ 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);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, mode, 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);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ KUNIT_ASSERT_EQ(test,
+ conn_state->hdmi.broadcast_rgb,
+ DRM_HDMI_BROADCAST_RGB_AUTO);
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ KUNIT_EXPECT_TRUE(test, conn_state->hdmi.is_full_range);
+}
+
+/*
+ * Test that for an HDMI connector, with an HDMI monitor, if the
+ * Broadcast RGB property is set to full with a mode that isn't the
+ * VIC-1 mode, we will get a full RGB Quantization Range.
+ */
+static void drm_test_check_broadcast_rgb_full_cea_mode(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_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);
+
+ conn = &priv->connector;
+ KUNIT_ASSERT_TRUE(test, conn->display_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);
+ KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
+
+ 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);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ conn_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_FULL;
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ KUNIT_ASSERT_EQ(test,
+ conn_state->hdmi.broadcast_rgb,
+ DRM_HDMI_BROADCAST_RGB_FULL);
+
+ KUNIT_EXPECT_TRUE(test, conn_state->hdmi.is_full_range);
+}
+
+/*
+ * Test that for an HDMI connector, with an HDMI monitor, if the
+ * Broadcast RGB property is set to full with a VIC-1 mode, we will get
+ * a full RGB Quantization Range.
+ */
+static void drm_test_check_broadcast_rgb_full_cea_mode_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_atomic_state *state;
+ struct drm_display_mode *mode;
+ 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);
+
+ drm = &priv->drm;
+ conn = &priv->connector;
+ KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
+
+ 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);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, mode, 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);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ conn_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_FULL;
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ KUNIT_ASSERT_EQ(test,
+ conn_state->hdmi.broadcast_rgb,
+ DRM_HDMI_BROADCAST_RGB_FULL);
+
+ KUNIT_EXPECT_TRUE(test, conn_state->hdmi.is_full_range);
+}
+
+/*
+ * Test that for an HDMI connector, with an HDMI monitor, if the
+ * Broadcast RGB property is set to limited with a mode that isn't the
+ * VIC-1 mode, we will get a limited RGB Quantization Range.
+ */
+static void drm_test_check_broadcast_rgb_limited_cea_mode(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_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);
+
+ conn = &priv->connector;
+ KUNIT_ASSERT_TRUE(test, conn->display_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);
+ KUNIT_ASSERT_NE(test, drm_match_cea_mode(preferred), 1);
+
+ 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);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ conn_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_LIMITED;
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ KUNIT_ASSERT_EQ(test,
+ conn_state->hdmi.broadcast_rgb,
+ DRM_HDMI_BROADCAST_RGB_LIMITED);
+
+ KUNIT_EXPECT_FALSE(test, conn_state->hdmi.is_full_range);
+}
+
+/*
+ * Test that for an HDMI connector, with an HDMI monitor, if the
+ * Broadcast RGB property is set to limited with a VIC-1 mode, we will
+ * get a limited RGB Quantization Range.
+ */
+static void drm_test_check_broadcast_rgb_limited_cea_mode_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_atomic_state *state;
+ struct drm_display_mode *mode;
+ 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);
+
+ drm = &priv->drm;
+ conn = &priv->connector;
+ KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
+
+ 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);
+
+ drm = &priv->drm;
+ crtc = priv->crtc;
+ ret = light_up_connector(test, drm, crtc, conn, mode, 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);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ conn_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_LIMITED;
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ conn_state = drm_atomic_get_connector_state(state, conn);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ KUNIT_ASSERT_EQ(test,
+ conn_state->hdmi.broadcast_rgb,
+ DRM_HDMI_BROADCAST_RGB_LIMITED);
+
+ KUNIT_EXPECT_FALSE(test, conn_state->hdmi.is_full_range);
+}
+
/*
* 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.
*/
@@ -1192,10 +1536,21 @@ 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_auto_cea_mode),
+ KUNIT_CASE(drm_test_check_broadcast_rgb_auto_cea_mode_vic_1),
+ KUNIT_CASE(drm_test_check_broadcast_rgb_full_cea_mode),
+ KUNIT_CASE(drm_test_check_broadcast_rgb_full_cea_mode_vic_1),
+ KUNIT_CASE(drm_test_check_broadcast_rgb_limited_cea_mode),
+ KUNIT_CASE(drm_test_check_broadcast_rgb_limited_cea_mode_vic_1),
+ /*
+ * TODO: When we'll have YUV output support, we need to check
+ * that the limited range is always set to limited no matter
+ * what the value of Broadcast RGB is.
+ */
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),
--
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 v11 19/28] drm/connector: hdmi: Add RGB Quantization Range to the connector state
From: Maxime Ripard @ 2024-03-26 15:40 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: <20240326-kms-hdmi-connector-state-v11-0-c5680ffcf261@kernel.org>
HDMI controller drivers will need to figure out the RGB range they need
to configure based on a mode and property values. Let's expose that in
the HDMI connector state so drivers can just use that value.
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/display/drm_hdmi_state_helper.c | 29 +++++++++++++++++++++++++
drivers/gpu/drm/drm_atomic.c | 1 +
include/drm/drm_connector.h | 6 +++++
3 files changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index c844cbeb675b..e693fe51abf5 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -49,10 +49,37 @@ connector_state_get_mode(const struct drm_connector_state *conn_state)
return NULL;
return &crtc_state->mode;
}
+static bool hdmi_is_full_range(const struct drm_connector *connector,
+ const struct drm_connector_state *conn_state)
+{
+ const struct drm_display_info *display = &connector->display_info;
+ const struct drm_display_mode *mode =
+ connector_state_get_mode(conn_state);
+
+ /*
+ * The Broadcast RGB property only applies to RGB format, and
+ * i915 just assumes limited range for YCbCr output, so let's
+ * just do the same.
+ */
+ if (conn_state->hdmi.output_format != HDMI_COLORSPACE_RGB)
+ return false;
+
+ if (conn_state->hdmi.broadcast_rgb == DRM_HDMI_BROADCAST_RGB_FULL)
+ return true;
+
+ if (conn_state->hdmi.broadcast_rgb == DRM_HDMI_BROADCAST_RGB_LIMITED)
+ return false;
+
+ if (!display->is_hdmi)
+ return true;
+
+ return drm_default_rgb_quant_range(mode) == HDMI_QUANTIZATION_RANGE_FULL ? true : false;
+}
+
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)
@@ -307,10 +334,12 @@ 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;
+ new_conn_state->hdmi.is_full_range = hdmi_is_full_range(connector, new_conn_state);
+
ret = hdmi_compute_config(connector, new_conn_state, mode);
if (ret)
return ret;
if (old_conn_state->hdmi.broadcast_rgb != new_conn_state->hdmi.broadcast_rgb ||
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3e57d98d8418..ec6c6beda5c9 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1145,10 +1145,11 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
drm_printf(p, "\tbroadcast_rgb=%s\n",
drm_hdmi_connector_get_broadcast_rgb_name(state->hdmi.broadcast_rgb));
+ drm_printf(p, "\tis_full_range=%c\n", state->hdmi.is_full_range ? 'y' : 'n');
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));
drm_printf(p, "\ttmds_char_rate=%llu\n", state->hdmi.tmds_char_rate);
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index a40eaf3a8ce4..f5e960e89de3 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1068,10 +1068,16 @@ struct drm_connector_state {
* @broadcast_rgb: Connector property to pass the
* Broadcast RGB selection value.
*/
enum drm_hdmi_broadcast_rgb broadcast_rgb;
+ /**
+ * @is_full_range: Is the output supposed to use a full
+ * RGB Quantization Range or not?
+ */
+ bool is_full_range;
+
/**
* @output_bpc: Bits per color channel to output.
*/
unsigned int output_bpc;
--
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
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