| Message ID | 20170411011801.15788-1-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 11, 2017 at 3:18 AM, Eric Anholt <eric@anholt.net> wrote: > From: Tom Cooksey <tom.cooksey@arm.com> Well that can be debated at this point. I think it should have your Author: tag and just Tom in the Signed-off-by, then mention the history in the commit message. > This is a modesetting driver for the pl111 CLCD display controller > found on various ARM platforms such as the Versatile Express. The > driver has only been tested on the bcm911360_entphn platform so far, > with PRIME-based buffer sharing between vc4 and clcd. > > It reuses the existing devicetree binding, while not using quite as > many of its properties as the fbdev driver does (those are left for > future work). > > v2: Nearly complete rewrite by anholt, cutting 2/3 of the code thanks > to DRM core's excellent new helpers. > v3: Don't match pl110 any more, don't attach if we don't have a DRM > panel, use DRM_GEM_CMA_FOPS, update MAINTAINERS, use the simple > display helper, use drm_gem_cma_dumb_create (same as our wrapper). > v4: Change the driver's .name to not clash with fbdev in sysfs, drop > platform alias, drop redundant "drm" in DRM driver name, hook up > .prepare_fb to the CMA helper so that DMA fences should work. > v5: Move register definitions inside the driver directory, fix build > in COMPILE_TEST and !AMBA mode. > > Signed-off-by: Tom Cooksey <tom.cooksey@arm.com> > Signed-off-by: Eric Anholt <eric@anholt.net> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> This is as good starting point as any. We need to get moving with this. Some minor things below that can just as well be fixed later. > create mode 100644 drivers/gpu/drm/pl111/Kconfig Maybe someone would want to place this under drivers/gpu/drm/arm since it is obviously an ARM block. But since ARM has pretty much dropped the ball on this IP it is probably best to keep it separate like this. > +++ b/drivers/gpu/drm/pl111/Kconfig > @@ -0,0 +1,12 @@ > +config DRM_PL111 > + tristate "DRM Support for PL111 CLCD Controller" > + depends on DRM > + depends on ARM || ARM64 || COMPILE_TEST > + select DRM_KMS_HELPER > + select DRM_KMS_CMA_HELPER > + select DRM_GEM_CMA_HELPER > + select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE > + help > + Choose this option for DRM support for the PL111 CLCD controller. > + If M is selected the module will be called pl111_drm. I must say the driver is *really* slim and readable with all the new helpers from DRM, good job all who refactored the DRM support for simple framebuffer systems. > + panel = of_drm_find_panel(panel_node); > + of_node_put(panel_node); Was meaning to ask whether the panel bindings used by the fbdev drivers are 100% compatible with what DRM is using and from your code it seems like yes, they are? > +irqreturn_t pl111_irq(int irq, void *data) > +{ > + struct pl111_drm_dev_private *priv = data; > + u32 irq_stat; > + irqreturn_t status = IRQ_NONE; > + > + irq_stat = readl(priv->regs + CLCD_PL111_MIS); > + > + if (!irq_stat) > + return IRQ_NONE; > + > + if (irq_stat & CLCD_IRQ_NEXTBASE_UPDATE) { > + drm_crtc_handle_vblank(&priv->pipe.crtc); > + > + status = IRQ_HANDLED; > + } > + > + /* Clear the interrupt once done */ > + writel(irq_stat, priv->regs + CLCD_PL111_ICR); > + > + return status; > +} And this is the first time in history that the vblank interrupt on PL11x is actually used for something, which in itself is a good reason to migrate to this driver. Yours, Linus Walleij
On Tue, Apr 11, 2017 at 11:17 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> +++ b/drivers/gpu/drm/pl111/Kconfig >> @@ -0,0 +1,12 @@ >> +config DRM_PL111 >> + tristate "DRM Support for PL111 CLCD Controller" >> + depends on DRM >> + depends on ARM || ARM64 || COMPILE_TEST >> + select DRM_KMS_HELPER >> + select DRM_KMS_CMA_HELPER >> + select DRM_GEM_CMA_HELPER >> + select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE >> + help >> + Choose this option for DRM support for the PL111 CLCD controller. >> + If M is selected the module will be called pl111_drm. > > I must say the driver is *really* slim and readable with all the new > helpers from DRM, good job all who refactored the DRM support > for simple framebuffer systems. Mission accomplished, thanks a lot for the kudos. Also adding Noralf, so he knows people really value his work (he's done the simple pipe helpers used by pl111 here). -Daniel
On Mon, Apr 10, 2017 at 06:18:01PM -0700, Eric Anholt wrote: > v5: Move register definitions inside the driver directory, fix build > in COMPILE_TEST and !AMBA mode. Why is it necessary to move the register definitions there, when they're already available in linux/amba/clcd.h and are required for the FB driver? It is absurd to have driver specific register definitions.
Russell King - ARM Linux <linux@armlinux.org.uk> writes: > On Mon, Apr 10, 2017 at 06:18:01PM -0700, Eric Anholt wrote: >> v5: Move register definitions inside the driver directory, fix build >> in COMPILE_TEST and !AMBA mode. > > Why is it necessary to move the register definitions there, when > they're already available in linux/amba/clcd.h and are required > for the FB driver? > > It is absurd to have driver specific register definitions. Because after v2, v3, and v4, I still didn't have an ack on the patch to move the register definitions from linux/amba/clcd.h to linux/amba/clcd-reg.h. If you'd like to go back and ack that, I'd reuse them.
Linus Walleij <linus.walleij@linaro.org> writes: > On Tue, Apr 11, 2017 at 3:18 AM, Eric Anholt <eric@anholt.net> wrote: > >> From: Tom Cooksey <tom.cooksey@arm.com> > > Well that can be debated at this point. I think it should have > your Author: tag and just Tom in the Signed-off-by, then mention > the history in the commit message. I'm happy giving credit to the original author, unless he wants to duck responsibility at this point. I probably wouldn't have done this work if I didn't have his to start from. >> This is a modesetting driver for the pl111 CLCD display controller >> found on various ARM platforms such as the Versatile Express. The >> driver has only been tested on the bcm911360_entphn platform so far, >> with PRIME-based buffer sharing between vc4 and clcd. >> >> It reuses the existing devicetree binding, while not using quite as >> many of its properties as the fbdev driver does (those are left for >> future work). >> >> v2: Nearly complete rewrite by anholt, cutting 2/3 of the code thanks >> to DRM core's excellent new helpers. >> v3: Don't match pl110 any more, don't attach if we don't have a DRM >> panel, use DRM_GEM_CMA_FOPS, update MAINTAINERS, use the simple >> display helper, use drm_gem_cma_dumb_create (same as our wrapper). >> v4: Change the driver's .name to not clash with fbdev in sysfs, drop >> platform alias, drop redundant "drm" in DRM driver name, hook up >> .prepare_fb to the CMA helper so that DMA fences should work. >> v5: Move register definitions inside the driver directory, fix build >> in COMPILE_TEST and !AMBA mode. >> >> Signed-off-by: Tom Cooksey <tom.cooksey@arm.com> >> Signed-off-by: Eric Anholt <eric@anholt.net> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > This is as good starting point as any. We need to get moving with > this. Some minor things below that can just as well be fixed later. Thanks! >> create mode 100644 drivers/gpu/drm/pl111/Kconfig > > Maybe someone would want to place this under > drivers/gpu/drm/arm since it is obviously an ARM block. > But since ARM has pretty much dropped the ball on this > IP it is probably best to keep it separate like this. > >> +++ b/drivers/gpu/drm/pl111/Kconfig >> @@ -0,0 +1,12 @@ >> +config DRM_PL111 >> + tristate "DRM Support for PL111 CLCD Controller" >> + depends on DRM >> + depends on ARM || ARM64 || COMPILE_TEST >> + select DRM_KMS_HELPER >> + select DRM_KMS_CMA_HELPER >> + select DRM_GEM_CMA_HELPER >> + select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE >> + help >> + Choose this option for DRM support for the PL111 CLCD controller. >> + If M is selected the module will be called pl111_drm. > > I must say the driver is *really* slim and readable with all the new > helpers from DRM, good job all who refactored the DRM support > for simple framebuffer systems. > >> + panel = of_drm_find_panel(panel_node); >> + of_node_put(panel_node); > > Was meaning to ask whether the panel bindings used by the > fbdev drivers are 100% compatible with what DRM is using > and from your code it seems like yes, they are? Yeah, the panel bindings seem fine. We'll need to build equivalent panel drivers within DRM for anyone that wants to switch over, but that's not hard.
On Tue, Apr 11, 2017 at 09:06:31AM -0700, Eric Anholt wrote: > Russell King - ARM Linux <linux@armlinux.org.uk> writes: > > > On Mon, Apr 10, 2017 at 06:18:01PM -0700, Eric Anholt wrote: > >> v5: Move register definitions inside the driver directory, fix build > >> in COMPILE_TEST and !AMBA mode. > > > > Why is it necessary to move the register definitions there, when > > they're already available in linux/amba/clcd.h and are required > > for the FB driver? > > > > It is absurd to have driver specific register definitions. > > Because after v2, v3, and v4, I still didn't have an ack on the patch to > move the register definitions from linux/amba/clcd.h to > linux/amba/clcd-reg.h. If you'd like to go back and ack that, I'd reuse > them. I don't remember seeing such a patch, sorry.
Russell King - ARM Linux <linux@armlinux.org.uk> writes: > On Tue, Apr 11, 2017 at 09:06:31AM -0700, Eric Anholt wrote: >> Russell King - ARM Linux <linux@armlinux.org.uk> writes: >> >> > On Mon, Apr 10, 2017 at 06:18:01PM -0700, Eric Anholt wrote: >> >> v5: Move register definitions inside the driver directory, fix build >> >> in COMPILE_TEST and !AMBA mode. >> > >> > Why is it necessary to move the register definitions there, when >> > they're already available in linux/amba/clcd.h and are required >> > for the FB driver? >> > >> > It is absurd to have driver specific register definitions. >> >> Because after v2, v3, and v4, I still didn't have an ack on the patch to >> move the register definitions from linux/amba/clcd.h to >> linux/amba/clcd-reg.h. If you'd like to go back and ack that, I'd reuse >> them. > > I don't remember seeing such a patch, sorry. https://patchwork.kernel.org/patch/9654991/
Linus Walleij <linus.walleij@linaro.org> writes: > On Tue, Apr 11, 2017 at 3:18 AM, Eric Anholt <eric@anholt.net> wrote: > >> From: Tom Cooksey <tom.cooksey@arm.com> > > Well that can be debated at this point. I think it should have > your Author: tag and just Tom in the Signed-off-by, then mention > the history in the commit message. > >> This is a modesetting driver for the pl111 CLCD display controller >> found on various ARM platforms such as the Versatile Express. The >> driver has only been tested on the bcm911360_entphn platform so far, >> with PRIME-based buffer sharing between vc4 and clcd. >> >> It reuses the existing devicetree binding, while not using quite as >> many of its properties as the fbdev driver does (those are left for >> future work). >> >> v2: Nearly complete rewrite by anholt, cutting 2/3 of the code thanks >> to DRM core's excellent new helpers. >> v3: Don't match pl110 any more, don't attach if we don't have a DRM >> panel, use DRM_GEM_CMA_FOPS, update MAINTAINERS, use the simple >> display helper, use drm_gem_cma_dumb_create (same as our wrapper). >> v4: Change the driver's .name to not clash with fbdev in sysfs, drop >> platform alias, drop redundant "drm" in DRM driver name, hook up >> .prepare_fb to the CMA helper so that DMA fences should work. >> v5: Move register definitions inside the driver directory, fix build >> in COMPILE_TEST and !AMBA mode. >> >> Signed-off-by: Tom Cooksey <tom.cooksey@arm.com> >> Signed-off-by: Eric Anholt <eric@anholt.net> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > This is as good starting point as any. We need to get moving with > this. Some minor things below that can just as well be fixed later. Oh, one last thing I think we need to figure out: I'm using TIM2_CLKSEL, which seems to be necessary on this platform. My understanding is that this means that the pixel clock is divided from clcdclk instead of apb_pclk. Do you agree? The fbdev driver is using clk_get(&fb->dev->dev, NULL) and not TIM2_CLKSEL, which I'm surprised by because I would have thought that would give us the first clock from the DT node (also clcdclk).
On Wed, Apr 12, 2017 at 12:13 AM, Eric Anholt <eric@anholt.net> wrote: > Oh, one last thing I think we need to figure out: I'm using TIM2_CLKSEL, > which seems to be necessary on this platform. My understanding is that > this means that the pixel clock is divided from clcdclk instead of > apb_pclk. Do you agree? Yes the pixed clock is always derived from clcdclk. In most older ARM reference designs this is a VCO so that is why there is a clk_set_rate() on this in the fbdev code. (On some platforms that even has no effect I guess.) > The fbdev driver is using > clk_get(&fb->dev->dev, NULL) and not TIM2_CLKSEL, which I'm surprised by > because I would have thought that would give us the first clock from the > DT node (also clcdclk). So that thing is a 1-bit line that can select one of two clocks to be muxed into the PL111/CLCD. I guess that up until now all platforms just left that line dangling in the silicon. Congratulations, you came here first ;) Though when I look at the Nomadik it seems that it might be muxing the clock between 48 and 72 MHz, and I've been using 48MHz all along ooopsie. The current assumption in the bindings is that we have only one clock and TIM2_CLKSEL is N/A. If we want proper clcdclk handling with CLKSEL you should probably add some code to implement a real mux clock for this using <linux/clock-provider.h> and drivers/clk/clk-mux.c with select COMMON_CLK so that the driver still only sees clcdclk but that in turn is a mux that can select one of two sources and will react to the clk_set_rate() call by selecting the clock which is closest in frequency to what you want. This needs a small patch to alter the bindings too I guess. A small clock node inside the CLCD, just like PCI bridges have irqchips inside them etc: clcd@10120000 { compatible = "arm,pl110", "arm,primecell"; reg = <0x10120000 0x1000>; (...) clocks = <&clcdclk>, <&foo>; clock-names = "clcdclk", "apb_pclk"; clcdclk: clock-controller@0 { compatible = "arm,pl11x-clock-mux"; clocks = <&source_a>, <&source_b>; }; }; This can be set up easily in the OF probe path since that is what we're doing: just look for this subnode, if it is there create the clock controller. I do not think the clk maintainers would mind a small mux clock controller inside the CLCD driver to handle this mux if we need it. It would *maybe* also be possible to add a second "clcdclk2" to the block and make an educated decision on which clock to use in the driver but that is not as elegant as using the clock framework mux clock I think. Yours, Linus Walleij
On 04/12/2017 09:40 AM, Linus Walleij wrote: > On Wed, Apr 12, 2017 at 12:13 AM, Eric Anholt <eric@anholt.net> wrote: > >> Oh, one last thing I think we need to figure out: I'm using TIM2_CLKSEL, >> which seems to be necessary on this platform. My understanding is that >> this means that the pixel clock is divided from clcdclk instead of >> apb_pclk. Do you agree? > > Yes the pixed clock is always derived from clcdclk. > > In most older ARM reference designs this is a VCO so that > is why there is a clk_set_rate() on this in the fbdev code. > (On some platforms that even has no effect I guess.) > >> The fbdev driver is using >> clk_get(&fb->dev->dev, NULL) and not TIM2_CLKSEL, which I'm surprised by >> because I would have thought that would give us the first clock from the >> DT node (also clcdclk). > > So that thing is a 1-bit line that can select one of two clocks > to be muxed into the PL111/CLCD. > > I guess that up until now all platforms just left that line dangling in > the silicon. Congratulations, you came here first ;) > > Though when I look at the Nomadik it seems that it might be muxing > the clock between 48 and 72 MHz, and I've been using 48MHz > all along ooopsie. > > The current assumption in the bindings is that we have only > one clock and TIM2_CLKSEL is N/A. > > If we want proper clcdclk handling with CLKSEL you should > probably add some code to implement a real mux clock for > this using <linux/clock-provider.h> and drivers/clk/clk-mux.c > with select COMMON_CLK > so that the driver still only sees clcdclk but that in turn is a > mux that can select one of two sources and will react to > the clk_set_rate() call by selecting the clock which is > closest in frequency to what you want. > > This needs a small patch to alter the bindings too I guess. > A small clock node inside the CLCD, just like PCI bridges have > irqchips inside them etc: > > clcd@10120000 { > compatible = "arm,pl110", "arm,primecell"; > reg = <0x10120000 0x1000>; > (...) > clocks = <&clcdclk>, <&foo>; > clock-names = "clcdclk", "apb_pclk"; > > clcdclk: clock-controller@0 { > compatible = "arm,pl11x-clock-mux"; > clocks = <&source_a>, <&source_b>; > }; > }; > > This can be set up easily in the OF probe path since that > is what we're doing: just look for this subnode, if it is there > create the clock controller. > > I do not think the clk maintainers would mind a small mux > clock controller inside the CLCD driver to handle this mux > if we need it. Hi Linus, Indeed it's the way of handling this use case, but no need to add a clk node here, you can copy what we did in pwm-meson, meson-gx-mmc.c, or dwmac-meson8b.c (we went further by also adding clk dividers). In the probe code, simply add a clock mux provider with the two parents clock names (these can and should be platform specific) and only call a clk_set_parent() with the clock specified in the node clocks cell. > > It would *maybe* also be possible to add a second "clcdclk2" > to the block and make an educated decision on which clock > to use in the driver but that is not as elegant as using the > clock framework mux clock I think. > > Yours, > Linus Walleij Neil
On 04/12, Neil Armstrong wrote: > On 04/12/2017 09:40 AM, Linus Walleij wrote: > > On Wed, Apr 12, 2017 at 12:13 AM, Eric Anholt <eric@anholt.net> wrote: > > > >> Oh, one last thing I think we need to figure out: I'm using TIM2_CLKSEL, > >> which seems to be necessary on this platform. My understanding is that > >> this means that the pixel clock is divided from clcdclk instead of > >> apb_pclk. Do you agree? > > > > Yes the pixed clock is always derived from clcdclk. > > > > In most older ARM reference designs this is a VCO so that > > is why there is a clk_set_rate() on this in the fbdev code. > > (On some platforms that even has no effect I guess.) > > > >> The fbdev driver is using > >> clk_get(&fb->dev->dev, NULL) and not TIM2_CLKSEL, which I'm surprised by > >> because I would have thought that would give us the first clock from the > >> DT node (also clcdclk). > > > > So that thing is a 1-bit line that can select one of two clocks > > to be muxed into the PL111/CLCD. > > > > I guess that up until now all platforms just left that line dangling in > > the silicon. Congratulations, you came here first ;) > > > > Though when I look at the Nomadik it seems that it might be muxing > > the clock between 48 and 72 MHz, and I've been using 48MHz > > all along ooopsie. > > > > The current assumption in the bindings is that we have only > > one clock and TIM2_CLKSEL is N/A. > > > > If we want proper clcdclk handling with CLKSEL you should > > probably add some code to implement a real mux clock for > > this using <linux/clock-provider.h> and drivers/clk/clk-mux.c > > with select COMMON_CLK > > so that the driver still only sees clcdclk but that in turn is a > > mux that can select one of two sources and will react to > > the clk_set_rate() call by selecting the clock which is > > closest in frequency to what you want. > > > > This needs a small patch to alter the bindings too I guess. > > A small clock node inside the CLCD, just like PCI bridges have > > irqchips inside them etc: > > > > clcd@10120000 { > > compatible = "arm,pl110", "arm,primecell"; > > reg = <0x10120000 0x1000>; > > (...) > > clocks = <&clcdclk>, <&foo>; > > clock-names = "clcdclk", "apb_pclk"; > > > > clcdclk: clock-controller@0 { > > compatible = "arm,pl11x-clock-mux"; > > clocks = <&source_a>, <&source_b>; > > }; > > }; > > > > This can be set up easily in the OF probe path since that > > is what we're doing: just look for this subnode, if it is there > > create the clock controller. > > > > I do not think the clk maintainers would mind a small mux > > clock controller inside the CLCD driver to handle this mux > > if we need it. > > Hi Linus, > > Indeed it's the way of handling this use case, but no need to add > a clk node here, you can copy what we did in pwm-meson, meson-gx-mmc.c, > or dwmac-meson8b.c (we went further by also adding clk dividers). > > In the probe code, simply add a clock mux provider with the two parents > clock names (these can and should be platform specific) and only call > a clk_set_parent() with the clock specified in the node clocks cell. > +1 Avoiding a binding update is nice.
On Tue, Apr 11, 2017 at 02:00:21PM -0700, Eric Anholt wrote: > Russell King - ARM Linux <linux@armlinux.org.uk> writes: > > > On Tue, Apr 11, 2017 at 09:06:31AM -0700, Eric Anholt wrote: > >> Russell King - ARM Linux <linux@armlinux.org.uk> writes: > >> > >> > On Mon, Apr 10, 2017 at 06:18:01PM -0700, Eric Anholt wrote: > >> >> v5: Move register definitions inside the driver directory, fix build > >> >> in COMPILE_TEST and !AMBA mode. > >> > > >> > Why is it necessary to move the register definitions there, when > >> > they're already available in linux/amba/clcd.h and are required > >> > for the FB driver? > >> > > >> > It is absurd to have driver specific register definitions. > >> > >> Because after v2, v3, and v4, I still didn't have an ack on the patch to > >> move the register definitions from linux/amba/clcd.h to > >> linux/amba/clcd-reg.h. If you'd like to go back and ack that, I'd reuse > >> them. > > > > I don't remember seeing such a patch, sorry. > > https://patchwork.kernel.org/patch/9654991/ Looks fine, apart from the missing #ifndef...#endif guard around the header file.
On Wed, Apr 12, 2017 at 09:40:38AM +0200, Linus Walleij wrote: > On Wed, Apr 12, 2017 at 12:13 AM, Eric Anholt <eric@anholt.net> wrote: > > Oh, one last thing I think we need to figure out: I'm using TIM2_CLKSEL, > > which seems to be necessary on this platform. My understanding is that > > this means that the pixel clock is divided from clcdclk instead of > > apb_pclk. Do you agree? > > Yes the pixed clock is always derived from clcdclk. > > In most older ARM reference designs this is a VCO so that > is why there is a clk_set_rate() on this in the fbdev code. > (On some platforms that even has no effect I guess.) > > > The fbdev driver is using > > clk_get(&fb->dev->dev, NULL) and not TIM2_CLKSEL, which I'm surprised by > > because I would have thought that would give us the first clock from the > > DT node (also clcdclk). > > So that thing is a 1-bit line that can select one of two clocks > to be muxed into the PL111/CLCD. > > I guess that up until now all platforms just left that line dangling in > the silicon. Congratulations, you came here first ;) > > Though when I look at the Nomadik it seems that it might be muxing > the clock between 48 and 72 MHz, and I've been using 48MHz > all along ooopsie. > > The current assumption in the bindings is that we have only > one clock and TIM2_CLKSEL is N/A. > > If we want proper clcdclk handling with CLKSEL you should > probably add some code to implement a real mux clock for > this using <linux/clock-provider.h> and drivers/clk/clk-mux.c > with select COMMON_CLK > so that the driver still only sees clcdclk but that in turn is a > mux that can select one of two sources and will react to > the clk_set_rate() call by selecting the clock which is > closest in frequency to what you want. > > This needs a small patch to alter the bindings too I guess. > A small clock node inside the CLCD, just like PCI bridges have > irqchips inside them etc: > > clcd@10120000 { > compatible = "arm,pl110", "arm,primecell"; > reg = <0x10120000 0x1000>; > (...) > clocks = <&clcdclk>, <&foo>; > clock-names = "clcdclk", "apb_pclk"; > > clcdclk: clock-controller@0 { > compatible = "arm,pl11x-clock-mux"; > clocks = <&source_a>, <&source_b>; > }; > }; > > This can be set up easily in the OF probe path since that > is what we're doing: just look for this subnode, if it is there > create the clock controller. > > I do not think the clk maintainers would mind a small mux > clock controller inside the CLCD driver to handle this mux > if we need it. > > It would *maybe* also be possible to add a second "clcdclk2" > to the block and make an educated decision on which clock > to use in the driver but that is not as elegant as using the > clock framework mux clock I think. We've had drivers (like imx-drm) embedding clk stuff within itself in a similar manner to what you're suggesting, and it ended up being a problem when it came to working out which is the correct clock to use. That stuff got ripped out of imx-drm and replaced with a saner solution (that doesn't use CCF) before imx-drm moved out of staging. The reason for this was to get a saner solution to "I want a clock running at X MHz, which clock gives me the closest to the requested rate" without using the problematical fractional divider^w^wfrequency modulator that severely upsets HDMI. What I think you need to ask here is not how you should be modelling it in DT, but how you're going to control it in the driver. Under what circumstance will you select one CLKSEL state or the other? There's also consideration of the effect CLKSEL has - it's effectively a GPIO output from the module that is available for the system integrator to do whatever they choose with. It just happens to have the name "CLKSEL" - but it doesn't have to select a clock. So, I don't think it's clear cut whether it should be exposed as a GPIO and the GPIO based clk-mux used with it, or whether we should update the binding to allow a second clock to be specified, giving the driver the ability to make its own choice about which clock should be selected.
Russell King - ARM Linux <linux@armlinux.org.uk> writes: > On Tue, Apr 11, 2017 at 02:00:21PM -0700, Eric Anholt wrote: >> Russell King - ARM Linux <linux@armlinux.org.uk> writes: >> >> > On Tue, Apr 11, 2017 at 09:06:31AM -0700, Eric Anholt wrote: >> >> Russell King - ARM Linux <linux@armlinux.org.uk> writes: >> >> >> >> > On Mon, Apr 10, 2017 at 06:18:01PM -0700, Eric Anholt wrote: >> >> >> v5: Move register definitions inside the driver directory, fix build >> >> >> in COMPILE_TEST and !AMBA mode. >> >> > >> >> > Why is it necessary to move the register definitions there, when >> >> > they're already available in linux/amba/clcd.h and are required >> >> > for the FB driver? >> >> > >> >> > It is absurd to have driver specific register definitions. >> >> >> >> Because after v2, v3, and v4, I still didn't have an ack on the patch to >> >> move the register definitions from linux/amba/clcd.h to >> >> linux/amba/clcd-reg.h. If you'd like to go back and ack that, I'd reuse >> >> them. >> > >> > I don't remember seeing such a patch, sorry. >> >> https://patchwork.kernel.org/patch/9654991/ > > Looks fine, apart from the missing #ifndef...#endif guard around the > header file. Are you good with the current version with the ifdef guards? I'd like to merge it through drm-misc.
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index e998ee0d0dd5..71bf510d47e8 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -11,6 +11,7 @@ Linux GPU Driver Developer's Guide drm-kms-helpers drm-uapi i915 + pl111 tinydrm vc4 vga-switcheroo diff --git a/Documentation/gpu/pl111.rst b/Documentation/gpu/pl111.rst new file mode 100644 index 000000000000..9b03736d33dd --- /dev/null +++ b/Documentation/gpu/pl111.rst @@ -0,0 +1,6 @@ +========================================== + drm/pl111 ARM PrimeCell PL111 CLCD Driver +========================================== + +.. kernel-doc:: drivers/gpu/drm/pl111/pl111_drv.c + :doc: ARM PrimeCell PL111 CLCD Driver diff --git a/MAINTAINERS b/MAINTAINERS index e03e6bcefac3..5fbafb39ee2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4160,6 +4160,12 @@ F: drivers/gpu/drm/* F: include/drm/drm* F: include/uapi/drm/drm* +DRM DRIVER FOR ARM PL111 CLCD +M: Eric Anholt <eric@anholt.net> +T: git git://anongit.freedesktop.org/drm/drm-misc +S: Supported +F: drivers/gpu/drm/pl111/ + DRM DRIVER FOR AST SERVER GRAPHICS CHIPS M: Dave Airlie <airlied@redhat.com> S: Odd Fixes diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 78d7fc0ebb57..d1c6c12199b7 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -274,6 +274,8 @@ source "drivers/gpu/drm/meson/Kconfig" source "drivers/gpu/drm/tinydrm/Kconfig" +source "drivers/gpu/drm/pl111/Kconfig" + # Keep legacy drivers last menuconfig DRM_LEGACY diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 59f0f9b696eb..28dcf93e1d8f 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -96,3 +96,4 @@ obj-y += hisilicon/ obj-$(CONFIG_DRM_ZTE) += zte/ obj-$(CONFIG_DRM_MXSFB) += mxsfb/ obj-$(CONFIG_DRM_TINYDRM) += tinydrm/ +obj-$(CONFIG_DRM_PL111) += pl111/ diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig new file mode 100644 index 000000000000..ede49efd531f --- /dev/null +++ b/drivers/gpu/drm/pl111/Kconfig @@ -0,0 +1,12 @@ +config DRM_PL111 + tristate "DRM Support for PL111 CLCD Controller" + depends on DRM + depends on ARM || ARM64 || COMPILE_TEST + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DRM_GEM_CMA_HELPER + select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE + help + Choose this option for DRM support for the PL111 CLCD controller. + If M is selected the module will be called pl111_drm. + diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile new file mode 100644 index 000000000000..01caee727c13 --- /dev/null +++ b/drivers/gpu/drm/pl111/Makefile @@ -0,0 +1,5 @@ +pl111_drm-y += pl111_connector.o \ + pl111_display.o \ + pl111_drv.o + +obj-$(CONFIG_DRM_PL111) += pl111_drm.o diff --git a/drivers/gpu/drm/pl111/pl111_connector.c b/drivers/gpu/drm/pl111/pl111_connector.c new file mode 100644 index 000000000000..9702812a1456 --- /dev/null +++ b/drivers/gpu/drm/pl111/pl111_connector.c @@ -0,0 +1,127 @@ +/* + * (C) COPYRIGHT 2012-2013 ARM Limited. All rights reserved. + * + * Parts of this file were based on sources as follows: + * + * Copyright (c) 2006-2008 Intel Corporation + * Copyright (c) 2007 Dave Airlie <airlied@linux.ie> + * Copyright (C) 2011 Texas Instruments + * + * This program is free software and is provided to you under the terms of the + * GNU General Public License version 2 as published by the Free Software + * Foundation, and any use by you of this program is subject to the terms of + * such GNU licence. + * + */ + +/** + * pl111_drm_connector.c + * Implementation of the connector functions for PL111 DRM + */ +#include <linux/version.h> +#include <linux/shmem_fs.h> +#include <linux/dma-buf.h> + +#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_of.h> +#include <drm/drm_panel.h> + +#include "pl111_drm.h" +#include "pl111_regs.h" + +static void pl111_connector_destroy(struct drm_connector *connector) +{ + struct pl111_drm_connector *pl111_connector = + to_pl111_connector(connector); + + if (pl111_connector->panel) + drm_panel_detach(pl111_connector->panel); + + drm_connector_unregister(connector); + drm_connector_cleanup(connector); +} + +static enum drm_connector_status pl111_connector_detect(struct drm_connector + *connector, bool force) +{ + struct pl111_drm_connector *pl111_connector = + to_pl111_connector(connector); + + return (pl111_connector->panel ? + connector_status_connected : + connector_status_disconnected); +} + +static int pl111_connector_helper_get_modes(struct drm_connector *connector) +{ + struct pl111_drm_connector *pl111_connector = + to_pl111_connector(connector); + + if (!pl111_connector->panel) + return 0; + + return drm_panel_get_modes(pl111_connector->panel); +} + +const struct drm_connector_funcs connector_funcs = { + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = pl111_connector_destroy, + .detect = pl111_connector_detect, + .dpms = drm_atomic_helper_connector_dpms, + .reset = drm_atomic_helper_connector_reset, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +const struct drm_connector_helper_funcs connector_helper_funcs = { + .get_modes = pl111_connector_helper_get_modes, +}; + +/* Walks the OF graph to find the panel node and then asks DRM to look + * up the panel. + */ +static struct drm_panel *pl111_get_panel(struct device *dev) +{ + struct device_node *endpoint, *panel_node; + struct device_node *np = dev->of_node; + struct drm_panel *panel; + + endpoint = of_graph_get_next_endpoint(np, NULL); + if (!endpoint) { + dev_err(dev, "no endpoint to fetch panel\n"); + return NULL; + } + + /* don't proceed if we have an endpoint but no panel_node tied to it */ + panel_node = of_graph_get_remote_port_parent(endpoint); + of_node_put(endpoint); + if (!panel_node) { + dev_err(dev, "no valid panel node\n"); + return NULL; + } + + panel = of_drm_find_panel(panel_node); + of_node_put(panel_node); + + return panel; +} + +int pl111_connector_init(struct drm_device *dev) +{ + struct pl111_drm_dev_private *priv = dev->dev_private; + struct pl111_drm_connector *pl111_connector = &priv->connector; + struct drm_connector *connector = &pl111_connector->connector; + + drm_connector_init(dev, connector, &connector_funcs, + DRM_MODE_CONNECTOR_DPI); + drm_connector_helper_add(connector, &connector_helper_funcs); + + pl111_connector->panel = pl111_get_panel(dev->dev); + if (pl111_connector->panel) + drm_panel_attach(pl111_connector->panel, connector); + + return 0; +} + diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c new file mode 100644 index 000000000000..75b52a94474d --- /dev/null +++ b/drivers/gpu/drm/pl111/pl111_display.c @@ -0,0 +1,345 @@ +/* + * (C) COPYRIGHT 2012-2013 ARM Limited. All rights reserved. + * + * Parts of this file were based on sources as follows: + * + * Copyright (c) 2006-2008 Intel Corporation + * Copyright (c) 2007 Dave Airlie <airlied@linux.ie> + * Copyright (C) 2011 Texas Instruments + * + * This program is free software and is provided to you under the terms of the + * GNU General Public License version 2 as published by the Free Software + * Foundation, and any use by you of this program is subject to the terms of + * such GNU licence. + * + */ + +#include <linux/clk.h> +#include <linux/version.h> +#include <linux/dma-buf.h> +#include <linux/of_graph.h> + +#include <drm/drmP.h> +#include <drm/drm_panel.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_fb_cma_helper.h> + +#include "pl111_drm.h" +#include "pl111_regs.h" + +irqreturn_t pl111_irq(int irq, void *data) +{ + struct pl111_drm_dev_private *priv = data; + u32 irq_stat; + irqreturn_t status = IRQ_NONE; + + irq_stat = readl(priv->regs + CLCD_PL111_MIS); + + if (!irq_stat) + return IRQ_NONE; + + if (irq_stat & CLCD_IRQ_NEXTBASE_UPDATE) { + drm_crtc_handle_vblank(&priv->pipe.crtc); + + status = IRQ_HANDLED; + } + + /* Clear the interrupt once done */ + writel(irq_stat, priv->regs + CLCD_PL111_ICR); + + return status; +} + +static u32 pl111_get_fb_offset(struct drm_plane_state *pstate) +{ + struct drm_framebuffer *fb = pstate->fb; + struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, 0); + + return (obj->paddr + + fb->offsets[0] + + fb->format->cpp[0] * pstate->src_x + + fb->pitches[0] * pstate->src_y); +} + +static int pl111_display_check(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *pstate, + struct drm_crtc_state *cstate) +{ + const struct drm_display_mode *mode = &cstate->mode; + struct drm_framebuffer *old_fb = pipe->plane.state->fb; + struct drm_framebuffer *fb = pstate->fb; + + if (mode->hdisplay % 16) + return -EINVAL; + + if (fb) { + u32 offset = pl111_get_fb_offset(pstate); + + /* FB base address must be dword aligned. */ + if (offset & 3) + return -EINVAL; + + /* There's no pitch register -- the mode's hdisplay + * controls it. + */ + if (fb->pitches[0] != mode->hdisplay * fb->format->cpp[0]) + return -EINVAL; + + /* We can't change the FB format in a flicker-free + * manner (and only update it during CRTC enable). + */ + if (old_fb && old_fb->format != fb->format) + cstate->mode_changed = true; + } + + return 0; +} + +static void pl111_display_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *cstate) +{ + struct drm_crtc *crtc = &pipe->crtc; + struct drm_plane *plane = &pipe->plane; + struct drm_device *drm = crtc->dev; + struct pl111_drm_dev_private *priv = drm->dev_private; + const struct drm_display_mode *mode = &cstate->mode; + struct drm_framebuffer *fb = plane->state->fb; + struct drm_connector *connector = &priv->connector.connector; + u32 cntl; + u32 ppl, hsw, hfp, hbp; + u32 lpp, vsw, vfp, vbp; + u32 cpl; + int ret; + + ret = clk_set_rate(priv->clk, mode->clock * 1000); + if (ret) { + dev_err(drm->dev, + "Failed to set pixel clock rate to %d: %d\n", + mode->clock * 1000, ret); + } + + clk_prepare_enable(priv->clk); + + ppl = (mode->hdisplay / 16) - 1; + hsw = mode->hsync_end - mode->hsync_start - 1; + hfp = mode->hsync_start - mode->hdisplay - 1; + hbp = mode->htotal - mode->hsync_end - 1; + + lpp = mode->vdisplay - 1; + vsw = mode->vsync_end - mode->vsync_start - 1; + vfp = mode->vsync_start - mode->vdisplay; + vbp = mode->vtotal - mode->vsync_end; + + cpl = mode->hdisplay - 1; + + writel((ppl << 2) | + (hsw << 8) | + (hfp << 16) | + (hbp << 24), + priv->regs + CLCD_TIM0); + writel(lpp | + (vsw << 10) | + (vfp << 16) | + (vbp << 24), + priv->regs + CLCD_TIM1); + /* XXX: We currently always use CLCDCLK with no divisor. We + * could probably reduce power consumption by using HCLK + * (apb_pclk) with a divisor when it gets us near our target + * pixel clock. + */ + writel(((mode->flags & DRM_MODE_FLAG_NHSYNC) ? TIM2_IHS : 0) | + ((mode->flags & DRM_MODE_FLAG_NVSYNC) ? TIM2_IVS : 0) | + ((connector->display_info.bus_flags & + DRM_BUS_FLAG_DE_LOW) ? TIM2_IOE : 0) | + ((connector->display_info.bus_flags & + DRM_BUS_FLAG_PIXDATA_NEGEDGE) ? TIM2_IPC : 0) | + TIM2_BCD | + (cpl << 16) | + TIM2_CLKSEL, + priv->regs + CLCD_TIM2); + writel(0, priv->regs + CLCD_TIM3); + + drm_panel_prepare(priv->connector.panel); + + /* Enable and Power Up */ + cntl = CNTL_LCDEN | CNTL_LCDTFT | CNTL_LCDPWR | CNTL_LCDVCOMP(1); + + /* Note that the the hardware's format reader takes 'r' from + * the low bit, while DRM formats list channels from high bit + * to low bit as you read left to right. + */ + switch (fb->format->format) { + case DRM_FORMAT_ABGR8888: + case DRM_FORMAT_XBGR8888: + cntl |= CNTL_LCDBPP24; + break; + case DRM_FORMAT_ARGB8888: + case DRM_FORMAT_XRGB8888: + cntl |= CNTL_LCDBPP24 | CNTL_BGR; + break; + case DRM_FORMAT_BGR565: + cntl |= CNTL_LCDBPP16_565; + break; + case DRM_FORMAT_RGB565: + cntl |= CNTL_LCDBPP16_565 | CNTL_BGR; + break; + case DRM_FORMAT_ABGR1555: + case DRM_FORMAT_XBGR1555: + cntl |= CNTL_LCDBPP16; + break; + case DRM_FORMAT_ARGB1555: + case DRM_FORMAT_XRGB1555: + cntl |= CNTL_LCDBPP16 | CNTL_BGR; + break; + case DRM_FORMAT_ABGR4444: + case DRM_FORMAT_XBGR4444: + cntl |= CNTL_LCDBPP16_444; + break; + case DRM_FORMAT_ARGB4444: + case DRM_FORMAT_XRGB4444: + cntl |= CNTL_LCDBPP16_444 | CNTL_BGR; + break; + default: + WARN_ONCE(true, "Unknown FB format 0x%08x\n", + fb->format->format); + break; + } + + writel(cntl, priv->regs + CLCD_PL111_CNTL); + + drm_panel_enable(priv->connector.panel); + + drm_crtc_vblank_on(crtc); +} + +void pl111_display_disable(struct drm_simple_display_pipe *pipe) +{ + struct drm_crtc *crtc = &pipe->crtc; + struct drm_device *drm = crtc->dev; + struct pl111_drm_dev_private *priv = drm->dev_private; + + drm_crtc_vblank_off(crtc); + + drm_panel_disable(priv->connector.panel); + + /* Disable and Power Down */ + writel(0, priv->regs + CLCD_PL111_CNTL); + + drm_panel_unprepare(priv->connector.panel); + + clk_disable_unprepare(priv->clk); +} + +static void pl111_display_update(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *old_pstate) +{ + struct drm_crtc *crtc = &pipe->crtc; + struct drm_device *drm = crtc->dev; + struct pl111_drm_dev_private *priv = drm->dev_private; + struct drm_pending_vblank_event *event = crtc->state->event; + struct drm_plane *plane = &pipe->plane; + struct drm_plane_state *pstate = plane->state; + struct drm_framebuffer *fb = pstate->fb; + + if (fb) { + u32 addr = pl111_get_fb_offset(pstate); + + writel(addr, priv->regs + CLCD_UBAS); + } + + if (event) { + crtc->state->event = NULL; + + spin_lock_irq(&crtc->dev->event_lock); + if (crtc->state->active && drm_crtc_vblank_get(crtc) == 0) + drm_crtc_arm_vblank_event(crtc, event); + else + drm_crtc_send_vblank_event(crtc, event); + spin_unlock_irq(&crtc->dev->event_lock); + } +} + +int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc) +{ + struct pl111_drm_dev_private *priv = drm->dev_private; + + writel(CLCD_IRQ_NEXTBASE_UPDATE, priv->regs + CLCD_PL111_IENB); + + return 0; +} + +void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc) +{ + struct pl111_drm_dev_private *priv = drm->dev_private; + + writel(0, priv->regs + CLCD_PL111_IENB); +} + +static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *plane_state) +{ + return drm_fb_cma_prepare_fb(&pipe->plane, plane_state); +} + +const struct drm_simple_display_pipe_funcs pl111_display_funcs = { + .check = pl111_display_check, + .enable = pl111_display_enable, + .disable = pl111_display_disable, + .update = pl111_display_update, + .prepare_fb = pl111_display_prepare_fb, +}; + +int pl111_display_init(struct drm_device *drm) +{ + struct pl111_drm_dev_private *priv = drm->dev_private; + struct device *dev = drm->dev; + struct device_node *endpoint; + u32 tft_r0b0g0[3]; + int ret; + static const u32 formats[] = { + DRM_FORMAT_ABGR8888, + DRM_FORMAT_XBGR8888, + DRM_FORMAT_ARGB8888, + DRM_FORMAT_XRGB8888, + DRM_FORMAT_BGR565, + DRM_FORMAT_RGB565, + DRM_FORMAT_ABGR1555, + DRM_FORMAT_XBGR1555, + DRM_FORMAT_ARGB1555, + DRM_FORMAT_XRGB1555, + DRM_FORMAT_ABGR4444, + DRM_FORMAT_XBGR4444, + DRM_FORMAT_ARGB4444, + DRM_FORMAT_XRGB4444, + }; + + endpoint = of_graph_get_next_endpoint(dev->of_node, NULL); + if (!endpoint) + return -ENODEV; + + if (of_property_read_u32_array(endpoint, + "arm,pl11x,tft-r0g0b0-pads", + tft_r0b0g0, + ARRAY_SIZE(tft_r0b0g0)) != 0) { + dev_err(dev, "arm,pl11x,tft-r0g0b0-pads should be 3 ints\n"); + of_node_put(endpoint); + return -ENOENT; + } + of_node_put(endpoint); + + if (tft_r0b0g0[0] != 0 || + tft_r0b0g0[1] != 8 || + tft_r0b0g0[2] != 16) { + dev_err(dev, "arm,pl11x,tft-r0g0b0-pads != [0,8,16] not yet supported\n"); + return -EINVAL; + } + + ret = drm_simple_display_pipe_init(drm, &priv->pipe, + &pl111_display_funcs, + formats, ARRAY_SIZE(formats), + &priv->connector.connector); + if (ret) + return ret; + + return 0; +} diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h new file mode 100644 index 000000000000..f381593921b7 --- /dev/null +++ b/drivers/gpu/drm/pl111/pl111_drm.h @@ -0,0 +1,56 @@ +/* + * + * (C) COPYRIGHT 2012-2013 ARM Limited. All rights reserved. + * + * + * Parts of this file were based on sources as follows: + * + * Copyright (c) 2006-2008 Intel Corporation + * Copyright (c) 2007 Dave Airlie <airlied@linux.ie> + * Copyright (C) 2011 Texas Instruments + * + * This program is free software and is provided to you under the terms of the + * GNU General Public License version 2 as published by the Free Software + * Foundation, and any use by you of this program is subject to the terms of + * such GNU licence. + * + */ + +#ifndef _PL111_DRM_H_ +#define _PL111_DRM_H_ + +#include <drm/drm_gem.h> +#include <drm/drm_simple_kms_helper.h> + +#define CLCD_IRQ_NEXTBASE_UPDATE BIT(2) + +struct pl111_drm_connector { + struct drm_connector connector; + struct drm_panel *panel; +}; + +struct pl111_drm_dev_private { + struct drm_device *drm; + + struct pl111_drm_connector connector; + struct drm_simple_display_pipe pipe; + struct drm_fbdev_cma *fbdev; + + void *regs; + struct clk *clk; +}; + +#define to_pl111_connector(x) \ + container_of(x, struct pl111_drm_connector, connector) + +int pl111_display_init(struct drm_device *dev); +int pl111_enable_vblank(struct drm_device *drm, unsigned int crtc); +void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc); +irqreturn_t pl111_irq(int irq, void *data); +int pl111_connector_init(struct drm_device *dev); +int pl111_encoder_init(struct drm_device *dev); +int pl111_dumb_create(struct drm_file *file_priv, + struct drm_device *dev, + struct drm_mode_create_dumb *args); + +#endif /* _PL111_DRM_H_ */ diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c new file mode 100644 index 000000000000..5e6fd1f91a86 --- /dev/null +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -0,0 +1,272 @@ +/* + * (C) COPYRIGHT 2012-2013 ARM Limited. All rights reserved. + * + * Parts of this file were based on sources as follows: + * + * Copyright (c) 2006-2008 Intel Corporation + * Copyright (c) 2007 Dave Airlie <airlied@linux.ie> + * Copyright (C) 2011 Texas Instruments + * + * This program is free software and is provided to you under the terms of the + * GNU General Public License version 2 as published by the Free Software + * Foundation, and any use by you of this program is subject to the terms of + * such GNU licence. + * + */ + +/** + * DOC: ARM PrimeCell PL111 CLCD Driver + * + * The PL111 is a simple LCD controller that can support TFT and STN + * displays. This driver exposes a standard KMS interface for them. + * + * This driver uses the same Device Tree binding as the fbdev CLCD + * driver. While the fbdev driver supports panels that may be + * connected to the CLCD internally to the CLCD driver, in DRM the + * panels get split out to drivers/gpu/drm/panels/. This means that, + * in converting from using fbdev to using DRM, you also need to write + * a panel driver (which may be as simple as an entry in + * panel-simple.c). + * + * The driver currently doesn't expose the cursor. The DRM API for + * cursors requires support for 64x64 ARGB8888 cursor images, while + * the hardware can only support 64x64 monochrome with masking + * cursors. While one could imagine trying to hack something together + * to look at the ARGB8888 and program reasonable in monochrome, we + * just don't expose the cursor at all instead, and leave cursor + * support to the X11 software cursor layer. + * + * TODO: + * + * - Fix race between setting plane base address and getting IRQ for + * vsync firing the pageflip completion. + * + * - Expose the correct set of formats we can support based on the + * "arm,pl11x,tft-r0g0b0-pads" DT property. + * + * - Use the "max-memory-bandwidth" DT property to filter the + * supported formats. + * + * - Read back hardware state at boot to skip reprogramming the + * hardware when doing a no-op modeset. + * + * - Use the internal clock divisor to reduce power consumption by + * using HCLK (apb_pclk) when appropriate. + */ + +#include <linux/amba/bus.h> +#include <linux/version.h> +#include <linux/shmem_fs.h> +#include <linux/dma-buf.h> +#include <linux/module.h> +#include <linux/slab.h> + +#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_fb_cma_helper.h> + +#include "pl111_drm.h" +#include "pl111_regs.h" + +#define DRIVER_DESC "DRM module for PL111" + +struct drm_mode_config_funcs mode_config_funcs = { + .fb_create = drm_fb_cma_create, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + +static int pl111_modeset_init(struct drm_device *dev) +{ + struct drm_mode_config *mode_config; + struct pl111_drm_dev_private *priv = dev->dev_private; + int ret = 0; + + drm_mode_config_init(dev); + mode_config = &dev->mode_config; + mode_config->funcs = &mode_config_funcs; + mode_config->min_width = 1; + mode_config->max_width = 1024; + mode_config->min_height = 1; + mode_config->max_height = 768; + + ret = pl111_connector_init(dev); + if (ret) { + dev_err(dev->dev, "Failed to create pl111_drm_connector\n"); + goto out_config; + } + + /* Don't actually attach if we didn't find a drm_panel + * attached to us. This will allow a kernel to include both + * the fbdev pl111 driver and this one, and choose between + * them based on which subsystem has support for the panel. + */ + if (!priv->connector.panel) { + dev_info(dev->dev, + "Disabling due to lack of DRM panel device.\n"); + ret = -ENODEV; + goto out_config; + } + + ret = pl111_display_init(dev); + if (ret != 0) { + dev_err(dev->dev, "Failed to init display\n"); + goto out_config; + } + + ret = drm_vblank_init(dev, 1); + if (ret != 0) { + dev_err(dev->dev, "Failed to init vblank\n"); + goto out_config; + } + + drm_mode_config_reset(dev); + + priv->fbdev = drm_fbdev_cma_init(dev, 32, + dev->mode_config.num_connector); + + drm_kms_helper_poll_init(dev); + + goto finish; + +out_config: + drm_mode_config_cleanup(dev); +finish: + return ret; +} + +DEFINE_DRM_GEM_CMA_FOPS(drm_fops); + +static void pl111_lastclose(struct drm_device *dev) +{ + struct pl111_drm_dev_private *priv = dev->dev_private; + + drm_fbdev_cma_restore_mode(priv->fbdev); +} + +static struct drm_driver pl111_drm_driver = { + .driver_features = + DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC, + .lastclose = pl111_lastclose, + .ioctls = NULL, + .fops = &drm_fops, + .name = "pl111", + .desc = DRIVER_DESC, + .date = "20170317", + .major = 1, + .minor = 0, + .patchlevel = 0, + .dumb_create = drm_gem_cma_dumb_create, + .dumb_destroy = drm_gem_dumb_destroy, + .dumb_map_offset = drm_gem_cma_dumb_map_offset, + .gem_free_object = drm_gem_cma_free_object, + .gem_vm_ops = &drm_gem_cma_vm_ops, + + .enable_vblank = pl111_enable_vblank, + .disable_vblank = pl111_disable_vblank, + + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .gem_prime_import = drm_gem_prime_import, + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, + .gem_prime_export = drm_gem_prime_export, + .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, +}; + +#ifdef CONFIG_ARM_AMBA +static int pl111_amba_probe(struct amba_device *amba_dev, + const struct amba_id *id) +{ + struct device *dev = &amba_dev->dev; + struct pl111_drm_dev_private *priv; + struct drm_device *drm; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + drm = drm_dev_alloc(&pl111_drm_driver, dev); + if (IS_ERR(drm)) + return PTR_ERR(drm); + amba_set_drvdata(amba_dev, drm); + priv->drm = drm; + drm->dev_private = priv; + + priv->clk = devm_clk_get(dev, "clcdclk"); + if (IS_ERR(priv->clk)) { + dev_err(dev, "CLCD: unable to get clk.\n"); + ret = PTR_ERR(priv->clk); + goto dev_unref; + } + + priv->regs = devm_ioremap_resource(dev, &amba_dev->res); + if (!priv->regs) { + dev_err(dev, "%s failed mmio\n", __func__); + return -EINVAL; + } + + /* turn off interrupts before requesting the irq */ + writel(0, priv->regs + CLCD_PL111_IENB); + + ret = devm_request_irq(dev, amba_dev->irq[0], pl111_irq, 0, + "pl111", priv); + if (ret != 0) { + dev_err(dev, "%s failed irq %d\n", __func__, ret); + return ret; + } + + ret = pl111_modeset_init(drm); + if (ret != 0) + goto dev_unref; + + ret = drm_dev_register(drm, 0); + if (ret < 0) + goto dev_unref; + + return 0; + +dev_unref: + drm_dev_unref(drm); + return ret; +} + +static int pl111_amba_remove(struct amba_device *amba_dev) +{ + struct drm_device *drm = amba_get_drvdata(amba_dev); + struct pl111_drm_dev_private *priv = drm->dev_private; + + drm_dev_unregister(drm); + if (priv->fbdev) + drm_fbdev_cma_fini(priv->fbdev); + drm_mode_config_cleanup(drm); + drm_dev_unref(drm); + + return 0; +} + +static struct amba_id pl111_id_table[] = { + { + .id = 0x00041111, + .mask = 0x000fffff, + }, + {0, 0}, +}; + +static struct amba_driver pl111_amba_driver = { + .drv = { + .name = "drm-clcd-pl111", + }, + .probe = pl111_amba_probe, + .remove = pl111_amba_remove, + .id_table = pl111_id_table, +}; + +module_amba_driver(pl111_amba_driver); +#endif /* CONFIG_ARM_AMBA */ + +MODULE_DESCRIPTION(DRIVER_DESC); +MODULE_AUTHOR("ARM Ltd."); +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/pl111/pl111_regs.h b/drivers/gpu/drm/pl111/pl111_regs.h new file mode 100644 index 000000000000..1b924c4a63ab --- /dev/null +++ b/drivers/gpu/drm/pl111/pl111_regs.h @@ -0,0 +1,76 @@ +/* + * David A Rusling + * + * Copyright (C) 2001 ARM Limited + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of this archive + * for more details. + */ + +/* + * CLCD Controller Internal Register addresses + */ +#define CLCD_TIM0 0x00000000 +#define CLCD_TIM1 0x00000004 +#define CLCD_TIM2 0x00000008 +#define CLCD_TIM3 0x0000000c +#define CLCD_UBAS 0x00000010 +#define CLCD_LBAS 0x00000014 + +#define CLCD_PL110_IENB 0x00000018 +#define CLCD_PL110_CNTL 0x0000001c +#define CLCD_PL110_STAT 0x00000020 +#define CLCD_PL110_INTR 0x00000024 +#define CLCD_PL110_UCUR 0x00000028 +#define CLCD_PL110_LCUR 0x0000002C + +#define CLCD_PL111_CNTL 0x00000018 +#define CLCD_PL111_IENB 0x0000001c +#define CLCD_PL111_RIS 0x00000020 +#define CLCD_PL111_MIS 0x00000024 +#define CLCD_PL111_ICR 0x00000028 +#define CLCD_PL111_UCUR 0x0000002c +#define CLCD_PL111_LCUR 0x00000030 + +#define CLCD_PALL 0x00000200 +#define CLCD_PALETTE 0x00000200 + +#define TIM2_CLKSEL (1 << 5) +#define TIM2_IVS (1 << 11) +#define TIM2_IHS (1 << 12) +#define TIM2_IPC (1 << 13) +#define TIM2_IOE (1 << 14) +#define TIM2_BCD (1 << 26) + +#define CNTL_LCDEN (1 << 0) +#define CNTL_LCDBPP1 (0 << 1) +#define CNTL_LCDBPP2 (1 << 1) +#define CNTL_LCDBPP4 (2 << 1) +#define CNTL_LCDBPP8 (3 << 1) +#define CNTL_LCDBPP16 (4 << 1) +#define CNTL_LCDBPP16_565 (6 << 1) +#define CNTL_LCDBPP16_444 (7 << 1) +#define CNTL_LCDBPP24 (5 << 1) +#define CNTL_LCDBW (1 << 4) +#define CNTL_LCDTFT (1 << 5) +#define CNTL_LCDMONO8 (1 << 6) +#define CNTL_LCDDUAL (1 << 7) +#define CNTL_BGR (1 << 8) +#define CNTL_BEBO (1 << 9) +#define CNTL_BEPO (1 << 10) +#define CNTL_LCDPWR (1 << 11) +#define CNTL_LCDVCOMP(x) ((x) << 12) +#define CNTL_LDMAFIFOTIME (1 << 15) +#define CNTL_WATERMARK (1 << 16) + +/* ST Microelectronics variant bits */ +#define CNTL_ST_1XBPP_444 0x0 +#define CNTL_ST_1XBPP_5551 (1 << 17) +#define CNTL_ST_1XBPP_565 (1 << 18) +#define CNTL_ST_CDWID_12 0x0 +#define CNTL_ST_CDWID_16 (1 << 19) +#define CNTL_ST_CDWID_18 (1 << 20) +#define CNTL_ST_CDWID_24 ((1 << 19)|(1 << 20)) +#define CNTL_ST_CEAEN (1 << 21) +#define CNTL_ST_LCDBPP24_PACKED (6 << 1)