exynos4412: porting hdmiddc and hdmiphy node entries

exynos4412: porting hdmiddc and hdmiphy node entries

[Tobias Jakobi]

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

Hello,

I'm trying to get the HDMI port working on a Exynos4412 based board.
Attached is a snippet of a dts. This config was supposed to "work" in
the past.

However with 3.15-rc1 some things changed. samsung,exynos4210-hdmiddc
and samsung,exynos4212-hdmiphy have no function anymore, the code that
previously handled these compatible strings is gone.

So, it looks like that without some patching, HDMI support is atm broken.

I have applied these:
http://www.spinics.net/lists/linux-samsung-soc/msg28161.html
http://www.spinics.net/lists/linux-samsung-soc/msg28259.html

With the first one I can drop a clock from the hdmi node. But then
trouble starts.

So, first of all I'm unsure what the 'hdmiddc' node should be converted
to. Documentation (exynos_hdmi.txt) doesn't help here, since it just
says "phandle to the hdmi ddc node". What kind of 'hdmi ddc node'? From
the code it looks like that it should point to an i2c adapter now. So
should it point to 'i2c_2' now?

The second thing is 'phy', which should be a "phandle to the hdmi ddc
node". Again, no idea what that node should be. Apparantly such nodes
can't be created with current kernel code anyway, primary reason to
apply the simply-phy patches.

OK, so I have to put a simple-phys node in my dts now. Or, wait, do I
just replace the hdmiphy node with a simple-phys node?

Would look something like this:
hdmiphy: simple-phys@38 {
  compatible = "samsung,exynos4412-simple-phy";
  reg = <0x38 0x10000>;
  #phy-cells = <1>;
};

Somehow this doesn't look right. And indeed:
https://patchwork.kernel.org/patch/4021121/

At least for exynos5250 this node is not a child on an i2c adapter.

And yes, I would still have to add 'phys' and 'phy-names' to the hdmi
node. This looks wrong again. Why do I have to specify 'phys' when I
already have 'phy' there? Isn't that redundant?

Well, you see, lots of confusion here. I would appreciate any kind of
help on how to proceed here.

With best wishes,
Tobias

