Re: [RFC v2 6/8] MIPS: DTS: jz4780: account for Synopsys HDMI driver and LCD controller

[Paul Cercueil <paul@crapouillou.net>]

Hi Nikolaus,


Le mer., mars 11, 2020 at 13:43, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> Hi Paul,
> 
>>  Am 02.03.2020 um 20:27 schrieb Paul Cercueil <paul@crapouillou.net>:
>> 
>>  Hi Nikolaus,
>> 
>> 
>>  Le ven., févr. 28, 2020 at 19:19, H. Nikolaus Schaller 
>> <hns@goldelico.com> a écrit :
>>>  From: Paul Boddie <paul@boddie.org.uk>
>>>  A specialisation of the generic Synopsys HDMI driver is employed 
>>> for JZ4780
>>>  HDMI support. This requires a new driver, plus device tree and 
>>> configuration
>>>  modifications.
>>>  Signed-off-by: Paul Boddie <paul@boddie.org.uk>
>>>  Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>  ---
>>>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 32 
>>> ++++++++++++++++++++++++++
>>>  1 file changed, 32 insertions(+)
>>>  diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi 
>>> b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>  index f928329b034b..391d4e1efd35 100644
>>>  --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>  +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>  @@ -433,4 +433,36 @@
>>>  		status = "disabled";
>>>  	};
>>>  +
>>>  +	hdmi: hdmi@10180000 {
>>>  +		compatible = "ingenic,jz4780-dw-hdmi";
>>>  +		reg = <0x10180000 0x8000>;
>>>  +		reg-io-width = <4>;
>>>  +
>>>  +		clocks = <&cgu JZ4780_CLK_HDMI>, <&cgu JZ4780_CLK_AHB0>;
>>>  +		clock-names = "isfr" , "iahb";
>>>  +
>>>  +		assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
>>>  +		assigned-clock-rates = <27000000>;
>> 
>>  I *think* this should go to the board file.
>> 
>>>  +
>>>  +		interrupt-parent = <&intc>;
>>>  +		interrupts = <3>;
>>>  +
>>>  +		/* ddc-i2c-bus = <&i2c4>; */
>>>  +
>>>  +		status = "disabled";
>>>  +	};
>>>  +
>>>  +	lcd: lcd@13050000 {
>> 
>>  The node name should be 'lcd-controller'.
>> 
>>>  +		compatible = "ingenic,jz4740-lcd";
>> 
>>  The JZ4780's LCD controller is much newer than the JZ4740 one, so 
>> even if it works with the "ingenic,jz4740-lcd" compatible string, 
>> you want it as a fallback.
>>  So this should be: compatible = "ingenic,jz4780-lcd", 
>> "ingenic,jz4740-lcd".
>> 
>>  That means the YAML should be updated too.
> 
> I have started to look into jz4780 HDMI setup again.
> 
> Well, there is no driver compatible to "ingenic,jz4780-lcd" so far
> and it is questionalbe if we need a different one.
> 
> I think we should rather make the driver also compatible
> than adding a fallback to ingenic,jz4740-lcdto the DTS.
> 
> The reason why this is better even if both LCDC are almost
> compatible is that the jz4780 allows for much bigger displays
> and therefore should have its own jz_soc_info with 4k x 2k
> as maximum.

Sure, feel free to extend the driver.

> Next I tried to find out if the LCDC are really compatible.
> 
> Well the jz4780 has two lcdc instances but they are separated
> by the reg addr. Next, there are unique features (like picture in
> picture with alpha blending) but those are probably disabled
> if not programmed from reset state. This may become a reason
> to separate or augment the driver for the jz4780 but at the
> moment we can ignore that.

Two LCDC instances -> two lcd-controller@... nodes. It's that simple.

The other features you listed are outside the LCDC, so outside the 
scope of this driver.

> There are also subtly different bit definitions and register
> widths (e.g. 24 bit in addition to 16/18 bit modes or more bits
> for the sync position) but it looks as if the ingenic_drm
> driver already handles this.
> 
> Then I tried to read back the registers. Strangely they
> are all 0x00000000. So there is no programming of the
> lcd-controller in our DT setup with HDMI at all!

How did you read them?
Do it from the regmap: should be "cat 
/sys/kernel/debug/regmap/13050000.lcd-controller/registers" (not sure 
about the path)

> I also checked that ingenic_drm_probe() is called and
> returns successfully 0. It also reports that a /dev/fb
> has been created:
> 
> [    7.908830] ingenic-drm 13050000.lcd-controller: fb0: 
> ingenic-drmdrmf frame buffer device
> 
> But for example ingenic_drm_encoder_atomic_mode_set() is
> never called which should write some registers of the LCDC.
> 
> I only did see some calls to ingenic_drm_encoder_atomic_check().
> 
> This of course explains why we have no HDMI signals despite
> proper HPD and a /dev/fb0. Because the LCDC is not being
> programmed.

It won't be called until the HDMI driver says that the cable is 
plugged, and there's a client application (e.g. fbdev emulation) 
running. So the problem is most likely within the HDMI driver.

Cheers,
-Paul

> Any ideas / hints how to check or improve?
> 
> BR and thanks,
> Nikolaus
>