| Message ID | 20181218153742.1313125-15-lkundrak@v3.sk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Armada DRM support for OLPC XO-1.75 laptop | expand |
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. 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? Thanks. > > Signed-off-by: Lubomir Rintel <lkundrak@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 >
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@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 > >
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 {
It needs to be enabled (at least on MMP2) in order for the register writes to LCDC to work. Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- drivers/gpu/drm/armada/armada_crtc.c | 7 +++++++ drivers/gpu/drm/armada/armada_crtc.h | 1 + 2 files changed, 8 insertions(+)