[PATCH V2 4/4] board: rockchip: Enable PD_VO before driver access

[Chris Morgan]

On Tue, Sep 24, 2024 at 11:19:49AM +0200, Quentin Schulz wrote:
> Hi Chris,
> 
> On 9/23/24 7:38 PM, Chris Morgan wrote:
> > On Mon, Sep 23, 2024 at 01:24:34PM +0200, Quentin Schulz wrote:
> > > Hi Chris,
> > > 
> > > On 9/19/24 4:00 PM, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan at hotmail.com>
> > > > 
> > > > Enable the PD_VO power domain before driver access on the rk3568 SoC.
> > > > 
> > > > Signed-off-by: Chris Morgan <macromorgan at hotmail.com>
> > > > ---
> > > >    arch/arm/mach-rockchip/rk3568/rk3568.c | 6 ++++++
> > > >    1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > > > index 1b3e40074e3..d5742b68d68 100644
> > > > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > > > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > > > @@ -26,6 +26,8 @@
> > > >    #define PMU_BASE_ADDR		0xfdd90000
> > > >    #define PMU_NOC_AUTO_CON0	(0x70)
> > > >    #define PMU_NOC_AUTO_CON1	(0x74)
> > > > +#define PMU_PWR_GATE_SFTCON	(0xa0)
> > > > +#define PMU_PD_VO_DWN_ENA	BIT(7)
> > > >    #define EDP_PHY_GRF_BASE	0xfdcb0000
> > > >    #define EDP_PHY_GRF_CON0	(EDP_PHY_GRF_BASE + 0x00)
> > > >    #define EDP_PHY_GRF_CON10	(EDP_PHY_GRF_BASE + 0x28)
> > > > @@ -130,6 +132,10 @@ int arch_cpu_init(void)
> > > >    	writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_1);
> > > >    	writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_2);
> > > >    	writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_3);
> > > > +
> > > > +	/* Enable VO power domain for display */
> > > > +	writel((PMU_PD_VO_DWN_ENA << 16),
> > > > +	       PMU_BASE_ADDR + PMU_PWR_GATE_SFTCON);
> > > 
> > > Maybe we want to guard this with a symbol for video stuff in U-Boot? I
> > > assume Linux is capable of handling this just fine without U-Boot's
> > > intervention?
> > > 
> > > At the very least have #if CONFIG_IS_ENABLED(VIDEO) ?
> > > 
> > > What do you think?
> > 
> > I'm game if you are. To me this just brings back the default behavior
> > of the binary A-TF from Rockchip where the PD_VO was enabled, but if
> > we want to guard this I'm okay.
> > 
> 
> Just to be sure I understand correctly...
> 
> Would a device without this power domain enabled by U-Boot still have
> working video in Linux with upstream TF-A?

Ideally yes. I assume this requires a functional power domain driver in
Linux. If you lack such then video won't work without this patch,
conversely if you lack such and don't use video then you'll have higher
power draw with this patch.

Honestly I figure we can guard it for now and if we find upstream video
issues on mainline A-TF we know this is a great place to start for
troubleshooting it.

Chris

> 
> Cheers,
> Quentin