[PATCH 3/3] ARM: bcm2835: Add VC4 to the device tree.

[Stephen Warren]

On 03/08/2016 11:04 AM, Eric Anholt wrote:
> Stephen Warren  writes:
>
>> On 03/04/2016 01:32 PM, Eric Anholt wrote:
>>> VC4 is the GPU (display and 3D) present on the 283x.
>>
>>> diff --git a/arch/arm/boot/dts/bcm2835-rpi-a-plus.dts 
>>> b/arch/arm/boot/dts/bcm2835-rpi-a-plus.dts
>>
>>> + {
>>> +   hpd-gpios = < 46 GPIO_ACTIVE_LOW>;
>>> +};
>>
>> Isn't that the same everywhere? If so, adding it to bcm2835-rpi.dtsi
>> seems like a better idea; it'd avoid duplicating it everywhere.
>
> It's not the same everywhere (_HIGH vs _LOW), which is why it's in the
> individual files.

Oh right, it looks OK then. The series,
Acked-by: Stephen Warren 

One could reduce the duplication by moving the common block into the 
common .dtsi file, but using a board-defined #define for the polarity:

bcm2835-rpi-a-plus.dts:

#define RPI_HDMI_HPD_POLARITY
#include "bcm2835-rpi.dtsi"

bcm2835-rpi.dtsi:

 {
hpd-gpios = < 46 RPI_HDMI_HPD_POLARITY>;
};

... although this case is so tiny I'm not sure there's any benefit 
trying to unify it like that right now.

[PATCH 3/3] ARM: bcm2835: Add VC4 to the device tree.

[Stephen Warren]

On 03/04/2016 01:32 PM, Eric Anholt wrote:
> VC4 is the GPU (display and 3D) present on the 283x.

> diff --git a/arch/arm/boot/dts/bcm2835-rpi-a-plus.dts 
> b/arch/arm/boot/dts/bcm2835-rpi-a-plus.dts

> + {
> + hpd-gpios = < 46 GPIO_ACTIVE_LOW>;
> +};

Isn't that the same everywhere? If so, adding it to bcm2835-rpi.dtsi 
seems like a better idea; it'd avoid duplicating it everywhere.

bcm2835 (Raspberry Pi) KMS driver

[Stephen Warren]

On 10/11/2015 06:39 AM, Stefan Wahren wrote:
> Am 09.10.2015 um 23:27 schrieb Eric Anholt:
>> This is a respin of the Raspberry Pi KMS series.  Now that we've got a
>> real clock driver, I can actually set new video modes.  Also in this
>> version, most of the custom DT stuff from before is gone, thanks to
>> finding exynos's platform_driver component matching code (I have sent
>> separate patches to drivers/base to make helpers for doing it).
>>
>> https://github.com/anholt/linux/tree/vc4-kms-squash-2
>
> I want to point out that git format-patch could prepare a nice cover
> letter and usually the changelog should go there.

Well, I guess you could put it there, but that wouldn't remove the need 
to put the changelog in the individual patches too, so that reviewers 
don't have to switch back and forth between different messages just to 
find out what changed in each patch.

+1 on sending the cover letter using git format-patch/send-email 
thoughl; the threading here is a little odd.

[PATCH v2 7/7] ARM: bcm2835: Add VC4 to the device tree.

[Stephen Warren]

On 08/18/2015 03:54 PM, Eric Anholt wrote:
> VC4 is the GPU (display and 3D) present on the 2835.

This patch and patch 1 seem OK to me, although I'll withhold any ack
until the DT binding design discussion with Rob has been resolved. I
haven't looked at the OF graph bindings he mentioned so have no clue
about his suggestions.

[PATCH v2 6/7] ARM: bcm2835: Add the DDC I2C controller to the device tree.

[Stephen Warren]

On 08/18/2015 03:54 PM, Eric Anholt wrote:
> We need to use it for getting video modes over HDMI.

This patch,
Acked-by: Stephen Warren 

[PATCH 7/7] ARM: bcm2835: Add VC4 to the device tree.

[Stephen Warren]

On 08/12/2015 06:56 PM, Eric Anholt wrote:
> Signed-off-by: Eric Anholt 

Patch description?


> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi

>   arm-pmu {
>   compatible = "arm,arm1176-pmu";
>   };
> +
> + hdmi: brcm,vc4-hdmi at 7e902000 {

It'd be nice to keep all the DT nodes with a reg property together, and
sorted in reg order.

As before, I think any DT node for a HW block that may-or-may-not-be
used based on board connectivity/features should be disabled in the SoC
.dtsi file, and enabled in the board's DT file if the feature is useful
for that board.

[PATCH 6/7] ARM: bcm2835: Add the DDC I2C controller to the device tree.

[Stephen Warren]

On 08/12/2015 06:56 PM, Eric Anholt wrote:
> We need to use it for getting video modes over HDMI.

> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi

> + i2c2: i2c at 7e805000 {
> + compatible = "brcm,bcm2835-i2c";
> + reg = <0x7e805000 0x1000>;
> + interrupts = <2 21>;
> + clocks = <_i2c>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };

In an SoC .dtsi file, you'd typically write:

status = "disabled";

... in all nodes that represent IO controllers that interface to
external HW, so that board DT files can/must explicitly choose to enable
the device if it's actually in use on the board. Some systems might not
have HDMI and hence might not hook up the HDMI_SCL/SDA pads.

BCM2835-ARM-Peripherals.pdf states "Note that the BSC2 master is used
dedicated with the HDMI interface and should not be accessed by user
programs.". Does this imply the Linux kernel shouldn't be touching this
I2C controller; that the VC4 firmware might also be attempting to use
it? I wonder how any such sharing of the HW would work.

[PATCH 3/7] drm/vc4: Add KMS support for Raspberry Pi.

[Stephen Warren]

On 08/12/2015 06:56 PM, Eric Anholt wrote:
> This is the start of a full VC4 driver.  Right now this just supports
> configuring the display using a pre-existing video mode (because
> changing the pixel clock isn't available yet, and doesn't work when it
> is).  However, this is enough for fbcon and bringing up X using
> xf86-video-modesetting.

> diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig

> +config DRM_VC4
> + tristate "Broadcom VC4 Graphics"

> + help
> +   Choose this option if you have a system that has a Broadcom
> +   VC4 GPU, such as the Raspberry Pi or other BCM2708/BCM2835.
> +
> +   This driver requires that "avoid_warnings=2" be present in
> +   the config.txt for the firmware, to keep it from smashing
> +   our display setup.

The need for "avoid_warnings=2" seems like it will trip people up. I
don't think it's in any config.txt I've seen. Can you expand more on that?

[PATCH 2/7] MAINTAINERS: Add myself for the new VC4 (RPi GPU) graphics driver.

[Stephen Warren]

On 08/12/2015 06:56 PM, Eric Anholt wrote:

> diff --git a/MAINTAINERS b/MAINTAINERS

> +DRM DRIVERS FOR VC4
> +M:   Eric Anholt 
> +T:   git git://github.com/anholt/linux
> +S:   Maintained
> +F:   drivers/gpu/drm/vc4/*

S: Supported

?

[PATCH 1/7] drm/vc4: Add devicetree bindings for VC4.

[Stephen Warren]

On 08/12/2015 06:56 PM, Eric Anholt wrote:
> Signed-off-by: Eric Anholt 

This one definitely needs a patch description, since someone might not
know what a VC4 is, and "git log" won't show the text from the binding
doc itself. I'd suggest adding the initial paragraph of the binding doc
as the patch description, or more.

> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt 
> b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt

> +Required properties for VC4:
> +- compatible:Should be "brcm,vc4"
> +- crtcs: List of references to pixelvalve scanout engines

s/references to/phandles of/ would be more typical DT language.

> +- hvss:  List of references to HVS video scalers
> +- encoders:  List of references to output encoders (HDMI, SDTV)

Would it make sense to make all those nodes child node of the vc4
object. That way, there's no need to have these lists of objects; they
can be automatically built up as the DT is enumerated. I know that e.g.
the NVIDIA Tegra host1x binding works this way, and I think it may have
been inspired by other similar cases.

Of course, this is only appropriate if the HW modules really are
logically children of the VC4 HW module. Perhaps they aren't. If they
aren't though, I wonder what this "vc4" module actually represents in HW?

> +Required properties for HDMI
> +- compatible:Should be "brcm,vc4-hdmi"
> +- reg:   Physical base address and length of the two register 
> ranges
> +   ("HDMI" and "HD")

I'd add "in that order" right before ")".

> +Example:
> +/ {
> + soc {

Minor nit: Examples often don't include any nodes "above" the nodes
whose bindings are being documented.

[PATCH v5 00/11] Improvements to Tegra-based Chromebook support

[Stephen Warren]

On 02/12/2015 01:50 AM, Tomeu Vizoso wrote:
> Hello,
>
> this series adds support for the Tegra-based HP Chromebook 14 (aka nyan
> blaze), which is very similar to the Acer Chromebook 13 (aka nyan big).
> Because they both include tegra124-nyan.dtsi, some improvements to Blaze
> support have also benefitted the Big. I have tested that USB2, the panels,
> HDMI, the trackpad, Wifi and sound work on both.
>
> The leaf DTs contain the whole pinmux configuration as generated by
> tegra-pinmux-scripts. I chose to not put the common configuration in the
> common dtsi so we can paste the output as is and be sure that the kernel
> doesn't diverge from the canonical data.

At a quick glance this series looks OK,
Acked-by: Stephen Warren 

[5/5] ARM: tegra: jetson-tk1: enable GK20A GPU

[Stephen Warren]

On 09/25/2014 10:41 AM, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 09:48:01AM -0600, Stephen Warren wrote:
>> On 09/25/2014 07:27 AM, Sjoerd Simons wrote:
>>> Playing a bit with todays linux-next on my jetson, it seems this patch is
>>> still required for enabling the GPU. Is there anything blocking it (firmware
>>> not available yet in liux-firmware?)
>>
>> I think initially I was waiting for the DRM patch "drm/nouvea: support for
>> probing platform devices" to be applied, but it looks like that's been
>> applied already, so only patches 4 and 5 in this series are still
>> outstanding.
>>
>> Alex, wasn't there also some issue where the VPR register had to be
>> programmed, and if it wasn't there'd be a hang when the GPU registers were
>> touched? If we've added code to Nouveau/tegradrm to detect that and avoid
>> the problem, then I guess we can commit these last two patches for 3.19. A
>> resend after the 3.18 merge window might help.
>
> A patch that programs VPR was merged into U-Boot (though I don't think
> it's made it into master yet). I'm not sure we can reasonably check for
> that in Nouveau, given that the register is somewhere completely
> unrelated. In fact I think the U-Boot patch was triggered by some
> discussion about how to solve this and it was decided that it shouldn't
> be done in the kernel, but U-Boot should set it up.
>
> That said, perhaps one solution would be to make U-Boot enable the gk20a
> device if it's set up the VPR and disable it otherwise?

For that to work, we'd need the DT to say status="disabled" by default 
for the GPU, and for the fixed U-Boot (and indeed every other 
bootloader...) to enable the GPU node. This would allow people with old 
versions of U-Boot (or other bootloaders) to continue to boot. This 
means bootloaders would only have to set status="okay", but never have 
to set status="disabled", which at least simplifies them a tiny bit.

[5/5] ARM: tegra: jetson-tk1: enable GK20A GPU

[Stephen Warren]

On 09/25/2014 07:27 AM, Sjoerd Simons wrote:
> Playing a bit with todays linux-next on my jetson, it seems this patch is
> still required for enabling the GPU. Is there anything blocking it (firmware
> not available yet in liux-firmware?)

I think initially I was waiting for the DRM patch "drm/nouvea: support 
for probing platform devices" to be applied, but it looks like that's 
been applied already, so only patches 4 and 5 in this series are still 
outstanding.

Alex, wasn't there also some issue where the VPR register had to be 
programmed, and if it wasn't there'd be a hang when the GPU registers 
were touched? If we've added code to Nouveau/tegradrm to detect that and 
avoid the problem, then I guess we can commit these last two patches for 
3.19. A resend after the 3.18 merge window might help.

> On Mon, May 19, 2014 at 06:24:10PM +0900, Alexandre Courbot wrote:
>> Signed-off-by: Alexandre Courbot 
>> ---
>>   arch/arm/boot/dts/tegra124-jetson-tk1.dts | 8 +++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts 
>> b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
>> index e31fb61a81d3..15a194d1277f 100644
>> --- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
>> +++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
>> @@ -30,6 +30,12 @@
>>  };
>>  };
>>
>> +gpu at 0,5700 {
>> +status = "okay";
>> +
>> +vdd-supply = <_gpu>;
>> +};
>> +
>>  pinmux: pinmux at 0,7868 {
>>  pinctrl-names = "default";
>>  pinctrl-0 = <_default>;
>> @@ -1505,7 +1511,7 @@
>>  regulator-always-on;
>>  };
>>
>> -sd6 {
>> +vdd_gpu: sd6 {
>>  regulator-name = "+VDD_GPU_AP";
>>  regulator-min-microvolt = <65>;
>>  regulator-max-microvolt = <120>;
>

[PATCH V2] drm/tegra: add MODULE_DEVICE_TABLEs

[Stephen Warren]

On 06/18/2014 04:21 PM, Stephen Warren wrote:
> From: Stephen Warren 
> 
> When tegra-drm.ko is built as a module, these MODULE_DEVICE_TABLEs allow
> the module to be auto-loaded since the module will match the devices
> instantiated from device tree.
> 
> (Notes for stable: in 3.14+, just git rm any conflicting file, since they
> are added in later kernels. For 3.13 and below, manual merging will be
> needed)
> 
> Cc: 
> Signed-off-by: Stephen Warren 
> ---
> v2: Remove change to drm.c, since the match table there isn't used for
> probing.

Thierry, I think this patch got lost? It doesn't appear in next-20140721
anyway.

[PATCH 4/4] ARM: tegra: roth: add display DT node

[Stephen Warren]

On 07/02/2014 09:10 PM, Alexandre Courbot wrote:
> On 07/03/2014 12:55 AM, Stephen Warren wrote:
>> On 07/02/2014 06:19 AM, Alexandre Courbot wrote:
>>> DSI support has been fixed to support continuous clock behavior that the
>>> panel used on SHIELD requires, so finally add its device tree node since
>>> it is functional.
>>
>>> diff --git a/arch/arm/boot/dts/tegra114-roth.dts
>>> b/arch/arm/boot/dts/tegra114-roth.dts
>>
>>> +host1x at 5000 {
>>> +dsi at 5430 {
>>
>> That node looks fine, but I wonder why we need to mark a bunch of
>> regulators as always-on? shouldn't the references to vdd-supply and
>> power-supply from this node be enough? If not, perhaps the tree
>> structure of the regulators isn't correct, or the DSI or panel bindings
>> don't allow enough regulators to be referenced?
> 
> regulator-always-on is indeed not needed for vdd_lcd. I actually had a
> patch in my tree removing this line that I forgot to squash. Will post a
> v2 for this patch that fixes this, thanks.
> 
> vdd-2v8-display needs to remain always-on however. Here we may hit a
> limitation of the simple-panel driver, where only one power supply can
> be provided.

Can't we fix the simple-panel driver to allow a list of regulators in
the property?

I suppose that this technique is OK though; we can always simplify the
DT later if the simple-panel binding gets enhanced.

[PATCH v3 0/3] drm/nouveau: support for probing platform devices

[Stephen Warren]

On 07/02/2014 03:09 AM, Alexandre Courbot wrote:
> On Thu, Jun 26, 2014 at 2:33 PM, Alexandre Courbot  
> wrote:
>> This series adds support for probing platform devices on Nouveau, as well as
>> the DT bindings for GK20A. It doesn't enable the GPU yet on Tegra boards 
>> since
>> a few extra things need to be supported before that.
>>
>> This version is mostly identical to v2 but fixes an important issue: the 
>> drvdata
>> must be set to the drm_device for sysfs to work, so the platform device
>> structure now includes the nouveau_device flattened into it to let us compute
>> the address of one from the other. Since the platform device resources 
>> (clocks,
>> regulators, ...) need to live longer than the nouveau_device, they are stored
>> into their own structure which is allocated separately.
...
> Stephen, Thierry,
> 
> Ben has merged the first patch of this series into the Nouveau tree.
> Could you get patches 2 and 3 through the Tegra tree? Thanks!

I've applied patches 2 and 3 to Tegra's for-3.17/dt branch.

[PATCH 4/4] ARM: tegra: roth: add display DT node

[Stephen Warren]

On 07/02/2014 06:19 AM, Alexandre Courbot wrote:
> DSI support has been fixed to support continuous clock behavior that the
> panel used on SHIELD requires, so finally add its device tree node since
> it is functional.

> diff --git a/arch/arm/boot/dts/tegra114-roth.dts 
> b/arch/arm/boot/dts/tegra114-roth.dts

> + host1x at 5000 {
> + dsi at 5430 {

That node looks fine, but I wonder why we need to mark a bunch of
regulators as always-on? shouldn't the references to vdd-supply and
power-supply from this node be enough? If not, perhaps the tree
structure of the regulators isn't correct, or the DSI or panel bindings
don't allow enough regulators to be referenced?

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Stephen Warren]

On 06/18/2014 05:14 PM, Thierry Reding wrote:
> On Wed, Jun 18, 2014 at 04:09:06PM -0600, Stephen Warren wrote:
>> On 06/18/2014 04:03 PM, Thierry Reding wrote:
...
>>> From what I remember, Mike was fairly strongly opposing the idea of
>>> virtual clocks, but what you're proposing here sounds like it would
>>> assume the existence of virtual clocks. clk_set_rate() per client
>>> doesn't work with the current API as I understand it.
>>>
>>> Or perhaps what you're proposing isn't about the individual clocks at
>>> all but rather about a mechanism to express constraints for a set of
>>> clocks?
>>
>> This doesn't have anything to do with virtual clocks. As you mention,
>> it's just about constraints.
>>
>> One user of clock "cpu" wants min rate 216MHz. Another wants max rate
>> 1GHz. cpufreq will request some rate between the 2, or be capped to
>> those limits. These set of imposed constraints would need to be stored
>> per client of the clock, not per HW clock, since many clients could set
>> different max rates (e.g. thermal throttle 1.5GHz due to temperature,
>> CPU policy 1GHz due to the user selecting low CPU power, etc.)
>>
>> Similarly for audio, of there are N clients of 1 clock/PLL, and they
>> each want the PLL to run at a different rate, something needs to detect
>> that and deny it.
> 
> I'm wondering how this should work with the current API. Could the clock
> core be modified to return a per-client struct clk * that references the
> hardware clock internally? Or do we need to add a new API?

I would assume the we can just change struct clk and hide the details
from any driver. Hopefully only clock-core and clock-drivers would need
any changes.

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140618/e86187fa/attachment.sig>

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Stephen Warren]

On 06/18/2014 04:19 PM, St?phane Marchesin wrote:
> On Wed, Jun 18, 2014 at 3:00 PM, Thierry Reding
>  wrote:
>> On Wed, Jun 18, 2014 at 07:23:47PM +0200, Tomeu Vizoso wrote:
>>> On 06/17/2014 06:15 PM, Stephen Warren wrote:
>>>> On 06/17/2014 06:16 AM, Tomeu Vizoso wrote:
>>>>> On 06/16/2014 10:02 PM, Stephen Warren wrote:
>>>>>> On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
>>>>>>> +#ifdef CONFIG_TEGRA124_EMC
>>>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
>>>>>>> long rate);
>>>>>>> +void tegra124_emc_set_floor(unsigned long freq);
>>>>>>> +void tegra124_emc_set_ceiling(unsigned long freq);
>>>>>>> +#else
>>>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
>>>>>>> long rate)
>>>>>>> +{ return -ENODEV; }
>>>>>>> +void tegra124_emc_set_floor(unsigned long freq)
>>>>>>> +{ return; }
>>>>>>> +void tegra124_emc_set_ceiling(unsigned long freq)
>>>>>>> +{ return; }
>>>>>>> +#endif
>>>>>>
>>>>>> I'll repeat what I said off-list so that we can have the whole
>>>>>> conversation on the list:
>>>>>>
>>>>>> That looks like a custom Tegra-specific API. I think it'd be much better
>>>>>> to integrate this into the common clock framework as a standard clock
>>>>>> constraints API. There are other use-cases for clock constraints besides
>>>>>> EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
>>>>>> SoCs too).
>>>>>
>>>>> Yes, I wrote a bit in the cover letter about our requirements and how
>>>>> they map to the CCF. Could you please comment on that?
>>>>
>>>> My comments remain the same. I believe this is something that belongs in
>>>> the clock driver, or at the least, some API that takes a struct clock as
>>>> its parameter, so that drivers can use the existing DT clock lookup
>>>> mechanism.
>>>
>>> Ok, let me put this strawman here to see if I have gotten close to what you
>>> have in mind:
>>>
>>> * add per-client accounting (Rabin's patches referenced before)
>>>
>>> * add clk_set_floor, to be used by cpufreq, load stats, etc.
>>>
>>> * add clk_set_ceiling, to be used by battery drivers, thermal, etc.
>>>
>>> * an EMC driver would collect bandwidth and latency requests from consumers
>>> and call clk_set_floor on the EMC clock.
>>>
>>> * the EMC driver would also register for rate change notifications in the
>>> EMC clock and would update the latency allowance registers at that point.
>>
>> Latency allowance registers are part of the MC rather than the EMC. So I
>> think we have two options: a) have a unified driver for MC and EMC or b)
>> provide two parts of the API in two drivers.
>>
>> Or perhaps c), create a generic framework that both MC and EMC can
>> register with (bandwidth for EMC, latency for MC).
> 
> Is there any motivation for keeping MC and EMC separate? In my mind,
> the solution was always to handle those together.

Well, they are documented as being separate HW modules in the TRM.

I know there's an interlock in HW so that when the EMC clock is changed,
the EMC registers can flip atomically to a new configuration.

I'm not aware of any similar HW interlock between MC and EMC registers.
That would imply that very tight co-ordination shouldn't be required.

Do the MC latency allowance registers /really/ need to be *very tightly*
managed whenever the EMC clock is changed, or is it just a matter of it
being a good idea to update EMC clock and MC latency allowance registers
at roughly the same time? Even if there's some co-ordination required,
maybe it can be handled rather like cpufreq notifications; use clock
pre-rate change notifications to set MC up in a way that'll work at both
old/new EMC clocks, and then clock post-rate notifications to the final
MC configuration?

[PATCH V2] drm/tegra: add MODULE_DEVICE_TABLEs

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

When tegra-drm.ko is built as a module, these MODULE_DEVICE_TABLEs allow
the module to be auto-loaded since the module will match the devices
instantiated from device tree.

(Notes for stable: in 3.14+, just git rm any conflicting file, since they
are added in later kernels. For 3.13 and below, manual merging will be
needed)

Cc: 
Signed-off-by: Stephen Warren 
---
v2: Remove change to drm.c, since the match table there isn't used for
probing.
---
 drivers/gpu/drm/tegra/dc.c| 1 +
 drivers/gpu/drm/tegra/dpaux.c | 1 +
 drivers/gpu/drm/tegra/dsi.c   | 1 +
 drivers/gpu/drm/tegra/gr2d.c  | 1 +
 drivers/gpu/drm/tegra/gr3d.c  | 1 +
 drivers/gpu/drm/tegra/hdmi.c  | 1 +
 drivers/gpu/drm/tegra/sor.c   | 1 +
 7 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index ef40381f3909..48c3bc460eef 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1303,6 +1303,7 @@ static const struct of_device_id tegra_dc_of_match[] = {
/* sentinel */
}
 };