[-- Attachment #2: exynos-hdmi-dts.txt --]
[-- Type: text/plain, Size: 1311 bytes --]

hdmi {
	compatible = "samsung,exynos4212-hdmi";
	reg = <0x12D00000 0x100000>;
	interrupts = <0 92 0>;
	hpd-gpio = <&gpx3 7 0>;

	clocks = <&clock 271>, <&clock 136>, <&clock 139>,
		<&clock 178>, <&clock 396>;
	clock-names = "hdmi", "sclk_hdmi", "sclk_pixel",
		"sclk_hdmiphy", "mout_hdmi";

	pinctrl-names = "default";
	pinctrl-0 = <&hdmi_hpd_irq>;

	samsung,power-domain = <&pd_tv>;

	vdd_osc-supply = <&ldo10_reg>;
	vdd_pll-supply = <&ldo8_reg>;
	vdd-supply = <&ldo8_reg>;
	hdmi-en-supply = <&reg_sysvdd>;

	#address-cells = <1>;
	#size-cells = <1>;

	phy-power-control {
		reg = <0x10020700 0x04>;
	};
};

i2c_2: i2c@13880000 {
	status = "okay";
	pinctrl-0 = <&i2c2_bus>;
	pinctrl-names = "default";

	samsung,i2c-slave-addr = <0x10>;
	samsung,i2c-sda-delay = <100>;
	samsung,i2c-max-bus-freq = <400000>;

	hdmiddc@50 {
		compatible = "samsung,exynos4210-hdmiddc";
		reg = <0x50>;
	};
};

i2c_8: i2c@138e0000 {
	status = "okay";

	samsung,i2c-slave-addr = <0x10>;
	samsung,i2c-sda-delay = <100>;
	samsung,i2c-max-bus-freq = <400000>;

	compatible = "samsung,s3c2440-hdmiphy-i2c";
	reg = <0x138E0000 0x1000>;

	interrupts = <0 93 0>;

	#address-cells = <1>;
	#size-cells = <0>;

	clocks = <&clock 325>;
	clock-names = "i2c";

	hdmiphy@38 {
		compatible = "samsung,exynos4212-hdmiphy";
		reg = <0x38>;
	};
};

Re: exynos4412: porting hdmiddc and hdmiphy node entries

[Tomasz Stanislawski]

Hi Tobias,
Sorry for a late reply.
Please refer to the comments below.

On 04/27/2014 02:33 AM, Tobias Jakobi wrote:
> Hello,
> 
> I'm trying to get the HDMI port working on a Exynos4412 based board.
> Attached is a snippet of a dts. This config was supposed to "work" in
> the past.
> 
> However with 3.15-rc1 some things changed. samsung,exynos4210-hdmiddc
> and samsung,exynos4212-hdmiphy have no function anymore, the code that
> previously handled these compatible strings is gone.
> 
> So, it looks like that without some patching, HDMI support is atm broken.
> 
> I have applied these:
> http://www.spinics.net/lists/linux-samsung-soc/msg28161.html
> http://www.spinics.net/lists/linux-samsung-soc/msg28259.html
> 
> With the first one I can drop a clock from the hdmi node. But then
> trouble starts.
> 
> So, first of all I'm unsure what the 'hdmiddc' node should be converted
> to. Documentation (exynos_hdmi.txt) doesn't help here, since it just
> says "phandle to the hdmi ddc node". What kind of 'hdmi ddc node'? From
> the code it looks like that it should point to an i2c adapter now. So
> should it point to 'i2c_2' now?

The spec is wrong. It should be a handle to I2C adapter.
The semantics of this node was changed in patch
"drm/exynos: hdmi: use i2c_adapter instead of i2c_client"
8fa04aae2aa8bafcfc027856904ebee0060506d0

> 
> The second thing is 'phy', which should be a "phandle to the hdmi ddc
> node". Again, no idea what that node should be. Apparantly such nodes
> can't be created with current kernel code anyway, primary reason to
> apply the simply-phy patches.

The meaning of hdmiphy is ambiguous.
In exynos4 spec there are _TWO_ components named HDMIPHY.
The first one is a PLL that generates a clock for HDMI subsystem.
This PLL is controlled by I2C.

You can find an example of its bindings at:
http://lists.freedesktop.org/archives/dri-devel/2013-October/047687.html


The second one is HDMI's physical interface located in PMU unit.

The exynos-simple-phy is dedicated to controller the former (component of PMU).

The 'phy' attribute in DT refers to the PLL.
There is a debate if 'phy' should refer to I2C device itself or
rather to I2C bus. Currently it refers to driverless instance
of I2C device.

> 
> OK, so I have to put a simple-phys node in my dts now. Or, wait, do I
> just replace the hdmiphy node with a simple-phys node?
> 
> Would look something like this:
> hdmiphy: simple-phys@38 {
>   compatible = "samsung,exynos4412-simple-phy";
>   reg = <0x38 0x10000>;
>   #phy-cells = <1>;
> };
> 
> Somehow this doesn't look right. And indeed:
> https://patchwork.kernel.org/patch/4021121/
> 
> At least for exynos5250 this node is not a child on an i2c adapter.

This is a completely different HDMIPHY.

> 
> And yes, I would still have to add 'phys' and 'phy-names' to the hdmi
> node. This looks wrong again. Why do I have to specify 'phys' when I
> already have 'phy' there? Isn't that redundant?

'phys' and 'phy-names' refers to PHY interfaces delivered by phy-core.
The attribute named 'phy' refers to I2C device used to controller HDMI's PLL.
The naming convention is very misleading.

> 
> Well, you see, lots of confusion here. I would appreciate any kind of
> help on how to proceed here.

The documentation for HDMI's bindings should be fixed.

> 
> With best wishes,
> Tobias
> 

Regards,
Tomasz Stanislawski

Re: exynos4412: porting hdmiddc and hdmiphy node entries

[Tobias Jakobi]

Hello Tomasz,

thanks a lot for the support!

> The meaning of hdmiphy is ambiguous.
> In exynos4 spec there are _TWO_ components named HDMIPHY.
> The first one is a PLL that generates a clock for HDMI subsystem.
> This PLL is controlled by I2C.
> 
> You can find an example of its bindings at:
> http://lists.freedesktop.org/archives/dri-devel/2013-October/047687.html
OK, that looks exactly like the bindings I currently have in the dts.


> The second one is HDMI's physical interface located in PMU unit.
> 
> The exynos-simple-phy is dedicated to controller the former (component of PMU).
> 
> The 'phy' attribute in DT refers to the PLL.
> There is a debate if 'phy' should refer to I2C device itself or
> rather to I2C bus. Currently it refers to driverless instance
> of I2C device.
By 'driverless' you mean that the node isn't probed by any specific
driver code / the 'compatible' string isn't used anywhere in the kernel
(like it's currently the situation?)


>> Somehow this doesn't look right. And indeed:
>> https://patchwork.kernel.org/patch/4021121/
>>
>> At least for exynos5250 this node is not a child on an i2c adapter.
> 
> This is a completely different HDMIPHY.
I'm not sure I understand. Different from the PPL and the PMU HDMIPHY?


>> And yes, I would still have to add 'phys' and 'phy-names' to the hdmi
>> node. This looks wrong again. Why do I have to specify 'phys' when I
>> already have 'phy' there? Isn't that redundant?
> 
> 'phys' and 'phy-names' refers to PHY interfaces delivered by phy-core.
> The attribute named 'phy' refers to I2C device used to controller HDMI's PLL.
> The naming convention is very misleading.
Hmm, indeed this is very confusing. So with the move to
exynos-simple-phy, I guess I need these entries as well, am I correct?


>> Well, you see, lots of confusion here. I would appreciate any kind of
>> help on how to proceed here.
> 
> The documentation for HDMI's bindings should be fixed.
Well, I can agree with that :)


With best wishes,
Tobias

Re: exynos4412: porting hdmiddc and hdmiphy node entries

[Tobias Jakobi]

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

Hello,

I made some progress with the HDMI output on the board.

I updated to v4 of the exynos-simple-phy driver.
@Rahul: You can add a tested-by from me, if you want.

My dts now looks like this:
https://github.com/tobiasjakobi/linux-odroid/blob/odroid-3.15.y/arch/arm/boot/dts/exynos4412-odroidx2.dts

This works, but I have to force disabling of the LCD0 powerdomain:
https://github.com/tobiasjakobi/linux-odroid/commit/65b97415b90f54240e03a065cfea1097629fb17e

It looks like that the HDMI doesn't work properly when LCD0 pd is
switched off, which it normally is for my use case. The nodes related to
HDMI also don't seem to reference it. It seems like only the TV pd is
referenced, and from the looks of the kernel log this one gets
enabled/disabled properly.
@Inki: Is this a known problem?

Another issue which I then encountered can be easily triggered with
'modetest' from libdrm/tests:
modetest -M exynos -v -s 15@6:640x480

This triggers a kernel warning (mixer_dpms). I attached the full output.
Note that the warning appear when exiting modetest (so it's probably
triggeed by crtc restore or something).

With best wishes,
Tobias


[-- Attachment #2: exynos_mixer_warning.txt --]
[-- Type: text/plain, Size: 4443 bytes --]

[   86.760000] WARNING: CPU: 0 PID: 2512 at drivers/gpu/drm/exynos/exynos_mixer.c:620 mixer_dpms+0x54c/0x674()
[   86.760000] failed to reset Video Processor
[   86.760000] Modules linked in: bridge stp llc bnep rfcomm ecb btusb bluetooth s5p_mfc usb_storage videobuf2_dma_contig videobuf2_memops videobuf2_core
[   86.760000] CPU: 0 PID: 2512 Comm: modetest Not tainted 3.15.0-rc5+ #13
[   86.760000] Backtrace: 
[   86.760000] [<c0011a5c>] (dump_backtrace) from [<c0011bf8>] (show_stack+0x18/0x1c)
[   86.760000]  r6:0000026c r5:00000009 r4:00000000 r3:00000000
[   86.760000] [<c0011be0>] (show_stack) from [<c041f494>] (dump_stack+0x88/0xd4)
[   86.760000] [<c041f40c>] (dump_stack) from [<c0023a6c>] (warn_slowpath_common+0x6c/0x90)
[   86.760000]  r4:e5919a98 r3:e5918000
[   86.760000] [<c0023a00>] (warn_slowpath_common) from [<c0023b34>] (warn_slowpath_fmt+0x38/0x40)
[   86.760000]  r8:c05be648 r7:00000000 r6:c059e414 r5:e6395410 r4:00000000
[   86.760000] [<c0023b00>] (warn_slowpath_fmt) from [<c0260fc8>] (mixer_dpms+0x54c/0x674)
[   86.760000]  r3:c01c9268 r2:c0516aa0
[   86.760000] [<c0260a7c>] (mixer_dpms) from [<c024ffa4>] (exynos_drm_crtc_dpms+0x74/0x114)
[   86.760000]  r10:00000001 r9:e612c9d4 r8:c05be648 r7:e5399780 r6:00000000 r5:c05ff240
[   86.760000]  r4:e612d000
[   86.760000] [<c024ff30>] (exynos_drm_crtc_dpms) from [<c02500f8>] (exynos_drm_crtc_commit+0x1c/0x4c)
[   86.760000]  r8:e612c800 r7:e5399780 r6:e6129e40 r5:c05be648 r4:e612d000
[   86.760000] [<c02500dc>] (exynos_drm_crtc_commit) from [<c0230954>] (drm_crtc_helper_set_mode+0x3c4/0x518)
[   86.760000]  r5:e612c9d8 r4:e612d000
[   86.760000] [<c0230590>] (drm_crtc_helper_set_mode) from [<c0231354>] (drm_crtc_helper_set_config+0x778/0x9d0)
[   86.760000]  r10:e618fc80 r9:00000000 r8:c05ff240 r7:e612c9d8 r6:e612c9c0 r5:e612c9b4
[   86.760000]  r4:e612d000
[   86.760000] [<c0230bdc>] (drm_crtc_helper_set_config) from [<c0241e74>] (drm_mode_set_config_internal+0x60/0xec)
[   86.760000]  r10:c05ff240 r9:e612c88c r8:00000000 r7:e6384300 r6:e618fc80 r5:e612d000
[   86.760000]  r4:e6398900
[   86.760000] [<c0241e14>] (drm_mode_set_config_internal) from [<c0234a10>] (drm_fb_helper_restore_fbdev_mode+0xb8/0xd8)
[   86.760000]  r6:e618fc80 r5:00000000 r4:00000001 r3:00000000
[   86.760000] [<c0234958>] (drm_fb_helper_restore_fbdev_mode) from [<c0250e00>] (exynos_drm_fbdev_restore_mode+0x34/0x40)
[   86.760000]  r8:e612c800 r7:e612c800 r6:e612c838 r5:e612c800 r4:e618f200
[   86.760000] [<c0250dcc>] (exynos_drm_fbdev_restore_mode) from [<c024f6f4>] (exynos_drm_lastclose+0x10/0x14)
[   86.760000]  r5:e612c850 r4:e5a61480
[   86.760000] [<c024f6e4>] (exynos_drm_lastclose) from [<c02389cc>] (drm_lastclose+0x38/0x154)
[   86.760000] [<c0238994>] (drm_lastclose) from [<c0238eac>] (drm_release+0x3c4/0x5bc)
[   86.760000]  r10:e6127f80 r9:e612c88c r8:e5a614f4 r7:e612c800 r6:e612c838 r5:e612c850
[   86.760000]  r4:e5a61480 r3:00000000
[   86.760000] [<c0238ae8>] (drm_release) from [<c00d7a8c>] (__fput+0x88/0x1c4)
[   86.760000]  r10:00000000 r9:e5a616c8 r8:00000008 r7:e5c46990 r6:e59483d0 r5:e632b100
[   86.760000]  r4:e5a616c0
[   86.760000] [<c00d7a04>] (__fput) from [<c00d7c2c>] (____fput+0x10/0x14)
[   86.760000]  r10:e5990ff8 r9:418004fc r8:e5918000 r7:e50ac800 r6:00000000 r5:c05ceb70
[   86.760000]  r4:e50acbb0
[   86.760000] [<c00d7c1c>] (____fput) from [<c003c528>] (task_work_run+0xb0/0xe4)
[   86.760000] [<c003c478>] (task_work_run) from [<c0025894>] (do_exit+0x2b4/0x8e4)
[   86.760000]  r7:e5990fc0 r6:e50ac800 r5:00000002 r4:e50acbc8
[   86.760000] [<c00255e0>] (do_exit) from [<c0025fd8>] (do_group_exit+0x44/0xb8)
[   86.760000]  r7:e5bee8c4
[   86.760000] [<c0025f94>] (do_group_exit) from [<c00310e8>] (get_signal_to_deliver+0x1e4/0x558)
[   86.760000]  r7:e5bee8c4 r6:e5918000 r5:e5919edc r4:e5918000
[   86.760000] [<c0030f04>] (get_signal_to_deliver) from [<c041c554>] (do_signal+0xb4/0x3d0)
[   86.760000]  r10:b6def2ec r9:e5918000 r8:b6def2f0 r7:fffffdfe r6:e5918000 r5:00000001
[   86.760000]  r4:e5919fb0
[   86.760000] [<c041c4a0>] (do_signal) from [<c0011568>] (do_work_pending+0x8c/0xcc)
[   86.760000]  r10:00000000 r8:e5919fb0 r7:c000eca4 r6:e5918000 r5:e5918000 r4:bec65e18
[   86.760000] [<c00114dc>] (do_work_pending) from [<c000eb60>] (work_pending+0xc/0x20)
[   86.760000]  r8:c000eca4 r7:0000008e r6:bec65e18 r5:00000003 r4:bec65e18 r3:00000000
[   86.760000] ---[ end trace de2bb103e2dbf872 ]---