Re: [PATCH] tegra: ventana: display and backlight DT entries

[Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3297 bytes --]

On Tue, Nov 13, 2012 at 07:23:24PM +0900, Alexandre Courbot wrote:
> Enable internal panel:
> - add EDID file
> - add power sequence to control backlight and panel (panel is currently
>   controlled by the backlight sequence, this will need to be fixed once
>   the panel framework has power sequences support)
> 
> Also enable HDMI output.
> 
> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Hi Alex,

Thanks for the patch. It seems like we may actually get the tegra-drm in
for 3.8, so I think it's time we start collecting patches to the DTS
files to enable it on the various boards. I can collect these in my
tegra/next branch to provide a common location and feed them to Stephen
at the appropriate time.

I think the canonical subject for Tegra-related patches against arch/arm
is: "ARM: tegra". A few more comments inline.

> ---
>  arch/arm/boot/dts/tegra20-ventana.dts  | 114 +++++++++++++++++++++++++++++++--
>  arch/arm/boot/dts/tegra20-ventana.edid | Bin 0 -> 128 bytes
>  2 files changed, 109 insertions(+), 5 deletions(-)
>  create mode 100644 arch/arm/boot/dts/tegra20-ventana.edid
> 
> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
> index adc4754..849a374 100644
> --- a/arch/arm/boot/dts/tegra20-ventana.dts
> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
> @@ -318,11 +318,30 @@
>  		};
>  	};
>  
> -	i2c@7000c400 {
> +	hdmiddc: i2c@7000c400 {
>  		status = "okay";
>  		clock-frequency = <400000>;
>  	};

I believe you said Mark already told you, but this probably needs to
reference the DDC in the i2c-pinmux rather than the physical parent.

>  
> +	host1x {
> +                dc@54200000 {

This looks like it is indented using spaces instead of tabs.

> +			rgb {
> +				status = "okay";
> +				nvidia,edid = /incbin/("tegra20-ventana.edid");

We've briefly discussed this on IRC already, but for the sake of
completeness I'll restate it here. I think this should be converted to
the bindings as defined by the videomode helpers. These are not merged
yet, but they provide a much more readable representation than a binary
blob.

I know that Stephen mentioned using the nvidia,edid property for boards
where the blob is actually available in some sort. I seem to remember
him mentioning Ventana in particular, but I may be wrong.

> +			};
> +		};
> +
> +		hdmi {
> +			status = "okay";
> +
> +			vdd-supply = <&avddhdmi>;
> +			pll-supply = <&avddhdmipll>;
> +
> +			nvidia,hpd-gpio = <&gpio 111 0>; /* PN7 */
> +			nvidia,ddc-i2c-bus = <&hdmiddc>;
> +		};
> +	};
> +

The host1x node should be sorted higher, according to its unit address.
I believe the right place would be immediately above the pinmux node.

> +	backlight {
> +		compatible = "pwm-backlight";
> +		brightness-levels = <0 16 32 48 64 80 96 112 128 144 160 176 192 208 224 240 255>;
> +		default-brightness-level = <12>;
> +
> +		/* resources used by the power sequences */
> +		pwms = <&pwm 2 5000000>;
> +		pwm-names = "backlight";
> +		power-supply = <&vdd_bl_reg>;
> +		panel-supply = <&vdd_panel_reg>;
> +
> +		power-sequences {
> +			power-on {
> +				/*
> +				 * Panel power and LDVS enable.

"LVDS".

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]