[PATCH 3/3] board: rockchip: Fix panel detection for mainline A-TF

[Kever Yang]

Hi Chris,

On 2024/9/18 21:38, Chris Morgan wrote:
> On Wed, Sep 18, 2024 at 11:06:34AM +0800, Kever Yang wrote:
>> Hi Chris,
>>
>> Please update the subject with something like "Enable the VO PD before
>> driver access",
>>
>> and the commit message for the change reason is enough.
>>
>> and the config is more soc level, I think you can add this to
>> arch_cpu_init() for we don't have a power domain driver.
> To my knowledge I'm the only board trying to access the video before
> Linux proper. Do you want me to add this for all rk3568 boards then
> (from within rk3568.c under arch_cpu_init() as you say)?
Yes, please move it to rk3568.c in case other board need it.
>   I'm thinking
> if I do though I can at least get rid of the memory barrier.
>
>> On 2024/9/17 05:01, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan at hotmail.com>
>>>
>>> The current panel detection logic crashes when the device boots with
>>> mainline A-TF, causing a reboot loop. It turns out mainline A-TF
>>> doesn't enable the VO power domain like the BSP A-TF did.
>>>
>>> Set the VO domain on and use a memory barrier to ensure it is powered
>>> up before we attempt to do the panel detection.
>>>
>>> Signed-off-by: Chris Morgan <macromorgan at hotmail.com>
>>> ---
>>>    board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 13 +++++++++++++
>>>    configs/anbernic-rgxx3-rk3566_defconfig    |  2 --
>>>    2 files changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
>>> index c1d1826fd14..f4e7c1ab360 100644
>>> --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
>>> +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
>>> @@ -22,6 +22,10 @@
>>>    DECLARE_GLOBAL_DATA_PTR;
>>> +#define PMU_BASE_ADDR		0xfdd90000
>>> +#define PMU_PWR_GATE_SFTCON	0xa0
>>> +#define PMU_PD_VO_DWN_ENA	BIT(7)
>>> +
>>>    #define GPIO0_BASE		0xfdd60000
>>>    #define GPIO4_BASE		0xfe770000
>>>    #define GPIO_SWPORT_DR_L	0x0000
>>> @@ -311,6 +315,15 @@ int rgxx3_detect_display(void)
>>>    	int i;
>>>    	u8 panel_id[2];
>>> +	/* Disable VO power domain power-down */
>> Disable?
> It's weird verbiage but the data sheet says "Enable power down PD_VO"
> by writing a 1, so I figure I'm disabling it by writing a 0.

OK, understand now.

To make it simple and clear, could you just say "Enable PD_VO for display"?

>
>>> +	writel((PMU_PD_VO_DWN_ENA << 16),
>>> +	       PMU_BASE_ADDR + PMU_PWR_GATE_SFTCON);
>>> +	/*
>>> +	 * System will crash if the power domain isn't enabled before
>>> +	 * we start trying to talk to the DSI panel.
>>> +	 */
>>> +	wmb();
>>> +
>>>    	/*
>>>    	 * Take panel out of reset status.
>>>    	 * Set GPIO4_A0 to output.
>>> diff --git a/configs/anbernic-rgxx3-rk3566_defconfig b/configs/anbernic-rgxx3-rk3566_defconfig
>>> index f5e5470df8c..49d3766613e 100644
>>> --- a/configs/anbernic-rgxx3-rk3566_defconfig
>>> +++ b/configs/anbernic-rgxx3-rk3566_defconfig
>>> @@ -49,8 +49,6 @@ CONFIG_SPL_REGMAP=y
>>>    CONFIG_SPL_SYSCON=y
>>>    CONFIG_SPL_ADC=y
>>>    CONFIG_SPL_CLK=y
>>> -CONFIG_ARM_SMCCC_FEATURES=y
>>> -CONFIG_SCMI_FIRMWARE=y
>> This should be a separate patch, and what's the change?
>>
>>
> Mainline A-TF seems to get angry with U-Boot when these are enabled.
> By angry I mean it freezes. It's all a related change to allowing
> mainline A-TF, though I should probably call it out better as a
> required change. Again I also specify though in the commit message
> that mainline A-TF still doesn't work with Linux proper because of
> the missing psci clk and psci reset bits, but at least with this
> change U-Boot won't be contributing to the problem (for this board).

The scmi clock is missing on rk3568 mainline A-TF now, but it should 
return error with something like

NOT SUPPORT instead of hang inside.

Any way, this is a separate change, we don't need it if the ATF supports 
scmi clock later.

Thanks,

- Kever

>> Thanks,
>>
>> - Kever
>>
>>>    CONFIG_ROCKCHIP_GPIO=y
>>>    CONFIG_SYS_I2C_ROCKCHIP=y
>>>    CONFIG_MISC=y
> Thank you,
> Chris