[RFC 14/16] drm/armada: optionally enable the AXI clock

[Lubomir Rintel]

On Thu, 2019-01-17 at 11:09 +0000, Russell King - ARM Linux admin
wrote:
> On Tue, Dec 18, 2018 at 04:37:40PM +0100, Lubomir Rintel wrote:
> > It needs to be enabled (at least on MMP2) in order for the register
> > writes to LCDC to work.
> 
> Dove also has an AXI clock input to the LCDC, but it isn't clear
> whether that is necessary for register access or not.  From a quick
> search of the documentation, there doesn't appear to be an enable bit
> for the AXI clock.  Unfortunately, the documentation of clocks on Dove
> is not very good.
> 
> However, Dove LCDCs can have four clock inputs for the pixel clock -
> AXI, PLL and two external clock inputs.  It isn't clear what this AXI
> clock is, and whether it's the same clock used for register access.

The MMP2 LCDC also has four. I don't have the documentation but
according to James [1], who has one, there are:

0 - AXI clock
1 - Display 1, from APMU
2 - Display 2, from APMU
3 - HDMI PLL

[1] https://lists.freedesktop.org/archives/dri-devel/2018-December/201021.html

> Can you check whether the AXI clock used for the pixel clock is the
> same as the AXI bus clock for MMP2 and document that please?

Turns out -- it is not. The PMU generates a separate "peripheral" clock
that is required for the LCDC register access and two "AXI" clocks
(with a configurable divider).

However, the clk-of-mmp2 driver confuses this a bit: it exports the
first AXI clock together with peripheral clock as a single clock [2].

[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mmp/clk-of-mmp2.c?h=v4.20#n232

There's no documentation, but openfirmware strongly suggests it is the
case [3] (d428284c is the APMU's Display 1 control register) :

   setreg d428284c 08    \ PMUA_DISPLAY1_CLK_RES_CTL - AXI Clk enabled
   setreg d428284c 09    \ plus AXI released from reset
   setreg d428284c 19    \ plus peripheral clock enabled
   setreg d428284c 1b    \ plus peripheral released from reset

[3] http://dev.laptop.org/git/users/quozl/openfirmware/tree/cpu/arm/mmp2/sp.bth#n322

I guess I'll need to fix clk-of-mmp2 to expose the two as separate
clocks...

Here are the configurations I've observed to work on the XO laptop,
confirming the above:

(0xd428284c = PMUA_DISPLAY1_CLK_RES_CTL, APMU Display 1 clocks
0xd4282910 = PMUA_DISPLAY1_CLK_RES_CTL, APMU Display 2 clocks
0xd420b1a8 = LCDC LCD_CFG_SCLK_DIV)

  Only the peripheral clock enabled in PMUA:
  # busybox devmem 0xd428284c 32 0x00000009
  # busybox devmem 0xd4282910 32 0x00000000
  Let LCDC use its AXI clock (0), divisor 5 (dunno why)
  # busybox devmem 0xd420b1a8 32 0x00001105
 
  Enable perhipheral clock and generate display 1 clock
  from AXI, divisor 7
  # busybox devmem 0xd428284c 32 0x0000071b
  # busybox devmem 0xd4282910 32 0x00000000
  Pick display 1 clock, divisor 2
  # busybox devmem 0xd420b1a8 32 0x40001102
  This is the configuration used by OLPC firmware
 
  Enable perhipheral clock and generate display 2 clock
  from AXI, divisor 7
  # busybox devmem 0xd428284c 32 0x00000009
  # busybox devmem 0xd4282910 32 0x00000712
  Pick display 2 clock, divisor 2
  # busybox devmem 0xd420b1a8 32 0x80001102

I don't actually have a clue which clock is actually used as an input
when the LCDC is configured to use "AXI" clock. I don't think I need to
care at this point -- I'm not going to use it and will go with
"Display  1".

Please let me know if the above makes sense to you and I'll go ahead
and prepare (hopefully well commented) patches.

Thank you
Lubo

> 
> Thanks.
> 
> > Signed-off-by: Lubomir Rintel <lkundrak at v3.sk>
> > ---
> >  drivers/gpu/drm/armada/armada_crtc.c | 7 +++++++
> >  drivers/gpu/drm/armada/armada_crtc.h | 1 +
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> > index 5400fb925bcd..973c377975a1 100644
> > --- a/drivers/gpu/drm/armada/armada_crtc.c
> > +++ b/drivers/gpu/drm/armada/armada_crtc.c
> > @@ -679,6 +679,7 @@ static void armada_drm_crtc_destroy(struct drm_crtc *crtc)
> >  
> >  	of_node_put(dcrtc->crtc.port);
> >  
> > +	clk_disable_unprepare(dcrtc->axiclk);
> >  	kfree(dcrtc);
> >  }
> >  
> > @@ -748,6 +749,11 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
> >  	dcrtc->clk = ERR_PTR(-EINVAL);
> >  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> >  
> > +	dcrtc->axiclk = devm_clk_get(dev, "axiclk");
> > +	if (IS_ERR(dcrtc->axiclk))
> > +		dcrtc->axiclk = NULL;
> > +	WARN_ON(clk_prepare_enable(dcrtc->axiclk));
> > +
> >  	endpoint = of_get_next_child(port, NULL);
> >  	of_property_read_u32(endpoint, "bus-width", &bus_width);
> >  	of_node_put(endpoint);
> > @@ -829,6 +835,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
> >  err_crtc_init:
> >  	primary->funcs->destroy(primary);
> >  err_crtc:
> > +	clk_disable_unprepare(dcrtc->axiclk);
> >  	kfree(dcrtc);
> >  
> >  	return ret;
> > diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h
> > index 7ebd337b60af..b07faea7257d 100644
> > --- a/drivers/gpu/drm/armada/armada_crtc.h
> > +++ b/drivers/gpu/drm/armada/armada_crtc.h
> > @@ -39,6 +39,7 @@ struct armada_crtc {
> >  	const struct armada_variant *variant;
> >  	unsigned		num;
> >  	void __iomem		*base;
> > +	struct clk		*axiclk;
> >  	struct clk		*clk;
> >  	struct clk		*extclk[2];
> >  	struct {
> > -- 
> > 2.19.1
> >