* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: John Garry @ 2016-11-08 15:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1609380.NN50qvVsP7@wuerfel>
On 08/11/2016 15:10, Arnd Bergmann wrote:
> On Tuesday, November 8, 2016 1:49:43 PM CET John Garry wrote:
>>
>> Hi Arnd,
>>
>> Thanks for the reference.
>>
>> I think the i2c interface doesn't fully satisfy our requirements as we
>> need more than just a slave bus address when accessing the slave device
>> (which I think is what i2c uses). We also need to pass "offset" and
>> "mod_mask" arguments to the djtag adapter to access specific registers
>> in the slave device.
>
> Ok. Are those values constant per device, or maybe a range? We may want to
> include those in the reg property as well then.
>
> Arnd
>
Hi Arnd,
I'm not sure, I'll defer to Tan/Anurup.
Cheers,
John
>
> .
>
^ permalink raw reply
* [PATCH 0/8] firmware: arm_scpi: add support for legacy SCPI protocol
From: Sudeep Holla @ 2016-11-08 15:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108145118.GR1041@n2100.armlinux.org.uk>
On 08/11/16 14:51, Russell King - ARM Linux wrote:
> On Wed, Nov 02, 2016 at 10:52:03PM -0600, Sudeep Holla wrote:
>> This is minor rework of the series[1] from Neil Armstrong's to support
>> legacy SCPI protocol to make DT bindings more generic and move out all
>> the platform specific bindings out of the generic binding document.
>
> Is this what would be in my HBI0282B Juno?
>
No, it's one on the AmLogic Meson GXBB platform. Juno never supported
that except that old firmware use it internally. By that I mean some
version of trusted firmware used legacy SCPI but they are generally
bundled together in fip, so you should not see any issue with upgrade.
> (Note: I only have the original firmware on the board, as the Linaro
> firmware drops are completely broken for it.)
>
I am currently trying to run Linaro 16.10 release, I don't see any issue
except network boot from UEFI which is known and reported.
I will go through your logs in detail and try to replicate your issue.
I assume you have tried replacing the entry contents of the uSD with the
release. I will start with that now.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: Arnd Bergmann @ 2016-11-08 15:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2bac1a34-591b-6557-15bf-40db25c3d129@huawei.com>
On Tuesday, November 8, 2016 1:49:43 PM CET John Garry wrote:
>
> Hi Arnd,
>
> Thanks for the reference.
>
> I think the i2c interface doesn't fully satisfy our requirements as we
> need more than just a slave bus address when accessing the slave device
> (which I think is what i2c uses). We also need to pass "offset" and
> "mod_mask" arguments to the djtag adapter to access specific registers
> in the slave device.
Ok. Are those values constant per device, or maybe a range? We may want to
include those in the reg property as well then.
Arnd
^ permalink raw reply
* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: Arnd Bergmann @ 2016-11-08 15:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5821D736.7080803@gmail.com>
On Tuesday, November 8, 2016 7:16:30 PM CET Anurup M wrote:
> >>>> 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.
> There is a difference in djtag version of CPU and IO die (in some chips).
> For ex: in hip06 djtag for CPU is v1 and for IO is v2.
> so they use different readwrite handlers djtag_readwrite_(v1/2).
>
> + /* 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 the same djtag version, there is no difference in handling in the
> driver.
Right, but my point was about the compatibility with the older chips
using the same IP block, marking the ones as compatible that actually
use the same interface.
I also see that the compatible strings have the version included in
them, and you can probably drop them by requiring them only in the
fallback:
compatible = "hisilicon,hip05-cpu-djtag", "hisilicon,djtag-v1";
compatible = "hisilicon,hip05-io-djtag", "hisilicon,djtag-v1";
compatible = "hisilicon,hip06-cpu-djtag", "hisilicon,djtag-v1";
compatible = "hisilicon,hip06-io-djtag", "hisilicon,djtag-v2";
compatible = "hisilicon,hip07-cpu-djtag", "hisilicon,djtag-v2";
compatible = "hisilicon,hip07-io-djtag", "hisilicon,djtag-v2";
We want to have the first entry be as specific as possible, but
the last (second) entry is the one that can be used by the driver
for matching. When a future hip08/hip09/... chip uses an existing
interface, you then don't have to update the driver.
Arnd
^ permalink raw reply
* [PATCH RFC 2/7] ARM: S3C64XX: Add DMA slave maps for PL080 devices
From: Charles Keepax @ 2016-11-08 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108144445.GD1575@localhost.localdomain>
On Tue, Nov 08, 2016 at 02:44:45PM +0000, Charles Keepax wrote:
> On Fri, Nov 04, 2016 at 05:14:49PM +0100, Sylwester Nawrocki wrote:
> > This patch adds DMA slave map tables to the pl080 devices's
> > platform_data in order to support the new channel request API.
> > A few devices for which there was no DMA support with current
> > code are omitted from the tables.
> >
> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > ---
> > arch/arm/mach-s3c64xx/pl080.c | 32 ++++++++++++++++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> >
> > diff --git a/arch/arm/mach-s3c64xx/pl080.c b/arch/arm/mach-s3c64xx/pl080.c
> > index 89c5a62..8c88680 100644
> > --- a/arch/arm/mach-s3c64xx/pl080.c
> > +++ b/arch/arm/mach-s3c64xx/pl080.c
> <snip>
> > @@ -134,6 +153,8 @@ struct pl08x_platform_data s3c64xx_dma0_plat_data = {
> > .put_xfer_signal = pl08x_put_xfer_signal,
> > .slave_channels = s3c64xx_dma0_info,
> > .num_slave_channels = ARRAY_SIZE(s3c64xx_dma0_info),
> > + .slave_map = s3c64xx_dma0_slave_map,
> > + .slavecnt = ARRAY_SIZE(s3c64xx_dma0_slave_map),
> > };
>
> Here we add a .slavecnt but the pl08x_platform_data structure doesn't
> contain that field. I can't see it on the branch you linked in
> the cover letter either, is it added by a patch on another branch
> I am missing?
>
Ah I think I see it should be .slave_map_len here.
Thanks,
Charles
^ permalink raw reply
* [PATCH] iommu/dma-iommu: properly respect configured address space size
From: Marek Szyprowski @ 2016-11-08 14:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d215c05c-b922-4bee-a2dc-695669a801f6@arm.com>
Hi Robin,
On 2016-11-08 15:44, Robin Murphy wrote:
> On 08/11/16 13:41, Marek Szyprowski wrote:
>> On 2016-11-08 12:37, Robin Murphy wrote:
>>> 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?
>> Yes, I found this issue while working on next version of ARM & ARM64
>> DMA-mapping/IOMMU integration patchset and adapting Exynos drivers for the
>> new IOMMU/DMA-mapping glue.
>>
>> Some Exynos devices (codec and camera isp) operate only on the limited (and
>> really small: 256M for example) DMA window. They use non-standard way of
>> addressing memory: an offset from the firmware base. However they still
>> have
>> 32bit DMA mask, as the firmware can be located basically everywhere in the
>> real DMA address space, but then they can access only next 256M from that
>> firmware base.
> OK, that's good to know, thanks. However, I think in this case it sounds
> like it's really the DMA mask that's the underlying problem - if those
> blocks themselves can only drive 28 address bits, then the struct
> devices representing them should have 28-bit DMA masks, and the
> "firmware base" of whoever's driving the upper bits modelled with a
> dma_pfn_offset. Even if they do have full 32-bit interfaces themselves,
> but are constrained to segment-offset addressing internally, I still
> think it would be tidier to represent things that way.
>
> At some point in dma-iommu development I did have support for DMA
> offsets upstream of the IOMMU, and am happy to reinstate it if there's a
> real use case (assuming you can't simply always set the firmware base to
> 0 when using the IOMMU).
That would indeed look a bit simpler, but I've already tried such approach
and the firmware crashes when its base in real DMA address space is set to
zero.
>>>> 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.
>> Right, this dma_32bit_pfn was really misleading at the first glance,
>> but then I found that this was something like end_pfn in case of dma-iommu
>> code.
> Yes, that was my incorrect assumption - at the time I interpreted it as
> a de-facto upper limit which was still possible to allocate above in
> special circumstances, which turns out to be almost entirely backwards.
> I'd rather not bake that into the dma-iommu code any further if we can
> avoid it (I'll try throwing an RFC together to clear up what's there
> already).
Okay.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH 0/8] firmware: arm_scpi: add support for legacy SCPI protocol
From: Russell King - ARM Linux @ 2016-11-08 14:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla@arm.com>
On Wed, Nov 02, 2016 at 10:52:03PM -0600, Sudeep Holla wrote:
> This is minor rework of the series[1] from Neil Armstrong's to support
> legacy SCPI protocol to make DT bindings more generic and move out all
> the platform specific bindings out of the generic binding document.
Is this what would be in my HBI0282B Juno?
(Note: I only have the original firmware on the board, as the Linaro
firmware drops are completely broken for it.)
--
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 v2 0/5] ARM: da850: new drivers for better LCDC support
From: Sekhar Nori @ 2016-11-08 14:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477925138-23457-1-git-send-email-bgolaszewski@baylibre.com>
+ Arnd, Olof
On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
> This series adds two new drivers in order to better support the LCDC
> rev1 present on the da850 boards.
>
> The first patch adds a new memory driver which allows to write to the
> DDR2/mDDR memory controller present on the da8xx SoCs.
>
> The second patch adds a new bus driver which allows to interact with
> the MSTPRI registers of the SYSCFG0 module
I think patches 1/5 and 2/5 are ready to be merged. If there are no
objections I would like to send a pull request for them to be merged
through ARM-SoC tree for v4.10 kernel.
Thanks,
Sekhar
>
> As is mentioned in the comments: we don't want to commit to supporting
> stable interfaces (DT bindings or sysfs attributes) so we hardcode the
> settings required by some boards (for now only da850-lcdk) with the
> hope that linux gets an appropriate framework for performance knobs
> in the future.
>
> Potential extensions of these drivers should be straightforward in the
> future.
>
> Subsequent patches add DT nodes for the new drivers: disabled nodes
> in da850.dtsi and enabled in da850-lcdk.dts.
>
> The last patch adds a workaround for current lack of support for drm
> bridges in tilcdc.
>
> Tested on a da850-lcdk with a display connected over VGA and two
> additional patches for tilcdc (sent to linux-drm): ran simple modetest
> for supported resolutions, used X.org and fluxbox as graphical
> environment, played video with mplayer.
>
> v1 -> v2:
> - used regular readl()/writel() instead of __raw_** versions
> - used resource_size() instead of calculating the size by hand
> - used ioremap instead of syscon in patch [2/5]
> - added the DT nodes in patches [3/5]-[5/5]
>
> Bartosz Golaszewski (5):
> ARM: memory: da8xx-ddrctl: new driver
> ARM: bus: da8xx-mstpri: new driver
> ARM: dts: da850: add the mstpri and ddrctl nodes
> ARM: dts: da850-lcdk: enable mstpri and ddrctl nodes
> ARM: dts: da850-lcdk: add tilcdc panel node
>
> .../devicetree/bindings/bus/ti,da850-mstpri.txt | 20 ++
> .../memory-controllers/ti-da8xx-ddrctl.txt | 20 ++
> arch/arm/boot/dts/da850-lcdk.dts | 71 ++++++
> arch/arm/boot/dts/da850.dtsi | 11 +
> drivers/bus/Kconfig | 9 +
> drivers/bus/Makefile | 2 +
> drivers/bus/da8xx-mstpri.c | 269 +++++++++++++++++++++
> drivers/memory/Kconfig | 8 +
> drivers/memory/Makefile | 1 +
> drivers/memory/da8xx-ddrctl.c | 175 ++++++++++++++
> 10 files changed, 586 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
> create mode 100644 drivers/bus/da8xx-mstpri.c
> create mode 100644 drivers/memory/da8xx-ddrctl.c
>
^ permalink raw reply
* [PATCH 0/5] ARM: OMAP: dead code removal
From: Tony Lindgren @ 2016-11-08 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKP5S=05P4p7qBXtHZepQ7Xchuj-2FZdVXiy+FBjTv9WRxhCsw@mail.gmail.com>
* Nicolae Rosia <nicolae.rosia.oss@gmail.com> [161107 23:56]:
> On Wed, Nov 2, 2016 at 2:51 PM, Joshua Clayton <stillcompiling@gmail.com> wrote:
> > I think the commit logs for these patches
> > need a little detail on why the code is
> > no longer needed. For posterity.
> Hi,
>
> Thanks, I have updated the series but I unfortunately I've sent it as
> a reply to the old thread.
That's fine, I've tagged it for next but have not applied
yet.
Regards,
Tony
^ permalink raw reply
* [PATCH RFC 2/7] ARM: S3C64XX: Add DMA slave maps for PL080 devices
From: Charles Keepax @ 2016-11-08 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478276094-19135-4-git-send-email-s.nawrocki@samsung.com>
On Fri, Nov 04, 2016 at 05:14:49PM +0100, Sylwester Nawrocki wrote:
> This patch adds DMA slave map tables to the pl080 devices's
> platform_data in order to support the new channel request API.
> A few devices for which there was no DMA support with current
> code are omitted from the tables.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> arch/arm/mach-s3c64xx/pl080.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm/mach-s3c64xx/pl080.c b/arch/arm/mach-s3c64xx/pl080.c
> index 89c5a62..8c88680 100644
> --- a/arch/arm/mach-s3c64xx/pl080.c
> +++ b/arch/arm/mach-s3c64xx/pl080.c
<snip>
> @@ -134,6 +153,8 @@ struct pl08x_platform_data s3c64xx_dma0_plat_data = {
> .put_xfer_signal = pl08x_put_xfer_signal,
> .slave_channels = s3c64xx_dma0_info,
> .num_slave_channels = ARRAY_SIZE(s3c64xx_dma0_info),
> + .slave_map = s3c64xx_dma0_slave_map,
> + .slavecnt = ARRAY_SIZE(s3c64xx_dma0_slave_map),
> };
Here we add a .slavecnt but the pl08x_platform_data structure doesn't
contain that field. I can't see it on the branch you linked in
the cover letter either, is it added by a patch on another branch
I am missing?
> @@ -224,6 +254,8 @@ struct pl08x_platform_data s3c64xx_dma1_plat_data = {
> .put_xfer_signal = pl08x_put_xfer_signal,
> .slave_channels = s3c64xx_dma1_info,
> .num_slave_channels = ARRAY_SIZE(s3c64xx_dma1_info),
> + .slave_map = s3c64xx_dma1_slave_map,
> + .slavecnt = ARRAY_SIZE(s3c64xx_dma1_slave_map),
> };
ditto.
Thanks,
Charles
^ permalink raw reply
* [PATCH] iommu/dma-iommu: properly respect configured address space size
From: Robin Murphy @ 2016-11-08 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8286728f-fab3-8179-5215-e156da426244@samsung.com>
On 08/11/16 13:41, Marek Szyprowski wrote:
> Hi Robin,
>
>
> On 2016-11-08 12:37, Robin Murphy wrote:
>> 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?
>
> Yes, I found this issue while working on next version of ARM & ARM64
> DMA-mapping/IOMMU integration patchset and adapting Exynos drivers for the
> new IOMMU/DMA-mapping glue.
>
> Some Exynos devices (codec and camera isp) operate only on the limited (and
> really small: 256M for example) DMA window. They use non-standard way of
> addressing memory: an offset from the firmware base. However they still
> have
> 32bit DMA mask, as the firmware can be located basically everywhere in the
> real DMA address space, but then they can access only next 256M from that
> firmware base.
OK, that's good to know, thanks. However, I think in this case it sounds
like it's really the DMA mask that's the underlying problem - if those
blocks themselves can only drive 28 address bits, then the struct
devices representing them should have 28-bit DMA masks, and the
"firmware base" of whoever's driving the upper bits modelled with a
dma_pfn_offset. Even if they do have full 32-bit interfaces themselves,
but are constrained to segment-offset addressing internally, I still
think it would be tidier to represent things that way.
At some point in dma-iommu development I did have support for DMA
offsets upstream of the IOMMU, and am happy to reinstate it if there's a
real use case (assuming you can't simply always set the firmware base to
0 when using the IOMMU).
>>> 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.
>
> Right, this dma_32bit_pfn was really misleading at the first glance,
> but then I found that this was something like end_pfn in case of dma-iommu
> code.
Yes, that was my incorrect assumption - at the time I interpreted it as
a de-facto upper limit which was still possible to allocate above in
special circumstances, which turns out to be almost entirely backwards.
I'd rather not bake that into the dma-iommu code any further if we can
avoid it (I'll try throwing an RFC together to clear up what's there
already).
Robin.
>> 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.
>
> The proper support for end_pfn would be a better approach probably,
> especially if we consider readability of the code.
>
> Best regards
^ permalink raw reply
* [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Jean-Francois Moine @ 2016-11-08 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161107223741.gjxj4tqwuxud2iqc@lukather>
On Mon, 7 Nov 2016 23:37:41 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Fri, Oct 28, 2016 at 07:34:20PM +0200, Jean-Francois Moine wrote:
> > On Fri, 28 Oct 2016 00:03:16 +0200
> > Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
[snip]
> > > > > We've been calling them bus and mod.
> > > >
> > > > I can understand "bus" (which is better than "apb"), but why "mod"?
> > >
> > > Allwinner has been calling the clocks that are supposed to generate
> > > the external signals (depending on where you were looking) module or
> > > mod clocks (which is also why we have mod in the clock
> > > compatibles). The module 1 clocks being used for the audio and the
> > > module 0 for the rest (SPI, MMC, NAND, display, etc.)
> >
> > I did not find any 'module' in the H3 documentation.
> > So, is it really a good name?
>
> It's true that they use it less nowadays, but they still do,
> ie. https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/clk/sunxi/clk-sun8iw7.c#L513
There is a 'mod' suffix, but it is used for the bus gates only, not for
the main clocks.
> And we have to remain consistent anyway.
I don't see any consistency in the H3 DT:
- the bus gates are named "ahb" and apb"
- the (main) clocks are named "mmc", "usb0_phy" and "ir"
There is no "bus" nor "mod".
> > > > > > +
> > > > > > +- resets: phandle to the reset of the device
> > > > > > +
> > > > > > +- ports: phandle's to the LCD ports
> > > > >
> > > > > Please use the OF graph.
> > > >
> > > > These ports are references to the graph of nodes. See
> > > > http://www.kernelhub.org/?msg=911825&p=2
> > >
> > > In an OF-graph, your phandle to the LCD controller would be replaced
> > > by an output endpoint.
> >
> > This is the DE controller. There is no endpoint link at this level.
>
> The display engine definitely has an endpoint: the TCON.
Not at all. The video chain is simply
CRTC (TCON) -> connector (HDMI/LCD/DAC/..)
The DE is an ancillary device which handles the planes.
> > The Device Engine just handles the planes of the LCDs, but, indeed,
> > the LCDs must know about the DE and the DE must know about the LCDs.
> > There are 2 ways to realize this knowledge in the DT:
> > 1) either the DE has one or two phandle's to the LCDs,
> > 2) or the LCDs have a phandle to the DE.
> >
> > I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
> > which is part of the video link (OF-graph LCD <-> connector).
> > It would be possible to have phandles to the LCDs themselves, but this
> > asks for more code.
> >
> > The second way is also possible, but it also complexifies a bit the
> > exchanges DE <-> LCD.
>
> I'm still not sure how it would complexify anything, and why you can't
> use the display graph to model the relation between the display engine
> and the TCON (and why you want to use a generic property that refers
> to the of-graph while it really isn't).
Complexification:
1- my solution:
At startup time, the DE device is the DRM device. It has to know the
devices entering in the video chains.
The CRTCs (LCD/TCON) are found by
ports[i] -> parent
The connectors are found by
ports[i] -> endpoint -> remote_endpoint -> parent
2- with ports pointing to the LCDs:
The CRTCs (LCD/TCON) are simply
ports[i]
The connectors are found by
loop on all ports of ports[i]
ports[i][j] -> endpoint -> remote_endpoint -> parent
3- with a phandle to the DE in the LCDs:
The DE cannot be the DRM device because there is no information about
the video devices in the DT. Then, the DRM devices are the LCDs.
These LCDs must give their indices to the DE. So, the DE must implement
some callback function to accept a LCD definition, and there must be
a list of DEs in the driver to make the association DE <-> LCD[i]
Some more problem may be raised if a user wants to have the same frame
buffer on the 2 LCDs of a DE.
Anyway, my solution is already used in the IMX Soc.
See 'display-subsystem' in arch/arm/boot/dts/imx6q.dtsi for an example.
> > > > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > > > +{
> > > > > > + struct priv *priv = drm->dev_private;
> > > > > > + struct lcd *lcd = priv->lcds[crtc];
> > > > > > +
> > > > > > + tcon_write(lcd->mmio, gint0,
> > > > > > + tcon_read(lcd->mmio, gint0) &
> > > > > > + ~TCON_GINT0_TCON1_Vb_Int_En);
> > > > > > +}
> > > > > > +
> > > > > > +/* panel functions */
> > > > >
> > > > > Panel functions? In the CRTC driver?
> > > >
> > > > Yes, dumb panel.
> > >
> > > What do you mean by that? Using a Parallel/RGB interface?
> >
> > Sorry, I though this was a well-known name. The 'dump panel' was used
> > in the documentation of my previous ARM machine as the video frame sent
> > to the HDMI controller. 'video_frame' is OK for you?
>
> If it's the frame sent to the encoder, then it would be the CRTC by
> DRM's nomenclature.
The CRTC is a software entity. The frame buffer is a hardware entity.
> > > > > > +static const struct {
> > > > > > + char chan;
> > > > > > + char layer;
> > > > > > + char pipe;
> > > > > > +} plane2layer[DE2_N_PLANES] = {
> > > > > > + [DE2_PRIMARY_PLANE] = {0, 0, 0},
> > > > > > + [DE2_CURSOR_PLANE] = {1, 0, 1},
> > > > > > + [DE2_VI_PLANE] = {0, 1, 0},
> > > > > > +};
[snip]
> > > > >
> > > > > Comments?
> > > >
> > > > This
> > > > primary plane is channel 0 (VI), layer 0, pipe 0
> > > > cursor plane is channel 1 (UI), layer 0, pipe 1
> > > > overlay plane is channel 0 (VI), layer 1, pipe 0
> > > > or the full explanation:
> > > > Constraints:
> > > > The VI channels can do RGB or YUV, while UI channels can do RGB
> > > > only.
> > > > The LCD 0 has 1 VI channel and 4 UI channels, while
> > > > LCD 1 has only 1 VI channel and 1 UI channel.
> > > > The cursor must go to a channel bigger than the primary channel,
> > > > otherwise it is not transparent.
> > > > First try:
> > > > Letting the primary plane (usually RGB) in the 2nd channel (UI),
> > > > as this is done in the legacy driver, asks for the cursor to go
> > > > to the next channel (UI), but this one does not exist in LCD1.
> > > > Retained layout:
> > > > So, we must use only 2 channels for the same behaviour on LCD0
> > > > (H3) and LCD1 (A83T)
> > > > The retained combination is:
> > > > - primary plane in the first channel (VI),
> > > > - cursor plane inthe 2nd channel (UI), and
> > > > - overlay plane in the 1st channel (VI).
> > > >
> > > > Note that there could be 3 overlay planes (a channel has 4
> > > > layers), but I am not sure that the A83T or the H3 could
> > > > support 3 simultaneous video streams...
> > >
> > > Do you know if the pipe works in the old display engine?
> > >
> > > Especially about the two-steps composition that wouldn't allow you to
> > > have alpha on all the planes?
> > >
> > > If it is similar, I think hardcoding the pipe number is pretty bad,
> > > because that would restrict the combination of planes and formats,
> > > while some other might have worked.
> >
> > From what I understood about the DE2, the pipes just define the priority
> > of the overlay channels (one pipe for one channel).
> > With the cursor constraint, there must be at least 2 channels in
> > order (primary, cursor). Then, with these 2 channels/pipes, there can be
> > 6 so-called overlay planes (3 RGB/YUV and 3 RGB only).
> > Enabling the pipes 2 and 3 (LCD 0 only) would offer 8 more planes, but
> > RGB only. Then, it might be useful to have dynamic pipes.
>
> That's very valuable (and definitely should go into a comment),
> thanks!
>
> I still believe that's it should be into a (simple at first)
> atomic_check. That would be easier to extend and quite easy to
> document and get simply by looking at the code.
Sorry for I don't understand what you mean.
> > > > > > +static int __init de2_drm_init(void)
> > > > > > +{
> > > > > > + int ret;
> > > > > > +
> > > > > > +/* uncomment to activate the drm traces at startup time */
> > > > > > +/* drm_debug = DRM_UT_CORE | DRM_UT_DRIVER | DRM_UT_KMS |
> > > > > > + DRM_UT_PRIME | DRM_UT_ATOMIC; */
> > > > >
> > > > > That's useless.
> > > >
> > > > Right, but it seems that some people don't know how to debug a DRM
> > > > driver. This is only a reminder.
> > > >
> > > > > > + DRM_DEBUG_DRIVER("\n");
> > > > > > +
> > > > > > + ret = platform_driver_register(&de2_lcd_platform_driver);
> > > > > > + if (ret < 0)
> > > > > > + return ret;
> > > > > > +
> > > > > > + ret = platform_driver_register(&de2_drm_platform_driver);
> > > > > > + if (ret < 0)
> > > > > > + platform_driver_unregister(&de2_lcd_platform_driver);
> > > > > > +
> > > > > > + return ret;
> > > > > > +}
> > > > >
> > > > > And that really shouldn't be done that way.
> > > >
> > > > May you explain?
> > >
> > > This goes against the whole idea of the device and driver
> > > model. Drivers should only register themselves, device should be
> > > created by buses (or by using some external components if the bus
> > > can't: DT, ACPI, etc.). If there's a match, you get probed.
> > >
> > > A driver that creates its own device just to probe itself violates
> > > that.
> >
> > In this function (module init), there is no driver yet.
> > The module contains 2 drivers: the DE (planes) and the LCD (CRTC),
> > and there is no macro to handle such modules.
>
> Ah, yes, my bad. I thought you were registering a device and a
> driver. Still this is a very unusual pattern. Why do you need to split
> the two? Can't you just merge them?
The DE and the LCDs are different devices on different drivers.
A DE must be only one device because it has to handle concurent
accesses from its 2 LCDs. Then 2 drivers.
But only one module. Why? Because there cannot be double direction
calls from one module to an other one, and, in our case, for example,
- the DRM (DE) device must call vblank functions which are handled in
the CRTC (TCON) device, and
- the CRTC device must call DE initialization functions at startup time.
> > > > > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > > > > +{
> > > > > > + int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > > > > +
> > > > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_PRIMARY_PLANE],
> > > > > > + DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > > > > + if (ret >= 0)
> > > > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_CURSOR_PLANE],
> > > > > > + DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > > >
> > > > > Nothing looks really special about that cursor plane. Any reasion not
> > > > > to make it an overlay?
> > > >
> > > > As explained above (channel/layer/pipe plane definitions), the cursor
> > > > cannot go in a channel lower or equal to the one of the primary plane.
> > > > Then, it must be known and, so, have an explicit plane.
> > >
> > > If you were to make it a plane, you could use atomic_check to check
> > > this and make sure this doesn't happen. And you would gain a generic
> > > plane that can be used for other purposes if needed.
> >
> > The function drm_crtc_init_with_planes() offers a cursor plane for free.
> > On the other side, having 6 overlay planes is more than the SoCs can
> > support.
>
> It's not really for free, it costs you a generic plane that could
> definitely be used for something else and cannot anymore because
> they've been hardcoded to a cursor.
>
> And having a camera, the VPU or even an application directly output
> directly into one of these planes seems a much better use of a generic
> plane than a cursor.
Looking at the harder case (A83T), there may be 8 planes on 2 channels.
Using a primary plane and the cursor,
8 planes - primary plane - cursor plane = 6 planes
6 planes are available.
If I count correctly, in your example:
one camera + one VPU + one application = 3 planes
3 planes are used.
So, 3 planes are still available.
On the other side, removing the cursor would just let one more plane.
Do we really need this one? In other words, I'd be pleased to know how
you run 7 applications doing video overlay.
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Sudeep Holla @ 2016-11-08 14:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478148731-11712-7-git-send-email-sudeep.holla@arm.com>
Hi Rob,
On 03/11/16 04:52, Sudeep Holla wrote:
> This patch adds specific compatible to support legacy SCPI protocol.
>
Sorry for messing it up before, I think this version is much better.
Only this patch introduces new compatible, while 5,7,8/8 are just
reorganization to move the platform specific stuff out of the generic
SCPI bindings. It would be good if you can have a quick look and
provide ack if you are happy with these patches.
--
Regards,
Sudeep
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> Documentation/devicetree/bindings/arm/arm,scpi.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> index d1882c4540d0..ebd03fc93135 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -7,7 +7,9 @@ by Linux to initiate various system control and power operations.
>
> Required properties:
>
> -- compatible : should be "arm,scpi"
> +- compatible : should be
> + * "arm,scpi" : For implementations complying to SCPI v1.0 or above
> + * "arm,legacy-scpi" : For implementations complying pre SCPI v1.0
> - mboxes: List of phandle and mailbox channel specifiers
> All the channels reserved by remote SCP firmware for use by
> SCPI message protocol should be specified in any order
>
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe
From: Auger Eric @ 2016-11-08 14:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108024559.GA20591@arm.com>
Hi Will,
On 08/11/2016 03:45, Will Deacon wrote:
> Hi all,
>
> I figured this was a reasonable post to piggy-back on for the LPC minutes
> relating to guest MSIs on arm64.
>
> On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
>> We can always have QEMU reject hot-adding the device if the reserved
>> region overlaps existing guest RAM, but I don't even really see how we
>> advise users to give them a reasonable chance of avoiding that
>> possibility. Apparently there are also ARM platforms where MSI pages
>> cannot be remapped to support the previous programmable user/VM
>> address, is it even worthwhile to support those platforms? Does that
>> decision influence whether user programmable MSI reserved regions are
>> really a second class citizen to fixed reserved regions? I expect
>> we'll be talking about this tomorrow morning, but I certainly haven't
>> come up with any viable solutions to this. Thanks,
>
> At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
> microconference. I presented some slides to illustrate some of the issues
> we're trying to solve:
>
> http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
>
> Punit took some notes (thanks!) on the etherpad here:
>
> https://etherpad.openstack.org/p/LPC2016_PCI
Thanks to both of you for the minutes and slides. Unfortunately I could
not travel but my ears were burning ;-)
>
> although the discussion was pretty lively and jumped about, so I've had
> to go from memory where the notes didn't capture everything that was
> said.
>
> To summarise, arm64 platforms differ in their handling of MSIs when compared
> to x86:
>
> 1. The physical memory map is not standardised (Jon pointed out that
> this is something that was realised late on)
> 2. MSIs are usually treated the same as DMA writes, in that they must be
> mapped by the SMMU page tables so that they target a physical MSI
> doorbell
> 3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
> doorbell built into the PCI RC)
> 4. Platforms typically have some set of addresses that abort before
> reaching the SMMU (e.g. because the PCI identifies them as P2P).
>
> All of this means that userspace (QEMU) needs to identify the memory
> regions corresponding to points (3) and (4) and ensure that they are
> not allocated in the guest physical (IPA) space. For platforms that can
> remap the MSI doorbell as in (2), then some space also needs to be
> allocated for that.
>
> Rather than treat these as separate problems, a better interface is to
> tell userspace about a set of reserved regions, and have this include
> the MSI doorbell, irrespective of whether or not it can be remapped.
> Don suggested that we statically pick an address for the doorbell in a
> similar way to x86, and have the kernel map it there. We could even pick
> 0xfee00000. If it conflicts with a reserved region on the platform (due
> to (4)), then we'd obviously have to (deterministically?) allocate it
> somewhere else, but probably within the bottom 4G.
This is tentatively achieved now with
[1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
(http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1264506.html)
>
> The next question is how to tell userspace about all of the reserved
> regions. Initially, the idea was to extend VFIO, however Alex pointed
> out a horrible scenario:
>
> 1. QEMU spawns a VM on system 0
> 2. VM is migrated to system 1
> 3. QEMU attempts to passthrough a device using PCI hotplug
>
> In this scenario, the guest memory map is chosen at step (1), yet there
> is no VFIO fd available to determine the reserved regions. Furthermore,
> the reserved regions may vary between system 0 and system 1. This pretty
> much rules out using VFIO to determine the reserved regions.Alex suggested
> that the SMMU driver can advertise the regions via /sys/class/iommu/. This
> would solve part of the problem, but migration between systems with
> different memory maps can still cause problems if the reserved regions
> of the new system conflict with the guest memory map chosen by QEMU.
OK so I understand we do not want anymore the VFIO chain capability API
(patch 5 of above series) but we prefer a sysfs approach instead.
I understand the sysfs approach which allows the userspace to get the
info earlier and independently on VFIO. Keeping in mind current QEMU
virt - which is not the only userspace - will not do much from this info
until we bring upheavals in virt address space management. So if I am
not wrong, at the moment the main action to be undertaken is the
rejection of the PCI hotplug in case we detect a collision.
I can respin [1]
- studying and taking into account Robin's comments about dm_regions
similarities
- removing the VFIO capability chain and replacing this by a sysfs API
Would that be OK?
What about Alex comments who wanted to report the usable memory ranges
instead of unusable memory ranges?
Also did you have a chance to discuss the following items:
1) the VFIO irq safety assessment
2) the MSI reserved size computation (is an arbitrary size OK?)
Thanks
Eric
> Jon pointed out that most people are pretty conservative about hardware
> choices when migrating between them -- that is, they may only migrate
> between different revisions of the same SoC, or they know ahead of time
> all of the memory maps they want to support and this could be communicated
> by way of configuration to libvirt. It would be up to QEMU to fail the
> hotplug if it detected a conflict. Alex asked if there was a security
> issue with DMA bypassing the SMMU, but there aren't currently any systems
> where that is known to happen. Such a system would surely not be safe for
> passthrough.
>
> Ben mused that a way to handle conflicts dynamically might be to hotplug
> on the entire host bridge in the guest, passing firmware tables describing
> the new reserved regions as a property of the host bridge. Whilst this
> may well solve the issue, it was largely considered future work due to
> its invasive nature and dependency on firmware tables (and guest support)
> that do not currently exist.
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v2 0/2] arm64: fix the bugs found in the hugetlb test
From: Catalin Marinas @ 2016-11-08 14:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478583879-14654-1-git-send-email-shijie.huang@arm.com>
On Tue, Nov 08, 2016 at 01:44:37PM +0800, Huang Shijie wrote:
> (3) The test result in the Softiron and Juno-r1 boards:
>
> This detail test result shows below (both the "make func" & "make stress"):
>
> 4KB granule:
>
> 1.1) PTE + Contiguous bit : 4K x 16 = 64K (per huge page size)
> Test result : PASS
>
> 1.2) PMD : 2M x 1 = 2M (per huge page size)
> Test result : PASS
>
> 1.3) PMD + Contiguous bit : 2M x 16 = 32M (per huge page size)
> Test result : PASS
>
> 64KB granule:
>
> 3.1) PTE + Contiguous bit : 64K x 32 = 2M (per huge page size)
> Test result : PASS
>
> 3.2) PMD + Contiguous bit : 512M x 32 = 16G (per huge page size)
> Test result : no hardware to support this test
Don't we have support for single (non-contiguous) PMD huge page with 64K
pages (512M per huge page)? I gave it a try and it seems to work (though
without your patches applied ;)).
> Huang Shijie (2):
> arm64: hugetlb: remove the wrong pmd check in find_num_contig()
> arm64: hugetlb: fix the wrong address for several functions
For these patches:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
I'm not sure whether Will plans to push them into 4.9. AFAICT, the
contiguous huge pages never worked properly, so we may not count it as a
regression but a new feature. If Will doesn't take them, I'll queue the
patches for 4.10.
BTW, I think we also need to fix this (no functional change though):
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 2e49bd252fe7..0a4c97b618ec 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -323,7 +323,7 @@ __setup("hugepagesz=", setup_hugepagesz);
static __init int add_default_hugepagesz(void)
{
if (size_to_hstate(CONT_PTES * PAGE_SIZE) == NULL)
- hugetlb_add_hstate(CONT_PMD_SHIFT);
+ hugetlb_add_hstate(CONT_PTE_SHIFT);
return 0;
}
arch_initcall(add_default_hugepagesz);
--
Catalin
^ permalink raw reply related
* [PATCH v3 2/2] arm64: Support systems without FP/ASIMD
From: Suzuki K Poulose @ 2016-11-08 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478613381-5718-1-git-send-email-suzuki.poulose@arm.com>
The arm64 kernel assumes that FP/ASIMD units are always present
and accesses the FP/ASIMD specific registers unconditionally. This
could cause problems when they are absent. This patch adds the
support for kernel handling systems without FP/ASIMD by skipping the
register access within the kernel. For kvm, we trap the accesses
to FP/ASIMD and inject an undefined instruction exception to the VM.
The callers of the exported kernel_neon_begin_partial() should
make sure that the FP/ASIMD is supported.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/include/asm/cpufeature.h | 5 +++++
arch/arm64/include/asm/neon.h | 3 ++-
arch/arm64/kernel/cpufeature.c | 15 +++++++++++++++
arch/arm64/kernel/fpsimd.c | 14 ++++++++++++++
arch/arm64/kvm/handle_exit.c | 11 +++++++++++
arch/arm64/kvm/hyp/hyp-entry.S | 9 ++++++++-
arch/arm64/kvm/hyp/switch.c | 5 ++++-
8 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 87b4465..4174f09 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -34,7 +34,8 @@
#define ARM64_HAS_32BIT_EL0 13
#define ARM64_HYP_OFFSET_LOW 14
#define ARM64_MISMATCHED_CACHE_LINE_SIZE 15
+#define ARM64_HAS_NO_FPSIMD 16
-#define ARM64_NCAPS 16
+#define ARM64_NCAPS 17
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 9890d20..ce45770 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -213,6 +213,11 @@ static inline bool system_supports_mixed_endian_el0(void)
return id_aa64mmfr0_mixed_endian_el0(read_system_reg(SYS_ID_AA64MMFR0_EL1));
}
+static inline bool system_supports_fpsimd(void)
+{
+ return !cpus_have_const_cap(ARM64_HAS_NO_FPSIMD);
+}
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm64/include/asm/neon.h b/arch/arm64/include/asm/neon.h
index 13ce4cc..ad4cdc9 100644
--- a/arch/arm64/include/asm/neon.h
+++ b/arch/arm64/include/asm/neon.h
@@ -9,8 +9,9 @@
*/
#include <linux/types.h>
+#include <asm/fpsimd.h>
-#define cpu_has_neon() (1)
+#define cpu_has_neon() system_supports_fpsimd()
#define kernel_neon_begin() kernel_neon_begin_partial(32)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index fc2bd19..f89385d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -746,6 +746,14 @@ static bool hyp_offset_low(const struct arm64_cpu_capabilities *entry,
return idmap_addr > GENMASK(VA_BITS - 2, 0) && !is_kernel_in_hyp_mode();
}
+static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int __unused)
+{
+ u64 pfr0 = read_system_reg(SYS_ID_AA64PFR0_EL1);
+
+ return cpuid_feature_extract_signed_field(pfr0,
+ ID_AA64PFR0_FP_SHIFT) < 0;
+}
+
static const struct arm64_cpu_capabilities arm64_features[] = {
{
.desc = "GIC system register CPU interface",
@@ -829,6 +837,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.def_scope = SCOPE_SYSTEM,
.matches = hyp_offset_low,
},
+ {
+ /* FP/SIMD is not implemented */
+ .capability = ARM64_HAS_NO_FPSIMD,
+ .def_scope = SCOPE_SYSTEM,
+ .min_field_value = 0,
+ .matches = has_no_fpsimd,
+ },
{},
};
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 394c61d..b883f1f 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -127,6 +127,8 @@ void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
void fpsimd_thread_switch(struct task_struct *next)
{
+ if (!system_supports_fpsimd())
+ return;
/*
* Save the current FPSIMD state to memory, but only if whatever is in
* the registers is in fact the most recent userland FPSIMD state of
@@ -157,6 +159,8 @@ void fpsimd_thread_switch(struct task_struct *next)
void fpsimd_flush_thread(void)
{
+ if (!system_supports_fpsimd())
+ return;
memset(¤t->thread.fpsimd_state, 0, sizeof(struct fpsimd_state));
fpsimd_flush_task_state(current);
set_thread_flag(TIF_FOREIGN_FPSTATE);
@@ -168,6 +172,8 @@ void fpsimd_flush_thread(void)
*/
void fpsimd_preserve_current_state(void)
{
+ if (!system_supports_fpsimd())
+ return;
preempt_disable();
if (!test_thread_flag(TIF_FOREIGN_FPSTATE))
fpsimd_save_state(¤t->thread.fpsimd_state);
@@ -181,6 +187,8 @@ void fpsimd_preserve_current_state(void)
*/
void fpsimd_restore_current_state(void)
{
+ if (!system_supports_fpsimd())
+ return;
preempt_disable();
if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
struct fpsimd_state *st = ¤t->thread.fpsimd_state;
@@ -199,6 +207,8 @@ void fpsimd_restore_current_state(void)
*/
void fpsimd_update_current_state(struct fpsimd_state *state)
{
+ if (!system_supports_fpsimd())
+ return;
preempt_disable();
fpsimd_load_state(state);
if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
@@ -228,6 +238,8 @@ static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
*/
void kernel_neon_begin_partial(u32 num_regs)
{
+ if (WARN_ON(!system_supports_fpsimd()))
+ return;
if (in_interrupt()) {
struct fpsimd_partial_state *s = this_cpu_ptr(
in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
@@ -252,6 +264,8 @@ EXPORT_SYMBOL(kernel_neon_begin_partial);
void kernel_neon_end(void)
{
+ if (!system_supports_fpsimd())
+ return;
if (in_interrupt()) {
struct fpsimd_partial_state *s = this_cpu_ptr(
in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index a204adf..1bfe30d 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -57,6 +57,16 @@ static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
return 1;
}
+/*
+ * Guest access to FP/ASIMD registers are routed to this handler only
+ * when the system doesn't support FP/ASIMD.
+ */
+static int handle_no_fpsimd(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ kvm_inject_undefined(vcpu);
+ return 1;
+}
+
/**
* kvm_handle_wfx - handle a wait-for-interrupts or wait-for-event
* instruction executed by a guest
@@ -144,6 +154,7 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug,
[ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
[ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
+ [ESR_ELx_EC_FP_ASIMD] = handle_no_fpsimd,
};
static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index 4e92399..5e9052f 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -106,9 +106,16 @@ el1_trap:
* x0: ESR_EC
*/
- /* Guest accessed VFP/SIMD registers, save host, restore Guest */
+ /*
+ * We trap the first access to the FP/SIMD to save the host context
+ * and restore the guest context lazily.
+ * If FP/SIMD is not implemented, handle the trap and inject an
+ * undefined instruction exception to the guest.
+ */
+alternative_if_not ARM64_HAS_NO_FPSIMD
cmp x0, #ESR_ELx_EC_FP_ASIMD
b.eq __fpsimd_guest_restore
+alternative_else_nop_endif
mrs x1, tpidr_el2
mov x0, #ARM_EXCEPTION_TRAP
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 83037cd..8bcae7b 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -21,6 +21,7 @@
#include <asm/kvm_asm.h>
#include <asm/kvm_emulate.h>
#include <asm/kvm_hyp.h>
+#include <asm/fpsimd.h>
static bool __hyp_text __fpsimd_enabled_nvhe(void)
{
@@ -76,9 +77,11 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
* traps are only taken to EL2 if the operation would not otherwise
* trap to EL1. Therefore, always make sure that for 32-bit guests,
* we set FPEXC.EN to prevent traps to EL1, when setting the TFP bit.
+ * If FP/ASIMD is not implemented, FPEXC is UNDEFINED and any access to
+ * it will cause an exception.
*/
val = vcpu->arch.hcr_el2;
- if (!(val & HCR_RW)) {
+ if (!(val & HCR_RW) && system_supports_fpsimd()) {
write_sysreg(1 << 30, fpexc32_el2);
isb();
}
--
2.7.4
^ permalink raw reply related
* [PATCH v3 1/2] arm64: Add hypervisor safe helper for checking constant capabilities
From: Suzuki K Poulose @ 2016-11-08 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478613381-5718-1-git-send-email-suzuki.poulose@arm.com>
The hypervisor may not have full access to the kernel data structures
and hence cannot safely use cpus_have_cap() helper for checking the
system capability. Add a safe helper for hypervisors to check a constant
system capability, which *doesn't* fall back to checking the bitmap
maintained by the kernel. With this, make the cpus_have_cap() only
check the bitmask and force constant cap checks to use the new API
for quicker checks.
Cc: Robert Ritcher <rritcher@cavium.com>
Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 19 ++++++++++++-------
arch/arm64/kernel/cpufeature.c | 2 +-
arch/arm64/kernel/process.c | 2 +-
drivers/irqchip/irq-gic-v3.c | 13 +------------
4 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 0bc0b1d..9890d20 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -9,8 +9,6 @@
#ifndef __ASM_CPUFEATURE_H
#define __ASM_CPUFEATURE_H
-#include <linux/jump_label.h>
-
#include <asm/cpucaps.h>
#include <asm/hwcap.h>
#include <asm/sysreg.h>
@@ -27,6 +25,8 @@
#ifndef __ASSEMBLY__
+#include <linux/bug.h>
+#include <linux/jump_label.h>
#include <linux/kernel.h>
/* CPU feature register tracking */
@@ -104,14 +104,19 @@ static inline bool cpu_have_feature(unsigned int num)
return elf_hwcap & (1UL << num);
}
+/* System capability check for constant caps */
+static inline bool cpus_have_const_cap(int num)
+{
+ if (num >= ARM64_NCAPS)
+ return false;
+ return static_branch_unlikely(&cpu_hwcap_keys[num]);
+}
+
static inline bool cpus_have_cap(unsigned int num)
{
if (num >= ARM64_NCAPS)
return false;
- if (__builtin_constant_p(num))
- return static_branch_unlikely(&cpu_hwcap_keys[num]);
- else
- return test_bit(num, cpu_hwcaps);
+ return test_bit(num, cpu_hwcaps);
}
static inline void cpus_set_cap(unsigned int num)
@@ -200,7 +205,7 @@ static inline bool cpu_supports_mixed_endian_el0(void)
static inline bool system_supports_32bit_el0(void)
{
- return cpus_have_cap(ARM64_HAS_32BIT_EL0);
+ return cpus_have_const_cap(ARM64_HAS_32BIT_EL0);
}
static inline bool system_supports_mixed_endian_el0(void)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index c02504e..fc2bd19 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1102,5 +1102,5 @@ void __init setup_cpu_features(void)
static bool __maybe_unused
cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
{
- return (cpus_have_cap(ARM64_HAS_PAN) && !cpus_have_cap(ARM64_HAS_UAO));
+ return (cpus_have_const_cap(ARM64_HAS_PAN) && !cpus_have_const_cap(ARM64_HAS_UAO));
}
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 01753cd..18354f3 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -282,7 +282,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
memset(childregs, 0, sizeof(struct pt_regs));
childregs->pstate = PSR_MODE_EL1h;
if (IS_ENABLED(CONFIG_ARM64_UAO) &&
- cpus_have_cap(ARM64_HAS_UAO))
+ cpus_have_const_cap(ARM64_HAS_UAO))
childregs->pstate |= PSR_UAO_BIT;
p->thread.cpu_context.x19 = stack_start;
p->thread.cpu_context.x20 = stk_sz;
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 19d642e..26e1d7f 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -120,11 +120,10 @@ static void gic_redist_wait_for_rwp(void)
}
#ifdef CONFIG_ARM64
-static DEFINE_STATIC_KEY_FALSE(is_cavium_thunderx);
static u64 __maybe_unused gic_read_iar(void)
{
- if (static_branch_unlikely(&is_cavium_thunderx))
+ if (cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_23154))
return gic_read_iar_cavium_thunderx();
else
return gic_read_iar_common();
@@ -905,14 +904,6 @@ static const struct irq_domain_ops partition_domain_ops = {
.select = gic_irq_domain_select,
};
-static void gicv3_enable_quirks(void)
-{
-#ifdef CONFIG_ARM64
- if (cpus_have_cap(ARM64_WORKAROUND_CAVIUM_23154))
- static_branch_enable(&is_cavium_thunderx);
-#endif
-}
-
static int __init gic_init_bases(void __iomem *dist_base,
struct redist_region *rdist_regs,
u32 nr_redist_regions,
@@ -935,8 +926,6 @@ static int __init gic_init_bases(void __iomem *dist_base,
gic_data.nr_redist_regions = nr_redist_regions;
gic_data.redist_stride = redist_stride;
- gicv3_enable_quirks();
-
/*
* Find out how many interrupts are supported.
* The GIC only supports up to 1020 interrupt sources (SGI+PPI+SPI)
--
2.7.4
^ permalink raw reply related
* [PATCH v3 0/2] arm64: Support systems without FP/ASIMD
From: Suzuki K Poulose @ 2016-11-08 13:56 UTC (permalink / raw)
To: linux-arm-kernel
This series adds supports to the kernel and KVM hyp to handle
systems without FP/ASIMD properly. At the moment the kernel
doesn't check if the FP unit is available before accessing
the registers (e.g during context switch). Also for KVM,
we trap the FP/ASIMD accesses and handle it by injecting an
undefined instruction into the VM on systems without FP.
Tested on a FVP_Base-AEM-v8A model by disabling VFP on at
least one CPU ( -C clusterX.cpuY.vfp-present=0 ).
Changes since V2:
- Dropped cleanup patch for arm64/crypto/aes-ce-ccm-glue.c
- Removed static_key check from cpus_have_cap. All users with
constant caps should use the new API to make use of static_keys.
- Removed a dedicated static_key used in irqchip-gic-v3.c for
Cavium errata with the new API.
Applies on v4.9-rc4 + [1] (which is pushed for rc5)
[1] http://marc.info/?l=linux-arm-kernel&m=147819889813214&w=2
Suzuki K Poulose (2):
arm64: Add hypervisor safe helper for checking constant capabilities
arm64: Support systems without FP/ASIMD
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/include/asm/cpufeature.h | 24 +++++++++++++++++-------
arch/arm64/include/asm/neon.h | 3 ++-
arch/arm64/kernel/cpufeature.c | 17 ++++++++++++++++-
arch/arm64/kernel/fpsimd.c | 14 ++++++++++++++
arch/arm64/kernel/process.c | 2 +-
arch/arm64/kvm/handle_exit.c | 11 +++++++++++
arch/arm64/kvm/hyp/hyp-entry.S | 9 ++++++++-
arch/arm64/kvm/hyp/switch.c | 5 ++++-
drivers/irqchip/irq-gic-v3.c | 13 +------------
10 files changed, 76 insertions(+), 25 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: John Garry @ 2016-11-08 13:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2030692.HPgjBCTYG6@wuerfel>
On 08/11/2016 11:45, Arnd Bergmann wrote:
> 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
Hi Arnd,
Thanks for the reference.
I think the i2c interface doesn't fully satisfy our requirements as we
need more than just a slave bus address when accessing the slave device
(which I think is what i2c uses). We also need to pass "offset" and
"mod_mask" arguments to the djtag adapter to access specific registers
in the slave device.
Cheers,
John
>
> Arnd
>
> .
>
^ permalink raw reply
* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: Anurup M @ 2016-11-08 13:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <22120038.dbaYpsQoUH@wuerfel>
On Tuesday 08 November 2016 05:13 PM, Arnd Bergmann wrote:
> 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.
There is a difference in djtag version of CPU and IO die (in some chips).
For ex: in hip06 djtag for CPU is v1 and for IO is v2.
so they use different readwrite handlers djtag_readwrite_(v1/2).
+ /* 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 the same djtag version, there is no difference in handling in the
driver.
Thanks,
Anurup
> Arnd
>
^ permalink raw reply
* [PATCH] iommu/dma-iommu: properly respect configured address space size
From: Marek Szyprowski @ 2016-11-08 13:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <68e7a18b-739e-b73e-eacf-3cb6c1bd279a@arm.com>
Hi Robin,
On 2016-11-08 12:37, Robin Murphy wrote:
> 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?
Yes, I found this issue while working on next version of ARM & ARM64
DMA-mapping/IOMMU integration patchset and adapting Exynos drivers for the
new IOMMU/DMA-mapping glue.
Some Exynos devices (codec and camera isp) operate only on the limited (and
really small: 256M for example) DMA window. They use non-standard way of
addressing memory: an offset from the firmware base. However they still have
32bit DMA mask, as the firmware can be located basically everywhere in the
real DMA address space, but then they can access only next 256M from that
firmware base.
>
>> 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.
Right, this dma_32bit_pfn was really misleading at the first glance,
but then I found that this was something like end_pfn in case of dma-iommu
code.
> 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.
The proper support for end_pfn would be a better approach probably,
especially if we consider readability of the code.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [alsa-devel] [PATCH 2/9] ALSA: ac97: add an ac97 bus
From: Lars-Peter Clausen @ 2016-11-08 13:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477510907-23495-3-git-send-email-robert.jarzmik@free.fr>
On 10/26/2016 09:41 PM, Robert Jarzmik wrote:
> AC97 is a bus for sound usage. It enables for a AC97 AC-Link to link one
> controller to 0 to 4 AC97 codecs.
>
> The goal of this new implementation is to implement a device/driver
> model for AC97, with an automatic scan of the bus and automatic
> discovery of AC97 codec devices.
>
Good work, a couple of comments inline.
[...]
> diff --git a/include/sound/ac97/codec.h b/include/sound/ac97/codec.h
> new file mode 100644
> index 000000000000..8901c1200522
> --- /dev/null
> +++ b/include/sound/ac97/codec.h
> @@ -0,0 +1,115 @@
> +/*
> + * Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __SOUND_AC97_CODEC2_H
> +#define __SOUND_AC97_CODEC2_H
> +
> +#include <linux/device.h>
> +
> +#define AC97_ID(vendor_id1, vendor_id2) \
> + ((((vendor_id1) & 0xffff) << 16) | ((vendor_id2) & 0xffff))
> +#define AC97_DRIVER_ID(vendor_id1, vendor_id2, mask_id1, mask_id2, _data) \
> + { .id = (((vendor_id1) & 0xffff) << 16) | ((vendor_id2) & 0xffff), \
> + .mask = (((mask_id1) & 0xffff) << 16) | ((mask_id2) & 0xffff), \
> + .data = (_data) }
> +
> +#define to_ac97_device(d) container_of(d, struct ac97_codec_device, dev)
> +#define to_ac97_driver(d) container_of(d, struct ac97_codec_driver, driver)
In my opinion these should be inline functions rather than macros as that
generates much more legible compiler errors e.g. in case there is a type
mismatch.
[...]
> +struct ac97_codec_driver {
> + struct device_driver driver;
> + int (*probe)(struct ac97_codec_device *);
> + int (*remove)(struct ac97_codec_device *);
> + int (*suspend)(struct ac97_codec_device *);
> + int (*resume)(struct ac97_codec_device *);
> + void (*shutdown)(struct ac97_codec_device *);
The suspend, resume and shutdown callbacks are never used. Which is good,
since all new frameworks should use dev_pm_ops. Just drop the from the struct.
> + const struct ac97_id *id_table;
> +};
[...]
> diff --git a/include/sound/ac97/controller.h b/include/sound/ac97/controller.h
> new file mode 100644
> index 000000000000..5ff59bd7e324
> --- /dev/null
> +++ b/include/sound/ac97/controller.h
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef AC97_CONTROLLER_H
> +#define AC97_CONTROLLER_H
> +
> +#include <linux/list.h>
> +
> +#define AC97_BUS_MAX_CODECS 4
> +#define AC97_SLOTS_AVAILABLE_ALL 0xf
> +
> +struct device;
> +
> +/**
> + * struct ac97_controller - The AC97 controller of the AC-Link
> + * @ops: the AC97 operations.
> + * @controllers: linked list of all existing controllers.
> + * @dev: the device providing the AC97 controller.
> + * @slots_available: the mask of accessible/scanable codecs.
> + * @codecs: the 4 possible AC97 codecs (NULL if none found).
> + * @codecs_pdata: platform_data for each codec (NULL if no pdata).
> + *
> + * This structure is internal to AC97 bus, and should not be used by the
> + * controllers themselves, excepting for using @dev.
> + */
> +struct ac97_controller {
> + const struct ac97_controller_ops *ops;
> + struct list_head controllers;
> + struct device *dev;
I'd make the controller itself a struct dev, rather than just having the
pointer to the parent. This is more idiomatic and matches what other
subsystems do. It has several advantages, you get proper refcounting of your
controller structure, the controller gets its own sysfs directory where the
CODECs appear as children, you don't need the manual sysfs attribute
creation and removal in ac97_controler_{un,}register().
> + unsigned short slots_available;
> + struct ac97_codec_device *codecs[AC97_BUS_MAX_CODECS];
If you make the controller a struct dev you can also remove this, since the
device driver core tracks the children of a device.
> + void *codecs_pdata[AC97_BUS_MAX_CODECS];
> +};
> +
> +/**
> + * struct ac97_controller_ops - The AC97 operations
> + * @reset: Cold reset of the AC97 AC-Link.
> + * @warm_reset: Warm reset of the AC97 AC-Link.
> + * @read: Read of a single AC97 register.
> + * Returns the register value or a negative error code.
> + * @write: Write of a single AC97 register.
> + * @wait: Wait for the current AC97 operation to finish (might be NULL).
> + * @init: Initialization of the AC97 AC-Link (might be NULL).
> + *
> + * These are the basic operation an AC97 controller must provide for an AC97
> + * access functions. Amongst these, all but the last 2 are mandatory.
> + * The slot number is also known as the AC97 codec number, between 0 and 3.
> + */
> +struct ac97_controller_ops {
> + void (*reset)(struct ac97_controller *adrv);
> + void (*warm_reset)(struct ac97_controller *adrv);
> + int (*write)(struct ac97_controller *adrv, int slot,
> + unsigned short reg, unsigned short val);
> + int (*read)(struct ac97_controller *adrv, int slot, unsigned short reg);
> + void (*wait)(struct ac97_controller *adrv, int slot);
> + void (*init)(struct ac97_controller *adrv, int slot);
Neither wait nor init are ever used.
> +};
> +
[...]
> +/*
> + * Protects ac97_controllers and each ac97_controller structure.
> + */
> +static DEFINE_MUTEX(ac97_controllers_mutex);
> +static LIST_HEAD(ac97_controllers);
> +
> +static struct bus_type ac97_bus_type;
> +
> +static struct ac97_codec_device *
> +ac97_codec_find(struct ac97_controller *ac97_ctrl, int codec_num)
unsigned int codec_num
> +{
> + if ((codec_num < 0) || (codec_num >= AC97_BUS_MAX_CODECS))
> + return ERR_PTR(-ERANGE);
I'd make this EINVAL.
> +
> + return ac97_ctrl->codecs[codec_num];
> +}
> +
> +static void ac97_codec_release(struct device *dev)
> +{
> + struct ac97_codec_device *adev;
> + struct ac97_controller *ac97_ctrl;
> +
> + adev = container_of(dev, struct ac97_codec_device, dev);
to_ac97_device()
> + ac97_ctrl = adev->ac97_ctrl;
> + ac97_ctrl->codecs[adev->num] = NULL;
> + sysfs_remove_link(&dev->kobj, "ac97_controller");
> + kfree(adev);
> +}
> +
> +static int ac97_codec_add(struct ac97_controller *ac97_ctrl, int idx,
> + unsigned int vendor_id)
> +{
> + struct ac97_codec_device *codec;
> + char *codec_name;
> + int ret;
> +
> + codec = kzalloc(sizeof(*codec), GFP_KERNEL);
> + if (!codec)
> + return -ENOMEM;
> + ac97_ctrl->codecs[idx] = codec;
> + codec->vendor_id = vendor_id;
> + codec->dev.release = ac97_codec_release;
> + codec->dev.bus = &ac97_bus_type;
> + codec->dev.parent = ac97_ctrl->dev;
> + codec->num = idx;
> + codec->ac97_ctrl = ac97_ctrl;
> +
> + codec_name = kasprintf(GFP_KERNEL, "%s:%d", dev_name(ac97_ctrl->dev),
> + idx);
> + codec->dev.init_name = codec_name;
init_name is only for statically allocated devices. Use dev_set_name(dev,
...). No need for kasprintf() either as dev_set_name() takes a format string.
For this you need to split device_register into device_initialize() and
device_add(). But usually that is what you want anyway.
> +
> + ret = device_register(&codec->dev);
> + kfree(codec_name);
> + if (ret)
> + goto err_free_codec;
> +
> + ret = sysfs_create_link(&codec->dev.kobj, &ac97_ctrl->dev->kobj,
> + "ac97_controller");
Since the CODEC is a child of the controller this should not be necessary as
this just points one directory up. It's like `ln -s .. parent`
> + if (ret)
> + goto err_unregister_device;
> +
> + return 0;
> +err_unregister_device:
> + put_device(&codec->dev);
> +err_free_codec:
> + kfree(codec);
Since the struct is reference counted, the freeing is done in the release
callback and this leads to a double free.
> + ac97_ctrl->codecs[idx] = NULL;
> +
> + return ret;
> +}
[...]
> +/**
> + * snd_ac97_codec_driver_register - register an AC97 codec driver
> + * @dev: AC97 driver codec to register
> + *
> + * Register an AC97 codec driver to the ac97 bus driver, aka. the AC97 digital
> + * controller.
> + *
> + * Returns 0 on success or error code
> + */
> +int snd_ac97_codec_driver_register(struct ac97_codec_driver *drv)
> +{
> + int ret;
> +
> + drv->driver.bus = &ac97_bus_type;
> +
> + ret = driver_register(&drv->driver);
> + if (!ret)
> + ac97_rescan_all_controllers();
Rescanning the bus when a new codec driver is registered should not be
neccessary. The bus is scanned once when the controller is registered, this
creates the device. The device driver core will take care of binding the
device to the driver, if the driver is registered after thed evice.
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(snd_ac97_codec_driver_register);
> +
[...]
> +static int ac97_ctrl_codecs_unregister(struct ac97_controller *ac97_ctrl)
> +{
> + int i;
> +
> + for (i = 0; i < AC97_BUS_MAX_CODECS; i++)
> + if (ac97_ctrl->codecs[i])
> + put_device(&ac97_ctrl->codecs[i]->dev);
This should be device_unregister() to match the device_register() in
ac97_codec_add().
> +
> + return 0;
> +}
> +
> +static ssize_t cold_reset_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t len)
> +{
> + struct ac97_controller *ac97_ctrl = ac97_ctrl_find(dev);
> +
> + if (!dev)
> + return -ENODEV;
dev is never NULL here. And for the ac97_ctrl there is a race condition. It
could be unregistered and freed after ac97_ctrl_find() returned sucessfully,
but before ac97_ctrl->ops is used.
> +
> + ac97_ctrl->ops->reset(ac97_ctrl);
> + return len;
> +}
> +static DEVICE_ATTR_WO(cold_reset);
> +
> +static ssize_t warm_reset_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t len)
> +{
> + struct ac97_controller *ac97_ctrl = ac97_ctrl_find(dev);
> +
> + if (!dev)
> + return -ENODEV;
Same here.
> +
> + ac97_ctrl->ops->warm_reset(ac97_ctrl);
> + return len;
> +}
> +static DEVICE_ATTR_WO(warm_reset);
> +
> +static struct attribute *ac97_controller_device_attrs[] = {
> + &dev_attr_cold_reset.attr,
> + &dev_attr_warm_reset.attr,
> + NULL
> +};
This adds new userspace ABI that is not documented at the moment.
> +
> +static const struct attribute_group ac97_controller_attr_group = {
> + .name = "ac97_operations",
> + .attrs = ac97_controller_device_attrs,
> +};
> +
> +/**
> + * snd_ac97_controller_register - register an ac97 controller
> + * @ops: the ac97 bus operations
> + * @dev: the device providing the ac97 DC function
> + * @slots_available: mask of the ac97 codecs that can be scanned and probed
> + * bit0 => codec 0, bit1 => codec 1 ... bit 3 => codec 3
> + *
> + * Register a digital controller which can control up to 4 ac97 codecs. This is
> + * the controller side of the AC97 AC-link, while the slave side are the codecs.
> + *
> + * Returns 0 upon success, negative value upon error
> + */
> +int snd_ac97_controller_register(const struct ac97_controller_ops *ops,
> + struct device *dev,
> + unsigned short slots_available,
> + void **codecs_pdata)
In my opinion this should return a handle to a ac97 controller which can
then be passed to snd_ac97_controller_unregister(). This is in my opinion
the better approach rather than looking up the controller by parent device.
> +{
> + struct ac97_controller *ac97_ctrl;
> + int ret, i;
> +
> + ac97_ctrl = kzalloc(sizeof(*ac97_ctrl), GFP_KERNEL);
> + if (!ac97_ctrl)
> + return -ENOMEM;
> +
> + for (i = 0; i < AC97_BUS_MAX_CODECS && codecs_pdata; i++)
> + ac97_ctrl->codecs_pdata[i] = codecs_pdata[i];
> +
> + ret = sysfs_create_group(&dev->kobj, &ac97_controller_attr_group);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&ac97_controllers_mutex);
> + ac97_ctrl->ops = ops;
> + ac97_ctrl->slots_available = slots_available;
> + ac97_ctrl->dev = dev;
> + list_add(&ac97_ctrl->controllers, &ac97_controllers);
Stricly speeaking only the list_add needs to be protected.
> + mutex_unlock(&ac97_controllers_mutex);
> +
> + ac97_bus_reset(ac97_ctrl);
> + ac97_bus_scan(ac97_ctrl);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(snd_ac97_controller_register);
> +
> +/**
> + * snd_ac97_controller_unregister - unregister an ac97 controller
> + * @dev: the device previously provided to ac97_controller_register()
> + *
> + * Returns 0 on success, negative upon error
Unregister must not be able to fail. Hotunplug is one of the core concepts
of the device driver model and there is really nothing that can be done to
prevent a device from disappearing, so there is no sensible way of handling
the error (and your pxa driver modifications simply ignore it as well).
This also means the framework needs to cope with the case where the
controller is removed and the CODEC devices are still present. All future
operations should return -ENODEV in that case.
> + */
> +int snd_ac97_controller_unregister(struct device *dev)
> +{
> + struct ac97_controller *ac97_ctrl;
> + int ret = -ENODEV, i;
> +
> + mutex_lock(&ac97_controllers_mutex);
> + ac97_ctrl = ac97_ctrl_find(dev);
> + if (ac97_ctrl) {
> + ret = 0;
> + for (i = 0; i < AC97_BUS_MAX_CODECS; i++)
> + if (ac97_ctrl->codecs[i] &&
> + device_is_registered(&ac97_ctrl->codecs[i]->dev))
> + ret = -EBUSY;
> + if (!ret)
> + ret = ac97_ctrl_codecs_unregister(ac97_ctrl);
> + if (!ret) {
> + list_del(&ac97_ctrl->controllers);
> + sysfs_remove_group(&dev->kobj,
> + &ac97_controller_attr_group);
> + }
> + }
> + mutex_unlock(&ac97_controllers_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(snd_ac97_controller_unregister);
[...]
> +static struct bus_type ac97_bus_type = {
> + .name = "ac97",
> + .dev_attrs = ac97_dev_attrs,
dev_attrs is deprecated in favor of dev_groups (See commit 880ffb5c6).
> + .match = ac97_bus_match,
> + .pm = &ac97_pm,
> + .probe = ac97_bus_probe,
> + .remove = ac97_bus_remove,
> +};
> +
> +static int __init ac97_bus_init(void)
> +{
> + return bus_register(&ac97_bus_type);
> +}
> +subsys_initcall(ac97_bus_init);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Robert Jarzmik <robert.jarzmik@free.fr>");
> diff --git a/sound/ac97/codec.c b/sound/ac97/codec.c
> new file mode 100644
> index 000000000000..a835f03744bf
> --- /dev/null
> +++ b/sound/ac97/codec.c
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <sound/ac97_codec.h>
> +#include <sound/ac97/codec.h>
> +#include <sound/ac97/controller.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <sound/soc.h> /* For compat_ac97_* */
> +
I'm not sure I understand what this file does.
[...]
^ permalink raw reply
* [PATCH RFC 00/12] tda998x updates
From: Russell King - ARM Linux @ 2016-11-08 13:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108133215.GQ1041@n2100.armlinux.org.uk>
On Tue, Nov 08, 2016 at 01:32:15PM +0000, Russell King - ARM Linux wrote:
> Unfortunately, my drm-tda998x-devel branch is slightly out of date with
> these patches it's the original set of 10 patches. I've not pushed
> these ones out to that branch yet, as I've three additional patches on
> top of these which aren't "ready" for pushing out.
Here's the delta between the branch and what I just posted:
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 1b262a89b7e1..3a5e5c466972 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -43,12 +43,12 @@ struct tda998x_priv {
u16 rev;
u8 current_page;
bool is_on;
- bool is_hdmi_sink;
- bool is_hdmi_config;
+ bool supports_infoframes;
+ bool sink_has_audio;
u8 vip_cntrl_0;
u8 vip_cntrl_1;
u8 vip_cntrl_2;
- unsigned long tdms_clock;
+ unsigned long tmds_clock;
struct tda998x_audio_params audio_params;
struct platform_device *audio_pdev;
@@ -774,7 +774,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
* assume 100MHz requires larger divider.
*/
adiv = AUDIO_DIV_SERCLK_8;
- if (priv->tdms_clock > 100000)
+ if (priv->tmds_clock > 100000)
adiv++; /* AUDIO_DIV_SERCLK_16 */
/* S/PDIF asks for a larger divider */
@@ -869,8 +869,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
}
mutex_lock(&priv->audio_mutex);
- /* We must not program the TDA998x for audio if the sink is DVI. */
- if (priv->is_hdmi_config)
+ if (priv->supports_infoframes && priv->sink_has_audio)
ret = tda998x_configure_audio(priv, &audio);
else
ret = 0;
@@ -962,6 +961,27 @@ static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
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;
+
+ mutex_lock(&priv->audio_mutex);
+ 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;
+ }
+ mutex_unlock(&priv->audio_mutex);
+
+ return ret;
+}
+
static enum drm_connector_status
tda998x_connector_detect(struct drm_connector *connector, bool force)
{
@@ -980,7 +1000,7 @@ static void tda998x_connector_destroy(struct drm_connector *connector)
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,
@@ -1072,11 +1092,7 @@ 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);
-
- mutex_lock(&priv->audio_mutex);
drm_edid_to_eld(connector, edid);
- mutex_unlock(&priv->audio_mutex);
kfree(edid);
@@ -1350,8 +1366,22 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
/* must be last register set: */
reg_write(priv, REG_TBG_CNTRL_0, 0);
- /* Only setup the info frames if the sink is HDMI */
- if (priv->is_hdmi_sink) {
+ priv->tmds_clock = adjusted_mode->clock;
+
+ /* 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);
@@ -1360,13 +1390,11 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
tda998x_write_avi(priv, adjusted_mode);
- priv->tdms_clock = adjusted_mode->clock;
-
- 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);
}
- priv->is_hdmi_config = priv->is_hdmi_sink;
mutex_unlock(&priv->audio_mutex);
}
--
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 related
* [PATCH RFC 00/12] tda998x updates
From: Russell King - ARM Linux @ 2016-11-08 13:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2264579a-3273-d9c4-761a-f50f547eb39b@arm.com>
On Tue, Nov 08, 2016 at 01:19:51PM +0000, Robin Murphy wrote:
> Hi Russell,
>
> On 08/11/16 12:24, Russell King - ARM Linux wrote:
> > 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 just merged your drm-tda998x-devel branch and booted it on my Juno,
> and I can at least confirm that DVI still seems to still work as before.
> Anything in particular I should be looking out for? (Unfortunately the
> only handy HDMI monitor is one of those slightly-out-of-spec ones which
> has never really worked with the pixel clocks Juno provides)
Unfortunately, my drm-tda998x-devel branch is slightly out of date with
these patches it's the original set of 10 patches. I've not pushed
these ones out to that branch yet, as I've three additional patches on
top of these which aren't "ready" for pushing out.
--
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 4/6] ARM: dts: add basic support for Rockchip RK1108 SOC
From: Heiko Stübner @ 2016-11-08 13:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0516ad0b-bfbe-ec80-fdb6-e118dab3e758@rock-chips.com>
Am Dienstag, 8. November 2016, 20:31:55 schrieb Andy Yan:
> Hi Heiko:
>
> On 2016?11?04? 16:00, Heiko Stuebner wrote:
> > Am Donnerstag, 3. November 2016, 20:40:48 CET schrieb Andy Yan:
> >> + gic: interrupt-controller at 32010000 {
> >> + compatible = "arm,cortex-a15-gic";
> >
> > compatible = "arm,gic-400"; ?
> >
> >> + interrupt-controller;
> >> + #interrupt-cells = <3>;
> >> + #address-cells = <0>;
> >> +
> >> + reg = <0x32011000 0x1000>,
> >> + <0x32012000 0x1000>;
> >
> > please provide all 4 register areas and also the interrupt (
>
> I only found 2 register areas in our rockchip linux 3.10 source
> code. And haven't found the interrupt. From the arm,gic bindings, the
> interrupt property is optional. So am not sure if we
> really need it here.
Devicetree is a hardware description, so it's not a factor if we "need" it but
only if it is present in the hardware. And we really want this information to
be complete, as these additional areas are necessary if someone wants to use
the virtualization extensions the cortext-A7 does contain.
The gic is a very standard component and the gic400 used here should definitly
have those two additional areas as well as the interrupt.
I think the memory areas are pretty standard and should be for the rk1108:
reg = <0x32011000 0x1000>,
<0x32012000 0x1000>,
<0x32014000 0x2000>,
<0x32016000 0x2000>;
The TRM talks about 128 SPI and 3 PPI interrupts but the irq-list does not
contain them, so this seems to be an error in the TRM, as the gic interrupt
should be one of those PPI interrupts.
Heiko
^ 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