* [PATCH] ARM: dts: da850-lcdk: Add ethernet0 alias to DT
From: Sekhar Nori @ 2016-11-28 5:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124143545.12817-1-fparent@baylibre.com>
On Thursday 24 November 2016 08:05 PM, Fabien Parent wrote:
> In order to avoid Linux generating a random mac address on every boot,
> add an ethernet0 alias that will allow u-boot to patch the dtb with
> the MAC address programmed into the EEPROM.
>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
Applied to v4.10/dt. Thanks!
Regards,
Sekhar
^ permalink raw reply
* [PATCH] ARM: dts: da850: enable the memctrl and mstpri nodes per board
From: Sekhar Nori @ 2016-11-28 5:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479979884-9624-1-git-send-email-bgolaszewski@baylibre.com>
On Thursday 24 November 2016 03:01 PM, Bartosz Golaszewski wrote:
> Currently the memory controller and master priorities drivers are
> enabled in da850.dtsi. For boards for which there are no settings
> defined, this makes these drivers emit error messages.
>
> Disable the nodes in da850.dtsi and only enable them for da850-lcdk -
> the only board that currently needs them.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Applied to v4.10/dt
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
From: Kevin Hilman @ 2016-11-28 6:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20fbc946-d56c-31a3-4ae7-cf61df96a3c3@ti.com>
On Wed, Nov 23, 2016 at 9:03 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Thursday 24 November 2016 04:18 AM, David Lechner wrote:
>> On 11/23/2016 04:32 PM, Kevin Hilman wrote:
>>> David Lechner <david@lechnology.com> writes:
>>>
>>>> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote:
>>>>> 2016-11-22 23:23 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote:
>>>>>>>
>>>>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
>>>>>>> controller drivers to da850.dtsi.
>>>>>>>
>>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>>> ---
>>>>>>> v1 -> v2:
>>>>>>> - moved the priority controller node above the cfgchip node
>>>>>>> - renamed added nodes to better reflect their purpose
>>>>>>>
>>>>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>>>>>> 1 file changed, 8 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi
>>>>>>> b/arch/arm/boot/dts/da850.dtsi
>>>>>>> index 1bb1f6d..412eec6 100644
>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>>> @@ -210,6 +210,10 @@
>>>>>>> };
>>>>>>>
>>>>>>> };
>>>>>>> + prictrl: priority-controller at 14110 {
>>>>>>> + compatible = "ti,da850-mstpri";
>>>>>>> + reg = <0x14110 0x0c>;
>>>>>>
>>>>>>
>>>>>> I think we should add status = "disabled"; here and let boards opt in.
>>>>>>
>>>>>>> + };
>>>>>>> cfgchip: chip-controller at 1417c {
>>>>>>> compatible = "ti,da830-cfgchip", "syscon",
>>>>>>> "simple-mfd";
>>>>>>> reg = <0x1417c 0x14>;
>>>>>>> @@ -451,4 +455,8 @@
>>>>>>> 1 0 0x68000000 0x00008000>;
>>>>>>> status = "disabled";
>>>>>>> };
>>>>>>> + memctrl: memory-controller at b0000000 {
>>>>>>> + compatible = "ti,da850-ddr-controller";
>>>>>>> + reg = <0xb0000000 0xe8>;
>>>>>>
>>>>>>
>>>>>> same here. status = "disabled";
>>>>>>
>>>>>>> + };
>>>>>>> };
>>>>>>>
>>>>>
>>>>> Hi David,
>>>>>
>>>>> I did that initially[1][2] and it was rejected by Kevin[3] and
>>>>> Laurent[4].
>>>>>
>>>>> FYI this patch has already been queued by Sekhar.
>>>>
>>>> Thanks. I did not see those threads.
>>>>
>>>> FYI to maintainers, having these enabled by default causes error
>>>> messages in the kernel log for other boards that are not supported by
>>>> the drivers.
>>>
>>> Then the driver is too noisy and should be cleaned up.
>>>
>>>> Since there is only one board that is supported and soon
>>>> to be 2 that are not, I would rather have this disabled by default to
>>>> avoid the error messages.
>>>
>>> IMO, what exactly are the error messages? Sounds like the driver is
>>> being too verbose, and calling things errors that are not really errors.
>>
>> It is just one line per driver.
>>
>> dev_err(dev, "no master priorities defined for this board\n");
>>
>> and
>>
>> dev_err(dev, "no settings defined for this board\n");
>>
>>
>> Since "ti,da850-lcdk" is the only board supported in these drivers, all
>> other boards will see these error messages.
>
> Thats pretty bad. Sorry about that. The original justification for
> keeping them enabled all the time was that they are in-SoC modules with
> no external dependencies (like IO lines or voltage rails) so they can be
> enabled on all boards that use DA850. While that remains true, the
> configuration itself is board specific.
>
> I think the error messages are still useful, so instead of silencing
> them, I think we should go back to keeping these nodes disabled by
> default and enabling only on boards which have support for it in the driver.
I don't have a strong preference for the enabled/disabled by default,
but I think the error messages are not error messages. It seems
perfectly reasonable for boards to accept the reset (or bootloader)
configuration of these registers, and not consider that an error.
Kevin
^ permalink raw reply
* [PATCH v2] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-28 6:56 UTC (permalink / raw)
To: linux-arm-kernel
Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
This patch adds it. This is based on devm_clk_get()
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
- update git log
drivers/clk/clkdev.c | 26 ++++++++++++++++++++++++++
include/linux/clk.h | 7 +++++++
2 files changed, 33 insertions(+)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 89cc700..93a613b 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -55,6 +55,32 @@ struct clk *of_clk_get(struct device_node *np, int index)
}
EXPORT_SYMBOL(of_clk_get);
+static void devm_of_clk_release(struct device *dev, void *res)
+{
+ clk_put(*(struct clk **)res);
+}
+
+struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ struct clk **ptr, *clk;
+
+ ptr = devres_alloc(devm_of_clk_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ clk = of_clk_get(np, index);
+ if (!IS_ERR(clk)) {
+ *ptr = clk;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return clk;
+}
+EXPORT_SYMBOL(devm_of_clk_get);
+
static struct clk *__of_clk_get_by_name(struct device_node *np,
const char *dev_id,
const char *name)
diff --git a/include/linux/clk.h b/include/linux/clk.h
index a89ba4e..33cd540 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -502,6 +502,8 @@ struct of_phandle_args;
#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
struct clk *of_clk_get(struct device_node *np, int index);
+struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index);
struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
#else
@@ -509,6 +511,11 @@ static inline struct clk *of_clk_get(struct device_node *np, int index)
{
return ERR_PTR(-ENOENT);
}
+static inline struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ return ERR_PTR(-ENOENT);
+}
static inline struct clk *of_clk_get_by_name(struct device_node *np,
const char *name)
{
--
1.9.1
^ permalink raw reply related
* Adding a .platform_init callback to sdhci_arasan_ops
From: Michal Simek @ 2016-11-28 7:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d50dcd8a-2489-4e82-1dc7-13e93e38d5ff@laposte.net>
+Sai for Xilinx perspective.
On 25.11.2016 16:24, Sebastian Frias wrote:
> Hi,
>
> When using the Arasan SDHCI HW IP, there is a set of parameters called
> "Hardware initialized registers"
>
> (Table 7, Section "Pin Signals", page 56 of Arasan "SD3.0/SDIO3.0/eMMC4.4
> AHB Host Controller", revision 6.0 document)
>
> In some platforms those signals are connected to registers that need to
> be programmed at some point for proper driver/HW initialisation.
>
> I found that the 'struct sdhci_ops' contains a '.platform_init' callback
> that is called from within 'sdhci_pltfm_init', and that seems a good
> candidate for a place to program those registers (*).
>
> Do you agree?
>
> Best regards,
>
> Sebastian
>
>
> (*): This has been prototyped on 4.7 as working properly.
> However, upstream commit:
>
> commit 3ea4666e8d429223fbb39c1dccee7599ef7657d5
> Author: Douglas Anderson <dianders@chromium.org>
> Date: Mon Jun 20 10:56:47 2016 -0700
>
> mmc: sdhci-of-arasan: Properly set corecfg_baseclkfreq on rk3399
> ...
>
> could affect this solution because of the way the 'sdhci_arasan_of_match'
> struct is used after that commit.
>
^ permalink raw reply
* [PATCH v3 00/12] Initial Tegra186 support
From: Thierry Reding @ 2016-11-28 7:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161126133927.GE20568@xo-6d-61-c0.localdomain>
On Sat, Nov 26, 2016 at 02:39:27PM +0100, Pavel Machek wrote:
> Hi!
>
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Hi everyone,
> >
> > This is a set of patches to add initial support for Tegra186. It is
> > based on Joseph's patches but I rewrote some of the drivers to be a
> > little easier to comprehend and maintain (hopefully). I've also
> > included clock and reset drivers as a proof of concept.
>
> Is there any phone/tablet on the market with this chipset?
I'm not aware of any. The chip is primarily targetted at automotive
use-cases and probably not suited for phones or tablets.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161128/602266c6/attachment-0001.sig>
^ permalink raw reply
* [PATCH] ARM: dts: da850: specify the maximum bandwidth for tilcdc
From: Tomi Valkeinen @ 2016-11-28 7:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8829c208-0674-43c0-8449-ef764071583f@ti.com>
On 28/11/16 07:24, Sekhar Nori wrote:
> On Friday 25 November 2016 09:07 PM, Bartosz Golaszewski wrote:
>> It has been determined that the maximum resolution supported correctly
>> by tilcdc rev1 on da850 SoCs is 800x600 at 60. Due to memory throughput
>> constraints we must filter out higher modes.
>>
>> Specify the max-bandwidth property for the display node for
>> da850-based boards.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>> arch/arm/boot/dts/da850.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 8e30d9b..9b7c444 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -452,6 +452,7 @@
>> compatible = "ti,da850-tilcdc";
>> reg = <0x213000 0x1000>;
>> interrupts = <52>;
>> + max-bandwidth = <28800000>;
>
> If this is effectively the max pixel clock that the device supports,
> then why not use the datasheet specified value of 37.5 MHz (Tc = 26.66 ns).
There's a separate property for max-pixelclock. This one is maximum
pixels per second (which does sound almost the same), but the doc says
it's about the particular memory interface + LCDC combination.
But this 'max-bandwidth' does sound quite odd, as the it really should
be bytes, not pixels... Bad bindings again, which we just have to use.
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161128/12541df9/attachment.sig>
^ permalink raw reply
* [PATCH] ARM: dts: da850: specify the maximum bandwidth for tilcdc
From: Sekhar Nori @ 2016-11-28 7:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <953743fb-54f9-fa6f-bfdd-43d92271864f@ti.com>
On Monday 28 November 2016 01:12 PM, Tomi Valkeinen wrote:
> On 28/11/16 07:24, Sekhar Nori wrote:
>> On Friday 25 November 2016 09:07 PM, Bartosz Golaszewski wrote:
>>> It has been determined that the maximum resolution supported correctly
>>> by tilcdc rev1 on da850 SoCs is 800x600 at 60. Due to memory throughput
>>> constraints we must filter out higher modes.
>>>
>>> Specify the max-bandwidth property for the display node for
>>> da850-based boards.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> ---
>>> arch/arm/boot/dts/da850.dtsi | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 8e30d9b..9b7c444 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -452,6 +452,7 @@
>>> compatible = "ti,da850-tilcdc";
>>> reg = <0x213000 0x1000>;
>>> interrupts = <52>;
>>> + max-bandwidth = <28800000>;
>>
>> If this is effectively the max pixel clock that the device supports,
>> then why not use the datasheet specified value of 37.5 MHz (Tc = 26.66 ns).
>
> There's a separate property for max-pixelclock. This one is maximum
> pixels per second (which does sound almost the same), but the doc says
> it's about the particular memory interface + LCDC combination.
DA850 supports both mDDR and DDR2, at slightly different speeds. So
memory bandwidth limitation is also board specific. This should probably
move to board file.
But I would like to know why using max-pixelclock is not good enough.
Have experiments shown that LCDC on DA850 LCDK underflows even if pixel
clock is below the datasheet recommendation?
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Uwe Kleine-König @ 2016-11-28 8:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161127231009.GA17704@lunn.ch>
Hello Andrew,
On Mon, Nov 28, 2016 at 12:10:09AM +0100, Andrew Lunn wrote:
> > Try to see it from my perspective: I see that some vf610 device I don't
> > have (found via `git grep marvell,mv88e6` or so) uses
> > "marvell,mv88e6085". I then assume it has that device on board. How
> > would I know it doesn't? Same for the other boards you mention.
> >
> > Unfortunately some of your replies are slightly cryptic. Had you simply
> > replied 'please just use "marvell,mv88e6085" instead', it would've been
> > much more clear what you want. (Same for extending the subject instead
> > of just pointing to some FAQ.)
>
> By reading the FAQ you have learnt more than me saying put the correct
> tree in the subject line. By asking you to explain why you need a
> compatible string, i'm trying to make you think, look at the code and
> understand it. In the future, you might think and understand the code
> before posting a patch, and then we all save time.
I agree to Andreas though, that it makes an school teacher impression.
Something like:
Please fix the subject. Check the FAQ for the details, which btw
is worth a read completely.
is IMHO better in this regard and once you found the problem there you
don't need to ask back if it's that what was meant.
> > So are you okay with patch 1/2 documenting the compatible? Then we could
> > drop 2/2 and use "marvell,mv88e6176", "marvell,mv88e6085" instead of
> > just the latter. Or would you rather drop both and keep the actual chip
> > a comment?
>
> A comment only please.
I still wonder (and didn't get an answer back when I asked about this)
why a comment is preferred here. For other devices I know it's usual and
requested by the maintainers to use:
compatible = "exact name", "earlyer device to match driver";
. This is more robust, documents the situation more formally and makes
it better greppable. The price to pay is only a few bytes in the dtb
which IMO is ok.
Best regards
Uwe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161128/c9e0e382/attachment.sig>
^ permalink raw reply
* [PATCH v2 1/3] ARM: davinci: hawk: use gpio descriptor for mmc pins
From: Sekhar Nori @ 2016-11-28 8:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124150454.23899-2-ahaslam@baylibre.com>
On Thursday 24 November 2016 08:34 PM, Axel Haslam wrote:
> Currently the mmc driver is polling the gpio to know if the
> card was removed.
>
> By using a gpio descriptor instead of the platform callbacks,
> the driver will be able to register the gpio using the mmc core
> API's designed for this purpose.
s/API's/APIs
>
> This has the advantage that an irq will be registered, and
> polling is no longer needed. Also, a dependency on platform
> callbacks is removed for this board.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> ---
> arch/arm/mach-davinci/board-omapl138-hawk.c | 42 ++++++++---------------------
> 1 file changed, 11 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
> index a4e8726..a2966d3 100644
> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
> @@ -15,6 +15,7 @@
> #include <linux/gpio.h>
> #include <linux/platform_data/gpio-davinci.h>
> #include <linux/regulator/machine.h>
> +#include <linux/gpio/machine.h>
Moved this include to below linux/gpio.h
Applied to v4.10/soc
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v2 2/3] ARM: davinci: da850-evm: use gpio descriptor for mmc pins
From: Sekhar Nori @ 2016-11-28 8:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124150454.23899-3-ahaslam@baylibre.com>
On Thursday 24 November 2016 08:34 PM, Axel Haslam wrote:
> Currently the mmc driver is polling the gpio to know if the
> card was removed.
>
> By using a gpio descriptor instead of the platform callbacks,
> the driver will be able to register the gpio using the mmc core
> API's designed for this purpose.
>
> This has the advantage that an irq will be registered, and
> polling is no longer needed. Also, a dependency on platform
> callbacks is removed for this board.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
Applied.
Thanks,
Sekhar
^ permalink raw reply
* [RFC PATCH 3/3] dt-bindings: display: add Amlogic Meson DRM Bindings
From: Laurent Pinchart @ 2016-11-28 8:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480089791-12517-4-git-send-email-narmstrong@baylibre.com>
Hi Neil,
Thank you for the patch.
On Friday 25 Nov 2016 17:03:11 Neil Armstrong wrote:
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> .../bindings/display/meson/meson-drm.txt | 134 +++++++++++++++++
> 1 file changed, 134 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/meson/meson-drm.txt
>
> diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file
> mode 100644
> index 0000000..89c1b5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> @@ -0,0 +1,134 @@
> +Amlogic Meson Display Controller
> +================================
> +
> +The Amlogic Meson Display controller is composed of several components
> +that are going to be documented below:
> +
> +DMC|---------------VPU (Video Processing Unit)------------|------HHI------|
> + | vd1 _______ _____________ _____________ | |
> +D |-------| |----| | | | | HDMI PLL |
> +D | vd2 | VIU | | Video Post | | Video Encs |<---|-----VCLK |
> +R |-------| |----| Processing | | | | |
> + | osd2 | | | |---| Enci ------|----|-----VDAC------|
> +R |-------| CSC |----| Scalers | | Encp ------|----|----HDMI-TX----|
> +A | osd1 | | | Blenders | | Encl-------|----|---------------|
> +M |-------|______|----|____________| |____________| | |
> +___|______________________________________________________|_______________|
> +
> +
> +VIU: Video Input Unit
> +---------------------
> +
> +The Video Input Unit is in charge of the pixel scanout from the DDR memory.
> +It fetches the frames addresses, stride and parameters from the "Canvas"
> memory.
> +This part is also in charge of the CSC (Colorspace Conversion).
> +It can handle 2 OSD Planes and 2 Video Planes.
> +
> +VPP: Video Processing Unit
Do you mean "Video Post Processing" ? In your diagram above Video Processing
Unit is abbreviated VPU and covers the VIU, VPP and encoders.
> +--------------------------
> +
> +The Video Processing Unit is in charge if the scaling and blending of the
> +various planes into a single pixel stream.
> +There is a special "pre-blending" used by the video planes with a dedicated
> +scaler and a "post-blending" to merge with the OSD Planes.
> +The OSD planes also have a dedicated scaler for one of the OSD.
> +
> +VENC: Video Encoders
> +--------------------
> +
> +The VENC is composed of the multiple pixel encoders :
> + - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
> + - ENCP : Progressive Video Encoder for HDMI
> + - ENCL : LCD LVDS Encoder
> +The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and
> clock
> +tree and provides the scanout clock to the VPP and VIU.
> +The ENCI is connected to a single VDAC for Composite Output.
> +The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
> +
> +Device Tree Bindings:
> +---------------------
> +
> +VPU: Video Processing Unit
> +--------------------------
> +
> +Required properties:
> + - compatible: value should be different for each SoC family as :
> + - GXBB (S905) : "amlogic,meson-gxbb-vpu"
> + - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
> + - GXM (S912) : "amlogic,meson-gxm-vpu"
> + followed by the common "amlogic,meson-gx-vpu"
> + - reg: base address and size of he following memory-mapped regions :
> + - vpu
> + - hhi
> + - dmc
> + - reg-names: should contain the names of the previous memory regions
> + - interrupts: should contain the VENC Vsync interrupt number
> +
> +- ports: A ports node with endpoint definitions as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
> + second port should be the output endpoints for VENC connectors.
> +
> +VENC CBVS Output
> +----------------------
> +
> +The VENC can output Composite/CVBS output via a decicated VDAC.
> +
> +Required properties:
> + - compatible: value must be one of:
> + - compatible: value should be different for each SoC family as :
One of those two lines is redundant.
> + - GXBB (S905) : "amlogic,meson-gxbb-venc-cvbs"
> + - GXL (S905X, S905D) : "amlogic,meson-gxl-venc-cvbs"
> + - GXM (S912) : "amlogic,meson-gxm-venc-cvbs"
> + followed by the common "amlogic,meson-gx-venc-cvbs"
> +
No registers ? Are the encoders registers part of the VPU register space,
intertwined in a way that they can't be specified separately here ?
> +- ports: A ports node with endpoint definitions as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
> + first port should be the input endpoints, connected ot the VPU node.
> +
> +Example:
> +
> +venc_cvbs: venc-cvbs {
> + compatible = "amlogic,meson-gxbb-venc-cvbs";
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + enc_cvbs_in: port at 0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + venc_cvbs_in_vpu: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <&vpu_out_venc_cvbs>;
> + };
> + };
> + };
> +};
> +
> +vpu: vpu at d0100000 {
> + compatible = "amlogic,meson-gxbb-vpu";
> + reg = <0x0 0xd0100000 0x0 0x100000>,
> + <0x0 0xc883c000 0x0 0x1000>,
> + <0x0 0xc8838000 0x0 0x1000>;
> + reg-names = "base", "hhi", "dmc";
> + interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + vpu_out: port at 1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vpu_out_venc_cvbs: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <&venc_cvbs_in_vpu>;
> + };
> + };
> + };
> +};
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH net-next 1/5] net: mvneta: Use cacheable memory to store the rx buffer virtual address
From: Jisheng Zhang @ 2016-11-28 8:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7e6004f918d3fcde9ae71e7893d26b19086236a3.1480087510.git-series.gregory.clement@free-electrons.com>
Hi Gregory,
On Fri, 25 Nov 2016 16:30:14 +0100 Gregory CLEMENT wrote:
> Until now the virtual address of the received buffer were stored in the
> cookie field of the rx descriptor. However, this field is 32-bits only
> which prevents to use the driver on a 64-bits architecture.
>
> With this patch the virtual address is stored in an array not shared with
> the hardware (no more need to use the DMA API). Thanks to this, it is
> possible to use cache contrary to the access of the rx descriptor member.
>
> The change is done in the swbm path only because the hwbm uses the cookie
> field, this also means that currently the hwbm is not usable in 64-bits.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/net/ethernet/marvell/mvneta.c | 96 ++++++++++++++++++++++++----
> 1 file changed, 84 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 87274d4ab102..b6849f88cab7 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -561,6 +561,9 @@ struct mvneta_rx_queue {
> u32 pkts_coal;
> u32 time_coal;
>
> + /* Virtual address of the RX buffer */
> + void **buf_virt_addr;
can we store buf_phys_addr in cacheable memory as well?
> +
> /* Virtual address of the RX DMA descriptors array */
> struct mvneta_rx_desc *descs;
>
> @@ -1573,10 +1576,14 @@ static void mvneta_tx_done_pkts_coal_set(struct mvneta_port *pp,
>
> /* Handle rx descriptor fill by setting buf_cookie and buf_phys_addr */
> static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc,
> - u32 phys_addr, u32 cookie)
> + u32 phys_addr, void *virt_addr,
> + struct mvneta_rx_queue *rxq)
> {
> - rx_desc->buf_cookie = cookie;
> + int i;
> +
> rx_desc->buf_phys_addr = phys_addr;
> + i = rx_desc - rxq->descs;
> + rxq->buf_virt_addr[i] = virt_addr;
> }
>
> /* Decrement sent descriptors counter */
> @@ -1781,7 +1788,8 @@ EXPORT_SYMBOL_GPL(mvneta_frag_free);
>
> /* Refill processing for SW buffer management */
> static int mvneta_rx_refill(struct mvneta_port *pp,
> - struct mvneta_rx_desc *rx_desc)
> + struct mvneta_rx_desc *rx_desc,
> + struct mvneta_rx_queue *rxq)
>
> {
> dma_addr_t phys_addr;
> @@ -1799,7 +1807,7 @@ static int mvneta_rx_refill(struct mvneta_port *pp,
> return -ENOMEM;
> }
>
> - mvneta_rx_desc_fill(rx_desc, phys_addr, (u32)data);
> + mvneta_rx_desc_fill(rx_desc, phys_addr, data, rxq);
> return 0;
> }
>
> @@ -1861,7 +1869,12 @@ static void mvneta_rxq_drop_pkts(struct mvneta_port *pp,
>
> for (i = 0; i < rxq->size; i++) {
> struct mvneta_rx_desc *rx_desc = rxq->descs + i;
> - void *data = (void *)rx_desc->buf_cookie;
> + void *data;
> +
> + if (!pp->bm_priv)
> + data = rxq->buf_virt_addr[i];
> + else
> + data = (void *)(uintptr_t)rx_desc->buf_cookie;
>
> dma_unmap_single(pp->dev->dev.parent, rx_desc->buf_phys_addr,
> MVNETA_RX_BUF_SIZE(pp->pkt_size), DMA_FROM_DEVICE);
> @@ -1894,12 +1907,13 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> unsigned char *data;
> dma_addr_t phys_addr;
> u32 rx_status, frag_size;
> - int rx_bytes, err;
> + int rx_bytes, err, index;
>
> rx_done++;
> rx_status = rx_desc->status;
> rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
> - data = (unsigned char *)rx_desc->buf_cookie;
> + index = rx_desc - rxq->descs;
> + data = (unsigned char *)rxq->buf_virt_addr[index];
> phys_addr = rx_desc->buf_phys_addr;
>
> if (!mvneta_rxq_desc_is_first_last(rx_status) ||
> @@ -1938,7 +1952,7 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> }
>
> /* Refill processing */
> - err = mvneta_rx_refill(pp, rx_desc);
> + err = mvneta_rx_refill(pp, rx_desc, rxq);
> if (err) {
> netdev_err(dev, "Linux processing - Can't refill\n");
> rxq->missed++;
> @@ -2020,7 +2034,7 @@ static int mvneta_rx_hwbm(struct mvneta_port *pp, int rx_todo,
> rx_done++;
> rx_status = rx_desc->status;
> rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
> - data = (unsigned char *)rx_desc->buf_cookie;
> + data = (u8 *)(uintptr_t)rx_desc->buf_cookie;
> phys_addr = rx_desc->buf_phys_addr;
> pool_id = MVNETA_RX_GET_BM_POOL_ID(rx_desc);
> bm_pool = &pp->bm_priv->bm_pools[pool_id];
> @@ -2708,6 +2722,57 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
> return rx_done;
> }
>
> +/* Refill processing for HW buffer management */
> +static int mvneta_rx_hwbm_refill(struct mvneta_port *pp,
> + struct mvneta_rx_desc *rx_desc)
> +
> +{
> + dma_addr_t phys_addr;
> + void *data;
> +
> + data = mvneta_frag_alloc(pp->frag_size);
> + if (!data)
> + return -ENOMEM;
> +
> + phys_addr = dma_map_single(pp->dev->dev.parent, data,
> + MVNETA_RX_BUF_SIZE(pp->pkt_size),
> + DMA_FROM_DEVICE);
> + if (unlikely(dma_mapping_error(pp->dev->dev.parent, phys_addr))) {
> + mvneta_frag_free(pp->frag_size, data);
> + return -ENOMEM;
> + }
> +
> + phys_addr += pp->rx_offset_correction;
> + rx_desc->buf_phys_addr = phys_addr;
> + rx_desc->buf_cookie = (uintptr_t)data;
> +
> + return 0;
> +}
> +
> +/* Handle rxq fill: allocates rxq skbs; called when initializing a port */
> +static int mvneta_rxq_bm_fill(struct mvneta_port *pp,
> + struct mvneta_rx_queue *rxq,
> + int num)
> +{
> + int i;
> +
> + for (i = 0; i < num; i++) {
> + memset(rxq->descs + i, 0, sizeof(struct mvneta_rx_desc));
> + if (mvneta_rx_hwbm_refill(pp, rxq->descs + i) != 0) {
> + netdev_err(pp->dev, "%s:rxq %d, %d of %d buffs filled\n",
> + __func__, rxq->id, i, num);
> + break;
> + }
> + }
> +
> + /* Add this number of RX descriptors as non occupied (ready to
> + * get packets)
> + */
> + mvneta_rxq_non_occup_desc_add(pp, rxq, i);
> +
> + return i;
> +}
> +
> /* Handle rxq fill: allocates rxq skbs; called when initializing a port */
> static int mvneta_rxq_fill(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
> int num)
> @@ -2716,7 +2781,7 @@ static int mvneta_rxq_fill(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
>
> for (i = 0; i < num; i++) {
> memset(rxq->descs + i, 0, sizeof(struct mvneta_rx_desc));
> - if (mvneta_rx_refill(pp, rxq->descs + i) != 0) {
> + if (mvneta_rx_refill(pp, rxq->descs + i, rxq) != 0) {
> netdev_err(pp->dev, "%s:rxq %d, %d of %d buffs filled\n",
> __func__, rxq->id, i, num);
> break;
> @@ -2784,14 +2849,21 @@ static int mvneta_rxq_init(struct mvneta_port *pp,
> mvneta_rxq_buf_size_set(pp, rxq,
> MVNETA_RX_BUF_SIZE(pp->pkt_size));
> mvneta_rxq_bm_disable(pp, rxq);
> +
> + rxq->buf_virt_addr = devm_kmalloc(pp->dev->dev.parent,
> + rxq->size * sizeof(void *),
> + GFP_KERNEL);
I would suggest allocate this buffer during probe. Otherwise, there's
memory leak if we either change the mtu or close then open the eth in
a loop, e.g
while true
do
ifconfig eth0 up
ifconfig eth0 down
done
Thanks,
Jisheng
> + if (!rxq->buf_virt_addr)
> + return -ENOMEM;
> +
> + mvneta_rxq_fill(pp, rxq, rxq->size);
> } else {
> mvneta_rxq_bm_enable(pp, rxq);
> mvneta_rxq_long_pool_set(pp, rxq);
> mvneta_rxq_short_pool_set(pp, rxq);
> + mvneta_rxq_bm_fill(pp, rxq, rxq->size);
> }
>
> - mvneta_rxq_fill(pp, rxq, rxq->size);
> -
> return 0;
> }
>
^ permalink raw reply
* [PATCH net-next 0/5] Support Armada 37xx SoC (ARMv8 64-bits) in mvneta driver
From: Jisheng Zhang @ 2016-11-28 8:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.2b146800967005632cd02d4da77397e6e2fdf51f.1480087510.git-series.gregory.clement@free-electrons.com>
Hi Gregory,
On Fri, 25 Nov 2016 16:30:13 +0100 Gregory CLEMENT wrote:
> Hi,
>
> The Armada 37xx is a new ARMv8 SoC from Marvell using same network
> controller as the older Armada 370/38x/XP SoCs. This series adapts the
> driver in order to be able to use it on this new SoC. The main changes
> are:
>
> - 64-bits support: the first patches allow using the driver on a 64-bit
> architecture.
>
> - MBUS support: the mbus configuration is different on Armada 37xx
> from the older SoCs.
>
> - per cpu interrupt: Armada 37xx do not support per cpu interrupt for
> the NETA IP, the non-per-CPU behavior was added back.
>
> The first item is solved by patches 1 to 3.
> The 2 last items are solved by patch 4.
> In patch 5 the dt support is added.
>
> Beside Armada 37xx, the series have been tested on Armada XP and
> Armada 38x (with Hardware Buffer Management and with Software Buffer
> Managment).
AFAICT, this is a V2? seems no Change log ;)
Thanks,
Jisheng
^ permalink raw reply
* [kvm-unit-tests PATCH v7 01/11] run_tests: allow forcing of acceleration mode
From: Andrew Jones @ 2016-11-28 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-2-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:23PM +0000, Alex Benn?e wrote:
> While tests can be pegged to tcg it is useful to override this from time
> to time, especially when testing correctness on real systems.
> ---
> run_tests.sh | 8 ++++++--
> scripts/runtime.bash | 4 ++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index 254129d..b88c36f 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -13,9 +13,10 @@ function usage()
> {
> cat <<EOF
>
> -Usage: $0 [-g group] [-h] [-v]
> +Usage: $0 [-g group] [-a accel] [-h] [-v]
>
> -g: Only execute tests in the given group
> + -a: Force acceleration mode (tcg/kvm)
> -h: Output this help text
> -v: Enables verbose mode
>
> @@ -28,11 +29,14 @@ EOF
> RUNTIME_arch_run="./$TEST_DIR/run"
> source scripts/runtime.bash
>
> -while getopts "g:hv" opt; do
> +while getopts "g:a:hv" opt; do
> case $opt in
> g)
> only_group=$OPTARG
> ;;
> + a)
> + force_accel=$OPTARG
> + ;;
> h)
> usage
> exit
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 11a40a9..578cf32 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -75,6 +75,10 @@ function run()
> return;
> fi
>
> + if [ -n "$force_accel" ]; then
> + accel=$force_accel
> + fi
> +
> if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
> echo "`SKIP` $1 ($arch only)"
> return 2
> --
> 2.10.1
We can already do 'ACCEL=tcg ./run_tests.sh' to force, for example, tcg.
Additionally, you can add any env you want to the config.mak after running
configure,
echo ACCEL=tcg >> config.mak
If you still prefer a cmdline parameter, then I'd suggest a boolean
instead, with the default being KVM. So the param would be '-tcg', or
something.
Thanks,
drew
^ permalink raw reply
* [kvm-unit-tests PATCH v7 02/11] run_tests: allow disabling of timeouts
From: Andrew Jones @ 2016-11-28 9:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-3-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:24PM +0000, Alex Benn?e wrote:
> Certainly during development of the tests and MTTCG there are times when
> the timeout just gets in the way.
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> ---
> run_tests.sh | 8 ++++++--
> scripts/runtime.bash | 4 ++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index b88c36f..4f2e5cb 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -13,10 +13,11 @@ function usage()
> {
> cat <<EOF
>
> -Usage: $0 [-g group] [-a accel] [-h] [-v]
> +Usage: $0 [-g group] [-a accel] [-t] [-h] [-v]
>
> -g: Only execute tests in the given group
> -a: Force acceleration mode (tcg/kvm)
> + -t: disable timeouts
> -h: Output this help text
> -v: Enables verbose mode
>
> @@ -29,7 +30,7 @@ EOF
> RUNTIME_arch_run="./$TEST_DIR/run"
> source scripts/runtime.bash
>
> -while getopts "g:a:hv" opt; do
> +while getopts "g:a:thv" opt; do
> case $opt in
> g)
> only_group=$OPTARG
> @@ -37,6 +38,9 @@ while getopts "g:a:hv" opt; do
> a)
> force_accel=$OPTARG
> ;;
> + t)
> + no_timeout="yes"
> + ;;
> h)
> usage
> exit
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 578cf32..968ff6d 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -79,6 +79,10 @@ function run()
> accel=$force_accel
> fi
>
> + if [ "$no_timeout" = "yes" ]; then
> + timeout=""
> + fi
> +
> if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
> echo "`SKIP` $1 ($arch only)"
> return 2
> --
> 2.10.1
>
A timeout value of zero disables the timeout. So you just need to run
TIMEOUT=0 ./run_tests.sh, or add it to config.mak.
drew
^ permalink raw reply
* [Qemu-devel] [kvm-unit-tests PATCH v7 03/11] run_tests: allow passing of options to QEMU
From: Andrew Jones @ 2016-11-28 9:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-4-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:25PM +0000, Alex Benn?e wrote:
> This introduces a the option -o for passing of options directly to QEMU
> which is useful. In my case I'm using it to toggle MTTCG on an off:
>
> ./run_tests.sh -t -o "-tcg mttcg=on"
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> ---
> run_tests.sh | 10 +++++++---
> scripts/functions.bash | 13 +++++++------
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index 4f2e5cb..05cc7fb 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -13,10 +13,11 @@ function usage()
> {
> cat <<EOF
>
> -Usage: $0 [-g group] [-a accel] [-t] [-h] [-v]
> +Usage: $0 [-g group] [-a accel] [-o qemu_opts] [-t] [-h] [-v]
>
> -g: Only execute tests in the given group
> -a: Force acceleration mode (tcg/kvm)
> + -o: additional options for QEMU command line
> -t: disable timeouts
> -h: Output this help text
> -v: Enables verbose mode
> @@ -30,7 +31,7 @@ EOF
> RUNTIME_arch_run="./$TEST_DIR/run"
> source scripts/runtime.bash
>
> -while getopts "g:a:thv" opt; do
> +while getopts "g:a:o:thv" opt; do
> case $opt in
> g)
> only_group=$OPTARG
> @@ -38,6 +39,9 @@ while getopts "g:a:thv" opt; do
> a)
> force_accel=$OPTARG
> ;;
> + o)
> + extra_opts=$OPTARG
> + ;;
> t)
> no_timeout="yes"
> ;;
> @@ -67,4 +71,4 @@ RUNTIME_log_stdout () {
> config=$TEST_DIR/unittests.cfg
> rm -f test.log
> printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> -for_each_unittest $config run
> +for_each_unittest $config run "$extra_opts"
> diff --git a/scripts/functions.bash b/scripts/functions.bash
> index ee9143c..d38a69e 100644
> --- a/scripts/functions.bash
> +++ b/scripts/functions.bash
> @@ -2,11 +2,12 @@
> function for_each_unittest()
> {
> local unittests="$1"
> - local cmd="$2"
> - local testname
> + local cmd="$2"
> + local extra_opts=$3
> + local testname
We use tabs in this file. Not sure why cmd and testname got
changed too...
> local smp
> local kernel
> - local opts
> + local opts=$extra_opts
> local groups
> local arch
> local check
> @@ -21,7 +22,7 @@ function for_each_unittest()
> testname=${BASH_REMATCH[1]}
> smp=1
> kernel=""
> - opts=""
> + opts=$extra_opts
> groups=""
> arch=""
> check=""
> @@ -32,7 +33,7 @@ function for_each_unittest()
> elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> smp=${BASH_REMATCH[1]}
> elif [[ $line =~ ^extra_params\ *=\ *(.*)$ ]]; then
> - opts=${BASH_REMATCH[1]}
> + opts="$opts ${BASH_REMATCH[1]}"
> elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
> groups=${BASH_REMATCH[1]}
> elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> @@ -45,6 +46,6 @@ function for_each_unittest()
> timeout=${BASH_REMATCH[1]}
> fi
> done
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> + "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> exec {fd}<&-
> }
> --
> 2.10.1
>
>
This is a pretty good idea, but I think I might like the extra options
to be given like this instead
./run_tests.sh [run_tests.sh options] -- [qemu options]
Thanks,
drew
^ permalink raw reply
* [PATCH v2] clkdev: add devm_of_clk_get()
From: Russell King - ARM Linux @ 2016-11-28 9:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8737icayac.wl%kuninori.morimoto.gx@renesas.com>
On Mon, Nov 28, 2016 at 06:56:52AM +0000, Kuninori Morimoto wrote:
> Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
> This patch adds it. This is based on devm_clk_get()
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Please put this in drivers/clk/clk-devres.c, where you'll find that
we have devm_clk_release() which is identical to your
devm_of_clk_release(). It'll also not need the dummy definition of
devm_of_clk_get().
Thanks.
> ---
> v1 -> v2
>
> - update git log
>
> drivers/clk/clkdev.c | 26 ++++++++++++++++++++++++++
> include/linux/clk.h | 7 +++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 89cc700..93a613b 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -55,6 +55,32 @@ struct clk *of_clk_get(struct device_node *np, int index)
> }
> EXPORT_SYMBOL(of_clk_get);
>
> +static void devm_of_clk_release(struct device *dev, void *res)
> +{
> + clk_put(*(struct clk **)res);
> +}
> +
> +struct clk *devm_of_clk_get(struct device *dev,
> + struct device_node *np, int index)
> +{
> + struct clk **ptr, *clk;
> +
> + ptr = devres_alloc(devm_of_clk_release, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + clk = of_clk_get(np, index);
> + if (!IS_ERR(clk)) {
> + *ptr = clk;
> + devres_add(dev, ptr);
> + } else {
> + devres_free(ptr);
> + }
> +
> + return clk;
> +}
> +EXPORT_SYMBOL(devm_of_clk_get);
> +
> static struct clk *__of_clk_get_by_name(struct device_node *np,
> const char *dev_id,
> const char *name)
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index a89ba4e..33cd540 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -502,6 +502,8 @@ struct of_phandle_args;
>
> #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
> struct clk *of_clk_get(struct device_node *np, int index);
> +struct clk *devm_of_clk_get(struct device *dev,
> + struct device_node *np, int index);
> struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
> struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
> #else
> @@ -509,6 +511,11 @@ static inline struct clk *of_clk_get(struct device_node *np, int index)
> {
> return ERR_PTR(-ENOENT);
> }
> +static inline struct clk *devm_of_clk_get(struct device *dev,
> + struct device_node *np, int index)
> +{
> + return ERR_PTR(-ENOENT);
> +}
> static inline struct clk *of_clk_get_by_name(struct device_node *np,
> const char *name)
> {
> --
> 1.9.1
>
--
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
* [kvm-unit-tests PATCH v7 04/11] libcflat: add PRI(dux)32 format types
From: Andrew Jones @ 2016-11-28 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-5-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:26PM +0000, Alex Benn?e wrote:
> So we can have portable formatting of uint32_t types.
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> ---
> lib/libcflat.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index bdcc561..6dab5be 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -55,12 +55,17 @@ typedef _Bool bool;
> #define true 1
>
> #if __SIZEOF_LONG__ == 8
> +# define __PRI32_PREFIX
> # define __PRI64_PREFIX "l"
> # define __PRIPTR_PREFIX "l"
> #else
> +# define __PRI32_PREFIX "l"
But a 32-bit value is an 'int' and an 'int' shouldn't ever
require an 'l'. Why was this necessary?
> # define __PRI64_PREFIX "ll"
> # define __PRIPTR_PREFIX
> #endif
> +#define PRId32 __PRI32_PREFIX "d"
> +#define PRIu32 __PRI32_PREFIX "u"
> +#define PRIx32 __PRI32_PREFIX "x"
> #define PRId64 __PRI64_PREFIX "d"
> #define PRIu64 __PRI64_PREFIX "u"
> #define PRIx64 __PRI64_PREFIX "x"
> --
> 2.10.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply
* [PATCH v2] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-28 9:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128091456.GR14217@n2100.armlinux.org.uk>
Hi Russell
> > Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
> > This patch adds it. This is based on devm_clk_get()
> >
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Please put this in drivers/clk/clk-devres.c, where you'll find that
> we have devm_clk_release() which is identical to your
> devm_of_clk_release(). It'll also not need the dummy definition of
> devm_of_clk_get().
OK, will do
^ permalink raw reply
* [RFC PATCH 3/3] dt-bindings: display: add Amlogic Meson DRM Bindings
From: Neil Armstrong @ 2016-11-28 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3721857.XEGXu9B51N@avalon>
Hi Laurent,
On 11/28/2016 09:33 AM, Laurent Pinchart wrote:
> Hi Neil,
>
> Thank you for the patch.
>
> On Friday 25 Nov 2016 17:03:11 Neil Armstrong wrote:
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> .../bindings/display/meson/meson-drm.txt | 134 +++++++++++++++++
>> 1 file changed, 134 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/display/meson/meson-drm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt
>> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file
>> mode 100644
>> index 0000000..89c1b5f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
>> @@ -0,0 +1,134 @@
>> +Amlogic Meson Display Controller
>> +================================
>> +
>> +The Amlogic Meson Display controller is composed of several components
>> +that are going to be documented below:
>> +
>> +DMC|---------------VPU (Video Processing Unit)------------|------HHI------|
>> + | vd1 _______ _____________ _____________ | |
>> +D |-------| |----| | | | | HDMI PLL |
>> +D | vd2 | VIU | | Video Post | | Video Encs |<---|-----VCLK |
>> +R |-------| |----| Processing | | | | |
>> + | osd2 | | | |---| Enci ------|----|-----VDAC------|
>> +R |-------| CSC |----| Scalers | | Encp ------|----|----HDMI-TX----|
>> +A | osd1 | | | Blenders | | Encl-------|----|---------------|
>> +M |-------|______|----|____________| |____________| | |
>> +___|______________________________________________________|_______________|
>> +
>> +
>> +VIU: Video Input Unit
>> +---------------------
>> +
>> +The Video Input Unit is in charge of the pixel scanout from the DDR memory.
>> +It fetches the frames addresses, stride and parameters from the "Canvas"
>> memory.
>> +This part is also in charge of the CSC (Colorspace Conversion).
>> +It can handle 2 OSD Planes and 2 Video Planes.
>> +
>> +VPP: Video Processing Unit
>
> Do you mean "Video Post Processing" ? In your diagram above Video Processing
> Unit is abbreviated VPU and covers the VIU, VPP and encoders.
Exact, I meant VPP here.
>
>> +--------------------------
>> +
>> +The Video Processing Unit is in charge if the scaling and blending of the
>> +various planes into a single pixel stream.
>> +There is a special "pre-blending" used by the video planes with a dedicated
>> +scaler and a "post-blending" to merge with the OSD Planes.
>> +The OSD planes also have a dedicated scaler for one of the OSD.
>> +
>> +VENC: Video Encoders
>> +--------------------
>> +
>> +The VENC is composed of the multiple pixel encoders :
>> + - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
>> + - ENCP : Progressive Video Encoder for HDMI
>> + - ENCL : LCD LVDS Encoder
>> +The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and
>> clock
>> +tree and provides the scanout clock to the VPP and VIU.
>> +The ENCI is connected to a single VDAC for Composite Output.
>> +The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
>> +
>> +Device Tree Bindings:
>> +---------------------
>> +
>> +VPU: Video Processing Unit
>> +--------------------------
>> +
>> +Required properties:
>> + - compatible: value should be different for each SoC family as :
>> + - GXBB (S905) : "amlogic,meson-gxbb-vpu"
>> + - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
>> + - GXM (S912) : "amlogic,meson-gxm-vpu"
>> + followed by the common "amlogic,meson-gx-vpu"
>> + - reg: base address and size of he following memory-mapped regions :
>> + - vpu
>> + - hhi
>> + - dmc
>> + - reg-names: should contain the names of the previous memory regions
>> + - interrupts: should contain the VENC Vsync interrupt number
>> +
>> +- ports: A ports node with endpoint definitions as defined in
>> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
>> + second port should be the output endpoints for VENC connectors.
>> +
>> +VENC CBVS Output
>> +----------------------
>> +
>> +The VENC can output Composite/CVBS output via a decicated VDAC.
>> +
>> +Required properties:
>> + - compatible: value must be one of:
>> + - compatible: value should be different for each SoC family as :
>
> One of those two lines is redundant.
Will fix.
>
>> + - GXBB (S905) : "amlogic,meson-gxbb-venc-cvbs"
>> + - GXL (S905X, S905D) : "amlogic,meson-gxl-venc-cvbs"
>> + - GXM (S912) : "amlogic,meson-gxm-venc-cvbs"
>> + followed by the common "amlogic,meson-gx-venc-cvbs"
>> +
>
> No registers ? Are the encoders registers part of the VPU register space,
> intertwined in a way that they can't be specified separately here ?
Exact, all the video registers on the Amlogic SoC are part of a long history of fixup/enhance from very old SoCs, it's
quite hard to distinguish a Venc registers array since they are mixed with the multiple encoders registers...
The only separate registers are the VDAC and HDMI PHY, I may move them to these separate nodes since they are part of the HHI register space.
It is a problem if I move them in the next release ? Next release will certainly have HDMI support, and will have these refactorings.
>
>> +- ports: A ports node with endpoint definitions as defined in
>> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
>> + first port should be the input endpoints, connected ot the VPU node.
>> +
>> +Example:
>> +
>> +venc_cvbs: venc-cvbs {
>> + compatible = "amlogic,meson-gxbb-venc-cvbs";
>> + status = "okay";
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + enc_cvbs_in: port at 0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + venc_cvbs_in_vpu: endpoint at 0 {
>> + reg = <0>;
>> + remote-endpoint = <&vpu_out_venc_cvbs>;
>> + };
>> + };
>> + };
>> +};
>> +
>> +vpu: vpu at d0100000 {
>> + compatible = "amlogic,meson-gxbb-vpu";
>> + reg = <0x0 0xd0100000 0x0 0x100000>,
>> + <0x0 0xc883c000 0x0 0x1000>,
>> + <0x0 0xc8838000 0x0 0x1000>;
>> + reg-names = "base", "hhi", "dmc";
>> + interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + vpu_out: port at 1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> +
>> + vpu_out_venc_cvbs: endpoint at 0 {
>> + reg = <0>;
>> + remote-endpoint = <&venc_cvbs_in_vpu>;
>> + };
>> + };
>> + };
>> +};
>
Thanks for the review !
Neil
^ permalink raw reply
* [PATCH v3] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-28 9:32 UTC (permalink / raw)
To: linux-arm-kernel
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
This patch adds it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v2 -> v3
- implement in clk-devres.c, and reused existing devm_clk_release()
drivers/clk/clk-devres.c | 21 +++++++++++++++++++++
include/linux/clk.h | 7 +++++++
2 files changed, 28 insertions(+)
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 8f57154..2449b25 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -53,3 +53,24 @@ void devm_clk_put(struct device *dev, struct clk *clk)
WARN_ON(ret);
}
EXPORT_SYMBOL(devm_clk_put);
+
+struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ struct clk **ptr, *clk;
+
+ ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ clk = of_clk_get(np, index);
+ if (!IS_ERR(clk)) {
+ *ptr = clk;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return clk;
+}
+EXPORT_SYMBOL(devm_of_clk_get);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 123c027..1b713db 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -506,6 +506,8 @@ static inline void clk_disable_unprepare(struct clk *clk)
#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
struct clk *of_clk_get(struct device_node *np, int index);
+struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index);
struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
#else
@@ -513,6 +515,11 @@ static inline struct clk *of_clk_get(struct device_node *np, int index)
{
return ERR_PTR(-ENOENT);
}
+static inline struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ return ERR_PTR(-ENOENT);
+}
static inline struct clk *of_clk_get_by_name(struct device_node *np,
const char *name)
{
--
1.9.1
^ permalink raw reply related
* [Qemu-devel] [kvm-unit-tests PATCH v7 06/11] arm/Makefile.common: force -fno-pic
From: Andrew Jones @ 2016-11-28 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-7-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:28PM +0000, Alex Benn?e wrote:
> As distro compilers move towards defaults for build hardening for things
> like ASLR we need to force -fno-pic. Failure to do can lead to weird
> relocation problems when we build our "lat" binaries.
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> ---
> arm/Makefile.common | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index 52f7440..cca0d9c 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -21,6 +21,7 @@ phys_base = $(LOADADDR)
>
> CFLAGS += -std=gnu99
> CFLAGS += -ffreestanding
> +CFLAGS += -fno-pic
> CFLAGS += -Wextra
> CFLAGS += -O2
> CFLAGS += -I lib -I lib/libfdt
> --
> 2.10.1
>
>
Applied to arm/next
https://github.com/rhdrjones/kvm-unit-tests/commits/arm/next
Thanks,
drew
^ permalink raw reply
* [RFC PATCH 1/3] drm: Add support for Amlogic Meson Graphic Controller
From: Neil Armstrong @ 2016-11-28 9:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128081601.wapare3pyzmzghvp@phenom.ffwll.local>
Hi Daniel,
On 11/28/2016 09:16 AM, Daniel Vetter wrote:
> On Fri, Nov 25, 2016 at 05:03:09PM +0100, Neil Armstrong wrote:
>> The Amlogic Meson Display controller is composed of several components :
>>
>> DMC|---------------VPU (Video Processing Unit)----------------|------HHI------|
>> | vd1 _______ _____________ _________________ | |
>> D |-------| |----| | | | | HDMI PLL |
>> D | vd2 | VIU | | Video Post | | Video Encoders |<---|-----VCLK |
>> R |-------| |----| Processing | | | | |
>> | osd2 | | | |---| Enci ----------|----|-----VDAC------|
>> R |-------| CSC |----| Scalers | | Encp ----------|----|----HDMI-TX----|
>> A | osd1 | | | Blenders | | Encl ----------|----|---------------|
>> M |-------|______|----|____________| |________________| | |
>> ___|__________________________________________________________|_______________|
>>
>>
>> VIU: Video Input Unit
>> ---------------------
>>
>> The Video Input Unit is in charge of the pixel scanout from the DDR memory.
>> It fetches the frames addresses, stride and parameters from the "Canvas" memory.
>> This part is also in charge of the CSC (Colorspace Conversion).
>> It can handle 2 OSD Planes and 2 Video Planes.
>>
>> VPP: Video Processing Unit
>> --------------------------
>>
>> The Video Processing Unit is in charge if the scaling and blending of the
>> various planes into a single pixel stream.
>> There is a special "pre-blending" used by the video planes with a dedicated
>> scaler and a "post-blending" to merge with the OSD Planes.
>> The OSD planes also have a dedicated scaler for one of the OSD.
>>
>> VENC: Video Encoders
>> --------------------
>>
>> The VENC is composed of the multiple pixel encoders :
>> - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
>> - ENCP : Progressive Video Encoder for HDMI
>> - ENCL : LCD LVDS Encoder
>> The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and clock
>> tree and provides the scanout clock to the VPP and VIU.
>> The ENCI is connected to a single VDAC for Composite Output.
>> The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
>>
>> This driver is a DRM/KMS driver using the following DRM components :
>> - GEM-CMA
>> - PRIME-CMA
>> - Atomic Modesetting
>> - FBDev-CMA
>>
>> For the following SoCs :
>> - GXBB Family (S905)
>> - GXL Family (S905X, S905D)
>> - GXM Family (S912)
>>
>> The current driver only supports the CVBS PAL/NTSC output modes, but the
>> CRTC/Planes management should support bigger modes.
>> But Advanced Colorspace Conversion, Scaling and HDMI Modes will be added in
>> a second time.
>>
>> The Device Tree bindings makes use of the endpoints video interface definitions
>> to connect to the optional CVBS and in the future the HDMI Connector nodes.
>>
>> HDMI Support is planned for a next release.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>
> Few small comments below, but looks reasonable overall. Once you have acks
> for the DT part pls submit the entire series as a pull request to Dave
> Airlie (with an additional patch to add a MAINTAINERS entry).
Thanks for the review.
Ok, will add the MAINTAINERS entry.
>
> Cheers, Daniel
>
>> ---
>> drivers/gpu/drm/Kconfig | 2 +
>> drivers/gpu/drm/Makefile | 1 +
>> drivers/gpu/drm/meson/Kconfig | 8 +
>> drivers/gpu/drm/meson/Makefile | 5 +
>> drivers/gpu/drm/meson/meson_canvas.c | 96 +++
>> drivers/gpu/drm/meson/meson_canvas.h | 31 +
>> drivers/gpu/drm/meson/meson_crtc.c | 176 ++++
>> drivers/gpu/drm/meson/meson_crtc.h | 34 +
>> drivers/gpu/drm/meson/meson_cvbs.c | 293 +++++++
>> drivers/gpu/drm/meson/meson_cvbs.h | 32 +
>> drivers/gpu/drm/meson/meson_drv.c | 383 +++++++++
>> drivers/gpu/drm/meson/meson_drv.h | 68 ++
>> drivers/gpu/drm/meson/meson_plane.c | 150 ++++
>> drivers/gpu/drm/meson/meson_plane.h | 32 +
>> drivers/gpu/drm/meson/meson_registers.h | 1395 +++++++++++++++++++++++++++++++
>> drivers/gpu/drm/meson/meson_vclk.c | 169 ++++
>> drivers/gpu/drm/meson/meson_vclk.h | 36 +
>> drivers/gpu/drm/meson/meson_venc.c | 286 +++++++
>> drivers/gpu/drm/meson/meson_venc.h | 77 ++
>> drivers/gpu/drm/meson/meson_viu.c | 497 +++++++++++
>> drivers/gpu/drm/meson/meson_viu.h | 37 +
>> drivers/gpu/drm/meson/meson_vpp.c | 189 +++++
>> drivers/gpu/drm/meson/meson_vpp.h | 43 +
>> 23 files changed, 4040 insertions(+)
>> create mode 100644 drivers/gpu/drm/meson/Kconfig
>> create mode 100644 drivers/gpu/drm/meson/Makefile
>> create mode 100644 drivers/gpu/drm/meson/meson_canvas.c
>> create mode 100644 drivers/gpu/drm/meson/meson_canvas.h
>> create mode 100644 drivers/gpu/drm/meson/meson_crtc.c
>> create mode 100644 drivers/gpu/drm/meson/meson_crtc.h
>> create mode 100644 drivers/gpu/drm/meson/meson_cvbs.c
>> create mode 100644 drivers/gpu/drm/meson/meson_cvbs.h
>> create mode 100644 drivers/gpu/drm/meson/meson_drv.c
>> create mode 100644 drivers/gpu/drm/meson/meson_drv.h
>> create mode 100644 drivers/gpu/drm/meson/meson_plane.c
>> create mode 100644 drivers/gpu/drm/meson/meson_plane.h
>> create mode 100644 drivers/gpu/drm/meson/meson_registers.h
>> create mode 100644 drivers/gpu/drm/meson/meson_vclk.c
>> create mode 100644 drivers/gpu/drm/meson/meson_vclk.h
>> create mode 100644 drivers/gpu/drm/meson/meson_venc.c
>> create mode 100644 drivers/gpu/drm/meson/meson_venc.h
>> create mode 100644 drivers/gpu/drm/meson/meson_viu.c
>> create mode 100644 drivers/gpu/drm/meson/meson_viu.h
>> create mode 100644 drivers/gpu/drm/meson/meson_vpp.c
>> create mode 100644 drivers/gpu/drm/meson/meson_vpp.h
>>
[...]
>> +
>> +static void meson_crtc_atomic_begin(struct drm_crtc *crtc,
>> + struct drm_crtc_state *state)
>> +{
>> + struct meson_crtc *meson_crtc = to_meson_crtc(crtc);
>> + unsigned long flags;
>> +
>> + if (crtc->state->event) {
>> + WARN_ON(drm_crtc_vblank_get(crtc) != 0);
>> +
>> + spin_lock_irqsave(&crtc->dev->event_lock, flags);
>> + meson_crtc->event = crtc->state->event;
>> + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>> + crtc->state->event = NULL;
>
> If you set this to NULL here
>> + }
>> +}
>> +
>> +static void meson_crtc_atomic_flush(struct drm_crtc *crtc,
>> + struct drm_crtc_state *old_crtc_state)
>> +{
>> + struct meson_crtc *meson_crtc = to_meson_crtc(crtc);
>> + struct drm_pending_vblank_event *event = crtc->state->event;
>> +
>> + if (meson_crtc->priv->viu.osd1_enabled)
>> + meson_crtc->priv->viu.osd1_commit = true;
>> +
>> + if (event) {
>> + crtc->state->event = NULL;
>> +
>> + spin_lock_irq(&crtc->dev->event_lock);
>> + if (drm_crtc_vblank_get(crtc) == 0)
>> + drm_crtc_arm_vblank_event(crtc, event);
>> + else
>> + drm_crtc_send_vblank_event(crtc, event);
>> + spin_unlock_irq(&crtc->dev->event_lock);
>> + }
>
> This here becomes dead code. And indeed it is, since you have your own
> special crtc/vblank irq handling code right below. Please remove to avoid
> confusion.
Ok, will clarify.
>
>> +}
>> +
>> +static const struct drm_crtc_helper_funcs meson_crtc_helper_funcs = {
>> + .enable = meson_crtc_enable,
>> + .disable = meson_crtc_disable,
>> + .atomic_begin = meson_crtc_atomic_begin,
>> + .atomic_flush = meson_crtc_atomic_flush,
>> +};
>> +
>> +void meson_crtc_irq(struct meson_drm *priv)
>> +{
>> + struct meson_crtc *meson_crtc = to_meson_crtc(priv->crtc);
>> + unsigned long flags;
>> +
>> + meson_viu_sync_osd1(priv);
>> +
>> + drm_crtc_handle_vblank(priv->crtc);
>> +
>> + spin_lock_irqsave(&priv->drm->event_lock, flags);
>> + if (meson_crtc->event) {
>> + drm_crtc_send_vblank_event(priv->crtc, meson_crtc->event);
>> + drm_crtc_vblank_put(priv->crtc);
>> + meson_crtc->event = NULL;
>> + }
>> + spin_unlock_irqrestore(&priv->drm->event_lock, flags);
>> +}
>> +
[...]
>> +
>> +static int meson_cvbs_encoder_atomic_check(struct drm_encoder *encoder,
>> + struct drm_crtc_state *crtc_state,
>> + struct drm_connector_state *conn_state)
>> +{
>> + return 0;
>> +}
>
> Dummy atomic_check isn't needed, pls remove.
OK, with your following comments, will replace with a proper mode check here.
>> +
>> +static void meson_cvbs_encoder_disable(struct drm_encoder *encoder)
>> +{
>> + struct meson_cvbs *meson_cvbs = encoder_to_meson_cvbs(encoder);
>> +
>> + meson_venci_cvbs_disable(meson_cvbs->priv);
>> +}
>> +
>> +static void meson_cvbs_encoder_enable(struct drm_encoder *encoder)
>> +{
>> + struct meson_cvbs *meson_cvbs = encoder_to_meson_cvbs(encoder);
>> +
>> + meson_venci_cvbs_enable(meson_cvbs->priv);
>> +}
>
> Personally I'd remove the indirection above, more direct code is easier to
> read.
I understand, I'll maybe change the meson_venci_cvbs_XXable to be directly added to the ops.
I want to keep the registers setup in separate files and keep a clean DRM/HW separation.
>> +
>> +static void meson_cvbs_encoder_mode_set(struct drm_encoder *encoder,
>> + struct drm_display_mode *mode,
>> + struct drm_display_mode *adjusted_mode)
>> +{
>> + struct meson_cvbs *meson_cvbs = encoder_to_meson_cvbs(encoder);
>> + int i;
>> +
>> + drm_mode_debug_printmodeline(mode);
>> +
>> + for (i = 0; i < ARRAY_SIZE(meson_cvbs_modes); ++i) {
>> + struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>> +
>> + if (drm_mode_equal(mode, &meson_mode->mode)) {
>> + meson_cvbs->mode = meson_mode;
>> +
>> + meson_venci_cvbs_mode_set(meson_cvbs->priv,
>> + meson_mode->enci);
>> + break;
>> + }
>> + }
>> +}
>
> What happens if userspace sets a mode you don't have? I guess you do need
> a real atomic_check, so you can return -EINVAL if that's the case ;-)
Will add a proper atomic_check !
>
>> +
>> +static const struct drm_encoder_helper_funcs meson_cvbs_encoder_helper_funcs = {
>> + .atomic_check = meson_cvbs_encoder_atomic_check,
>> + .disable = meson_cvbs_encoder_disable,
>> + .enable = meson_cvbs_encoder_enable,
>> + .mode_set = meson_cvbs_encoder_mode_set,
>> +};
>> +
>> +/* Connector */
>> +
>> +static void meson_cvbs_connector_destroy(struct drm_connector *connector)
>> +{
>> + drm_connector_cleanup(connector);
>> +}
>> +
>> +static enum drm_connector_status
>> +meson_cvbs_connector_detect(struct drm_connector *connector, bool force)
>> +{
>
> FIXME: Implement load-detect?
Actually it's not implemented anywhere on Amlogic SoCs, will add a FIXME comment here !
>
>> + return connector_status_connected;
>> +}
>> +
>> +static int meson_cvbs_connector_get_modes(struct drm_connector *connector)
>> +{
>> + struct drm_device *dev = connector->dev;
>> + struct drm_display_mode *mode;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(meson_cvbs_modes); ++i) {
>> + struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>> +
>> + mode = drm_mode_duplicate(dev, &meson_mode->mode);
>> + if (!mode) {
>> + DRM_ERROR("Failed to create a new display mode\n");
>> + return 0;
>> + }
>> +
>> + drm_mode_probed_add(connector, mode);
>> + }
>> +
>> + return i;
>> +}
>> +
>> +static int meson_cvbs_connector_mode_valid(struct drm_connector *connector,
>> + struct drm_display_mode *mode)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(meson_cvbs_modes); ++i) {
>> + struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>> +
>> + if (drm_mode_equal(mode, &meson_mode->mode))
>> + return MODE_OK;
>> + }
>> +
>> + return MODE_BAD;
>> +}
>
> Ok, this is confusion, but I thought the docs explain this. mode_valid is
> only to validate the modes added in get_modes. This is useful for outputs
> which add modes from an EDID, but not in this case. Having a mode_valid
> unfortunately doesn't ensure that these modes will be rejected in a
> modeset, for that you need a separate mode_fixup or atomic_check on the
> encoder.
>
> It's a bit a long-standing issue, but not entirely non-trivial to fix up:
> In the general case the atomic_check is for a specific configuration,
> whereaas mode_valid must only filter modes that won't work in any
> configuration. For display blocks with lots of shared resources there's a
> big difference between the two.
>
> Please double-check the kerneldoc for all these hooks, and if this is not
> clearly enough explained for you then pls raise this (or even better,
> submit at patch).
OK, for now it seems quite clear, but I clearly missed the atomic_check case.
>
>> +
>> +static const struct drm_connector_funcs meson_cvbs_connector_funcs = {
>> + .dpms = drm_atomic_helper_connector_dpms,
>> + .detect = meson_cvbs_connector_detect,
>> + .fill_modes = drm_helper_probe_single_connector_modes,
>> + .destroy = meson_cvbs_connector_destroy,
>> + .reset = drm_atomic_helper_connector_reset,
>> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
[...]
>> +
>> +static int meson_drv_bind(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct meson_drm *priv;
>> + struct drm_device *drm;
>> + struct resource *res;
>> + void __iomem *regs;
>> + int ret;
>> +
>> + drm = drm_dev_alloc(&meson_driver, dev);
>> + if (IS_ERR(drm))
>> + return PTR_ERR(drm);
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv) {
>> + ret = -ENOMEM;
>> + goto free_drm;
>> + }
>> + drm->dev_private = priv;
>> + priv->drm = drm;
>> + priv->dev = dev;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "base");
>> + regs = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(regs))
>> + return PTR_ERR(regs);
>> +
>> + priv->io_base = regs;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hhi");
>> + /* Simply ioremap since it may be a shared register zone */
>> + regs = devm_ioremap(dev, res->start, resource_size(res));
>> + if (!regs)
>> + return -EADDRNOTAVAIL;
>> +
>> + priv->hhi = devm_regmap_init_mmio(dev, regs,
>> + &meson_regmap_config);
>> + if (IS_ERR(priv->hhi)) {
>> + dev_err(&pdev->dev, "Couldn't create the HHI regmap\n");
>> + return PTR_ERR(priv->hhi);
>> + }
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
>> + /* Simply ioremap since it may be a shared register zone */
>> + regs = devm_ioremap(dev, res->start, resource_size(res));
>> + if (!regs)
>> + return -EADDRNOTAVAIL;
>> +
>> + priv->dmc = devm_regmap_init_mmio(dev, regs,
>> + &meson_regmap_config);
>> + if (IS_ERR(priv->dmc)) {
>> + dev_err(&pdev->dev, "Couldn't create the DMC regmap\n");
>> + return PTR_ERR(priv->dmc);
>> + }
>> +
>> + priv->vsync_irq = platform_get_irq(pdev, 0);
>> +
>> + /* Hardware Initialization */
>> +
>> + meson_vpp_init(priv);
>> + meson_viu_init(priv);
>> + meson_venc_init(priv);
>> +
>> + drm_vblank_init(drm, 1);
>> + drm_mode_config_init(drm);
>> +
>> + /* Components Initialization */
>> +
>> + ret = component_bind_all(drm->dev, drm);
>> + if (ret) {
>> + dev_err(drm->dev, "Couldn't bind all components\n");
>> + goto free_drm;
>> + }
>> +
>> + ret = meson_plane_create(priv);
>> + if (ret)
>> + goto free_drm;
>> +
>> + ret = meson_crtc_create(priv);
>> + if (ret)
>> + goto free_drm;
>> +
>> + ret = drm_irq_install(drm, priv->vsync_irq);
>> + if (ret)
>> + goto free_drm;
>> +
>> + drm_mode_config_reset(drm);
>> + drm->mode_config.max_width = 8192;
>> + drm->mode_config.max_height = 8192;
>> + drm->mode_config.funcs = &meson_mode_config_funcs;
>> +
>> + priv->fbdev = drm_fbdev_cma_init(drm, 32,
>> + drm->mode_config.num_crtc,
>> + drm->mode_config.num_connector);
>> + if (IS_ERR(priv->fbdev)) {
>> + ret = PTR_ERR(priv->fbdev);
>> + goto free_drm;
>> + }
>> +
>> + drm_kms_helper_poll_init(drm);
>> +
>> + ret = drm_dev_register(drm, 0);
>> + if (ret)
>> + goto free_drm;
>> +
>> + platform_set_drvdata(pdev, priv);
>
> You need this before the drm_dev_register call I think.
Would be cleaner indeed.
>> +
>> + return 0;
>> +
>> +free_drm:
>> + drm_dev_unref(drm);
>> +
>> + return ret;
>> +}
>> +
[...]
>> +
>> +static int meson_plane_atomic_check(struct drm_plane *plane,
>> + struct drm_plane_state *state)
>> +{
>> + struct drm_rect src = {
>> + .x1 = state->src_x,
>> + .y1 = state->src_y,
>> + .x2 = state->src_x + state->src_w,
>> + .y2 = state->src_y + state->src_h,
>> + };
>> + struct drm_rect dest = {
>> + .x1 = state->crtc_x,
>> + .y1 = state->crtc_y,
>> + .x2 = state->crtc_x + state->crtc_w,
>> + .y2 = state->crtc_y + state->crtc_h,
>> + };
>> +
>> + if (state->fb) {
>> + int ret;
>> +
>> + ret = drm_rect_calc_hscale(&src, &dest,
>> + DRM_PLANE_HELPER_NO_SCALING,
>> + DRM_PLANE_HELPER_NO_SCALING);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = drm_rect_calc_vscale(&src, &dest,
>> + DRM_PLANE_HELPER_NO_SCALING,
>> + DRM_PLANE_HELPER_NO_SCALING);
>> + if (ret < 0)
>> + return ret;
>> + }
>
> drm_plane_helper_check_state gives you the above in less code.
Ok
>
>> +
>> + return 0;
>> +}
>> +
>> +static void meson_plane_atomic_update(struct drm_plane *plane,
>> + struct drm_plane_state *old_state)
>> +{
>> + struct meson_plane *meson_plane = to_meson_plane(plane);
>> +
>> + /*
>> + * Update Coordinates
>> + * Update Formats
>> + * Update Buffer
>> + * Enable Plane
>> + */
>> + meson_viu_update_osd1(meson_plane->priv, plane);
>> + meson_canvas_update_osd1_buffer(meson_plane->priv, plane);
>> +}
>> +
[...]
>> +
>> +void meson_viu_update_osd1(struct meson_drm *priv, struct drm_plane *plane)
>> +{
>> + struct drm_plane_state *state = plane->state;
>> + struct drm_framebuffer *fb = state->fb;
>> + struct drm_rect src = {
>> + .x1 = (state->src_x),
>> + .y1 = (state->src_y),
>> + .x2 = (state->src_x + state->src_w),
>> + .y2 = (state->src_y + state->src_h),
>> + };
>> + struct drm_rect dest = {
>> + .x1 = state->crtc_x,
>> + .y1 = state->crtc_y,
>> + .x2 = state->crtc_x + state->crtc_w,
>> + .y2 = state->crtc_y + state->crtc_h,
>> + };
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&priv->drm->event_lock, flags);
>> +
>> + /* Enable OSD and BLK0, set max global alpha */
>> + priv->viu.osd1_ctrl_stat = OSD_ENABLE |
>> + (0xFF << OSD_GLOBAL_ALPHA_SHIFT) |
>> + OSD_BLK0_ENABLE;
>> +
>> + /* Set up BLK0 to point to the right canvas */
>> + priv->viu.osd1_blk0_cfg[0] = ((MESON_CANVAS_ID_OSD1 << OSD_CANVAS_SEL) |
>> + OSD_ENDIANNESS_LE);
>> +
>> + /* On GXBB, Use the old non-HDR RGB2YUV converter */
>> + if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_OUTPUT_COLOR_RGB;
>> +
>> + switch (fb->pixel_format) {
>> + case DRM_FORMAT_XRGB8888:
>> + /* For XRGB, replace the pixel's alpha by 0xFF */
>> + writel_bits_relaxed(OSD_REPLACE_EN, OSD_REPLACE_EN,
>> + priv->io_base + _REG(VIU_OSD1_CTRL_STAT2));
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_32 |
>> + OSD_COLOR_MATRIX_32_ARGB;
>> + break;
>> + case DRM_FORMAT_ARGB8888:
>> + /* For ARGB, use the pixel's alpha */
>> + writel_bits_relaxed(OSD_REPLACE_EN, 0,
>> + priv->io_base + _REG(VIU_OSD1_CTRL_STAT2));
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_32 |
>> + OSD_COLOR_MATRIX_32_ARGB;
>> + break;
>> + case DRM_FORMAT_RGB888:
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_24 |
>> + OSD_COLOR_MATRIX_24_RGB;
>> + break;
>> + case DRM_FORMAT_RGB565:
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_16 |
>> + OSD_COLOR_MATRIX_16_RGB565;
>> + break;
>> + };
>> +
>> + if (state->crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) {
>> + priv->viu.osd1_interlace = true;
>> +
>> + dest.y1 /= 2;
>> + dest.y2 /= 2;
>> + } else {
>> + priv->viu.osd1_interlace = true;
>> + meson_vpp_disable_interlace_vscaler_osd1(priv);
>> + }
>> +
>> + /*
>> + * The format of these registers is (x2 << 16 | x1),
>> + * where x2 is exclusive.
>> + * e.g. +30x1920 would be (1919 << 16) | 30
>> + */
>> + priv->viu.osd1_blk0_cfg[1] = ((fixed16_to_int(src.x2) - 1) << 16) |
>> + fixed16_to_int(src.x1);
>> + priv->viu.osd1_blk0_cfg[2] = ((fixed16_to_int(src.y2) - 1) << 16) |
>> + fixed16_to_int(src.y1);
>> + priv->viu.osd1_blk0_cfg[3] = ((dest.x2 - 1) << 16) | dest.x1;
>> + priv->viu.osd1_blk0_cfg[4] = ((dest.y2 - 1) << 16) | dest.y1;
>> +
>> + spin_unlock_irqrestore(&priv->drm->event_lock, flags);
>> +}
>> +
>> +void meson_viu_sync_osd1(struct meson_drm *priv)
>> +{
>> + /* Update the OSD registers */
>> + if (priv->viu.osd1_enabled && priv->viu.osd1_commit) {
>> + writel_relaxed(priv->viu.osd1_ctrl_stat,
>> + priv->io_base + _REG(VIU_OSD1_CTRL_STAT));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[0],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W0));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[1],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W1));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[2],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W2));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[3],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W3));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[4],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W4));
>> +
>> + if (priv->viu.osd1_interlace) {
>> + struct drm_plane *plane = priv->primary_plane;
>> + struct drm_plane_state *state = plane->state;
>> + struct drm_rect dest = {
>> + .x1 = state->crtc_x,
>> + .y1 = state->crtc_y,
>> + .x2 = state->crtc_x + state->crtc_w,
>> + .y2 = state->crtc_y + state->crtc_h,
>> + };
>> +
>> + meson_vpp_setup_interlace_vscaler_osd1(priv, &dest);
>> + }
>> +
>> + meson_vpp_enable_osd1(priv);
>> +
>> + priv->viu.osd1_commit = false;
>> + }
>> +}
>
> Again I'd remove the indirection and for these put them into your plane
> implementation directly.
OK, I'll make them callable from the DRM ops directly.
>
>> +
>> +
>> +/* OSD csc defines */
>> +
>> +enum viu_matrix_sel_e {
>> + VIU_MATRIX_OSD_EOTF = 0,
>> + VIU_MATRIX_OSD,
>> +};
>> +
>> +enum viu_lut_sel_e {
>> + VIU_LUT_OSD_EOTF = 0,
>> + VIU_LUT_OSD_OETF,
>> +};
>> +
>> +#define COEFF_NORM(a) ((int)((((a) * 2048.0) + 1) / 2))
>> +#define MATRIX_5X3_COEF_SIZE 24
>> +
>> +#define EOTF_COEFF_NORM(a) ((int)((((a) * 4096.0) + 1) / 2))
>> +#define EOTF_COEFF_SIZE 10
>> +#define EOTF_COEFF_RIGHTSHIFT 1
>> +
>> +static int RGB709_to_YUV709l_coeff[MATRIX_5X3_COEF_SIZE] = {
>> + 0, 0, 0, /* pre offset */
>> + COEFF_NORM(0.181873), COEFF_NORM(0.611831), COEFF_NORM(0.061765),
>> + COEFF_NORM(-0.100251), COEFF_NORM(-0.337249), COEFF_NORM(0.437500),
>> + COEFF_NORM(0.437500), COEFF_NORM(-0.397384), COEFF_NORM(-0.040116),
>> + 0, 0, 0, /* 10'/11'/12' */
>> + 0, 0, 0, /* 20'/21'/22' */
>> + 64, 512, 512, /* offset */
>> + 0, 0, 0 /* mode, right_shift, clip_en */
>> +};
>> +
>> +/* eotf matrix: bypass */
>> +static int eotf_bypass_coeff[EOTF_COEFF_SIZE] = {
>> + EOTF_COEFF_NORM(1.0), EOTF_COEFF_NORM(0.0), EOTF_COEFF_NORM(0.0),
>> + EOTF_COEFF_NORM(0.0), EOTF_COEFF_NORM(1.0), EOTF_COEFF_NORM(0.0),
>> + EOTF_COEFF_NORM(0.0), EOTF_COEFF_NORM(0.0), EOTF_COEFF_NORM(1.0),
>> + EOTF_COEFF_RIGHTSHIFT /* right shift */
>> +};
>> +
>> +void meson_viu_set_osd_matrix(struct meson_drm *priv,
>> + enum viu_matrix_sel_e m_select,
>> + int *m, bool csc_on)
>> +{
>> + if (m_select == VIU_MATRIX_OSD) {
>> + /* osd matrix, VIU_MATRIX_0 */
>> + writel(((m[0] & 0xfff) << 16) | (m[1] & 0xfff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_PRE_OFFSET0_1));
>> + writel(m[2] & 0xfff,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_PRE_OFFSET2));
>> + writel(((m[3] & 0x1fff) << 16) | (m[4] & 0x1fff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COEF00_01));
>> + writel(((m[5] & 0x1fff) << 16) | (m[6] & 0x1fff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COEF02_10));
>> + writel(((m[7] & 0x1fff) << 16) | (m[8] & 0x1fff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COEF11_12));
>> + writel(((m[9] & 0x1fff) << 16) | (m[10] & 0x1fff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COEF20_21));
>> +
>> + if (m[21]) {
>> + writel(((m[11] & 0x1fff) << 16) | (m[12] & 0x1fff),
>> + priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COEF22_30));
>> + writel(((m[13] & 0x1fff) << 16) | (m[14] & 0x1fff),
>> + priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COEF31_32));
>> + writel(((m[15] & 0x1fff) << 16) | (m[16] & 0x1fff),
>> + priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COEF40_41));
>> + writel(m[17] & 0x1fff, priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42));
>> + } else
>> + writel((m[11] & 0x1fff) << 16, priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COEF22_30));
>> +
>> + writel(((m[18] & 0xfff) << 16) | (m[19] & 0xfff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_OFFSET0_1));
>> + writel(m[20] & 0xfff,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_OFFSET2));
>> +
>> + writel_bits_relaxed(3 << 30, m[21] << 30,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42));
>> + writel_bits_relaxed(7 << 16, m[22] << 16,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42));
>> +
>> + /* 23 reserved for clipping control */
>> + writel_bits_relaxed(BIT(0), csc_on ? BIT(0) : 0,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_CTRL));
>> + writel_bits_relaxed(BIT(1), 0,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_CTRL));
>> + } else if (m_select == VIU_MATRIX_OSD_EOTF) {
>> + int i;
>> +
>> + /* osd eotf matrix, VIU_MATRIX_OSD_EOTF */
>> + for (i = 0; i < 5; i++)
>> + writel(((m[i * 2] & 0x1fff) << 16) |
>> + (m[i * 2 + 1] & 0x1fff), priv->io_base +
>> + _REG(VIU_OSD1_EOTF_CTL + i + 1));
>> +
>> + writel_bits_relaxed(BIT(30), csc_on ? BIT(30) : 0,
>> + priv->io_base + _REG(VIU_OSD1_EOTF_CTL));
>> + writel_bits_relaxed(BIT(31), csc_on ? BIT(31) : 0,
>> + priv->io_base + _REG(VIU_OSD1_EOTF_CTL));
>> + }
>> +}
>> +
>> +#define OSD_EOTF_LUT_SIZE 33
>> +#define OSD_OETF_LUT_SIZE 41
>> +
>> +void meson_viu_set_osd_lut(struct meson_drm *priv, enum viu_lut_sel_e lut_sel,
>> + unsigned int *r_map, unsigned int *g_map,
>> + unsigned int *b_map,
>> + bool csc_on)
>> +{
>> + unsigned int addr_port;
>> + unsigned int data_port;
>> + unsigned int ctrl_port;
>> + int i;
>> +
>> + if (lut_sel == VIU_LUT_OSD_EOTF) {
>> + addr_port = VIU_OSD1_EOTF_LUT_ADDR_PORT;
>> + data_port = VIU_OSD1_EOTF_LUT_DATA_PORT;
>> + ctrl_port = VIU_OSD1_EOTF_CTL;
>> + } else if (lut_sel == VIU_LUT_OSD_OETF) {
>> + addr_port = VIU_OSD1_OETF_LUT_ADDR_PORT;
>> + data_port = VIU_OSD1_OETF_LUT_DATA_PORT;
>> + ctrl_port = VIU_OSD1_OETF_CTL;
>> + } else
>> + return;
>> +
>> + if (lut_sel == VIU_LUT_OSD_OETF) {
>> + writel(0, priv->io_base + _REG(addr_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(r_map[i * 2] | (r_map[i * 2 + 1] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + writel(r_map[OSD_OETF_LUT_SIZE - 1] | (g_map[0] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(g_map[i * 2 + 1] | (g_map[i * 2 + 2] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(b_map[i * 2] | (b_map[i * 2 + 1] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + writel(b_map[OSD_OETF_LUT_SIZE - 1],
>> + priv->io_base + _REG(data_port));
>> +
>> + if (csc_on)
>> + writel_bits_relaxed(0x7 << 29, 7 << 29,
>> + priv->io_base + _REG(ctrl_port));
>> + else
>> + writel_bits_relaxed(0x7 << 29, 0,
>> + priv->io_base + _REG(ctrl_port));
>> + } else if (lut_sel == VIU_LUT_OSD_EOTF) {
>> + writel(0, priv->io_base + _REG(addr_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(r_map[i * 2] | (r_map[i * 2 + 1] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + writel(r_map[OSD_EOTF_LUT_SIZE - 1] | (g_map[0] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(g_map[i * 2 + 1] | (g_map[i * 2 + 2] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(b_map[i * 2] | (b_map[i * 2 + 1] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + writel(b_map[OSD_EOTF_LUT_SIZE - 1],
>> + priv->io_base + _REG(data_port));
>> +
>> + if (csc_on)
>> + writel_bits_relaxed(7 << 27, 7 << 27,
>> + priv->io_base + _REG(ctrl_port));
>> + else
>> + writel_bits_relaxed(7 << 27, 0,
>> + priv->io_base + _REG(ctrl_port));
>> +
>> + writel_bits_relaxed(BIT(31), BIT(31),
>> + priv->io_base + _REG(ctrl_port));
>> + }
>> +}
>> +
>> +/* eotf lut: linear */
>> +static unsigned int eotf_33_linear_mapping[OSD_EOTF_LUT_SIZE] = {
>> + 0x0000, 0x0200, 0x0400, 0x0600,
>> + 0x0800, 0x0a00, 0x0c00, 0x0e00,
>> + 0x1000, 0x1200, 0x1400, 0x1600,
>> + 0x1800, 0x1a00, 0x1c00, 0x1e00,
>> + 0x2000, 0x2200, 0x2400, 0x2600,
>> + 0x2800, 0x2a00, 0x2c00, 0x2e00,
>> + 0x3000, 0x3200, 0x3400, 0x3600,
>> + 0x3800, 0x3a00, 0x3c00, 0x3e00,
>> + 0x4000
>> +};
>> +
>> +/* osd oetf lut: linear */
>> +static unsigned int oetf_41_linear_mapping[OSD_OETF_LUT_SIZE] = {
>> + 0, 0, 0, 0,
>> + 0, 32, 64, 96,
>> + 128, 160, 196, 224,
>> + 256, 288, 320, 352,
>> + 384, 416, 448, 480,
>> + 512, 544, 576, 608,
>> + 640, 672, 704, 736,
>> + 768, 800, 832, 864,
>> + 896, 928, 960, 992,
>> + 1023, 1023, 1023, 1023,
>> + 1023
>> +};
>
> Might be fun to expose this using the color manager stuff, see
> drm_crtc_enable_color_mgmt().
Yes, I'll use them when the HDMI stuff lands ! This will certainly need such helpers !
>> +
>> +static void meson_viu_load_matrix(struct meson_drm *priv)
>> +{
>> + /* eotf lut bypass */
>> + meson_viu_set_osd_lut(priv, VIU_LUT_OSD_EOTF,
>> + eotf_33_linear_mapping, /* R */
>> + eotf_33_linear_mapping, /* G */
>> + eotf_33_linear_mapping, /* B */
>> + false);
>> +
>> + /* eotf matrix bypass */
>> + meson_viu_set_osd_matrix(priv, VIU_MATRIX_OSD_EOTF,
>> + eotf_bypass_coeff,
>> + false);
>> +
>> + /* oetf lut bypass */
>> + meson_viu_set_osd_lut(priv, VIU_LUT_OSD_OETF,
>> + oetf_41_linear_mapping, /* R */
>> + oetf_41_linear_mapping, /* G */
>> + oetf_41_linear_mapping, /* B */
>> + false);
>> +
>> + /* osd matrix RGB709 to YUV709 limit */
>> + meson_viu_set_osd_matrix(priv, VIU_MATRIX_OSD,
>> + RGB709_to_YUV709l_coeff,
>> + true);
>> +}
>> +
Neil
^ permalink raw reply
* [RFC PATCH 3/3] dt-bindings: display: add Amlogic Meson DRM Bindings
From: Laurent Pinchart @ 2016-11-28 9:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9f32e3bd-531b-0be7-8579-3af52469c421@baylibre.com>
Hi Neil,
On Monday 28 Nov 2016 10:23:43 Neil Armstrong wrote:
> On 11/28/2016 09:33 AM, Laurent Pinchart wrote:
> > On Friday 25 Nov 2016 17:03:11 Neil Armstrong wrote:
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >>
> >> .../bindings/display/meson/meson-drm.txt | 134 +++++++++++++++
> >> 1 file changed, 134 insertions(+)
> >> create mode 100644
> >>
> >> Documentation/devicetree/bindings/display/meson/meson-drm.txt
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> >> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file
> >> mode 100644
> >> index 0000000..89c1b5f
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> >> @@ -0,0 +1,134 @@
> >> +Amlogic Meson Display Controller
> >> +================================
> >> +
> >> +The Amlogic Meson Display controller is composed of several components
> >> +that are going to be documented below:
> >> +
> >> +DMC|---------------VPU (Video Processing Unit)------------|------
> >> HHI------|
> >> + | vd1 _______ _____________ _____________ |
> >> |
> >> +D |-------| |----| | | | | HDMI PLL
> >> |
> >> +D | vd2 | VIU | | Video Post | | Video Encs |<---|-----VCLK
> >> |
> >> +R |-------| |----| Processing | | | |
> >> |
> >> + | osd2 | | | |---| Enci ------|----|-----
> >> VDAC------|
> >> +R |-------| CSC |----| Scalers | | Encp ------|----|----HDMI-
> >> TX----|
> >> +A | osd1 | | | Blenders | |
> >> Encl-------|----|---------------|
> >> +M |-------|______|----|____________| |____________| |
> >> |
> >> +___|______________________________________________________|____________
> >> ___|
> >> +
> >> +
> >> +VIU: Video Input Unit
> >> +---------------------
> >> +
> >> +The Video Input Unit is in charge of the pixel scanout from the DDR
> >> memory.
> >> +It fetches the frames addresses, stride and parameters from the "Canvas"
> >> memory.
> >> +This part is also in charge of the CSC (Colorspace Conversion).
> >> +It can handle 2 OSD Planes and 2 Video Planes.
> >> +
> >> +VPP: Video Processing Unit
> >
> > Do you mean "Video Post Processing" ? In your diagram above Video
> > Processing Unit is abbreviated VPU and covers the VIU, VPP and encoders.
>
> Exact, I meant VPP here.
>
> >> +--------------------------
> >> +
> >> +The Video Processing Unit is in charge if the scaling and blending of
> >> the
> >> +various planes into a single pixel stream.
> >> +There is a special "pre-blending" used by the video planes with a
> >> dedicated
> >> +scaler and a "post-blending" to merge with the OSD Planes.
> >> +The OSD planes also have a dedicated scaler for one of the OSD.
> >> +
> >> +VENC: Video Encoders
> >> +--------------------
> >> +
> >> +The VENC is composed of the multiple pixel encoders :
> >> + - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
> >> + - ENCP : Progressive Video Encoder for HDMI
> >> + - ENCL : LCD LVDS Encoder
> >> +The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and
> >> clock
> >> +tree and provides the scanout clock to the VPP and VIU.
> >> +The ENCI is connected to a single VDAC for Composite Output.
> >> +The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
> >> +
> >> +Device Tree Bindings:
> >> +---------------------
> >> +
> >> +VPU: Video Processing Unit
> >> +--------------------------
> >> +
> >> +Required properties:
> >> + - compatible: value should be different for each SoC family as :
> >> + - GXBB (S905) : "amlogic,meson-gxbb-vpu"
> >> + - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
> >> + - GXM (S912) : "amlogic,meson-gxm-vpu"
> >> + followed by the common "amlogic,meson-gx-vpu"
> >> + - reg: base address and size of he following memory-mapped regions :
> >> + - vpu
> >> + - hhi
> >> + - dmc
> >> + - reg-names: should contain the names of the previous memory regions
> >> + - interrupts: should contain the VENC Vsync interrupt number
> >> +
> >> +- ports: A ports node with endpoint definitions as defined in
> >> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
> >> + second port should be the output endpoints for VENC connectors.
> >> +
> >> +VENC CBVS Output
> >> +----------------------
> >> +
> >> +The VENC can output Composite/CVBS output via a decicated VDAC.
> >> +
> >> +Required properties:
> >> + - compatible: value must be one of:
> >> + - compatible: value should be different for each SoC family as :
> > One of those two lines is redundant.
>
> Will fix.
>
> >> + - GXBB (S905) : "amlogic,meson-gxbb-venc-cvbs"
> >> + - GXL (S905X, S905D) : "amlogic,meson-gxl-venc-cvbs"
> >> + - GXM (S912) : "amlogic,meson-gxm-venc-cvbs"
> >> + followed by the common "amlogic,meson-gx-venc-cvbs"
> >> +
> >
> > No registers ? Are the encoders registers part of the VPU register space,
> > intertwined in a way that they can't be specified separately here ?
>
> Exact, all the video registers on the Amlogic SoC are part of a long history
> of fixup/enhance from very old SoCs, it's quite hard to distinguish a Venc
> registers array since they are mixed with the multiple encoders
> registers...
In that case is there really a reason to model the encoders as separate nodes
in DT ?
> The only separate registers are the VDAC and HDMI PHY, I may move them to
> these separate nodes since they are part of the HHI register space.
>
> It is a problem if I move them in the next release ? Next release will
> certainly have HDMI support, and will have these refactorings.
Given that DT bindings are considered as a stable ABI, I'm afraid it's an
issue.
> >> +- ports: A ports node with endpoint definitions as defined in
> >> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
> >> + first port should be the input endpoints, connected ot the VPU node.
> >> +
> >> +Example:
> >> +
> >> +venc_cvbs: venc-cvbs {
> >> + compatible = "amlogic,meson-gxbb-venc-cvbs";
> >> + status = "okay";
> >> +
> >> + ports {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + enc_cvbs_in: port at 0 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + reg = <0>;
> >> +
> >> + venc_cvbs_in_vpu: endpoint at 0 {
> >> + reg = <0>;
> >> + remote-endpoint = <&vpu_out_venc_cvbs>;
> >> + };
> >> + };
> >> + };
> >> +};
> >> +
> >> +vpu: vpu at d0100000 {
> >> + compatible = "amlogic,meson-gxbb-vpu";
> >> + reg = <0x0 0xd0100000 0x0 0x100000>,
> >> + <0x0 0xc883c000 0x0 0x1000>,
> >> + <0x0 0xc8838000 0x0 0x1000>;
> >> + reg-names = "base", "hhi", "dmc";
> >> + interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
> >> +
> >> + ports {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + vpu_out: port at 1 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + reg = <1>;
> >> +
> >> + vpu_out_venc_cvbs: endpoint at 0 {
> >> + reg = <0>;
> >> + remote-endpoint = <&venc_cvbs_in_vpu>;
> >> + };
> >> + };
> >> + };
> >> +};
>
> Thanks for the review !
--
Regards,
Laurent Pinchart
^ 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