* [PATCH v2 07/19] arm64: insn: Add encoder for bitwise operations using litterals
From: James Morse @ 2017-12-13 15:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <07971a66-1b36-d2cd-e590-2b7b65d14b97@arm.com>
Hi Marc,
On 13/12/17 14:32, Marc Zyngier wrote:
> On 12/12/17 18:32, James Morse wrote:
>> On 11/12/17 14:49, Marc Zyngier wrote:
>>> We lack a way to encode operations such as AND, ORR, EOR that take
>>> an immediate value. Doing so is quite involved, and is all about
>>> reverse engineering the decoding algorithm described in the
>>> pseudocode function DecodeBitMasks().
>>> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
>>> index 7e432662d454..326b17016485 100644
>>> --- a/arch/arm64/kernel/insn.c
>>> +++ b/arch/arm64/kernel/insn.c
>>
>>> +static u32 aarch64_encode_immediate(u64 imm,
>>> + enum aarch64_insn_variant variant,
>>> + u32 insn)
>>> +{
>>> + unsigned int immr, imms, n, ones, ror, esz, tmp;
>>> + u64 mask;
>>
>> [...]
>>
>>> + /* Compute the rotation */
>>> + if (range_of_ones(imm)) {
>>> + /*
>>> + * Pattern: 0..01..10..0
>>> + *
>>> + * Compute how many rotate we need to align it right
>>> + */
>>> + ror = ffs(imm) - 1;
>>
>> (how come range_of_ones() uses __ffs64() on the same value?)
>
> News flash: range_of_ones is completely buggy. It will fail on the
> trivial value 1 (__ffs64(1) = 0; 0 - 1 = -1; val >> -1 is... ermmmm).
> I definitely got mixed up between the two.
They do different things!? Aaaaaahhhh....
[ ...]
>> Unless I've gone wrong, I think the 'Trim imm to the element size' code needs to
>> move up into the esz-reducing loop so it doesn't happen for a 64bit immediate.
> Yup. I've stashed the following patch:
>
> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> index b8fb2d89b3a6..e58be1c57f18 100644
> --- a/arch/arm64/kernel/insn.c
> +++ b/arch/arm64/kernel/insn.c
> @@ -1503,8 +1503,7 @@ pstate_check_t * const aarch32_opcode_cond_checks[16] = {
> static bool range_of_ones(u64 val)
> {
> /* Doesn't handle full ones or full zeroes */
> - int x = __ffs64(val) - 1;
> - u64 sval = val >> x;
> + u64 sval = val >> __ffs64(val);
>
> /* One of Sean Eron Anderson's bithack tricks */
> return ((sval + 1) & (sval)) == 0;
> @@ -1515,7 +1514,7 @@ static u32 aarch64_encode_immediate(u64 imm,
> u32 insn)
> {
> unsigned int immr, imms, n, ones, ror, esz, tmp;
> - u64 mask;
> + u64 mask = ~0UL;
>
> /* Can't encode full zeroes or full ones */
> if (!imm || !~imm)
> @@ -1543,8 +1542,12 @@ static u32 aarch64_encode_immediate(u64 imm,
> for (tmp = esz; tmp > 2; tmp /= 2) {
> u64 emask = BIT(tmp / 2) - 1;
>
> - if ((imm & emask) != ((imm >> (tmp / 2)) & emask))
> + if ((imm & emask) != ((imm >> (tmp / 2)) & emask)) {
> + /* Trim imm to the element size */
> + mask = BIT(esz - 1) - 1;
> + imm &= mask;
Won't this still lose the top bit? It generates 0x7fffffff for esz=32, and for
esz=32 we run through here when the two 16bit values are different.
This still runs for a 64bit immediate. The 0xf80000000fffffff example compares
0xf8000000 with 0fffffff then breaks here on the first iteration of this loop.
With this change it still attempts to generate a 64bit mask.
I was thinking of something like [0]. That only runs when we know the two
tmp:halves match, it just keeps the bottom tmp:half for the next run and never
runs for a 64bit immediate.
> break;
> + }
>
> esz = tmp;
> }
> @@ -1552,10 +1555,6 @@ static u32 aarch64_encode_immediate(u64 imm,
> /* N is only set if we're encoding a 64bit value */
> n = esz == 64;
>
> - /* Trim imm to the element size */
> - mask = BIT(esz - 1) - 1;
> - imm &= mask;
> -
> /* That's how many ones we need to encode */
> ones = hweight64(imm);
>
> I really need to run this against gas in order to make sure
> I get the same parameters for all the possible values.
Sounds good,
Thanks,
James
[0] Not even built:
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 12d3ec2154c2..d9fbdea7b18d 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -1529,15 +1529,15 @@ static u32 aarch64_encode_immediate(u64 imm,
break;
esz = tmp;
+
+ /* Trim imm to the element size */
+ mask = BIT(esz) - 1;
+ imm &= mask;
}
/* N is only set if we're encoding a 64bit value */
n = esz == 64;
- /* Trim imm to the element size */
- mask = BIT(esz - 1) - 1;
- imm &= mask;
-
/* That's how many ones we need to encode */
ones = hweight64(imm);
^ permalink raw reply related
* [PATCH v4 4/4] ARM: pinctrl: sunxi-pinctrl: fix pin funtion can not be match correctly.
From: Maxime Ripard @ 2017-12-13 15:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213144748.GA18267@arx-s1>
Hi,
Thanks for your patch!
On Wed, Dec 13, 2017 at 10:47:48PM +0800, hao_zhang wrote:
> Pin function can not be match correctly when SUNXI_PIN describe with
> mutiple variant and same function.
>
> such as:
> on pinctrl-sun4i-a10.c
>
> SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 2),
> SUNXI_FUNCTION(0x0, "gpio_in"),
> SUNXI_FUNCTION(0x1, "gpio_out"),
> SUNXI_FUNCTION_VARIANT(0x2, "pwm", /* PWM0 */
> PINCTRL_SUN4I_A10 |
> PINCTRL_SUN7I_A20),
> SUNXI_FUNCTION_VARIANT(0x3, "pwm", /* PWM0 */
> PINCTRL_SUN8I_R40)),
>
> it would always match to the first variant function
> (PINCTRL_SUN4I_A10, PINCTRL_SUN7I_A20)
>
> so we should add variant compare on it.
>
> Signed-off-by: hao_zhang <hao5781286@gmail.com>
> ---
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 4b6cb25..f23e74e 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -83,9 +83,11 @@ sunxi_pinctrl_desc_find_function_by_name(struct sunxi_pinctrl *pctl,
> struct sunxi_desc_function *func = pin->functions;
>
> while (func->name) {
> - if (!strcmp(func->name, func_name))
> + if (!strcmp(func->name, func_name)) {
> + if (!(func->variant) ||
> + (func->variant & pctl->variant))
I guess it would be better to have:
if (!strcmp(func->name, func_name) &&
(!func->variant || (func->variant & pctl->variant)))
Once fixed,
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/5c4188c3/attachment.sig>
^ permalink raw reply
* [PATCH v4 1/4] dt-bindings: pwm: binding allwinner sun8i R40/V40/T3.
From: Maxime Ripard @ 2017-12-13 15:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213144309.GA18167@arx-s1>
Hi,
On Wed, Dec 13, 2017 at 10:43:09PM +0800, hao_zhang wrote:
> This patch adds allwinner R40, V40, T3 pwm binding documents.
>
> Signed-off-by: hao_zhang <hao5781286@gmail.com>
> ---
> Documentation/devicetree/bindings/pwm/pwm-sun8i.txt | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
> new file mode 100644
> index 0000000..76750d8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
> @@ -0,0 +1,18 @@
> +Allwinner sun8i R40/V40/T3 SoC PWM controller
> +
> +Required properties:
> +- compatible: should be one of:
> +- "allwinner,sun8i-r40-pwm"
Please indent that part.
Once fixed,
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/9a73189c/attachment.sig>
^ permalink raw reply
* [PATCH v2 07/19] arm64: insn: Add encoder for bitwise operations using litterals
From: Marc Zyngier @ 2017-12-13 15:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5A314AFF.60300@arm.com>
On 13/12/17 15:45, James Morse wrote:
> Hi Marc,
>
> On 13/12/17 14:32, Marc Zyngier wrote:
>> On 12/12/17 18:32, James Morse wrote:
>>> On 11/12/17 14:49, Marc Zyngier wrote:
>>>> We lack a way to encode operations such as AND, ORR, EOR that take
>>>> an immediate value. Doing so is quite involved, and is all about
>>>> reverse engineering the decoding algorithm described in the
>>>> pseudocode function DecodeBitMasks().
>
>>>> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
>>>> index 7e432662d454..326b17016485 100644
>>>> --- a/arch/arm64/kernel/insn.c
>>>> +++ b/arch/arm64/kernel/insn.c
>>>
>>>> +static u32 aarch64_encode_immediate(u64 imm,
>>>> + enum aarch64_insn_variant variant,
>>>> + u32 insn)
>>>> +{
>>>> + unsigned int immr, imms, n, ones, ror, esz, tmp;
>>>> + u64 mask;
>>>
>>> [...]
>>>
>>>> + /* Compute the rotation */
>>>> + if (range_of_ones(imm)) {
>>>> + /*
>>>> + * Pattern: 0..01..10..0
>>>> + *
>>>> + * Compute how many rotate we need to align it right
>>>> + */
>>>> + ror = ffs(imm) - 1;
>>>
>>> (how come range_of_ones() uses __ffs64() on the same value?)
>>
>> News flash: range_of_ones is completely buggy. It will fail on the
>> trivial value 1 (__ffs64(1) = 0; 0 - 1 = -1; val >> -1 is... ermmmm).
>> I definitely got mixed up between the two.
>
> They do different things!? Aaaaaahhhh....
>
> [ ...]
>
>>> Unless I've gone wrong, I think the 'Trim imm to the element size' code needs to
>>> move up into the esz-reducing loop so it doesn't happen for a 64bit immediate.
>
>
>> Yup. I've stashed the following patch:
>>
>> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
>> index b8fb2d89b3a6..e58be1c57f18 100644
>> --- a/arch/arm64/kernel/insn.c
>> +++ b/arch/arm64/kernel/insn.c
>> @@ -1503,8 +1503,7 @@ pstate_check_t * const aarch32_opcode_cond_checks[16] = {
>> static bool range_of_ones(u64 val)
>> {
>> /* Doesn't handle full ones or full zeroes */
>> - int x = __ffs64(val) - 1;
>> - u64 sval = val >> x;
>> + u64 sval = val >> __ffs64(val);
>>
>> /* One of Sean Eron Anderson's bithack tricks */
>> return ((sval + 1) & (sval)) == 0;
>> @@ -1515,7 +1514,7 @@ static u32 aarch64_encode_immediate(u64 imm,
>> u32 insn)
>> {
>> unsigned int immr, imms, n, ones, ror, esz, tmp;
>> - u64 mask;
>> + u64 mask = ~0UL;
>>
>> /* Can't encode full zeroes or full ones */
>> if (!imm || !~imm)
>> @@ -1543,8 +1542,12 @@ static u32 aarch64_encode_immediate(u64 imm,
>> for (tmp = esz; tmp > 2; tmp /= 2) {
>> u64 emask = BIT(tmp / 2) - 1;
>>
>> - if ((imm & emask) != ((imm >> (tmp / 2)) & emask))
>> + if ((imm & emask) != ((imm >> (tmp / 2)) & emask)) {
>> + /* Trim imm to the element size */
>> + mask = BIT(esz - 1) - 1;
>> + imm &= mask;
>
> Won't this still lose the top bit? It generates 0x7fffffff for esz=32, and for
> esz=32 we run through here when the two 16bit values are different.
>
> This still runs for a 64bit immediate. The 0xf80000000fffffff example compares
> 0xf8000000 with 0fffffff then breaks here on the first iteration of this loop.
> With this change it still attempts to generate a 64bit mask.
>
> I was thinking of something like [0]. That only runs when we know the two
> tmp:halves match, it just keeps the bottom tmp:half for the next run and never
> runs for a 64bit immediate.
You're right. Again. And I can't think. That's it, I'm implementing the
testing rig.
> [0] Not even built:
> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> index 12d3ec2154c2..d9fbdea7b18d 100644
> --- a/arch/arm64/kernel/insn.c
> +++ b/arch/arm64/kernel/insn.c
> @@ -1529,15 +1529,15 @@ static u32 aarch64_encode_immediate(u64 imm,
> break;
>
> esz = tmp;
> +
> + /* Trim imm to the element size */
> + mask = BIT(esz) - 1;
> + imm &= mask;
> }
>
> /* N is only set if we're encoding a 64bit value */
> n = esz == 64;
>
> - /* Trim imm to the element size */
> - mask = BIT(esz - 1) - 1;
> - imm &= mask;
> -
> /* That's how many ones we need to encode */
> ones = hweight64(imm);
This is definitely much better.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH] IPI performance benchmark
From: Konrad Rzeszutek Wilk @ 2017-12-13 15:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213104747.m4b5pbnlcroodfe7@yury-thinkpad>
On Wed, Dec 13, 2017 at 01:47:47PM +0300, Yury Norov wrote:
> On Mon, Dec 11, 2017 at 10:33:32AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Mon, Dec 11, 2017 at 05:16:00PM +0300, Yury Norov wrote:
> > > This benchmark sends many IPIs in different modes and measures
> > > time for IPI delivery (first column), and total time, ie including
> > > time to acknowledge the receive by sender (second column).
> > >
> > > The scenarios are:
> > > Dry-run: do everything except actually sending IPI. Useful
> > > to estimate system overhead.
> > > Self-IPI: Send IPI to self CPU.
> > > Normal IPI: Send IPI to some other CPU.
> > > Broadcast IPI: Send broadcast IPI to all online CPUs.
> > >
> > > For virtualized guests, sending and reveiving IPIs causes guest exit.
> > > I used this test to measure performance impact on KVM subsystem of
> > > Christoffer Dall's series "Optimize KVM/ARM for VHE systems".
> > >
> > > https://www.spinics.net/lists/kvm/msg156755.html
> > >
> > > Test machine is ThunderX2, 112 online CPUs. Below the results normalized
> > > to host dry-run time. Smaller - better.
> >
> > Would it make sense to also add spinlock contention tests? Meaning make
> > this framework a bit more generic so you could do IPI and you could
> > also do spinlock contention?
> >
> > Like:
> > http://xenbits.xen.org/gitweb/?p=xentesttools/bootstrap.git;a=blob;f=root_image/drivers/spinlock_hog/spinlock_hog.c;h=040a154808452576b1aa5720a6282981319a5360;hb=HEAD
>
> There's kernel/locking/locktorture.c for spinlock testing. Maybe it
> worth to add new testcase there? If you find my 'framework' more
> suitable for you, I'm also OK with it. Is my understanding correct
> that you want something like broadcast IPI case, but with different
> payload?
Yes, exactly!
But as I said, you have probably other things on your mind so this
is more of - 'it would be cool if you could do it, but if you don't
get to it - I understand' type.
^ permalink raw reply
* [PATCH v4 2/4] ARM: PWM: add allwinner sun8i R40/V40/T3 pwm support.
From: Maxime Ripard @ 2017-12-13 15:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213144446.GA18217@arx-s1>
Hi,
Thanks for your patch.
It looks mostly good except for a few things below:
On Wed, Dec 13, 2017 at 10:44:46PM +0800, hao_zhang wrote:
> This patch add allwinner sun8i R40/V40/T3 pwm support.
>
> Signed-off-by: hao_zhang <hao5781286@gmail.com>
> ---
> drivers/pwm/Kconfig | 10 +
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-sun8i-r40.c | 449 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 460 insertions(+)
> create mode 100644 drivers/pwm/pwm-sun8i-r40.c
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 763ee50..cde5a70 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -444,6 +444,16 @@ config PWM_SUN4I
> To compile this driver as a module, choose M here: the module
> will be called pwm-sun4i.
>
> +config PWM_SUN8I_R40
> + tristate "Allwinner PWM SUN8I R40 support"
> + depends on ARCH_SUNXI || COMPILE_TEST
> + depends on HAS_IOMEM && COMMON_CLK
> + help
> + Generic PWM framework driver for Allwinner SoCs R40, V40, T3.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called pwm-sun8i-r40.
> +
> config PWM_TEGRA
> tristate "NVIDIA Tegra PWM support"
> depends on ARCH_TEGRA
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 0258a74..026a55b 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_PWM_STM32) += pwm-stm32.o
> obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.o
> obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o
> obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o
> +obj-$(CONFIG_PWM_SUN8I_R40) += pwm-sun8i-r40.o
> obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o
> obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
> obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
> diff --git a/drivers/pwm/pwm-sun8i-r40.c b/drivers/pwm/pwm-sun8i-r40.c
> new file mode 100644
> index 0000000..8df538d
> --- /dev/null
> +++ b/drivers/pwm/pwm-sun8i-r40.c
> @@ -0,0 +1,449 @@
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/time.h>
> +
> +#define PWM_IRQ_ENABLE_REG 0x0000
> +#define PCIE(ch) BIT(ch)
> +
> +#define PWM_IRQ_STATUS_REG 0x0004
> +#define PIS(ch) BIT(ch)
> +
> +#define CAPTURE_IRQ_ENABLE_REG 0x0010
> +#define CFIE(ch) BIT(ch << 1 + 1)
> +#define CRIE(ch) BIT(ch << 1)
> +
> +#define CAPTURE_IRQ_STATUS_REG 0x0014
> +#define CFIS(ch) BIT(ch << 1 + 1)
> +#define CRIS(ch) BIT(ch << 1)
> +
> +#define CLK_CFG_REG(ch) (0x0020 + (ch >> 1) * 4)
> +#define CLK_SRC BIT(7)
> +#define CLK_SRC_BYPASS_SEC BIT(6)
> +#define CLK_SRC_BYPASS_FIR BIT(5)
> +#define CLK_GATING BIT(4)
> +#define CLK_DIV_M GENMASK(3, 0)
> +
> +#define PWM_DZ_CTR_REG(ch) (0x0030 + (ch >> 1) * 4)
> +#define PWM_DZ_INTV GENMASK(15, 8)
> +#define PWM_DZ_EN BIT(0)
> +
> +#define PWM_ENABLE_REG 0x0040
> +#define PWM_EN(ch) BIT(ch)
> +
> +#define CAPTURE_ENABLE_REG 0x0044
> +#define CAP_EN(ch) BIT(ch)
> +
> +#define PWM_CTR_REG(ch) (0x0060 + ch * 0x20)
> +#define PWM_PERIOD_RDY BIT(11)
> +#define PWM_PUL_START BIT(10)
> +#define PWM_MODE BIT(9)
> +#define PWM_ACT_STA BIT(8)
> +#define PWM_PRESCAL_K GENMASK(7, 0)
> +
> +#define PWM_PERIOD_REG(ch) (0x0064 + ch * 0x20)
> +#define PWM_ENTIRE_CYCLE GENMASK(31, 16)
> +#define PWM_ACT_CYCLE GENMASK(15, 0)
> +
> +#define PWM_CNT_REG(ch) (0x0068 + ch * 0x20)
> +#define PWM_CNT_VAL GENMASK(15, 0)
> +
> +#define CAPTURE_CTR_REG(ch) (0x006c + ch * 0x20)
> +#define CAPTURE_CRLF BIT(2)
> +#define CAPTURE_CFLF BIT(1)
> +#define CAPINV BIT(0)
> +
> +#define CAPTURE_RISE_REG(ch) (0x0070 + ch * 0x20)
> +#define CAPTURE_CRLR GENMASK(15, 0)
> +
> +#define CAPTURE_FALL_REG(ch) (0x0074 + ch * 0x20)
> +#define CAPTURE_CFLR GENMASK(15, 0)
> +
> +struct sunxi_pwm_data {
You should use the sun8i_pwm prefix (including r40 if you want).
> +static inline u32 sunxi_pwm_readl(struct sunxi_pwm_chip *chip,
> + unsigned long offset)
> +{
> + return readl(chip->base + offset);
> +}
> +
> +static inline void sunxi_pwm_writel(struct sunxi_pwm_chip *chip,
> + u32 val, unsigned long offset)
> +{
> + writel(val, chip->base + offset);
> +}
> +
> +static void sunxi_pwm_set_bit(struct sunxi_pwm_chip *sunxi_pwm,
> + unsigned long reg, u32 bit)
> +{
> + u32 val;
> +
> + val = sunxi_pwm_readl(sunxi_pwm, reg);
> + val |= bit;
> + sunxi_pwm_writel(sunxi_pwm, val, reg);
> +}
> +
> +static void sunxi_pwm_clear_bit(struct sunxi_pwm_chip *sunxi_pwm,
> + unsigned long reg, u32 bit)
> +{
> + u32 val;
> +
> + val = sunxi_pwm_readl(sunxi_pwm, reg);
> + val &= ~bit;
> + sunxi_pwm_writel(sunxi_pwm, val, reg);
> +}
> +
> +static void sunxi_pwm_set_value(struct sunxi_pwm_chip *sunxi_pwm,
> + unsigned long reg, u32 mask, u32 val)
> +{
> + u32 tmp;
> +
> + tmp = sunxi_pwm_readl(sunxi_pwm, reg);
> + tmp &= ~mask;
> + tmp |= mask & val;
> + sunxi_pwm_writel(sunxi_pwm, tmp, reg);
> +}
> +
> +static void sunxi_pwm_set_polarity(struct sunxi_pwm_chip *chip, u32 ch,
> + enum pwm_polarity polarity)
> +{
> + if (polarity == PWM_POLARITY_NORMAL)
> + sunxi_pwm_set_bit(chip, PWM_CTR_REG(ch), PWM_ACT_STA);
> + else
> + sunxi_pwm_clear_bit(chip, PWM_CTR_REG(ch), PWM_ACT_STA);
> +}
> +
> +static void sunxi_dump_reg(struct sunxi_pwm_chip *sunxi_pwm, u8 ch)
> +{
> + dev_dbg(sunxi_pwm->chip.dev, "ch: %d\n"
> + "\tPWM_IRQ_ENABLE_REG(%04x): \t0x%08x\n"
> + "\tPWM_IRQ_STATUS_REG(%04x): \t0x%08x\n"
> + "\tCAPTURE_IRQ_ENABLE_REG(%04x): \t0x%08x\n"
> + "\tCAPTURE_IRQ_STATUS_REG(%04x): \t0x%08x\n"
> + "\tCLK_CFG_REG(%04x)(%d): \t0x%08x\n"
> + "\tPWM_DZ_CTR_REG(%04x)(%d): \t0x%08x\n"
> + "\tPWM_ENABLE_REG(%04x): \t0x%08x\n"
> + "\tCAPTURE_ENABLE_REG(%04x): \t0x%08x\n"
> + "\tPWM_CTR_REG(%04x)(%d): \t0x%08x\n"
> + "\tPWM_PERIOD_REG(%04x)(%d): \t0x%08x\n"
> + "\tPWM_CNT_REG(%04x)(%d): \t0x%08x\n"
> + "\tCAPTURE_CTR_REG(%04x)(%d): \t0x%08x\n"
> + "\tCAPTURE_RISE_REG(%04x)(%d): \t0x%08x\n"
> + "\tCAPTURE_FALL_REG(%04x)(%d): \t0x%08x\n",
> + ch,
> + PWM_IRQ_ENABLE_REG,
> + readl(sunxi_pwm->base + PWM_IRQ_ENABLE_REG),
> + PWM_IRQ_STATUS_REG,
> + readl(sunxi_pwm->base + PWM_IRQ_STATUS_REG),
> + CAPTURE_IRQ_ENABLE_REG,
> + readl(sunxi_pwm->base + CAPTURE_IRQ_ENABLE_REG),
> + CAPTURE_IRQ_STATUS_REG,
> + readl(sunxi_pwm->base + CAPTURE_IRQ_STATUS_REG),
> + CLK_CFG_REG(ch),
> + ch, readl(sunxi_pwm->base + CLK_CFG_REG(ch)),
> + PWM_DZ_CTR_REG(ch),
> + ch, readl(sunxi_pwm->base + PWM_DZ_CTR_REG(ch)),
> + PWM_ENABLE_REG,
> + readl(sunxi_pwm->base + PWM_ENABLE_REG),
> + CAPTURE_ENABLE_REG,
> + readl(sunxi_pwm->base + CAPTURE_ENABLE_REG),
> + PWM_CTR_REG(ch),
> + ch, readl(sunxi_pwm->base + PWM_CTR_REG(ch)),
> + PWM_PERIOD_REG(ch),
> + ch, readl(sunxi_pwm->base + PWM_PERIOD_REG(ch)),
> + PWM_CNT_REG(ch),
> + ch, readl(sunxi_pwm->base + PWM_CNT_REG(ch)),
> + CAPTURE_CTR_REG(ch),
> + ch, readl(sunxi_pwm->base + CAPTURE_CTR_REG(ch)),
> + CAPTURE_RISE_REG(ch),
> + ch, readl(sunxi_pwm->base + CAPTURE_RISE_REG(ch)),
> + CAPTURE_FALL_REG(ch),
> + ch, readl(sunxi_pwm->base + CAPTURE_FALL_REG(ch)));
> +}
You should really consider using a regmap here. It provides all the
accessors you have defined above, you can read the regmap content
directly from debugfs without recompiling your kernel, and you have
ftrace events as well to trace the read / writes as they happen. All
of that for free, without having to maintain it in your driver :)
> +static int sunxi_pwm_config(struct sunxi_pwm_chip *sunxi_pwm, u8 ch,
> + struct pwm_state *state)
> +{
> + u64 clk_rate, clk_div, val;
> + u16 prescaler = 0;
> + u8 id = 0;
> +
> + clk_rate = clk_get_rate(sunxi_pwm->clk);
> + dev_dbg(sunxi_pwm->chip.dev, "clock rate:%lld\n", clk_rate);
This is also something you can retrieve through ftrace events.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/e0da1db0/attachment.sig>
^ permalink raw reply
* [PATCH v4 3/4] ARM: dts: add pwm node for r40.
From: Maxime Ripard @ 2017-12-13 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213144628.GA18246@arx-s1>
Hi,
On Wed, Dec 13, 2017 at 10:46:28PM +0800, hao_zhang wrote:
> This patch add pwm node for r40.
>
> Signed-off-by: hao_zhang <hao5781286@gmail.com>
> ---
> arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 6 ++++++
> arch/arm/boot/dts/sun8i-r40.dtsi | 13 +++++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> index 8c5efe2..6cf6273 100644
> --- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> +++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> @@ -196,6 +196,12 @@
> status = "okay";
> };
>
> +&pwm {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwm_pins>;
> + status = "okay";
> +};
> +
Can you split this part into a separate patch?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/c23a1b7b/attachment.sig>
^ permalink raw reply
* [PATCH 2/3] reset: meson-axg: add compatible string for Meson-AXG SoC
From: Philipp Zabel @ 2017-12-13 16:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3b61170c-c352-6b23-ab19-82c76954a911@amlogic.com>
Hi Yixun,
On Wed, 2017-12-13 at 22:07 +0800, Yixun Lan wrote:
> Hi Philipp
>
> On 11/10/2017 04:46 PM, Yixun Lan wrote:
> > Try to add compatible string explictly to support new Meson-AXG SoC.
> >
> > Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> > ---
> > drivers/reset/reset-meson.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
> > index c419a3753d00..93cbee1ae8ef 100644
> > --- a/drivers/reset/reset-meson.c
> > +++ b/drivers/reset/reset-meson.c
> > @@ -139,6 +139,8 @@ static const struct of_device_id meson_reset_dt_ids[] = {
> > .data = &meson_reset_meson8_ops, },
> > { .compatible = "amlogic,meson-gxbb-reset",
> > .data = &meson_reset_gx_ops, },
> > + { .compatible = "amlogic,meson-axg-reset",
> > + .data = &meson_reset_gx_ops, },
> > { /* sentinel */ },
> > };
> >
> >
>
> it's generally a ping to the status of these two patches[1], are they
> ready to go? or do you have any comment? or do you want me to send
> another version with Neil's Reviewed-by added[1]
I forgot to update you on the status. I have put both patches on the
reset/next branch with Neil's R-b, they are in linux-next:
0e5721f76252 ("reset: meson-axg: add compatible string for Meson-AXG SoC")
c16292578ffa ("dt-bindings: reset: Add bindings for the Meson-AXG SoC Reset Controller")
I'll include them with the next pull request.
regards
Philipp
^ permalink raw reply
* [PATCH 1/8] drm/sun4i: backend: Move line stride setup to buffer setup function
From: Neil Armstrong @ 2017-12-13 16:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bf7958df88df8014866fc2dcacfeb1bea45a5659.1513178989.git-series.maxime.ripard@free-electrons.com>
On 13/12/2017 16:33, Maxime Ripard wrote:
> Setup the line stride in the buffer setup function, since it's tied to the
> buffer itself, and is not needed when we do not set the buffer in the
> backend.
>
> This is for example the case when using the frontend and then routing its
> output to the backend.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_backend.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index 847eecbe4d14..c99d1a7e815a 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -141,7 +141,6 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
> int layer, struct drm_plane *plane)
> {
> struct drm_plane_state *state = plane->state;
> - struct drm_framebuffer *fb = state->fb;
>
> DRM_DEBUG_DRIVER("Updating layer %d\n", layer);
>
> @@ -153,12 +152,6 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
> state->crtc_h));
> }
>
> - /* Set the line width */
> - DRM_DEBUG_DRIVER("Layer line width: %d bits\n", fb->pitches[0] * 8);
> - regmap_write(backend->engine.regs,
> - SUN4I_BACKEND_LAYLINEWIDTH_REG(layer),
> - fb->pitches[0] * 8);
> -
> /* Set height and width */
> DRM_DEBUG_DRIVER("Layer size W: %u H: %u\n",
> state->crtc_w, state->crtc_h);
> @@ -218,6 +211,13 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> u32 lo_paddr, hi_paddr;
> dma_addr_t paddr;
>
> + /* Set the line width */
> + DRM_DEBUG_DRIVER("Layer line width: %d bits\n", fb->pitches[0] * 8);
> + regmap_write(backend->engine.regs,
> + SUN4I_BACKEND_LAYLINEWIDTH_REG(layer),
> + fb->pitches[0] * 8);
> +
> +
> /* Get the start of the displayed memory */
> paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
> DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [PATCH 2/8] drm/sun4i: backend: Allow a NULL plane pointer to retrieve the format
From: Neil Armstrong @ 2017-12-13 16:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6a1686760e61baa9f21fd2f12e56a8de92eb1714.1513178989.git-series.maxime.ripard@free-electrons.com>
On 13/12/2017 16:33, Maxime Ripard wrote:
> The function converting the DRM format to its equivalent in the backend
> registers was assuming that we were having a plane.
>
> However, we might want to use that function when setting up a plane using
> the frontend, in which case we will not have a plane associated to the
> backend's layer. Yet, we still need to setup the format to the one output
> by the frontend.
>
> Test for NULL plane pointers before referencing them, so that we can work
> around it.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_backend.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index c99d1a7e815a..f971d3fb5ee4 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -93,7 +93,7 @@ void sun4i_backend_layer_enable(struct sun4i_backend *backend,
> static int sun4i_backend_drm_format_to_layer(struct drm_plane *plane,
> u32 format, u32 *mode)
> {
> - if ((plane->type == DRM_PLANE_TYPE_PRIMARY) &&
> + if (plane && (plane->type == DRM_PLANE_TYPE_PRIMARY) &&
> (format == DRM_FORMAT_ARGB8888))
> format = DRM_FORMAT_XRGB8888;
>
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [PATCH 05/12] arm64: mm: Remove VMALLOC checks from update_mapping_prot(.)
From: Catalin Marinas @ 2017-12-13 16:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212153923.a7z6kcowxj7thljy@capper-debian.cambridge.arm.com>
On Tue, Dec 12, 2017 at 03:39:23PM +0000, Steve Capper wrote:
> It is not apparent to me how mark_linear_text_alias_ro(.) guarantees
> that no page table entries for the linear map are split, though.
map_mem() ensures that when mapped via __map_memblock(), no contiguous
entries are created. Also both ends of the _text..__init_begin would be
mapped to the granularity permitted by their alignment so that no later
splitting is necessary.
--
Catalin
^ permalink raw reply
* [PATCH 3/8] drm/sun4i: sun4i_layer: Add a custom plane state
From: Neil Armstrong @ 2017-12-13 16:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8ecbf0bda89d11539dbf9357f0fe77561ebdd4c3.1513178989.git-series.maxime.ripard@free-electrons.com>
On 13/12/2017 16:33, Maxime Ripard wrote:
> We will need to store some additional data in the future to the state.
> Create a custom plane state that will embed those data, in order to store
> the pipe or whether or not that plane should use the frontend.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_layer.c | 48 ++++++++++++++++++++++++++++--
> drivers/gpu/drm/sun4i/sun4i_layer.h | 10 ++++++-
> 2 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index 7bddf12548d3..c3afcf888906 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -25,6 +25,48 @@ struct sun4i_plane_desc {
> uint32_t nformats;
> };
>
> +static void sun4i_backend_layer_reset(struct drm_plane *plane)
> +{
> + struct sun4i_layer_state *state;
> +
> + if (plane->state) {
> + state = state_to_sun4i_layer_state(plane->state);
> +
> + __drm_atomic_helper_plane_destroy_state(&state->state);
Maybe a blank line here ?
> + kfree(state);
> + plane->state = NULL;
> + }
> +
> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> + if (state) {
> + plane->state = &state->state;
> + plane->state->plane = plane;
> + }
> +}
> +
> +static struct drm_plane_state *
> +sun4i_backend_layer_duplicate_state(struct drm_plane *plane)
> +{
> + struct sun4i_layer_state *copy;
> +
> + copy = kzalloc(sizeof(*copy), GFP_KERNEL);
> + if (!copy)
> + return NULL;
> +
> + __drm_atomic_helper_plane_duplicate_state(plane, ©->state);
> +
> + return ©->state;
> +}
> +
> +static void sun4i_backend_layer_destroy_state(struct drm_plane *plane,
> + struct drm_plane_state *state)
> +{
> + struct sun4i_layer_state *s_state = state_to_sun4i_layer_state(state);
> +
> + __drm_atomic_helper_plane_destroy_state(state);
You can add a blank line here
> + kfree(s_state);
> +}
> +
> static void sun4i_backend_layer_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> @@ -52,11 +94,11 @@ static const struct drm_plane_helper_funcs sun4i_backend_layer_helper_funcs = {
> };
>
> static const struct drm_plane_funcs sun4i_backend_layer_funcs = {
> - .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> - .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> + .atomic_destroy_state = sun4i_backend_layer_destroy_state,
> + .atomic_duplicate_state = sun4i_backend_layer_duplicate_state,
> .destroy = drm_plane_cleanup,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .reset = drm_atomic_helper_plane_reset,
> + .reset = sun4i_backend_layer_reset,
> .update_plane = drm_atomic_helper_update_plane,
> };
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.h b/drivers/gpu/drm/sun4i/sun4i_layer.h
> index 4e84f438b346..d2c19348d1b0 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.h
> @@ -22,12 +22,22 @@ struct sun4i_layer {
> int id;
> };
>
> +struct sun4i_layer_state {
> + struct drm_plane_state state;
> +};
> +
> static inline struct sun4i_layer *
> plane_to_sun4i_layer(struct drm_plane *plane)
> {
> return container_of(plane, struct sun4i_layer, plane);
> }
>
> +static inline struct sun4i_layer_state *
> +state_to_sun4i_layer_state(struct drm_plane_state *state)
> +{
> + return container_of(state, struct sun4i_layer_state, state);
> +}
> +
> struct drm_plane **sun4i_layers_init(struct drm_device *drm,
> struct sunxi_engine *engine);
>
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [PATCH 4/8] drm/sun4i: crtc: Add a custom crtc atomic_check
From: Neil Armstrong @ 2017-12-13 16:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f7ce67ed8b926609a9d9290696a036b103be233c.1513178989.git-series.maxime.ripard@free-electrons.com>
On 13/12/2017 16:33, Maxime Ripard wrote:
> We have some restrictions on what the planes and CRTC can provide that are
> tied to only one generation of display engines.
>
> For example, on the first generation, we can only have one YUV plane or one
> plane that uses the frontend output.
>
> Let's allow our engines to provide an atomic_check callback to validate the
> current configuration.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_crtc.c | 14 ++++++++++++++
> drivers/gpu/drm/sun4i/sunxi_engine.h | 2 ++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> index 5decae0069d0..2a565325714f 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> @@ -46,6 +46,19 @@ static struct drm_encoder *sun4i_crtc_get_encoder(struct drm_crtc *crtc)
> return NULL;
> }
>
> +static int sun4i_crtc_atomic_check(struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> +{
> + struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
> + struct sunxi_engine *engine = scrtc->engine;
> + int ret = 0;
> +
> + if (engine && engine->ops && engine->ops->atomic_check)
> + ret = engine->ops->atomic_check(engine, state);
> +
> + return ret;
> +}
> +
> static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc,
> struct drm_crtc_state *old_state)
> {
> @@ -125,6 +138,7 @@ static void sun4i_crtc_mode_set_nofb(struct drm_crtc *crtc)
> }
>
> static const struct drm_crtc_helper_funcs sun4i_crtc_helper_funcs = {
> + .atomic_check = sun4i_crtc_atomic_check,
> .atomic_begin = sun4i_crtc_atomic_begin,
> .atomic_flush = sun4i_crtc_atomic_flush,
> .atomic_enable = sun4i_crtc_atomic_enable,
> diff --git a/drivers/gpu/drm/sun4i/sunxi_engine.h b/drivers/gpu/drm/sun4i/sunxi_engine.h
> index 4cb70ae65c79..42655230aeba 100644
> --- a/drivers/gpu/drm/sun4i/sunxi_engine.h
> +++ b/drivers/gpu/drm/sun4i/sunxi_engine.h
> @@ -16,6 +16,8 @@ struct drm_device;
> struct sunxi_engine;
>
> struct sunxi_engine_ops {
> + int (*atomic_check)(struct sunxi_engine *engine,
> + struct drm_crtc_state *state);
> void (*commit)(struct sunxi_engine *engine);
> struct drm_plane **(*layers_init)(struct drm_device *drm,
> struct sunxi_engine *engine);
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [PATCH 5/8] drm/sun4i: Add a driver for the display frontend
From: Neil Armstrong @ 2017-12-13 16:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <600d29233eb0dcca3af815fbed8995e35f78b4ee.1513178989.git-series.maxime.ripard@free-electrons.com>
On 13/12/2017 16:33, Maxime Ripard wrote:
> The display frontend is an hardware block that can be used to implement
> some more advanced features like hardware scaling or colorspace
> conversions. It can also be used to implement the output format of the VPU.
>
> Let's create a minimal driver for it that will only enable the hardware
> scaling features.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/Makefile | 3 +-
> drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
> drivers/gpu/drm/sun4i/sun4i_backend.h | 1 +-
> drivers/gpu/drm/sun4i/sun4i_drv.c | 16 +-
> drivers/gpu/drm/sun4i/sun4i_drv.h | 1 +-
> drivers/gpu/drm/sun4i/sun4i_frontend.c | 377 ++++++++++++++++++++++++++-
> drivers/gpu/drm/sun4i/sun4i_frontend.h | 102 +++++++-
> 7 files changed, 500 insertions(+), 5 deletions(-)
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_frontend.c
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_frontend.h
>
> diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
> index 0c2f8c7facae..b660d82011f4 100644
> --- a/drivers/gpu/drm/sun4i/Makefile
> +++ b/drivers/gpu/drm/sun4i/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> sun4i-backend-y += sun4i_backend.o sun4i_layer.o
> +sun4i-frontend-y += sun4i_frontend.o
>
> sun4i-drm-y += sun4i_drv.o
> sun4i-drm-y += sun4i_framebuffer.o
> @@ -21,6 +22,6 @@ obj-$(CONFIG_DRM_SUN4I) += sun4i-tcon.o
> obj-$(CONFIG_DRM_SUN4I) += sun4i_tv.o
> obj-$(CONFIG_DRM_SUN4I) += sun6i_drc.o
>
> -obj-$(CONFIG_DRM_SUN4I_BACKEND) += sun4i-backend.o
> +obj-$(CONFIG_DRM_SUN4I_BACKEND) += sun4i-backend.o sun4i-frontend.o
> obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o
> obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index f971d3fb5ee4..e83e1fe43823 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -367,6 +367,11 @@ static int sun4i_backend_bind(struct device *dev, struct device *master,
> if (backend->engine.id < 0)
> return backend->engine.id;
>
> + backend->frontend = sun4i_backend_find_frontend(drv, dev->of_node);
> + if (IS_ERR(backend->frontend)) {
> + dev_err(dev, "Couldn't find matching frontend, frontend features disabled\n");
Maybe a dev_warn ?
> + }
> +
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> regs = devm_ioremap_resource(dev, res);
> if (IS_ERR(regs))
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index ac3cc029f5cd..ba1410fd5410 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -145,6 +145,7 @@
>
> struct sun4i_backend {
> struct sunxi_engine engine;
> + struct sun4i_frontend *frontend;
>
> struct reset_control *reset;
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 75c76cdd82bc..17bf9bfd98ba 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -98,6 +98,7 @@ static int sun4i_drv_bind(struct device *dev)
> goto free_drm;
> }
> drm->dev_private = drv;
> + INIT_LIST_HEAD(&drv->frontend_list);
> INIT_LIST_HEAD(&drv->engine_list);
> INIT_LIST_HEAD(&drv->tcon_list);
>
> @@ -239,9 +240,11 @@ static int sun4i_drv_add_endpoints(struct device *dev,
> int count = 0;
>
> /*
> - * We don't support the frontend for now, so we will never
> - * have a device bound. Just skip over it, but we still want
> - * the rest our pipeline to be added.
> + * The frontend has been disabled in all of our old device
> + * trees. If we find a node that is the frontend and is
> + * disabled, we should just follow through and parse its
> + * child, but without adding it to the component list.
> + * Otherwise, we obviously want to add it to the list.
> */
> if (!sun4i_drv_node_is_frontend(node) &&
> !of_device_is_available(node))
> @@ -254,7 +257,12 @@ static int sun4i_drv_add_endpoints(struct device *dev,
> if (sun4i_drv_node_is_connector(node))
> return 0;
>
> - if (!sun4i_drv_node_is_frontend(node)) {
> + /*
> + * If the device is either just a regular device, or an
> + * enabled frontend, we add it to our component list.
> + */
> + if (!sun4i_drv_node_is_frontend(node) ||
> + (sun4i_drv_node_is_frontend(node) && of_device_is_available(node))) {
> /* Add current component */
> DRM_DEBUG_DRIVER("Adding component %pOF\n", node);
> drm_of_component_match_add(dev, match, compare_of, node);
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.h b/drivers/gpu/drm/sun4i/sun4i_drv.h
> index a960c89270cc..9c26a345f85c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.h
> @@ -19,6 +19,7 @@
>
> struct sun4i_drv {
> struct list_head engine_list;
> + struct list_head frontend_list;
> struct list_head tcon_list;
>
> struct drm_fbdev_cma *fbdev;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c
> new file mode 100644
> index 000000000000..1be0e86d1457
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c
> @@ -0,0 +1,377 @@
// SPDX-Licence-Identifier... new format ?
> +/*
> + * Copyright (C) 2017 Free Electrons
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +#include <drm/drmP.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include "sun4i_drv.h"
> +#include "sun4i_frontend.h"
> +
> +static const u32 sun4i_frontend_vert_coef[32] = {
> + 0x00004000, 0x000140ff, 0x00033ffe, 0x00043ffd,
> + 0x00063efc, 0xff083dfc, 0x000a3bfb, 0xff0d39fb,
> + 0xff0f37fb, 0xff1136fa, 0xfe1433fb, 0xfe1631fb,
> + 0xfd192ffb, 0xfd1c2cfb, 0xfd1f29fb, 0xfc2127fc,
> + 0xfc2424fc, 0xfc2721fc, 0xfb291ffd, 0xfb2c1cfd,
> + 0xfb2f19fd, 0xfb3116fe, 0xfb3314fe, 0xfa3611ff,
> + 0xfb370fff, 0xfb390dff, 0xfb3b0a00, 0xfc3d08ff,
> + 0xfc3e0600, 0xfd3f0400, 0xfe3f0300, 0xff400100,
> +};
> +
> +static const u32 sun4i_frontend_horz_coef[64] = {
> + 0x40000000, 0x00000000, 0x40fe0000, 0x0000ff03,
> + 0x3ffd0000, 0x0000ff05, 0x3ffc0000, 0x0000ff06,
> + 0x3efb0000, 0x0000ff08, 0x3dfb0000, 0x0000ff09,
> + 0x3bfa0000, 0x0000fe0d, 0x39fa0000, 0x0000fe0f,
> + 0x38fa0000, 0x0000fe10, 0x36fa0000, 0x0000fe12,
> + 0x33fa0000, 0x0000fd16, 0x31fa0000, 0x0000fd18,
> + 0x2ffa0000, 0x0000fd1a, 0x2cfa0000, 0x0000fc1e,
> + 0x29fa0000, 0x0000fc21, 0x27fb0000, 0x0000fb23,
> + 0x24fb0000, 0x0000fb26, 0x21fb0000, 0x0000fb29,
> + 0x1ffc0000, 0x0000fa2b, 0x1cfc0000, 0x0000fa2e,
> + 0x19fd0000, 0x0000fa30, 0x16fd0000, 0x0000fa33,
> + 0x14fd0000, 0x0000fa35, 0x11fe0000, 0x0000fa37,
> + 0x0ffe0000, 0x0000fa39, 0x0dfe0000, 0x0000fa3b,
> + 0x0afe0000, 0x0000fa3e, 0x08ff0000, 0x0000fb3e,
> + 0x06ff0000, 0x0000fb40, 0x05ff0000, 0x0000fc40,
> + 0x03ff0000, 0x0000fd41, 0x01ff0000, 0x0000fe42,
> +};
> +
> +static void sun4i_frontend_scaler_init(struct sun4i_frontend *frontend)
> +{
> + int i;
> +
> + for (i = 0; i < 32; i++) {
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZCOEF0_REG(i),
> + sun4i_frontend_horz_coef[2 * i]);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_HORZCOEF0_REG(i),
> + sun4i_frontend_horz_coef[2 * i]);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZCOEF1_REG(i),
> + sun4i_frontend_horz_coef[2 * i + 1]);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_HORZCOEF1_REG(i),
> + sun4i_frontend_horz_coef[2 * i + 1]);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTCOEF_REG(i),
> + sun4i_frontend_vert_coef[i]);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTCOEF_REG(i),
> + sun4i_frontend_vert_coef[i]);
> + }
> +
> + regmap_update_bits(frontend->regs, SUN4I_FRONTEND_FRM_CTRL_REG, BIT(23), BIT(23));
> +}
> +
> +int sun4i_frontend_init(struct sun4i_frontend *frontend)
> +{
> + int ret;
> +
> + if (atomic_read(&frontend->users))
> + return -EBUSY;
> +
> + ret = reset_control_deassert(frontend->reset);
> + if (ret) {
> + DRM_DEBUG_DRIVER("Couldn't deassert our reset line\n");
> + return ret;
> + }
> +
> + clk_set_rate(frontend->mod_clk, 300000000);
> +
> + clk_prepare_enable(frontend->bus_clk);
> + clk_prepare_enable(frontend->mod_clk);
> + clk_prepare_enable(frontend->ram_clk);
> +
> + regmap_update_bits(frontend->regs, SUN4I_FRONTEND_EN_REG,
> + SUN4I_FRONTEND_EN_EN,
> + SUN4I_FRONTEND_EN_EN);
> +
> + regmap_update_bits(frontend->regs, SUN4I_FRONTEND_BYPASS_REG,
> + SUN4I_FRONTEND_BYPASS_CSC_EN,
> + SUN4I_FRONTEND_BYPASS_CSC_EN);
> +
> + sun4i_frontend_scaler_init(frontend);
> +
> + atomic_inc(&frontend->users);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(sun4i_frontend_init);
> +
> +void sun4i_frontend_exit(struct sun4i_frontend *frontend)
> +{
> + atomic_dec(&frontend->users);
> +
> + clk_disable_unprepare(frontend->ram_clk);
> + clk_disable_unprepare(frontend->mod_clk);
> + clk_disable_unprepare(frontend->bus_clk);
Maybe a blank line ?
> + reset_control_assert(frontend->reset);
> +}
> +EXPORT_SYMBOL(sun4i_frontend_exit);
> +
> +void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
> + struct drm_plane *plane)
> +{
> + struct drm_plane_state *state = plane->state;
> + struct drm_framebuffer *fb = state->fb;
> + struct drm_gem_cma_object *gem;
> + dma_addr_t paddr;
> + int bpp;
> +
> + /* Get the physical address of the buffer in memory */
> + gem = drm_fb_cma_get_gem_obj(fb, 0);
> +
> + DRM_DEBUG_DRIVER("Using GEM @ %pad\n", &gem->paddr);
> +
> + /* Set the line width */
> + DRM_DEBUG_DRIVER("Frontend stride: %d bytes\n", fb->pitches[0]);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_LINESTRD0_REG,
> + fb->pitches[0]);
> +
> + /* Compute the start of the displayed memory */
> + bpp = fb->format->cpp[0];
> + paddr = gem->paddr + fb->offsets[0];
> + paddr += (state->src_x >> 16) * bpp;
> + paddr += (state->src_y >> 16) * fb->pitches[0];
> +
> + DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR0_REG, paddr);
> +}
> +
> +static int sun4i_frontend_drm_format_to_input_fmt(uint32_t fmt, u32 *val)
> +{
> + switch (fmt) {
> + case DRM_FORMAT_ARGB8888:
> + case DRM_FORMAT_XRGB8888:
> + *val = 3;
> + return 0;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int sun4i_frontend_drm_format_to_output_fmt(uint32_t fmt, u32 *val)
> +{
> + switch (fmt) {
> + case DRM_FORMAT_ARGB8888:
> + *val = 2;
> + return 0;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
> + struct drm_plane *plane, uint32_t out_fmt)
> +{
> + struct drm_plane_state *state = plane->state;
> + struct drm_framebuffer *fb = state->fb;
> + u32 out_fmt_val;
> + u32 in_fmt_val;
> + int ret;
> +
> + ret = sun4i_frontend_drm_format_to_input_fmt(fb->format->format,
> + &in_fmt_val);
> + if (ret) {
> + DRM_DEBUG_DRIVER("Invalid input format\n");
> + return ret;
> + }
> +
> + ret = sun4i_frontend_drm_format_to_output_fmt(out_fmt, &out_fmt_val);
> + if (ret) {
> + DRM_DEBUG_DRIVER("Invalid output format\n");
> + return ret;
> + }
> +
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZPHASE_REG, 0x400);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_HORZPHASE_REG, 0x400);
> +
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTPHASE0_REG, 0x400);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTPHASE0_REG, 0x400);
> +
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTPHASE1_REG, 0x400);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTPHASE1_REG, 0x400);
> +
> + regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG, 0x151);
> + /* SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) | */
> + /* SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(5) | */
> + /* SUN4I_FRONTEND_INPUT_FMT_PS(1)); */
Why are these commented ?
> + regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG, 0x82);
> + /* SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(1)); */
Same here
> +
> + return 0;
> +}
> +
> +void sun4i_frontend_update_coord(struct sun4i_frontend *frontend,
> + struct drm_plane *plane)
> +{
> + struct drm_plane_state *state = plane->state;
> +
> + /* Set height and width */
> + DRM_DEBUG_DRIVER("Frontend size W: %u H: %u\n",
> + state->crtc_w, state->crtc_h);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_INSIZE_REG,
> + SUN4I_FRONTEND_INSIZE(state->src_h >> 16,
> + state->src_w >> 16));
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_INSIZE_REG,
> + SUN4I_FRONTEND_INSIZE(state->src_h >> 16,
> + state->src_w >> 16));
> +
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_OUTSIZE_REG,
> + SUN4I_FRONTEND_OUTSIZE(state->crtc_h, state->crtc_w));
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_OUTSIZE_REG,
> + SUN4I_FRONTEND_OUTSIZE(state->crtc_h, state->crtc_w));
> +
> + DRM_DEBUG_DRIVER("Frontend horizontal scaling factor %d.%d\n", 1, 0);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_HORZFACT_REG,
> + state->src_w / state->crtc_w);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_HORZFACT_REG,
> + state->src_w / state->crtc_w);
> +
> + DRM_DEBUG_DRIVER("Frontend vertical scaling factor %d.%d\n", 1, 0);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH0_VERTFACT_REG,
> + state->src_h / state->crtc_h);
> + regmap_write(frontend->regs, SUN4I_FRONTEND_CH1_VERTFACT_REG,
> + state->src_h / state->crtc_h);
> +
> + regmap_write_bits(frontend->regs, SUN4I_FRONTEND_FRM_CTRL_REG,
> + SUN4I_FRONTEND_FRM_CTRL_REG_RDY,
> + SUN4I_FRONTEND_FRM_CTRL_REG_RDY);
> +}
> +EXPORT_SYMBOL(sun4i_frontend_update_coord);
> +
> +int sun4i_frontend_enable(struct sun4i_frontend *frontend)
> +{
> + regmap_write_bits(frontend->regs, SUN4I_FRONTEND_FRM_CTRL_REG,
> + SUN4I_FRONTEND_FRM_CTRL_FRM_START,
> + SUN4I_FRONTEND_FRM_CTRL_FRM_START);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(sun4i_frontend_enable);
> +
> +static struct regmap_config sun4i_frontend_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = 0x0a14,
> +};
> +
> +static int sun4i_frontend_bind(struct device *dev, struct device *master,
> + void *data)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct sun4i_frontend *frontend;
> + struct drm_device *drm = data;
> + struct sun4i_drv *drv = drm->dev_private;
> + struct resource *res;
> + void __iomem *regs;
> +
> + frontend = devm_kzalloc(dev, sizeof(*frontend), GFP_KERNEL);
> + if (!frontend)
> + return -ENOMEM;
Blank line ?
> + dev_set_drvdata(dev, frontend);
> + frontend->node = dev->of_node;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + regs = devm_ioremap_resource(dev, res);
> + if (IS_ERR(regs))
> + return PTR_ERR(regs);
> +
> + frontend->regs = devm_regmap_init_mmio(dev, regs,
> + &sun4i_frontend_regmap_config);
> + if (IS_ERR(frontend->regs)) {
> + dev_err(dev, "Couldn't create the frontend regmap\n");
> + return PTR_ERR(frontend->regs);
> + }
> +
> + frontend->reset = devm_reset_control_get(dev, NULL);
> + if (IS_ERR(frontend->reset)) {
> + dev_err(dev, "Couldn't get our reset line\n");
> + return PTR_ERR(frontend->reset);
> + }
> +
> + frontend->bus_clk = devm_clk_get(dev, "ahb");
> + if (IS_ERR(frontend->bus_clk)) {
> + dev_err(dev, "Couldn't get our bus clock\n");
> + return PTR_ERR(frontend->bus_clk);
> + }
> +
> + frontend->mod_clk = devm_clk_get(dev, "mod");
> + if (IS_ERR(frontend->mod_clk)) {
> + dev_err(dev, "Couldn't get our mod clock\n");
> + return PTR_ERR(frontend->mod_clk);
> + }
> +
> + frontend->ram_clk = devm_clk_get(dev, "ram");
> + if (IS_ERR(frontend->ram_clk)) {
> + dev_err(dev, "Couldn't get our ram clock\n");
> + return PTR_ERR(frontend->ram_clk);
> + }
> +
> + list_add_tail(&frontend->list, &drv->frontend_list);
> +
> + return 0;
> +}
> +
> +static void sun4i_frontend_unbind(struct device *dev, struct device *master,
> + void *data)
> +{
> + struct sun4i_frontend *frontend = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(frontend->mod_clk);
> + clk_disable_unprepare(frontend->bus_clk);
Blank line ?
> + reset_control_assert(frontend->reset);
> +}
> +
> +static const struct component_ops sun4i_frontend_ops = {
> + .bind = sun4i_frontend_bind,
> + .unbind = sun4i_frontend_unbind,
> +};
> +
> +static int sun4i_frontend_probe(struct platform_device *pdev)
> +{
> + return component_add(&pdev->dev, &sun4i_frontend_ops);
> +}
> +
> +static int sun4i_frontend_remove(struct platform_device *pdev)
> +{
> + component_del(&pdev->dev, &sun4i_frontend_ops);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sun4i_frontend_of_table[] = {
> + { .compatible = "allwinner,sun5i-a13-display-frontend" },
> + { .compatible = "allwinner,sun6i-a31-display-frontend" },
> + { .compatible = "allwinner,sun8i-a33-display-frontend" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, sun4i_frontend_of_table);
> +
> +static struct platform_driver sun4i_frontend_driver = {
> + .probe = sun4i_frontend_probe,
> + .remove = sun4i_frontend_remove,
> + .driver = {
> + .name = "sun4i-frontend",
> + .of_match_table = sun4i_frontend_of_table,
> + },
> +};
> +module_platform_driver(sun4i_frontend_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Allwinner A10 Display Engine Frontend Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.h b/drivers/gpu/drm/sun4i/sun4i_frontend.h
> new file mode 100644
> index 000000000000..2df9f6de0f3f
> --- /dev/null
> +++ b/drivers/gpu/drm/sun4i/sun4i_frontend.h
> @@ -0,0 +1,102 @@
// SPDX-Licence-Identifier... new format ?
> +/*
> + * Copyright (C) 2017 Free Electrons
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#ifndef _SUN4I_FRONTEND_H_
> +#define _SUN4I_FRONTEND_H_
> +
> +#include <linux/atomic.h>
> +#include <linux/list.h>
> +
> +#define SUN4I_FRONTEND_EN_REG 0x000
> +#define SUN4I_FRONTEND_EN_EN BIT(0)
> +
> +#define SUN4I_FRONTEND_FRM_CTRL_REG 0x004
> +#define SUN4I_FRONTEND_FRM_CTRL_FRM_START BIT(16)
> +#define SUN4I_FRONTEND_FRM_CTRL_COEF_RDY BIT(1)
> +#define SUN4I_FRONTEND_FRM_CTRL_REG_RDY BIT(0)
> +
> +#define SUN4I_FRONTEND_BYPASS_REG 0x008
> +#define SUN4I_FRONTEND_BYPASS_CSC_EN BIT(1)
> +
> +#define SUN4I_FRONTEND_BUF_ADDR0_REG 0x020
> +
> +#define SUN4I_FRONTEND_LINESTRD0_REG 0x040
> +
> +#define SUN4I_FRONTEND_INPUT_FMT_REG 0x04c
> +#define SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(mod) ((mod) << 8)
> +#define SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(fmt) ((fmt) << 4)
> +#define SUN4I_FRONTEND_INPUT_FMT_PS(ps) (ps)
> +
> +#define SUN4I_FRONTEND_OUTPUT_FMT_REG 0x05c
> +#define SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(fmt) (fmt)
> +
> +#define SUN4I_FRONTEND_CH0_INSIZE_REG 0x100
> +#define SUN4I_FRONTEND_INSIZE(h, w) ((((h) - 1) << 16) | (((w) - 1)))
> +
> +#define SUN4I_FRONTEND_CH0_OUTSIZE_REG 0x104
> +#define SUN4I_FRONTEND_OUTSIZE(h, w) ((((h) - 1) << 16) | (((w) - 1)))
> +
> +#define SUN4I_FRONTEND_CH0_HORZFACT_REG 0x108
> +#define SUN4I_FRONTEND_HORZFACT(i, f) (((i) << 16) | (f))
> +
> +#define SUN4I_FRONTEND_CH0_VERTFACT_REG 0x10c
> +#define SUN4I_FRONTEND_VERTFACT(i, f) (((i) << 16) | (f))
> +
> +#define SUN4I_FRONTEND_CH0_HORZPHASE_REG 0x110
> +#define SUN4I_FRONTEND_CH0_VERTPHASE0_REG 0x114
> +#define SUN4I_FRONTEND_CH0_VERTPHASE1_REG 0x118
> +
> +#define SUN4I_FRONTEND_CH1_INSIZE_REG 0x200
> +#define SUN4I_FRONTEND_CH1_OUTSIZE_REG 0x204
> +#define SUN4I_FRONTEND_CH1_HORZFACT_REG 0x208
> +#define SUN4I_FRONTEND_CH1_VERTFACT_REG 0x20c
> +
> +#define SUN4I_FRONTEND_CH1_HORZPHASE_REG 0x210
> +#define SUN4I_FRONTEND_CH1_VERTPHASE0_REG 0x214
> +#define SUN4I_FRONTEND_CH1_VERTPHASE1_REG 0x218
> +
> +#define SUN4I_FRONTEND_CH0_HORZCOEF0_REG(i) (0x400 + i * 4)
> +#define SUN4I_FRONTEND_CH0_HORZCOEF1_REG(i) (0x480 + i * 4)
> +#define SUN4I_FRONTEND_CH0_VERTCOEF_REG(i) (0x500 + i * 4)
> +#define SUN4I_FRONTEND_CH1_HORZCOEF0_REG(i) (0x600 + i * 4)
> +#define SUN4I_FRONTEND_CH1_HORZCOEF1_REG(i) (0x680 + i * 4)
> +#define SUN4I_FRONTEND_CH1_VERTCOEF_REG(i) (0x700 + i * 4)
> +
> +struct clk;
> +struct device_node;
> +struct drm_plane;
> +struct regmap;
> +struct reset_control;
> +
> +struct sun4i_frontend {
> + struct list_head list;
> + atomic_t users;
> + struct device_node *node;
> +
> + struct clk *bus_clk;
> + struct clk *mod_clk;
> + struct clk *ram_clk;
> + struct regmap *regs;
> + struct reset_control *reset;
> +};
> +
> +int sun4i_frontend_init(struct sun4i_frontend *frontend);
> +void sun4i_frontend_exit(struct sun4i_frontend *frontend);
> +int sun4i_frontend_enable(struct sun4i_frontend *frontend);
> +
> +void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend,
> + struct drm_plane *plane);
> +void sun4i_frontend_update_coord(struct sun4i_frontend *frontend,
> + struct drm_plane *plane);
> +int sun4i_frontend_update_formats(struct sun4i_frontend *frontend,
> + struct drm_plane *plane, uint32_t out_fmt);
> +
> +#endif /* _SUN4I_FRONTEND_H_ */
>
Apart these nits,
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [PATCH 6/8] drm/sun4i: sun4i_layer: Wire in the frontend
From: Neil Armstrong @ 2017-12-13 16:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0cd2570c9cbe20d094cee0e8e9918c3a0f6af2fa.1513178989.git-series.maxime.ripard@free-electrons.com>
On 13/12/2017 16:33, Maxime Ripard wrote:
> Now that we have a driver, we can make use of it. This is done by
> adding a flag to our custom plane state that will trigger whether we should
> use the frontend on that particular plane or not.
>
> The rest is just plumbing to set up the backend to not perform the DMA but
> receive its data from the frontend.
>
> Note that we're still not making any use of the frontend itself, as no one
> is setting the flag yet.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_backend.c | 53 ++++++++++++++++++++++++++++-
> drivers/gpu/drm/sun4i/sun4i_backend.h | 3 ++-
> drivers/gpu/drm/sun4i/sun4i_layer.c | 27 ++++++++++++--
> drivers/gpu/drm/sun4i/sun4i_layer.h | 1 +-
> 4 files changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index e83e1fe43823..f1d19767c55d 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -26,6 +26,7 @@
>
> #include "sun4i_backend.h"
> #include "sun4i_drv.h"
> +#include "sun4i_frontend.h"
> #include "sun4i_layer.h"
> #include "sunxi_engine.h"
>
> @@ -203,6 +204,30 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
> return 0;
> }
>
> +int sun4i_backend_update_layer_frontend(struct sun4i_backend *backend,
> + int layer, uint32_t fmt)
> +{
> + u32 val;
> + int ret;
> +
> + ret = sun4i_backend_drm_format_to_layer(NULL, fmt, &val);
> + if (ret) {
> + DRM_DEBUG_DRIVER("Invalid format\n");
> + return ret;
> + }
> +
> + regmap_update_bits(backend->engine.regs,
> + SUN4I_BACKEND_ATTCTL_REG0(layer),
> + SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN,
> + SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN);
> +
> + regmap_update_bits(backend->engine.regs,
> + SUN4I_BACKEND_ATTCTL_REG1(layer),
> + SUN4I_BACKEND_ATTCTL_REG1_LAY_FBFMT, val);
> +
> + return 0;
> +}
> +
> int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> int layer, struct drm_plane *plane)
> {
> @@ -330,6 +355,34 @@ static int sun4i_backend_of_get_id(struct device_node *node)
> return ret;
> }
>
> +static struct sun4i_frontend *sun4i_backend_find_frontend(struct sun4i_drv *drv,
> + struct device_node *node)
> +{
> + struct device_node *port, *ep, *remote;
> + struct sun4i_frontend *frontend;
> +
> + port = of_graph_get_port_by_id(node, 0);
> + if (!port)
> + return ERR_PTR(-EINVAL);
> +
> + for_each_available_child_of_node(port, ep) {
> + remote = of_graph_get_remote_port_parent(ep);
> + if (!remote)
> + continue;
> +
> + /* does this node match any registered engines? */
> + list_for_each_entry(frontend, &drv->frontend_list, list) {
> + if (remote == frontend->node) {
> + of_node_put(remote);
> + of_node_put(port);
> + return frontend;
> + }
> + }
> + }
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
> .commit = sun4i_backend_commit,
> .layers_init = sun4i_layers_init,
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index ba1410fd5410..636a51521e77 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -72,6 +72,7 @@
> #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(x) ((x) << 15)
> #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL_MASK GENMASK(11, 10)
> #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL(x) ((x) << 10)
> +#define SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN BIT(1)
>
> #define SUN4I_BACKEND_ATTCTL_REG1(l) (0x8a0 + (0x4 * (l)))
> #define SUN4I_BACKEND_ATTCTL_REG1_LAY_HSCAFCT GENMASK(15, 14)
> @@ -171,5 +172,7 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
> int layer, struct drm_plane *plane);
> int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> int layer, struct drm_plane *plane);
> +int sun4i_backend_update_layer_frontend(struct sun4i_backend *backend,
> + int layer, uint32_t in_fmt);
>
> #endif /* _SUN4I_BACKEND_H_ */
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index c3afcf888906..3b58667a06dc 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -15,6 +15,7 @@
> #include <drm/drmP.h>
>
> #include "sun4i_backend.h"
> +#include "sun4i_frontend.h"
> #include "sun4i_layer.h"
> #include "sunxi_engine.h"
>
> @@ -70,21 +71,41 @@ static void sun4i_backend_layer_destroy_state(struct drm_plane *plane,
> static void sun4i_backend_layer_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> + struct sun4i_layer_state *layer_state = state_to_sun4i_layer_state(old_state);
> struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
> struct sun4i_backend *backend = layer->backend;
> + struct sun4i_frontend *frontend = backend->frontend;
>
> sun4i_backend_layer_enable(backend, layer->id, false);
> +
> + if (layer_state->uses_frontend)
> + sun4i_frontend_exit(frontend);
> }
>
> static void sun4i_backend_layer_atomic_update(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> + struct sun4i_layer_state *layer_state = state_to_sun4i_layer_state(plane->state);
> struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
> struct sun4i_backend *backend = layer->backend;
> + struct sun4i_frontend *frontend = backend->frontend;
> +
> + if (layer_state->uses_frontend) {
> + sun4i_frontend_init(frontend);
> + sun4i_frontend_update_coord(frontend, plane);
> + sun4i_frontend_update_buffer(frontend, plane);
> + sun4i_frontend_update_formats(frontend, plane,
> + DRM_FORMAT_ARGB8888);
> + sun4i_backend_update_layer_frontend(backend, layer->id,
> + DRM_FORMAT_ARGB8888);
> + sun4i_backend_update_layer_coord(backend, layer->id, plane);
> + sun4i_frontend_enable(frontend);
> + } else {
> + sun4i_backend_update_layer_coord(backend, layer->id, plane);
> + sun4i_backend_update_layer_formats(backend, layer->id, plane);
> + sun4i_backend_update_layer_buffer(backend, layer->id, plane);
> + }
>
> - sun4i_backend_update_layer_coord(backend, layer->id, plane);
> - sun4i_backend_update_layer_formats(backend, layer->id, plane);
> - sun4i_backend_update_layer_buffer(backend, layer->id, plane);
> sun4i_backend_layer_enable(backend, layer->id, true);
> }
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.h b/drivers/gpu/drm/sun4i/sun4i_layer.h
> index d2c19348d1b0..75b4868ba87c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.h
> @@ -24,6 +24,7 @@ struct sun4i_layer {
>
> struct sun4i_layer_state {
> struct drm_plane_state state;
> + bool uses_frontend;
> };
>
> static inline struct sun4i_layer *
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [PATCH 7/8] drm/sun4i: sun4i_layer: Add a custom atomic_check for the frontend
From: Neil Armstrong @ 2017-12-13 16:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4fbc349e2c3aa2efeadb55ea120c64b56c707cb4.1513178989.git-series.maxime.ripard@free-electrons.com>
On 13/12/2017 16:33, Maxime Ripard wrote:
> Now that we have everything in place, we can start enabling the frontend.
> This is more difficult than one would assume since there can only be one
> plane using the frontend per-backend.
>
> We therefore need to make sure that the userspace will not try to setup
> multiple planes using it, since that would be impossible. In order to
> prevent that, we can create an atomic_check callback that will check that
> only one plane will effectively make use of the frontend in a given
> configuration, and will toggle the switch in that plane state so that the
> proper setup function can do their role.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_backend.c | 65 ++++++++++++++++++++++++++++-
> drivers/gpu/drm/sun4i/sun4i_backend.h | 2 +-
> 2 files changed, 67 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index f1d19767c55d..a7b87a12990e 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -11,6 +11,7 @@
> */
>
> #include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_crtc.h>
> #include <drm/drm_crtc_helper.h>
> @@ -271,6 +272,69 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> return 0;
> }
>
> +static bool sun4i_backend_plane_uses_scaler(struct drm_plane_state *state)
> +{
> + u16 src_h = state->src_h >> 16;
> + u16 src_w = state->src_w >> 16;
> +
> + DRM_DEBUG_DRIVER("Input size %dx%d, output size %dx%d\n",
> + src_w, src_h, state->crtc_w, state->crtc_h);
> +
> + if ((state->crtc_h != src_h) || (state->crtc_w != src_w))
> + return true;
> +
> + return false;
> +}
> +
> +static bool sun4i_backend_plane_uses_frontend(struct drm_plane_state *state)
> +{
> + struct sun4i_layer *layer = plane_to_sun4i_layer(state->plane);
> + struct sun4i_backend *backend = layer->backend;
> +
> + if (IS_ERR(backend->frontend))
> + return false;
> +
> + return sun4i_backend_plane_uses_scaler(state);
> +}
> +
> +static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
> + struct drm_crtc_state *crtc_state)
> +{
> + struct drm_atomic_state *state = crtc_state->state;
> + struct drm_device *drm = state->dev;
> + struct drm_plane *plane;
> + unsigned int num_frontend_planes = 0;
> +
> + DRM_DEBUG_DRIVER("Starting checking our planes\n");
> +
> + if (!crtc_state->planes_changed)
> + return 0;
> +
> + drm_for_each_plane_mask(plane, drm, crtc_state->plane_mask) {
> + struct drm_plane_state *plane_state =
> + drm_atomic_get_plane_state(state, plane);
> + struct sun4i_layer_state *layer_state =
> + state_to_sun4i_layer_state(plane_state);
> +
> + if (sun4i_backend_plane_uses_frontend(plane_state)) {
> + DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
> + plane->index);
> +
> + layer_state->uses_frontend = true;
> + num_frontend_planes++;
> + } else {
> + layer_state->uses_frontend = false;
> + }
> + }
> +
> + if (num_frontend_planes > SUN4I_BACKEND_NUM_FRONTEND_LAYERS) {
> + DRM_DEBUG_DRIVER("Too many planes going through the frontend, rejecting\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int sun4i_backend_init_sat(struct device *dev) {
> struct sun4i_backend *backend = dev_get_drvdata(dev);
> int ret;
> @@ -384,6 +448,7 @@ static struct sun4i_frontend *sun4i_backend_find_frontend(struct sun4i_drv *drv,
> }
>
> static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
> + .atomic_check = sun4i_backend_atomic_check,
> .commit = sun4i_backend_commit,
> .layers_init = sun4i_layers_init,
> .apply_color_correction = sun4i_backend_apply_color_correction,
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index 636a51521e77..3b5531440fa3 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -144,6 +144,8 @@
> #define SUN4I_BACKEND_HWCCOLORTAB_OFF 0x4c00
> #define SUN4I_BACKEND_PIPE_OFF(p) (0x5000 + (0x400 * (p)))
>
> +#define SUN4I_BACKEND_NUM_FRONTEND_LAYERS 1
> +
> struct sun4i_backend {
> struct sunxi_engine engine;
> struct sun4i_frontend *frontend;
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [PATCH 0/8] drm/sun4i: Support the Display Engine frontend
From: Thomas van Kleef @ 2017-12-13 16:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.63722fe4aeb908cf3c6f73874bdf69b193fe43c9.1513178989.git-series.maxime.ripard@free-electrons.com>
Hi,
On 13-12-17 16:33, Maxime Ripard wrote:
> Hi,
>
> This is a first serie to enable the display engine frontend.
>
> This hardware block is found in the first generation Display Engine from
> Allwinner. Its role is to implement more advanced features that the
> associated backend, even though the backend alone can be used (and was used
> so far) for basic composition.
>
> Among those features, we will find hardware scaling, that is supported in
> this serie, colorspace conversions, or more exotic formats support such as
> the one output by the VPU.
So, if I have read the code correctly. The frontend will be used whenever the
input size differs from the output size.
>
> Let me know what you think,
> Maxime
>
> Maxime Ripard (8):
> drm/sun4i: backend: Move line stride setup to buffer setup function
> drm/sun4i: backend: Allow a NULL plane pointer to retrieve the format
> drm/sun4i: sun4i_layer: Add a custom plane state
> drm/sun4i: crtc: Add a custom crtc atomic_check
> drm/sun4i: Add a driver for the display frontend
> drm/sun4i: sun4i_layer: Wire in the frontend
> drm/sun4i: sun4i_layer: Add a custom atomic_check for the frontend
> ARM: dts: sun8i: a33 Enable our display frontend
>
> arch/arm/boot/dts/sun8i-a33.dtsi | 1 +-
> drivers/gpu/drm/sun4i/Makefile | 3 +-
> drivers/gpu/drm/sun4i/sun4i_backend.c | 139 +++++++++-
> drivers/gpu/drm/sun4i/sun4i_backend.h | 6 +-
> drivers/gpu/drm/sun4i/sun4i_crtc.c | 14 +-
> drivers/gpu/drm/sun4i/sun4i_drv.c | 16 +-
> drivers/gpu/drm/sun4i/sun4i_drv.h | 1 +-
> drivers/gpu/drm/sun4i/sun4i_frontend.c | 377 ++++++++++++++++++++++++++-
> drivers/gpu/drm/sun4i/sun4i_frontend.h | 102 +++++++-
> drivers/gpu/drm/sun4i/sun4i_layer.c | 75 ++++-
> drivers/gpu/drm/sun4i/sun4i_layer.h | 11 +-
> drivers/gpu/drm/sun4i/sunxi_engine.h | 2 +-
> 12 files changed, 727 insertions(+), 20 deletions(-)
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_frontend.c
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_frontend.h
>
> base-commit: 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323
>
^ permalink raw reply
* [PATCH 6/8] drm/sun4i: sun4i_layer: Wire in the frontend
From: Thomas van Kleef @ 2017-12-13 16:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0cd2570c9cbe20d094cee0e8e9918c3a0f6af2fa.1513178989.git-series.maxime.ripard@free-electrons.com>
On 13-12-17 16:33, Maxime Ripard wrote:
> Now that we have a driver, we can make use of it. This is done by
> adding a flag to our custom plane state that will trigger whether we should
> use the frontend on that particular plane or not.
>
> The rest is just plumbing to set up the backend to not perform the DMA but
> receive its data from the frontend.
>
> Note that we're still not making any use of the frontend itself, as no one
> is setting the flag yet.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_backend.c | 53 ++++++++++++++++++++++++++++-
> drivers/gpu/drm/sun4i/sun4i_backend.h | 3 ++-
> drivers/gpu/drm/sun4i/sun4i_layer.c | 27 ++++++++++++--
> drivers/gpu/drm/sun4i/sun4i_layer.h | 1 +-
> 4 files changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index e83e1fe43823..f1d19767c55d 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -26,6 +26,7 @@
>
> #include "sun4i_backend.h"
> #include "sun4i_drv.h"
> +#include "sun4i_frontend.h"
> #include "sun4i_layer.h"
> #include "sunxi_engine.h"
>
> @@ -203,6 +204,30 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
> return 0;
> }
>
> +int sun4i_backend_update_layer_frontend(struct sun4i_backend *backend,
> + int layer, uint32_t fmt)
> +{
> + u32 val;
> + int ret;
> +
> + ret = sun4i_backend_drm_format_to_layer(NULL, fmt, &val);
> + if (ret) {
> + DRM_DEBUG_DRIVER("Invalid format\n");
> + return ret;
> + }
> +
> + regmap_update_bits(backend->engine.regs,
> + SUN4I_BACKEND_ATTCTL_REG0(layer),
> + SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN,
> + SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN);
> +
> + regmap_update_bits(backend->engine.regs,
> + SUN4I_BACKEND_ATTCTL_REG1(layer),
> + SUN4I_BACKEND_ATTCTL_REG1_LAY_FBFMT, val);
> +
> + return 0;
> +}
> +
> int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> int layer, struct drm_plane *plane)
> {
> @@ -330,6 +355,34 @@ static int sun4i_backend_of_get_id(struct device_node *node)
> return ret;
> }
>
> +static struct sun4i_frontend *sun4i_backend_find_frontend(struct sun4i_drv *drv,
> + struct device_node *node)
> +{
> + struct device_node *port, *ep, *remote;
> + struct sun4i_frontend *frontend;
> +
> + port = of_graph_get_port_by_id(node, 0);
> + if (!port)
> + return ERR_PTR(-EINVAL);
> +
> + for_each_available_child_of_node(port, ep) {
> + remote = of_graph_get_remote_port_parent(ep);
> + if (!remote)
> + continue;
> +
> + /* does this node match any registered engines? */
> + list_for_each_entry(frontend, &drv->frontend_list, list) {
> + if (remote == frontend->node) {
> + of_node_put(remote);
> + of_node_put(port);
> + return frontend;
> + }
> + }
> + }
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
> .commit = sun4i_backend_commit,
> .layers_init = sun4i_layers_init,
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index ba1410fd5410..636a51521e77 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -72,6 +72,7 @@
> #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(x) ((x) << 15)
> #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL_MASK GENMASK(11, 10)
> #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL(x) ((x) << 10)
> +#define SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN BIT(1)
>
> #define SUN4I_BACKEND_ATTCTL_REG1(l) (0x8a0 + (0x4 * (l)))
> #define SUN4I_BACKEND_ATTCTL_REG1_LAY_HSCAFCT GENMASK(15, 14)
> @@ -171,5 +172,7 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
> int layer, struct drm_plane *plane);
> int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> int layer, struct drm_plane *plane);
> +int sun4i_backend_update_layer_frontend(struct sun4i_backend *backend,
> + int layer, uint32_t in_fmt);
>
> #endif /* _SUN4I_BACKEND_H_ */
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index c3afcf888906..3b58667a06dc 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -15,6 +15,7 @@
> #include <drm/drmP.h>
>
> #include "sun4i_backend.h"
> +#include "sun4i_frontend.h"
> #include "sun4i_layer.h"
> #include "sunxi_engine.h"
>
> @@ -70,21 +71,41 @@ static void sun4i_backend_layer_destroy_state(struct drm_plane *plane,
> static void sun4i_backend_layer_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> + struct sun4i_layer_state *layer_state = state_to_sun4i_layer_state(old_state);
> struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
> struct sun4i_backend *backend = layer->backend;
> + struct sun4i_frontend *frontend = backend->frontend;
>
> sun4i_backend_layer_enable(backend, layer->id, false);
> +
> + if (layer_state->uses_frontend)
> + sun4i_frontend_exit(frontend);
> }
>
> static void sun4i_backend_layer_atomic_update(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> + struct sun4i_layer_state *layer_state = state_to_sun4i_layer_state(plane->state);
> struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
> struct sun4i_backend *backend = layer->backend;
> + struct sun4i_frontend *frontend = backend->frontend;
> +
> + if (layer_state->uses_frontend) {
> + sun4i_frontend_init(frontend);
> + sun4i_frontend_update_coord(frontend, plane);
> + sun4i_frontend_update_buffer(frontend, plane);
> + sun4i_frontend_update_formats(frontend, plane,
> + DRM_FORMAT_ARGB8888);
> + sun4i_backend_update_layer_frontend(backend, layer->id,
> + DRM_FORMAT_ARGB8888);
> + sun4i_backend_update_layer_coord(backend, layer->id, plane);
> + sun4i_frontend_enable(frontend);
> + } else {
> + sun4i_backend_update_layer_coord(backend, layer->id, plane);
> + sun4i_backend_update_layer_formats(backend, layer->id, plane);
> + sun4i_backend_update_layer_buffer(backend, layer->id, plane);
> + }
>
> - sun4i_backend_update_layer_coord(backend, layer->id, plane);
> - sun4i_backend_update_layer_formats(backend, layer->id, plane);
> - sun4i_backend_update_layer_buffer(backend, layer->id, plane);
> sun4i_backend_layer_enable(backend, layer->id, true);
> }
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.h b/drivers/gpu/drm/sun4i/sun4i_layer.h
> index d2c19348d1b0..75b4868ba87c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.h
> @@ -24,6 +24,7 @@ struct sun4i_layer {
>
> struct sun4i_layer_state {
> struct drm_plane_state state;
> + bool uses_frontend;
> };
>
> static inline struct sun4i_layer *
>
I wondered if the following is still valid?
In file sun4i_layer.c, func sun4i_layers_init
------------------
/*
* The hardware is a bit unusual here.
*
* Even though it supports 4 layers, it does the composition
* in two separate steps.
*
* The first one is assigning a layer to one of its two
* pipes. If more that 1 layer is assigned to the same pipe,
* and if pixels overlaps, the pipe will take the pixel from
* the layer with the highest priority.
*
* The second step is the actual alpha blending, that takes
* the two pipes as input, and uses the eventual alpha
* component to do the transparency between the two.
*
* This two steps scenario makes us unable to guarantee a
* robust alpha blending between the 4 layers in all
* situations. So we just expose two layers, one per pipe. On
* SoCs that support it, sprites could fill the need for more
* layers.
*/
for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
struct sun4i_layer *layer;
layer = sun4i_layer_init_one(drm, backend, plane);
if (IS_ERR(layer)) {
dev_err(drm->dev, "Couldn't initialize %s plane\n",
i ? "overlay" : "primary");
return ERR_CAST(layer);
};
DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
i ? "overlay" : "primary", plane->pipe);
regmap_update_bits(engine->regs, SUN4I_BACKEND_ATTCTL_REG0(i),
SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK,
SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(plane->pipe));
layer->id = i;
planes[i] = &layer->plane;
};
-----------------
I have been using the 3rd layer (layer2) as the input for the frontend. This essentially
overlays the other 2 layers. Is it still the case that we can only have 2 layers here?
Or could be present 1 additional layer when it is attached to the frontend?
Perhaps this is not relevant for this patchset.
Regards,
Thomas van Kleef
Vitsch Electronics
http://Vitsch.nl/
http://VitschVPN.nl/
tel: +31-(0)40-7113051
KvK nr: 17174380
BTW nr: NL142748201B01
^ permalink raw reply
* [RFC 3/9] arm64: handle 52-bit addresses in TTBR
From: Kristina Martsenko @ 2017-12-13 16:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <78ff6e58-a02e-b914-11c4-c9174d31d607@arm.com>
On 07/12/17 14:51, Robin Murphy wrote:
> On 07/12/17 12:29, Kristina Martsenko wrote:
>> On 21/11/17 14:39, Robin Murphy wrote:
>>> Hi Kristina,
>>>
>>> On 21/11/17 11:57, Kristina Martsenko wrote:
>>>> The top 4 bits of a 52-bit physical address are positioned at bits 2..5
>>>> in the TTBR registers. Introduce a couple of macros to move the bits
>>>> there, and change all TTBR writers to use them.
>>>>
>>>> Leave TTBR0 PAN code unchanged, to avoid complicating it. A system with
>>>> 52-bit PA will have PAN anyway (because it's ARMv8.1 or later), and a
>>>> system without 52-bit PA can only use up to 48-bit PAs. Add a kconfig
>>>> dependency to ensure PAN is configured.
>>>>
>>>> In addition, when using 52-bit PA there is a special alignment
>>>> requirement on the top-level table. We don't currently have any VA_BITS
>>>> configuration that would violate the requirement, but one could be
>>>> added
>>>> in the future, so add a compile-time BUG_ON to check for it.
>>>>
>>>> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
>>>> ---
>>
>> [...]
>>
>>>> diff --git a/arch/arm64/include/asm/assembler.h
>>>> b/arch/arm64/include/asm/assembler.h
>>>> index 04cf94766b78..ba3c796b9fe1 100644
>>>> --- a/arch/arm64/include/asm/assembler.h
>>>> +++ b/arch/arm64/include/asm/assembler.h
>>>> @@ -512,4 +512,21 @@ alternative_else_nop_endif
>>>> ?? #endif
>>>> ?????? .endm
>>>> ?? +/*
>>>> + * Arrange a physical address in a TTBR register, taking care of 52-bit
>>>> + * addresses.
>>>> + *
>>>> + *???? phys:??? physical address, preserved
>>>> + *???? ttbr:??? returns the TTBR value
>>>> + */
>>>> +??? .macro??? phys_to_ttbr, phys, ttbr
>>>> +#ifdef CONFIG_ARM64_PA_BITS_52
>>>> +??? and??? \ttbr, \phys, #(1 << 48) - 1
>>>> +??? orr??? \ttbr, \ttbr, \phys, lsr #48 - 2
>>>> +??? bic??? \ttbr, \ttbr, #(1 << 2) - 1
>>>
>>> Is there any reason for masking off each end of the result separately
>>> like this, or could we just do it more straightforwardly?
>>
>> I don't recall any reason, maybe just to keep it simple, to avoid having
>> a separate mask macro.
>>
>>> #define TTBR_BADDR_MASK ((1 << 46) - 1) << 2
>>>
>>> ?????orr??? \ttbr, \phys, \phys, lsr #46
>>> ?????and??? \ttbr, \ttbr, #TTBR_BADDR_MASK
>>>
>>> (and equivalently for phys_to_pte in patch 4)
>>
>> Ok, I'll rewrite it like this. (Note that mask is 52-bit-specific
>> though.)
>
> I don't see that it need be 52-bit specific - true the BADDR field
> itself is strictly bits 47:1, but AFAICS bit 1 is always going to be
> RES0: either explicitly in the 52-bit case, or from the (x-1):1
> definition otherwise, since the requirement that a table must be aligned
> to its size infers an absolute lower bound of x >= 3 (it may be larger
> still due to other reasons, but I'm finding this area of the ARM ARM
> obnoxiously difficult to read). Thus defining the mask as covering 47:2
> should still be reasonable in all cases.
Yes BADDR is bits 47:1, and AFAICS bits 3:1 will be RES0 in the
non-52-bit case, since x >= 4 (since minimum 2 entries in a table). So I
think a non-52-bit mask should be 47:1 or 47:4. But in this patch, we
don't need a non-52-bit macro anyway, just one for the 52-bit case. I
will send a v2 today, please respond there if you're not happy with the
approach.
>>> Even better if there's a convenient place to define the mask such that
>>> it can be shared with KVM's existing VTTBR stuff too.
>>
>> Do you mean VTTBR_BADDR_MASK? I don't think this would be useful there,
>> VTTBR_BADDR_MASK checks the alignment of the address that goes into
>> VTTBR (not the VTTBR value itself), and takes into account specifically
>> the 40-bit IPA and concatenated page tables.
>
> Ah, I see - from skimming the code I managed to assume VTTBR_BADDR_MASK
> was a mask for the actual VTTBR.BADDR register field; I hadn't delved
> into all the VTTBR_X gubbins (yuck). Fair enough, scratch that idea. At
> least TTBR_ASID_MASK[1] gets to have a friend :)
Yep, although TTBR_BADDR_MASK will go into pgtable-hwdef.h (like
TTBR_CNP_BIT [1]).
Kristina
[1] https://patchwork.kernel.org/patch/9992927/
>
> Robin.
>
> [1] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1555176.html
>
>>>> +#else
>>>> +??? mov??? \ttbr, \phys
>>>> +#endif
>>>> +??? .endm
>>>> +
>>>> ?? #endif??? /* __ASM_ASSEMBLER_H */
>
^ permalink raw reply
* [PATCH v2 6/7] cpufreq: Add DVFS support for Armada 37xx
From: Gregory CLEMENT @ 2017-12-13 16:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171212072426.GK25177@vireshk-i7>
Hi Viresh,
On mar., d?c. 12 2017, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 07-12-17, 14:56, Gregory CLEMENT wrote:
>> +/* Power management in North Bridge register set */
>> +#define ARMADA_37XX_NB_L0L1 0x18
>> +#define ARMADA_37XX_NB_L2L3 0x1C
>> +#define ARMADA_37XX_NB_TBG_DIV_OFF 13
>> +#define ARMADA_37XX_NB_TBG_DIV_MASK 0x7
>> +#define ARMADA_37XX_NB_CLK_SEL_OFF 11
>> +#define ARMADA_37XX_NB_CLK_SEL_MASK 0x1
>> +#define ARMADA_37XX_NB_CLK_SEL_TBG 0x1
>> +#define ARMADA_37XX_NB_TBG_SEL_OFF 9
>> +#define ARMADA_37XX_NB_TBG_SEL_MASK 0x3
>> +#define ARMADA_37XX_NB_VDD_SEL_OFF 6
>> +#define ARMADA_37XX_NB_VDD_SEL_MASK 0x3
>> +#define ARMADA_37XX_NB_CONFIG_SHIFT 16
>> +#define ARMADA_37XX_NB_DYN_MOD 0x24
>> +#define ARMADA_37XX_NB_CLK_SEL_EN BIT(26)
>> +#define ARMADA_37XX_NB_TBG_EN BIT(28)
>> +#define ARMADA_37XX_NB_DIV_EN BIT(29)
>> +#define ARMADA_37XX_NB_VDD_EN BIT(30)
>> +#define ARMADA_37XX_NB_DFS_EN BIT(31)
>> +#define ARMADA_37XX_NB_CPU_LOAD 0x30
>> +#define ARMADA_37XX_NB_CPU_LOAD_MASK 0x3
>> +#define ARMADA_37XX_DVFS_LOAD_0 0
>> +#define ARMADA_37XX_DVFS_LOAD_1 1
>> +#define ARMADA_37XX_DVFS_LOAD_2 2
>> +#define ARMADA_37XX_DVFS_LOAD_3 3
>
> I thought you agreed to using space instead of tab after #define ?
Me too! Actually I did it, and I don't know what happened with this
patch.
However it will be part of the next version and I will double check it.
>
> Looks fine otherwise. You can add below after fixing above tab/space thing:
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Thanks,
Gregory
>
> --
> viresh
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH 1/2] cpufreq: ARM: sort the Kconfig menu
From: Gregory CLEMENT @ 2017-12-13 16:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <97091943-dcb8-47ab-0e55-831d35b1f73e@infradead.org>
Hi Randy,
On mar., d?c. 12 2017, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 12/12/2017 08:54 AM, Gregory CLEMENT wrote:
>> Group all the related big LITTLE configuration together and sort the
>> other entries in alphabetic order.
>>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> drivers/cpufreq/Kconfig.arm | 82 ++++++++++++++++++++++-----------------------
>> 1 file changed, 41 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index bdce4488ded1..0baf43837b51 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -2,6 +2,23 @@
>> # ARM CPU Frequency scaling drivers
>> #
>>
>> +config ACPI_CPPC_CPUFREQ
>> + tristate "CPUFreq driver based on the ACPI CPPC spec"
>> + depends on ACPI_PROCESSOR
>> + select ACPI_CPPC_LIB
>> + default n
>
> Drop "default n" since that is the default default.
This lines and as weel as all you pointed was already here, I've jsute
move things. But OK why not fixing it too.
Gregory
>
>> + help
>> + This adds a CPUFreq driver which uses CPPC methods
>> + as described in the ACPIv5.1 spec. CPPC stands for
>> + Collaborative Processor Performance Controls. It
>> + is based on an abstract continuous scale of CPU
>> + performance values which allows the remote power
>> + processor to flexibly optimize for power and
>> + performance. CPPC relies on power management firmware
>> + support for its operation.
>> +
>> + If in doubt, say N.
>> +
>> # big LITTLE core layer and glue drivers
>> config ARM_BIG_LITTLE_CPUFREQ
>> tristate "Generic ARM big LITTLE CPUfreq driver"
>> @@ -12,6 +29,30 @@ config ARM_BIG_LITTLE_CPUFREQ
>> help
>> This enables the Generic CPUfreq driver for ARM big.LITTLE platforms.
>>
>> +config ARM_DT_BL_CPUFREQ
>> + tristate "Generic probing via DT for ARM big LITTLE CPUfreq driver"
>> + depends on ARM_BIG_LITTLE_CPUFREQ && OF
>> + help
>> + This enables probing via DT for Generic CPUfreq driver for ARM
>> + big.LITTLE platform. This gets frequency tables from DT.
>> +
>> +config ARM_SCPI_CPUFREQ
>> + tristate "SCPI based CPUfreq driver"
>> + depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL && COMMON_CLK_SCPI
>> + help
>
> Fix the help and tristate lines -- use tab instead of spaces.
>
>> + This adds the CPUfreq driver support for ARM big.LITTLE platforms
>> + using SCPI protocol for CPU power management.
>> +
>> + This driver uses SCPI Message Protocol driver to interact with the
>> + firmware providing the CPU DVFS functionality.
>> +
>> +config ARM_VEXPRESS_SPC_CPUFREQ
>> + tristate "Versatile Express SPC based CPUfreq driver"
>> + depends on ARM_BIG_LITTLE_CPUFREQ && ARCH_VEXPRESS_SPC
>> + help
>
> Use tab instead of spaces above. Oh, and one line below.
>
>> + This add the CPUfreq driver support for Versatile Express
>> + big.LITTLE platforms using SPC for power management.
>> +
>> config ARM_BRCMSTB_AVS_CPUFREQ
>> tristate "Broadcom STB AVS CPUfreq driver"
>> depends on ARCH_BRCMSTB || COMPILE_TEST
>
>
> --
> ~Randy
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports
From: Tony Lindgren @ 2017-12-13 16:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213091527.GA18859@lenoch>
* Ladislav Michl <ladis@linux-mips.org> [171213 09:17]:
> On Tue, Dec 12, 2017 at 10:21:50AM -0800, Tony Lindgren wrote:
> > * Ladislav Michl <ladis@linux-mips.org> [171212 18:06]:
> > > I do not follow. Each general-purpose timer module has its own interrupt line,
> > > so claiming that irq directly using request_irq seems enough. Could you
> > > explain interrupt controller idea a bit more?
> >
> > Well let's assume we have drivers/clocksource/timer-dm.c implement
> > an irq controller. Then the pwm driver would just do:
> >
> > pwm9: dmtimer-pwm {
> > compatible = "ti,omap-dmtimer-pwm";
> > #pwm-cells = <3>;
> > ti,timers = <&timer9>;
> > ti,clock-source = <0x00>; /* timer_sys_ck */
> > interrupts-extended = <&timer9 IRQ_TYPE_SOMETHING>;
> > };
> >
> > Then you can do whatever you need to in the pwm driver with
> > enable_irq/disable_irq + a handler?
>
> That seems to work. Now should we map 1:1 to timer interrupt or
> have separate interrupt for match, overflow and capture?
> Former would need some more dm_timer_ops to determine interrupt
> source, while later would work "automagically" - but I haven't
> tested it yet.
Hmm the second option sounds appealing to me as then the device
tree binding really follows the hardware:
bit 2 capture interrupt
bit 1 overflow interrupt
bit 0 match interrupt
and in the dts we can describe these with:
interrupts-extended = <&timer 9 2 IRQ_TYPE_EDGE_SOMETHING>;
interrupts-extended = <&timer 9 1 IRQ_TYPE_EDGE_SOMETHING>;
interrupts-extended = <&timer 9 0 IRQ_TYPE_EDGE_SOMETHING>;
I think these are all edge based on "Capture Mode Functionality"
chapter in the TRM?
> > If reading the line status is needed.. Then maybe the GPIO framework
> > needs to have hardware timer support instead?
>
> It does not seem OMAP can read event pin value in event capture mode.
Sounds like a bug somehwere, probably in software?
> > Anyways, just thinking out loud how we could have a Linux generic
> > hardware timer framework that drivers like pwm could then use.
>
> I need a bit longer chain:
> dmtimer -> pwm -> rc (which calls ir_raw_event_store from interrupt)
> Is extending pwm core with interrpt callback the right thing there?
> Something like:
> (*pulse_captured)(ktime_t width, ktime_t last_edge);
OK seems like having drivers/clocksource/timer-dm.c a chained
interrupt handler should do it :) Anyways, that's a different
set of patches on top of these.
Regards,
Tony
^ permalink raw reply
* [RFC v2] arm64: KVM: KVM API extensions for SVE
From: Dave Martin @ 2017-12-13 16:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
Here's a second, slightly more complete stab at the KVM API extensions
for SVE.
I haven't started implementing in earnest yet, so any comments at this
stage would be very helpful.
[libvir-list readers: this is a proposal for extending the KVM API on
AArch64 systems to support the Scalable Vector Extension [1], [2].
This has some interesting configuration and migration quirks -- see
"Vector length control" in particular, and feel free to throw questions
my way...]
Cheers
---Dave
[1] Overview
https://community.arm.com/processors/b/blog/posts/technology-update-the-scalable-vector-extension-sve-for-the-armv8-a-architecture
[2] Architecture spec
https://developer.arm.com/products/architecture/a-profile/docs/ddi0584/latest/arm-architecture-reference-manual-supplement-the-scalable-vector-extension-sve-for-armv8-a
---8<---
New feature KVM_ARM_VCPU_SVE:
* enables exposure of SVE to the guest
* enables visibility of / access to KVM_REG_ARM_SVE_*() via KVM reg
ioctls. The main purposes of this are a) is to allow userspace to hide
weird-sized registers that it doesn't understand how to deal with,
and b) allow SVE to be hidden from the VM so that it can migrate to
nodes that don't support SVE.
ZCR_EL1 is not specifically hidden, since it is "just a system register"
and does not have a weird size or semantics etc.
Registers:
* A new register size is defined KVM_REG_SIZE_U2048 (which can be
encoded sensibly using the next unused value for the reg size field
in the reg ID) (grep KVM_REG_SIZE_).
* Reg IDs for the SVE regs will be defined as "coproc" 0x14
(i.e., 0x14 << KVM_REG_ARM_COPROC_SHIFT)
KVM_REG_ARM_SVE_Z(n, i) is slice i of Zn (each slice is 2048 bits)
KVM_REG_ARM_SVE_P(n, i) is slice i of Pn (each slice is 256 bits)
KVM_REG_ARM_FFR(i) is slice i of FFR (each slice is 256 bits)
The slice sizes allow each register to be read/written in exactly
one slice for SVE.
Surplus bits (beyond the maximum VL supported by the vcpu) will
be read-as-zero write-ignore.
Reading/writing surplus slices will probably be forbidden, and the
surplus slices would not be reported via KVM_GET_REG_LIST.
(We could make these RAZ/WI too, but I'm not sure if it's worth it,
or why it would be useful.)
Future extensions to the architecture might grow the registers up
to 32 slices: this may or may not actually happen, but SVE keeps the
possibilty open. I've tried to design for it.
* KVM_REG_ARM_SVE_Z(n, 0) bits [127:0] alias Vn in
KVM_REG_ARM_CORE(fp_regs.v[n]) .. KVM_REG_ARM_CORE(fp_regs.v[n])+3.
It's simplest for userspace if the two views always appear to be
in sync, but it's unclear whether this is really useful. Perhaps
this can be relaxed if it's a big deal for the KVM implementation;
I don't know yet.
Vector length control:
Some means is needed to determine the set of vector lengths visible
to guest software running on a vcpu.
When a vcpu is created, the set would be defaulted to the maximal set
that can be supported while permitting each vcpu to run on any host
CPU. SVE has some virtualisation quirks which mean that this set may
exclude some vector lengths that are available for host userspace
applications. The common case should be that the sets are the same
however.
* New ioctl KVM_ARM_VCPU_{SET,GET}_SVE_VLS to set or retrieve the set of
vector lengths available to the guest.
Adding random vcpu ioctls
To configure a non-default set of vector lengths,
KVM_ARM_VCPU_SET_SVE_VLS can be called: this would only be permitted
before the vcpu is first run.
This is primarily intended for supporting migration, by providing a
robust check that the destination node will run the vcpu correctly.
In a cluster with non-uniform SVE implementation across nodes, this
also allows a specific set of VLs to be requested that the caller
knows is usable across the whole cluster.
For migration purposes, userspace would need to do
KVM_ARM_VCPU_GET_SVE_VLS@the origin node and store the returned
set as VM metadata: on the destination node,
KVM_ARM_VCPU_SET_SVE_VLS should be used to request that exact set of
VLs: if the destination node can't support that set of VLs, the call
will fail.
The interface would look something like:
ioctl(vcpu_fd, KVM_ARM_SVE_SET_VLS, __u64 vqs[SVE_VQ_MAX / 64]);
How to expose this to the user in an intelligible way would be a
problem for userspace to solve.
At present, other than initialising each vcpu to the maximum
supportable set of VLs, I don't propose having a way to probe for
what sets of VLs are supportable: the above call either succeeds or
fails.
Cheers
---Dave
^ permalink raw reply
* [RFC v2] arm64: KVM: KVM API extensions for SVE
From: Peter Maydell @ 2017-12-13 16:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171213165543.GO22781@e103592.cambridge.arm.com>
On 13 December 2017 at 16:55, Dave Martin <Dave.Martin@arm.com> wrote:
> Vector length control:
>
> Some means is needed to determine the set of vector lengths visible
> to guest software running on a vcpu.
>
> When a vcpu is created, the set would be defaulted to the maximal set
> that can be supported while permitting each vcpu to run on any host
> CPU. SVE has some virtualisation quirks which mean that this set may
> exclude some vector lengths that are available for host userspace
> applications. The common case should be that the sets are the same
> however.
>
> * New ioctl KVM_ARM_VCPU_{SET,GET}_SVE_VLS to set or retrieve the set of
> vector lengths available to the guest.
>
> Adding random vcpu ioctls
>
> To configure a non-default set of vector lengths,
> KVM_ARM_VCPU_SET_SVE_VLS can be called: this would only be permitted
> before the vcpu is first run.
>
> This is primarily intended for supporting migration, by providing a
> robust check that the destination node will run the vcpu correctly.
> In a cluster with non-uniform SVE implementation across nodes, this
> also allows a specific set of VLs to be requested that the caller
> knows is usable across the whole cluster.
>
> For migration purposes, userspace would need to do
> KVM_ARM_VCPU_GET_SVE_VLS at the origin node and store the returned
> set as VM metadata: on the destination node,
> KVM_ARM_VCPU_SET_SVE_VLS should be used to request that exact set of
> VLs: if the destination node can't support that set of VLs, the call
> will fail.
Can we just do this with the existing ONE_REG APIs? If you
expose this via those, then QEMU doesn't need to do anything
for migration at all. This is the same way we (intend to)
check any optional-feature compatibility at each end, for
instance features exposed in guest-visible ID registers.
It's just that the "register" for the SVE vector-lengths case
is one that's not visible to the guest.
thanks
-- PMM
^ permalink raw reply
* [PATCH v2 1/3] arm64: mm: Support Common Not Private translations
From: Vladimir Murzin @ 2017-12-13 16:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5A313704.1090503@arm.com>
Hi James,
On 13/12/17 14:19, James Morse wrote:
> Hi Vladimir,
>
> On 11/10/17 13:19, Vladimir Murzin wrote:
>> Common Not Private (CNP) is a feature of ARMv8.2 extension which
>> allows translation table entries to be shared between different PEs in
>> the same inner shareable domain, so the hardware can use this fact to
>> optimise the caching of such entries in the TLB.
>>
>> CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to
>> the hardware that the translation table entries pointed to by this
>> TTBR are the same as every PE in the same inner shareable domain for
>> which the equivalent TTBR also has CNP bit set. In case CNP bit is set
>> but TTBR does not point at the same translation table entries or a
>> given ASID and VMID, then the system is mis-configured, so the results
>> of translations are UNPREDICTABLE.
>>
>> This patch adds support for Common Not Private translations on
>> different exceptions levels:
>>
>> (1) For EL0 there are a few cases we need to care of changes in
>> TTBR0_EL1:
>> - a switch to idmap
>> - software emulated PAN
>> we rule out latter via Kconfig options and for the former we make
>> sure that CNP is set for non-zero ASIDs only.
>>
>> (2) For EL1 we postpone setting CNP till all cpus are up and rely on
>> cpufeature framework to 1) patch the code which is sensitive to
>> CNP and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be
>> reprogrammed as result of hibernation or cpuidle (via __enable_mmu).
>> cpuidle's path has been changed to restore CnP and for hibernation
>> the code has been changed to save raw TTBR1_EL1 and blindly restore
>> it on resume.
>
>
> While I remember:
>
> This feature is going to be fun for kdump, we may leave secondary CPUs running
> if they don't take the IPI to crash-out of the kernel. Worse, if we don't have
> PSCI they just spin in a loop while the surviving CPU brings up the crash kernel
> and maybe-enables CNP...
>
> I think the best fix for this is to refuse to enable CNP at all if we're a crash
> kernel. There is stuff in the DT to indicate this... we should know about the
> 'elfcorehdr' before cpufeature runs. (I don't think we should rely on the
> cmdline option).
>
> kexec is unaffected because it always powers-off the secondary CPUs before
> leaving the old kernel. This behaves much more like a normal boot.
Thanks, I'll look into it.
Vladimir
>
>
> Thanks,
>
> James
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox