Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
On Fri, Mar 24, 2017 at 12:31 AM, Russell King - ARM Linux wrote: > On Thu, Mar 23, 2017 at 10:54:53PM +0100, Linus Walleij wrote: >> Hm, I certainly want it... but it would be unreasonable of me to expect >> Eric to cold-code a big upfront design for systems he can't even test >> this on. >> >> What I would request would rather be: please do not put in any >> immediate roadblocks and keep it in the back of your head that I will >> maybe come in and add support for the PL110 systems once this >> works. (Of course that would be with the aim to deprecate and >> delete the old fbdev driver in favor of this one.) > > I'm only suggesting that it doesn't match the PL110 hardware as long as > it doesn't support the PL110... Of course. Let's proceed like this. Yours, Linus Walleij
Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
On Thu, Mar 23, 2017 at 10:54:53PM +0100, Linus Walleij wrote: > Hm, I certainly want it... but it would be unreasonable of me to expect > Eric to cold-code a big upfront design for systems he can't even test > this on. > > What I would request would rather be: please do not put in any > immediate roadblocks and keep it in the back of your head that I will > maybe come in and add support for the PL110 systems once this > works. (Of course that would be with the aim to deprecate and > delete the old fbdev driver in favor of this one.) I'm only suggesting that it doesn't match the PL110 hardware as long as it doesn't support the PL110... -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
On Sat, Mar 18, 2017 at 12:09 AM, Russell King - ARM Linux wrote: > On Fri, Mar 17, 2017 at 03:47:42PM -0700, Eric Anholt wrote: >> 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). > > As we're multiplatform on ARM, and this is using the PL11x AMBA IDs, > we must ensure that it's compatible with everything that the fbdev > driver is compatible with... however, I suspect that's not worth the > effort (unless Linus W wants it?) Hm, I certainly want it... but it would be unreasonable of me to expect Eric to cold-code a big upfront design for systems he can't even test this on. What I would request would rather be: please do not put in any immediate roadblocks and keep it in the back of your head that I will maybe come in and add support for the PL110 systems once this works. (Of course that would be with the aim to deprecate and delete the old fbdev driver in favor of this one.) I can help with some RealView testing along the way to begin with. Yours, Linus Walleij
Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
Daniel Vetter writes:
> On Fri, Mar 17, 2017 at 03:47:42PM -0700, Eric Anholt wrote:
>> From: Tom Cooksey
>>
>> 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.
>>
>> Signed-off-by: Tom Cooksey
>> Signed-off-by: Eric Anholt
>
> Looks pretty. A few things below, but nothing big. I'd say if the "how
> generic do we want this to be for now" question is resolved it's ready to
> go in.
>
> If you want this in drm-misc (imo fine, you're already there so doesn't
> really extend the scope of the experiment), then please also add a
> MAINTAINERS entry with the drm-misc git repo and yourself as reviewer.
Will do.
>> diff --git a/drivers/gpu/drm/pl111/pl111_connector.c
>> b/drivers/gpu/drm/pl111/pl111_connector.c
>> new file mode 100644
>> index ..9811d1eadb63
>> --- /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
>> + * 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
>> +#include
>> +#include
>> +#include
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "pl111_drm.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);
>> +}
>
> Probably the umptenth time I've seen this :(
>
> One option I think would work well is if we have a generic "wrap a
> drm_panel into a drm_bridge" driver and just glue that in with an of
> helper as the last element in the enc/transcoder. Laurent has that
> practically written already, but he insist in calling it the lvds bridge,
> because it's for a 100% dummy lvds transcoder.
>
> But since it's 100% dummy it's indistinguishable from pure sw
> abstraction/impendence mismatch helper.
>
> Anyway, just an idea, not going to ask you to do this, but if drm_panel
> takes of like crazy we'll probably want this.
It seems like a generalization of lvds_encoder to wrap a panel as a
connector would be useful. I think I'd like to look at that as a
follow-on change.
>> +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;
>> +
>> +if (!priv)
>> +return -EINVAL;
>> +
>> +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_primary_plane_init(dev);
>> +if (ret != 0) {
>> +dev_err(dev->dev, "Failed to init primary plane\n");
>> +goto out_config;
>> +}
>
> I assume this display IP has a pile of planes? Otherwise the simple pipe
> helpers look like a perfect fit.
Only two, actually. And the other (cursor) is a 64
Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
On Fri, Mar 17, 2017 at 03:47:42PM -0700, Eric Anholt wrote: > From: Tom Cooksey > > 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. > > Signed-off-by: Tom Cooksey > Signed-off-by: Eric Anholt Looks pretty. A few things below, but nothing big. I'd say if the "how generic do we want this to be for now" question is resolved it's ready to go in. If you want this in drm-misc (imo fine, you're already there so doesn't really extend the scope of the experiment), then please also add a MAINTAINERS entry with the drm-misc git repo and yourself as reviewer. Cheers, Daniel > --- > Documentation/gpu/index.rst | 1 + > Documentation/gpu/pl111.rst | 6 + > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile| 1 + > drivers/gpu/drm/pl111/Kconfig | 12 ++ > drivers/gpu/drm/pl111/Makefile | 8 + > drivers/gpu/drm/pl111/pl111_connector.c | 127 ++ > drivers/gpu/drm/pl111/pl111_crtc.c | 239 ++ > drivers/gpu/drm/pl111/pl111_drm.h | 64 +++ > drivers/gpu/drm/pl111/pl111_drv.c | 292 > > drivers/gpu/drm/pl111/pl111_encoder.c | 50 ++ > drivers/gpu/drm/pl111/pl111_gem.c | 35 > drivers/gpu/drm/pl111/pl111_plane.c | 167 ++ > 13 files changed, 1004 insertions(+) > create mode 100644 Documentation/gpu/pl111.rst > create mode 100644 drivers/gpu/drm/pl111/Kconfig > create mode 100644 drivers/gpu/drm/pl111/Makefile > create mode 100644 drivers/gpu/drm/pl111/pl111_connector.c > create mode 100644 drivers/gpu/drm/pl111/pl111_crtc.c > create mode 100644 drivers/gpu/drm/pl111/pl111_drm.h > create mode 100644 drivers/gpu/drm/pl111/pl111_drv.c > create mode 100644 drivers/gpu/drm/pl111/pl111_encoder.c > create mode 100644 drivers/gpu/drm/pl111/pl111_gem.c > create mode 100644 drivers/gpu/drm/pl111/pl111_plane.c > > 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 ..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/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 59aae43005ee..99810a529bb0 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -95,3 +95,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 ..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 ..20a7fd76d513 > --- /dev/null > +++ b/drivers/gpu/drm/pl111/Makefile > @@ -0,0 +1,8 @@ > +pl111_drm-y += pl111_connector.o \ > + pl111_crtc.o \ > + pl111_dr
Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
Russell King - ARM Linux writes: > On Fri, Mar 17, 2017 at 03:47:42PM -0700, Eric Anholt wrote: >> 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). > > As we're multiplatform on ARM, and this is using the PL11x AMBA IDs, > we must ensure that it's compatible with everything that the fbdev > driver is compatible with... however, I suspect that's not worth the > effort (unless Linus W wants it?) > > If we make it PL111 specific, then we don't need to handle Integrator > CP, or the Versatile PB/AB weirdness. The only thing left is the > power etc enable bits on Realview which uses the PL111. See the > code for Realview in drivers/video/fbdev/amba-clcd-versatile.c. Restricting to PL111 for now sounds good to me. Those Realview bits look like they're turning on a power domain -- shouldn't we represent those as a regulator or a power domain? If we did, then plugging that into a panel driver sounds straightforward. (that's assuming that they're powering panel. not the controller -- I can't quite tell from the code I've browsed so far) signature.asc Description: PGP signature
Re: [PATCH 2/2] drm/pl111: Initial drm/kms driver for pl111
On Fri, Mar 17, 2017 at 03:47:42PM -0700, Eric Anholt wrote: > 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). As we're multiplatform on ARM, and this is using the PL11x AMBA IDs, we must ensure that it's compatible with everything that the fbdev driver is compatible with... however, I suspect that's not worth the effort (unless Linus W wants it?) If we make it PL111 specific, then we don't need to handle Integrator CP, or the Versatile PB/AB weirdness. The only thing left is the power etc enable bits on Realview which uses the PL111. See the code for Realview in drivers/video/fbdev/amba-clcd-versatile.c. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
