Re: [PATCH] drm/armada: add mmp2 support

[Lubomir Rintel]

On Tue, 2019-01-22 at 19:10 +0000, Russell King - ARM Linux admin
wrote:
> On Mon, Jan 21, 2019 at 07:03:49AM +0100, Lubomir Rintel wrote:
> > Heavily based on the Armada 510 (Dove) support. Like with 510 support, this
> > also just supports a single source clock -- the "Display 1" clock as
> > generated by the APMU. This one was chosen because the OLPC XO 1.75 laptop
> > uses it for its internal panel.
> > 
> > If anyone uses this to drive a MIPI or HDMI encoder, they may want to
> > extend this to choose a different source for the pixel clock -- it should
> > be a reasonably straightforward thing to do.
> > 
> > The data sheet is not available, but James Cameron of OLPC kindly
> > provided some details about the LCD_SCLK_DIV register.
> > 
> > Link: https://lists.freedesktop.org/archives/dri-devel/2018-December/201021.html
> > Signed-off-by: Lubomir Rintel <lkundrak@xxxxx>
> > ---
> >  drivers/gpu/drm/armada/Makefile      |  1 +
> >  drivers/gpu/drm/armada/armada_610.c  | 93 ++++++++++++++++++++++++++++
> >  drivers/gpu/drm/armada/armada_crtc.c |  4 ++
> >  drivers/gpu/drm/armada/armada_drm.h  |  1 +
> >  drivers/gpu/drm/armada/armada_hw.h   | 10 +++
> >  5 files changed, 109 insertions(+)
> >  create mode 100644 drivers/gpu/drm/armada/armada_610.c
> > 
> > diff --git a/drivers/gpu/drm/armada/Makefile b/drivers/gpu/drm/armada/Makefile
> > index 9bc3c3213724..5bbf86324cda 100644
> > --- a/drivers/gpu/drm/armada/Makefile
> > +++ b/drivers/gpu/drm/armada/Makefile
> > @@ -2,6 +2,7 @@
> >  armada-y	:= armada_crtc.o armada_drv.o armada_fb.o armada_fbdev.o \
> >  		   armada_gem.o armada_overlay.o armada_plane.o armada_trace.o
> >  armada-y	+= armada_510.o
> > +armada-y	+= armada_610.o
> >  armada-$(CONFIG_DEBUG_FS) += armada_debugfs.o
> >  
> >  obj-$(CONFIG_DRM_ARMADA) := armada.o
> > diff --git a/drivers/gpu/drm/armada/armada_610.c b/drivers/gpu/drm/armada/armada_610.c
> > new file mode 100644
> > index 000000000000..278b204038ea
> > --- /dev/null
> > +++ b/drivers/gpu/drm/armada/armada_610.c
> > @@ -0,0 +1,93 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2012 Russell King
> > + * Copyright (C) 2018,2019 Lubomir Rintel
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * Armada MMP2 variant support
> > + */
> > +#include <linux/clk.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_crtc.h>
> > +#include "armada_crtc.h"
> > +#include "armada_drm.h"
> > +#include "armada_hw.h"
> > +
> > +static int armada610_crtc_init(struct armada_crtc *dcrtc, struct device *dev)
> > +{
> > +	struct clk *clk;
> > +
> > +	clk = devm_clk_get(dev, "disp0");
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk) == -ENOENT ? -EPROBE_DEFER : PTR_ERR(clk);
> > +
> > +	dcrtc->extclk[0] = clk;
> 
> I've been reworking the clocking support for Armada, you can find the
> current code in my git tree's drm-armada-devel branch (as mentioned
> in MAINTAINERS).  You'll need to update to that before I can apply
> this.
> 
> The clocks are named in Dove's TRM as:
> 
>    0 = AXIbus: Select AXI bus clock as pixel clock source.
>    1 = EXT_REF_CLK0: LCD_EXT_REF_CLK[0]
>    2 = PLLDivider: Select PLL divider input clock as pixel clock source.
>    3 = EXT_REF_CLK1: LCD_EXT_REF_CLK[1]

What we know (from [1]) about MMP2 clock sources is:

0 AXI
1 LCD Display 1
2 LCD Display 2
3 HDMI PLL

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

> So I chose to use these neumonics in the Armada 510.  Please can we
> keep to naming the clock inputs as per documented names please?

Are you suggesting that clocks that are available on MMP2 are
essentially the same as on 510? Like this?

0 AXI -- axiclk
1 LCD Display 1 -- ext_ref_clk0
2 LCD Display 2 --
ext_ref_clk1
3 HDMI PLL -- plldivider

If that is the case, then I'm happy to take your word on it. I had to
guess.

Why I guessed differently was because the clock numbers in
the LCD_SCLK_DIV register don't seem to match and I thought the
"external" clock sources on 510 are external to the SoC (while they two
"LCD Display" clocks on MMP2 are generated by the PMU).

> Also, have a look at how Armada 510 gets its clocks from DT - note
> that the array they're placed in is ordered by priority (iow, if we
> have an external clock, we use that in preference to the more
> restricted axibus and plldivider clocks.)

Okay. I'll follow up with an updated patch over the course of the next
week.

> 
> Thanks.

Thank you
Lubo