* [PATCH] gpio: zynq: Setup chip->base based on alias ID
From: Michal Simek @ 2018-05-23 10:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdbCsUBMcHg+xGOQCBmhoJsqh9VjK+4pjhG5Hf6Fjzwjww@mail.gmail.com>
>> If you take a look at
>> arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
>> which is Ultra96 board gpio-line-names are filled there for the whole PS
>> part. Definitely take a look and let know if you find out any issue there.
>
> It looks good. I even managed to boot my Ultra96 board which
> was sent over as price in some competition (great hardware!)
Wonderful that you get hw available to be closer to issues.
Maybe it would be worth to give you some guidance what to do with PL and
how to add more stuff there especially in connection to GPIO.
We have pinctrl driver in vendor tree but my colleague needs to upstream
firmware interface first.
>> zynq/zynqmp gpio controller contains PS pins (hard part) and PL pins
>> coming to logic.
>>
>> I can't describe PL gpio pins because it can be whatever even I have
>> done that for one fixed hw design.
>>
>> Interesting part on that sha1 you shared is how "NC" pin is described.
>>
>> gpio pin 35 I have on zcu100 as "" but it should be maybe TP_PAD which
>> is really just a pad on real board. And the rest of "" gpio names are
>> connected to PL.
>
> How to handle anything routed to/from programmable logic is
> in a bit of mess right now, I understand this work isn't the
> easiest :/
I am happy to discuss this to find out a way how this can be handled.
There are several things together.
On arm64 how to disable/enable access for NS software.
Maybe also separate pins PS pins from PL pins. Right now all are
together and not sure if this is ideal in sense of DT overal for
example. Separate nodes would enable better flexibility. Also if you
look at partial reconfiguration you can just route just part of pins
which suggest list of gpios used there.
Thanks,
Michal
^ permalink raw reply
* [PATCH 07/14] arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
From: Julien Grall @ 2018-05-23 10:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-8-marc.zyngier@arm.com>
Hi Marc,
On 05/22/2018 04:06 PM, Marc Zyngier wrote:
> In order to avoid checking arm64_ssbd_callback_required on each
> kernel entry/exit even if no mitigation is required, let's
> add yet another alternative that by default jumps over the mitigation,
> and that gets nop'ed out if we're doing dynamic mitigation.
>
> Think of it as a poor man's static key...
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Cheers,
> ---
> arch/arm64/kernel/cpu_errata.c | 14 ++++++++++++++
> arch/arm64/kernel/entry.S | 3 +++
> 2 files changed, 17 insertions(+)
>
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index f1d4e75b0ddd..8f686f39b9c1 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -283,6 +283,20 @@ void __init arm64_update_smccc_conduit(struct alt_instr *alt,
> *updptr = cpu_to_le32(insn);
> }
>
> +void __init arm64_enable_wa2_handling(struct alt_instr *alt,
> + __le32 *origptr, __le32 *updptr,
> + int nr_inst)
> +{
> + BUG_ON(nr_inst != 1);
> + /*
> + * Only allow mitigation on EL1 entry/exit and guest
> + * ARCH_WORKAROUND_2 handling if the SSBD state allows it to
> + * be flipped.
> + */
> + if (arm64_get_ssbd_state() == ARM64_SSBD_EL1_ENTRY)
> + *updptr = cpu_to_le32(aarch64_insn_gen_nop());
> +}
> +
> static void do_ssbd(bool state)
> {
> switch (psci_ops.conduit) {
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 29ad672a6abd..e6f6e2339b22 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -142,6 +142,9 @@ alternative_else_nop_endif
> // to save/restore them if required.
> .macro apply_ssbd, state, targ, tmp1, tmp2
> #ifdef CONFIG_ARM64_SSBD
> +alternative_cb arm64_enable_wa2_handling
> + b \targ
> +alternative_cb_end
> ldr_this_cpu \tmp2, arm64_ssbd_callback_required, \tmp1
> cbz \tmp2, \targ
> mov w0, #ARM_SMCCC_ARCH_WORKAROUND_2
>
--
Julien Grall
^ permalink raw reply
* [PATCH 06/14] arm64: ssbd: Add global mitigation state accessor
From: Julien Grall @ 2018-05-23 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-7-marc.zyngier@arm.com>
Hi Marc,
On 05/22/2018 04:06 PM, Marc Zyngier wrote:
> We're about to need the mitigation state in various parts of the
> kernel in order to do the right thing for userspace and guests.
>
> Let's expose an accessor that will let other subsystems know
> about the state.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Cheers,
> ---
> arch/arm64/include/asm/cpufeature.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 9bc548e22784..1bacdf57f0af 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -543,6 +543,16 @@ static inline u64 read_zcr_features(void)
> #define ARM64_SSBD_FORCE_ENABLE 2
> #define ARM64_SSBD_MITIGATED 3
>
> +static inline int arm64_get_ssbd_state(void)
> +{
> +#ifdef CONFIG_ARM64_SSBD
> + extern int ssbd_state;
> + return ssbd_state;
> +#else
> + return ARM64_SSBD_UNKNOWN;
> +#endif
> +}
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
>
--
Julien Grall
^ permalink raw reply
* [PATCH 05/14] arm64: Add 'ssbd' command-line option
From: Julien Grall @ 2018-05-23 10:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-6-marc.zyngier@arm.com>
Hi Marc,
On 05/22/2018 04:06 PM, Marc Zyngier wrote:
> On a system where the firmware implements ARCH_WORKAROUND_2,
> it may be useful to either permanently enable or disable the
> workaround for cases where the user decides that they'd rather
> not get a trap overhead, and keep the mitigation permanently
> on or off instead of switching it on exception entry/exit.
>
> In any case, default to the mitigation being enabled.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Cheers,
> ---
> Documentation/admin-guide/kernel-parameters.txt | 17 ++++
> arch/arm64/include/asm/cpufeature.h | 6 ++
> arch/arm64/kernel/cpu_errata.c | 102 ++++++++++++++++++++----
> 3 files changed, 109 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f2040d46f095..646e112c6f63 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4092,6 +4092,23 @@
> expediting. Set to zero to disable automatic
> expediting.
>
> + ssbd= [ARM64,HW]
> + Speculative Store Bypass Disable control
> +
> + On CPUs that are vulnerable to the Speculative
> + Store Bypass vulnerability and offer a
> + firmware based mitigation, this parameter
> + indicates how the mitigation should be used:
> +
> + force-on: Unconditionnaly enable mitigation for
> + for both kernel and userspace
> + force-off: Unconditionnaly disable mitigation for
> + for both kernel and userspace
> + kernel: Always enable mitigation in the
> + kernel, and offer a prctl interface
> + to allow userspace to register its
> + interest in being mitigated too.
> +
> stack_guard_gap= [MM]
> override the default stack gap protection. The value
> is in page units and it defines how many pages prior
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 09b0f2a80c8f..9bc548e22784 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -537,6 +537,12 @@ static inline u64 read_zcr_features(void)
> return zcr;
> }
>
> +#define ARM64_SSBD_UNKNOWN -1
> +#define ARM64_SSBD_FORCE_DISABLE 0
> +#define ARM64_SSBD_EL1_ENTRY 1
> +#define ARM64_SSBD_FORCE_ENABLE 2
> +#define ARM64_SSBD_MITIGATED 3
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 7fd6d5b001f5..f1d4e75b0ddd 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -235,6 +235,38 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
> #ifdef CONFIG_ARM64_SSBD
> DEFINE_PER_CPU_READ_MOSTLY(u64, arm64_ssbd_callback_required);
>
> +int ssbd_state __read_mostly = ARM64_SSBD_EL1_ENTRY;
> +
> +static const struct ssbd_options {
> + const char *str;
> + int state;
> +} ssbd_options[] = {
> + { "force-on", ARM64_SSBD_FORCE_ENABLE, },
> + { "force-off", ARM64_SSBD_FORCE_DISABLE, },
> + { "kernel", ARM64_SSBD_EL1_ENTRY, },
> +};
> +
> +static int __init ssbd_cfg(char *buf)
> +{
> + int i;
> +
> + if (!buf || !buf[0])
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(ssbd_options); i++) {
> + int len = strlen(ssbd_options[i].str);
> +
> + if (strncmp(buf, ssbd_options[i].str, len))
> + continue;
> +
> + ssbd_state = ssbd_options[i].state;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +early_param("ssbd", ssbd_cfg);
> +
> void __init arm64_update_smccc_conduit(struct alt_instr *alt,
> __le32 *origptr, __le32 *updptr,
> int nr_inst)
> @@ -272,44 +304,82 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
> int scope)
> {
> struct arm_smccc_res res;
> - bool supported = true;
> + bool required = true;
> + s32 val;
>
> WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
>
> - if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
> + if (psci_ops.smccc_version == SMCCC_VERSION_1_0) {
> + ssbd_state = ARM64_SSBD_UNKNOWN;
> return false;
> + }
>
> - /*
> - * The probe function return value is either negative
> - * (unsupported or mitigated), positive (unaffected), or zero
> - * (requires mitigation). We only need to do anything in the
> - * last case.
> - */
> switch (psci_ops.conduit) {
> case PSCI_CONDUIT_HVC:
> arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
> ARM_SMCCC_ARCH_WORKAROUND_2, &res);
> - if ((int)res.a0 != 0)
> - supported = false;
> break;
>
> case PSCI_CONDUIT_SMC:
> arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
> ARM_SMCCC_ARCH_WORKAROUND_2, &res);
> - if ((int)res.a0 != 0)
> - supported = false;
> break;
>
> default:
> - supported = false;
> + ssbd_state = ARM64_SSBD_UNKNOWN;
> + return false;
> }
>
> - if (supported) {
> - __this_cpu_write(arm64_ssbd_callback_required, 1);
> + val = (s32)res.a0;
> +
> + switch (val) {
> + case SMCCC_RET_NOT_SUPPORTED:
> + ssbd_state = ARM64_SSBD_UNKNOWN;
> + return false;
> +
> + case SMCCC_RET_NOT_REQUIRED:
> + ssbd_state = ARM64_SSBD_MITIGATED;
> + return false;
> +
> + case SMCCC_RET_SUCCESS:
> + required = true;
> + break;
> +
> + case 1: /* Mitigation not required on this CPU */
> + required = false;
> + break;
> +
> + default:
> + WARN_ON(1);
> + return false;
> + }
> +
> + switch (ssbd_state) {
> + case ARM64_SSBD_FORCE_DISABLE:
> + pr_info_once("%s disabled from command-line\n", entry->desc);
> + do_ssbd(false);
> + required = false;
> + break;
> +
> + case ARM64_SSBD_EL1_ENTRY:
> + if (required) {
> + __this_cpu_write(arm64_ssbd_callback_required, 1);
> + do_ssbd(true);
> + }
> + break;
> +
> + case ARM64_SSBD_FORCE_ENABLE:
> + pr_info_once("%s forced from command-line\n", entry->desc);
> do_ssbd(true);
> + required = true;
> + break;
> +
> + default:
> + WARN_ON(1);
> + break;
> }
>
> - return supported;
> + return required;
> }
> #endif /* CONFIG_ARM64_SSBD */
>
>
--
Julien Grall
^ permalink raw reply
* [PATCH 04/14] arm64: Add ARCH_WORKAROUND_2 probing
From: Julien Grall @ 2018-05-23 10:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-5-marc.zyngier@arm.com>
Hi Marc,
On 05/22/2018 04:06 PM, Marc Zyngier wrote:
> As for Spectre variant-2, we rely on SMCCC 1.1 to provide the
> discovery mechanism for detecting the SSBD mitigation.
>
> A new capability is also allocated for that purpose, and a
> config option.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/Kconfig | 9 ++++++
> arch/arm64/include/asm/cpucaps.h | 3 +-
> arch/arm64/kernel/cpu_errata.c | 69 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index eb2cf4938f6d..b2103b4df467 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -938,6 +938,15 @@ config HARDEN_EL2_VECTORS
>
> If unsure, say Y.
>
> +config ARM64_SSBD
> + bool "Speculative Store Bypass Disable" if EXPERT
> + default y
> + help
> + This enables mitigation of the bypassing of previous stores
> + by speculative loads.
> +
> + If unsure, say Y.
> +
> menuconfig ARMV8_DEPRECATED
> bool "Emulate deprecated/obsolete ARMv8 instructions"
> depends on COMPAT
> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> index bc51b72fafd4..5b2facf786ba 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -48,7 +48,8 @@
> #define ARM64_HAS_CACHE_IDC 27
> #define ARM64_HAS_CACHE_DIC 28
> #define ARM64_HW_DBM 29
> +#define ARM64_SSBD 30
NIT: Could you indent 30 the same way as the other number?
Reviewed-by: Julien Grall <julien.grall@arm.com>
Cheers,
--
Julien Grall
^ permalink raw reply
* [PATCH 03/14] arm64: Add per-cpu infrastructure to call ARCH_WORKAROUND_2
From: Julien Grall @ 2018-05-23 10:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-4-marc.zyngier@arm.com>
Hi Marc,
On 05/22/2018 04:06 PM, Marc Zyngier wrote:
> In a heterogeneous system, we can end up with both affected and
> unaffected CPUs. Let's check their status before calling into the
> firmware.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Cheers,
> ---
> arch/arm64/kernel/cpu_errata.c | 2 ++
> arch/arm64/kernel/entry.S | 11 +++++++----
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 46b3aafb631a..0288d6cf560e 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -233,6 +233,8 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
> #endif /* CONFIG_HARDEN_BRANCH_PREDICTOR */
>
> #ifdef CONFIG_ARM64_SSBD
> +DEFINE_PER_CPU_READ_MOSTLY(u64, arm64_ssbd_callback_required);
> +
> void __init arm64_update_smccc_conduit(struct alt_instr *alt,
> __le32 *origptr, __le32 *updptr,
> int nr_inst)
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index f33e6aed3037..29ad672a6abd 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -140,8 +140,10 @@ alternative_else_nop_endif
>
> // This macro corrupts x0-x3. It is the caller's duty
> // to save/restore them if required.
> - .macro apply_ssbd, state
> + .macro apply_ssbd, state, targ, tmp1, tmp2
> #ifdef CONFIG_ARM64_SSBD
> + ldr_this_cpu \tmp2, arm64_ssbd_callback_required, \tmp1
> + cbz \tmp2, \targ
> mov w0, #ARM_SMCCC_ARCH_WORKAROUND_2
> mov w1, #\state
> alternative_cb arm64_update_smccc_conduit
> @@ -176,12 +178,13 @@ alternative_cb_end
> ldr x19, [tsk, #TSK_TI_FLAGS] // since we can unmask debug
> disable_step_tsk x19, x20 // exceptions when scheduling.
>
> - apply_ssbd 1
> + apply_ssbd 1, 1f, x22, x23
>
> #ifdef CONFIG_ARM64_SSBD
> ldp x0, x1, [sp, #16 * 0]
> ldp x2, x3, [sp, #16 * 1]
> #endif
> +1:
>
> mov x29, xzr // fp pointed to user-space
> .else
> @@ -323,8 +326,8 @@ alternative_if ARM64_WORKAROUND_845719
> alternative_else_nop_endif
> #endif
> 3:
> - apply_ssbd 0
> -
> + apply_ssbd 0, 5f, x0, x1
> +5:
> .endif
>
> msr elr_el1, x21 // set up the return data
>
--
Julien Grall
^ permalink raw reply
* [PATCH v3 00/13] add power domain support for Rockchip Socs
From: Ulf Hansson @ 2018-05-23 10:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
On 23 May 2018 at 08:48, Elaine Zhang <zhangqing@rock-chips.com> wrote:
> add power domain support for RK3036/RK3128/RK3228/PX30 Soc.
> fix up the wrong value when set power domain up.
>
> Change in V2:
> Fix up the commit message description and Assign author.
>
> Change in V3:
> [PATCH 01/13]: The Copyright description use SPDX tag instead.
> [PATCH 05/13]: The Copyright description use SPDX tag instead.
> [PATCH 08/13]: The Copyright description use SPDX tag instead.
> [PATCH 11/13]: The Copyright description use SPDX tag instead.
>
> Caesar Wang (3):
> dt-bindings: power: add RK3036 SoCs header for power-domain
> dt-bindings: add binding for rk3036 power domains
> Soc: rockchip: power-domain: add power domain support for rk3036
>
> Elaine Zhang (6):
> dt-bindings: power: add RK3128 SoCs header for power-domain
> dt-bindings: add binding for rk3128 power domains
> soc: rockchip: power-domain: add power domain support for rk3128
> dt-bindings: power: add RK3228 SoCs header for power-domain
> dt-bindings: add binding for rk3228 power domains
> soc: rockchip: power-domain: add power domain support for rk3228
>
> Finley Xiao (4):
> soc: rockchip: power-domain: Fix wrong value when power up pd
> dt-bindings: power: add PX30 SoCs header for power-domain
> dt-bindings: add binding for px30 power domains
> soc: rockchip: power-domain: add power domain support for px30
>
> .../bindings/soc/rockchip/power_domain.txt | 12 +++
> drivers/soc/rockchip/pm_domains.c | 116 ++++++++++++++++++++-
> include/dt-bindings/power/px30-power.h | 27 +++++
> include/dt-bindings/power/rk3036-power.h | 13 +++
> include/dt-bindings/power/rk3128-power.h | 14 +++
> include/dt-bindings/power/rk3228-power.h | 21 ++++
> 6 files changed, 202 insertions(+), 1 deletion(-)
> create mode 100644 include/dt-bindings/power/px30-power.h
> create mode 100644 include/dt-bindings/power/rk3036-power.h
> create mode 100644 include/dt-bindings/power/rk3128-power.h
> create mode 100644 include/dt-bindings/power/rk3228-power.h
>
> --
> 1.9.1
>
>
Seems like the changes in v3 is very small, so feel free to add, for the series:
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
^ permalink raw reply
* [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-23 9:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523094033.GW17671@n2100.armlinux.org.uk>
On 23-05-18, 10:40, Russell King - ARM Linux wrote:
> On Wed, May 23, 2018 at 12:05:24PM +0300, Ilia Lin wrote:
> > + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> > + if (IS_ERR(np))
> > + return PTR_ERR(np);
> ...
> > +
> > + pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> > + if (!IS_ERR(pdev))
>
> Do you need to hold a reference to `np' here?
I am starting to feel bad for Ilia now. The problem is that there was
a lot of stuff wrong with the patch and even with so many reviewers it
wasn't easy to notice all the problems it had.
But you are right, this reference needs to be dropped.
--
viresh
^ permalink raw reply
* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-23 9:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <22068fb1-f6e8-f2ec-d7f4-ab9e93469d7f@nvidia.com>
On 23 May 2018 at 11:45, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 23/05/18 10:33, Ulf Hansson wrote:
>>
>> On 23 May 2018 at 11:27, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>>
>>>
>>>
>>> On 05/23/2018 02:37 PM, Jon Hunter wrote:
>>>>
>>>>
>>>> On 23/05/18 07:12, Ulf Hansson wrote:
>>>>
>>>> ...
>>>>
>>>>>>>>> Thanks for sending this. Believe it or not this has still been on
>>>>>>>>> my to-do list
>>>>>>>>> and so we definitely need a solution for Tegra.
>>>>>>>>>
>>>>>>>>> Looking at the above it appears that additional power-domains
>>>>>>>>> exposed as devices
>>>>>>>>> to the client device. So I assume that this means that the drivers
>>>>>>>>> for devices
>>>>>>>>> with multiple power-domains will need to call RPM APIs for each of
>>>>>>>>> these
>>>>>>>>> additional power-domains. Is that correct?
>>>>>>>>
>>>>>>>>
>>>>>>>> They can, but should not!
>>>>>>>>
>>>>>>>> Instead, the driver shall use device_link_add() and
>>>>>>>> device_link_del(),
>>>>>>>> dynamically, depending on what PM domain that their original device
>>>>>>>> needs for the current running use case.
>>>>>>>>
>>>>>>>> In that way, they keep existing runtime PM deployment, operating on
>>>>>>>> its original device.
>>>>>>>
>>>>>>>
>>>>>>> OK, sounds good. Any reason why the linking cannot be handled by the
>>>>>>> above API? Is there a use-case where you would not want it linked?
>>>>>>
>>>>>>
>>>>>> I am guessing the linking is what would give the driver the ability to
>>>>>> decide which subset of powerdomains it actually wants to control
>>>>>> at any point using runtime PM. If we have cases wherein the driver
>>>>>> would want to turn on/off _all_ its associated powerdomains _always_
>>>>>> then a default linking of all would help.
>>>>>
>>>>>
>>>>> First, I think we need to decide on *where* the linking should be
>>>>> done, not at both places, as that would just mess up synchronization
>>>>> of who is responsible for calling the device_link_del() at detach.
>>>>>
>>>>> Second, It would in principle be fine to call device_link_add() and
>>>>> device_link_del() as a part of the attach/detach APIs. However, there
>>>>> is a downside to such solution, which would be that the driver then
>>>>> needs call the detach API, just to do device_link_del(). Of course
>>>>> then it would also needs to call the attach API later if/when needed.
>>>>> Doing this adds unnecessary overhead - comparing to just let the
>>>>> driver call device_link_add|del() when needed. On the upside, yes, it
>>>>> would put less burden on the drivers as it then only needs to care
>>>>> about using one set of functions.
>>>>>
>>>>> Which solution do you prefer?
>>>>
>>>>
>>>> Any reason why we could not add a 'boolean' argument to the API to
>>>> indicate whether the new device should be linked? I think that I prefer the
>>>> API handles it, but I can see there could be instances where drivers may
>>>> wish to handle it themselves.
>>>>
>>>> Rajendra, do you have a use-case right now where the driver would want
>>>> to handle the linking?
>>>
>>>
>>> So if I understand this right, any driver which does want to control
>>> individual powerdomain state would
>>> need to do the linking itself right?
>>>
>>> What I am saying is, if I have device A, with powerdomains X and Y, and
>>> if I want to turn on only X,
>>> then I would want only X to be linked with A, and at a later point if I
>>> want both X and Y to be turned on,
>>> I would then go ahead and link both X and Y to A? Is that correct or did
>>> I get it all wrong?
>>
>>
>> Correct!
>>
>>>
>>> I know atleast Camera on msm8996 would need to do this since it has 2 vfe
>>> powerdoamins, which can be
>>> turned on one at a time (depending on what resolution needs to be
>>> supported) or both together if we
>>> really need very high resolution using both vfe modules.
>>
>>
>> I think this is also the case for the Tegra XUSB subsystem.
>>
>> The usb device is always attached to one PM domain, but depending on
>> if super-speed mode is used, another PM domain for that logic needs to
>> be powered on as well.
>>
>> Jon, please correct me if I am wrong!
>
>
> Yes this is technically correct, however, in reality I think we are always
> going to enable the superspeed domain if either the host or device domain is
> enabled. So we would probably always link the superspeed with the host and
> device devices.
Why? Wouldn't that waste power if the superspeed mode isn't used?
Kind regards
Uffe
^ permalink raw reply
* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-23 9:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFoqLPRwEJ9b+XBpB4Zy79SGEC-a8V8shNhMkYOCQa0=oA@mail.gmail.com>
On 23/05/18 10:33, Ulf Hansson wrote:
> On 23 May 2018 at 11:27, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>
>>
>> On 05/23/2018 02:37 PM, Jon Hunter wrote:
>>>
>>> On 23/05/18 07:12, Ulf Hansson wrote:
>>>
>>> ...
>>>
>>>>>>>> Thanks for sending this. Believe it or not this has still been on my to-do list
>>>>>>>> and so we definitely need a solution for Tegra.
>>>>>>>>
>>>>>>>> Looking at the above it appears that additional power-domains exposed as devices
>>>>>>>> to the client device. So I assume that this means that the drivers for devices
>>>>>>>> with multiple power-domains will need to call RPM APIs for each of these
>>>>>>>> additional power-domains. Is that correct?
>>>>>>>
>>>>>>> They can, but should not!
>>>>>>>
>>>>>>> Instead, the driver shall use device_link_add() and device_link_del(),
>>>>>>> dynamically, depending on what PM domain that their original device
>>>>>>> needs for the current running use case.
>>>>>>>
>>>>>>> In that way, they keep existing runtime PM deployment, operating on
>>>>>>> its original device.
>>>>>>
>>>>>> OK, sounds good. Any reason why the linking cannot be handled by the above API? Is there a use-case where you would not want it linked?
>>>>>
>>>>> I am guessing the linking is what would give the driver the ability to decide which subset of powerdomains it actually wants to control
>>>>> at any point using runtime PM. If we have cases wherein the driver would want to turn on/off _all_ its associated powerdomains _always_
>>>>> then a default linking of all would help.
>>>>
>>>> First, I think we need to decide on *where* the linking should be
>>>> done, not at both places, as that would just mess up synchronization
>>>> of who is responsible for calling the device_link_del() at detach.
>>>>
>>>> Second, It would in principle be fine to call device_link_add() and
>>>> device_link_del() as a part of the attach/detach APIs. However, there
>>>> is a downside to such solution, which would be that the driver then
>>>> needs call the detach API, just to do device_link_del(). Of course
>>>> then it would also needs to call the attach API later if/when needed.
>>>> Doing this adds unnecessary overhead - comparing to just let the
>>>> driver call device_link_add|del() when needed. On the upside, yes, it
>>>> would put less burden on the drivers as it then only needs to care
>>>> about using one set of functions.
>>>>
>>>> Which solution do you prefer?
>>>
>>> Any reason why we could not add a 'boolean' argument to the API to indicate whether the new device should be linked? I think that I prefer the API handles it, but I can see there could be instances where drivers may wish to handle it themselves.
>>>
>>> Rajendra, do you have a use-case right now where the driver would want to handle the linking?
>>
>> So if I understand this right, any driver which does want to control individual powerdomain state would
>> need to do the linking itself right?
>>
>> What I am saying is, if I have device A, with powerdomains X and Y, and if I want to turn on only X,
>> then I would want only X to be linked with A, and at a later point if I want both X and Y to be turned on,
>> I would then go ahead and link both X and Y to A? Is that correct or did I get it all wrong?
>
> Correct!
>
>>
>> I know atleast Camera on msm8996 would need to do this since it has 2 vfe powerdoamins, which can be
>> turned on one at a time (depending on what resolution needs to be supported) or both together if we
>> really need very high resolution using both vfe modules.
>
> I think this is also the case for the Tegra XUSB subsystem.
>
> The usb device is always attached to one PM domain, but depending on
> if super-speed mode is used, another PM domain for that logic needs to
> be powered on as well.
>
> Jon, please correct me if I am wrong!
Yes this is technically correct, however, in reality I think we are
always going to enable the superspeed domain if either the host or
device domain is enabled. So we would probably always link the
superspeed with the host and device devices.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH] gpio: zynq: Setup chip->base based on alias ID
From: Linus Walleij @ 2018-05-23 9:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8b344d48-592d-8be8-bea0-e29b3e8469a1@xilinx.com>
On Tue, May 15, 2018 at 3:26 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> Have you done any decision about this patch?
Yeah I applied the patch. It's a bit of "choose the lesser evil" approach
but what can I do...
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] gpio: zynq: Setup chip->base based on alias ID
From: Linus Walleij @ 2018-05-23 9:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57e4bbdd-1e30-0352-f758-998b64a6b77f@xilinx.com>
On Wed, May 2, 2018 at 4:19 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 2.5.2018 15:56, Linus Walleij wrote:
>> Would it be possible that I apply the patch, and somehow also
>> establish some understanding with all users of the Xilinx
>> platform that whatever legacy applications are out there
>> must start to migrate towards using the character device so
>> this reliance on the numberspace doesn't stick around forever?
>
> When someone contacts me for asking guidance for gpio I am telling them
> not to use legacy sysfs interface and use libgpiod.
Awesome, thanks for your support :)
> Last one was a week
> ago in connection to Ultra96 and libmraa.
>
> But even chardev is not supported there now.
> https://github.com/intel-iot-devkit/mraa/issues/713
Hm I hope this happens soon. Manavanninan is doing a great
job in switching this over, I just have to accept that the pace
isn't always what it should be in all upstream projects, and
deal with an imperfect world.
> If you take a look at
> arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> which is Ultra96 board gpio-line-names are filled there for the whole PS
> part. Definitely take a look and let know if you find out any issue there.
It looks good. I even managed to boot my Ultra96 board which
was sent over as price in some competition (great hardware!)
> zynq/zynqmp gpio controller contains PS pins (hard part) and PL pins
> coming to logic.
>
> I can't describe PL gpio pins because it can be whatever even I have
> done that for one fixed hw design.
>
> Interesting part on that sha1 you shared is how "NC" pin is described.
>
> gpio pin 35 I have on zcu100 as "" but it should be maybe TP_PAD which
> is really just a pad on real board. And the rest of "" gpio names are
> connected to PL.
How to handle anything routed to/from programmable logic is
in a bit of mess right now, I understand this work isn't the
easiest :/
Let's go for this patch and try to improve the situation gradually
moving forward.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Russell King - ARM Linux @ 2018-05-23 9:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527066324-24726-1-git-send-email-ilialin@codeaurora.org>
On Wed, May 23, 2018 at 12:05:24PM +0300, Ilia Lin wrote:
> + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> + if (IS_ERR(np))
> + return PTR_ERR(np);
...
> +
> + pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> + if (!IS_ERR(pdev))
Do you need to hold a reference to `np' here?
> + return 0;
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH v2 13/40] vfio: Add support for Shared Virtual Addressing
From: Xu Zaibo @ 2018-05-23 9:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-14-jean-philippe.brucker@arm.com>
Hi,
On 2018/5/12 3:06, Jean-Philippe Brucker wrote:
> Add two new ioctls for VFIO containers. VFIO_IOMMU_BIND_PROCESS creates a
> bond between a container and a process address space, identified by a
> Process Address Space ID (PASID). Devices in the container append this
> PASID to DMA transactions in order to access the process' address space.
> The process page tables are shared with the IOMMU, and mechanisms such as
> PCI ATS/PRI are used to handle faults. VFIO_IOMMU_UNBIND_PROCESS removes a
> bond created with VFIO_IOMMU_BIND_PROCESS.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>
>
>
>
>
>
> +static int vfio_iommu_bind_group(struct vfio_iommu *iommu,
> + struct vfio_group *group,
> + struct vfio_mm *vfio_mm)
> +{
> + int ret;
> + bool enabled_sva = false;
> + struct vfio_iommu_sva_bind_data data = {
> + .vfio_mm = vfio_mm,
> + .iommu = iommu,
> + .count = 0,
> + };
> +
> + if (!group->sva_enabled) {
> + ret = iommu_group_for_each_dev(group->iommu_group, NULL,
> + vfio_iommu_sva_init);
Do we need to do *sva_init here or do anything to avoid repeated initiation?
while another process already did initiation at this device, I think
that current process will get an EEXIST.
Thanks.
> + if (ret)
> + return ret;
> +
> + group->sva_enabled = enabled_sva = true;
> + }
> +
> + ret = iommu_group_for_each_dev(group->iommu_group, &data,
> + vfio_iommu_sva_bind_dev);
> + if (ret && data.count > 1)
> + iommu_group_for_each_dev(group->iommu_group, vfio_mm,
> + vfio_iommu_sva_unbind_dev);
> + if (ret && enabled_sva) {
> + iommu_group_for_each_dev(group->iommu_group, NULL,
> + vfio_iommu_sva_shutdown);
> + group->sva_enabled = false;
> + }
> +
> + return ret;
> +}
>
>
>
> @@ -1442,6 +1636,10 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (ret)
> goto out_detach;
>
> + ret = vfio_iommu_replay_bind(iommu, group);
> + if (ret)
> + goto out_detach;
> +
> if (resv_msi) {
> ret = iommu_get_msi_cookie(domain->domain, resv_msi_base);
> if (ret)
> @@ -1547,6 +1745,11 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
> continue;
>
> iommu_detach_group(domain->domain, iommu_group);
> + if (group->sva_enabled) {
> + iommu_group_for_each_dev(iommu_group, NULL,
> + vfio_iommu_sva_shutdown);
> + group->sva_enabled = false;
> + }
Here, why shut down here? If another process is working on the device,
there may be a crash?
Thanks.
> list_del(&group->next);
> kfree(group);
> /*
> @@ -1562,6 +1765,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
>
^ permalink raw reply
* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-23 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <00ba829d-faf4-168c-db00-531621b9280f@codeaurora.org>
On 23 May 2018 at 11:27, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
>
> On 05/23/2018 02:37 PM, Jon Hunter wrote:
>>
>> On 23/05/18 07:12, Ulf Hansson wrote:
>>
>> ...
>>
>>>>>>> Thanks for sending this. Believe it or not this has still been on my to-do list
>>>>>>> and so we definitely need a solution for Tegra.
>>>>>>>
>>>>>>> Looking at the above it appears that additional power-domains exposed as devices
>>>>>>> to the client device. So I assume that this means that the drivers for devices
>>>>>>> with multiple power-domains will need to call RPM APIs for each of these
>>>>>>> additional power-domains. Is that correct?
>>>>>>
>>>>>> They can, but should not!
>>>>>>
>>>>>> Instead, the driver shall use device_link_add() and device_link_del(),
>>>>>> dynamically, depending on what PM domain that their original device
>>>>>> needs for the current running use case.
>>>>>>
>>>>>> In that way, they keep existing runtime PM deployment, operating on
>>>>>> its original device.
>>>>>
>>>>> OK, sounds good. Any reason why the linking cannot be handled by the above API? Is there a use-case where you would not want it linked?
>>>>
>>>> I am guessing the linking is what would give the driver the ability to decide which subset of powerdomains it actually wants to control
>>>> at any point using runtime PM. If we have cases wherein the driver would want to turn on/off _all_ its associated powerdomains _always_
>>>> then a default linking of all would help.
>>>
>>> First, I think we need to decide on *where* the linking should be
>>> done, not at both places, as that would just mess up synchronization
>>> of who is responsible for calling the device_link_del() at detach.
>>>
>>> Second, It would in principle be fine to call device_link_add() and
>>> device_link_del() as a part of the attach/detach APIs. However, there
>>> is a downside to such solution, which would be that the driver then
>>> needs call the detach API, just to do device_link_del(). Of course
>>> then it would also needs to call the attach API later if/when needed.
>>> Doing this adds unnecessary overhead - comparing to just let the
>>> driver call device_link_add|del() when needed. On the upside, yes, it
>>> would put less burden on the drivers as it then only needs to care
>>> about using one set of functions.
>>>
>>> Which solution do you prefer?
>>
>> Any reason why we could not add a 'boolean' argument to the API to indicate whether the new device should be linked? I think that I prefer the API handles it, but I can see there could be instances where drivers may wish to handle it themselves.
>>
>> Rajendra, do you have a use-case right now where the driver would want to handle the linking?
>
> So if I understand this right, any driver which does want to control individual powerdomain state would
> need to do the linking itself right?
>
> What I am saying is, if I have device A, with powerdomains X and Y, and if I want to turn on only X,
> then I would want only X to be linked with A, and at a later point if I want both X and Y to be turned on,
> I would then go ahead and link both X and Y to A? Is that correct or did I get it all wrong?
Correct!
>
> I know atleast Camera on msm8996 would need to do this since it has 2 vfe powerdoamins, which can be
> turned on one at a time (depending on what resolution needs to be supported) or both together if we
> really need very high resolution using both vfe modules.
I think this is also the case for the Tegra XUSB subsystem.
The usb device is always attached to one PM domain, but depending on
if super-speed mode is used, another PM domain for that logic needs to
be powered on as well.
Jon, please correct me if I am wrong!
Kind regards
Uffe
^ permalink raw reply
* [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-23 9:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527066324-24726-1-git-send-email-ilialin@codeaurora.org>
On 23-05-18, 12:05, Ilia Lin wrote:
> In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
> the CPU frequency subset and voltage value of each OPP varies
> based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
> defines the voltage and frequency value based on the msm-id in SMEM
> and speedbin blown in the efuse combination.
> The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
> to provide the OPP framework with required information.
> This is used to determine the voltage and frequency value for each OPP of
> operating-points-v2 table when it is parsed by the OPP framework.
>
> Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
> ---
> drivers/cpufreq/Kconfig.arm | 10 +++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/cpufreq-dt-platdev.c | 3 +
> drivers/cpufreq/qcom-cpufreq-kryo.c | 163 +++++++++++++++++++++++++++++++++++
> 4 files changed, 177 insertions(+)
> create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
Hi Ilia,
So the patch looks good now. But I don't see how this series is going
to get merged. You are touching all parts of the kernel and no single
maintainer would be able to get these merged easily.
What I would suggest is that you divided the series in at least 3
parts.
- Patch 10 and 11, just the cpufreq stuff. So that Rafael can apply
those independently. Also mention the dependency on my OPP patches
in the cover letter for those two patches.
- All clk patches in another series, so that Stephen can apply them.
- And finally everything else to go via ARM-Soc.
Maybe things wouldn't be that simple, but that's what you would need
to do to get things merged. And please add all Acked by tags you got
to the patches.
For this patch:
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* [PATCH V3 8/8] dt-bindings: stm32: add compatible for syscon
From: Christophe ROULLIER @ 2018-05-23 9:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522172238.GA26145@rob-hp-laptop>
On 05/22/2018 07:22 PM, Rob Herring wrote:
> On Mon, May 21, 2018 at 10:07:26AM +0200, Christophe Roullier wrote:
>> This patch describes syscon DT bindings.
>>
>> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
>> ---
>> Documentation/devicetree/bindings/arm/stm32.txt | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/stm32.txt b/Documentation/devicetree/bindings/arm/stm32.txt
>> index 6808ed9..e46ebad 100644
>> --- a/Documentation/devicetree/bindings/arm/stm32.txt
>> +++ b/Documentation/devicetree/bindings/arm/stm32.txt
>> @@ -8,3 +8,8 @@ using one of the following compatible strings:
>> st,stm32f746
>> st,stm32h743
>> st,stm32mp157
>> +
>> +Required nodes:
>> +- syscon: the soc bus node must have a system controller node pointing to the
>> + global control registers, with the compatible string
>> + "st,stm32mp157-syscfg", "syscon";
>
> Please don't mix soc/board bindings with other nodes. So perhaps
> stm32-syscon.txt.
>
> Rob
>
Hi Rob,
Is it ok for you with this tree file:
Documentation/devicetree/bindings/arm/stm32/stm32.txt
Documentation/devicetree/bindings/arm/stm32/stm32-syscon.txt
With stm32-syscon.txt:
---------------------------------------------------
STMicroelectronics STM32 Platforms System Controller
Properties:
- compatible : should contain two values. First value must be :
- " st,stm32mp157-syscfg " - for stm32mp157 based SoCs,
second value must be always "syscon".
- reg : offset and length of the register set.
Example:
syscfg: system-config at 50020000 {
compatible = "st,stm32mp157-syscfg", "syscon";
reg = <0x50020000 0x400>;
};
---------------------------------------------------
Do we need to update also all MCU family (stm32f4, stm32h7, stm32f7)
property to be coherent ?
Thanks for your feedback.
Christophe.
^ permalink raw reply
* [PATCH v5 7/7] drm/i2c: tda998x: register as a drm bridge
From: Peter Rosin @ 2018-05-23 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>
This makes this driver work with all(?) drivers that are not
componentized and instead expect to connect to a panel/bridge. That
said, the only one tested is atmel-hlcdc.
This hooks the relevant work function previously called by the encoder
and the component also to the bridge, since the encoder goes away when
connecting to the bridge interface of the driver and the equivalent of
bind/unbind of the component is handled by bridge attach/detach.
The lifetime requirements of a bridge and a component are slightly
different, which is the reason for struct tda998x_bridge.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 170 ++++++++++++++++++++++++++++++++------
1 file changed, 143 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d401b3d0095c..d47e49c2991a 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -36,6 +36,14 @@ struct tda998x_audio_port {
u8 config; /* AP value */
};
+struct tda998x_priv;
+
+struct tda998x_bridge {
+ struct tda998x_priv *priv;
+ struct device *dev;
+ struct drm_bridge bridge;
+};
+
struct tda998x_priv {
struct i2c_client *cec;
struct i2c_client *hdmi;
@@ -63,6 +71,8 @@ struct tda998x_priv {
wait_queue_head_t edid_delay_waitq;
bool edid_delay_active;
+ struct tda998x_bridge *bridge;
+ bool local_encoder;
struct drm_encoder encoder;
struct drm_connector connector;
@@ -75,6 +85,9 @@ struct tda998x_priv {
#define enc_to_tda998x_priv(x) \
container_of(x, struct tda998x_priv, encoder)
+#define bridge_to_tda998x_bridge(x) \
+ container_of(x, struct tda998x_bridge, bridge)
+
/* The TDA9988 series of devices use a paged register scheme.. to simplify
* things we encode the page # in upper bits of the register #. To read/
* write a given register, we need to make sure CURPAGE register is set
@@ -842,7 +855,8 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
struct hdmi_codec_daifmt *daifmt,
struct hdmi_codec_params *params)
{
- struct tda998x_priv *priv = dev_get_drvdata(dev);
+ struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+ struct tda998x_priv *priv = bridge->priv;
int i, ret;
struct tda998x_audio_params audio = {
.sample_width = params->sample_width,
@@ -899,7 +913,8 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
static void tda998x_audio_shutdown(struct device *dev, void *data)
{
- struct tda998x_priv *priv = dev_get_drvdata(dev);
+ struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+ struct tda998x_priv *priv = bridge->priv;
mutex_lock(&priv->audio_mutex);
@@ -912,7 +927,8 @@ static void tda998x_audio_shutdown(struct device *dev, void *data)
int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
{
- struct tda998x_priv *priv = dev_get_drvdata(dev);
+ struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+ struct tda998x_priv *priv = bridge->priv;
mutex_lock(&priv->audio_mutex);
@@ -925,7 +941,8 @@ int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
static int tda998x_audio_get_eld(struct device *dev, void *data,
uint8_t *buf, size_t len)
{
- struct tda998x_priv *priv = dev_get_drvdata(dev);
+ struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+ struct tda998x_priv *priv = bridge->priv;
mutex_lock(&priv->audio_mutex);
memcpy(buf, priv->connector.eld,
@@ -1126,7 +1143,10 @@ tda998x_connector_best_encoder(struct drm_connector *connector)
{
struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
- return &priv->encoder;
+ if (priv->local_encoder)
+ return &priv->encoder;
+ else
+ return priv->bridge->bridge.encoder;
}
static
@@ -1140,6 +1160,7 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
struct drm_device *drm)
{
struct drm_connector *connector = &priv->connector;
+ struct drm_encoder *encoder;
int ret;
connector->interlace_allowed = 1;
@@ -1156,7 +1177,8 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
if (ret)
return ret;
- drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
+ encoder = tda998x_connector_best_encoder(&priv->connector);
+ drm_mode_connector_attach_encoder(&priv->connector, encoder);
return 0;
}
@@ -1671,8 +1693,10 @@ static void tda998x_set_config(struct tda998x_priv *priv,
priv->audio_params = p->audio_params;
}
-static int tda998x_init(struct device *dev, struct drm_device *drm)
+static int tda998x_init(struct device *dev, struct drm_device *drm,
+ bool local_encoder)
{
+ struct tda998x_bridge *bridge = dev_get_drvdata(dev);
struct tda998x_encoder_params *params = dev->platform_data;
struct i2c_client *client = to_i2c_client(dev);
struct tda998x_priv *priv;
@@ -1683,18 +1707,22 @@ static int tda998x_init(struct device *dev, struct drm_device *drm)
if (!priv)
return -ENOMEM;
- dev_set_drvdata(dev, priv);
+ bridge->priv = priv;
+ priv->bridge = bridge;
+ priv->local_encoder = local_encoder;
- if (dev->of_node)
- crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
+ if (local_encoder) {
+ if (dev->of_node)
+ crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
- /* If no CRTCs were found, fall back to our old behaviour */
- if (crtcs == 0) {
- dev_warn(dev, "Falling back to first CRTC\n");
- crtcs = 1 << 0;
- }
+ /* If no CRTCs were found, fall back to our old behaviour */
+ if (crtcs == 0) {
+ dev_warn(dev, "Falling back to first CRTC\n");
+ crtcs = 1 << 0;
+ }
- priv->encoder.possible_crtcs = crtcs;
+ priv->encoder.possible_crtcs = crtcs;
+ }
ret = tda998x_create(client, priv);
if (ret)
@@ -1703,11 +1731,15 @@ static int tda998x_init(struct device *dev, struct drm_device *drm)
if (!dev->of_node && params)
tda998x_set_config(priv, params);
- drm_encoder_helper_add(&priv->encoder, &tda998x_encoder_helper_funcs);
- ret = drm_encoder_init(drm, &priv->encoder, &tda998x_encoder_funcs,
- DRM_MODE_ENCODER_TMDS, NULL);
- if (ret)
- goto err_encoder;
+ if (local_encoder) {
+ drm_encoder_helper_add(&priv->encoder,
+ &tda998x_encoder_helper_funcs);
+ ret = drm_encoder_init(drm, &priv->encoder,
+ &tda998x_encoder_funcs,
+ DRM_MODE_ENCODER_TMDS, NULL);
+ if (ret)
+ goto err_encoder;
+ }
ret = tda998x_connector_init(priv, drm);
if (ret)
@@ -1716,7 +1748,8 @@ static int tda998x_init(struct device *dev, struct drm_device *drm)
return 0;
err_connector:
- drm_encoder_cleanup(&priv->encoder);
+ if (local_encoder)
+ drm_encoder_cleanup(&priv->encoder);
err_encoder:
tda998x_destroy(priv);
return ret;
@@ -1724,10 +1757,12 @@ static int tda998x_init(struct device *dev, struct drm_device *drm)
static void tda998x_fini(struct device *dev)
{
- struct tda998x_priv *priv = dev_get_drvdata(dev);
+ struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+ struct tda998x_priv *priv = bridge->priv;
drm_connector_cleanup(&priv->connector);
- drm_encoder_cleanup(&priv->encoder);
+ if (priv->local_encoder)
+ drm_encoder_cleanup(&priv->encoder);
tda998x_destroy(priv);
}
@@ -1735,7 +1770,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
{
struct drm_device *drm = data;
- return tda998x_init(dev, drm);
+ return tda998x_init(dev, drm, true);
}
static void tda998x_unbind(struct device *dev, struct device *master,
@@ -1749,19 +1784,100 @@ static const struct component_ops tda998x_ops = {
.unbind = tda998x_unbind,
};
+/* DRM bridge functions */
+
+static int tda998x_bridge_attach(struct drm_bridge *dbridge)
+{
+ struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+ struct device *dev = bridge->dev;
+ struct drm_device *drm = bridge->bridge.dev;
+
+ return tda998x_init(dev, drm, false);
+}
+
+static void tda998x_bridge_detach(struct drm_bridge *dbridge)
+{
+ struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+ struct device *dev = bridge->dev;
+
+ tda998x_fini(dev);
+}
+
+static void tda998x_bridge_enable(struct drm_bridge *dbridge)
+{
+ struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+ struct tda998x_priv *priv = bridge->priv;
+
+ tda998x_enable(priv);
+}
+
+static void tda998x_bridge_disable(struct drm_bridge *dbridge)
+{
+ struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+ struct tda998x_priv *priv = bridge->priv;
+
+ tda998x_disable(priv);
+}
+
+static void tda998x_bridge_mode_set(struct drm_bridge *dbridge,
+ struct drm_display_mode *mode,
+ struct drm_display_mode *adjusted_mode)
+{
+ struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+ struct tda998x_priv *priv = bridge->priv;
+
+ tda998x_mode_set(priv, mode, adjusted_mode);
+}
+
+static const struct drm_bridge_funcs tda998x_bridge_funcs = {
+ .attach = tda998x_bridge_attach,
+ .detach = tda998x_bridge_detach,
+ .enable = tda998x_bridge_enable,
+ .disable = tda998x_bridge_disable,
+ .mode_set = tda998x_bridge_mode_set,
+};
+
static int
tda998x_probe(struct i2c_client *client, const struct i2c_device_id *id)
{
+ struct device *dev = &client->dev;
+ struct tda998x_bridge *bridge;
+ int ret;
+
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
dev_warn(&client->dev, "adapter does not support I2C\n");
return -EIO;
}
- return component_add(&client->dev, &tda998x_ops);
+
+ bridge = devm_kzalloc(dev, sizeof(*bridge), GFP_KERNEL);
+ if (!bridge)
+ return -ENOMEM;
+
+ bridge->dev = dev;
+ dev_set_drvdata(dev, bridge);
+
+ bridge->bridge.funcs = &tda998x_bridge_funcs;
+#ifdef CONFIG_OF
+ bridge->bridge.of_node = dev->of_node;
+#endif
+ drm_bridge_add(&bridge->bridge);
+
+ ret = component_add(dev, &tda998x_ops);
+
+ if (ret)
+ drm_bridge_remove(&bridge->bridge);
+
+ return ret;
}
static int tda998x_remove(struct i2c_client *client)
{
- component_del(&client->dev, &tda998x_ops);
+ struct device *dev = &client->dev;
+ struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+
+ drm_bridge_remove(&bridge->bridge);
+ component_del(dev, &tda998x_ops);
+
return 0;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v5 6/7] drm/i2c: tda998x: split encoder and component functions from the work
From: Peter Rosin @ 2018-05-23 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>
This enables reuse of the machinery for the case where a drm_bridge
needs to do the same work via different interfaces.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 3dda07a2fd2f..d401b3d0095c 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1202,11 +1202,10 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
}
static void
-tda998x_encoder_mode_set(struct drm_encoder *encoder,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+tda998x_mode_set(struct tda998x_priv *priv,
+ struct drm_display_mode *mode,
+ struct drm_display_mode *adjusted_mode)
{
- struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
u16 ref_pix, ref_line, n_pix, n_line;
u16 hs_pix_s, hs_pix_e;
u16 vs1_pix_s, vs1_pix_e, vs1_line_s, vs1_line_e;
@@ -1413,6 +1412,16 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
mutex_unlock(&priv->audio_mutex);
}
+static void
+tda998x_encoder_mode_set(struct drm_encoder *encoder,
+ struct drm_display_mode *mode,
+ struct drm_display_mode *adjusted_mode)
+{
+ struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+
+ tda998x_mode_set(priv, mode, adjusted_mode);
+}
+
static void tda998x_destroy(struct tda998x_priv *priv)
{
/* disable all IRQs and free the IRQ handler */
@@ -1662,11 +1671,10 @@ static void tda998x_set_config(struct tda998x_priv *priv,
priv->audio_params = p->audio_params;
}
-static int tda998x_bind(struct device *dev, struct device *master, void *data)
+static int tda998x_init(struct device *dev, struct drm_device *drm)
{
struct tda998x_encoder_params *params = dev->platform_data;
struct i2c_client *client = to_i2c_client(dev);
- struct drm_device *drm = data;
struct tda998x_priv *priv;
u32 crtcs = 0;
int ret;
@@ -1714,8 +1722,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
return ret;
}
-static void tda998x_unbind(struct device *dev, struct device *master,
- void *data)
+static void tda998x_fini(struct device *dev)
{
struct tda998x_priv *priv = dev_get_drvdata(dev);
@@ -1724,6 +1731,19 @@ static void tda998x_unbind(struct device *dev, struct device *master,
tda998x_destroy(priv);
}
+static int tda998x_bind(struct device *dev, struct device *master, void *data)
+{
+ struct drm_device *drm = data;
+
+ return tda998x_init(dev, drm);
+}
+
+static void tda998x_unbind(struct device *dev, struct device *master,
+ void *data)
+{
+ tda998x_fini(dev);
+}
+
static const struct component_ops tda998x_ops = {
.bind = tda998x_bind,
.unbind = tda998x_unbind,
--
2.11.0
^ permalink raw reply related
* [PATCH v5 5/7] drm/i2c: tda998x: split tda998x_encoder_dpms into enable/disable
From: Peter Rosin @ 2018-05-23 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>
This fits better with the drm_bridge callbacks for when this
driver becomes a drm_bridge.
Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 64 ++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 933d309d2e0f..3dda07a2fd2f 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1163,36 +1163,42 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
/* DRM encoder functions */
-static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
+static void tda998x_enable(struct tda998x_priv *priv)
{
- struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
- bool on;
+ if (priv->is_on)
+ return;
- /* we only care about on or off: */
- on = mode == DRM_MODE_DPMS_ON;
+ /* enable video ports, audio will be enabled later */
+ reg_write(priv, REG_ENA_VP_0, 0xff);
+ reg_write(priv, REG_ENA_VP_1, 0xff);
+ reg_write(priv, REG_ENA_VP_2, 0xff);
+ /* set muxing after enabling ports: */
+ reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0);
+ reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1);
+ reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2);
- if (on == priv->is_on)
- return;
+ priv->is_on = true;
+}
- if (on) {
- /* enable video ports, audio will be enabled later */
- reg_write(priv, REG_ENA_VP_0, 0xff);
- reg_write(priv, REG_ENA_VP_1, 0xff);
- reg_write(priv, REG_ENA_VP_2, 0xff);
- /* set muxing after enabling ports: */
- reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0);
- reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1);
- reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2);
-
- priv->is_on = true;
- } else {
- /* disable video ports */
- reg_write(priv, REG_ENA_VP_0, 0x00);
- reg_write(priv, REG_ENA_VP_1, 0x00);
- reg_write(priv, REG_ENA_VP_2, 0x00);
+static void tda998x_disable(struct tda998x_priv *priv)
+{
+ /* disable video ports */
+ reg_write(priv, REG_ENA_VP_0, 0x00);
+ reg_write(priv, REG_ENA_VP_1, 0x00);
+ reg_write(priv, REG_ENA_VP_2, 0x00);
- priv->is_on = false;
- }
+ priv->is_on = false;
+}
+
+static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
+{
+ struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+
+ /* we only care about on or off: */
+ if (mode == DRM_MODE_DPMS_ON)
+ tda998x_enable(priv);
+ else
+ tda998x_disable(priv);
}
static void
@@ -1606,12 +1612,16 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
static void tda998x_encoder_prepare(struct drm_encoder *encoder)
{
- tda998x_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
+ struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+
+ tda998x_disable(priv);
}
static void tda998x_encoder_commit(struct drm_encoder *encoder)
{
- tda998x_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
+ struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+
+ tda998x_enable(priv);
}
static const struct drm_encoder_helper_funcs tda998x_encoder_helper_funcs = {
--
2.11.0
^ permalink raw reply related
* [PATCH v5 4/7] drm/i2c: tda998x: find the drm_device via the drm_connector
From: Peter Rosin @ 2018-05-23 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>
This prepares for being a drm_bridge which will not register the
encoder. That makes the connector the better choice.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 421c8a72369e..933d309d2e0f 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -630,7 +630,7 @@ static void tda998x_detect_work(struct work_struct *work)
{
struct tda998x_priv *priv =
container_of(work, struct tda998x_priv, detect_work);
- struct drm_device *dev = priv->encoder.dev;
+ struct drm_device *dev = priv->connector.dev;
if (dev)
drm_kms_helper_hotplug_event(dev);
--
2.11.0
^ permalink raw reply related
* [PATCH v5 3/7] drm/atmel-hlcdc: support bus-width (12/16/18/24) in endpoint nodes
From: Peter Rosin @ 2018-05-23 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>
This beats the heuristic that the connector is involved in what format
should be output for cases where this fails.
E.g. if there is a bridge that changes format between the encoder and the
connector, or if some of the RGB pins between the lcd controller and the
encoder are not routed on the PCB.
This is critical for the devices that have the "conflicting output
formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
RGB bits move around depending on the selected output mode. For
devices that do not have the "conflicting output formats" issue
(SAMA5D2, SAMA5D4), this is completely irrelevant.
Acked-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 70 +++++++++++++++++-------
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 67 ++++++++++++++++++++---
3 files changed, 111 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index d73281095fac..c38a479ada98 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -226,6 +226,55 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
#define ATMEL_HLCDC_RGB888_OUTPUT BIT(3)
#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
+static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
+{
+ struct drm_connector *connector = state->connector;
+ struct drm_display_info *info = &connector->display_info;
+ struct drm_encoder *encoder;
+ unsigned int supported_fmts = 0;
+ int j;
+
+ encoder = state->best_encoder;
+ if (!encoder)
+ encoder = connector->encoder;
+
+ switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
+ case 0:
+ break;
+ case MEDIA_BUS_FMT_RGB444_1X12:
+ return ATMEL_HLCDC_RGB444_OUTPUT;
+ case MEDIA_BUS_FMT_RGB565_1X16:
+ return ATMEL_HLCDC_RGB565_OUTPUT;
+ case MEDIA_BUS_FMT_RGB666_1X18:
+ return ATMEL_HLCDC_RGB666_OUTPUT;
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ return ATMEL_HLCDC_RGB888_OUTPUT;
+ default:
+ return -EINVAL;
+ }
+
+ for (j = 0; j < info->num_bus_formats; j++) {
+ switch (info->bus_formats[j]) {
+ case MEDIA_BUS_FMT_RGB444_1X12:
+ supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
+ break;
+ case MEDIA_BUS_FMT_RGB565_1X16:
+ supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
+ break;
+ case MEDIA_BUS_FMT_RGB666_1X18:
+ supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
+ break;
+ default:
+ break;
+ }
+ }
+
+ return supported_fmts;
+}
+
static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
{
unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
@@ -238,31 +287,12 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
for_each_new_connector_in_state(state->state, connector, cstate, i) {
- struct drm_display_info *info = &connector->display_info;
unsigned int supported_fmts = 0;
- int j;
if (!cstate->crtc)
continue;
- for (j = 0; j < info->num_bus_formats; j++) {
- switch (info->bus_formats[j]) {
- case MEDIA_BUS_FMT_RGB444_1X12:
- supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
- break;
- case MEDIA_BUS_FMT_RGB565_1X16:
- supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
- break;
- case MEDIA_BUS_FMT_RGB666_1X18:
- supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
- break;
- case MEDIA_BUS_FMT_RGB888_1X24:
- supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
- break;
- default:
- break;
- }
- }
+ supported_fmts = atmel_hlcdc_connector_output_mode(cstate);
if (crtc->dc->desc->conflicting_output_formats)
output_fmts &= supported_fmts;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 60c937f42114..4cc1e03f0aee 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -441,5 +441,6 @@ void atmel_hlcdc_crtc_irq(struct drm_crtc *c);
int atmel_hlcdc_crtc_create(struct drm_device *dev);
int atmel_hlcdc_create_outputs(struct drm_device *dev);
+int atmel_hlcdc_encoder_get_bus_fmt(struct drm_encoder *encoder);
#endif /* DRM_ATMEL_HLCDC_H */
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
index 8db51fb131db..db4d6aaaef93 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
@@ -27,33 +27,86 @@
#include "atmel_hlcdc_dc.h"
+struct atmel_hlcdc_rgb_output {
+ struct drm_encoder encoder;
+ int bus_fmt;
+};
+
static const struct drm_encoder_funcs atmel_hlcdc_panel_encoder_funcs = {
.destroy = drm_encoder_cleanup,
};
+static struct atmel_hlcdc_rgb_output *
+atmel_hlcdc_encoder_to_rgb_output(struct drm_encoder *encoder)
+{
+ return container_of(encoder, struct atmel_hlcdc_rgb_output, encoder);
+}
+
+int atmel_hlcdc_encoder_get_bus_fmt(struct drm_encoder *encoder)
+{
+ struct atmel_hlcdc_rgb_output *output;
+
+ output = atmel_hlcdc_encoder_to_rgb_output(encoder);
+
+ return output->bus_fmt;
+}
+
+static int atmel_hlcdc_of_bus_fmt(struct device_node *ep)
+{
+ u32 bus_width = 0;
+
+ of_property_read_u32(ep, "bus-width", &bus_width);
+
+ switch (bus_width) {
+ case 0:
+ return 0;
+ case 12:
+ return MEDIA_BUS_FMT_RGB444_1X12;
+ case 16:
+ return MEDIA_BUS_FMT_RGB565_1X16;
+ case 18:
+ return MEDIA_BUS_FMT_RGB666_1X18;
+ case 24:
+ return MEDIA_BUS_FMT_RGB888_1X24;
+ default:
+ return -EINVAL;
+ }
+}
+
static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint)
{
- struct drm_encoder *encoder;
+ struct atmel_hlcdc_rgb_output *output;
+ struct device_node *ep;
struct drm_panel *panel;
struct drm_bridge *bridge;
int ret;
+ ep = of_graph_get_endpoint_by_regs(dev->dev->of_node, 0, endpoint);
+ if (!ep)
+ return -ENODEV;
+
ret = drm_of_find_panel_or_bridge(dev->dev->of_node, 0, endpoint,
&panel, &bridge);
if (ret)
return ret;
- encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
- if (!encoder)
+ output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL);
+ if (!output)
return -EINVAL;
- ret = drm_encoder_init(dev, encoder,
+ output->bus_fmt = atmel_hlcdc_of_bus_fmt(ep);
+ if (output->bus_fmt < 0) {
+ dev_err(dev->dev, "invalid bus width\n");
+ return -EINVAL;
+ }
+
+ ret = drm_encoder_init(dev, &output->encoder,
&atmel_hlcdc_panel_encoder_funcs,
DRM_MODE_ENCODER_NONE, NULL);
if (ret)
return ret;
- encoder->possible_crtcs = 0x1;
+ output->encoder.possible_crtcs = 0x1;
if (panel) {
bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown);
@@ -62,7 +115,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint)
}
if (bridge) {
- ret = drm_bridge_attach(encoder, bridge, NULL);
+ ret = drm_bridge_attach(&output->encoder, bridge, NULL);
if (!ret)
return 0;
@@ -70,7 +123,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint)
drm_panel_bridge_remove(bridge);
}
- drm_encoder_cleanup(encoder);
+ drm_encoder_cleanup(&output->encoder);
return ret;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v5 2/7] dt-bindings: display: atmel: optional video-interface of endpoints
From: Peter Rosin @ 2018-05-23 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>
With bus-type/bus-width properties in the endpoint nodes, the video-
interface of the connection can be specified for cases where the
heuristic fails to select the correct output mode. This can happen
e.g. if not all RGB pins are routed on the PCB; the driver has no
way of knowing this, and needs to be told explicitly.
This is critical for the devices that have the "conflicting output
formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
RGB bits move around depending on the selected output mode. For
devices that do not have the "conflicting output formats" issue
(SAMA5D2, SAMA5D4), this is completely irrelevant.
Acked-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../devicetree/bindings/display/atmel/hlcdc-dc.txt | 26 ++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
index 82f2acb3d374..9de434a8f523 100644
--- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
+++ b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
@@ -15,6 +15,14 @@ Required children nodes:
to external devices using the OF graph reprensentation (see ../graph.txt).
At least one port node is required.
+Optional properties in grandchild nodes:
+ Any endpoint grandchild node may specify a desired video interface
+ according to ../../media/video-interfaces.txt, specifically
+ - bus-type: must be <0>.
+ - bus-width: recognized values are <12>, <16>, <18> and <24>, and
+ override any output mode selection heuristic, forcing "rgb444",
+ "rgb565", "rgb666" and "rgb888" respectively.
+
Example:
hlcdc: hlcdc at f0030000 {
@@ -50,3 +58,21 @@ Example:
#pwm-cells = <3>;
};
};
+
+
+Example 2: With a video interface override to force rgb565; as above
+but with these changes/additions:
+
+ &hlcdc {
+ hlcdc-display-controller {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
+
+ port at 0 {
+ hlcdc_panel_output: endpoint at 0 {
+ bus-type = <0>;
+ bus-width = <16>;
+ };
+ };
+ };
+ };
--
2.11.0
^ permalink raw reply related
* [PATCH v5 1/7] dt-bindings: display: bridge: lvds-transmitter: add ti, ds90c185
From: Peter Rosin @ 2018-05-23 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>
Start list of actual chips compatible with "lvds-encoder".
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../devicetree/bindings/display/bridge/lvds-transmitter.txt | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index fd39ad34c383..50220190c203 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -22,7 +22,13 @@ among others.
Required properties:
-- compatible: Must be "lvds-encoder"
+- compatible: Must be one or more of the following
+ - "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
+ - "lvds-encoder" for a generic LVDS encoder device
+
+ When compatible with the generic version, nodes must list the
+ device-specific version corresponding to the device first
+ followed by the generic version.
Required nodes:
--
2.11.0
^ permalink raw reply related
* [PATCH v5 0/7] Add tda998x (HDMI) support to atmel-hlcdc
From: Peter Rosin @ 2018-05-23 9:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi!
I naively thought that since there was support for both nxp,tda19988 (in
the tda998x driver) and the atmel-hlcdc, things would be a smooth ride.
But it wasn't, so I started looking around and realized I had to fix
things.
In v1 and v2 I fixed things by making the atmel-hlcdc driver a master
component, but now I fix things by making the tda998x driver a bridge
instead. This was after a suggestion from Boris Brezillon the
atmel-hlcdc maintainer), so there was some risk of bias ... but after
comparing what was needed, I too find the bridge approach more direct.
In v4, an issue was noted by Russell King in that the tda998x device
might be unbound with shattering results for the main drm device.
This is of course a real problem and it needs to be fixed. I have
submitted a series [1] that tries to do that. However, since there are
currently *10* other bridges (analogix-anx78xx, adv7511, megachips_stdp*,
nxp-ptn3460, parade-ps8622, sii902x, sii9234, sil-sii8620, tc358767 and
ti-tfp410) with exactly the same problem (they are all I2C devices that
might be unexpectedly unbound without safety-net, at least that is how I
read it), I think it should be ok to add one more and then fix them all
instead of holding this series hostage. Apparently people don't go around
and unbind I2C devices all that often...
In addition to the above, our PCB interface between the SAMA5D3 and the
HDMI encoder is only using 16 bits, and this has to be described
somewhere, or the atmel-hlcdc driver have no chance of selecting the
correct output mode. Since I have similar problems with a ds90c185 lvds
encoder I added patches to override the atmel-hlcdc output format via
DT properties compatible with the media video-interface binding and
things start to play together.
Anyway, this series solves some real issues for my HW.
Also, is it perhaps possible that patches 1-3 can be taken independently
from 4-7? There is no hard dependency between the two parts of this series.
Patches 1-3 have the relevant tags and should be uncontroversial...
Cheers,
Peter
Changes since v4 https://lkml.org/lkml/2018/4/23/92
- added reviewed-by from Rob to patch 2/7
- dropped patch 8, since the issue noted by Russell King is not present
when working with components as tilcdc currently do when handling tda998x
Changes since v3 https://lkml.org/lkml/2018/4/19/736
- moved the meat of the drm_of_media_bus_fmt function from patch 3/7
to a driver-local atmel_hlcdc_of_bus_fmt function, and squashed with
patch 4/7 (this is now patch 3/8).
- patch 3/8 now parses and stores the DT bus format together with the
encoder in a reintroduced struct atmel_hlcdc_rgb_output instead of
allocating a separate encoder-indexed array for the bus formats.
- patch 5/8 is new and splits tda988x_encoder_dpms into a pair of
functions to enable/disable separately. This fits better with both
the current code and for the followup drm_bridge patch (7/8).
- adjust patch 6/8 and 7/8 to the above simplification.
- added patch 8/8 that removes component master code from tilcdc.
Changes since v2 https://lkml.org/lkml/2018/4/17/385
- patch 2/7 fixed spelling and added an example
- patch 4/7 parse the DT up front and store the result indexed by encoder
- old patch 5/6 and 6/6 dropped
- patch 5-7/7 are new and makes the tda998x driver a drm_bridge
Changes since v1 https://lkml.org/lkml/2018/4/9/294
- added reviewed-by from Rob to patch 1/6
- patch 2/6 changed so that the bus format override is in the endpoint
DT node, and follows the binding of media video-interfaces.
- patch 3/6 is new, it adds drm_of_media_bus_fmt which parses above
media video-interface binding (partially).
- patch 4/6 now makes use of the above helper (and also fixes problems
with the 3/5 patch from v1 when no override was specified).
- do not mention unrelated connector display_info details in the cover
letter and commit messages.
[1] https://lkml.org/lkml/2018/5/16/380
Peter Rosin (7):
dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185
dt-bindings: display: atmel: optional video-interface of endpoints
drm/atmel-hlcdc: support bus-width (12/16/18/24) in endpoint nodes
drm/i2c: tda998x: find the drm_device via the drm_connector
drm/i2c: tda998x: split tda998x_encoder_dpms into enable/disable
drm/i2c: tda998x: split encoder and component functions from the work
drm/i2c: tda998x: register as a drm bridge
.../devicetree/bindings/display/atmel/hlcdc-dc.txt | 26 ++
.../bindings/display/bridge/lvds-transmitter.txt | 8 +-
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 70 ++++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 67 +++++-
drivers/gpu/drm/i2c/tda998x_drv.c | 268 ++++++++++++++++-----
6 files changed, 351 insertions(+), 89 deletions(-)
--
2.11.0
^ 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