+MODULE_DEVICE_TABLE(of, tegra_dc_of_match);

 static int tegra_dc_parse_dt(struct tegra_dc *dc)
 {
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 3f132e356e9c..708f783ead47 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -382,6 +382,7 @@ static const struct of_device_id tegra_dpaux_of_match[] = {
{ .compatible = "nvidia,tegra124-dpaux", },
{ },
 };
+MODULE_DEVICE_TABLE(of, tegra_dpaux_of_match);

 struct platform_driver tegra_dpaux_driver = {
.driver = {
diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index bd56f2affa78..97c409f10456 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -982,6 +982,7 @@ static const struct of_device_id tegra_dsi_of_match[] = {
{ .compatible = "nvidia,tegra114-dsi", },
{ },
 };
+MODULE_DEVICE_TABLE(of, tegra_dsi_of_match);

 struct platform_driver tegra_dsi_driver = {
.driver = {
diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
index 7c53941f2a9e..02cd3e37a6ec 100644
--- a/drivers/gpu/drm/tegra/gr2d.c
+++ b/drivers/gpu/drm/tegra/gr2d.c
@@ -121,6 +121,7 @@ static const struct of_device_id gr2d_match[] = {
{ .compatible = "nvidia,tegra20-gr2d" },
{ },
 };
+MODULE_DEVICE_TABLE(of, gr2d_match);

 static const u32 gr2d_addr_regs[] = {
GR2D_UA_BASE_ADDR,
diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index 30f5ba9bd6d0..2bea2b2d204e 100644
--- a/drivers/gpu/drm/tegra/gr3d.c
+++ b/drivers/gpu/drm/tegra/gr3d.c
@@ -130,6 +130,7 @@ static const struct of_device_id tegra_gr3d_match[] = {
{ .compatible = "nvidia,tegra20-gr3d" },
{ }
 };
+MODULE_DEVICE_TABLE(of, tegra_gr3d_match);

 static const u32 gr3d_addr_regs[] = {
GR3D_IDX_ATTRIBUTE( 0),
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index a0b8d8539d07..84ea0c8b47f7 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -1370,6 +1370,7 @@ static const struct of_device_id tegra_hdmi_of_match[] = {
{ .compatible = "nvidia,tegra20-hdmi", .data = _hdmi_config },
{ },
 };
+MODULE_DEVICE_TABLE(of, tegra_hdmi_of_match);

 static int tegra_hdmi_probe(struct platform_device *pdev)
 {
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 27c979b50111..061a5c501124 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1455,6 +1455,7 @@ static const struct of_device_id tegra_sor_of_match[] = {
{ .compatible = "nvidia,tegra124-sor", },
{ },
 };
+MODULE_DEVICE_TABLE(of, tegra_sor_of_match);

 struct platform_driver tegra_sor_driver = {
.driver = {
-- 
1.8.1.5

[PATCH] drm/tegra: add MODULE_DEVICE_TABLEs

[Stephen Warren]

On 06/18/2014 03:51 PM, Thierry Reding wrote:
> On Wed, Jun 18, 2014 at 03:19:15PM -0600, Stephen Warren wrote:
>> From: Stephen Warren 
>>
>> When tegra-drm.ko is built as a module, these MODULE_DEVICE_TABLEs allow
>> the module to be auto-loaded since the module will match the devices
>> instantiated from device tree.
> 
> I vaguely remember doing something like this a while back and getting a
> bunch of link-time errors. But I assume that you've tested this, so I
> must be remembering wrongly.

Were the problems due to:

a) Simply building the tegradrm driver as modules.

I vaguely recall some runtime issues with tegradrm as a module, but I'm
not sure about build issues. I don't think this patch could make this
any worse.

b) Building as modules works, but adding MODULE_DEVICE_TABLE broke that.

This seems unlikely since *many* module in the kernel have a
MODULE_DEVICE_TABLE...

Certainly, with this patch applied, building tegradrm as a module in
next-20140611 works out just fine, and the code runs fine too. Building
tegra_defconfig (which has tegradrm builtin) on Linus' master with this
patch applied also works out fine.

I'll post v2 with the issue you mentioned addressed.

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140618/12b77b6c/attachment.sig>

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Stephen Warren]

On 06/18/2014 04:03 PM, Thierry Reding wrote:
> On Wed, Jun 18, 2014 at 11:46:49AM -0600, Stephen Warren wrote:
>> On 06/18/2014 11:23 AM, Tomeu Vizoso wrote:
>>> On 06/17/2014 06:15 PM, Stephen Warren wrote:
>>>> On 06/17/2014 06:16 AM, Tomeu Vizoso wrote:
>>>>> On 06/16/2014 10:02 PM, Stephen Warren wrote:
>>>>>> On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
>>>>>>> +#ifdef CONFIG_TEGRA124_EMC
>>>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
>>>>>>> long rate);
>>>>>>> +void tegra124_emc_set_floor(unsigned long freq);
>>>>>>> +void tegra124_emc_set_ceiling(unsigned long freq);
>>>>>>> +#else
>>>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
>>>>>>> long rate)
>>>>>>> +{ return -ENODEV; }
>>>>>>> +void tegra124_emc_set_floor(unsigned long freq)
>>>>>>> +{ return; }
>>>>>>> +void tegra124_emc_set_ceiling(unsigned long freq)
>>>>>>> +{ return; }
>>>>>>> +#endif
>>>>>>
>>>>>> I'll repeat what I said off-list so that we can have the whole
>>>>>> conversation on the list:
>>>>>>
>>>>>> That looks like a custom Tegra-specific API. I think it'd be much
>>>>>> better
>>>>>> to integrate this into the common clock framework as a standard clock
>>>>>> constraints API. There are other use-cases for clock constraints
>>>>>> besides
>>>>>> EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
>>>>>> SoCs too).
>>>>>
>>>>> Yes, I wrote a bit in the cover letter about our requirements and how
>>>>> they map to the CCF. Could you please comment on that?
>>>>
>>>> My comments remain the same. I believe this is something that belongs in
>>>> the clock driver, or at the least, some API that takes a struct clock as
>>>> its parameter, so that drivers can use the existing DT clock lookup
>>>> mechanism.
>>>
>>> Ok, let me put this strawman here to see if I have gotten close to what
>>> you have in mind:
>>>
>>> * add per-client accounting (Rabin's patches referenced before)
>>>
>>> * add clk_set_floor, to be used by cpufreq, load stats, etc.
>>>
>>> * add clk_set_ceiling, to be used by battery drivers, thermal, etc.
>>
>> Yes. I'd expect those to be maintained per-client, and so the clock core
>> (or whatever higher level code implements clk_set_floor/ceiling)
>> performs the logic that "blends" together all the different requests
>> from different clients.
>>
>> As an aside, for audio usage, I would expect clk_set_rate to be a
>> per-client (rather than per HW clock) operation too, and to error out if
>> one client says it wants to set pll_a to the rate needed for
>> 44.1KHz-based audio and a different client wants the rate for
>> 48KHz-based audio.
> 
> From what I remember, Mike was fairly strongly opposing the idea of
> virtual clocks, but what you're proposing here sounds like it would
> assume the existence of virtual clocks. clk_set_rate() per client
> doesn't work with the current API as I understand it.
> 
> Or perhaps what you're proposing isn't about the individual clocks at
> all but rather about a mechanism to express constraints for a set of
> clocks?

This doesn't have anything to do with virtual clocks. As you mention,
it's just about constraints.

One user of clock "cpu" wants min rate 216MHz. Another wants max rate
1GHz. cpufreq will request some rate between the 2, or be capped to
those limits. These set of imposed constraints would need to be stored
per client of the clock, not per HW clock, since many clients could set
different max rates (e.g. thermal throttle 1.5GHz due to temperature,
CPU policy 1GHz due to the user selecting low CPU power, etc.)

Similarly for audio, of there are N clients of 1 clock/PLL, and they
each want the PLL to run at a different rate, something needs to detect
that and deny it.


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140618/c955e82b/attachment.sig>

[PATCH] drm/tegra: add MODULE_DEVICE_TABLEs

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

When tegra-drm.ko is built as a module, these MODULE_DEVICE_TABLEs allow
the module to be auto-loaded since the module will match the devices
instantiated from device tree.

(Notes for stable: in 3.14+, just git rm any conflicting file, since they
are added in later kernels. For 3.13 and below, manual merging will be
needed)

Cc: 
Signed-off-by: Stephen Warren 
---
 drivers/gpu/drm/tegra/dc.c| 1 +
 drivers/gpu/drm/tegra/dpaux.c | 1 +
 drivers/gpu/drm/tegra/drm.c   | 1 +
 drivers/gpu/drm/tegra/dsi.c   | 1 +
 drivers/gpu/drm/tegra/gr2d.c  | 1 +
 drivers/gpu/drm/tegra/gr3d.c  | 1 +
 drivers/gpu/drm/tegra/hdmi.c  | 1 +
 drivers/gpu/drm/tegra/sor.c   | 1 +
 8 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index ef40381f3909..48c3bc460eef 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1303,6 +1303,7 @@ static const struct of_device_id tegra_dc_of_match[] = {
/* sentinel */
}
 };
+MODULE_DEVICE_TABLE(of, tegra_dc_of_match);

 static int tegra_dc_parse_dt(struct tegra_dc *dc)
 {
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 3f132e356e9c..708f783ead47 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -382,6 +382,7 @@ static const struct of_device_id tegra_dpaux_of_match[] = {
{ .compatible = "nvidia,tegra124-dpaux", },
{ },
 };
+MODULE_DEVICE_TABLE(of, tegra_dpaux_of_match);

 struct platform_driver tegra_dpaux_driver = {
.driver = {
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 3396f9f6a9f7..a1f9b06a75d5 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -694,6 +694,7 @@ static const struct of_device_id host1x_drm_subdevs[] = {
{ .compatible = "nvidia,tegra124-hdmi", },
{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, host1x_drm_subdevs);

 static struct host1x_driver host1x_drm_driver = {
.name = "drm",
diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index bd56f2affa78..97c409f10456 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -982,6 +982,7 @@ static const struct of_device_id tegra_dsi_of_match[] = {
{ .compatible = "nvidia,tegra114-dsi", },
{ },
 };
+MODULE_DEVICE_TABLE(of, tegra_dsi_of_match);

 struct platform_driver tegra_dsi_driver = {
.driver = {
diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
index 7c53941f2a9e..02cd3e37a6ec 100644
--- a/drivers/gpu/drm/tegra/gr2d.c
+++ b/drivers/gpu/drm/tegra/gr2d.c
@@ -121,6 +121,7 @@ static const struct of_device_id gr2d_match[] = {
{ .compatible = "nvidia,tegra20-gr2d" },
{ },
 };
+MODULE_DEVICE_TABLE(of, gr2d_match);

 static const u32 gr2d_addr_regs[] = {
GR2D_UA_BASE_ADDR,
diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index 30f5ba9bd6d0..2bea2b2d204e 100644
--- a/drivers/gpu/drm/tegra/gr3d.c
+++ b/drivers/gpu/drm/tegra/gr3d.c
@@ -130,6 +130,7 @@ static const struct of_device_id tegra_gr3d_match[] = {
{ .compatible = "nvidia,tegra20-gr3d" },
{ }
 };
+MODULE_DEVICE_TABLE(of, tegra_gr3d_match);

 static const u32 gr3d_addr_regs[] = {
GR3D_IDX_ATTRIBUTE( 0),
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index ba067bb767e3..ffe26547328d 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -1450,6 +1450,7 @@ static const struct of_device_id tegra_hdmi_of_match[] = {
{ .compatible = "nvidia,tegra20-hdmi", .data = _hdmi_config },
{ },
 };
+MODULE_DEVICE_TABLE(of, tegra_hdmi_of_match);

 static int tegra_hdmi_probe(struct platform_device *pdev)
 {
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 27c979b50111..061a5c501124 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1455,6 +1455,7 @@ static const struct of_device_id tegra_sor_of_match[] = {
{ .compatible = "nvidia,tegra124-sor", },
{ },
 };
+MODULE_DEVICE_TABLE(of, tegra_sor_of_match);

 struct platform_driver tegra_sor_driver = {
.driver = {
-- 
1.8.1.5

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Stephen Warren]

On 06/18/2014 11:23 AM, Tomeu Vizoso wrote:
> On 06/17/2014 06:15 PM, Stephen Warren wrote:
>> On 06/17/2014 06:16 AM, Tomeu Vizoso wrote:
>>> On 06/16/2014 10:02 PM, Stephen Warren wrote:
>>>> On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
>>>>> +#ifdef CONFIG_TEGRA124_EMC
>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
>>>>> long rate);
>>>>> +void tegra124_emc_set_floor(unsigned long freq);
>>>>> +void tegra124_emc_set_ceiling(unsigned long freq);
>>>>> +#else
>>>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
>>>>> long rate)
>>>>> +{ return -ENODEV; }
>>>>> +void tegra124_emc_set_floor(unsigned long freq)
>>>>> +{ return; }
>>>>> +void tegra124_emc_set_ceiling(unsigned long freq)
>>>>> +{ return; }
>>>>> +#endif
>>>>
>>>> I'll repeat what I said off-list so that we can have the whole
>>>> conversation on the list:
>>>>
>>>> That looks like a custom Tegra-specific API. I think it'd be much
>>>> better
>>>> to integrate this into the common clock framework as a standard clock
>>>> constraints API. There are other use-cases for clock constraints
>>>> besides
>>>> EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
>>>> SoCs too).
>>>
>>> Yes, I wrote a bit in the cover letter about our requirements and how
>>> they map to the CCF. Could you please comment on that?
>>
>> My comments remain the same. I believe this is something that belongs in
>> the clock driver, or at the least, some API that takes a struct clock as
>> its parameter, so that drivers can use the existing DT clock lookup
>> mechanism.
> 
> Ok, let me put this strawman here to see if I have gotten close to what
> you have in mind:
> 
> * add per-client accounting (Rabin's patches referenced before)
> 
> * add clk_set_floor, to be used by cpufreq, load stats, etc.
> 
> * add clk_set_ceiling, to be used by battery drivers, thermal, etc.

Yes. I'd expect those to be maintained per-client, and so the clock core
(or whatever higher level code implements clk_set_floor/ceiling)
performs the logic that "blends" together all the different requests
from different clients.

As an aside, for audio usage, I would expect clk_set_rate to be a
per-client (rather than per HW clock) operation too, and to error out if
one client says it wants to set pll_a to the rate needed for
44.1KHz-based audio and a different client wants the rate for
48KHz-based audio.

> * an EMC driver would collect bandwidth and latency requests from
> consumers and call clk_set_floor on the EMC clock.
> 
> * the EMC driver would also register for rate change notifications in
> the EMC clock and would update the latency allowance registers at that
> point.
> 
> How does it sound?

At a high level, yes this sounds about right to me.

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Stephen Warren]

On 06/17/2014 06:16 AM, Tomeu Vizoso wrote:
> On 06/16/2014 10:02 PM, Stephen Warren wrote:
>> On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:

>> This binding looks quite anaemic vs.
>> Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt; I
>> would expect that this binding needs all the EMC register data from the
>> tegra20-emc binding too. Can the two bindings be identical?
> 
> There's even less stuff needed right now, as all what ultimately the EMC
> driver does is call clk_set_rate on the EMC clock. As the T124 EMC
> driver gains more features, they should get more similar.

IIRC, even changing the EMC clock rate requires modifying the memory
controller's programming (e.g. delays/taps/tuning etc.). That's exactly
what the more complex stuff in the nvidia,tegra20-emc.txt is all about.
I not convinced that a driver that just modifies the clock rate without
adjusting the EMC programming will work reliably.

>>> +#ifdef CONFIG_TEGRA124_EMC
>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
>>> long rate);
>>> +void tegra124_emc_set_floor(unsigned long freq);
>>> +void tegra124_emc_set_ceiling(unsigned long freq);
>>> +#else
>>> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned
>>> long rate)
>>> +{ return -ENODEV; }
>>> +void tegra124_emc_set_floor(unsigned long freq)
>>> +{ return; }
>>> +void tegra124_emc_set_ceiling(unsigned long freq)
>>> +{ return; }
>>> +#endif
>>
>> I'll repeat what I said off-list so that we can have the whole
>> conversation on the list:
>>
>> That looks like a custom Tegra-specific API. I think it'd be much better
>> to integrate this into the common clock framework as a standard clock
>> constraints API. There are other use-cases for clock constraints besides
>> EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
>> SoCs too).
> 
> Yes, I wrote a bit in the cover letter about our requirements and how
> they map to the CCF. Could you please comment on that?

My comments remain the same. I believe this is something that belongs in
the clock driver, or at the least, some API that takes a struct clock as
its parameter, so that drivers can use the existing DT clock lookup
mechanism.

[RFC PATCH 3/4] drm/tegra: Request memory bandwidth for the display controller

[Stephen Warren]

On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
> Request it based solely on the current mode's refresh rate. More
> accurate requirements can be requested in future patches.

> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c

> + bandwidth = mode->clock * window.bits_per_pixel / 8;
> + err = tegra124_emc_reserve_bandwidth(TEGRA_EMC_CONSUMER_DISP1, 
> bandwidth);

DISP1 shouldn't be hard-coded here; the code should use DISP1 or DISP2
based on head or DC identity. We certainly have some boards capable of
dual-head operation.

[RFC PATCH 1/4] memory: tegra124-emc: Add EMC driver

[Stephen Warren]

On 06/16/2014 07:35 AM, Tomeu Vizoso wrote:
> Adds functionality for registering memory bandwidth needs and setting
> the EMC clock rate based on that.
> 
> Also adds API for setting floor and ceiling frequency rates.

> diff --git 
> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra124-emc.txt 
> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra124-emc.txt
> new file mode 100644
> index 000..88e6a55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra124-emc.txt
> @@ -0,0 +1,26 @@
> +Tegra124 External Memory Controller
> +
> +Properties:
> +- compatible : Should contain "nvidia,tegra124-emc".
> +- reg : Should contain the register range of the device
> +- #address-cells : Should be 1
> +- #size-cells : Should be 0
> +- nvidia,mc : phandle to the mc bus connected to EMC.
> +- clocks : phandle to EMC, EMC shared bus override, and all parent clocks.
> +- clock-names : name of each clock.
> +- nvidia,pmc : phandle to the PMC syscon node.
> +- max-clock-frequency : optional, specifies the maximum EMC rate in kHz.
> +
> +Child device nodes describe the memory settings for different configurations 
> and
> +clock rates.

How do the child nodes do that? The binding needs to specify the format
of the child node. This binding looks quite anaemic vs.
Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-emc.txt; I
would expect that this binding needs all the EMC register data from the
tegra20-emc binding too. Can the two bindings be identical?

Can you explain what the nvidia,mc and nvidia,pmc references are needed
for? Hopefully, this driver isn't going to reach into those devices and
touch their registers directly.

> diff --git a/include/linux/platform_data/tegra_emc.h 
> b/include/linux/platform_data/tegra_emc.h

A header file that defines platform data format isn't the correct place
to put the definitions of public APIs. I'd expect something more like
.

> +#ifdef CONFIG_TEGRA124_EMC
> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned long 
> rate);
> +void tegra124_emc_set_floor(unsigned long freq);
> +void tegra124_emc_set_ceiling(unsigned long freq);
> +#else
> +int tegra124_emc_reserve_bandwidth(unsigned int consumer, unsigned long rate)
> +{ return -ENODEV; }
> +void tegra124_emc_set_floor(unsigned long freq)
> +{ return; }
> +void tegra124_emc_set_ceiling(unsigned long freq)
> +{ return; }
> +#endif

I'll repeat what I said off-list so that we can have the whole
conversation on the list:

That looks like a custom Tegra-specific API. I think it'd be much better
to integrate this into the common clock framework as a standard clock
constraints API. There are other use-cases for clock constraints besides
EMC scaling (e.g. some in audio on Tegra, and I'm sure many on other
SoCs too).

See https://lkml.org/lkml/2014/5/16/569 for some previous discussion on
this topic.

[PATCH 3/5] ARM: tegra: add GK20A GPU to Tegra124 DT

[Stephen Warren]

On 05/19/2014 03:24 AM, Alexandre Courbot wrote:
> From: Thierry Reding 
> 
> Add the GK20A device node to Tegra124's device tree.

At a quick glance, patches 3-5 look fine too. I'll hold off on applying
them until patches 1-2 have been applied to the DRM/... tree.

[PATCH 2/5] ARM: tegra: of: add GK20A device tree binding

[Stephen Warren]

On 05/19/2014 03:24 AM, Alexandre Courbot wrote:
> Add the device tree binding documentation for the GK20A GPU used in
> Tegra K1 SoCs.

A few minor nits, but otherwise,
Acked-by: Stephen Warren 

> diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt 
> b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt

> +Required properties:
> +- compatible: "nvidia,-"
> +  Currently recognized values:
> +  - nvidia,tegra124-gk20a
> +- reg: Physical base address and length of the controller's registers.
> +  Must contain two entries:
> +  - first entry for bar0
> +  - second entry for bar1
> +- interrupts: The interrupt outputs from the controller.

To be consistent with the clocks and resets properties, it'd be nice to
reword that as:

interrupts: Must contain an entry for each entry in interrupt-names.

> +- interrupt-names: Must include the following entries:

... and add the following here:

See ../interrupt-controller/interrupts.txt

> +/ {

No need to wrap a root node around this in the example.

> + gpu at 0,5700 {
...
> + };
> +

Extra blank line here.

> +};

[PATCH v3] drm/panel: Add support for EDT ETM0700G0DH6 and ET070080DH6 panels

[Stephen Warren]

On 05/15/2014 03:41 PM, Thierry Reding wrote:
> On Thu, May 15, 2014 at 09:54:07AM -0600, Stephen Warren wrote:
>> On 05/15/2014 04:54 AM, Thierry Reding wrote:
>>> On Thu, May 15, 2014 at 12:25:47PM +0200, Philipp Zabel wrote:
>>>> The EDT ETM0700G0DH6 and ET070080DH6 are 7" 800x480 panels,
>>>> which can be supported by the simple panel driver.
>>>>
>>>> Signed-off-by: Philipp Zabel 
>>>> ---
>>>> Changes since v2:
>>>>  - Added device tree binding documentation. Do we really want to add one 
>>>> little
>>>>file for each panel?
>>>
>>> I guess we could move all of the compatible values into simple-panel.txt
>>> but I don't see a need for that yet.
>>
>> If they aren't added there, then I believe checkpatch will complain
>> about patches that start to use the new compatible values, since they
>> won't be documented.
> 
> Added where? simple-panel.txt or in separate files per compatible?

I assume that checkpatch simply checks all files in
Documentation/devicetree/bindings. By "there", I meant "somewhere in
that directory".

Perhaps I misinterpreted the email I was replying to; I thought you'd
meant that we didn't need to document it yet, but it looks like you were
simply discussing where to document it. Sorry for the noise.

[PATCH v3] drm/panel: Add support for EDT ETM0700G0DH6 and ET070080DH6 panels

[Stephen Warren]

On 05/15/2014 04:54 AM, Thierry Reding wrote:
> On Thu, May 15, 2014 at 12:25:47PM +0200, Philipp Zabel wrote:
>> The EDT ETM0700G0DH6 and ET070080DH6 are 7" 800x480 panels,
>> which can be supported by the simple panel driver.
>>
>> Signed-off-by: Philipp Zabel 
>> ---
>> Changes since v2:
>>  - Added device tree binding documentation. Do we really want to add one 
>> little
>>file for each panel?
> 
> I guess we could move all of the compatible values into simple-panel.txt
> but I don't see a need for that yet.

If they aren't added there, then I believe checkpatch will complain
about patches that start to use the new compatible values, since they
won't be documented.

[PATCH 1/3] drm/dsi: Support device shutdown

[Stephen Warren]

On 04/29/2014 09:28 AM, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Hook up the MIPI DSI bus's .shutdown() function to allow drivers to
> implement code that should be run when a device is shut down.

The series,
Tested-by: Stephen Warren 

(On an NVIDIA Tegra Dalmore board. Now, the backlight actually turns off
when the system is shut down. Note that the backlight power on this
system is directly sourced from the AC adapter for some strange reason,
rather than passing through the main system PMIC, and hence backlight
power isn't shut off even when the rest of the system really is powered off)

[PATCH 1/3] ARM: tegra: Deprecate nvidia,hpd-gpio property

[Stephen Warren]

On 04/17/2014 06:02 AM, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Properties referencing GPIOs should use the plural suffix -gpios. This
> convention is encoded in the device tree backend of gpiod_get(), which
> we'll eventually want to migrate to.

Wouldn't it be simpler to fix the GPIO binding documentation and
gpiod_get() code to allow the -gpio suffix in addition to -gpios? It
always struck me as silly that the binding required a plural property
name when only a single entry made sense.

(For something like "clocks", since the property name applies to any
clock, and there certainly can be many clocks, a plural property name
makes sense. However, since each type of GPIO is "foo-gpios" rather than
an "foo" entry in "gpios", that same argument doesn't apply, and a
singular property name seems much more correct).

[PATCH V2] gpu: host1x: handle the correct # of syncpt regs

[Stephen Warren]

On 04/04/2014 04:31 PM, Stephen Warren wrote:
> From: Stephen Warren 
> 
> BIT_WORD() truncates rather than rounds, so the loops in
> syncpt_thresh_isr() and _host1x_intr_disable_all_syncpt_intrs() use <=
> rather than < in an attempt to process the correct number of registers
> when rounding of the conversion of count of bits to count of words is
> necessary. However, when rounding isn't necessary because the value is
> already a multiple of the divisor (as is the case for all values of
> nb_pts the code actually sees), this causes one too many registers to
> be processed.
> 
> Solve this by using and explicit DIV_ROUND_UP() call, rather than
> BIT_WORD(), and comparing with < rather than <=.

I don't see this in linux-next yet.

[PATCH] drm/tegra: replace IS_ERR and PTR_ERR with PTR_ERR_OR_ZERO

[Stephen Warren]

On 04/11/2014 02:36 AM, Duan Jiong wrote:
> This patch fixes coccinelle error regarding usage of IS_ERR and
> PTR_ERR instead of PTR_ERR_OR_ZERO.

Same comment as the other patch; I prefer the existing code (although
I'll defer to Thierry as maintainer of both these pieces of code).

It would help if the commit description quoted the coccinelle error.

[PATCH V2] gpu: host1x: handle the correct # of syncpt regs

[Stephen Warren]

On 04/07/2014 02:18 AM, Thierry Reding wrote:
> On Fri, Apr 04, 2014 at 04:31:05PM -0600, Stephen Warren wrote:
>> From: Stephen Warren 
>>
>> BIT_WORD() truncates rather than rounds, so the loops in
>> syncpt_thresh_isr() and _host1x_intr_disable_all_syncpt_intrs() use <=
>> rather than < in an attempt to process the correct number of registers
>> when rounding of the conversion of count of bits to count of words is
>> necessary. However, when rounding isn't necessary because the value is
>> already a multiple of the divisor (as is the case for all values of
>> nb_pts the code actually sees), this causes one too many registers to
>> be processed.
>>
>> Solve this by using and explicit DIV_ROUND_UP() call, rather than
>> BIT_WORD(), and comparing with < rather than <=.
>>
>> Signed-off-by: Stephen Warren 
>> ---
>> v2: Use DIV_ROUND_UP rather than BITS_TO_LONGS to avoid problems on 64-bit.
>> ---
>>  drivers/gpu/host1x/hw/intr_hw.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> If I understand correctly there's no immediate need for this to go to
> stable kernels, nor for it to be queued for 3.15, right? That is the
> potential extra write isn't causing any harm on actual hardware, is it?
> 
> In that case I'll queue this up for 3.16.

We should definitely apply this, and as far back as the code exists,
since the SW is touching non-existent registers, and that is presumably
undefined behaviour, which could potentially cause hard-to-diagnose bugs.

Besides, I want the mainline kernel to run on our simulator without
having to maintain patches for fixed issues.

[PATCH V2] gpu: host1x: handle the correct # of syncpt regs

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

BIT_WORD() truncates rather than rounds, so the loops in
syncpt_thresh_isr() and _host1x_intr_disable_all_syncpt_intrs() use <=
rather than < in an attempt to process the correct number of registers
when rounding of the conversion of count of bits to count of words is
necessary. However, when rounding isn't necessary because the value is
already a multiple of the divisor (as is the case for all values of
nb_pts the code actually sees), this causes one too many registers to
be processed.

Solve this by using and explicit DIV_ROUND_UP() call, rather than
BIT_WORD(), and comparing with < rather than <=.

Signed-off-by: Stephen Warren 
---
v2: Use DIV_ROUND_UP rather than BITS_TO_LONGS to avoid problems on 64-bit.
---
 drivers/gpu/host1x/hw/intr_hw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/host1x/hw/intr_hw.c b/drivers/gpu/host1x/hw/intr_hw.c
index db9017adfe2b..498b37e39058 100644
--- a/drivers/gpu/host1x/hw/intr_hw.c
+++ b/drivers/gpu/host1x/hw/intr_hw.c
@@ -47,7 +47,7 @@ static irqreturn_t syncpt_thresh_isr(int irq, void *dev_id)
unsigned long reg;
int i, id;

-   for (i = 0; i <= BIT_WORD(host->info->nb_pts); i++) {
+   for (i = 0; i < DIV_ROUND_UP(host->info->nb_pts, 32); i++) {
reg = host1x_sync_readl(host,
HOST1X_SYNC_SYNCPT_THRESH_CPU0_INT_STATUS(i));
for_each_set_bit(id, , BITS_PER_LONG) {
@@ -64,7 +64,7 @@ static void _host1x_intr_disable_all_syncpt_intrs(struct 
host1x *host)
 {
u32 i;

-   for (i = 0; i <= BIT_WORD(host->info->nb_pts); ++i) {
+   for (i = 0; i < DIV_ROUND_UP(host->info->nb_pts, 32); ++i) {
host1x_sync_writel(host, 0xu,
HOST1X_SYNC_SYNCPT_THRESH_INT_DISABLE(i));
host1x_sync_writel(host, 0xu,
-- 
1.8.1.5

[PATCH] gpu: host1x: handle the correct # of syncpt regs

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

BIT_WORD() truncates rather than rounds, so the loops in
syncpt_thresh_isr() and _host1x_intr_disable_all_syncpt_intrs() use <=
rather than < in an attempt to process the correct number of registers
when rounding of the conversion of count of bits to count of words is
necessary. However, when rounding isn't necessary (as is the case for
all values of nb_pts the code actually sees), this causes one too many
registers to be processed.

Solve this by using BITS_TO_LONGS() (which rounds internally), rather
than BIT_WORD(), and comparing with < rather than <=.

Signed-off-by: Stephen Warren 
---
 drivers/gpu/host1x/hw/intr_hw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/host1x/hw/intr_hw.c b/drivers/gpu/host1x/hw/intr_hw.c
index db9017adfe2b..17407b2de2bf 100644
--- a/drivers/gpu/host1x/hw/intr_hw.c
+++ b/drivers/gpu/host1x/hw/intr_hw.c
@@ -47,7 +47,7 @@ static irqreturn_t syncpt_thresh_isr(int irq, void *dev_id)
unsigned long reg;
int i, id;

-   for (i = 0; i <= BIT_WORD(host->info->nb_pts); i++) {
+   for (i = 0; i < BITS_TO_LONGS(host->info->nb_pts); i++) {
reg = host1x_sync_readl(host,
HOST1X_SYNC_SYNCPT_THRESH_CPU0_INT_STATUS(i));
for_each_set_bit(id, , BITS_PER_LONG) {
@@ -64,7 +64,7 @@ static void _host1x_intr_disable_all_syncpt_intrs(struct 
host1x *host)
 {
u32 i;

-   for (i = 0; i <= BIT_WORD(host->info->nb_pts); ++i) {
+   for (i = 0; i < BITS_TO_LONGS(host->info->nb_pts); ++i) {
host1x_sync_writel(host, 0xu,
HOST1X_SYNC_SYNCPT_THRESH_INT_DISABLE(i));
host1x_sync_writel(host, 0xu,
-- 
1.8.1.5

[PATCH] host1x: export host1x_syncpt_incr_max function

[Stephen Warren]

On 02/19/2014 03:23 PM, Bryan Wu wrote:
> Tegra V4L2 camera driver needs this function to do frame capture.

Does it need to be EXPORT_SYMBOL()d too, in case things are modules?

[PATCH 0/3] drm/tegra: Restore licensing consistency

[Stephen Warren]

On 02/12/2014 12:13 PM, Thierry Reding wrote:
> On Wed, Feb 12, 2014 at 09:46:57AM -0700, Stephen Warren wrote:
>> On 02/12/2014 12:30 AM, Thierry Reding wrote:
>>> The bulk of the Tegra DRM driver is GPL v2 licensed, but some of the new
>>> subdrivers were licensed under an MIT license. This was simply oversight
>>> on my part.
>>>
>>> One exception to this is the public header file that will eventually be
>>> used within libdrm. Since all the other headers in libdrm use the MIT
>>> license, do the same for Tegra.
>>
>> The series,
>> Acked-by: Stephen Warren 
>>
>> I guess this needs explicit acks from the following people outside
>> NVIDIA who touched these files:?
>>
>> Wei Yongjun  (dsi.c)
>> Emil Goode  (tegra_drm.h)
> 
> Does it? Those changes were mostly mechanical in nature, so I'm not sure
> if that qualifies them for having any bearing of the license of the
> file.
> 
> But if you insist I can resend with Wei and Emil on Cc.

Well, it seems simpler if you get an ack; that way it documents the
fact, and avoids any issues in the future, irrespective of whether those
changes convey (c) owderhsip or not.

I'd imagine you don't need a whole resend; just forward the email?

[PATCH 0/3] drm/tegra: Restore licensing consistency

[Stephen Warren]

On 02/12/2014 12:30 AM, Thierry Reding wrote:
> The bulk of the Tegra DRM driver is GPL v2 licensed, but some of the new
> subdrivers were licensed under an MIT license. This was simply oversight
> on my part.
> 
> One exception to this is the public header file that will eventually be
> used within libdrm. Since all the other headers in libdrm use the MIT
> license, do the same for Tegra.

The series,
Acked-by: Stephen Warren 

I guess this needs explicit acks from the following people outside
NVIDIA who touched these files:?

Wei Yongjun  (dsi.c)
Emil Goode  (tegra_drm.h)

[RFC 10/16] drm/nouveau/timer: skip calibration on GK20A

[Stephen Warren]

On 02/04/2014 01:39 AM, Alexandre Courbot wrote:
> On 02/04/2014 12:55 PM, Ben Skeggs wrote:
>> On Sat, Feb 1, 2014 at 1:16 PM, Alexandre Courbot
>>  wrote:
>>> GK20A's timer is directly attached to the system timer and cannot be
>>> calibrated. Skip the calibration phase on that chip since the
>>> corresponding registers do not exist.
>> Just a curiosity:  What timer resolution does the HW initialise at?
> 
> On T124 the timer input is the oscillator clock, which depending on the
> device can run between 12 and 48Mhz (IIUC).

On the one Tegra124 board we support upstream, the crystal is 12MHz. I
believe this is a typical/common value; almost all the Tegra boards we
support upstream run at this rate.

[PATCH v4 5/5] drm/tegra: Add eDP support

[Stephen Warren]

On 01/21/2014 12:24 PM, Thierry Reding wrote:
> Add support for eDP functionality found on Tegra124 and later SoCs. Only
> fast link training is currently supported.
> 
> Signed-off-by: Thierry Reding 
> ---
>  .../bindings/gpu/nvidia,tegra20-host1x.txt |   42 +

This part should go to the DT mailing list, although it looks fine to me.

> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c

> + * Copyright (C) 2013 NVIDIA Corporation

2014? Perhaps elsewhere too?

> +static void tegra_dpaux_write_fifo(struct tegra_dpaux *dpaux, const u8 
> *buffer,

Is that anything like writesl(); similar for
tegra_dpaux_read_fifo()/readsl()?

[PATCH v2 2/2] drm/tegra: Obtain head number from DT

[Stephen Warren]

On 01/15/2014 02:06 AM, Thierry Reding wrote:
> On Tue, Jan 14, 2014 at 10:53:19AM -0700, Stephen Warren wrote:
>> On 01/14/2014 07:45 AM, Thierry Reding wrote:
>>> The head number of a given display controller is fixed in hardware and
>>> required to program outputs appropriately. Relying on the driver probe
>>> order to determine this number will not work, since that could yield a
>>> situation where the second head was probed first and would be assigned
>>> head number 0 instead of 1.
>>>
>>> By explicitly specifying the head number in the device tree, it is no
>>> longer necessary to rely on these assumptions. As a fallback, if the
>>> property isn't available, derive the head number from the display
>>> controller node's position in the device tree. That's somewhat more
>>> reliable than the previous default but not a proper solution.

>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>
>>> +static int tegra_dc_parse_dt(struct tegra_dc *dc)
>>> +{
>>> +   struct device_node *np;
>>> +   u32 value = 0;
>>> +   int err;
>>> +
>>> +   err = of_property_read_u32(dc->dev->of_node, "nvidia,head", );
>>
>> If of_property_read_u32() returns an error, does it guarantee that value
>> is left unchanged? I suspect it'd be safer to add ...
> 
> That's the way it's always been at least. of_property_read_u32() ends up
> calling of_property_read_u32_array(), which looking at the code only
> modifies the out_values parameter when it knows that it will succeed.
> 
> Furthermore the function's kernel-doc explicitly says that "out_values
> is modified only if a valid u32 value can be decoded" (i.e. on success).

OK, that last bit is the important part. So, this is fine.

[PATCH v2 2/2] drm/tegra: Obtain head number from DT

[Stephen Warren]

On 01/14/2014 07:45 AM, Thierry Reding wrote:
> The head number of a given display controller is fixed in hardware and
> required to program outputs appropriately. Relying on the driver probe
> order to determine this number will not work, since that could yield a
> situation where the second head was probed first and would be assigned
> head number 0 instead of 1.
> 
> By explicitly specifying the head number in the device tree, it is no
> longer necessary to rely on these assumptions. As a fallback, if the
> property isn't available, derive the head number from the display
> controller node's position in the device tree. That's somewhat more
> reliable than the previous default but not a proper solution.

The series,
Tested-by: Stephen Warren 

This patch should really have been sent to the DT maintainers and list
since it changes a DT binding...

> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c

> +static int tegra_dc_parse_dt(struct tegra_dc *dc)
> +{
> + struct device_node *np;
> + u32 value = 0;
> + int err;
> +
> + err = of_property_read_u32(dc->dev->of_node, "nvidia,head", );

If of_property_read_u32() returns an error, does it guarantee that value
is left unchanged? I suspect it'd be safer to add ...

> + if (err < 0) {
> + dev_err(dc->dev, "missing \"nvidia,head\" property\n");
> +
> + /*
> +  * If the nvidia,head property isn't present, try to find the
> +  * correct head number by looking up the position of this
> +  * display controller's node within the device tree. Assuming
> +  * that the nodes are ordered properly in the DTS file and
> +  * that the translation into a flattened device tree blob
> +  * preserves that ordering this will actually yield the right
> +  * head number.
> +  *
> +  * If those assumptions don't hold, this will still work for
> +  * cases where only a single display controller is used.
> +  */

... "value = 0;" here?

> + for_each_matching_node(np, tegra_dc_of_match) {
> + if (np == dc->dev->of_node)
> + break;
> +
> + value++;
> + }
> + }
> +
> + dc->pipe = value;

[PATCH 2/2] drm/tegra: Obtain head number from DT

[Stephen Warren]

On 01/14/2014 07:14 AM, Thierry Reding wrote:
> On Mon, Jan 13, 2014 at 10:46:45AM -0700, Stephen Warren wrote:
>> On 01/13/2014 07:21 AM, Thierry Reding wrote:
>>> The head number of a given display controller is fixed in hardware and
>>> required to program outputs appropriately. Relying on the driver probe
>>> order to determine this number will not work, since that could yield a
>>> situation where the second head was probed first and would be assigned
>>> head number 0 instead of 1.
>>
>> This change makes the new properties mandatory, yet they aren't part of
>> the DT files yet. So, won't this patch break all display on Tegra?
> 
> I don't think it'll make anything worse than it currently is, since both
> display controllers can't run at the same time with the current code.

Sure it will; it will prevent any dc device from probing at all:

> +static int tegra_dc_parse_dt(struct tegra_dc *dc)
...
> + err = of_property_read_u32(dc->dev->of_node, "nvidia,head", );
> + if (err < 0)
 +  return err;
^^^
...
> @@ -1207,6 +1219,10 @@ static int tegra_dc_probe(struct platform_device *pdev)
...
> + err = tegra_dc_parse_dt(dc);
> + if (err < 0)
> + return err;
^^^

[PATCH 2/2] drm/tegra: Obtain head number from DT

[Stephen Warren]

On 01/13/2014 07:21 AM, Thierry Reding wrote:
> The head number of a given display controller is fixed in hardware and
> required to program outputs appropriately. Relying on the driver probe
> order to determine this number will not work, since that could yield a
> situation where the second head was probed first and would be assigned
> head number 0 instead of 1.

This change makes the new properties mandatory, yet they aren't part of
the DT files yet. So, won't this patch break all display on Tegra?

To avoid having to modify the Tegra DTs in this patch, can't the code
fall back to the existing broken algorithm if the property is missing, i.e.:

> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c

> - dc->pipe = tegra->drm->mode_config.num_crtc;

Instead,:

if (dc->pipe == -1)
dc->pipe = tegra->drm->mode_config.num_crtc;

> +static int tegra_dc_parse_dt(struct tegra_dc *dc)
> +{
> + u32 value;
> + int err;
> +
> + err = of_property_read_u32(dc->dev->of_node, "nvidia,head", );
> + if (err < 0)
> + return err;
> +
> + dc->pipe = value;

Instead:

err = ...
if (!err)
dc->pipe = value;
else
/* Perhaps also emit an error message here */
dc->pipe = -1;

[PATCH 1/2] drm/tegra: Fix possible CRTC mask for RGB outputs

[Stephen Warren]

On 01/13/2014 07:21 AM, Thierry Reding wrote:
> The mask of possible CRTCs that an output (DRM encoder) can be attached
> to is relative to the position within the DRM device's list of CRTCs.
> Deferred probing can cause this to not match the pipe number associated
> with a CRTC. Use the newly introduced drm_crtc_mask() to compute the
> mask by looking up the proper index of the given CRTC in the list.

> diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c

> @@ -258,7 +258,7 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct 
> tegra_dc *dc)

> - rgb->output.encoder.possible_crtcs = 1 << dc->pipe;
> + rgb->output.encoder.possible_crtcs = drm_crtc_mask(>base);

For me, on top of either next-20140109 or next-20140113, this causes:

> drivers/gpu/drm/tegra/rgb.c: In function ?tegra_dc_rgb_init?:
> drivers/gpu/drm/tegra/rgb.c:261:2: error: implicit declaration of function 
> ?drm_crtc_mask? [-Werror=implicit-function-declaration]

[PATCH] drm/panel: update EDID BLOB in panel_simple_get_modes()

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

This stashes away the EDID data so that the sysfs per-connector file
"edid" can display it. Without this change, the "edid" file is always
empty.

Signed-off-by: Stephen Warren 
---
 drivers/gpu/drm/panel/panel-simple.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 520b569ae3c8..59d52ca2c67f 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -162,6 +162,7 @@ static int panel_simple_get_modes(struct drm_panel *panel)
/* probe EDID if a DDC bus is available */
if (p->ddc) {
struct edid *edid = drm_get_edid(panel->connector, p->ddc);
+   drm_mode_connector_update_edid_property(panel->connector, edid);
if (edid) {
num += drm_add_edid_modes(panel->connector, edid);
kfree(edid);
-- 
1.8.1.5

[PATCH] drm/panel: Add support for Chungwa CLAA101WA01A panel

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

The Chungwa CLAA101WA01A is a 10.1" 1366x768 panel, which can be
supported by the simple panel driver.

Signed-off-by: Stephen Warren 
---
 .../bindings/panel/chunghwa,claa101wa01a.txt   |  7 ++
 drivers/gpu/drm/panel/panel-simple.c   | 25 ++
 2 files changed, 32 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/panel/chunghwa,claa101wa01a.txt

diff --git a/Documentation/devicetree/bindings/panel/chunghwa,claa101wa01a.txt 
b/Documentation/devicetree/bindings/panel/chunghwa,claa101wa01a.txt
new file mode 100644
index ..f24614e4d5ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/chunghwa,claa101wa01a.txt
@@ -0,0 +1,7 @@
+Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel
+
+Required properties:
+- compatible: should be "chunghwa,claa101wa01a"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index a2d5e3f1205e..d68a9635ab9e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -316,6 +316,28 @@ static const struct panel_desc auo_b101aw03 = {
},
 };

+static const struct drm_display_mode chungwa_claa101wa01a_mode = {
+   .clock = 72070,
+   .hdisplay = 1366,
+   .hsync_start = 1366 + 58,
+   .hsync_end = 1366 + 58 + 58,
+   .htotal = 1366 + 58 + 58 + 58,
+   .vdisplay = 768,
+   .vsync_start = 768 + 4,
+   .vsync_end = 768 + 4 + 4,
+   .vtotal = 768 + 4 + 4 + 4,
+   .vrefresh = 60,
+};
+
+static const struct panel_desc chungwa_claa101wa01a = {
+   .modes = _claa101wa01a_mode,
+   .num_modes = 1,
+   .size = {
+   .width = 220,
+   .height = 120,
+   },
+};
+
 static const struct drm_display_mode chunghwa_claa101wb01_mode = {
.clock = 69300,
.hdisplay = 1366,
@@ -365,6 +387,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "auo,b101aw03",
.data = _b101aw03,
}, {
+   .compatible = "chungwa,claa101wa01a",
+   .data = _claa101wa01a
+   }, {
.compatible = "chunghwa,claa101wb01",
.data = _claa101wb01
}, {
-- 
1.8.1.5

[PATCH 1/2] drm/panel: Add support for Samsung LTN101NT05 panel

[Stephen Warren]

On 12/21/2013 01:40 PM, Marc Dietrich wrote:
> The Samsung LNT101NT05 10.1" WXVGA panel can be supported by the simple panel
> driver.

Thierry, I assume you'll take patch 1/2 throught the appropriate DRM tree.

[PATCH] drm/tegra: fix compile w/ CONFIG_DYNAMIC_DEBUG

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

With CONFIG_DYNAMIC_DEBUG=y, the followin compile error occurs:

drivers/gpu/drm/tegra/mipi-phy.c: In function ?mipi_dphy_timing_validate?:
drivers/gpu/drm/tegra/mipi-phy.c:69:11: error: ?EINVAL? undeclared (first use 
in this function)
drivers/gpu/drm/tegra/mipi-phy.c:69:11: note: each undeclared identifier is 
reported only once for each function it appears in

Fix this by directly including the header that defines EINVAL.

Fixes: 39aa0a4f3be5 ("drm/tegra: Add DSI support")
Signed-off-by: Stephen Warren 
---
 drivers/gpu/drm/tegra/mipi-phy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tegra/mipi-phy.c b/drivers/gpu/drm/tegra/mipi-phy.c
index 1d2ad267cfce..e2c4aedaee78 100644
--- a/drivers/gpu/drm/tegra/mipi-phy.c
+++ b/drivers/gpu/drm/tegra/mipi-phy.c
@@ -20,6 +20,7 @@
  * OF THIS SOFTWARE.
  */

+#include 
 #include 

 #include "mipi-phy.h"
-- 
1.8.1.5

[PATCH 00/31] ARM: tegra: use common reset and DMA bindings

[Stephen Warren]

On 11/15/2013 01:53 PM, Stephen Warren wrote:
> From: Stephen Warren 
> 
> This series implements a common reset framework driver for Tegra, and
> updates all relevant Tegra drivers to use it. It also removes the custom
> DMA bindings and replaced them with the standard DMA DT bindings.
> 
> Historically, the Tegra clock driver has exported a custom API for module
> reset. This series removes that API, and transitions DT and drivers to
> the new reset framework.
> 
> The custom API used a "struct clk" to identify which module to reset, and
> consequently some DT bindings and drivers required clocks to be provided
> where they really needed just a reset identifier instead. Due to this
> known deficiency, I have always considered most Tegra bindings to be
> unstable. This series removes this excuse for instability, although I
> still consider some Tegra bindings unstable due to the need to convert to
> the common DMA bindings.
> 
> Historically, Tegra DMA channels have been represented in DT using a
> custom nvidia,dma-request-selector property. Now that standard DMA DT
> bindings exist, convert all Tegra bindings, DTs, and drivers to use the
> standard instead.
> 
> This series makes a DT-ABI-incompatible change to:
> - Require reset specifiers in DT where relevant.
> - Require standard DMA specifiers.
> - Remove clock specifiers from DT where they were only needed for reset.
> - Remove legacy DMA specifier properties.
> 
> I anticipate merging this whole series into the Tegra and arm-soc trees
> as its own branch, due to internal dependencies. This branch will be
> stable and can then be merged into any other subsystem trees should any
> conflicts arise.
> 
> This series depends on Peter's Tegra clock driver rework, available at
> git://nv-tegra.nvidia.com/user/pdeschrijver/linux tegra-clk-tegra124-0
> (or whatever version of that gets included in 3.14)

I've applied this series (and pulled in the DMA/ASoC/clk dependencies
required) to Tegra's for-3.14/dmas-resets-rework branch.

[PATCH 00/31] ARM: tegra: use common reset and DMA bindings

[Stephen Warren]

On 11/20/2013 10:03 AM, Arnd Bergmann wrote:
> On Wednesday 20 November 2013, Stephen Warren wrote:
>>> +- #iommu-cells : Must be <1>. This dictates the length of DMA specifiers in
>>> +  client nodes' dmas properties. The specifier represents the DMA request
>>> +  select value for the peripheral. For more details, consult the Tegra 
>>> TRM's
>>> +  documentation of the APB DMA channel control register REQ_SEL field.
>>>  
>>>  Examples:
>>>  
>>> @@ -36,4 +40,5 @@ apbdma: dma at 6000a000 {
>>>   clocks = <_car 34>;
>>>   resets = <_car 34>;
>>>   reset-names = "dma";
>>> + #iommu-cells = <1>;
> 
> 
> s/iommu/dma/
> 
> Otherwise looks good. The dts files are correct, so I guess it's just
> a typo here.

Thanks, fixed locally.

[PATCH 00/31] ARM: tegra: use common reset and DMA bindings

[Stephen Warren]

On 11/20/2013 08:37 AM, Arnd Bergmann wrote:
> On Friday 15 November 2013, Stephen Warren wrote:
>> This series implements a common reset framework driver for Tegra, and
>> updates all relevant Tegra drivers to use it. It also removes the custom
>> DMA bindings and replaced them with the standard DMA DT bindings.
> 
> The series is rather long, so I may have missed it, but I think you need one
> more patch to the apbdma binding to document the use of #dma-cells, what
> value it has, and what the format of the dma specifiers in slave drivers
> needs to be.

Yes, you're right. I will fold the following into "ARM: tegra: document
use of standard DMA DT bindings":

> diff --git a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt 
> b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> index 0b1e577ab9d3..0b0f9498e265 100644
> --- a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> +++ b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> @@ -11,6 +11,10 @@ Required properties:
>See ../reset/reset.txt for details.
>  - reset-names : Must include the following entries:
>- dma
> +- #iommu-cells : Must be <1>. This dictates the length of DMA specifiers in
> +  client nodes' dmas properties. The specifier represents the DMA request
> +  select value for the peripheral. For more details, consult the Tegra TRM's
> +  documentation of the APB DMA channel control register REQ_SEL field.
>  
>  Examples:
>  
> @@ -36,4 +40,5 @@ apbdma: dma at 6000a000 {
>   clocks = <_car 34>;
>   resets = <_car 34>;
>   reset-names = "dma";
> + #iommu-cells = <1>;
>  };

[PATCH 10/31] ARM: tegra: pass reset to tegra_powergate_sequence_power_up()

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

Tegra's clock driver now provides an implementation of the common
reset API (include/linux/reset.h). Use this instead of the old Tegra-
specific API; that will soon be removed.

Cc: treding at nvidia.com
Cc: pdeschrijver at nvidia.com
Cc: linux-tegra at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: Bjorn Helgaas 
Cc: linux-pci at vger.kernel.org
Cc: Terje Bergstr?m 
Cc: David Airlie 
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Stephen Warren 
---
This patch is part of a series with strong internal depdendencies. I'm
looking for an ack so that I can take the entire series through the Tegra
and arm-soc trees. The series will be part of a stable branch that can be
merged into other subsystems if needed to avoid/resolve dependencies.
---
 arch/arm/mach-tegra/powergate.c | 8 +---
 drivers/gpu/drm/tegra/gr3d.c| 6 --
 drivers/pci/host/pci-tegra.c| 3 ++-
 include/linux/tegra-powergate.h | 4 +++-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-tegra/powergate.c b/arch/arm/mach-tegra/powergate.c
index 85d28e756bb7..f6f5b54ff95e 100644
--- a/arch/arm/mach-tegra/powergate.c
+++ b/arch/arm/mach-tegra/powergate.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -144,11 +145,12 @@ int tegra_powergate_remove_clamping(int id)
 }

 /* Must be called with clk disabled, and returns with clk enabled */
-int tegra_powergate_sequence_power_up(int id, struct clk *clk)
+int tegra_powergate_sequence_power_up(int id, struct clk *clk,
+   struct reset_control *rst)
 {
int ret;

-   tegra_periph_reset_assert(clk);
+   reset_control_assert(rst);

ret = tegra_powergate_power_on(id);
if (ret)
@@ -165,7 +167,7 @@ int tegra_powergate_sequence_power_up(int id, struct clk 
*clk)
goto err_clamp;

udelay(10);
-   tegra_periph_reset_deassert(clk);
+   reset_control_deassert(rst);

return 0;

diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index f629e38b00e4..0cbb24b1ae04 100644
--- a/drivers/gpu/drm/tegra/gr3d.c
+++ b/drivers/gpu/drm/tegra/gr3d.c
@@ -279,7 +279,8 @@ static int gr3d_probe(struct platform_device *pdev)
}
}

-   err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_3D, gr3d->clk);
+   err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_3D, gr3d->clk,
+   gr3d->rst);
if (err < 0) {
dev_err(>dev, "failed to power up 3D unit\n");
return err;
@@ -287,7 +288,8 @@ static int gr3d_probe(struct platform_device *pdev)

if (gr3d->clk_secondary) {
err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_3D1,
-   gr3d->clk_secondary);
+   gr3d->clk_secondary,
+   gr3d->rst_secondary);
if (err < 0) {
dev_err(>dev,
"failed to power up secondary 3D unit\n");
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 174a5bc2d993..aace19edc469 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -956,7 +956,8 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
}

err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_PCIE,
-   pcie->pex_clk);
+   pcie->pex_clk,
+   pcie->pex_rst);
if (err) {
dev_err(pcie->dev, "powerup sequence failed: %d\n", err);
return err;
diff --git a/include/linux/tegra-powergate.h b/include/linux/tegra-powergate.h
index c98cfa406952..b5ad64aca071 100644
--- a/include/linux/tegra-powergate.h
+++ b/include/linux/tegra-powergate.h
@@ -19,6 +19,7 @@
 #define _MACH_TEGRA_POWERGATE_H_

 struct clk;
+struct reset_control;

 #define TEGRA_POWERGATE_CPU0
 #define TEGRA_POWERGATE_3D 1
@@ -51,6 +52,7 @@ int tegra_powergate_power_off(int id);
 int tegra_powergate_remove_clamping(int id);

 /* Must be called with clk disabled, and returns with clk enabled */
-int tegra_powergate_sequence_power_up(int id, struct clk *clk);
+int tegra_powergate_sequence_power_up(int id, struct clk *clk,
+   struct reset_control *rst);

 #endif /* _MACH_TEGRA_POWERGATE_H_ */
-- 
1.8.1.5

[PATCH 09/31] drm/tegra: use reset framework

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

Tegra's clock driver now provides an implementation of the common
reset API (include/linux/reset.h). Use this instead of the old Tegra-
specific API; that will soon be removed.

Cc: treding at nvidia.com
Cc: pdeschrijver at nvidia.com
Cc: linux-tegra at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: Terje Bergstr?m 
Cc: David Airlie 
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Stephen Warren 
---
This patch is part of a series with strong internal depdendencies. I'm
looking for an ack so that I can take the entire series through the Tegra
and arm-soc trees. The series will be part of a stable branch that can be
merged into other subsystems if needed to avoid/resolve dependencies.
---
 drivers/gpu/drm/tegra/Kconfig |  1 +
 drivers/gpu/drm/tegra/dc.c|  9 -
 drivers/gpu/drm/tegra/drm.h   |  3 +++
 drivers/gpu/drm/tegra/gr3d.c  | 16 
 drivers/gpu/drm/tegra/hdmi.c  | 14 +++---
 5 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 8961ba6a34b8..8db9b3bce001 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -2,6 +2,7 @@ config DRM_TEGRA
bool "NVIDIA Tegra DRM"
depends on ARCH_TEGRA || ARCH_MULTIPLATFORM
depends on DRM
+   depends on RESET_CONTROLLER
select TEGRA_HOST1X
select DRM_KMS_HELPER
select DRM_KMS_FB_HELPER
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index ae1cb31ead7e..c3be92879bea 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 

 #include "dc.h"
 #include "drm.h"
@@ -712,7 +713,7 @@ static void tegra_crtc_prepare(struct drm_crtc *crtc)
unsigned long value;

/* hardware initialization */
-   tegra_periph_reset_deassert(dc->clk);
+   reset_control_deassert(dc->rst);
usleep_range(1, 2);

if (dc->pipe)
@@ -1187,6 +1188,12 @@ static int tegra_dc_probe(struct platform_device *pdev)
return PTR_ERR(dc->clk);
}

+   dc->rst = devm_reset_control_get(>dev, "dc");
+   if (IS_ERR(dc->rst)) {
+   dev_err(>dev, "failed to get reset\n");
+   return PTR_ERR(dc->rst);
+   }
+
err = clk_prepare_enable(dc->clk);
if (err < 0)
return err;
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index fdfe259ed7f8..f717c18b28c2 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -19,6 +19,8 @@
 #include 
 #include 

+struct reset_control;
+
 struct tegra_fb {
struct drm_framebuffer base;
struct tegra_bo **planes;
@@ -93,6 +95,7 @@ struct tegra_dc {
int pipe;

struct clk *clk;
+   struct reset_control *rst;
void __iomem *regs;
int irq;

diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index 4cec8f526af7..f629e38b00e4 100644
--- a/drivers/gpu/drm/tegra/gr3d.c
+++ b/drivers/gpu/drm/tegra/gr3d.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include "drm.h"
@@ -22,6 +23,8 @@ struct gr3d {
struct host1x_channel *channel;
struct clk *clk_secondary;
struct clk *clk;
+   struct reset_control *rst_secondary;
+   struct reset_control *rst;

DECLARE_BITMAP(addr_regs, GR3D_NUM_REGS);
 };
@@ -255,12 +258,25 @@ static int gr3d_probe(struct platform_device *pdev)
return PTR_ERR(gr3d->clk);
}

+   gr3d->rst = devm_reset_control_get(>dev, "3d");
+   if (IS_ERR(gr3d->rst)) {
+   dev_err(>dev, "cannot get reset\n");
+   return PTR_ERR(gr3d->rst);
+   }
+
if (of_device_is_compatible(np, "nvidia,tegra30-gr3d")) {
gr3d->clk_secondary = devm_clk_get(>dev, "3d2");
if (IS_ERR(gr3d->clk)) {
dev_err(>dev, "cannot get secondary clock\n");
return PTR_ERR(gr3d->clk);
}
+
+   gr3d->rst_secondary = devm_reset_control_get(>dev,
+   "3d2");
+   if (IS_ERR(gr3d->rst_secondary)) {
+   dev_err(>dev, "cannot get secondary reset\n");
+   return PTR_ERR(gr3d->rst_secondary);
+   }
}

err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_3D, gr3d->clk);
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 0cd9bc2056e8..f3aad49633d6 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -12,6 

[PATCH 00/31] ARM: tegra: use common reset and DMA bindings

[Stephen Warren]

From: Stephen Warren <swar...@nvidia.com>

This series implements a common reset framework driver for Tegra, and
updates all relevant Tegra drivers to use it. It also removes the custom
DMA bindings and replaced them with the standard DMA DT bindings.

Historically, the Tegra clock driver has exported a custom API for module
reset. This series removes that API, and transitions DT and drivers to
the new reset framework.

The custom API used a "struct clk" to identify which module to reset, and
consequently some DT bindings and drivers required clocks to be provided
where they really needed just a reset identifier instead. Due to this
known deficiency, I have always considered most Tegra bindings to be
unstable. This series removes this excuse for instability, although I
still consider some Tegra bindings unstable due to the need to convert to
the common DMA bindings.

Historically, Tegra DMA channels have been represented in DT using a
custom nvidia,dma-request-selector property. Now that standard DMA DT
bindings exist, convert all Tegra bindings, DTs, and drivers to use the
standard instead.

This series makes a DT-ABI-incompatible change to:
- Require reset specifiers in DT where relevant.
- Require standard DMA specifiers.
- Remove clock specifiers from DT where they were only needed for reset.
- Remove legacy DMA specifier properties.

I anticipate merging this whole series into the Tegra and arm-soc trees
as its own branch, due to internal dependencies. This branch will be
stable and can then be merged into any other subsystem trees should any
conflicts arise.

This series depends on Peter's Tegra clock driver rework, available at
git://nv-tegra.nvidia.com/user/pdeschrijver/linux tegra-clk-tegra124-0
(or whatever version of that gets included in 3.14)

Cc: ac100 at lists.launchpad.net
Cc: Alan Stern 
Cc: alsa-devel at alsa-project.org
Cc: Bjorn Helgaas 
Cc: Dan Williams 
Cc: David Airlie 
Cc: devel at driverdev.osuosl.org
Cc: devicetree at vger.kernel.org
Cc: Dmitry Torokhov 
Cc: Dmitry Torokhov 
Cc: dri-devel at lists.freedesktop.org
Cc: Greg Kroah-Hartman 
Cc: Ian Campbell <ijc+devicetree at hellion.org.uk>
Cc: Julian Andres Klode 
Cc: Liam Girdwood 
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-i2c at vger.kernel.org
Cc: linux-input at vger.kernel.org
Cc: linux-pci at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: linux-spi at vger.kernel.org
Cc: linux-tegra at vger.kernel.org
Cc: linux-usb at vger.kernel.org
Cc: Marc Dietrich 
Cc: Mark Brown 
Cc: Mark Rutland 
Cc: Mike Turquette 
Cc: Pawel Moll 
Cc: pdeschrijver at nvidia.com
Cc: Rob Herring 
Cc: Terje Bergstr?m 
Cc: treding at nvidia.com
Cc: Wolfram Sang 

Stephen Warren (31):
  ARM: tegra: add missing clock documentation to DT bindings
  ARM: tegra: document reset properties in DT bindings
  ARM: tegra: document use of standard DMA DT bindings
  ARM: tegra: update DT files to add reset properties
  ARM: tegra: update DT files to add DMA properties
  ARM: tegra: select the reset framework
  clk: tegra: implement a reset driver
  pci: tegra: use reset framework
  drm/tegra: use reset framework
  ARM: tegra: pass reset to tegra_powergate_sequence_power_up()
  dma: add channel request API that supports deferred probe
  dma: tegra: use reset framework
  dma: tegra: register as an OF DMA controller
  ASoC: dmaengine: support deferred probe for DMA channels
  ASoC: dmaengine: add custom DMA config to snd_dmaengine_pcm_config
  ASoC: tegra: use reset framework
  ASoC: tegra: call pm_runtime APIs around register accesses
  ASoC: tegra: allocate AHUB FIFO during probe() not startup()
  ASoC: tegra: convert to standard DMA DT bindings
  i2c: tegra: use reset framework
  staging: nvec: use reset framework
  spi: tegra: use reset framework
  spi: tegra: convert to standard DMA DT bindings
  serial: tegra: use reset framework
  serial: tegra: convert to standard DMA DT bindings
  Input: tegra-kbc - use reset framework
  USB: EHCI: tegra: use reset framework
  ARM: tegra: remove legacy clock entries from DT
  ARM: tegra: remove legacy DMA entries from DT
  clk: tegra: remove legacy reset APIs
  clk: tegra: remove bogus PCIE_XCLK

 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt  |   1 +
 .../bindings/clock/nvidia,tegra114-car.txt |   4 +
 .../bindings/clock/nvidia,tegra124-car.txt |   4 +
 .../bindings/clock/nvidia,tegra20-car.txt  |   4 +
 .../bindings/clock/nvidia,tegra30-car.txt  |   4 +
 .../devicetree/bindings/dma/tegra20-apbdma.txt |   9 ++
 .../bindings/gpu/nvidia,tegra20-host1x.txt | 124 +++
 .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |  27 +++-
 .../bindings/input/nvidia,tegra20-kbc.txt  |   9 ++
 .../bindings/mmc/nvidia,tegra20-sdhci.txt  |   9 ++
 .../devicetree/bindings/nvec/nvidia,nvec.txt   |  12 ++
 .../bindings/pci/nvidia,tegra20-pcie.txt   |  28 ++--
 .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt |   9 ++
 .

[PATCH] drm/tegra: replace IS_ERR and PTR_ERR with PTR_ERR_OR_ZERO

[Stephen Warren]

On 11/06/2013 12:53 AM, Duan Jiong wrote:
> This patch fixes coccinelle error regarding usage of IS_ERR and
> PTR_ERR instead of PTR_ERR_OR_ZERO.

> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c

> @@ -199,10 +199,7 @@ int tegra_bo_dumb_create(struct drm_file *file, struct 
> drm_device *drm,
>  
>   bo = tegra_bo_create_with_handle(file, drm, args->size, 0,
>>handle);
> - if (IS_ERR(bo))
> - return PTR_ERR(bo);
> -
> - return 0;
> + return PTR_ERR_OR_ZERO(bo);
>  }

I suppose that's fine, although I wonder if it'll cause churn should we
ever need to add code to the tail end of the function.

BTW, why were all the similar patches that had nothing to do with Tegra
sent to the linux-tegra@ mailing list? You also didn't CC the
maintainers of drivers/gpu/drm/tegra/ on this patch. Plus this patch has
nothing to do with the rtc-linux@ mailing list.

[PATCH -next] drm/tegra: fix return value check

[Stephen Warren]

On 10/28/2013 04:38 PM, Thierry Reding wrote:
> On Mon, Oct 28, 2013 at 03:51:32PM -0600, Stephen Warren wrote:
>> On 10/28/2013 02:53 AM, Thierry Reding wrote:
>>> On Mon, Oct 21, 2013 at 11:34:07AM +0800, Wei Yongjun wrote:
>>>> From: Wei Yongjun 
>>>> 
>>>> In case of error, the function clk_get_parent() and 
>>>> devm_ioremap_resource() returns ERR_PTR() and never returns
>>>> NULL. The NULL test in the return value check should be
>>>> replaced with IS_ERR().
>>>> 
>>>> Signed-off-by: Wei Yongjun 
>>>> --- drivers/gpu/drm/tegra/dsi.c | 6 +++--- 1 file changed, 3 
>>>> insertions(+), 3 deletions(-)
>>> 
>>> I've applied this, but with the first hunk removed, since
>>> looking at the implementation of clk_get_parent() it can
>>> actually return NULL. In fact it seems like it will never
>>> return ERR_PTR().
>>> 
>>> I've also updated the commit message to reflect that.
>> 
>> Hmm. The documentation for clk_get() says:
> 
> The patch didn't check the return value clk_get() but
> clk_get_parent(). Here's the implementation:
> 
> struct clk *__clk_get_parent(struct clk *clk) { return !clk ? NULL
> : clk->parent; }
> 
> Note that clk_get_parent() in simply a locked version of the above.
> That will obviously only return ERR_PTR() if clk->parent happens to
> be set to one such value, which I don't think will ever happen.

Ah. That looks like a bug in __clk_get_parent() then, since !clk
doesn't sound like  the correct error case for it to be checking.
Shouldn't it return IS_ERR(clk) ? clk : clk->parent? Either that, or
clk_get() shouldn't return an error value if the rest of the clock
code doesn NULL checks.

[PATCH -next] drm/tegra: fix return value check

[Stephen Warren]

On 10/28/2013 02:53 AM, Thierry Reding wrote:
> On Mon, Oct 21, 2013 at 11:34:07AM +0800, Wei Yongjun wrote:
>> From: Wei Yongjun 
>> 
>> In case of error, the function clk_get_parent() and
>> devm_ioremap_resource() returns ERR_PTR() and never returns NULL.
>> The NULL test in the return value check should be replaced with
>> IS_ERR().
>> 
>> Signed-off-by: Wei Yongjun  --- 
>> drivers/gpu/drm/tegra/dsi.c | 6 +++--- 1 file changed, 3
>> insertions(+), 3 deletions(-)
> 
> I've applied this, but with the first hunk removed, since looking
> at the implementation of clk_get_parent() it can actually return
> NULL. In fact it seems like it will never return ERR_PTR().
> 
> I've also updated the commit message to reflect that.

Hmm. The documentation for clk_get() says:

/**
 * clk_get - lookup and obtain a reference to a clock producer.
 * @dev: device for clock "consumer"
 * @id: clock consumer ID
 *
 * Returns a struct clk corresponding to the clock producer, or
 * valid IS_ERR() condition containing errno.  The implementation
 * uses @dev and @id to determine the clock consumer, and thereby
 * the clock producer.  (IOW, @id may be identical strings, but
 * clk_get may return different clock producers depending on @dev.)

If the implementation doesn't match that, then it's a bug, and a whole
slew of drivers will need changing... On the surface, it looks like
the hunk you dropped was correct.

NULL may be a perfectly legal return value from a function that
returns either valid data or an IS_ERR() value.

[PATCH v2 27/27] drm/tegra: Add Tegra114 gr2d support

[Stephen Warren]

On 10/16/2013 02:48 AM, Thierry Reding wrote:
> On Tue, Oct 15, 2013 at 09:19:18AM -0600, Stephen Warren wrote:
>> On 10/15/2013 02:37 AM, Thierry Reding wrote:
>>> On Mon, Oct 14, 2013 at 12:14:47PM -0600, Stephen Warren
>>> wrote:
>>>> On 10/14/2013 08:00 AM, Thierry Reding wrote:
>>>>> On Mon, Oct 14, 2013 at 08:58:34AM +0300, Terje Bergstr?m 
>>>>> wrote:
>>>>>> On 12.10.2013 01:43, Stephen Warren wrote:
>>>>>>> On 10/07/2013 02:34 AM, Thierry Reding wrote:
>>>>>>>> The gr2d hardware in Tegra114 is compatible with that
>>>>>>>> of Tegra20 and Tegra30. No functionaly changes are 
>>>>>>>> required.
>>>>>>> Similarly here, if the HW is 100%
>>>>>>> backwards-compatible, there's no need to add compatible
>>>>>>> values to the driver.
>>>>>> 
>>>>>> We've used this mechanism for attaching a per-hw-version 
>>>>>> data structure in match table to accomodate differences
>>>>>> in how the hardware is power gated, reset, booted, some
>>>>>> per-soc performance related changes etc. It's also used
>>>>>> in staging features for new chips, such as disabling
>>>>>> power features when they're not working/verified yet.
>>>>>> 
>>>>>> Upstream driver is not yet in a state where that is 
>>>>>> relevant.
>>>>>> 
>>>>>> With this, would we still be able to do that with match 
>>>>>> table? It sounds like we could, because we can still
>>>>>> (even with multiple compatible properties) add separate
>>>>>> entries in match table and I guess the compatible
>>>>>> properties matched in order.
>>>>> 
>>>>> Yes, as long as the device tree files includes the most 
>>>>> specific value in the compatible this should still be
>>>>> possible. So we'd have this:
>>>>> 
>>>>> gr2d at 5414 { compatible = "nvida,tegra114-gr2d", 
>>>>> "nvidia,tegra20-gr2d"; ... };
>>>>> 
>>>>> and the driver will match on "nvidia,tegra20-gr2d" if the
>>>>> more specific "nvidia,tegra114-gr2d" is not there. When the
>>>>> driver is updated to support Tegra114 specific
>>>>> functionality, then a more specific entry can be added to
>>>>> the compatible table to handle it.
>>>> 
>>>> True, but the DT fragment above is also only accurate /if/ a 
>>>> driver that only knows about "nvidia,tegra20-gr2d" can
>>>> operate 100% of the features in Tegra20 HW on Tegra114 HW
>>>> forever.
>>> 
>>> Yes, but given that we also list "nvidia,tegra114-gr2d" in the 
>>> property it will be possible to add that to the driver when
>>> new functionality is implemented and immediately take advantage
>>> of it on Tegra114 hardware with an old device tree file which
>>> has both compatible values.
>> 
>> Taking advantage of new functionality isn't the issue. The issue
>> is whether a driver that was written purely to the spec of
>> Tegra20 can successfully operate on the HW. If it can't, then the
>> HW is not compatible with Tegra20. Terje's previous comments
>> sounded like the HW is not backwards-compatible, although his
>> more recent comments make it sound like only SW policy
>> differences, which shouldn't be part of DT anyway.
> 
> Well, as good as I can tell it is backwards-compatible. I've been 
> testing the code included in this series with the same simple test 
> program that clears a rectangle on Tegra20, Tegra30 and Tegra114.

All that means is that the subset of features we use so far is compatible.

> Furthermore our internal register specification files are
> identical, with the exception of some whitespace changes for all
> three generations. I think that qualifies as backwards-compatible?

On the other hand, that sounds like an almost perfect definition of
backwards-compatible. Can you also validate that any module
clock/power/reset inputs are identical? If so, the case is closed:-)

[PATCH v2 26/27] drm/tegra: Add DSI support

[Stephen Warren]

On 10/16/2013 02:40 AM, Thierry Reding wrote:
...
> Just to follow up with what we've discussed on IRC: I'll take a
> stab at refactoring the debugfs file output into something more
> generic that can possibly be leveraged by regmap as well. That
> should provide a somewhat more lightweight alternative to regmap
> for implementations that don't need regmap for anything else while
> at the same time providing something so not every driver has to
> come up with something custom.
> 
> Does that sum it up correctly?

Yes.

[PATCH v2 26/27] drm/tegra: Add DSI support

[Stephen Warren]

On 10/15/2013 02:33 AM, Thierry Reding wrote:
> On Mon, Oct 14, 2013 at 12:16:48PM -0600, Stephen Warren wrote:
>> On 10/14/2013 07:55 AM, Thierry Reding wrote:
>>> On Fri, Oct 11, 2013 at 04:43:35PM -0600, Stephen Warren
>>> wrote:
>>>> On 10/07/2013 02:34 AM, Thierry Reding wrote:
>>>>> This commit adds support for both DSI outputs found on
>>>>> Tegra. Only very minimal functionality is implemented, so
>>>>> advanced features like ganged mode won't work.
>>>>> 
>>>>> Due to the lack of other test hardware, some sections of
>>>>> the driver are hardcoded to work with Dalmore.
>>>> 
>>>>> diff --git a/drivers/gpu/drm/tegra/dsi.c
>>>>> b/drivers/gpu/drm/tegra/dsi.c
>>>> 
>>>>> +static int tegra_dsi_show_regs(struct seq_file *s, void
>>>>> *data) +{ +   struct drm_info_node *node = s->private; +
>>>>> struct tegra_dsi *dsi = node->info_ent->data; + +#define
>>>>> DUMP_REG(name)\ + 
>>>>> seq_printf(s, "%-32s %#05x
>>>>> %08lx\n", #name, name,\ +tegra_dsi_readl(dsi, name)) 
>>>>> + +   DUMP_REG(DSI_INCR_SYNCPT);
>>>> 
>>>> Does it make sense to use an MMIO regmap instead? That way,
>>>> you get all the debugfs files for free...
>>> 
>>> As far as I know, regmap doesn't give you the symbolic names
>>> for the registers. I find that a rather useful feature because
>>> it allows to easily compare the registers to the ones in our
>>> downstream kernels.
>> 
>> True. However, we should really be writing user-space scripts to
>> encode that information. Such a script could be useful e.g. if
>> reading the information directly from /dev/mem or JTAG too, and
>> bloating the kernel with debug strings doesn't seem like a great
>> idea.
> 
> I don't agree. While I see some value in having such userspace
> scripts, having the symbolic names in debugfs allows anyone to look
> at a readable form of the data without having to have access to
> those scripts. Besides it's called *debug*fs for a purpose, isn't
> it? There's plenty of edited data that's not just a plain dump of
> data.

Perhaps you should work to enhance regmap's debugfs files to support
optionally naming registers?

[PATCH v2 27/27] drm/tegra: Add Tegra114 gr2d support

[Stephen Warren]

On 10/15/2013 02:37 AM, Thierry Reding wrote:
> On Mon, Oct 14, 2013 at 12:14:47PM -0600, Stephen Warren wrote:
>> On 10/14/2013 08:00 AM, Thierry Reding wrote:
>>> On Mon, Oct 14, 2013 at 08:58:34AM +0300, Terje Bergstr?m
>>> wrote:
>>>> On 12.10.2013 01:43, Stephen Warren wrote:
>>>>> On 10/07/2013 02:34 AM, Thierry Reding wrote:
>>>>>> The gr2d hardware in Tegra114 is compatible with that of 
>>>>>> Tegra20 and Tegra30. No functionaly changes are
>>>>>> required.
>>>>> Similarly here, if the HW is 100% backwards-compatible,
>>>>> there's no need to add compatible values to the driver.
>>>> 
>>>> We've used this mechanism for attaching a per-hw-version
>>>> data structure in match table to accomodate differences in
>>>> how the hardware is power gated, reset, booted, some per-soc
>>>> performance related changes etc. It's also used in staging
>>>> features for new chips, such as disabling power features when
>>>> they're not working/verified yet.
>>>> 
>>>> Upstream driver is not yet in a state where that is
>>>> relevant.
>>>> 
>>>> With this, would we still be able to do that with match
>>>> table? It sounds like we could, because we can still (even
>>>> with multiple compatible properties) add separate entries in
>>>> match table and I guess the compatible properties matched in
>>>> order.
>>> 
>>> Yes, as long as the device tree files includes the most
>>> specific value in the compatible this should still be possible.
>>> So we'd have this:
>>> 
>>> gr2d at 5414 { compatible = "nvida,tegra114-gr2d", 
>>> "nvidia,tegra20-gr2d"; ... };
>>> 
>>> and the driver will match on "nvidia,tegra20-gr2d" if the more 
>>> specific "nvidia,tegra114-gr2d" is not there. When the driver
>>> is updated to support Tegra114 specific functionality, then a
>>> more specific entry can be added to the compatible table to
>>> handle it.
>> 
>> True, but the DT fragment above is also only accurate /if/ a
>> driver that only knows about "nvidia,tegra20-gr2d" can operate
>> 100% of the features in Tegra20 HW on Tegra114 HW forever.
> 
> Yes, but given that we also list "nvidia,tegra114-gr2d" in the
> property it will be possible to add that to the driver when new
> functionality is implemented and immediately take advantage of it
> on Tegra114 hardware with an old device tree file which has both
> compatible values.

Taking advantage of new functionality isn't the issue. The issue is
whether a driver that was written purely to the spec of Tegra20 can
successfully operate on the HW. If it can't, then the HW is not
compatible with Tegra20. Terje's previous comments sounded like the HW
is not backwards-compatible, although his more recent comments make it
sound like only SW policy differences, which shouldn't be part of DT
anyway.

[PATCH v2 17/27] drm/tegra: Add Tegra114 HDMI support

[Stephen Warren]

On 10/15/2013 02:13 AM, Thierry Reding wrote:
> On Mon, Oct 14, 2013 at 12:10:21PM -0600, Stephen Warren wrote:
>> On 10/12/2013 05:41 AM, Thierry Reding wrote:
>>> On Fri, Oct 11, 2013 at 04:19:19PM -0600, Stephen Warren
>>> wrote:
>>>> On 10/07/2013 02:34 AM, Thierry Reding wrote:
>>>>> From: Mikko Perttunen 
>>>>> 
>>>>> Tegra114 TMDS configuration requires a new peak_current
>>>>> field and the driver current override bit has changed
>>>>> position.
>>>> 
>>>>> diff --git a/drivers/gpu/drm/tegra/hdmi.c 
>>>>> b/drivers/gpu/drm/tegra/hdmi.c
>>>> 
>>>>> static const struct tmds_config tegra2_tmds_config[] = {
>>>>> @@ -223,6 +224,85 @@ static const struct tmds_config 
>>>>> tegra3_tmds_config[] = {
>>>> 
>>>> Not related to this patch, but those should have been named 
>>>> tegra20_tmds_config[] and tegra30_tmds_config[].
>>>> 
>>>>> static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,
>>>> 
>>>>> - value = tmds->drive_current |
>>>>> DRIVE_CURRENT_FUSE_OVERRIDE; - tegra_hdmi_writel(hdmi,
>>>>> value, HDMI_NV_PDISP_SOR_LANE_DRIVE_CURRENT); +   if 
>>>>> (of_device_is_compatible(np, "nvidia,tegra114-hdmi")) {
>>>> 
>>>> Let's not check this at run-time. Instead,
>>>> host1x_drm_subdevs[]'s .data field should be used to contain
>>>> either flags or a pointer to a configuration structure,
>>>> either of which can be directly consulted to determine the
>>>> properties of the HW in a feature-oriented/semantic way.
>>>> 
>>>> drivers/gpio/gpio-tegra.c's 
>>>> tegra20_gpio_config/tegra30_gpio_config/tegra_gpio_of_match 
>>>> provide a good example of this.
>>>> 
>>>> This means that if Tegra124 is identical to Tegra114, yet a 
>>>> hypothetical Tegra999 is different, you don't have to keep 
>>>> adjusting these if conditions throughout the code; they can 
>>>> simply refer to the same feature bit forever.
>>> 
>>> Okay, I'll see what I can come up with. It's unfortunately not
>>> as simple as the GPIO driver's parameterization, and who knows
>>> what other differences will be introduced in some later
>>> versions of this block.
>>> 
>>> What I mean is that at some point it becomes questionable
>>> whether it makes sense to parameterize at all if you have to
>>> encode the register offset and bit position within that
>>> register for a large number of bits.
>> 
>> Well, I wasn't advocating that we shouldn't have an if statement
>> at all. Simply that the if statement shouldn't be doing string
>> compares of specific HW. Either of the following would be fine:
>> 
>> if (hdmi->soc_data->some_feature_flag) // just represents some
>> code; doesn't have to be a function call do_something(); else; 
>> do_something_else();
>> 
>> or:
>> 
>> do_something(hdmi->soc_data->some_feature_value);
> 
> But the fact that a bit has moved from one register to another can 
> hardly be defined as feature. At least I couldn't come up with any 
> sensible name for one.

The feature is the name/identification of the register the field is
in. For example, foo_field_in_bar_reg. Admittedly this isn't "feature"
in the typical sense, but more a "facet of the SW-visible interface".

> We could of course just add a version number into a per-SoC
> descriptor and use that, but that's not any better than checking
> for the compatible value, really.

Even that would be better; it'd avoid a strcmp() every time the code
ran, and also allow the of_match table's data to map from compatible
value to register layout ID. It's quite possible that we have 4 SoCs:

SoC / Register location of field X / Other features, etc.
100   A  P
200   B  Q
300   B  Q
400   B  R

Where P, Q, R need different compatible values, but the location of
the register field we're talking about isn't 1:1 with the compatible
values, but rather many:1.

> Ideally of course the hardware wouldn't change in these ways from
> one generation to the next...
> 
> That said, I've opted to go with putting the register and bit
> position into a per-SoC descriptor and parameterize on that, along
> with a boolean flag for the existence of the IO peak current
> register.

Sounds good.

[PATCH v2 27/27] drm/tegra: Add Tegra114 gr2d support

[Stephen Warren]

On 10/14/2013 11:51 PM, Terje Bergstr?m wrote:
> On 14.10.2013 21:14, Stephen Warren wrote:
>> On 10/14/2013 08:00 AM, Thierry Reding wrote:
>>> Yes, as long as the device tree files includes the most specific
>>> value in the compatible this should still be possible. So we'd have
>>> this:
>>>
>>> gr2d at 5414 { compatible = "nvida,tegra114-gr2d",
>>> "nvidia,tegra20-gr2d"; ... };
>>>
>>> and the driver will match on "nvidia,tegra20-gr2d" if the more
>>> specific "nvidia,tegra114-gr2d" is not there. When the driver is
>>> updated to support Tegra114 specific functionality, then a more
>>> specific entry can be added to the compatible table to handle it.
>>
>> True, but the DT fragment above is also only accurate /if/ a driver
>> that only knows about "nvidia,tegra20-gr2d" can operate 100% of the
>> features in Tegra20 HW on Tegra114 HW forever.
> 
> I don't know of any hardware incompatibility. The only difference is
> related to something not directly to 2D. We moved host1x away from the
> power domain, so in Tegra114 we're able to power gate 2D and EPP. The
> DVFS tables are also different.
> 
> I'd say say adding the compatible property "nvidia,tegra20-gr2d" for
> Tegra114's 2D is accurate and we're able to use the match table to drive
> any SW policy differences.

The compatible value shouldn't be used for SW policy differences. DT is
about HW, not SW policy. You have to decide: either the HW is 100%
backwards-compatible, or it's not. You can't decide that the HW is
compatible, but then require different compatible values.

Re: [PATCH v2 16/27] drm/tegra: Add Tegra114 display controller support

[Stephen Warren]

On 10/12/2013 05:32 AM, Thierry Reding wrote:
 On Fri, Oct 11, 2013 at 04:14:27PM -0600, Stephen Warren wrote:
 On 10/07/2013 02:34 AM, Thierry Reding wrote:
 From: Mikko Perttunen mperttu...@nvidia.com
 
 The Tegra114 display controller is backwards-compatible with
 previous generations of the Tegra SoC. No code changes are
 required.
 
 If the HW is backwards-compatible, then there's no need to add
 extra compatible values to the driver; just write the following
 in the DT, and it'll just work:
 
 compatible = nvidia,tegra114-dc, nvidia,tegra30-dc;
 
 One reason why I thought it might be useful to still include this,
 even though unnecessary, was to match it to the host1x_drm_subdevs
 table. We can probably remove the entry from there as well,
 though.
 
 As far as I can tell, the same holds for Tegra30, which is also 
 backwards compatible with Tegra20 but the DTS doesn't contain the 
 Tegra20 compatible. So to keep ABI compatibility we'll need to keep
 the nvidia,tegra30-dc in the driver's match tables, but I could
 still update the DTS to include the nvidia,tegra20-dc for
 correctness.
 
 Does that make sense?

Yes, fixing the Tegra30 .dtsi file sounds like a good idea.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 15/27] gpu: host1x: Add support for Tegra114

[Stephen Warren]

On 10/12/2013 05:24 AM, Thierry Reding wrote:
 On Fri, Oct 11, 2013 at 04:13:07PM -0600, Stephen Warren wrote:
 On 10/07/2013 02:34 AM, Thierry Reding wrote:
 Tegra114 uses a slightly updated version of host1x with an
 additional syncpoint.
 
 drivers/gpu/host1x/hw/host1x02.c|  42 + 
 drivers/gpu/host1x/hw/host1x02.h|  26 +++ 
 drivers/gpu/host1x/hw/hw_host1x02_channel.h | 121
 ++ drivers/gpu/host1x/hw/hw_host1x02_sync.h|
 243  
 drivers/gpu/host1x/hw/hw_host1x02_uclass.h  | 175
 
 
 That seems like an awful lot of extra lines to support just one
 extra syncpoint. Are there other changes? If not, can the code
 be shared/parameterized somehow?
 
 Yeah, I don't like very much how this is currently done. I mean
 about half of this is actually duplicate code because of the static
 inline functions used for register defines. As discussed elsewhere
 this was originally meant to be used for coverage testing, but
 nobody's done anything like that as far as I know. I'm also not
 convinced that these would be very useful in coverage testing, but
 adding Terje on Cc and unless he or anyone else has any (strong)
 objections I'll go and remove the duplicate definitions and while
 at it see if I can come up with a way to share more
 code/definitions between versions of host1x.
 
 Do you see this as a blocker for 3.13 or can I do the cleanup
 after that? As far as I know Tegra124 has a more heavily modified
 version of host1x so implementing Tegra124 support might be a good
 opportunity to clean this up anyway.

I guess I'm unsure re: whether it's a blocker. It's certainly not some
kind of ABI issue, so it's not like it forces our hand later; we can
easily refactor the code later. However, I'm slightly worried that if
we do actually intend to do that, it'll be seen as code-churn. Still,
I guess if the main DRM maintainers don't object to this, I'm OK with it.

Re: Terje's points, we (e.g. Terje) should work with the HW designers
to stop moving things about, so we don't have incompatible HW.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 17/27] drm/tegra: Add Tegra114 HDMI support

[Stephen Warren]

On 10/12/2013 05:41 AM, Thierry Reding wrote:
 On Fri, Oct 11, 2013 at 04:19:19PM -0600, Stephen Warren wrote:
 On 10/07/2013 02:34 AM, Thierry Reding wrote:
 From: Mikko Perttunen mperttu...@nvidia.com
 
 Tegra114 TMDS configuration requires a new peak_current field
 and the driver current override bit has changed position.
 
 diff --git a/drivers/gpu/drm/tegra/hdmi.c
 b/drivers/gpu/drm/tegra/hdmi.c
 
 static const struct tmds_config tegra2_tmds_config[] = { @@
 -223,6 +224,85 @@ static const struct tmds_config
 tegra3_tmds_config[] = {
 
 Not related to this patch, but those should have been named 
 tegra20_tmds_config[] and tegra30_tmds_config[].
 
 static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,
 
 -   value = tmds-drive_current | DRIVE_CURRENT_FUSE_OVERRIDE; -
 tegra_hdmi_writel(hdmi, value,
 HDMI_NV_PDISP_SOR_LANE_DRIVE_CURRENT); +if
 (of_device_is_compatible(np, nvidia,tegra114-hdmi)) {
 
 Let's not check this at run-time. Instead, host1x_drm_subdevs[]'s
 .data field should be used to contain either flags or a pointer
 to a configuration structure, either of which can be directly
 consulted to determine the properties of the HW in a
 feature-oriented/semantic way.
 
 drivers/gpio/gpio-tegra.c's 
 tegra20_gpio_config/tegra30_gpio_config/tegra_gpio_of_match
 provide a good example of this.
 
 This means that if Tegra124 is identical to Tegra114, yet a
 hypothetical Tegra999 is different, you don't have to keep
 adjusting these if conditions throughout the code; they can
 simply refer to the same feature bit forever.
 
 Okay, I'll see what I can come up with. It's unfortunately not as
 simple as the GPIO driver's parameterization, and who knows what
 other differences will be introduced in some later versions of this
 block.
 
 What I mean is that at some point it becomes questionable whether
 it makes sense to parameterize at all if you have to encode the
 register offset and bit position within that register for a large
 number of bits.

Well, I wasn't advocating that we shouldn't have an if statement at
all. Simply that the if statement shouldn't be doing string compares
of specific HW. Either of the following would be fine:

if (hdmi-soc_data-some_feature_flag)
   // just represents some code; doesn't have to be a function call
   do_something();
else;
  do_something_else();

or:

do_something(hdmi-soc_data-some_feature_value);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 27/27] drm/tegra: Add Tegra114 gr2d support

[Stephen Warren]

On 10/13/2013 11:58 PM, Terje Bergström wrote:
 On 12.10.2013 01:43, Stephen Warren wrote:
 On 10/07/2013 02:34 AM, Thierry Reding wrote:
 The gr2d hardware in Tegra114 is compatible with that of Tegra20 and
 Tegra30. No functionaly changes are required.

 Similarly here, if the HW is 100% backwards-compatible, there's no need
 to add compatible values to the driver.
 
 We've used this mechanism for attaching a per-hw-version data structure
 in match table to accomodate differences in how the hardware is power
 gated, reset, booted, some per-soc performance related changes etc.

If there are differences in those aspects of the HW, such that a driver
written only to the full specification of e.g. Tegra30 would not work on
Tegra114, then the HW is not actually compatible, and hence we do need
multiple compatible values in DT, and entries in the of_match table.

It sounds like the statement in the commit description:

 The gr2d hardware in Tegra114 is compatible with that of Tegra20 and
 Tegra30. No functionaly changes are required.

Might not be absolutely accurate in terms of HW, but only in terms of
the features that the driver uses so far. It'd be good to explicitly
qualify this in the commit description.

...
 Upstream driver is not yet in a state where that is relevant.

The compatible values should be picked based on the full feature-set of
the HW, not based on the subset of features supported by a particular
driver.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 27/27] drm/tegra: Add Tegra114 gr2d support

[Stephen Warren]

On 10/14/2013 08:00 AM, Thierry Reding wrote:
 On Mon, Oct 14, 2013 at 08:58:34AM +0300, Terje Bergström wrote:
 On 12.10.2013 01:43, Stephen Warren wrote:
 On 10/07/2013 02:34 AM, Thierry Reding wrote:
 The gr2d hardware in Tegra114 is compatible with that of
 Tegra20 and Tegra30. No functionaly changes are required.
 Similarly here, if the HW is 100% backwards-compatible, there's
 no need to add compatible values to the driver.
 
 We've used this mechanism for attaching a per-hw-version data
 structure in match table to accomodate differences in how the
 hardware is power gated, reset, booted, some per-soc performance
 related changes etc. It's also used in staging features for new
 chips, such as disabling power features when they're not
 working/verified yet.
 
 Upstream driver is not yet in a state where that is relevant.
 
 With this, would we still be able to do that with match table? It
 sounds like we could, because we can still (even with multiple
 compatible properties) add separate entries in match table and I
 guess the compatible properties matched in order.
 
 Yes, as long as the device tree files includes the most specific
 value in the compatible this should still be possible. So we'd have
 this:
 
 gr2d@5414 { compatible = nvida,tegra114-gr2d,
 nvidia,tegra20-gr2d; ... };
 
 and the driver will match on nvidia,tegra20-gr2d if the more
 specific nvidia,tegra114-gr2d is not there. When the driver is
 updated to support Tegra114 specific functionality, then a more
 specific entry can be added to the compatible table to handle it.

True, but the DT fragment above is also only accurate /if/ a driver
that only knows about nvidia,tegra20-gr2d can operate 100% of the
features in Tegra20 HW on Tegra114 HW forever.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 26/27] drm/tegra: Add DSI support

[Stephen Warren]

On 10/14/2013 07:55 AM, Thierry Reding wrote:
 On Fri, Oct 11, 2013 at 04:43:35PM -0600, Stephen Warren wrote:
 On 10/07/2013 02:34 AM, Thierry Reding wrote:
 This commit adds support for both DSI outputs found on Tegra. Only very
 minimal functionality is implemented, so advanced features like ganged
 mode won't work.

 Due to the lack of other test hardware, some sections of the driver are
 hardcoded to work with Dalmore.

 diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c

 +static int tegra_dsi_show_regs(struct seq_file *s, void *data)
 +{
 +   struct drm_info_node *node = s-private;
 +   struct tegra_dsi *dsi = node-info_ent-data;
 +
 +#define DUMP_REG(name) \
 +   seq_printf(s, %-32s %#05x %08lx\n, #name, name,   \
 +  tegra_dsi_readl(dsi, name))
 +
 +   DUMP_REG(DSI_INCR_SYNCPT);

 Does it make sense to use an MMIO regmap instead? That way, you get all
 the debugfs files for free...
 
 As far as I know, regmap doesn't give you the symbolic names for the
 registers. I find that a rather useful feature because it allows to
 easily compare the registers to the ones in our downstream kernels.

True. However, we should really be writing user-space scripts to encode
that information. Such a script could be useful e.g. if reading the
information directly from /dev/mem or JTAG too, and bloating the kernel
with debug strings doesn't seem like a great idea.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 14/27] drm/tegra: Move driver to DRM tree

[Stephen Warren]

On 10/07/2013 02:34 AM, Thierry Reding wrote:
 In order to subsystem-wide changes easier, move the Tegra DRM driver
 ^^ make ?

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 15/27] gpu: host1x: Add support for Tegra114

[Stephen Warren]

On 10/07/2013 02:34 AM, Thierry Reding wrote:
 Tegra114 uses a slightly updated version of host1x with an additional
 syncpoint.

  drivers/gpu/host1x/hw/host1x02.c|  42 +
  drivers/gpu/host1x/hw/host1x02.h|  26 +++
  drivers/gpu/host1x/hw/hw_host1x02_channel.h | 121 ++
  drivers/gpu/host1x/hw/hw_host1x02_sync.h| 243 
 
  drivers/gpu/host1x/hw/hw_host1x02_uclass.h  | 175 

That seems like an awful lot of extra lines to support just one extra
syncpoint. Are there other changes? If not, can the code be
shared/parameterized somehow?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 16/27] drm/tegra: Add Tegra114 display controller support

[Stephen Warren]

On 10/07/2013 02:34 AM, Thierry Reding wrote:
 From: Mikko Perttunen mperttu...@nvidia.com
 
 The Tegra114 display controller is backwards-compatible with previous
 generations of the Tegra SoC. No code changes are required.

If the HW is backwards-compatible, then there's no need to add extra
compatible values to the driver; just write the following in the DT, and
it'll just work:

compatible = nvidia,tegra114-dc, nvidia,tegra30-dc;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 17/27] drm/tegra: Add Tegra114 HDMI support

[Stephen Warren]

On 10/07/2013 02:34 AM, Thierry Reding wrote:
 From: Mikko Perttunen mperttu...@nvidia.com
 
 Tegra114 TMDS configuration requires a new peak_current field and the
 driver current override bit has changed position.

 diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c

  static const struct tmds_config tegra2_tmds_config[] = {
 @@ -223,6 +224,85 @@ static const struct tmds_config tegra3_tmds_config[] = {

Not related to this patch, but those should have been named
tegra20_tmds_config[] and tegra30_tmds_config[].

  static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,

 - value = tmds-drive_current | DRIVE_CURRENT_FUSE_OVERRIDE;
 - tegra_hdmi_writel(hdmi, value, HDMI_NV_PDISP_SOR_LANE_DRIVE_CURRENT);
 + if (of_device_is_compatible(np, nvidia,tegra114-hdmi)) {

Let's not check this at run-time. Instead, host1x_drm_subdevs[]'s .data
field should be used to contain either flags or a pointer to a
configuration structure, either of which can be directly consulted to
determine the properties of the HW in a feature-oriented/semantic way.

drivers/gpio/gpio-tegra.c's
tegra20_gpio_config/tegra30_gpio_config/tegra_gpio_of_match provide a
good example of this.

This means that if Tegra124 is identical to Tegra114, yet a hypothetical
Tegra999 is different, you don't have to keep adjusting these if
conditions throughout the code; they can simply refer to the same
feature bit forever.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 17/27] drm/tegra: Add Tegra114 HDMI support

[Stephen Warren]

On 10/11/2013 04:19 PM, Stephen Warren wrote:
 On 10/07/2013 02:34 AM, Thierry Reding wrote:
 From: Mikko Perttunen mperttu...@nvidia.com

 Tegra114 TMDS configuration requires a new peak_current field and the
 driver current override bit has changed position.
 
 diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
 
  static const struct tmds_config tegra2_tmds_config[] = {
 @@ -223,6 +224,85 @@ static const struct tmds_config tegra3_tmds_config[] = {
 
 Not related to this patch, but those should have been named
 tegra20_tmds_config[] and tegra30_tmds_config[].

I see that patch 20 fixes this:-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 22/27] drm/panel: Add simple panel support

[Stephen Warren]

On 10/07/2013 02:34 AM, Thierry Reding wrote:
 Add a driver for simple panels. Such panels can have a regulator that
 provides the supply voltage and a separate GPIO to enable the panel.
 Optionally the panels can have a backlight associated with them so it
 can be enabled or disabled according to the panel's power management
 mode.
 
 Support is added for three panels: An AU Optronics 10.1 WSVGA, a
 Chunghwa Picture Tubes 10.1 WXGA and a Panasonic 10.1 WUXGA TFT LCD
 panel.

  .../devicetree/bindings/panel/auo,b101aw03.txt |   7 +
  .../bindings/panel/chunghwa,claa101wb03.txt|   7 +
  .../bindings/panel/panasonic,vvx10f004b00.txt  |   7 +
  .../devicetree/bindings/panel/simple-panel.txt |  21 ++

Since this patch defines new DT bindings, it should also be sent to the
DT binding maintainers and DT mailing list.

 diff --git a/drivers/gpu/drm/panel/panel-simple.c 
 b/drivers/gpu/drm/panel/panel-simple.c

 +static int panel_simple_remove(struct platform_device *pdev)
 +{
 + struct panel_simple *panel = platform_get_drvdata(pdev);
 +
 + if (gpio_is_valid(panel-enable_gpio)) {
 + if (panel-enable_gpio_flags  GPIO_ACTIVE_LOW)
 + gpio_set_value(panel-enable_gpio, 1);
 + else
 + gpio_set_value(panel-enable_gpio, 0);
 +
 + gpio_free(panel-enable_gpio);
 + }
 +
 + regulator_disable(panel-supply);

Can you just call panel_simple_disable() to do the HW cleanup, and just
do resource cleanup here?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 25/27] gpu: host1x: Add MIPI pad calibration support

[Stephen Warren]

On 10/07/2013 02:34 AM, Thierry Reding wrote:
 This driver adds support to perform calibration of the MIPI pads for CSI
 and DSI.

 diff --git a/drivers/gpu/host1x/mipi.c b/drivers/gpu/host1x/mipi.c

 +int tegra_mipi_calibrate(struct device *device)
...
 + err = of_parse_phandle_with_args(device-of_node, calibrate,
 +  #calibrate-cells, 0, args);
...
 +static struct of_device_id tegra_mipi_of_match[] = {
 + { .compatible = nvidia,tegra114-mipi, },

Is the DT binding for that compatible value documented anywhere? I'm not
sure what #calibrate-cells means; it doesn't seem to be used...
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 26/27] drm/tegra: Add DSI support

[Stephen Warren]

On 10/07/2013 02:34 AM, Thierry Reding wrote:
 This commit adds support for both DSI outputs found on Tegra. Only very
 minimal functionality is implemented, so advanced features like ganged
 mode won't work.
 
 Due to the lack of other test hardware, some sections of the driver are
 hardcoded to work with Dalmore.

 diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c

 +static int tegra_dsi_show_regs(struct seq_file *s, void *data)
 +{
 + struct drm_info_node *node = s-private;
 + struct tegra_dsi *dsi = node-info_ent-data;
 +
 +#define DUMP_REG(name)   \
 + seq_printf(s, %-32s %#05x %08lx\n, #name, name,   \
 +tegra_dsi_readl(dsi, name))
 +
 + DUMP_REG(DSI_INCR_SYNCPT);

Does it make sense to use an MMIO regmap instead? That way, you get all
the debugfs files for free...

 +static int tegra_dsi_probe(struct platform_device *pdev)

 + dsi-clk_parent = devm_clk_get(pdev-dev, parent);
 + if (IS_ERR(dsi-clk_parent))
 + return PTR_ERR(dsi-clk_parent);
...
 +static const struct of_device_id tegra_dsi_of_match[] = {
 + { .compatible = nvidia,tegra114-dsi, },

Is this DT binding documented? The clk_get() call above in particular
imposes the requirement that DT contain a clock with that name, which
should be part of the binding documentation.

Hopefully the values that this driver hard-codes won't be an issue for
the DT binding; we can simply make those values the default if
properties are missing. I assume it's likely that such a strategy will
work here?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 27/27] drm/tegra: Add Tegra114 gr2d support

[Stephen Warren]

On 10/07/2013 02:34 AM, Thierry Reding wrote:
 The gr2d hardware in Tegra114 is compatible with that of Tegra20 and
 Tegra30. No functionaly changes are required.

Similarly here, if the HW is 100% backwards-compatible, there's no need
to add compatible values to the driver.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv4 0/5] gpu: host1x: Add runtime pm support

[Stephen Warren]

On 10/08/2013 12:27 AM, Arto Merilainen wrote:
 This series adds runtime pm support for host1x, gr2d and dc. It retains the
 current behaviour if CONFIG_PM_RUNTIME is not enabled.
 
 The gr2d clock is enabled when a new job is submitted and disabled when
 the work is done. Due to parent-child relations between host1x-gr2d, this
 scheme enables and disables host1x clock.
 
 For dc, the clocks are enabled in .probe and disabled in .remove via runtime
 pm instead of direct clock APIs.
 
 Mayuresh is unfortunately not available to continue with the series and hence
 I will continue working on the patches.

The series, briefly,
Reviewed-by: Stephen Warren swar...@nvidia.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] simplefb: fix unmapping fb during destruction

[Stephen Warren]

On 10/02/2013 08:58 AM, David Herrmann wrote:
 Unfortunately, fbdev does not create its own struct device for
 framebuffers. Instead, it attaches to the device of the parent layer. This
 has the side-effect that devm_* managed resources are not cleaned up on
 framebuffer-destruction but rather during destruction of the
 parent-device. In case of fbdev this might be too late, though.
 remove_conflicting_framebuffer() may remove fbdev devices but keep the
 parent device as it is.
 
 Therefore, we now use plain ioremap() and unmap the framebuffer in the
 fb_destroy() callback. Note that we must not free the device here as this
 might race with the parent-device removal. Instead, we rely on
 unregister_framebuffer() as barrier and we're safe.

So, once the .fb_destroy callback has been executed, there's no other
callback to resurrect it? The framebuffer itself is still registered
until the device's remove, yet after .fb_destroy, the memory is
unmapped, which would be dangerous if the FB can be re-started.

If that's not an issue, this patch seems fine, so
Acked-by: Stephen Warren swar...@nvidia.com

 I know that simplefb was supposed to stay as simple as possible but I really
 think this series is the last set of fixes I have. Unfortunately framebuffer 
 DRM
 handover is mandatory so we cannot ignore it in simplefb.

I don't think this patch adds any significant complexity, so I'm not
worried at least.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv3 2/4] drm/tegra: Add runtime pm support for gr2d

[Stephen Warren]

On 09/24/2013 06:05 AM, Arto Merilainen wrote:
 From: Mayuresh Kulkarni mkulka...@nvidia.com
 
 This far we have enabled gr2d clock on device probe and disabled
 it on device deinitialisation. This patch adds runtime pm support
 for the hardware unit allowing dynamic power management. If pm
 runtime is not enabled, gr2d clock is enabled in device probe and

 diff --git a/drivers/gpu/host1x/drm/gr2d.c b/drivers/gpu/host1x/drm/gr2d.c

 @@ -327,11 +336,48 @@ static int __exit gr2d_remove(struct platform_device 
 *pdev)
   host1x_syncpt_free(gr2d-client.syncpts[i]);
  
   host1x_channel_free(gr2d-channel);
 +
 + if (pm_runtime_enabled(pdev-dev))
 + pm_runtime_disable(pdev-dev);
 + else
 + gr2d_runtime_suspend(pdev-dev);

This code is slightly different to the code in e.g.
sound/soc/tegra/tegra20_i2s.c:remove(), whereas the code in probe() is
identical. I'm not sure whether there's some advantage in this version?
If so, perhaps the sound drivers should be updated to be consistent. If
not, perhaps this driver should do the same thing as the I2S driver, so
we keep the drivers consistent, and provide the same example code
everywhere.

 +static int gr2d_runtime_suspend(struct device *dev)
 +{
 + struct gr2d *gr2d;
 +
 + gr2d = dev_get_drvdata(dev);
 + if (!gr2d)
 + return -EINVAL;

Presumably, gr2d will never be NULL here, unless there's some chronic
bug. Can't we re-write those last 5 lines as simply:

struct gr2d *grd2 = dev_get_drvdata(dev);

If that's not valid, we should probably update the audio drivers (and
perhaps others) too.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv3 4/4] gpu: host1x: Add runtime pm support for host1x

[Stephen Warren]

On 09/24/2013 06:05 AM, Arto Merilainen wrote:
 From: Mayuresh Kulkarni mkulka...@nvidia.com
 
 This patch adds runtime pm support for host1x hardware unit. This
 allows host1x clock to be turned off when it is idle. If pm runtime
 is not configured, we enable host1x clock in device probe and disable
 it in remove.

 diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c

 +static int host1x_runtime_suspend(struct device *dev);
 +static int host1x_runtime_resume(struct device *dev);

You could avoid having these prototypes by simply putting the function
bodies earlier on in the file, somewhere before they're used. I don't
care much either way, but I've certainly seen some people care about
this and ask for them to be moved.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/6] host1x: hdmi: Add Tegra114 support

[Stephen Warren]

On 08/28/2013 09:48 AM, Mikko Perttunen wrote:
 Add Tegra114 TMDS configuration, add new peak_current field and
 use new place for drive current override bit on Tegra114 platform.

 diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c

  static struct of_device_id host1x_of_match[] = {
 + { .compatible = nvidia,tegra114-host1x, .data = host1x01_info, },

We should add that value to the host1x DT binding documentation.

 diff --git a/drivers/gpu/host1x/drm/hdmi.c b/drivers/gpu/host1x/drm/hdmi.c

  static const struct tegra_hdmi_audio_config *
  tegra_hdmi_get_audio_config(unsigned int audio_freq, unsigned int pclk)
  {
 @@ -593,8 +673,20 @@ static void tegra_hdmi_setup_tmds(struct tegra_hdmi 
 *hdmi,
   tegra_hdmi_writel(hdmi, tmds-pll1, HDMI_NV_PDISP_SOR_PLL1);
   tegra_hdmi_writel(hdmi, tmds-pe_current, HDMI_NV_PDISP_PE_CURRENT);
  
 - value = tmds-drive_current | DRIVE_CURRENT_FUSE_OVERRIDE;
 - tegra_hdmi_writel(hdmi, value, HDMI_NV_PDISP_SOR_LANE_DRIVE_CURRENT);
 + if (of_device_is_compatible(hdmi-dev-of_node,
 + nvidia,tegra114-hdmi)) {

We shouldn't do this at run-time. Rather, set tegra_hdmi_of_match[]'s
.data field to a structure that represents the various features of the
HW, and then make this code conditional upon a feature flag in that
structure.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/6] host1x: hdmi: Detect whether display is connected with HDMI or DVI

[Stephen Warren]

On 08/28/2013 09:48 AM, Mikko Perttunen wrote:
 Use EDID data to determine whether the display supports HDMI or just DVI.
 This used to be hardcoded to be HDMI, which broke support for DVI displays
 that couldn't understand the interspersed audio/other data.
 
 If the EDID data isn't available, default to DVI, which should be a safer
 choice.

This seems like a bug-fix that might even be worth CC: stable?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/6] host1x: hdmi: Enable Vdd earlier for hotplug/DDC

[Stephen Warren]

On 08/28/2013 09:48 AM, Mikko Perttunen wrote:
 The Vdd regulator used to be enabled only at tegra_output_hdmi_enable,
 which is called after a sink is detected. However, the HDMI hotplug pin
 works by returning the voltage supplied by the Vdd pin, so this meant
 that the hotplug pin was never asserted and the sink was not detected
 unless the Vdd regulator was set to be always on.
 
 This patch moves the enable to the tegra_hdmi_drm_init function to make
 sure the regulator will get enabled.

The DT binding document isn't very clear on this topic (and should be
fixed): What is this regulator intended to control? If this regulator
solely controls the supply to the hotplug detection circuit, this change
makes sense. If the regulator mainly supplies something else (e.g. part
of the HDMI core on the Tegra chip), then perhaps this change isn't
correct. The correct approach might be to introduce another (optional)
regulator specifically for the hotplug circuit. Presumably both DT
properties vdd-supply and hotplug-supply could point at the same
regulator if that's the way the HW was wired up.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/6] clk: tegra114: Initialize clocks needed for HDMI

[Stephen Warren]

On 08/28/2013 09:48 AM, Mikko Perttunen wrote:
 Add host1x, disp1 and disp2 clocks to the clock initialization table.
 These clocks are required for HDMI support.

This patch should be sent to Mike Turquette so it can be taken through
the clock tree.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 5/6] ARM: tegra: Add host1x, DC and HDMI to Tegra114 device tree

[Stephen Warren]

On 08/28/2013 09:48 AM, Mikko Perttunen wrote:
 Add host1x, DC (display controller) and HDMI devices to Tegra114
 device tree.

Patches 5 and 6 should be sent separately to the Tegra maintainer (me)
to be taken through the Tegra tree.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/6] host1x: hdmi: Enable Vdd earlier for hotplug/DDC

[Stephen Warren]

On 09/04/2013 04:03 PM, Mikko Perttunen wrote:
 On 09/04/2013 09:44 PM, Stephen Warren wrote:
 On 08/28/2013 09:48 AM, Mikko Perttunen wrote:
 The Vdd regulator used to be enabled only at tegra_output_hdmi_enable,
 which is called after a sink is detected. However, the HDMI hotplug pin
 works by returning the voltage supplied by the Vdd pin, so this meant
 that the hotplug pin was never asserted and the sink was not detected
 unless the Vdd regulator was set to be always on.

 This patch moves the enable to the tegra_hdmi_drm_init function to make
 sure the regulator will get enabled.

 The DT binding document isn't very clear on this topic (and should be
 fixed): What is this regulator intended to control? If this regulator
 solely controls the supply to the hotplug detection circuit, this change
 makes sense. If the regulator mainly supplies something else (e.g. part
 of the HDMI core on the Tegra chip), then perhaps this change isn't
 correct. The correct approach might be to introduce another (optional)
 regulator specifically for the hotplug circuit. Presumably both DT
 properties vdd-supply and hotplug-supply could point at the same
 regulator if that's the way the HW was wired up.
 -- 
 To unsubscribe from this list: send the line unsubscribe linux-tegra in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

 
 AFAICT, it controls the Vdd pin on the HDMI port, so it just affects the
 hotplug pin and the DDC I2C bus power.

Ah OK, then this code change makes sense. It'd be useful to put what you
just wrote into the binding doc.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 2/3] drm/panel: Add simple panel support

[Stephen Warren]

On 08/30/2013 09:25 AM, Thierry Reding wrote:
 Add a driver for simple panels. Such panels can have a regulator that
 provides the supply voltage and a separate GPIO to enable the panel.
 Optionally the panels can have a backlight associated with them so it
 can be enabled or disabled according to the panel's power management
 mode.

 diff --git a/Documentation/devicetree/bindings/panel/panel-simple.txt 
 b/Documentation/devicetree/bindings/panel/panel-simple.txt

 +Simple display panel
 +
 +Required properties:
 +- compatible: should be one of:
 +  - auo,b101aw03: AU Optronics Corporation 10.1 WSVGA TFT LCD panel
 +  - cptt,claa101wb03: Chunghwa Picture Tubes Ltd. 10.1 WXGA TFT LCD panel
 +  - pc,vvx10f004b00: Panasonic Corporation 10.1 WUXGA TFT LCD panel
 +
 +Optional properties:
 +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
 +- power-supply: regulator to provide the supply voltage
 +- enable-gpios: GPIO pin to enable or disable the panel
 +- backlight: phandle of the backlight device attached to the panel

Do we need to represent the timing requirements e.g. between panel
enable and backlight enable, or do you expect the driver to hard-code
this based on the compatible value?

Looking at the driver code, it seems that it has direct knowledge of the
video mode that each panel supports, so DDC is actually optional unlike
what I asserted/assumed in my previous response. As such, I guess we
should have a separate DT binding document for each of the three panels
(compatible values) listed above that pretty much just says go look at
simple-panel.txt.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 2/3] drm/panel: Add simple panel support

[Stephen Warren]

On 08/30/2013 01:24 PM, Kumar Gala wrote:
 
 On Aug 30, 2013, at 10:25 AM, Thierry Reding wrote:
 
 Add a driver for simple panels. Such panels can have a regulator that
 provides the supply voltage and a separate GPIO to enable the panel.
 Optionally the panels can have a backlight associated with them so it
 can be enabled or disabled according to the panel's power management
 mode.

 Support is added for three panels: An AU Optronics 10.1 WSVGA, a
 Chunghwa Picture Tubes 10.1 WXGA and a Panasonic 10.1 WUXGA TFT LCD
 panel.

 diff --git a/Documentation/devicetree/bindings/panel/panel-simple.txt 
 b/Documentation/devicetree/bindings/panel/panel-simple.txt

 +Required properties:
 +- compatible: should be one of:
 +  - auo,b101aw03: AU Optronics Corporation 10.1 WSVGA TFT LCD panel
 +  - cptt,claa101wb03: Chunghwa Picture Tubes Ltd. 10.1 WXGA TFT LCD panel
 +  - pc,vvx10f004b00: Panasonic Corporation 10.1 WUXGA TFT LCD panel
 +
 
 It would seem there should be a more generic compatible to cover the basic 
 panel-simple case.

I would suggest only documenting simple-panel here, and not
documenting the specific panels at all; the panel-specific compatible
values would show up simply due to the rule that all compatible values
in *.dts should contain the exact HW model (e.g. auo,b101aw03), plus
any HW they're compatible with (i.e. simple-panel).

I'd suggest simple-panel rather than panel-simple since IIRC other
simple bindings are simple-xxx rather than xxx-simple.

 +Optional properties:
 +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
 +- power-supply: regulator to provide the supply voltage
 +- enable-gpios: GPIO pin to enable or disable the panel
 +- backlight: phandle of the backlight device attached to the panel
 +
 
 If these are all optional, what does it mean to be a panel-simple?

I think at least ddc-i2c-bus and backlight should be required
properties. I suppose it might be possible for the panel to be
always-on, and hence enable-gpios/power-supply could be optional?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[patch 2/2] gpu: host1x: returning success instead of -ENOMEM

[Stephen Warren]

On 08/23/2013 04:19 AM, Dan Carpenter wrote:
> There is a mistake here so it returns PTR_ERR(NULL) which is success
> instead of -ENOMEM.
> 
> Signed-off-by: Dan Carpenter 
> ---
> I can't compile this.

For the record, just do:

export CROSS_COMPILE=xxx
make ARCH=arm tegra_defconfig
make ARCH=arm zImage

The only slightly difficult part is getting a CROSS_COMPILE value. Many
distros ship at least some ARM cross-compiler in their standard
package-set these days, and if not, you can download the Linaro
compilers or go to ftp://ftp.kernel.org/pub/tools/crosstool/index.html.

Re: [patch 2/2] gpu: host1x: returning success instead of -ENOMEM

[Stephen Warren]

On 08/23/2013 04:19 AM, Dan Carpenter wrote:
 There is a mistake here so it returns PTR_ERR(NULL) which is success
 instead of -ENOMEM.
 
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
 ---
 I can't compile this.

For the record, just do:

export CROSS_COMPILE=xxx
make ARCH=arm tegra_defconfig
make ARCH=arm zImage

The only slightly difficult part is getting a CROSS_COMPILE value. Many
distros ship at least some ARM cross-compiler in their standard
package-set these days, and if not, you can download the Linaro
compilers or go to ftp://ftp.kernel.org/pub/tools/crosstool/index.html.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCHv2 5/5] ARM: dts: Add dt binding documentation for exynos rotator

[Stephen Warren]

On 08/09/2013 07:15 AM, Tomasz Figa wrote:
> Hi Chanho,
> 
> On Friday 09 of August 2013 16:40:53 Chanho Park wrote:
>> This patch describes each nodes of rotator and specifies a example how to
>> bind it.

>> diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt

>> +* Samsung Image Rotator
>> +
>> +Required properties:
>> +  - compatible : value should be following:
>> +(a) "samsung,exynos4210-rotator" for Rotator IP in Exynos4210
>> +(b) "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412
>> +(c) "samsung,exynos5250-rotator" for Rotator IP in Exynos5250

Is "rotator" the name that the HW documentation uses to refer to this
block? If so, those compatible values seem fine. If not, they seem
slightly too generic for me; perhaps better to use the raw HW block name?

Re: [PATCHv2 5/5] ARM: dts: Add dt binding documentation for exynos rotator

[Stephen Warren]

On 08/09/2013 07:15 AM, Tomasz Figa wrote:
 Hi Chanho,
 
 On Friday 09 of August 2013 16:40:53 Chanho Park wrote:
 This patch describes each nodes of rotator and specifies a example how to
 bind it.

 diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt

 +* Samsung Image Rotator
 +
 +Required properties:
 +  - compatible : value should be following:
 +(a) samsung,exynos4210-rotator for Rotator IP in Exynos4210
 +(b) samsung,exynos4212-rotator for Rotator IP in Exynos4212/4412
 +(c) samsung,exynos5250-rotator for Rotator IP in Exynos5250

Is rotator the name that the HW documentation uses to refer to this
block? If so, those compatible values seem fine. If not, they seem
slightly too generic for me; perhaps better to use the raw HW block name?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm: provide agp dummies for CONFIG_AGP=n

[Stephen Warren]

On 08/08/2013 02:19 PM, David Herrmann wrote:
> We currently rely on gcc dead-code elimination so the drm_agp_* helpers
> are not called if drm_core_has_AGP() is false. That's ugly as hell so
> provide "static inline" dummies for the case that AGP is disabled.
> 
> Fixes a build-regression introduced by:
> 
>   commit 28ec711cd427f8b61f73712a43b8100ba8ca933b
>   Author: David Herrmann 
>   Date:   Sat Jul 27 16:37:00 2013 +0200
> 
>   drm/agp: move AGP cleanup paths to drm_agpsupport.c

Tested-by: Stephen Warren 

Build break due to 28ec711 "drm/agp: move AGP cleanup paths to drm_agpsupport.c"

[Stephen Warren]

On 08/08/2013 12:13 PM, David Herrmann wrote:
> Hi
> 
> On Thu, Aug 8, 2013 at 8:00 PM, Stephen Warren  
> wrote:
>> In next-20130808, building tegra_defconfig for ARM yields:
>>
>>> drivers/built-in.o: In function `drm_lastclose':
>>> /home/swarren/shared/git_wa/kernel/kernel.git/drivers/gpu/drm/drm_drv.c:198:
>>>  undefined reference to `drm_agp_clear'
>>
>> That's because drm_agp_clear() is called unconditionally, yet is only
>> conditionally built into drm_agpsupport.c (#if __OS_HAS_AGP).
>>
>> Should the call from drm_drv.c be conditional, or should there be a
>> dummy static inline replacement in include/drm/drmP.h for when AGP
>> support isn't available?
> 
> Sorry, I missed testing with AGP=n and the code I fixed depended on
> dead-code-elimination to link correctly. I overlooked that. There is a
> patch pending on dri-devel:
> http://lists.freedesktop.org/archives/dri-devel/2013-August/043077.html

That makes it worse! To solve it, you need:

diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
index f926542..a184eee 100644
--- a/include/drm/drm_agpsupport.h
+++ b/include/drm/drm_agpsupport.h
@@ -8,7 +8,7 @@
 #include 
 #include 

-#ifdef __OS_HAS_AGP
+#if __OS_HAS_AGP

 void drm_free_agp(DRM_AGP_MEM * handle, int pages);
 int drm_bind_agp(DRM_AGP_MEM * handle, unsigned int start);