* [RESPIN-PATCH v2 0/2] Remove static mapping of SCU from mach-exynos
From: Pankaj Dubey @ 2016-11-08 11:19 UTC (permalink / raw)
To: linux-arm-kernel
This patch series is part of patch series [1], which adds support of SCU
device node for Cortex-A9 based Exynos4 SoC. First two patches of the same
has been accepted and hence not included them in v2.
This patch series does some cleanup for Exynos4 SoC based boards.
We are currently statically mapping SCU SFRs in mach-exynos/exynos.c
which can be avoided if map this from device node of SCU
[1]: https://www.spinics.net/lists/arm-kernel/msg540498.html
This patch series is prepared on Krzysztof's for-next
(SHA_ID: b33c7bb9d59c3f4100d4)
I have tested these patches on Exynos4210 based on Origen board for SMP boot.
Patchset v1 was tested by Marek Szyprowski on Exynos4412-based Odroid U3
for SMP boot and S2R.
To confirm concern raised by Alim, I intentionally removed SCU device node
from exynos4.dtsi and tested on Origen board, I can see code handles this
case gracefully without any crash and system boot was fine. Of-course in such
case only single core will be able to boot.
Following boot message observed on Origen in case of missing SCU node
-------------
[ 0.000864] CPU: Testing write buffer coherency: ok
[ 0.001068] CPU0: thread -1, cpu 0, socket 9, mpidr 80000900
[ 0.001456] exynos_scu_enable failed to map scu_base
[ 0.001477] Setting up static identity map for 0x40100000 - 0x40100058
[ 1.120003] CPU1: failed to come online
[ 1.120059] Brought up 1 CPUs
[ 1.120068] SMP: Total of 1 processors activated (48.00 BogoMIPS).
[ 1.120076] CPU: All CPU(s) started in SVC mode.
-------------
Changes since v1:
- Address review comments from Krzysztof
- Moved scu_enable from pm.c and suspend.c to single place in platsmp.c
- Added error handling during scu_enable in platsmp.c
- Added Reviewed-by tag from Alim.
Pankaj Dubey (2):
ARM: EXYNOS: Remove static mapping of SCU SFR
ARM: EXYNOS: Remove unused soc_is_exynos{4,5}
arch/arm/mach-exynos/common.h | 6 +----
arch/arm/mach-exynos/exynos.c | 22 ------------------
arch/arm/mach-exynos/include/mach/map.h | 2 --
arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++++-------
arch/arm/mach-exynos/pm.c | 5 ++--
arch/arm/mach-exynos/suspend.c | 13 ++++-------
arch/arm/plat-samsung/include/plat/map-s5p.h | 4 ----
7 files changed, 34 insertions(+), 52 deletions(-)
--
2.7.4
^ permalink raw reply
* [RESPIN-PATCH v2 1/2] ARM: EXYNOS: Remove static mapping of SCU SFR
From: Pankaj Dubey @ 2016-11-08 11:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478603984-504-1-git-send-email-pankaj.dubey@samsung.com>
Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards.
Instead use mapping from device tree node of SCU.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
---
arch/arm/mach-exynos/common.h | 1 +
arch/arm/mach-exynos/exynos.c | 22 ------------------
arch/arm/mach-exynos/include/mach/map.h | 2 --
arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++++-------
arch/arm/mach-exynos/pm.c | 5 ++--
arch/arm/mach-exynos/suspend.c | 13 ++++-------
arch/arm/plat-samsung/include/plat/map-s5p.h | 4 ----
7 files changed, 34 insertions(+), 47 deletions(-)
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9424a8a..dd5d8e8 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -161,6 +161,7 @@ extern void exynos_cpu_restore_register(void);
extern void exynos_pm_central_suspend(void);
extern int exynos_pm_central_resume(void);
extern void exynos_enter_aftr(void);
+extern int exynos_scu_enable(void);
extern struct cpuidle_exynos_data cpuidle_coupled_exynos_data;
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 757fc11..fa08ef9 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -28,15 +28,6 @@
#include "common.h"
-static struct map_desc exynos4_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_COREPERI_BASE,
- .pfn = __phys_to_pfn(EXYNOS4_PA_COREPERI),
- .length = SZ_8K,
- .type = MT_DEVICE,
- },
-};
-
static struct platform_device exynos_cpuidle = {
.name = "exynos_cpuidle",
#ifdef CONFIG_ARM_EXYNOS_CPUIDLE
@@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
return 1;
}
-/*
- * exynos_map_io
- *
- * register the standard cpu IO areas
- */
-static void __init exynos_map_io(void)
-{
- if (soc_is_exynos4())
- iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
-}
-
static void __init exynos_init_io(void)
{
debug_ll_io_init();
@@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
/* detect cpu id and rev. */
s5p_init_cpu(S5P_VA_CHIPID);
-
- exynos_map_io();
}
/*
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 5fb0040..0eef407 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -18,6 +18,4 @@
#define EXYNOS_PA_CHIPID 0x10000000
-#define EXYNOS4_PA_COREPERI 0x10500000
-
#endif /* __ASM_ARCH_MAP_H */
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index a5d6841..94405c7 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -168,6 +168,27 @@ int exynos_cluster_power_state(int cluster)
S5P_CORE_LOCAL_PWR_EN);
}
+/**
+ * exynos_scu_enable : enables SCU for Cortex-A9 based system
+ * returns 0 on success else non-zero error code
+ */
+int exynos_scu_enable(void)
+{
+ struct device_node *np;
+ void __iomem *scu_base;
+
+ np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+ scu_base = of_iomap(np, 0);
+ of_node_put(np);
+ if (!scu_base) {
+ pr_err("%s failed to map scu_base\n", __func__);
+ return -ENOMEM;
+ }
+ scu_enable(scu_base);
+ iounmap(scu_base);
+ return 0;
+}
+
static void __iomem *cpu_boot_reg_base(void)
{
if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
@@ -224,11 +245,6 @@ static void write_pen_release(int val)
sync_cache_w(&pen_release);
}
-static void __iomem *scu_base_addr(void)
-{
- return (void __iomem *)(S5P_VA_SCU);
-}
-
static DEFINE_SPINLOCK(boot_lock);
static void exynos_secondary_init(unsigned int cpu)
@@ -393,9 +409,11 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
exynos_set_delayed_reset_assertion(true);
- if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
- scu_enable(scu_base_addr());
-
+ if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+ /* if exynos_scu_enable fails, return */
+ if (exynos_scu_enable())
+ return;
+ }
/*
* Write the address of secondary startup into the
* system-wide flags register. The boot monitor waits
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 487295f..23db2af 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -18,6 +18,7 @@
#include <linux/cpu_pm.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/soc/samsung/exynos-regs-pmu.h>
#include <linux/soc/samsung/exynos-pmu.h>
@@ -26,8 +27,6 @@
#include <asm/suspend.h>
#include <asm/cacheflush.h>
-#include <mach/map.h>
-
#include "common.h"
static inline void __iomem *exynos_boot_vector_addr(void)
@@ -177,7 +176,7 @@ void exynos_enter_aftr(void)
cpu_suspend(0, exynos_aftr_finisher);
if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
- scu_enable(S5P_VA_SCU);
+ exynos_scu_enable();
if (call_firmware_op(resume) == -ENOSYS)
exynos_cpu_restore_register();
}
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 06332f6..c73c857 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -34,8 +34,6 @@
#include <asm/smp_scu.h>
#include <asm/suspend.h>
-#include <mach/map.h>
-
#include <plat/pm-common.h>
#include "common.h"
@@ -461,12 +459,11 @@ static void exynos_pm_resume(void)
/* For release retention */
exynos_pm_release_retention();
- if (cpuid == ARM_CPU_PART_CORTEX_A9)
- scu_enable(S5P_VA_SCU);
-
- if (call_firmware_op(resume) == -ENOSYS
- && cpuid == ARM_CPU_PART_CORTEX_A9)
- exynos_cpu_restore_register();
+ if (cpuid == ARM_CPU_PART_CORTEX_A9) {
+ exynos_scu_enable();
+ if (call_firmware_op(resume) == -ENOSYS)
+ exynos_cpu_restore_register();
+ }
early_wakeup:
diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
index 0fe2828..512ed1f 100644
--- a/arch/arm/plat-samsung/include/plat/map-s5p.h
+++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
@@ -15,10 +15,6 @@
#define S5P_VA_CHIPID S3C_ADDR(0x02000000)
-#define S5P_VA_COREPERI_BASE S3C_ADDR(0x02800000)
-#define S5P_VA_COREPERI(x) (S5P_VA_COREPERI_BASE + (x))
-#define S5P_VA_SCU S5P_VA_COREPERI(0x0)
-
#define VA_VIC(x) (S3C_VA_IRQ + ((x) * 0x10000))
#define VA_VIC0 VA_VIC(0)
#define VA_VIC1 VA_VIC(1)
--
2.7.4
^ permalink raw reply related
* [RESPIN-PATCH v2 2/2] ARM: EXYNOS: Remove unused soc_is_exynos{4,5}
From: Pankaj Dubey @ 2016-11-08 11:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478603984-504-1-git-send-email-pankaj.dubey@samsung.com>
As no more user of soc_is_exynos{4,5} we can safely remove them.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
---
arch/arm/mach-exynos/common.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index dd5d8e8..fb12d11 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -105,11 +105,6 @@ IS_SAMSUNG_CPU(exynos5800, EXYNOS5800_SOC_ID, EXYNOS5_SOC_MASK)
# define soc_is_exynos5800() 0
#endif
-#define soc_is_exynos4() (soc_is_exynos4210() || soc_is_exynos4212() || \
- soc_is_exynos4412())
-#define soc_is_exynos5() (soc_is_exynos5250() || soc_is_exynos5410() || \
- soc_is_exynos5420() || soc_is_exynos5800())
-
extern u32 cp15_save_diag;
extern u32 cp15_save_power;
--
2.7.4
^ permalink raw reply related
* [GIT PULL] ARM: OXNAS SoC updates for 4.10
From: Neil Armstrong @ 2016-11-08 11:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161107212648.GC2106@localhost>
On 11/07/2016 10:26 PM, Olof Johansson wrote:
> On Mon, Oct 31, 2016 at 10:52:53AM +0100, Neil Armstrong wrote:
>> Dear arm-soc maintainers,
>>
>> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
>>
>> Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
>>
>> are available in the git repository at:
>>
>> https://github.com/OXNAS/linux.git tags/oxnas-arm-soc-for-4.10
>>
>> for you to fetch changes up to b3cdb3c5ef514687891f03442f2677850340bcfa:
>>
>> ARM: oxnas: Add OX820 config and makefile entry (2016-10-31 10:41:33 +0100)
>>
>> ----------------------------------------------------------------
>> - Add SMP support for the Oxford Semiconductor OX820 SoC
>> from http://lkml.kernel.org/r/20161021085848.1754-1-narmstrong at baylibre.com
>>
>> ----------------------------------------------------------------
>> Neil Armstrong (2):
>> ARM: oxnas: Add OX820 SMP support
>> ARM: oxnas: Add OX820 config and makefile entry
>
> Hi Neil,
Hi Olof,
>
> Sorry for being picky here, please be patient with us. :-)
Seems legit !
>
> Overall comments on this patch set:
>
> 1) Copyrights seem to be a bit varied. All 3 new source files have
> different copyright holders. Only one has yours, one is from 2003, one
> is also from 2013. Is this expected? Seems like it's worth mentioning
> in commit messages at least.
Yes, the files comes from various places :
- hotplug.c was taken from versatile by Ma Haijun and left verbatim, so I left the original header
- headsmp.S was taken from the versatile code and adapted by Ma Haijun
- platsmp.c is a mix from versatile code, Ma Haijun code and my code for DT
The two first file were only modified in order to build and pass checkpatch actually.
I tried to find the *real* copyright holders, but it was not easy...
>
> 2) Please remove the linux/arch/... lines from the top of the files. It's
> something we've been trying to remove for a while and don't want to
> introduce more occurrances of.
>
> I'd say (2) is fine to do incremental, but please provide clarification on (1).
> I could just add that to the merge commit message so don't respin the branch
> yet, depending on what we hear back!
>
Ok, please tell me if/when I need to send a new pull for the (2) fix.
>> arch/arm/Makefile | 1 +
>> arch/arm/mach-oxnas/Kconfig | 30 +++++++++++++++++++---------
>> arch/arm/mach-oxnas/Makefile | 2 ++
>> arch/arm/mach-oxnas/headsmp.S | 28 ++++++++++++++++++++++++++
>> arch/arm/mach-oxnas/hotplug.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> arch/arm/mach-oxnas/platsmp.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
> Git tip: If you redirect the output to a file (or pipe through cat), the
> diffstat will be limited to 80 characters and look better in email.
>
>
> Thanks,
>
> -Olof
>
Thanks,
Neil
^ permalink raw reply
* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: John Garry @ 2016-11-08 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2467231.EbVekJun1R@wuerfel>
On 07/11/2016 20:08, Arnd Bergmann wrote:
> On Monday, November 7, 2016 2:15:10 PM CET John Garry wrote:
>>
>> Hi Arnd,
>>
>> The new bus type tries to model the djtag in a similar way to I2C/USB
>> driver arch, where we have a host bus adapter and child devices attached
>> to the bus. The child devices are bus driver devices and have bus
>> addresses. We think of the djtag as a separate bus, so we are modelling
>> it as such.
>>
>> The bus driver offers a simple host interface for clients to read/write
>> to the djtag bus: bus accesses are hidden from the client, the host
>> drives the bus.
>
> Ok, in that case we should probably start out by having a bus specific
> DT binding, and separating the description from that of the bus master
> device.
OK
>
> I'd suggest requiring #address-cells=<1> and #size-cells=<0> in the master
> node, and listing the children by reg property. If the address is not
> easily expressed as a single integer, use a larger #address-cells value.
We already have something equivalent to reg in "module-id" (see patch
02/11), which is the slave device bus address; here's a sample:
+ /* For L3 cache PMU */
+ pmul3c0 {
+ compatible = "hisilicon,hisi-pmu-l3c-v1";
+ scl-id = <0x02>;
+ num-events = <0x16>;
+ num-counters = <0x08>;
+ module-id = <0x04>;
+ num-banks = <0x04>;
+ cfgen-map = <0x02 0x04 0x01 0x08>;
+ counter-reg = <0x170>;
+ evctrl-reg = <0x04>;
+ event-en = <0x1000000>;
+ evtype-reg = <0x140>;
+ };
FYI, "module-id" is our own internal hw nomenclature.
>
> Another option that we have previously used was to actually pretend that
> a vendor specific bus is an i2c bus and use the i2c probing infrastructure,
> but that only makes sense if the software side closely resembles i2c
> (this was the case for Allwinner I think, but I have not looked at
> your driver in enough detail to know if it is true here as well).
>
OK, let me check this. By chance do you remember the specific AllWinner
driver/hw?
Cheers,
John
> Arnd
>
> .
>
^ permalink raw reply
* [PATCH] iommu/dma-iommu: properly respect configured address space size
From: Robin Murphy @ 2016-11-08 11:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478523973-8828-1-git-send-email-m.szyprowski@samsung.com>
Hi Marek,
On 07/11/16 13:06, Marek Szyprowski wrote:
> When one called iommu_dma_init_domain() with size smaller than device's
> DMA mask, the alloc_iova() will not respect it and always assume that all
> IOVA addresses will be allocated from the the (base ... dev->dma_mask) range.
Is that actually a problem for anything?
> This patch fixes this issue by taking the configured address space size
> parameter into account (if it is smaller than the device's dma_mask).
TBH I've been pondering ripping the size stuff out of dma-iommu, as it
all stems from me originally failing to understand what dma_32bit_pfn is
actually for. The truth is that iova_domains just don't have a size or
upper limit; however if devices with both large and small DMA masks
share a domain, then the top-down nature of the allocator means that
allocating for the less-capable devices would involve walking through
every out-of-range entry in the tree every time. Having cached32_node
based on dma_32bit_pfn just provides an optimised starting point for
searching within the smaller mask.
Would it hurt any of your use-cases to relax/rework the reinitialisation
checks in iommu_dma_init_domain()? Alternatively if we really do have a
case for wanting a hard upper limit, it might make more sense to add an
end_pfn to the iova_domain and handle it in the allocator itself.
Robin.
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/iommu/dma-iommu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index c5ab8667e6f2..8b4b72654359 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -212,11 +212,13 @@ static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size,
>
> if (domain->geometry.force_aperture)
> dma_limit = min(dma_limit, domain->geometry.aperture_end);
> +
> + dma_limit = min(dma_limit >> shift, (dma_addr_t)iovad->dma_32bit_pfn);
> /*
> * Enforce size-alignment to be safe - there could perhaps be an
> * attribute to control this per-device, or at least per-domain...
> */
> - return alloc_iova(iovad, length, dma_limit >> shift, true);
> + return alloc_iova(iovad, length, dma_limit, true);
> }
>
> /* The IOVA allocator knows what we mapped, so just unmap whatever that was */
>
^ permalink raw reply
* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: Arnd Bergmann @ 2016-11-08 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582180F7.5060100@gmail.com>
On Tuesday, November 8, 2016 1:08:31 PM CET Anurup M wrote:
> On Tuesday 08 November 2016 12:32 PM, Tan Xiaojun wrote:
> > On 2016/11/7 21:26, Arnd Bergmann wrote:
> >> On Wednesday, November 2, 2016 11:42:46 AM CET Anurup M wrote:
> >>> From: Tan Xiaojun <tanxiaojun@huawei.com>
> >>> + /* ensure the djtag operation is done */
> >>> + do {
> >>> + djtag_read32_relaxed(regs_base, SC_DJTAG_MSTR_START_EN_EX, &rd);
> >>> +
> >>> + if (!(rd & DJTAG_MSTR_START_EN_EX))
> >>> + break;
> >>> +
> >>> + udelay(1);
> >>> + } while (timeout--);
> >> This one is obviously not performance critical at all, so use a non-relaxed
> >> accessor. Same for the other two in this function.
> >>
> >> Are these functions ever called from atomic context? If yes, please document
> >> from what context they can be called, otherwise please consider changing
> >> the udelay calls into sleeping waits.
> >>
> > Yes, this is not reentrant.
> The read/write functions shall also be called from irq handler (for
> handling counter overflow).
> So need to use udelay calls. Shall Document it in v2.
Ok.
> >>> +static const struct of_device_id djtag_of_match[] = {
> >>> + /* for hip05(D02) cpu die */
> >>> + { .compatible = "hisilicon,hip05-cpu-djtag-v1",
> >>> + .data = (void *)djtag_readwrite_v1 },
> >>> + /* for hip05(D02) io die */
> >>> + { .compatible = "hisilicon,hip05-io-djtag-v1",
> >>> + .data = (void *)djtag_readwrite_v1 },
> >>> + /* for hip06(D03) cpu die */
> >>> + { .compatible = "hisilicon,hip06-cpu-djtag-v1",
> >>> + .data = (void *)djtag_readwrite_v1 },
> >>> + /* for hip06(D03) io die */
> >>> + { .compatible = "hisilicon,hip06-io-djtag-v2",
> >>> + .data = (void *)djtag_readwrite_v2 },
> >>> + /* for hip07(D05) cpu die */
> >>> + { .compatible = "hisilicon,hip07-cpu-djtag-v2",
> >>> + .data = (void *)djtag_readwrite_v2 },
> >>> + /* for hip07(D05) io die */
> >>> + { .compatible = "hisilicon,hip07-io-djtag-v2",
> >>> + .data = (void *)djtag_readwrite_v2 },
> >>> + {},
> >>> +};
> >>
> >> If these are backwards compatible, just mark them as compatible in DT,
> >> e.g. hip06 can use
> >>
> >> compatible = "hisilicon,hip06-cpu-djtag-v1", "hisilicon,hip05-cpu-djtag-v1";
> >>
> >> so you can tell the difference if you need to, but the driver only has to
> >> list the oldest one here.
> >>
> >> What is the difference between the cpu and io djtag interfaces?
> On some chips like hip06, the djtag version is different for IO die.
In what way? The driver doesn't seem to care about the difference.
Arnd
^ permalink raw reply
* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: Arnd Bergmann @ 2016-11-08 11:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c68064fa-a274-2e22-88a6-5fe388debf2b@huawei.com>
On Tuesday, November 8, 2016 11:23:35 AM CET John Garry wrote:
> On 07/11/2016 20:08, Arnd Bergmann wrote:
> > On Monday, November 7, 2016 2:15:10 PM CET John Garry wrote:
> >>
> >> Hi Arnd,
> >>
> >> The new bus type tries to model the djtag in a similar way to I2C/USB
> >> driver arch, where we have a host bus adapter and child devices attached
> >> to the bus. The child devices are bus driver devices and have bus
> >> addresses. We think of the djtag as a separate bus, so we are modelling
> >> it as such.
> >>
> >> The bus driver offers a simple host interface for clients to read/write
> >> to the djtag bus: bus accesses are hidden from the client, the host
> >> drives the bus.
> >
> > Ok, in that case we should probably start out by having a bus specific
> > DT binding, and separating the description from that of the bus master
> > device.
>
> OK
>
> >
> > I'd suggest requiring #address-cells=<1> and #size-cells=<0> in the master
> > node, and listing the children by reg property. If the address is not
> > easily expressed as a single integer, use a larger #address-cells value.
>
> We already have something equivalent to reg in "module-id" (see patch
> 02/11), which is the slave device bus address; here's a sample:
> + /* For L3 cache PMU */
> + pmul3c0 {
> + compatible = "hisilicon,hisi-pmu-l3c-v1";
> + scl-id = <0x02>;
> + num-events = <0x16>;
> + num-counters = <0x08>;
> + module-id = <0x04>;
> + num-banks = <0x04>;
> + cfgen-map = <0x02 0x04 0x01 0x08>;
> + counter-reg = <0x170>;
> + evctrl-reg = <0x04>;
> + event-en = <0x1000000>;
> + evtype-reg = <0x140>;
> + };
>
> FYI, "module-id" is our own internal hw nomenclature.
Yes, that was my interpretation as well. Please use the standard
"reg" property for this then.
> > Another option that we have previously used was to actually pretend that
> > a vendor specific bus is an i2c bus and use the i2c probing infrastructure,
> > but that only makes sense if the software side closely resembles i2c
> > (this was the case for Allwinner I think, but I have not looked at
> > your driver in enough detail to know if it is true here as well).
> >
>
> OK, let me check this. By chance do you remember the specific AllWinner
> driver/hw?
drivers/i2c/busses/i2c-sun6i-p2wi.c
Arnd
^ permalink raw reply
* [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: Mark Rutland @ 2016-11-08 11:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478576829-112707-3-git-send-email-yuanzhichang@hisilicon.com>
On Tue, Nov 08, 2016 at 11:47:08AM +0800, zhichang.yuan wrote:
> +Hisilicon Hip06 low-pin-count device
> + Usually LPC controller is part of PCI host bridge, so the legacy ISA ports
> + locate on LPC bus can be accessed direclty. But some SoCs have independent
> + LPC controller, and access the legacy ports by triggering LPC I/O cycles.
> + Hisilicon Hip06 implements this LPC device.
s/direclty/directly/
My understanding of ISA (which may be flawed) is that it's not part of
the PCI host bridge, but rather on x86 it happens to share the IO space
with PCI.
So, how about this becomes:
Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which
provides access to some legacy ISA devices.
I believe that we could theoretically have multiple independent LPC/ISA
busses, as is possible with PCI on !x86 systems. If the current ISA code
assumes a singleton bus, I think that's something that needs to be fixed
up more generically.
I don't see why we should need any architecture-specific code here. Why
can we not fix up the ISA bus code in drivers/of/address.c such that it
handles multiple ISA bus instances, and translates all sub-device
addresses relative to the specific bus instance?
> +Required properties:
> +- compatible: should be "hisilicon,low-pin-count"
This would be better as something like:
"hisilicon,hip06-lpc-controller"
If it's reused in other SoCs, we can add more strings as we usually do.
> +- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
> +- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
> +- reg: base memory range where the register set of this device is mapped.
> +
> +Note:
> + The node name before '@' must be "isa" to represent the binding stick to the
> + ISA/EISA binding specification.
> +
> +Example:
> +
> +isa at a01b0000 {
> + compatible = "hisilicom,low-pin-count";
s/hisilicom/hisilicon/
My comment above on the compatible string also applies.
> + #address-cells = <2>;
> + #size-cells = <1>;
> + reg = <0x0 0xa01b0000 0x0 0x1000>;
> +
> + ipmi0: bt at e4 {
> + compatible = "ipmi-bt";
> + device_type = "ipmi";
> + reg = <0x01 0xe4 0x04>;
> + status = "disabled";
> + };
> +};
Please remove the status property; it's irrelevant to the example.
[...]
> +/**
> + * indirect_io_enabled - check whether indirectIO is enabled.
> + * arm64_extio_ops will be set only when indirectIO mechanism had been
> + * initialized.
> + *
> + * Returns true when indirectIO is enabled.
> + */
> +bool indirect_io_enabled(void)
> +{
> + return arm64_extio_ops ? true : false;
> +}
return !!arm64_extio_ops;
> +/**
> + * addr_is_indirect_io - check whether the input taddr is for indirectIO.
> + * @taddr: the io address to be checked.
> + *
> + * Returns 1 when taddr is in the range; otherwise return 0.
> + */
> +int addr_is_indirect_io(u64 taddr)
> +{
> + if (arm64_extio_ops->start > taddr || arm64_extio_ops->end < taddr)
> + return 0;
> +
> + return 1;
> +}
Why not bool?
I don't think this is the right thing to do, regardless.
> + * of_isa_indirect_io - get the IO address from some isa reg property value.
> + * For some isa/lpc devices, no ranges property in ancestor node.
> + * The device addresses are described directly in their regs property.
> + * This fixup function will be called to get the IO address of isa/lpc
> + * devices when the normal of_translation failed.
> + *
> + * @parent: points to the parent dts node;
> + * @bus: points to the of_bus which can be used to parse address;
> + * @addr: the address from reg property;
> + * @na: the address cell counter of @addr;
> + * @presult: store the address paresed from @addr;
> + *
> + * return 1 when successfully get the I/O address;
> + * 0 will return for some failures.
> + */
> +static int of_get_isa_indirect_io(struct device_node *parent,
> + struct of_bus *bus, __be32 *addr,
> + int na, u64 *presult)
> +{
> + unsigned int flags;
> + unsigned int rlen;
> +
> + /* whether support indirectIO */
> + if (!indirect_io_enabled())
> + return 0;
> +
> + if (!of_bus_isa_match(parent))
> + return 0;
> +
> + flags = bus->get_flags(addr);
> + if (!(flags & IORESOURCE_IO))
> + return 0;
> +
> + /* there is ranges property, apply the normal translation directly. */
> + if (of_get_property(parent, "ranges", &rlen))
> + return 0;
> +
> + *presult = of_read_number(addr + 1, na - 1);
> + /* this fixup is only valid for specific I/O range. */
> + return addr_is_indirect_io(*presult);
> +}
> +
> static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> struct of_bus *pbus, __be32 *addr,
> int na, int ns, int pna, const char *rprop)
> @@ -595,6 +639,15 @@ static u64 __of_translate_address(struct device_node *dev,
> result = of_read_number(addr, na);
> break;
> }
> + /*
> + * For indirectIO device which has no ranges property, get
> + * the address from reg directly.
> + */
> + if (of_get_isa_indirect_io(dev, bus, addr, na, &result)) {
> + pr_debug("isa indirectIO matched(%s)..addr = 0x%llx\n",
> + of_node_full_name(dev), result);
> + break;
> + }
I don't believe this is the right place for this to live. This should
live in the isa of_bus, matched in the usual way with of_match_bus(),
and used in pbus->translate() in of_translate_one().
If we need to extend the prototypes of those functions, we should do so.
If we need to be able to register instance-specific translations or
indirection, we should build the infrastructure for that rather than
forcing a singleton translation in here.
Thanks,
Mark.
^ permalink raw reply
* [PATCH V2 3/5] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pavel Labath @ 2016-11-08 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108032941.GC20591@arm.com>
>>
>> /* Do we need to handle the stepping? */
>> if (is_default_overflow_handler(wp))
>> step = 1;
>> -
>> -unlock:
>> - rcu_read_unlock();
>> }
>> + if (min_dist > 0 && min_dist != -1) {
>> + /* No exact match found. */
>> + wp = slots[closest_match];
>> + info = counter_arch_bp(wp);
>> + info->trigger = addr;
>> + perf_bp_event(wp, regs);
>> + }
>
> Why don't we need to bother with the stepping in the case of a non-exact
> match?
Good catch. I think we do. I must have dropped that part somehow.
Pratyush, could you include the attached fixup in the next batch?
regards,
pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixup.diff
Type: text/x-patch
Size: 493 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/b4638fbe/attachment.bin>
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Mark Rutland @ 2016-11-08 12:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478576829-112707-2-git-send-email-yuanzhichang@hisilicon.com>
On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> For arm64, there is no I/O space as other architectural platforms, such as
> X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
> such as Hip06, when accessing some legacy ISA devices connected to LPC, those
> known port addresses are used to control the corresponding target devices, for
> example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
> normal MMIO mode in using.
This has nothing to do with arm64. Hardware with this kind of indirect
bus access could be integrated with a variety of CPU architectures. It
simply hasn't been, yet.
> To drive these devices, this patch introduces a method named indirect-IO.
> In this method the in/out pair in arch/arm64/include/asm/io.h will be
> redefined. When upper layer drivers call in/out with those known legacy port
> addresses to access the peripherals, the hooking functions corrresponding to
> those target peripherals will be called. Through this way, those upper layer
> drivers which depend on in/out can run on Hip06 without any changes.
As above, this has nothing to do with arm64, and as such, should live in
generic code, exactly as we would do if we had higher-level ISA
accessor ops.
Regardless, given the multi-instance case, I don't think this is
sufficient in general (and I think we need higher-level ISA accessors
to handle the indirection).
[...]
> diff --git a/arch/arm64/include/asm/extio.h b/arch/arm64/include/asm/extio.h
> new file mode 100644
> index 0000000..6ae0787
> --- /dev/null
> +++ b/arch/arm64/include/asm/extio.h
> +#ifndef __LINUX_EXTIO_H
> +#define __LINUX_EXTIO_H
This doesn't match the file naming, __ASM_EXTIO_H would be consistent
with other arm64 headers.
> +
> +struct extio_ops {
> + unsigned long start;/* inclusive, sys io addr */
> + unsigned long end;/* inclusive, sys io addr */
Please put whitespace before inline comments.
[...]
> +type in##bw(unsigned long addr) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + return read##bw(PCI_IOBASE + addr); \
> + return arm64_extio_ops->pfin ? \
> + arm64_extio_ops->pfin(arm64_extio_ops->devpara, \
> + addr, sizeof(type)) : -1; \
> +} \
> + \
> +void out##bw(type value, unsigned long addr) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + write##bw(value, PCI_IOBASE + addr); \
> + else \
> + if (arm64_extio_ops->pfout) \
> + arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> + addr, value, sizeof(type)); \
> +} \
> + \
> +void ins##bw(unsigned long addr, void *buffer, unsigned int count) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + reads##bw(PCI_IOBASE + addr, buffer, count); \
> + else \
> + if (arm64_extio_ops->pfins) \
> + arm64_extio_ops->pfins(arm64_extio_ops->devpara,\
> + addr, buffer, sizeof(type), count); \
> +} \
> + \
> +void outs##bw(unsigned long addr, const void *buffer, unsigned int count) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + writes##bw(PCI_IOBASE + addr, buffer, count); \
> + else \
> + if (arm64_extio_ops->pfouts) \
> + arm64_extio_ops->pfouts(arm64_extio_ops->devpara,\
> + addr, buffer, sizeof(type), count); \
> +}
> +
So all PCI I/O will be slowed down by irrelevant checks when this is
enabled?
[...]
> +static inline void arm64_set_extops(struct extio_ops *ops)
> +{
> + if (ops)
> + WRITE_ONCE(arm64_extio_ops, ops);
> +}
Why WRITE_ONCE()?
Is this not protected/propagated by some synchronisation mechanism?
WRITE_ONCE() is not sufficient to ensure that this is consistently
observed by readers, and regardless, I don't see READ_ONCE() anywhere in
this patch.
This looks very suspicious.
Thanks,
Mark.
^ permalink raw reply
* [PATCH] staging: vc04_services: Add 32-bit compatibility ioctls
From: Arnd Bergmann @ 2016-11-08 12:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108004835.7458-1-mzoran@crowfest.net>
On Monday, November 7, 2016 4:48:35 PM CET Michael Zoran wrote:
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 269 +++++++++++++++++++++
> .../vc04_services/interface/vchiq_arm/vchiq_if.h | 25 ++
> .../interface/vchiq_arm/vchiq_ioctl.h | 102 ++++++++
> 3 files changed, 396 insertions(+)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 8fcd940..df343a0 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -573,12 +573,40 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> "vchiq: could not connect: %d", status);
> break;
>
> +#if defined(CONFIG_64BIT)
> + case VCHIQ_IOC_CREATE_SERVICE32:
> +#endif
> case VCHIQ_IOC_CREATE_SERVICE: {
> VCHIQ_CREATE_SERVICE_T args;
> USER_SERVICE_T *user_service = NULL;
> void *userdata;
> int srvstate;
>
> +#if defined(CONFIG_64BIT)
> + if (cmd == VCHIQ_IOC_CREATE_SERVICE32) {
Better use CONFIG_COMPAT here. Also, a simple #ifdef is sufficient
as neither of those symbols can be a loadable module.
Also, just move all the compat handling into the .compat_ioctl
callback function and move out the common parts into helpers
for simplicity.
> +#if defined(CONFIG_64BIT)
> + if (cmd == VCHIQ_IOC_AWAIT_COMPLETION32) {
> + VCHIQ_AWAIT_COMPLETION32_T args32;
> +
> + if (copy_from_user(&args32, (const void __user *)arg,
> + sizeof(args32)) != 0) {
> + ret = -EFAULT;
> + break;
> + }
> +
> + args.count = args32.count;
> + args.buf =
> + (VCHIQ_COMPLETION_DATA_T *)(unsigned long)
> + args32.buf;
> + args.msgbufsize = args32.msgbufsize;
> + args.msgbufcount = args32.msgbufcount;
> + args.msgbufs = (void **)(unsigned long)args32.msgbufs;
> + } else
> +#endif
There seems to be a bit of confusion about the address space
here. args.buf should be a user space pointer, right?
> +#if defined(CONFIG_64BIT)
> +typedef struct {
> + u32 data;
> + unsigned int size;
> +} VCHIQ_ELEMENT32_T;
> +#endif
remove the typedefs, it just forces someone to clean it up later.
> #define VCHIQ_IOC_CONNECT _IO(VCHIQ_IOC_MAGIC, 0)
> #define VCHIQ_IOC_SHUTDOWN _IO(VCHIQ_IOC_MAGIC, 1)
> #define VCHIQ_IOC_CREATE_SERVICE \
> _IOWR(VCHIQ_IOC_MAGIC, 2, VCHIQ_CREATE_SERVICE_T)
> +#if defined(CONFIG_64BIT)
> +#define VCHIQ_IOC_CREATE_SERVICE32 \
> + _IOWR(VCHIQ_IOC_MAGIC, 2, VCHIQ_CREATE_SERVICE32_T)
> +#endif
No need for the #ifdef here.
Arnd
^ permalink raw reply
* [PATCH RFC 00/12] tda998x updates
From: Russell King - ARM Linux @ 2016-11-08 12:24 UTC (permalink / raw)
To: linux-arm-kernel
As no one responded to the previous round, I'm not spending soo much
time writing up a description of these changes again. It's also been
quite a long time, so I've forgotten all the details of the changes,
so I'll do my best.
Changes from the previous series include:
- reorder the initial three patches
- change the (now third patch)... I think to increase the size of the
locked region.
- fix edid parsing for infoframe generation - as was pointed out for
dw-hdmi, parsing the EDID in get_modes() is incorrect, as that method
will not be called when an override-edid is in effect. We need to
parse the override-edid. Moreover, infoframe generation should not
be keyed to whether the monitor is HDMI or not, CEA-861B allows non-
HDMI to send infoframes.
- only send audio if audio and infoframes are supported.
Otherwise, these are very much like the previous posting of the series,
except rebased upon the mali/hdlcd/tda998x change to remove the
drm_connector_register() call.
https://www.spinics.net/lists/dri-devel/msg121495.html
It'd be nice to have other tda998x users ack and test these patches,
I've tried to test on Juno, but the Juno situation seems to be a huge
fail. (HBI0282B completely fails with latest firmware - (a) FPGA image
incompatibilities io_b118 causes all FPGA AMBA devices to vanish, (b)
seems no way to get SCPI support on it - adding the BL0 executable
start address in the SCC registers seems to be incompatible with the
devchip, causing the PLLs to fail. In discussion with Sudeep over
these issues, but no idea where things are with it at the moment, other
than Sudeep needs to investigate. All Linaro firmware releases are
broken on HBI0282B.)
drivers/gpu/drm/i2c/tda998x_drv.c | 826 ++++++++++++++++++++------------------
1 file changed, 429 insertions(+), 397 deletions(-)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH RFC 01/12] drm/i2c: tda998x: avoid race in tda998x_encoder_mode_set()
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
As priv->audio_params can now be changed at run time, we need to be more
careful about how we deal with a mode set. We must take the audio lock
while checking if there's a valid audio configuration.
However, it's slightly worse than that - during mode set, we mute the
audio, and it must not be unmuted until we have finished the mode set.
It is possible that the audio side may start while a mode set is in
progress, so take the audio_mutex lock around the whole mode setting
procedure.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 088900d78ceb..1cc0433ce9d5 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1074,13 +1074,12 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
tda998x_write_avi(priv, adjusted_mode);
- if (priv->audio_params.format != AFMT_UNUSED) {
- mutex_lock(&priv->audio_mutex);
+ mutex_lock(&priv->audio_mutex);
+ if (priv->audio_params.format != AFMT_UNUSED)
tda998x_configure_audio(priv,
&priv->audio_params,
adjusted_mode->clock);
- mutex_unlock(&priv->audio_mutex);
- }
+ mutex_unlock(&priv->audio_mutex);
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 02/12] drm/i2c: tda998x: avoid racy access to mode clock
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
Avoid a racy access to the mode clock by storing the current mode clock
during a mode set under the audio mutex. This allows us to access it
from the audio path in a safe way.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 1cc0433ce9d5..ab58a13d815e 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -47,6 +47,7 @@ struct tda998x_priv {
u8 vip_cntrl_0;
u8 vip_cntrl_1;
u8 vip_cntrl_2;
+ unsigned long tmds_clock;
struct tda998x_audio_params audio_params;
struct platform_device *audio_pdev;
@@ -713,8 +714,7 @@ static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
static int
tda998x_configure_audio(struct tda998x_priv *priv,
- struct tda998x_audio_params *params,
- unsigned mode_clock)
+ struct tda998x_audio_params *params)
{
u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv;
u32 n;
@@ -771,7 +771,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
* assume 100MHz requires larger divider.
*/
adiv = AUDIO_DIV_SERCLK_8;
- if (mode_clock > 100000)
+ if (priv->tmds_clock > 100000)
adiv++; /* AUDIO_DIV_SERCLK_16 */
/* S/PDIF asks for a larger divider */
@@ -1064,6 +1064,10 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
/* must be last register set: */
reg_write(priv, REG_TBG_CNTRL_0, 0);
+ mutex_lock(&priv->audio_mutex);
+
+ priv->tmds_clock = adjusted_mode->clock;
+
/* Only setup the info frames if the sink is HDMI */
if (priv->is_hdmi_sink) {
/* We need to turn HDMI HDCP stuff on to get audio through */
@@ -1074,13 +1078,11 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
tda998x_write_avi(priv, adjusted_mode);
- mutex_lock(&priv->audio_mutex);
if (priv->audio_params.format != AFMT_UNUSED)
- tda998x_configure_audio(priv,
- &priv->audio_params,
- adjusted_mode->clock);
- mutex_unlock(&priv->audio_mutex);
+ tda998x_configure_audio(priv, &priv->audio_params);
}
+
+ mutex_unlock(&priv->audio_mutex);
}
static enum drm_connector_status
@@ -1226,9 +1228,6 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
.cea = params->cea,
};
- if (!priv->encoder.crtc)
- return -ENODEV;
-
memcpy(audio.status, params->iec.status,
min(sizeof(audio.status), sizeof(params->iec.status)));
@@ -1264,9 +1263,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
}
mutex_lock(&priv->audio_mutex);
- ret = tda998x_configure_audio(priv,
- &audio,
- priv->encoder.crtc->hwmode.clock);
+ ret = tda998x_configure_audio(priv, &audio);
if (ret == 0)
priv->audio_params = audio;
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 03/12] drm/i2c: tda998x: avoid race when programming audio
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
Avoid a race between programming audio and an in-progress mode set.
A mode set is complex, and disables the ability to send infoframes
to the sink, and is disruptive to audio - we have to mute the audio
FIFO while doing a mode set.
If an attempt is made to start up the audio side, we will undo the
audio FIFO mute before the mode set has completed.
Move the lock so that we prevent audio interfering with an in-progress
mode set.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index ab58a13d815e..bafe3c7ad84b 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -971,6 +971,8 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
div = 3;
}
+ mutex_lock(&priv->audio_mutex);
+
/* mute the audio FIFO: */
reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO);
@@ -1064,8 +1066,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
/* must be last register set: */
reg_write(priv, REG_TBG_CNTRL_0, 0);
- mutex_lock(&priv->audio_mutex);
-
priv->tmds_clock = adjusted_mode->clock;
/* Only setup the info frames if the sink is HDMI */
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 04/12] drm/i2c: tda998x: only configure infoframes and audio if supported
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
The CEA 861B specification indicates the situations when we are able to
send each infoframe based on the version of the EDID's CEA extension.
Update the tda998x driver to follow the CEA specification wrt sending
of infoframes.
Since we only support the generation of AVI version 2, this limits us
to CEA extension version 3, so we treat CEA extension version 2 as
CEA 861 (no infoframes, no audio.)
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index bafe3c7ad84b..75833a4650e8 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -43,7 +43,7 @@ struct tda998x_priv {
u16 rev;
u8 current_page;
int dpms;
- bool is_hdmi_sink;
+ bool supports_infoframes;
u8 vip_cntrl_0;
u8 vip_cntrl_1;
u8 vip_cntrl_2;
@@ -1068,8 +1068,20 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
priv->tmds_clock = adjusted_mode->clock;
- /* Only setup the info frames if the sink is HDMI */
- if (priv->is_hdmi_sink) {
+ /* CEA-861B section 6 says that:
+ * CEA version 1 (CEA-861) has no support for infoframes.
+ * CEA version 2 (CEA-861A) supports version 1 AVI infoframes,
+ * and optional basic audio.
+ * CEA version 3 (CEA-861B) supports version 1 and 2 AVI infoframes,
+ * and optional digital audio, with audio infoframes.
+ *
+ * Since we only support generation of version 2 AVI infoframes,
+ * ignore CEA version 2 and below (iow, behave as if we're a
+ * CEA-861 source.)
+ */
+ priv->supports_infoframes = priv->connector.display_info.cea_rev >= 3;
+
+ if (priv->supports_infoframes) {
/* We need to turn HDMI HDCP stuff on to get audio through */
reg &= ~TBG_CNTRL_1_DWIN_DIS;
reg_write(priv, REG_TBG_CNTRL_1, reg);
@@ -1180,7 +1192,6 @@ static int tda998x_connector_get_modes(struct drm_connector *connector)
drm_mode_connector_update_edid_property(connector, edid);
n = drm_add_edid_modes(connector, edid);
- priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
drm_edid_to_eld(connector, edid);
kfree(edid);
@@ -1263,7 +1274,10 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
}
mutex_lock(&priv->audio_mutex);
- ret = tda998x_configure_audio(priv, &audio);
+ if (priv->supports_infoframes)
+ ret = tda998x_configure_audio(priv, &audio);
+ else
+ ret = 0;
if (ret == 0)
priv->audio_params = audio;
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 05/12] drm/i2c: tda998x: only enable audio if supported by sink
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
Check for audio support by the attached sink by consulting the EDID
prior to enabling audio over the TMDS link. We must consult the EDID
after calling drm_helper_probe_single_connector_modes(), as this can
use an override EDID, or load a replacement EDID.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 75833a4650e8..fa13ac0e8691 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -44,6 +44,7 @@ struct tda998x_priv {
u8 current_page;
int dpms;
bool supports_infoframes;
+ bool sink_has_audio;
u8 vip_cntrl_0;
u8 vip_cntrl_1;
u8 vip_cntrl_2;
@@ -1090,13 +1091,33 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
tda998x_write_avi(priv, adjusted_mode);
- if (priv->audio_params.format != AFMT_UNUSED)
+ if (priv->audio_params.format != AFMT_UNUSED &&
+ priv->sink_has_audio)
tda998x_configure_audio(priv, &priv->audio_params);
}
mutex_unlock(&priv->audio_mutex);
}
+static int tda998x_connector_fill_modes(struct drm_connector *connector,
+ uint32_t maxX, uint32_t maxY)
+{
+ struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+ int ret;
+
+ ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
+
+ if (connector->edid_blob_ptr) {
+ struct edid *edid = (void *)connector->edid_blob_ptr->data;
+
+ priv->sink_has_audio = drm_detect_monitor_audio(edid);
+ } else {
+ priv->sink_has_audio = false;
+ }
+
+ return ret;
+}
+
static enum drm_connector_status
tda998x_connector_detect(struct drm_connector *connector, bool force)
{
@@ -1274,7 +1295,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
}
mutex_lock(&priv->audio_mutex);
- if (priv->supports_infoframes)
+ if (priv->supports_infoframes && priv->sink_has_audio)
ret = tda998x_configure_audio(priv, &audio);
else
ret = 0;
@@ -1608,7 +1629,7 @@ static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
static const struct drm_connector_funcs tda998x_connector_funcs = {
.dpms = tda998x_connector_dpms,
.reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
+ .fill_modes = tda998x_connector_fill_modes,
.detect = tda998x_connector_detect,
.destroy = tda998x_connector_destroy,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 06/12] drm/i2c: tda998x: correct function name in comments
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
Correct two references to tda998x_connector_get_modes() which were
incorrectly referring to tda998x_encoder_get_modes().
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index fa13ac0e8691..349e04a30210 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -581,9 +581,9 @@ tda998x_reset(struct tda998x_priv *priv)
* HPD assertion: it needs a delay of 100ms to avoid timing out while
* trying to read EDID data.
*
- * However, tda998x_encoder_get_modes() may be called at any moment
+ * However, tda998x_connector_get_modes() may be called at any moment
* after tda998x_connector_detect() indicates that we are connected, so
- * we need to delay probing modes in tda998x_encoder_get_modes() after
+ * we need to delay probing modes in tda998x_connector_get_modes() after
* we have seen a HPD inactive->active transition. This code implements
* that delay.
*/
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 07/12] drm/i2c: tda998x: move and rename tda998x_encoder_set_config()
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
The naming of tda998x_encoder_set_config() is a left-over from when
TDA998x was a slave encoder. Since this is part of the initialisation,
drop the _encoder from the name, and move it near tda998x_bind().
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 40 +++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 349e04a30210..6b966c19a449 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -822,25 +822,6 @@ tda998x_configure_audio(struct tda998x_priv *priv,
/* DRM encoder functions */
-static void tda998x_encoder_set_config(struct tda998x_priv *priv,
- const struct tda998x_encoder_params *p)
-{
- priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(p->swap_a) |
- (p->mirr_a ? VIP_CNTRL_0_MIRR_A : 0) |
- VIP_CNTRL_0_SWAP_B(p->swap_b) |
- (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0);
- priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) |
- (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) |
- VIP_CNTRL_1_SWAP_D(p->swap_d) |
- (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0);
- priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) |
- (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) |
- VIP_CNTRL_2_SWAP_F(p->swap_f) |
- (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
-
- priv->audio_params = p->audio_params;
-}
-
static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
{
struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
@@ -1636,6 +1617,25 @@ static const struct drm_connector_funcs tda998x_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
+static void tda998x_set_config(struct tda998x_priv *priv,
+ const struct tda998x_encoder_params *p)
+{
+ priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(p->swap_a) |
+ (p->mirr_a ? VIP_CNTRL_0_MIRR_A : 0) |
+ VIP_CNTRL_0_SWAP_B(p->swap_b) |
+ (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0);
+ priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) |
+ (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) |
+ VIP_CNTRL_1_SWAP_D(p->swap_d) |
+ (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0);
+ priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) |
+ (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) |
+ VIP_CNTRL_2_SWAP_F(p->swap_f) |
+ (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
+
+ priv->audio_params = p->audio_params;
+}
+
static int tda998x_bind(struct device *dev, struct device *master, void *data)
{
struct tda998x_encoder_params *params = dev->platform_data;
@@ -1668,7 +1668,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
return ret;
if (!dev->of_node && params)
- tda998x_encoder_set_config(priv, params);
+ tda998x_set_config(priv, params);
tda998x_encoder_set_polling(priv, &priv->connector);
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 08/12] drm/i2c: tda998x: group connector functions and funcs together
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
Group the TDA998x connector functions and funcs structures together
before the encoder support, rather than scattered amongst the rest of
the file. This keeps like code together.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 350 +++++++++++++++++++-------------------
1 file changed, 176 insertions(+), 174 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 6b966c19a449..3f3e1f0137f8 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -820,6 +820,182 @@ tda998x_configure_audio(struct tda998x_priv *priv,
return tda998x_write_aif(priv, ¶ms->cea);
}
+/* DRM connector functions */
+
+static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
+{
+ if (drm_core_check_feature(connector->dev, DRIVER_ATOMIC))
+ return drm_atomic_helper_connector_dpms(connector, mode);
+ else
+ return drm_helper_connector_dpms(connector, mode);
+}
+
+static int tda998x_connector_fill_modes(struct drm_connector *connector,
+ uint32_t maxX, uint32_t maxY)
+{
+ struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+ int ret;
+
+ ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
+
+ if (connector->edid_blob_ptr) {
+ struct edid *edid = (void *)connector->edid_blob_ptr->data;
+
+ priv->sink_has_audio = drm_detect_monitor_audio(edid);
+ } else {
+ priv->sink_has_audio = false;
+ }
+
+ return ret;
+}
+
+static enum drm_connector_status
+tda998x_connector_detect(struct drm_connector *connector, bool force)
+{
+ struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+ u8 val = cec_read(priv, REG_CEC_RXSHPDLEV);
+
+ return (val & CEC_RXSHPDLEV_HPD) ? connector_status_connected :
+ connector_status_disconnected;
+}
+
+static void tda998x_connector_destroy(struct drm_connector *connector)
+{
+ drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs tda998x_connector_funcs = {
+ .dpms = tda998x_connector_dpms,
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = tda998x_connector_fill_modes,
+ .detect = tda998x_connector_detect,
+ .destroy = tda998x_connector_destroy,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
+{
+ struct tda998x_priv *priv = data;
+ u8 offset, segptr;
+ int ret, i;
+
+ offset = (blk & 1) ? 128 : 0;
+ segptr = blk / 2;
+
+ reg_write(priv, REG_DDC_ADDR, 0xa0);
+ reg_write(priv, REG_DDC_OFFS, offset);
+ reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);
+ reg_write(priv, REG_DDC_SEGM, segptr);
+
+ /* enable reading EDID: */
+ priv->wq_edid_wait = 1;
+ reg_write(priv, REG_EDID_CTRL, 0x1);
+
+ /* flag must be cleared by sw: */
+ reg_write(priv, REG_EDID_CTRL, 0x0);
+
+ /* wait for block read to complete: */
+ if (priv->hdmi->irq) {
+ i = wait_event_timeout(priv->wq_edid,
+ !priv->wq_edid_wait,
+ msecs_to_jiffies(100));
+ if (i < 0) {
+ dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);
+ return i;
+ }
+ } else {
+ for (i = 100; i > 0; i--) {
+ msleep(1);
+ ret = reg_read(priv, REG_INT_FLAGS_2);
+ if (ret < 0)
+ return ret;
+ if (ret & INT_FLAGS_2_EDID_BLK_RD)
+ break;
+ }
+ }
+
+ if (i == 0) {
+ dev_err(&priv->hdmi->dev, "read edid timeout\n");
+ return -ETIMEDOUT;
+ }
+
+ ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
+ if (ret != length) {
+ dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
+ blk, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int tda998x_connector_get_modes(struct drm_connector *connector)
+{
+ struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+ struct edid *edid;
+ int n;
+
+ /*
+ * If we get killed while waiting for the HPD timeout, return
+ * no modes found: we are not in a restartable path, so we
+ * can't handle signals gracefully.
+ */
+ if (tda998x_edid_delay_wait(priv))
+ return 0;
+
+ if (priv->rev == TDA19988)
+ reg_clear(priv, REG_TX4, TX4_PD_RAM);
+
+ edid = drm_do_get_edid(connector, read_edid_block, priv);
+
+ if (priv->rev == TDA19988)
+ reg_set(priv, REG_TX4, TX4_PD_RAM);
+
+ if (!edid) {
+ dev_warn(&priv->hdmi->dev, "failed to read EDID\n");
+ return 0;
+ }
+
+ drm_mode_connector_update_edid_property(connector, edid);
+ n = drm_add_edid_modes(connector, edid);
+ drm_edid_to_eld(connector, edid);
+
+ kfree(edid);
+
+ return n;
+}
+
+static int tda998x_connector_mode_valid(struct drm_connector *connector,
+ struct drm_display_mode *mode)
+{
+ /* TDA19988 dotclock can go up to 165MHz */
+ struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+
+ if (mode->clock > ((priv->rev == TDA19988) ? 165000 : 150000))
+ return MODE_CLOCK_HIGH;
+ if (mode->htotal >= BIT(13))
+ return MODE_BAD_HVALUE;
+ if (mode->vtotal >= BIT(11))
+ return MODE_BAD_VVALUE;
+ return MODE_OK;
+}
+
+static struct drm_encoder *
+tda998x_connector_best_encoder(struct drm_connector *connector)
+{
+ struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+
+ return &priv->encoder;
+}
+
+static
+const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
+ .get_modes = tda998x_connector_get_modes,
+ .mode_valid = tda998x_connector_mode_valid,
+ .best_encoder = tda998x_connector_best_encoder,
+};
+
/* DRM encoder functions */
static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
@@ -855,21 +1031,6 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
priv->dpms = mode;
}
-static int tda998x_connector_mode_valid(struct drm_connector *connector,
- struct drm_display_mode *mode)
-{
- /* TDA19988 dotclock can go up to 165MHz */
- struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-
- if (mode->clock > ((priv->rev == TDA19988) ? 165000 : 150000))
- return MODE_CLOCK_HIGH;
- if (mode->htotal >= BIT(13))
- return MODE_BAD_HVALUE;
- if (mode->vtotal >= BIT(11))
- return MODE_BAD_VVALUE;
- return MODE_OK;
-}
-
static void
tda998x_encoder_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
@@ -1080,127 +1241,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
mutex_unlock(&priv->audio_mutex);
}
-static int tda998x_connector_fill_modes(struct drm_connector *connector,
- uint32_t maxX, uint32_t maxY)
-{
- struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
- int ret;
-
- ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
-
- if (connector->edid_blob_ptr) {
- struct edid *edid = (void *)connector->edid_blob_ptr->data;
-
- priv->sink_has_audio = drm_detect_monitor_audio(edid);
- } else {
- priv->sink_has_audio = false;
- }
-
- return ret;
-}
-
-static enum drm_connector_status
-tda998x_connector_detect(struct drm_connector *connector, bool force)
-{
- struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
- u8 val = cec_read(priv, REG_CEC_RXSHPDLEV);
-
- return (val & CEC_RXSHPDLEV_HPD) ? connector_status_connected :
- connector_status_disconnected;
-}
-
-static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
-{
- struct tda998x_priv *priv = data;
- u8 offset, segptr;
- int ret, i;
-
- offset = (blk & 1) ? 128 : 0;
- segptr = blk / 2;
-
- reg_write(priv, REG_DDC_ADDR, 0xa0);
- reg_write(priv, REG_DDC_OFFS, offset);
- reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);
- reg_write(priv, REG_DDC_SEGM, segptr);
-
- /* enable reading EDID: */
- priv->wq_edid_wait = 1;
- reg_write(priv, REG_EDID_CTRL, 0x1);
-
- /* flag must be cleared by sw: */
- reg_write(priv, REG_EDID_CTRL, 0x0);
-
- /* wait for block read to complete: */
- if (priv->hdmi->irq) {
- i = wait_event_timeout(priv->wq_edid,
- !priv->wq_edid_wait,
- msecs_to_jiffies(100));
- if (i < 0) {
- dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);
- return i;
- }
- } else {
- for (i = 100; i > 0; i--) {
- msleep(1);
- ret = reg_read(priv, REG_INT_FLAGS_2);
- if (ret < 0)
- return ret;
- if (ret & INT_FLAGS_2_EDID_BLK_RD)
- break;
- }
- }
-
- if (i == 0) {
- dev_err(&priv->hdmi->dev, "read edid timeout\n");
- return -ETIMEDOUT;
- }
-
- ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
- if (ret != length) {
- dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
- blk, ret);
- return ret;
- }
-
- return 0;
-}
-
-static int tda998x_connector_get_modes(struct drm_connector *connector)
-{
- struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
- struct edid *edid;
- int n;
-
- /*
- * If we get killed while waiting for the HPD timeout, return
- * no modes found: we are not in a restartable path, so we
- * can't handle signals gracefully.
- */
- if (tda998x_edid_delay_wait(priv))
- return 0;
-
- if (priv->rev == TDA19988)
- reg_clear(priv, REG_TX4, TX4_PD_RAM);
-
- edid = drm_do_get_edid(connector, read_edid_block, priv);
-
- if (priv->rev == TDA19988)
- reg_set(priv, REG_TX4, TX4_PD_RAM);
-
- if (!edid) {
- dev_warn(&priv->hdmi->dev, "failed to read EDID\n");
- return 0;
- }
-
- drm_mode_connector_update_edid_property(connector, edid);
- n = drm_add_edid_modes(connector, edid);
- drm_edid_to_eld(connector, edid);
-
- kfree(edid);
-
- return n;
-}
-
static void tda998x_encoder_set_polling(struct tda998x_priv *priv,
struct drm_connector *connector)
{
@@ -1579,44 +1619,6 @@ static const struct drm_encoder_funcs tda998x_encoder_funcs = {
.destroy = tda998x_encoder_destroy,
};
-static struct drm_encoder *
-tda998x_connector_best_encoder(struct drm_connector *connector)
-{
- struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-
- return &priv->encoder;
-}
-
-static
-const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
- .get_modes = tda998x_connector_get_modes,
- .mode_valid = tda998x_connector_mode_valid,
- .best_encoder = tda998x_connector_best_encoder,
-};
-
-static void tda998x_connector_destroy(struct drm_connector *connector)
-{
- drm_connector_cleanup(connector);
-}
-
-static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
-{
- if (drm_core_check_feature(connector->dev, DRIVER_ATOMIC))
- return drm_atomic_helper_connector_dpms(connector, mode);
- else
- return drm_helper_connector_dpms(connector, mode);
-}
-
-static const struct drm_connector_funcs tda998x_connector_funcs = {
- .dpms = tda998x_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .fill_modes = tda998x_connector_fill_modes,
- .detect = tda998x_connector_detect,
- .destroy = tda998x_connector_destroy,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
static void tda998x_set_config(struct tda998x_priv *priv,
const struct tda998x_encoder_params *p)
{
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 09/12] drm/i2c: tda998x: separate connector initialisation
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
Separate out the connector initialisation from the rest of the drivers
initialisation.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 46 ++++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 3f3e1f0137f8..af3dc5528663 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -996,6 +996,31 @@ const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
.best_encoder = tda998x_connector_best_encoder,
};
+static int tda998x_connector_init(struct tda998x_priv *priv,
+ struct drm_device *drm)
+{
+ struct drm_connector *connector = &priv->connector;
+ int ret;
+
+ connector->interlace_allowed = 1;
+
+ if (priv->hdmi->irq)
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+ else
+ connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+ DRM_CONNECTOR_POLL_DISCONNECT;
+
+ drm_connector_helper_add(connector, &tda998x_connector_helper_funcs);
+ ret = drm_connector_init(drm, connector, &tda998x_connector_funcs,
+ DRM_MODE_CONNECTOR_HDMIA);
+ if (ret)
+ return ret;
+
+ drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
+
+ return 0;
+}
+
/* DRM encoder functions */
static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
@@ -1241,16 +1266,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
mutex_unlock(&priv->audio_mutex);
}
-static void tda998x_encoder_set_polling(struct tda998x_priv *priv,
- struct drm_connector *connector)
-{
- if (priv->hdmi->irq)
- connector->polled = DRM_CONNECTOR_POLL_HPD;
- else
- connector->polled = DRM_CONNECTOR_POLL_CONNECT |
- DRM_CONNECTOR_POLL_DISCONNECT;
-}
-
static void tda998x_destroy(struct tda998x_priv *priv)
{
/* disable all IRQs and free the IRQ handler */
@@ -1662,7 +1677,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
crtcs = 1 << 0;
}
- priv->connector.interlace_allowed = 1;
priv->encoder.possible_crtcs = crtcs;
ret = tda998x_create(client, priv);
@@ -1672,24 +1686,16 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
if (!dev->of_node && params)
tda998x_set_config(priv, params);
- tda998x_encoder_set_polling(priv, &priv->connector);
-
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;
- drm_connector_helper_add(&priv->connector,
- &tda998x_connector_helper_funcs);
- ret = drm_connector_init(drm, &priv->connector,
- &tda998x_connector_funcs,
- DRM_MODE_CONNECTOR_HDMIA);
+ ret = tda998x_connector_init(priv, drm);
if (ret)
goto err_connector;
- drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
-
return 0;
err_connector:
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 10/12] drm/i2c: tda998x: group audio functions together
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
Group the TDA998x audio functions together rather than split between
two different locations in the file, keeping like code together.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 276 +++++++++++++++++++-------------------
1 file changed, 139 insertions(+), 137 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index af3dc5528663..b1ad5ed5d645 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -702,6 +702,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
}
+/* Audio support */
+
static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
{
if (on) {
@@ -820,6 +822,143 @@ tda998x_configure_audio(struct tda998x_priv *priv,
return tda998x_write_aif(priv, ¶ms->cea);
}
+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);
+ int i, ret;
+ struct tda998x_audio_params audio = {
+ .sample_width = params->sample_width,
+ .sample_rate = params->sample_rate,
+ .cea = params->cea,
+ };
+
+ memcpy(audio.status, params->iec.status,
+ min(sizeof(audio.status), sizeof(params->iec.status)));
+
+ switch (daifmt->fmt) {
+ case HDMI_I2S:
+ if (daifmt->bit_clk_inv || daifmt->frame_clk_inv ||
+ daifmt->bit_clk_master || daifmt->frame_clk_master) {
+ dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__,
+ daifmt->bit_clk_inv, daifmt->frame_clk_inv,
+ daifmt->bit_clk_master,
+ daifmt->frame_clk_master);
+ return -EINVAL;
+ }
+ for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
+ if (priv->audio_port[i].format == AFMT_I2S)
+ audio.config = priv->audio_port[i].config;
+ audio.format = AFMT_I2S;
+ break;
+ case HDMI_SPDIF:
+ for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
+ if (priv->audio_port[i].format == AFMT_SPDIF)
+ audio.config = priv->audio_port[i].config;
+ audio.format = AFMT_SPDIF;
+ break;
+ default:
+ dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt);
+ return -EINVAL;
+ }
+
+ if (audio.config == 0) {
+ dev_err(dev, "%s: No audio configutation found\n", __func__);
+ return -EINVAL;
+ }
+
+ mutex_lock(&priv->audio_mutex);
+ if (priv->supports_infoframes && priv->sink_has_audio)
+ ret = tda998x_configure_audio(priv, &audio);
+ else
+ ret = 0;
+
+ if (ret == 0)
+ priv->audio_params = audio;
+ mutex_unlock(&priv->audio_mutex);
+
+ return ret;
+}
+
+static void tda998x_audio_shutdown(struct device *dev, void *data)
+{
+ struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+ mutex_lock(&priv->audio_mutex);
+
+ reg_write(priv, REG_ENA_AP, 0);
+
+ priv->audio_params.format = AFMT_UNUSED;
+
+ mutex_unlock(&priv->audio_mutex);
+}
+
+int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
+{
+ struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+ mutex_lock(&priv->audio_mutex);
+
+ tda998x_audio_mute(priv, enable);
+
+ mutex_unlock(&priv->audio_mutex);
+ return 0;
+}
+
+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 drm_mode_config *config = &priv->encoder.dev->mode_config;
+ struct drm_connector *connector;
+ int ret = -ENODEV;
+
+ mutex_lock(&config->mutex);
+ list_for_each_entry(connector, &config->connector_list, head) {
+ if (&priv->encoder == connector->encoder) {
+ memcpy(buf, connector->eld,
+ min(sizeof(connector->eld), len));
+ ret = 0;
+ }
+ }
+ mutex_unlock(&config->mutex);
+
+ return ret;
+}
+
+static const struct hdmi_codec_ops audio_codec_ops = {
+ .hw_params = tda998x_audio_hw_params,
+ .audio_shutdown = tda998x_audio_shutdown,
+ .digital_mute = tda998x_audio_digital_mute,
+ .get_eld = tda998x_audio_get_eld,
+};
+
+static int tda998x_audio_codec_init(struct tda998x_priv *priv,
+ struct device *dev)
+{
+ struct hdmi_codec_pdata codec_data = {
+ .ops = &audio_codec_ops,
+ .max_i2s_channels = 2,
+ };
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) {
+ if (priv->audio_port[i].format == AFMT_I2S &&
+ priv->audio_port[i].config != 0)
+ codec_data.i2s = 1;
+ if (priv->audio_port[i].format == AFMT_SPDIF &&
+ priv->audio_port[i].config != 0)
+ codec_data.spdif = 1;
+ }
+
+ priv->audio_pdev = platform_device_register_data(
+ dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
+ &codec_data, sizeof(codec_data));
+
+ return PTR_ERR_OR_ZERO(priv->audio_pdev);
+}
+
/* DRM connector functions */
static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
@@ -1284,143 +1423,6 @@ static void tda998x_destroy(struct tda998x_priv *priv)
i2c_unregister_device(priv->cec);
}
-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);
- int i, ret;
- struct tda998x_audio_params audio = {
- .sample_width = params->sample_width,
- .sample_rate = params->sample_rate,
- .cea = params->cea,
- };
-
- memcpy(audio.status, params->iec.status,
- min(sizeof(audio.status), sizeof(params->iec.status)));
-
- switch (daifmt->fmt) {
- case HDMI_I2S:
- if (daifmt->bit_clk_inv || daifmt->frame_clk_inv ||
- daifmt->bit_clk_master || daifmt->frame_clk_master) {
- dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__,
- daifmt->bit_clk_inv, daifmt->frame_clk_inv,
- daifmt->bit_clk_master,
- daifmt->frame_clk_master);
- return -EINVAL;
- }
- for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
- if (priv->audio_port[i].format == AFMT_I2S)
- audio.config = priv->audio_port[i].config;
- audio.format = AFMT_I2S;
- break;
- case HDMI_SPDIF:
- for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
- if (priv->audio_port[i].format == AFMT_SPDIF)
- audio.config = priv->audio_port[i].config;
- audio.format = AFMT_SPDIF;
- break;
- default:
- dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt);
- return -EINVAL;
- }
-
- if (audio.config == 0) {
- dev_err(dev, "%s: No audio configutation found\n", __func__);
- return -EINVAL;
- }
-
- mutex_lock(&priv->audio_mutex);
- if (priv->supports_infoframes && priv->sink_has_audio)
- ret = tda998x_configure_audio(priv, &audio);
- else
- ret = 0;
-
- if (ret == 0)
- priv->audio_params = audio;
- mutex_unlock(&priv->audio_mutex);
-
- return ret;
-}
-
-static void tda998x_audio_shutdown(struct device *dev, void *data)
-{
- struct tda998x_priv *priv = dev_get_drvdata(dev);
-
- mutex_lock(&priv->audio_mutex);
-
- reg_write(priv, REG_ENA_AP, 0);
-
- priv->audio_params.format = AFMT_UNUSED;
-
- mutex_unlock(&priv->audio_mutex);
-}
-
-int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
-{
- struct tda998x_priv *priv = dev_get_drvdata(dev);
-
- mutex_lock(&priv->audio_mutex);
-
- tda998x_audio_mute(priv, enable);
-
- mutex_unlock(&priv->audio_mutex);
- return 0;
-}
-
-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 drm_mode_config *config = &priv->encoder.dev->mode_config;
- struct drm_connector *connector;
- int ret = -ENODEV;
-
- mutex_lock(&config->mutex);
- list_for_each_entry(connector, &config->connector_list, head) {
- if (&priv->encoder == connector->encoder) {
- memcpy(buf, connector->eld,
- min(sizeof(connector->eld), len));
- ret = 0;
- }
- }
- mutex_unlock(&config->mutex);
-
- return ret;
-}
-
-static const struct hdmi_codec_ops audio_codec_ops = {
- .hw_params = tda998x_audio_hw_params,
- .audio_shutdown = tda998x_audio_shutdown,
- .digital_mute = tda998x_audio_digital_mute,
- .get_eld = tda998x_audio_get_eld,
-};
-
-static int tda998x_audio_codec_init(struct tda998x_priv *priv,
- struct device *dev)
-{
- struct hdmi_codec_pdata codec_data = {
- .ops = &audio_codec_ops,
- .max_i2s_channels = 2,
- };
- int i;
-
- for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) {
- if (priv->audio_port[i].format == AFMT_I2S &&
- priv->audio_port[i].config != 0)
- codec_data.i2s = 1;
- if (priv->audio_port[i].format == AFMT_SPDIF &&
- priv->audio_port[i].config != 0)
- codec_data.spdif = 1;
- }
-
- priv->audio_pdev = platform_device_register_data(
- dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
- &codec_data, sizeof(codec_data));
-
- return PTR_ERR_OR_ZERO(priv->audio_pdev);
-}
-
/* I2C driver functions */
static int tda998x_get_audio_ports(struct tda998x_priv *priv,
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 11/12] drm/i2c: tda998x: remove complexity from tda998x_audio_get_eld()
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
tda998x_audio_get_eld() is needlessly complex - the connector associated
with the encoder is always our own priv->connector. Remove this
complexity, but ensure that there are no races when copying out the ELD.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index b1ad5ed5d645..226871e34442 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -910,21 +910,13 @@ 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 drm_mode_config *config = &priv->encoder.dev->mode_config;
- struct drm_connector *connector;
- int ret = -ENODEV;
-
- mutex_lock(&config->mutex);
- list_for_each_entry(connector, &config->connector_list, head) {
- if (&priv->encoder == connector->encoder) {
- memcpy(buf, connector->eld,
- min(sizeof(connector->eld), len));
- ret = 0;
- }
- }
- mutex_unlock(&config->mutex);
- return ret;
+ mutex_lock(&priv->audio_mutex);
+ memcpy(buf, priv->connector.eld,
+ min(sizeof(priv->connector.eld), len));
+ mutex_unlock(&priv->audio_mutex);
+
+ return 0;
}
static const struct hdmi_codec_ops audio_codec_ops = {
@@ -975,6 +967,7 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector,
struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
int ret;
+ mutex_lock(&priv->audio_mutex);
ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
if (connector->edid_blob_ptr) {
@@ -984,6 +977,7 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector,
} else {
priv->sink_has_audio = false;
}
+ mutex_unlock(&priv->audio_mutex);
return ret;
}
--
2.7.4
^ permalink raw reply related
* [PATCH RFC 12/12] drm/i2c: tda998x: switch to boolean is_on
From: Russell King @ 2016-11-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108122420.GP1041@n2100.armlinux.org.uk>
Rather than storing the DPMS mode (which will always be on or off) use a
boolean to store this instead.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 226871e34442..3a5e5c466972 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -42,7 +42,7 @@ struct tda998x_priv {
struct mutex mutex;
u16 rev;
u8 current_page;
- int dpms;
+ bool is_on;
bool supports_infoframes;
bool sink_has_audio;
u8 vip_cntrl_0;
@@ -1159,16 +1159,15 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
{
struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+ bool on;
/* we only care about on or off: */
- if (mode != DRM_MODE_DPMS_ON)
- mode = DRM_MODE_DPMS_OFF;
+ on = mode == DRM_MODE_DPMS_ON;
- if (mode == priv->dpms)
+ if (on == priv->is_on)
return;
- switch (mode) {
- case DRM_MODE_DPMS_ON:
+ 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);
@@ -1177,16 +1176,16 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
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);
- break;
- case DRM_MODE_DPMS_OFF:
+
+ 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);
- break;
- }
- priv->dpms = mode;
+ priv->is_on = false;
+ }
}
static void
@@ -1480,8 +1479,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
if (!priv->cec)
return -ENODEV;
- priv->dpms = DRM_MODE_DPMS_OFF;
-
mutex_init(&priv->mutex); /* protect the page access */
init_waitqueue_head(&priv->edid_delay_waitq);
setup_timer(&priv->edid_delay_timer, tda998x_edid_delay_done,
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox