* [RFC PATCH 0/7] add at91sam9 LCDC DRM driver
@ 2018-08-12 18:41 Sam Ravnborg
2018-08-12 18:46 ` [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ Sam Ravnborg
` (9 more replies)
0 siblings, 10 replies; 50+ messages in thread
From: Sam Ravnborg @ 2018-08-12 18:41 UTC (permalink / raw)
To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones,
Nicolas Ferre, Alexandre Belloni
Cc: devicetree, Sam Ravnborg, linux-arm-kernel, dri-devel, linux-pwm
New DRM based driver for at91sam9 SOC's that uses the
Atmel LCDC IP core.
This is first version of a patch set that adds
drivers for the Atmel LCDC IP core.
Posted for review as the basics works now.
The LCDC IP core contains two devices:
- a PWM often used for backlight
- a LCD display controller
Both devices are supported today by the atmel_lcdfb driver.
For this new set of drivers the compatible strings was
selected to avoid clash with the existing compatible
strings used for the atmel_lcdfb driver to allow them
to co-exist.
This patchset implements three drivers.
- A MFD driver that include the generic parts.
- A PWM driver.
- A DRM display controller driver.
This is the same split as used for the Atmel hlcdc IP.
The hlcdc and lcdc has only a few things in common and
trying to share the code for them was not a viable solution.
The DRM implementation has a few shortcomings compared to the
existing fbdev based driver:
- STN displays are not supported
Binding support is missing but most of the
STN specific functionality is otherwise ported
from the fbdev driver.
I assume the info should come from the panel
but as I lack HW I have not looked too much
into what is required.
- gamma support is missing
The driver utilises drm_simple_kms_helper and
this helper lacks support for setting up gamma.
If this is useful please let me know and I
will extend drm_simple_kms_helper to support this
and update the driver.
- modesetting is not checked (see TODO in file)
Is this required for such a simple setup?
- support for extra modes as applicable (and lcd-wiring-mode)
- support for AVR32 (is it relevant?)
The first patch renames .../drm/atmel-hlcdc to .../drm/atmel
to have a nice home for both drivers.
The drivers _works_ on a proprietary at91sam9263 based board
and I will eventually migrate the at91sam9263ek over
to use it as well.
Works is maybe a stretch - the following was tested:
modetest shows reasonable output
modetest -s shows some nice colored squares
vbltest shows 45.70 Hz (after terminating modetest -s with SIGINT)
drmdevice shows reasonable output
brightness can be controlled
Anything else that can be recommended for some basic tests?
How to test suspend/resume?
REVIEW
Please give feedback on general structure/architecture
Please check if the drm framework is used in the optimal way
And then the usual review from spelling errors, names, style etc.
All feedback (from spelling errors to general structure) appreciated!
Sam
Sam Ravnborg (7):
atmel-hlcdc: renamed directory to drm/atmel/
dt-binding: add bindings for Atmel LCDC mfd
mfd: add atmel-lcdc driver
dt-bindings: add bindings for Atmel LCDC pwm
pwm: add pwm-atmel-lcdc driver
dt-bindings: add bindings for Atmel lcdc-display-controller
drm: add Atmel LCDC display controller support
.../display/atmel/lcdc-display-controller.txt | 40 +
.../devicetree/bindings/mfd/atmel-lcdc.txt | 75 ++
.../devicetree/bindings/pwm/atmel-lcdc-pwm.txt | 30 +
MAINTAINERS | 9 +-
drivers/gpu/drm/Kconfig | 2 +-
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/atmel-hlcdc/Kconfig | 10 -
drivers/gpu/drm/atmel/Kconfig | 28 +
drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile | 2 +
.../drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c | 0
.../drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c | 0
.../drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h | 0
.../{atmel-hlcdc => atmel}/atmel_hlcdc_output.c | 0
.../drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c | 0
drivers/gpu/drm/atmel/atmel_lcdc-dc.c | 1094 ++++++++++++++++++++
drivers/mfd/Kconfig | 10 +
drivers/mfd/Makefile | 1 +
drivers/mfd/atmel-lcdc.c | 158 +++
drivers/pwm/Kconfig | 13 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-atmel-lcdc.c | 178 ++++
include/linux/mfd/atmel-lcdc.h | 184 ++++
22 files changed, 1824 insertions(+), 13 deletions(-)
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg @ 2018-08-12 18:46 ` Sam Ravnborg 2018-08-14 8:39 ` Daniel Vetter 2018-08-24 8:28 ` Boris Brezillon 2018-08-12 18:46 ` [PATCH v1 2/7] dt-binding: add bindings for Atmel LCDC mfd Sam Ravnborg ` (8 subsequent siblings) 9 siblings, 2 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-12 18:46 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, Sam Ravnborg, linux-arm-kernel, dri-devel, linux-pwm Use vendor name for directory, adding a suitable place for more atmel DRM drivers. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Boris Brezillon <boris.brezillon@bootlin.com> --- MAINTAINERS | 2 +- drivers/gpu/drm/Kconfig | 2 +- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/{atmel-hlcdc => atmel}/Kconfig | 6 ++++++ drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile | 0 drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c | 0 drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c | 0 drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h | 0 drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_output.c | 0 drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c | 0 10 files changed, 9 insertions(+), 3 deletions(-) rename drivers/gpu/drm/{atmel-hlcdc => atmel}/Kconfig (83%) rename drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile (100%) rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c (100%) rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c (100%) rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h (100%) rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_output.c (100%) rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c (100%) diff --git a/MAINTAINERS b/MAINTAINERS index 96e98e206b0d..09ce76a9a1dc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4681,7 +4681,7 @@ DRM DRIVERS FOR ATMEL HLCDC M: Boris Brezillon <boris.brezillon@bootlin.com> L: dri-devel@lists.freedesktop.org S: Supported -F: drivers/gpu/drm/atmel-hlcdc/ +F: drivers/gpu/drm/atmel/atmel-hlcdc* F: Documentation/devicetree/bindings/display/atmel/ T: git git://anongit.freedesktop.org/drm/drm-misc diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 2a72d2feb76d..4130df0c0dba 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -232,7 +232,7 @@ source "drivers/gpu/drm/cirrus/Kconfig" source "drivers/gpu/drm/armada/Kconfig" -source "drivers/gpu/drm/atmel-hlcdc/Kconfig" +source "drivers/gpu/drm/atmel/Kconfig" source "drivers/gpu/drm/rcar-du/Kconfig" diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index ef9f3dab287f..ce9829967128 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -76,7 +76,7 @@ obj-$(CONFIG_DRM_GMA500) += gma500/ obj-$(CONFIG_DRM_UDL) += udl/ obj-$(CONFIG_DRM_AST) += ast/ obj-$(CONFIG_DRM_ARMADA) += armada/ -obj-$(CONFIG_DRM_ATMEL_HLCDC) += atmel-hlcdc/ +obj-$(CONFIG_DRM_ATMEL) += atmel/ obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/ obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/ obj-y += omapdrm/ diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig b/drivers/gpu/drm/atmel/Kconfig similarity index 83% rename from drivers/gpu/drm/atmel-hlcdc/Kconfig rename to drivers/gpu/drm/atmel/Kconfig index 32bcc4bad06a..7cd3862f9d18 100644 --- a/drivers/gpu/drm/atmel-hlcdc/Kconfig +++ b/drivers/gpu/drm/atmel/Kconfig @@ -1,6 +1,12 @@ +config DRM_ATMEL + bool + help + Enable Atmel DRM support + config DRM_ATMEL_HLCDC tristate "DRM Support for ATMEL HLCDC Display Controller" depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC && ARM + select DRM_ATMEL select DRM_GEM_CMA_HELPER select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER diff --git a/drivers/gpu/drm/atmel-hlcdc/Makefile b/drivers/gpu/drm/atmel/Makefile similarity index 100% rename from drivers/gpu/drm/atmel-hlcdc/Makefile rename to drivers/gpu/drm/atmel/Makefile diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel/atmel_hlcdc_crtc.c similarity index 100% rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c rename to drivers/gpu/drm/atmel/atmel_hlcdc_crtc.c diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel/atmel_hlcdc_dc.c similarity index 100% rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c rename to drivers/gpu/drm/atmel/atmel_hlcdc_dc.c diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel/atmel_hlcdc_dc.h similarity index 100% rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h rename to drivers/gpu/drm/atmel/atmel_hlcdc_dc.h diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel/atmel_hlcdc_output.c similarity index 100% rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c rename to drivers/gpu/drm/atmel/atmel_hlcdc_output.c diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel/atmel_hlcdc_plane.c similarity index 100% rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c rename to drivers/gpu/drm/atmel/atmel_hlcdc_plane.c -- 2.12.0 ^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ 2018-08-12 18:46 ` [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ Sam Ravnborg @ 2018-08-14 8:39 ` Daniel Vetter 2018-08-14 16:19 ` Sam Ravnborg 2018-08-24 8:28 ` Boris Brezillon 1 sibling, 1 reply; 50+ messages in thread From: Daniel Vetter @ 2018-08-14 8:39 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel On Sun, Aug 12, 2018 at 08:46:23PM +0200, Sam Ravnborg wrote: > Use vendor name for directory, adding a suitable place > for more atmel DRM drivers. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Boris Brezillon <boris.brezillon@bootlin.com> > --- > MAINTAINERS | 2 +- > drivers/gpu/drm/Kconfig | 2 +- > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/{atmel-hlcdc => atmel}/Kconfig | 6 ++++++ > drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_output.c | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c | 0 > 10 files changed, 9 insertions(+), 3 deletions(-) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/Kconfig (83%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_output.c (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c (100%) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 96e98e206b0d..09ce76a9a1dc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4681,7 +4681,7 @@ DRM DRIVERS FOR ATMEL HLCDC > M: Boris Brezillon <boris.brezillon@bootlin.com> > L: dri-devel@lists.freedesktop.org > S: Supported > -F: drivers/gpu/drm/atmel-hlcdc/ > +F: drivers/gpu/drm/atmel/atmel-hlcdc* I'd strongly suggest you group-maintain the entire atmel stuff in drm-misc instead of only atmel-hlcdc. -Daniel > F: Documentation/devicetree/bindings/display/atmel/ > T: git git://anongit.freedesktop.org/drm/drm-misc > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 2a72d2feb76d..4130df0c0dba 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -232,7 +232,7 @@ source "drivers/gpu/drm/cirrus/Kconfig" > > source "drivers/gpu/drm/armada/Kconfig" > > -source "drivers/gpu/drm/atmel-hlcdc/Kconfig" > +source "drivers/gpu/drm/atmel/Kconfig" > > source "drivers/gpu/drm/rcar-du/Kconfig" > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index ef9f3dab287f..ce9829967128 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -76,7 +76,7 @@ obj-$(CONFIG_DRM_GMA500) += gma500/ > obj-$(CONFIG_DRM_UDL) += udl/ > obj-$(CONFIG_DRM_AST) += ast/ > obj-$(CONFIG_DRM_ARMADA) += armada/ > -obj-$(CONFIG_DRM_ATMEL_HLCDC) += atmel-hlcdc/ > +obj-$(CONFIG_DRM_ATMEL) += atmel/ > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/ > obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/ > obj-y += omapdrm/ > diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig b/drivers/gpu/drm/atmel/Kconfig > similarity index 83% > rename from drivers/gpu/drm/atmel-hlcdc/Kconfig > rename to drivers/gpu/drm/atmel/Kconfig > index 32bcc4bad06a..7cd3862f9d18 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/Kconfig > +++ b/drivers/gpu/drm/atmel/Kconfig > @@ -1,6 +1,12 @@ > +config DRM_ATMEL > + bool > + help > + Enable Atmel DRM support > + > config DRM_ATMEL_HLCDC > tristate "DRM Support for ATMEL HLCDC Display Controller" > depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC && ARM > + select DRM_ATMEL > select DRM_GEM_CMA_HELPER > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > diff --git a/drivers/gpu/drm/atmel-hlcdc/Makefile b/drivers/gpu/drm/atmel/Makefile > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/Makefile > rename to drivers/gpu/drm/atmel/Makefile > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel/atmel_hlcdc_crtc.c > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c > rename to drivers/gpu/drm/atmel/atmel_hlcdc_crtc.c > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel/atmel_hlcdc_dc.c > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > rename to drivers/gpu/drm/atmel/atmel_hlcdc_dc.c > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel/atmel_hlcdc_dc.h > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h > rename to drivers/gpu/drm/atmel/atmel_hlcdc_dc.h > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel/atmel_hlcdc_output.c > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > rename to drivers/gpu/drm/atmel/atmel_hlcdc_output.c > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel/atmel_hlcdc_plane.c > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c > rename to drivers/gpu/drm/atmel/atmel_hlcdc_plane.c > -- > 2.12.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ 2018-08-14 8:39 ` Daniel Vetter @ 2018-08-14 16:19 ` Sam Ravnborg 2018-08-16 7:41 ` Daniel Vetter 0 siblings, 1 reply; 50+ messages in thread From: Sam Ravnborg @ 2018-08-14 16:19 UTC (permalink / raw) To: Daniel Vetter Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel Hi Daniel. > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c (100%) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 96e98e206b0d..09ce76a9a1dc 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -4681,7 +4681,7 @@ DRM DRIVERS FOR ATMEL HLCDC > > M: Boris Brezillon <boris.brezillon@bootlin.com> > > L: dri-devel@lists.freedesktop.org > > S: Supported > > -F: drivers/gpu/drm/atmel-hlcdc/ > > +F: drivers/gpu/drm/atmel/atmel-hlcdc* > > I'd strongly suggest you group-maintain the entire atmel stuff in > drm-misc instead of only atmel-hlcdc. If OK with Boris then I am fine with this too. Will suggest so in v2. Sam ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ 2018-08-14 16:19 ` Sam Ravnborg @ 2018-08-16 7:41 ` Daniel Vetter 2018-08-22 20:09 ` Sam Ravnborg 0 siblings, 1 reply; 50+ messages in thread From: Daniel Vetter @ 2018-08-16 7:41 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel On Tue, Aug 14, 2018 at 06:19:52PM +0200, Sam Ravnborg wrote: > Hi Daniel. > > > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c (100%) > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 96e98e206b0d..09ce76a9a1dc 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -4681,7 +4681,7 @@ DRM DRIVERS FOR ATMEL HLCDC > > > M: Boris Brezillon <boris.brezillon@bootlin.com> > > > L: dri-devel@lists.freedesktop.org > > > S: Supported > > > -F: drivers/gpu/drm/atmel-hlcdc/ > > > +F: drivers/gpu/drm/atmel/atmel-hlcdc* > > > > I'd strongly suggest you group-maintain the entire atmel stuff in > > drm-misc instead of only atmel-hlcdc. > > If OK with Boris then I am fine with this too. > Will suggest so in v2. Boris acked too, but he's on vacation. He said on irc he'll review the patches when he's back. Meanwhile I guess you can prep by getting the fd.o account requested: https://www.freedesktop.org/wiki/AccountRequests/ And checking out the tooling we use to maintain drm-misc: https://dri.freedesktop.org/docs/dim/getting-started.html Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ 2018-08-16 7:41 ` Daniel Vetter @ 2018-08-22 20:09 ` Sam Ravnborg 2018-08-22 20:22 ` Daniel Vetter 0 siblings, 1 reply; 50+ messages in thread From: Sam Ravnborg @ 2018-08-22 20:09 UTC (permalink / raw) To: Daniel Vetter Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel Hi Daniel. > > > > > > I'd strongly suggest you group-maintain the entire atmel stuff in > > > drm-misc instead of only atmel-hlcdc. > > > > If OK with Boris then I am fine with this too. > > Will suggest so in v2. > > Boris acked too, but he's on vacation. He said on irc he'll review the > patches when he's back. Meanwhile I guess you can prep by getting the fd.o > account requested: > > https://www.freedesktop.org/wiki/AccountRequests/ > > And checking out the tooling we use to maintain drm-misc: > > https://dri.freedesktop.org/docs/dim/getting-started.html With enough on the plate already I decided to postpone this part until the migration to gitlab is done. Just to tell this was not ignored, only postponed. Sam _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ 2018-08-22 20:09 ` Sam Ravnborg @ 2018-08-22 20:22 ` Daniel Vetter 0 siblings, 0 replies; 50+ messages in thread From: Daniel Vetter @ 2018-08-22 20:22 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, Linux ARM On Wed, Aug 22, 2018 at 10:09 PM, Sam Ravnborg <sam@ravnborg.org> wrote: > Hi Daniel. > >> > > >> > > I'd strongly suggest you group-maintain the entire atmel stuff in >> > > drm-misc instead of only atmel-hlcdc. >> > >> > If OK with Boris then I am fine with this too. >> > Will suggest so in v2. >> >> Boris acked too, but he's on vacation. He said on irc he'll review the >> patches when he's back. Meanwhile I guess you can prep by getting the fd.o >> account requested: >> >> https://www.freedesktop.org/wiki/AccountRequests/ >> >> And checking out the tooling we use to maintain drm-misc: >> >> https://dri.freedesktop.org/docs/dim/getting-started.html > > With enough on the plate already I decided to postpone this > part until the migration to gitlab is done. > Just to tell this was not ignored, only postponed. Migrating the drm repos will start earliest next year (because fd.o admins need to rework some of the infrastructure). And this is the optimistic estimate. We'll also try to make the transition as seamless as possible for committers (the scripting we're using will auto-upgrade remotes and everything, plus accounts will get migrated). Holding off imo doesn't make sense, just means your driver won't land for another 6 months or so, or at least be in some funny maintainer limbo state. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ 2018-08-12 18:46 ` [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ Sam Ravnborg 2018-08-14 8:39 ` Daniel Vetter @ 2018-08-24 8:28 ` Boris Brezillon 2018-08-24 15:43 ` Sam Ravnborg 1 sibling, 1 reply; 50+ messages in thread From: Boris Brezillon @ 2018-08-24 8:28 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Nicolas Ferre, dri-devel, Rob Herring, Lee Jones, linux-arm-kernel On Sun, 12 Aug 2018 20:46:23 +0200 Sam Ravnborg <sam@ravnborg.org> wrote: > Use vendor name for directory, adding a suitable place > for more atmel DRM drivers. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Boris Brezillon <boris.brezillon@bootlin.com> > --- > MAINTAINERS | 2 +- > drivers/gpu/drm/Kconfig | 2 +- > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/{atmel-hlcdc => atmel}/Kconfig | 6 ++++++ > drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_output.c | 0 > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c | 0 > 10 files changed, 9 insertions(+), 3 deletions(-) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/Kconfig (83%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_output.c (100%) > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c (100%) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 96e98e206b0d..09ce76a9a1dc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4681,7 +4681,7 @@ DRM DRIVERS FOR ATMEL HLCDC > M: Boris Brezillon <boris.brezillon@bootlin.com> > L: dri-devel@lists.freedesktop.org > S: Supported > -F: drivers/gpu/drm/atmel-hlcdc/ > +F: drivers/gpu/drm/atmel/atmel-hlcdc* atmel_hlcdc* Also, I think you can keep the whole directory, and add your name plus update the entry description after adding code for the LCDC block. > F: Documentation/devicetree/bindings/display/atmel/ > T: git git://anongit.freedesktop.org/drm/drm-misc > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 2a72d2feb76d..4130df0c0dba 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -232,7 +232,7 @@ source "drivers/gpu/drm/cirrus/Kconfig" > > source "drivers/gpu/drm/armada/Kconfig" > > -source "drivers/gpu/drm/atmel-hlcdc/Kconfig" > +source "drivers/gpu/drm/atmel/Kconfig" > > source "drivers/gpu/drm/rcar-du/Kconfig" > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index ef9f3dab287f..ce9829967128 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -76,7 +76,7 @@ obj-$(CONFIG_DRM_GMA500) += gma500/ > obj-$(CONFIG_DRM_UDL) += udl/ > obj-$(CONFIG_DRM_AST) += ast/ > obj-$(CONFIG_DRM_ARMADA) += armada/ > -obj-$(CONFIG_DRM_ATMEL_HLCDC) += atmel-hlcdc/ > +obj-$(CONFIG_DRM_ATMEL) += atmel/ > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/ > obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/ > obj-y += omapdrm/ > diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig b/drivers/gpu/drm/atmel/Kconfig > similarity index 83% > rename from drivers/gpu/drm/atmel-hlcdc/Kconfig > rename to drivers/gpu/drm/atmel/Kconfig > index 32bcc4bad06a..7cd3862f9d18 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/Kconfig > +++ b/drivers/gpu/drm/atmel/Kconfig > @@ -1,6 +1,12 @@ > +config DRM_ATMEL > + bool > + help > + Enable Atmel DRM support > + Not sure why you need an extra Kconfig option? And if you really do (because you want to share common code), it should be implicitly selected by DRM_ATMEL_HLCDC and DRM_ATMEL_LCDC. > config DRM_ATMEL_HLCDC > tristate "DRM Support for ATMEL HLCDC Display Controller" > depends on DRM && OF && COMMON_CLK && MFD_ATMEL_HLCDC && ARM > + select DRM_ATMEL > select DRM_GEM_CMA_HELPER > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > diff --git a/drivers/gpu/drm/atmel-hlcdc/Makefile b/drivers/gpu/drm/atmel/Makefile > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/Makefile > rename to drivers/gpu/drm/atmel/Makefile > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel/atmel_hlcdc_crtc.c > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c > rename to drivers/gpu/drm/atmel/atmel_hlcdc_crtc.c > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel/atmel_hlcdc_dc.c > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > rename to drivers/gpu/drm/atmel/atmel_hlcdc_dc.c > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel/atmel_hlcdc_dc.h > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h > rename to drivers/gpu/drm/atmel/atmel_hlcdc_dc.h > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel/atmel_hlcdc_output.c > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > rename to drivers/gpu/drm/atmel/atmel_hlcdc_output.c > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel/atmel_hlcdc_plane.c > similarity index 100% > rename from drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c > rename to drivers/gpu/drm/atmel/atmel_hlcdc_plane.c _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ 2018-08-24 8:28 ` Boris Brezillon @ 2018-08-24 15:43 ` Sam Ravnborg 0 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-24 15:43 UTC (permalink / raw) To: Boris Brezillon Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Nicolas Ferre, dri-devel, Rob Herring, Lee Jones, linux-arm-kernel Hi Boris. On Fri, Aug 24, 2018 at 10:28:43AM +0200, Boris Brezillon wrote: > On Sun, 12 Aug 2018 20:46:23 +0200 > Sam Ravnborg <sam@ravnborg.org> wrote: > > > Use vendor name for directory, adding a suitable place > > for more atmel DRM drivers. > > > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > > Cc: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > MAINTAINERS | 2 +- > > drivers/gpu/drm/Kconfig | 2 +- > > drivers/gpu/drm/Makefile | 2 +- > > drivers/gpu/drm/{atmel-hlcdc => atmel}/Kconfig | 6 ++++++ > > drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile | 0 > > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c | 0 > > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c | 0 > > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h | 0 > > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_output.c | 0 > > drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c | 0 > > 10 files changed, 9 insertions(+), 3 deletions(-) > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/Kconfig (83%) > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile (100%) > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c (100%) > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c (100%) > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h (100%) > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_output.c (100%) > > rename drivers/gpu/drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c (100%) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 96e98e206b0d..09ce76a9a1dc 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -4681,7 +4681,7 @@ DRM DRIVERS FOR ATMEL HLCDC > > M: Boris Brezillon <boris.brezillon@bootlin.com> > > L: dri-devel@lists.freedesktop.org > > S: Supported > > -F: drivers/gpu/drm/atmel-hlcdc/ > > +F: drivers/gpu/drm/atmel/atmel-hlcdc* > > atmel_hlcdc* > > Also, I think you can keep the whole directory, and add your name plus > update the entry description after adding code for the LCDC block. Thanks, will do so in v2. > > > F: Documentation/devicetree/bindings/display/atmel/ > > T: git git://anongit.freedesktop.org/drm/drm-misc > > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > index 2a72d2feb76d..4130df0c0dba 100644 > > --- a/drivers/gpu/drm/Kconfig > > +++ b/drivers/gpu/drm/Kconfig > > @@ -232,7 +232,7 @@ source "drivers/gpu/drm/cirrus/Kconfig" > > > > source "drivers/gpu/drm/armada/Kconfig" > > > > -source "drivers/gpu/drm/atmel-hlcdc/Kconfig" > > +source "drivers/gpu/drm/atmel/Kconfig" > > > > source "drivers/gpu/drm/rcar-du/Kconfig" > > > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > > index ef9f3dab287f..ce9829967128 100644 > > --- a/drivers/gpu/drm/Makefile > > +++ b/drivers/gpu/drm/Makefile > > @@ -76,7 +76,7 @@ obj-$(CONFIG_DRM_GMA500) += gma500/ > > obj-$(CONFIG_DRM_UDL) += udl/ > > obj-$(CONFIG_DRM_AST) += ast/ > > obj-$(CONFIG_DRM_ARMADA) += armada/ > > -obj-$(CONFIG_DRM_ATMEL_HLCDC) += atmel-hlcdc/ > > +obj-$(CONFIG_DRM_ATMEL) += atmel/ > > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/ > > obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/ > > obj-y += omapdrm/ > > diff --git a/drivers/gpu/drm/atmel-hlcdc/Kconfig b/drivers/gpu/drm/atmel/Kconfig > > similarity index 83% > > rename from drivers/gpu/drm/atmel-hlcdc/Kconfig > > rename to drivers/gpu/drm/atmel/Kconfig > > index 32bcc4bad06a..7cd3862f9d18 100644 > > --- a/drivers/gpu/drm/atmel-hlcdc/Kconfig > > +++ b/drivers/gpu/drm/atmel/Kconfig > > @@ -1,6 +1,12 @@ > > +config DRM_ATMEL > > + bool > > + help > > + Enable Atmel DRM support > > + > > Not sure why you need an extra Kconfig option? And if you really do > (because you want to share common code), it should be implicitly > selected by DRM_ATMEL_HLCDC and DRM_ATMEL_LCDC. The extra config option is to avoid exposing every single option to the drm makefile, and it is indeed selected by the individual drivers. Similar pattern used in arm/Kconfig. But I see most other places have a prompt that enable the drivers so I will change this to a prompt in v2. Sam _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v1 2/7] dt-binding: add bindings for Atmel LCDC mfd 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ Sam Ravnborg @ 2018-08-12 18:46 ` Sam Ravnborg 2018-08-24 8:45 ` Boris Brezillon 2018-08-12 18:46 ` [PATCH v1 3/7] mfd: add atmel-lcdc driver Sam Ravnborg ` (7 subsequent siblings) 9 siblings, 1 reply; 50+ messages in thread From: Sam Ravnborg @ 2018-08-12 18:46 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, Boris Brezillon, dri-devel, linux-pwm, Sam Ravnborg, linux-arm-kernel The LCDC IP used by some Atmel SOC's have a multifunction device that include two sub-devices: - pwm - display controller This binding describe the multi function device that act as root for the sub-devices The Atmel SOC's are at91sam9 etc. The compatible name is intentionally prefixed with -mfd to avoid clash with existing compatible entries. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Lee Jones <lee.jones@linaro.org> Cc: Boris Brezillon <boris.brezillon@free-electrons.com> --- .../devicetree/bindings/mfd/atmel-lcdc.txt | 75 ++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/atmel-lcdc.txt diff --git a/Documentation/devicetree/bindings/mfd/atmel-lcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-lcdc.txt new file mode 100644 index 000000000000..70e9b7bda6c7 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/atmel-lcdc.txt @@ -0,0 +1,75 @@ +Device-Tree bindings for Atmel's LCDC (LCD Controller) MFD driver + +Required properties: + - compatible: value should be one of the following: + "atmel,at91sam9261-lcdc-mfd" + "atmel,at91sam9263-lcdc-mfd" + "atmel,at91sam9g10-lcdc-mfd" + "atmel,at91sam9g45-lcdc-mfd" + "atmel,at91sam9g46-lcdc-mfd" + "atmel,at91sam9m10-lcdc-mfd" + "atmel,at91sam9m11-lcdc-mfd" + "atmel,at91sam9rl-lcdc-mfd" + - reg: base address and size of the LCDC device registers. + - clock-names: the name of the 2 clocks requested by the LCDC device. + Should contain "lcdc_clk", and "hclk". + - clocks: should contain the 2 clocks requested by the LCDC device. + May specify the same clock twice is there is no need to enable + "hclk" to use the display. + - interrupts: should contain the description of the LCDC interrupt line + +The LCDC IP exposes two subdevices: + - a PWM chip: see ../pwm/atmel-lcdc-pwm.txt + - a Display Controller: see ../display/atmel/lcdc-display-controller.txt + +Example: + lcdc0: lcdc@700000 { + compatible = "atmel,at91sam9263-lcdc-mfd"; + reg = <0x700000 0x1000>; + interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; + clocks = <&lcd_clk>, <&lcd_clk>; + clock-names = "lcdc_clk", "hclk"; + + lcdc-display-controller { + compatible = "atmel,lcdc-display-controller"; + lcd-supply = <&lcdc_reg>; + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + lcdc_panel_output: endpoint@0 { + reg = <0>; + remote-endpoint = <&panel_input>; + }; + }; + }; + + lcdc_pwm: lcdc-pwm { + compatible = "atmel,lcdc-pwm"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_lcdc_pwm>; + #pwm-cells = <3>; + }; + + }; + + panel: panel { + compatible = "logictechnologies,lttd800480070-l2rt", "simple-panel"; + backlight = <&backlight>; + power-supply = <&panel_reg>; + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + #address-cells = <1>; + #size-cells = <0>; + + panel_input: endpoint@0 { + reg = <0>; + remote-endpoint = <&lcdc_panel_output>; + }; + }; + }; -- 2.12.0 ^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH v1 2/7] dt-binding: add bindings for Atmel LCDC mfd 2018-08-12 18:46 ` [PATCH v1 2/7] dt-binding: add bindings for Atmel LCDC mfd Sam Ravnborg @ 2018-08-24 8:45 ` Boris Brezillon 2018-08-24 15:58 ` Sam Ravnborg 0 siblings, 1 reply; 50+ messages in thread From: Boris Brezillon @ 2018-08-24 8:45 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, Nicolas Ferre, dri-devel, Rob Herring, Lee Jones, linux-arm-kernel On Sun, 12 Aug 2018 20:46:24 +0200 Sam Ravnborg <sam@ravnborg.org> wrote: > The LCDC IP used by some Atmel SOC's have a > multifunction device that include two sub-devices: > - pwm > - display controller > > This binding describe the multi function device > that act as root for the sub-devices > > The Atmel SOC's are at91sam9 etc. > > The compatible name is intentionally > prefixed with -mfd to avoid clash with existing > compatible entries. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Lee Jones <lee.jones@linaro.org> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > .../devicetree/bindings/mfd/atmel-lcdc.txt | 75 ++++++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/atmel-lcdc.txt > > diff --git a/Documentation/devicetree/bindings/mfd/atmel-lcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-lcdc.txt > new file mode 100644 > index 000000000000..70e9b7bda6c7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/atmel-lcdc.txt > @@ -0,0 +1,75 @@ > +Device-Tree bindings for Atmel's LCDC (LCD Controller) MFD driver > + > +Required properties: > + - compatible: value should be one of the following: > + "atmel,at91sam9261-lcdc-mfd" > + "atmel,at91sam9263-lcdc-mfd" > + "atmel,at91sam9g10-lcdc-mfd" > + "atmel,at91sam9g45-lcdc-mfd" > + "atmel,at91sam9g46-lcdc-mfd" > + "atmel,at91sam9m10-lcdc-mfd" > + "atmel,at91sam9m11-lcdc-mfd" > + "atmel,at91sam9rl-lcdc-mfd" I'm pretty sure we don't want the -mfd suffix in the compatible. I know it's here to avoid clashes with the old binding, so maybe we can find name. > + - reg: base address and size of the LCDC device registers. > + - clock-names: the name of the 2 clocks requested by the LCDC device. > + Should contain "lcdc_clk", and "hclk". > + - clocks: should contain the 2 clocks requested by the LCDC device. > + May specify the same clock twice is there is no need to enable > + "hclk" to use the display. > + - interrupts: should contain the description of the LCDC interrupt line > + > +The LCDC IP exposes two subdevices: > + - a PWM chip: see ../pwm/atmel-lcdc-pwm.txt > + - a Display Controller: see ../display/atmel/lcdc-display-controller.txt > + > +Example: > + lcdc0: lcdc@700000 { > + compatible = "atmel,at91sam9263-lcdc-mfd"; > + reg = <0x700000 0x1000>; > + interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; > + clocks = <&lcd_clk>, <&lcd_clk>; > + clock-names = "lcdc_clk", "hclk"; > + > + lcdc-display-controller { > + compatible = "atmel,lcdc-display-controller"; > + lcd-supply = <&lcdc_reg>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + lcdc_panel_output: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&panel_input>; > + }; > + }; > + }; > + > + lcdc_pwm: lcdc-pwm { > + compatible = "atmel,lcdc-pwm"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_lcdc_pwm>; > + #pwm-cells = <3>; > + }; > + > + }; Also, I don't remember why I decided to declare distinct nodes for the PWM and display controller, but you should probably try to only declare the lcdc node. Something like that: lcdc0: lcdc@700000 { compatible = "atmel,at91sam9263-lcdc"; reg = <0x700000 0x1000>; interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; clocks = <&lcd_clk>, <&lcd_clk>; clock-names = "lcdc_clk", "hclk"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_lcdc_pwm ...>; #pwm-cells = <3>; #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; #address-cells = <1>; #size-cells = <0>; lcdc_panel_output: endpoint@0 { reg = <0>; remote-endpoint = <&panel_input>; }; }; }; Note that this representation does not prevent us from having an MFD which declares the PWM and Display Engine, it's just that all devs will point to the same of_node. > + > + lcdc-display-controller { > + compatible = "atmel,lcdc-display-controller"; > + lcd-supply = <&lcdc_reg>; Hm, is this a regulator for the LCD controller or the LCD? If it's for the LCD it should be placed under the panel node. > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + lcdc_panel_output: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&panel_input>; > + }; > + }; > + }; > + > + lcdc_pwm: lcdc-pwm { > + compatible = "atmel,lcdc-pwm"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_lcdc_pwm>; > + #pwm-cells = <3>; > + }; > + > + }; > + > + panel: panel { > + compatible = "logictechnologies,lttd800480070-l2rt", "simple-panel"; > + backlight = <&backlight>; > + power-supply = <&panel_reg>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + panel_input: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&lcdc_panel_output>; > + }; > + }; > + }; _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 2/7] dt-binding: add bindings for Atmel LCDC mfd 2018-08-24 8:45 ` Boris Brezillon @ 2018-08-24 15:58 ` Sam Ravnborg 0 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-24 15:58 UTC (permalink / raw) To: Boris Brezillon Cc: Mark Rutland, devicetree, Alexandre Belloni, Boris Brezillon, Nicolas Ferre, linux-pwm, Rob Herring, dri-devel, Lee Jones, linux-arm-kernel Hi Boris. > > +The LCDC IP exposes two subdevices: > > + - a PWM chip: see ../pwm/atmel-lcdc-pwm.txt > > + - a Display Controller: see ../display/atmel/lcdc-display-controller.txt > > + > > +Example: > > + lcdc0: lcdc@700000 { > > + compatible = "atmel,at91sam9263-lcdc-mfd"; > > + reg = <0x700000 0x1000>; > > + interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; > > + clocks = <&lcd_clk>, <&lcd_clk>; > > + clock-names = "lcdc_clk", "hclk"; > > + > > + lcdc-display-controller { > > + compatible = "atmel,lcdc-display-controller"; > > + lcd-supply = <&lcdc_reg>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + lcdc_panel_output: endpoint@0 { > > + reg = <0>; > > + remote-endpoint = <&panel_input>; > > + }; > > + }; > > + }; > > + > > + lcdc_pwm: lcdc-pwm { > > + compatible = "atmel,lcdc-pwm"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_lcdc_pwm>; > > + #pwm-cells = <3>; > > + }; > > + > > + }; > > Also, I don't remember why I decided to declare distinct nodes for the > PWM and display controller, but you should probably try to only declare > the lcdc node. Something like that: > > lcdc0: lcdc@700000 { > compatible = "atmel,at91sam9263-lcdc"; > reg = <0x700000 0x1000>; > interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; > clocks = <&lcd_clk>, <&lcd_clk>; > clock-names = "lcdc_clk", "hclk"; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_lcdc_pwm ...>; > #pwm-cells = <3>; > #address-cells = <1>; > #size-cells = <0>; > > port@0 { > reg = <0>; > #address-cells = <1>; > #size-cells = <0>; > lcdc_panel_output: endpoint@0 { > reg = <0>; > remote-endpoint = <&panel_input>; > }; > }; > }; > > Note that this representation does not prevent us from having an MFD > which declares the PWM and Display Engine, it's just that all devs will > point to the same of_node. I have something similar in my tree now, just need to figure out the mfd parts. > > > + > > + lcdc-display-controller { > > + compatible = "atmel,lcdc-display-controller"; > > + lcd-supply = <&lcdc_reg>; > > Hm, is this a regulator for the LCD controller or the LCD? If it's for > the LCD it should be placed under the panel node. It was added only because I could do it and I was (and continue to be) a DT newbie. Will drop it in v2 as this is just an example which this part do not add any value to. > > + panel: panel { > > + compatible = "logictechnologies,lttd800480070-l2rt", "simple-panel"; Reminds me that I need to dig out a panel-simple patch for a few displays I use (logic + seiko). But they are otherwise not related to this work. Sam _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 2/7] dt-binding: add bindings for Atmel LCDC mfd Sam Ravnborg @ 2018-08-12 18:46 ` Sam Ravnborg 2018-08-14 11:09 ` kbuild test robot ` (3 more replies) 2018-08-12 18:46 ` [PATCH v1 4/7] dt-bindings: add bindings for Atmel LCDC pwm Sam Ravnborg ` (6 subsequent siblings) 9 siblings, 4 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-12 18:46 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, Boris Brezillon, dri-devel, linux-pwm, Sam Ravnborg, linux-arm-kernel The LCDC IP used by some Atmel SOC's have a multifunction device that include two sub-devices: - pwm - display controller This mfd device provide a regmap that can be used by the sub-devices to safely access the registers. The mfd device also support the clock used by the LCDC IP + a bus clock that in some cases are required. The driver is based on the atmel-hlcdc driver. The Atmel SOC's are at91sam9261, at91sam9263 etc. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Lee Jones <lee.jones@linaro.org> Cc: Boris Brezillon <boris.brezillon@free-electrons.com> --- drivers/mfd/Kconfig | 10 +++ drivers/mfd/Makefile | 1 + drivers/mfd/atmel-lcdc.c | 158 +++++++++++++++++++++++++++++++++++ include/linux/mfd/atmel-lcdc.h | 184 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 353 insertions(+) create mode 100644 drivers/mfd/atmel-lcdc.c create mode 100644 include/linux/mfd/atmel-lcdc.h diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index b860eb5aa194..f4851f0f033f 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -121,6 +121,16 @@ config MFD_ATMEL_HLCDC additional drivers must be enabled in order to use the functionality of the device. +config MFD_ATMEL_LCDC + tristate "Atmel LCDC (LCD Controller)" + select MFD_CORE + depends on OF + help + If you say yes here you get support for the LCDC block. + This driver provides common support for accessing the device, + additional drivers must be enabled in order to use the + functionality of the device. + config MFD_ATMEL_SMC bool select MFD_SYSCON diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index e9fd20dba18d..dba8465e0d96 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -186,6 +186,7 @@ obj-$(CONFIG_MFD_TPS65090) += tps65090.o obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o obj-$(CONFIG_MFD_ATMEL_FLEXCOM) += atmel-flexcom.o obj-$(CONFIG_MFD_ATMEL_HLCDC) += atmel-hlcdc.o +obj-$(CONFIG_MFD_ATMEL_LCDC) += atmel-lcdc.o obj-$(CONFIG_MFD_ATMEL_SMC) += atmel-smc.o obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o diff --git a/drivers/mfd/atmel-lcdc.c b/drivers/mfd/atmel-lcdc.c new file mode 100644 index 000000000000..8928976bafca --- /dev/null +++ b/drivers/mfd/atmel-lcdc.c @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Sam Ravnborg + * + * Author: Sam Ravnborg <sam@ravnborg.org> + * + * Based on atmel-hlcdc.c wich is: + * Copyright (C) 2014 Free Electrons + * Copyright (C) 2014 Atmel + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> + */ + +#include <linux/mfd/atmel-lcdc.h> +#include <linux/mfd/core.h> +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/clk.h> +#include <linux/io.h> + +#define ATMEL_LCDC_REG_MAX (0x1000 - 0x4) + +struct lcdc_regmap { + void __iomem *regs; +}; + +static const struct mfd_cell lcdc_cells[] = { + { + .name = "atmel-lcdc-pwm", + .of_compatible = "atmel,lcdc-pwm", + }, + { + .name = "atmel-lcdc-dc", + .of_compatible = "atmel,lcdc-display-controller", + }, +}; + +static int regmap_lcdc_reg_write(void *context, unsigned int reg, + unsigned int val) +{ + struct lcdc_regmap *regmap = context; + + writel(val, regmap->regs + reg); + + return 0; +} + +static int regmap_lcdc_reg_read(void *context, unsigned int reg, + unsigned int *val) +{ + struct lcdc_regmap *regmap = context; + + *val = readl(regmap->regs + reg); + + return 0; +} + +static const struct regmap_config lcdc_regmap_config = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, + .max_register = ATMEL_LCDC_REG_MAX, + .reg_write = regmap_lcdc_reg_write, + .reg_read = regmap_lcdc_reg_read, + .fast_io = true, +}; + +static int lcdc_probe(struct platform_device *pdev) +{ + struct atmel_mfd_lcdc *lcdc; + struct lcdc_regmap *regmap; + struct resource *res; + struct device *dev; + int ret; + + dev = &pdev->dev; + + regmap = devm_kzalloc(dev, sizeof(*regmap), GFP_KERNEL); + if (!regmap) + return -ENOMEM; + + lcdc = devm_kzalloc(dev, sizeof(*lcdc), GFP_KERNEL); + if (!lcdc) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + regmap->regs = devm_ioremap_resource(dev, res); + if (IS_ERR(regmap->regs)) { + dev_err(dev, "Failed to allocate IO mem (%ld)\n", + PTR_ERR(regmap->regs)); + return PTR_ERR(regmap->regs); + } + + lcdc->irq = platform_get_irq(pdev, 0); + if (lcdc->irq < 0) { + dev_err(dev, "Failed to get irq (%d)\n", lcdc->irq); + return lcdc->irq; + } + + lcdc->lcdc_clk = devm_clk_get(dev, "lcdc_clk"); + if (IS_ERR(lcdc->lcdc_clk)) { + dev_err(dev, "failed to get lcdc clock (%ld)\n", + PTR_ERR(lcdc->lcdc_clk)); + return PTR_ERR(lcdc->lcdc_clk); + } + + lcdc->bus_clk = devm_clk_get(dev, "hclk"); + if (IS_ERR(lcdc->bus_clk)) { + dev_err(dev, "failed to get bus clock (%ld)\n", + PTR_ERR(lcdc->bus_clk)); + return PTR_ERR(lcdc->bus_clk); + } + + lcdc->regmap = devm_regmap_init(dev, NULL, regmap, + &lcdc_regmap_config); + if (IS_ERR(lcdc->regmap)) { + dev_err(dev, "Failed to init regmap (%ld)\n", + PTR_ERR(lcdc->regmap)); + return PTR_ERR(lcdc->regmap); + } + + dev_set_drvdata(dev, lcdc); + + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, + lcdc_cells, ARRAY_SIZE(lcdc_cells), + NULL, 0, NULL); + if (ret < 0) + dev_err(dev, "Failed to add %d mfd devices (%d)\n", + ARRAY_SIZE(lcdc_cells), ret); + + return ret; +} + +static const struct of_device_id lcdc_match[] = { + { .compatible = "atmel,at91sam9261-lcdc-mfd" }, + { .compatible = "atmel,at91sam9263-lcdc-mfd" }, + { .compatible = "atmel,at91sam9g10-lcdc-mfd" }, + { .compatible = "atmel,at91sam9g45-lcdc-mfd" }, + { .compatible = "atmel,at91sam9g46-lcdc-mfd" }, + { .compatible = "atmel,at91sam9m10-lcdc-mfd" }, + { .compatible = "atmel,at91sam9m11-lcdc-mfd" }, + { .compatible = "atmel,at91sam9rl-lcdc-mfd" }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, lcdc_match); + +static struct platform_driver lcdc_driver = { + .probe = lcdc_probe, + .driver = { + .name = "atmel-lcdc", + .of_match_table = lcdc_match, + }, +}; +module_platform_driver(lcdc_driver); + +MODULE_ALIAS("platform:atmel-lcdc"); +MODULE_AUTHOR("Sam Ravnborg <sam@ravnborg.org>"); +MODULE_DESCRIPTION("Atmel LCDC mfd driver"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/mfd/atmel-lcdc.h b/include/linux/mfd/atmel-lcdc.h new file mode 100644 index 000000000000..fdab269baa8e --- /dev/null +++ b/include/linux/mfd/atmel-lcdc.h @@ -0,0 +1,184 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Sam Ravnborg + * + * Author: Sam Ravnborg <sam@ravnborg.org> + */ + +#ifndef __LINUX_MFD_LCDC_H +#define __LINUX_MFD_LCDC_H + +#include <linux/regmap.h> +#include <linux/clk.h> + +/** + * Structure shared by the Atmel LCD Controller device and its sub-devices. + * + * @regmap: register map used to access LCDC IP registers + * @lcdc_clk: the lcd controller peripheral clock + * @bus_clk: the bus clock clock (often the same as lcdc_clk) + * @irq: the lcdc irq + */ +struct atmel_mfd_lcdc { + struct regmap *regmap; + struct clk *lcdc_clk; + struct clk *bus_clk; + int irq; +}; + +#define ATMEL_LCDC_DMABADDR1 0x00 +#define ATMEL_LCDC_DMABADDR2 0x04 +#define ATMEL_LCDC_DMAFRMPT1 0x08 +#define ATMEL_LCDC_DMAFRMPT2 0x0c +#define ATMEL_LCDC_DMAFRMADD1 0x10 +#define ATMEL_LCDC_DMAFRMADD2 0x14 + +#define ATMEL_LCDC_DMAFRMCFG 0x18 +#define ATMEL_LCDC_FRSIZE (0x7fffff << 0) +#define ATMEL_LCDC_BLENGTH_OFFSET 24 +#define ATMEL_LCDC_BLENGTH (0x7f << ATMEL_LCDC_BLENGTH_OFFSET) + +#define ATMEL_LCDC_DMACON 0x1c +#define ATMEL_LCDC_DMAEN (0x1 << 0) +#define ATMEL_LCDC_DMARST (0x1 << 1) +#define ATMEL_LCDC_DMABUSY (0x1 << 2) +#define ATMEL_LCDC_DMAUPDT (0x1 << 3) +#define ATMEL_LCDC_DMA2DEN (0x1 << 4) + +#define ATMEL_LCDC_DMA2DCFG 0x20 +#define ATMEL_LCDC_ADDRINC_OFFSET 0 +#define ATMEL_LCDC_ADDRINC (0xffff) +#define ATMEL_LCDC_PIXELOFF_OFFSET 24 +#define ATMEL_LCDC_PIXELOFF (0x1f << 24) + +#define ATMEL_LCDC_LCDCON1 0x0800 +#define ATMEL_LCDC_BYPASS (1 << 0) +#define ATMEL_LCDC_CLKVAL_OFFSET 12 +#define ATMEL_LCDC_CLKVAL (0x1ff << ATMEL_LCDC_CLKVAL_OFFSET) +#define ATMEL_LCDC_LINCNT (0x7ff << 21) + +#define ATMEL_LCDC_LCDCON2 0x0804 +#define ATMEL_LCDC_DISTYPE (3 << 0) +#define ATMEL_LCDC_DISTYPE_STNMONO (0 << 0) +#define ATMEL_LCDC_DISTYPE_STNCOLOR (1 << 0) +#define ATMEL_LCDC_DISTYPE_TFT (2 << 0) +#define ATMEL_LCDC_SCANMOD (1 << 2) +#define ATMEL_LCDC_SCANMOD_SINGLE (0 << 2) +#define ATMEL_LCDC_SCANMOD_DUAL (1 << 2) +#define ATMEL_LCDC_IFWIDTH (3 << 3) +#define ATMEL_LCDC_IFWIDTH_4 (0 << 3) +#define ATMEL_LCDC_IFWIDTH_8 (1 << 3) +#define ATMEL_LCDC_IFWIDTH_16 (2 << 3) +#define ATMEL_LCDC_PIXELSIZE (7 << 5) +#define ATMEL_LCDC_PIXELSIZE_1 (0 << 5) +#define ATMEL_LCDC_PIXELSIZE_2 (1 << 5) +#define ATMEL_LCDC_PIXELSIZE_4 (2 << 5) +#define ATMEL_LCDC_PIXELSIZE_8 (3 << 5) +#define ATMEL_LCDC_PIXELSIZE_16 (4 << 5) +#define ATMEL_LCDC_PIXELSIZE_24 (5 << 5) +#define ATMEL_LCDC_PIXELSIZE_32 (6 << 5) +#define ATMEL_LCDC_INVVD (1 << 8) +#define ATMEL_LCDC_INVVD_NORMAL (0 << 8) +#define ATMEL_LCDC_INVVD_INVERTED (1 << 8) +#define ATMEL_LCDC_INVFRAME (1 << 9) +#define ATMEL_LCDC_INVFRAME_NORMAL (0 << 9) +#define ATMEL_LCDC_INVFRAME_INVERTED (1 << 9) +#define ATMEL_LCDC_INVLINE (1 << 10) +#define ATMEL_LCDC_INVLINE_NORMAL (0 << 10) +#define ATMEL_LCDC_INVLINE_INVERTED (1 << 10) +#define ATMEL_LCDC_INVCLK (1 << 11) +#define ATMEL_LCDC_INVCLK_NORMAL (0 << 11) +#define ATMEL_LCDC_INVCLK_INVERTED (1 << 11) +#define ATMEL_LCDC_INVDVAL (1 << 12) +#define ATMEL_LCDC_INVDVAL_NORMAL (0 << 12) +#define ATMEL_LCDC_INVDVAL_INVERTED (1 << 12) +#define ATMEL_LCDC_CLKMOD (1 << 15) +#define ATMEL_LCDC_CLKMOD_ACTIVEDISPLAY (0 << 15) +#define ATMEL_LCDC_CLKMOD_ALWAYSACTIVE (1 << 15) +#define ATMEL_LCDC_MEMOR (1 << 31) +#define ATMEL_LCDC_MEMOR_BIG (0 << 31) +#define ATMEL_LCDC_MEMOR_LITTLE (1 << 31) + +#define ATMEL_LCDC_TIM1 0x0808 +#define ATMEL_LCDC_VFP_OFFSET 0 +#define ATMEL_LCDC_VFP (0xffU << 0) +#define ATMEL_LCDC_VBP_OFFSET 8 +#define ATMEL_LCDC_VBP (0xffU << ATMEL_LCDC_VBP_OFFSET) +#define ATMEL_LCDC_VPW_OFFSET 16 +#define ATMEL_LCDC_VPW (0x3fU << ATMEL_LCDC_VPW_OFFSET) +#define ATMEL_LCDC_VHDLY_OFFSET 24 +#define ATMEL_LCDC_VHDLY (0xfU << ATMEL_LCDC_VHDLY_OFFSET) + +#define ATMEL_LCDC_TIM2 0x080c +#define ATMEL_LCDC_HBP_OFFSET 0 +#define ATMEL_LCDC_HBP (0xffU << 0) +#define ATMEL_LCDC_HPW_OFFSET 8 +#define ATMEL_LCDC_HPW (0x3fU << ATMEL_LCDC_HPW_OFFSET) +#define ATMEL_LCDC_HFP_OFFSET 21 +#define ATMEL_LCDC_HFP (0x7ffU << ATMEL_LCDC_HFP_OFFSET) + +#define ATMEL_LCDC_LCDFRMCFG 0x0810 +#define ATMEL_LCDC_LINEVAL (0x7ff << 0) +#define ATMEL_LCDC_HOZVAL_OFFSET 21 +#define ATMEL_LCDC_HOZVAL (0x7ff << ATMEL_LCDC_HOZVAL_OFFSET) + +#define ATMEL_LCDC_FIFO 0x0814 +#define ATMEL_LCDC_FIFOTH (0xffff) + +#define ATMEL_LCDC_MVAL 0x0818 + +#define ATMEL_LCDC_DP1_2 0x081c +#define ATMEL_LCDC_DP4_7 0x0820 +#define ATMEL_LCDC_DP3_5 0x0824 +#define ATMEL_LCDC_DP2_3 0x0828 +#define ATMEL_LCDC_DP5_7 0x082c +#define ATMEL_LCDC_DP3_4 0x0830 +#define ATMEL_LCDC_DP4_5 0x0834 +#define ATMEL_LCDC_DP6_7 0x0838 +#define ATMEL_LCDC_DP1_2_VAL (0xff) +#define ATMEL_LCDC_DP4_7_VAL (0xfffffff) +#define ATMEL_LCDC_DP3_5_VAL (0xfffff) +#define ATMEL_LCDC_DP2_3_VAL (0xfff) +#define ATMEL_LCDC_DP5_7_VAL (0xfffffff) +#define ATMEL_LCDC_DP3_4_VAL (0xffff) +#define ATMEL_LCDC_DP4_5_VAL (0xfffff) +#define ATMEL_LCDC_DP6_7_VAL (0xfffffff) + +#define ATMEL_LCDC_PWRCON 0x083c +#define ATMEL_LCDC_PWR (1 << 0) +#define ATMEL_LCDC_GUARDT_OFFSET 1 +#define ATMEL_LCDC_GUARDT (0x7f << ATMEL_LCDC_GUARDT_OFFSET) +#define ATMEL_LCDC_BUSY (1 << 31) + +#define ATMEL_LCDC_CONTRAST_CTR 0x0840 +#define ATMEL_LCDC_PS (3 << 0) +#define ATMEL_LCDC_PS_DIV1 (0 << 0) +#define ATMEL_LCDC_PS_DIV2 (1 << 0) +#define ATMEL_LCDC_PS_DIV4 (2 << 0) +#define ATMEL_LCDC_PS_DIV8 (3 << 0) +#define ATMEL_LCDC_POL (1 << 2) +#define ATMEL_LCDC_POL_NEGATIVE (0 << 2) +#define ATMEL_LCDC_POL_POSITIVE (1 << 2) +#define ATMEL_LCDC_ENA (1 << 3) +#define ATMEL_LCDC_ENA_PWMDISABLE (0 << 3) +#define ATMEL_LCDC_ENA_PWMENABLE (1 << 3) + +#define ATMEL_LCDC_CONTRAST_VAL 0x0844 +#define ATMEL_LCDC_CVAL (0xff) + +#define ATMEL_LCDC_IER 0x0848 +#define ATMEL_LCDC_IDR 0x084c +#define ATMEL_LCDC_IMR 0x0850 +#define ATMEL_LCDC_ISR 0x0854 +#define ATMEL_LCDC_ICR 0x0858 +#define ATMEL_LCDC_LNI (1 << 0) +#define ATMEL_LCDC_LSTLNI (1 << 1) +#define ATMEL_LCDC_EOFI (1 << 2) +#define ATMEL_LCDC_UFLWI (1 << 4) +#define ATMEL_LCDC_OWRI (1 << 5) +#define ATMEL_LCDC_MERI (1 << 6) + +#define ATMEL_LCDC_LUT(n) (0x0c00 + ((n)*4)) +#define ATMEL_LCDC_LUT_SIZE 256 + +#endif /* __LINUX_MFD_LCDC_H */ -- 2.12.0 ^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-12 18:46 ` [PATCH v1 3/7] mfd: add atmel-lcdc driver Sam Ravnborg @ 2018-08-14 11:09 ` kbuild test robot 2018-08-15 5:24 ` Lee Jones ` (2 subsequent siblings) 3 siblings, 0 replies; 50+ messages in thread From: kbuild test robot @ 2018-08-14 11:09 UTC (permalink / raw) Cc: Mark Rutland, devicetree, Alexandre Belloni, Boris Brezillon, Sam Ravnborg, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, linux-pwm, kbuild-all, Lee Jones, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 3488 bytes --] Hi Sam, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on at91/at91-next] [also build test WARNING on v4.18 next-20180813] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sam-Ravnborg/add-at91sam9-LCDC-DRM-driver/20180814-163056 base: https://git.kernel.org/pub/scm/linux/kernel/git/nferre/linux-at91.git at91-next config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 All warnings (new ones prefixed by >>): drivers/mfd/atmel-lcdc.c: In function 'lcdc_probe': >> drivers/mfd/atmel-lcdc.c:127:32: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=] dev_err(dev, "Failed to add %d mfd devices (%d)\n", ~^ %ld vim +127 drivers/mfd/atmel-lcdc.c 66 67 static int lcdc_probe(struct platform_device *pdev) 68 { 69 struct atmel_mfd_lcdc *lcdc; 70 struct lcdc_regmap *regmap; 71 struct resource *res; 72 struct device *dev; 73 int ret; 74 75 dev = &pdev->dev; 76 77 regmap = devm_kzalloc(dev, sizeof(*regmap), GFP_KERNEL); 78 if (!regmap) 79 return -ENOMEM; 80 81 lcdc = devm_kzalloc(dev, sizeof(*lcdc), GFP_KERNEL); 82 if (!lcdc) 83 return -ENOMEM; 84 85 res = platform_get_resource(pdev, IORESOURCE_MEM, 0); 86 regmap->regs = devm_ioremap_resource(dev, res); 87 if (IS_ERR(regmap->regs)) { 88 dev_err(dev, "Failed to allocate IO mem (%ld)\n", 89 PTR_ERR(regmap->regs)); 90 return PTR_ERR(regmap->regs); 91 } 92 93 lcdc->irq = platform_get_irq(pdev, 0); 94 if (lcdc->irq < 0) { 95 dev_err(dev, "Failed to get irq (%d)\n", lcdc->irq); 96 return lcdc->irq; 97 } 98 99 lcdc->lcdc_clk = devm_clk_get(dev, "lcdc_clk"); 100 if (IS_ERR(lcdc->lcdc_clk)) { 101 dev_err(dev, "failed to get lcdc clock (%ld)\n", 102 PTR_ERR(lcdc->lcdc_clk)); 103 return PTR_ERR(lcdc->lcdc_clk); 104 } 105 106 lcdc->bus_clk = devm_clk_get(dev, "hclk"); 107 if (IS_ERR(lcdc->bus_clk)) { 108 dev_err(dev, "failed to get bus clock (%ld)\n", 109 PTR_ERR(lcdc->bus_clk)); 110 return PTR_ERR(lcdc->bus_clk); 111 } 112 113 lcdc->regmap = devm_regmap_init(dev, NULL, regmap, 114 &lcdc_regmap_config); 115 if (IS_ERR(lcdc->regmap)) { 116 dev_err(dev, "Failed to init regmap (%ld)\n", 117 PTR_ERR(lcdc->regmap)); 118 return PTR_ERR(lcdc->regmap); 119 } 120 121 dev_set_drvdata(dev, lcdc); 122 123 ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, 124 lcdc_cells, ARRAY_SIZE(lcdc_cells), 125 NULL, 0, NULL); 126 if (ret < 0) > 127 dev_err(dev, "Failed to add %d mfd devices (%d)\n", 128 ARRAY_SIZE(lcdc_cells), ret); 129 130 return ret; 131 } 132 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 54384 bytes --] [-- Attachment #3: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-12 18:46 ` [PATCH v1 3/7] mfd: add atmel-lcdc driver Sam Ravnborg 2018-08-14 11:09 ` kbuild test robot @ 2018-08-15 5:24 ` Lee Jones 2018-08-15 20:40 ` Sam Ravnborg 2018-08-24 8:15 ` Boris Brezillon 2018-08-15 8:11 ` kbuild test robot 2018-08-24 8:48 ` Boris Brezillon 3 siblings, 2 replies; 50+ messages in thread From: Lee Jones @ 2018-08-15 5:24 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, linux-arm-kernel On Sun, 12 Aug 2018, Sam Ravnborg wrote: > The LCDC IP used by some Atmel SOC's have a > multifunction device that include two sub-devices: > - pwm > - display controller > > This mfd device provide a regmap that can be used by the > sub-devices to safely access the registers. > The mfd device also support the clock used by the > LCDC IP + a bus clock that in some cases are required. > > The driver is based on the atmel-hlcdc driver. > > The Atmel SOC's are at91sam9261, at91sam9263 etc. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Lee Jones <lee.jones@linaro.org> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > drivers/mfd/Kconfig | 10 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/atmel-lcdc.c | 158 +++++++++++++++++++++++++++++++++++ > include/linux/mfd/atmel-lcdc.h | 184 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 353 insertions(+) > create mode 100644 drivers/mfd/atmel-lcdc.c > create mode 100644 include/linux/mfd/atmel-lcdc.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index b860eb5aa194..f4851f0f033f 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -121,6 +121,16 @@ config MFD_ATMEL_HLCDC > additional drivers must be enabled in order to use the > functionality of the device. > > +config MFD_ATMEL_LCDC > + tristate "Atmel LCDC (LCD Controller)" > + select MFD_CORE > + depends on OF > + help > + If you say yes here you get support for the LCDC block. > + This driver provides common support for accessing the device, > + additional drivers must be enabled in order to use the > + functionality of the device. > + > config MFD_ATMEL_SMC > bool > select MFD_SYSCON > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index e9fd20dba18d..dba8465e0d96 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -186,6 +186,7 @@ obj-$(CONFIG_MFD_TPS65090) += tps65090.o > obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o > obj-$(CONFIG_MFD_ATMEL_FLEXCOM) += atmel-flexcom.o > obj-$(CONFIG_MFD_ATMEL_HLCDC) += atmel-hlcdc.o > +obj-$(CONFIG_MFD_ATMEL_LCDC) += atmel-lcdc.o > obj-$(CONFIG_MFD_ATMEL_SMC) += atmel-smc.o > obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o > obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o > diff --git a/drivers/mfd/atmel-lcdc.c b/drivers/mfd/atmel-lcdc.c > new file mode 100644 > index 000000000000..8928976bafca > --- /dev/null > +++ b/drivers/mfd/atmel-lcdc.c > @@ -0,0 +1,158 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Sam Ravnborg > + * > + * Author: Sam Ravnborg <sam@ravnborg.org> > + * > + * Based on atmel-hlcdc.c wich is: > + * Copyright (C) 2014 Free Electrons > + * Copyright (C) 2014 Atmel > + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> > + */ > + > +#include <linux/mfd/atmel-lcdc.h> > +#include <linux/mfd/core.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/clk.h> > +#include <linux/io.h> > + > +#define ATMEL_LCDC_REG_MAX (0x1000 - 0x4) > + > +struct lcdc_regmap { > + void __iomem *regs; > +}; > + > +static const struct mfd_cell lcdc_cells[] = { > + { > + .name = "atmel-lcdc-pwm", > + .of_compatible = "atmel,lcdc-pwm", > + }, > + { > + .name = "atmel-lcdc-dc", > + .of_compatible = "atmel,lcdc-display-controller", > + }, > +}; Will you be adding any more devices, or is this the entirety of the device? If the latter, I suggest that this doesn't warrant being an MFD. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-15 5:24 ` Lee Jones @ 2018-08-15 20:40 ` Sam Ravnborg 2018-08-16 8:28 ` Nicolas Ferre 2018-08-24 8:15 ` Boris Brezillon 1 sibling, 1 reply; 50+ messages in thread From: Sam Ravnborg @ 2018-08-15 20:40 UTC (permalink / raw) To: Lee Jones Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, dri-devel, Boris Brezillon, Rob Herring, linux-arm-kernel Hi Lee. > > + > > +static const struct mfd_cell lcdc_cells[] = { > > + { > > + .name = "atmel-lcdc-pwm", > > + .of_compatible = "atmel,lcdc-pwm", > > + }, > > + { > > + .name = "atmel-lcdc-dc", > > + .of_compatible = "atmel,lcdc-display-controller", > > + }, > > +}; > > Will you be adding any more devices, or is this the entirety of the > device? If the latter, I suggest that this doesn't warrant being an > MFD. Thats it. And others agree with you that this is not a good approach. So in v2 there will be no MFD. Thanks for confirming that the non-mfd way is the better approach. Sam ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-15 20:40 ` Sam Ravnborg @ 2018-08-16 8:28 ` Nicolas Ferre 2018-08-16 8:42 ` Lee Jones 2018-08-24 8:37 ` Boris Brezillon 0 siblings, 2 replies; 50+ messages in thread From: Nicolas Ferre @ 2018-08-16 8:28 UTC (permalink / raw) To: Sam Ravnborg, Lee Jones Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, dri-devel, Boris Brezillon, Rob Herring, linux-arm-kernel On 15/08/2018 at 22:40, Sam Ravnborg wrote: > Hi Lee. > >>> + >>> +static const struct mfd_cell lcdc_cells[] = { >>> + { >>> + .name = "atmel-lcdc-pwm", >>> + .of_compatible = "atmel,lcdc-pwm", >>> + }, >>> + { >>> + .name = "atmel-lcdc-dc", >>> + .of_compatible = "atmel,lcdc-display-controller", >>> + }, >>> +}; >> >> Will you be adding any more devices, or is this the entirety of the >> device? If the latter, I suggest that this doesn't warrant being an >> MFD. > Thats it. And others agree with you that this is not a good approach. > So in v2 there will be no MFD. > > Thanks for confirming that the non-mfd way is the better approach. MFD approach would have had the benefit of keeping this driver series architecture close to the HLCD one. This would have been easier to understand and use one SoC or another one from the AT91 product line.... Anyway, I'd wait for Boris' feedback for making a decision. Best regards, -- Nicolas Ferre ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-16 8:28 ` Nicolas Ferre @ 2018-08-16 8:42 ` Lee Jones 2018-08-24 8:37 ` Boris Brezillon 1 sibling, 0 replies; 50+ messages in thread From: Lee Jones @ 2018-08-16 8:42 UTC (permalink / raw) To: Nicolas Ferre Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, dri-devel, Boris Brezillon, Rob Herring, Sam Ravnborg, linux-arm-kernel On Thu, 16 Aug 2018, Nicolas Ferre wrote: > On 15/08/2018 at 22:40, Sam Ravnborg wrote: > > Hi Lee. > > > > > > + > > > > +static const struct mfd_cell lcdc_cells[] = { > > > > + { > > > > + .name = "atmel-lcdc-pwm", > > > > + .of_compatible = "atmel,lcdc-pwm", > > > > + }, > > > > + { > > > > + .name = "atmel-lcdc-dc", > > > > + .of_compatible = "atmel,lcdc-display-controller", > > > > + }, > > > > +}; > > > > > > Will you be adding any more devices, or is this the entirety of the > > > device? If the latter, I suggest that this doesn't warrant being an > > > MFD. > > Thats it. And others agree with you that this is not a good approach. > > So in v2 there will be no MFD. > > > > Thanks for confirming that the non-mfd way is the better approach. > > MFD approach would have had the benefit of keeping this driver series > architecture close to the HLCD one. This would have been easier to > understand and use one SoC or another one from the AT91 product line.... Yes, that is true. They are very similar drivers. Would it make sense to use the same driver for both devices? > Anyway, I'd wait for Boris' feedback for making a decision. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-16 8:28 ` Nicolas Ferre 2018-08-16 8:42 ` Lee Jones @ 2018-08-24 8:37 ` Boris Brezillon 1 sibling, 0 replies; 50+ messages in thread From: Boris Brezillon @ 2018-08-24 8:37 UTC (permalink / raw) To: Nicolas Ferre Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Sam Ravnborg, dri-devel, Rob Herring, Boris Brezillon, Lee Jones, linux-arm-kernel On Thu, 16 Aug 2018 10:28:54 +0200 Nicolas Ferre <nicolas.ferre@microchip.com> wrote: > On 15/08/2018 at 22:40, Sam Ravnborg wrote: > > Hi Lee. > > > >>> + > >>> +static const struct mfd_cell lcdc_cells[] = { > >>> + { > >>> + .name = "atmel-lcdc-pwm", > >>> + .of_compatible = "atmel,lcdc-pwm", > >>> + }, > >>> + { > >>> + .name = "atmel-lcdc-dc", > >>> + .of_compatible = "atmel,lcdc-display-controller", > >>> + }, > >>> +}; > >> > >> Will you be adding any more devices, or is this the entirety of the > >> device? If the latter, I suggest that this doesn't warrant being an > >> MFD. > > Thats it. And others agree with you that this is not a good approach. > > So in v2 there will be no MFD. > > > > Thanks for confirming that the non-mfd way is the better approach. > > MFD approach would have had the benefit of keeping this driver series > architecture close to the HLCD one. This would have been easier to > understand and use one SoC or another one from the AT91 product line.... > > Anyway, I'd wait for Boris' feedback for making a decision. If possible I'd like to keep the MFD approach, but let's see if we can have a single node in the DT instead of one for the MFD and 2 child nodes (for the display controller and the PWM). _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-15 5:24 ` Lee Jones 2018-08-15 20:40 ` Sam Ravnborg @ 2018-08-24 8:15 ` Boris Brezillon 2018-08-24 10:58 ` Lee Jones 1 sibling, 1 reply; 50+ messages in thread From: Boris Brezillon @ 2018-08-24 8:15 UTC (permalink / raw) To: Lee Jones Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, Nicolas Ferre, dri-devel, Rob Herring, Sam Ravnborg, linux-arm-kernel Hi Lee, On Wed, 15 Aug 2018 06:24:35 +0100 Lee Jones <lee.jones@linaro.org> wrote: > > +static const struct mfd_cell lcdc_cells[] = { > > + { > > + .name = "atmel-lcdc-pwm", > > + .of_compatible = "atmel,lcdc-pwm", > > + }, > > + { > > + .name = "atmel-lcdc-dc", > > + .of_compatible = "atmel,lcdc-display-controller", > > + }, > > +}; > > Will you be adding any more devices, or is this the entirety of the > device? If the latter, I suggest that this doesn't warrant being an > MFD. > Is there a lower limit to define when an MFD is recommended, or is it that you find a PWM (driving a backlight) and a display controller close enough to be implemented in a single driver? I personally prefer the separation we have today, because I can then place the drivers where they belong (PWM subsystem and DRM subsystem) and the respective maintainers know about these drivers. Regards, Boris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-24 8:15 ` Boris Brezillon @ 2018-08-24 10:58 ` Lee Jones 0 siblings, 0 replies; 50+ messages in thread From: Lee Jones @ 2018-08-24 10:58 UTC (permalink / raw) To: Boris Brezillon Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, Nicolas Ferre, dri-devel, Rob Herring, Sam Ravnborg, linux-arm-kernel On Fri, 24 Aug 2018, Boris Brezillon wrote: > Hi Lee, > > On Wed, 15 Aug 2018 06:24:35 +0100 > Lee Jones <lee.jones@linaro.org> wrote: > > > > +static const struct mfd_cell lcdc_cells[] = { > > > + { > > > + .name = "atmel-lcdc-pwm", > > > + .of_compatible = "atmel,lcdc-pwm", > > > + }, > > > + { > > > + .name = "atmel-lcdc-dc", > > > + .of_compatible = "atmel,lcdc-display-controller", > > > + }, > > > +}; > > > > Will you be adding any more devices, or is this the entirety of the > > device? If the latter, I suggest that this doesn't warrant being an > > MFD. > > > > Is there a lower limit to define when an MFD is recommended, or is it > that you find a PWM (driving a backlight) and a display controller > close enough to be implemented in a single driver? I was erring towards that latter. > I personally prefer the separation we have today, because I can then > place the drivers where they belong (PWM subsystem and DRM subsystem) > and the respective maintainers know about these drivers. Yes separation is good a good thing. Not placing them in MFD and having the individual parts reside in the appropriate subsystems are not mutually exclusive though. I assume the Display Controller is higher ranking than the PWM device right? Seeing as they are so closely bound i.e. the DC can't operated with the PWM, it would be justifiable to register the PWM from the DC. Of course if there is complicated set-up to be done, lots of devices are involved or there are shared functions between them, then that is where MFD usually steps in. However, since there is a very similar device already in MFD, I suggest you simply extend it to add support for this new device and have done. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-12 18:46 ` [PATCH v1 3/7] mfd: add atmel-lcdc driver Sam Ravnborg 2018-08-14 11:09 ` kbuild test robot 2018-08-15 5:24 ` Lee Jones @ 2018-08-15 8:11 ` kbuild test robot 2018-08-24 8:48 ` Boris Brezillon 3 siblings, 0 replies; 50+ messages in thread From: kbuild test robot @ 2018-08-15 8:11 UTC (permalink / raw) Cc: Mark Rutland, devicetree, Alexandre Belloni, Boris Brezillon, Sam Ravnborg, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, linux-pwm, kbuild-all, Lee Jones, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 5267 bytes --] Hi Sam, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on at91/at91-next] [also build test WARNING on v4.18 next-20180814] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sam-Ravnborg/add-at91sam9-LCDC-DRM-driver/20180814-163056 base: https://git.kernel.org/pub/scm/linux/kernel/git/nferre/linux-at91.git at91-next config: x86_64-randconfig-g0-08151425 (attached as .config) compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): In file included from include/linux/kernel.h:15:0, from include/linux/list.h:9, from include/linux/regmap.h:16, from include/linux/mfd/atmel-lcdc.h:11, from drivers/mfd/atmel-lcdc.c:13: drivers/mfd/atmel-lcdc.c: In function 'lcdc_probe': >> include/linux/build_bug.h:29:45: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=] #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); })) ^ include/linux/compiler-gcc.h:65:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO' #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0])) ^ include/linux/kernel.h:72:59: note: in expansion of macro '__must_be_array' #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) ^ >> drivers/mfd/atmel-lcdc.c:128:4: note: in expansion of macro 'ARRAY_SIZE' ARRAY_SIZE(lcdc_cells), ret); ^ -- In file included from include/linux/kernel.h:15:0, from include/linux/list.h:9, from include/linux/regmap.h:16, from include/linux/mfd/atmel-lcdc.h:11, from drivers//mfd/atmel-lcdc.c:13: drivers//mfd/atmel-lcdc.c: In function 'lcdc_probe': >> include/linux/build_bug.h:29:45: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=] #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); })) ^ include/linux/compiler-gcc.h:65:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO' #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0])) ^ include/linux/kernel.h:72:59: note: in expansion of macro '__must_be_array' #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) ^ drivers//mfd/atmel-lcdc.c:128:4: note: in expansion of macro 'ARRAY_SIZE' ARRAY_SIZE(lcdc_cells), ret); ^ vim +/ARRAY_SIZE +128 drivers/mfd/atmel-lcdc.c 66 67 static int lcdc_probe(struct platform_device *pdev) 68 { 69 struct atmel_mfd_lcdc *lcdc; 70 struct lcdc_regmap *regmap; 71 struct resource *res; 72 struct device *dev; 73 int ret; 74 75 dev = &pdev->dev; 76 77 regmap = devm_kzalloc(dev, sizeof(*regmap), GFP_KERNEL); 78 if (!regmap) 79 return -ENOMEM; 80 81 lcdc = devm_kzalloc(dev, sizeof(*lcdc), GFP_KERNEL); 82 if (!lcdc) 83 return -ENOMEM; 84 85 res = platform_get_resource(pdev, IORESOURCE_MEM, 0); 86 regmap->regs = devm_ioremap_resource(dev, res); 87 if (IS_ERR(regmap->regs)) { 88 dev_err(dev, "Failed to allocate IO mem (%ld)\n", 89 PTR_ERR(regmap->regs)); 90 return PTR_ERR(regmap->regs); 91 } 92 93 lcdc->irq = platform_get_irq(pdev, 0); 94 if (lcdc->irq < 0) { 95 dev_err(dev, "Failed to get irq (%d)\n", lcdc->irq); 96 return lcdc->irq; 97 } 98 99 lcdc->lcdc_clk = devm_clk_get(dev, "lcdc_clk"); 100 if (IS_ERR(lcdc->lcdc_clk)) { 101 dev_err(dev, "failed to get lcdc clock (%ld)\n", 102 PTR_ERR(lcdc->lcdc_clk)); 103 return PTR_ERR(lcdc->lcdc_clk); 104 } 105 106 lcdc->bus_clk = devm_clk_get(dev, "hclk"); 107 if (IS_ERR(lcdc->bus_clk)) { 108 dev_err(dev, "failed to get bus clock (%ld)\n", 109 PTR_ERR(lcdc->bus_clk)); 110 return PTR_ERR(lcdc->bus_clk); 111 } 112 113 lcdc->regmap = devm_regmap_init(dev, NULL, regmap, 114 &lcdc_regmap_config); 115 if (IS_ERR(lcdc->regmap)) { 116 dev_err(dev, "Failed to init regmap (%ld)\n", 117 PTR_ERR(lcdc->regmap)); 118 return PTR_ERR(lcdc->regmap); 119 } 120 121 dev_set_drvdata(dev, lcdc); 122 123 ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, 124 lcdc_cells, ARRAY_SIZE(lcdc_cells), 125 NULL, 0, NULL); 126 if (ret < 0) 127 dev_err(dev, "Failed to add %d mfd devices (%d)\n", > 128 ARRAY_SIZE(lcdc_cells), ret); 129 130 return ret; 131 } 132 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 29238 bytes --] [-- Attachment #3: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 3/7] mfd: add atmel-lcdc driver 2018-08-12 18:46 ` [PATCH v1 3/7] mfd: add atmel-lcdc driver Sam Ravnborg ` (2 preceding siblings ...) 2018-08-15 8:11 ` kbuild test robot @ 2018-08-24 8:48 ` Boris Brezillon 3 siblings, 0 replies; 50+ messages in thread From: Boris Brezillon @ 2018-08-24 8:48 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, Nicolas Ferre, dri-devel, Rob Herring, Lee Jones, linux-arm-kernel On Sun, 12 Aug 2018 20:46:25 +0200 Sam Ravnborg <sam@ravnborg.org> wrote: > The LCDC IP used by some Atmel SOC's have a > multifunction device that include two sub-devices: > - pwm > - display controller > > This mfd device provide a regmap that can be used by the > sub-devices to safely access the registers. > The mfd device also support the clock used by the > LCDC IP + a bus clock that in some cases are required. > > The driver is based on the atmel-hlcdc driver. Looks like it's (almost?) the same logic. It's probably better to have only one driver and just add new compatibles. > > The Atmel SOC's are at91sam9261, at91sam9263 etc. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Lee Jones <lee.jones@linaro.org> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > drivers/mfd/Kconfig | 10 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/atmel-lcdc.c | 158 +++++++++++++++++++++++++++++++++++ > include/linux/mfd/atmel-lcdc.h | 184 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 353 insertions(+) > create mode 100644 drivers/mfd/atmel-lcdc.c > create mode 100644 include/linux/mfd/atmel-lcdc.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index b860eb5aa194..f4851f0f033f 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -121,6 +121,16 @@ config MFD_ATMEL_HLCDC > additional drivers must be enabled in order to use the > functionality of the device. > > +config MFD_ATMEL_LCDC > + tristate "Atmel LCDC (LCD Controller)" > + select MFD_CORE > + depends on OF > + help > + If you say yes here you get support for the LCDC block. > + This driver provides common support for accessing the device, > + additional drivers must be enabled in order to use the > + functionality of the device. > + > config MFD_ATMEL_SMC > bool > select MFD_SYSCON > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index e9fd20dba18d..dba8465e0d96 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -186,6 +186,7 @@ obj-$(CONFIG_MFD_TPS65090) += tps65090.o > obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o > obj-$(CONFIG_MFD_ATMEL_FLEXCOM) += atmel-flexcom.o > obj-$(CONFIG_MFD_ATMEL_HLCDC) += atmel-hlcdc.o > +obj-$(CONFIG_MFD_ATMEL_LCDC) += atmel-lcdc.o > obj-$(CONFIG_MFD_ATMEL_SMC) += atmel-smc.o > obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o > obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o > diff --git a/drivers/mfd/atmel-lcdc.c b/drivers/mfd/atmel-lcdc.c > new file mode 100644 > index 000000000000..8928976bafca > --- /dev/null > +++ b/drivers/mfd/atmel-lcdc.c > @@ -0,0 +1,158 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Sam Ravnborg > + * > + * Author: Sam Ravnborg <sam@ravnborg.org> > + * > + * Based on atmel-hlcdc.c wich is: > + * Copyright (C) 2014 Free Electrons > + * Copyright (C) 2014 Atmel > + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> > + */ > + > +#include <linux/mfd/atmel-lcdc.h> > +#include <linux/mfd/core.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/clk.h> > +#include <linux/io.h> > + > +#define ATMEL_LCDC_REG_MAX (0x1000 - 0x4) > + > +struct lcdc_regmap { > + void __iomem *regs; > +}; > + > +static const struct mfd_cell lcdc_cells[] = { > + { > + .name = "atmel-lcdc-pwm", > + .of_compatible = "atmel,lcdc-pwm", > + }, > + { > + .name = "atmel-lcdc-dc", > + .of_compatible = "atmel,lcdc-display-controller", > + }, > +}; > + > +static int regmap_lcdc_reg_write(void *context, unsigned int reg, > + unsigned int val) > +{ > + struct lcdc_regmap *regmap = context; > + > + writel(val, regmap->regs + reg); > + > + return 0; > +} > + > +static int regmap_lcdc_reg_read(void *context, unsigned int reg, > + unsigned int *val) > +{ > + struct lcdc_regmap *regmap = context; > + > + *val = readl(regmap->regs + reg); > + > + return 0; > +} > + > +static const struct regmap_config lcdc_regmap_config = { > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > + .max_register = ATMEL_LCDC_REG_MAX, > + .reg_write = regmap_lcdc_reg_write, > + .reg_read = regmap_lcdc_reg_read, > + .fast_io = true, > +}; > + > +static int lcdc_probe(struct platform_device *pdev) > +{ > + struct atmel_mfd_lcdc *lcdc; > + struct lcdc_regmap *regmap; > + struct resource *res; > + struct device *dev; > + int ret; > + > + dev = &pdev->dev; > + > + regmap = devm_kzalloc(dev, sizeof(*regmap), GFP_KERNEL); > + if (!regmap) > + return -ENOMEM; > + > + lcdc = devm_kzalloc(dev, sizeof(*lcdc), GFP_KERNEL); > + if (!lcdc) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + regmap->regs = devm_ioremap_resource(dev, res); > + if (IS_ERR(regmap->regs)) { > + dev_err(dev, "Failed to allocate IO mem (%ld)\n", > + PTR_ERR(regmap->regs)); > + return PTR_ERR(regmap->regs); > + } > + > + lcdc->irq = platform_get_irq(pdev, 0); > + if (lcdc->irq < 0) { > + dev_err(dev, "Failed to get irq (%d)\n", lcdc->irq); > + return lcdc->irq; > + } > + > + lcdc->lcdc_clk = devm_clk_get(dev, "lcdc_clk"); > + if (IS_ERR(lcdc->lcdc_clk)) { > + dev_err(dev, "failed to get lcdc clock (%ld)\n", > + PTR_ERR(lcdc->lcdc_clk)); > + return PTR_ERR(lcdc->lcdc_clk); > + } > + > + lcdc->bus_clk = devm_clk_get(dev, "hclk"); > + if (IS_ERR(lcdc->bus_clk)) { > + dev_err(dev, "failed to get bus clock (%ld)\n", > + PTR_ERR(lcdc->bus_clk)); > + return PTR_ERR(lcdc->bus_clk); > + } > + > + lcdc->regmap = devm_regmap_init(dev, NULL, regmap, > + &lcdc_regmap_config); > + if (IS_ERR(lcdc->regmap)) { > + dev_err(dev, "Failed to init regmap (%ld)\n", > + PTR_ERR(lcdc->regmap)); > + return PTR_ERR(lcdc->regmap); > + } > + > + dev_set_drvdata(dev, lcdc); > + > + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, > + lcdc_cells, ARRAY_SIZE(lcdc_cells), > + NULL, 0, NULL); > + if (ret < 0) > + dev_err(dev, "Failed to add %d mfd devices (%d)\n", > + ARRAY_SIZE(lcdc_cells), ret); > + > + return ret; > +} > + > +static const struct of_device_id lcdc_match[] = { > + { .compatible = "atmel,at91sam9261-lcdc-mfd" }, > + { .compatible = "atmel,at91sam9263-lcdc-mfd" }, > + { .compatible = "atmel,at91sam9g10-lcdc-mfd" }, > + { .compatible = "atmel,at91sam9g45-lcdc-mfd" }, > + { .compatible = "atmel,at91sam9g46-lcdc-mfd" }, > + { .compatible = "atmel,at91sam9m10-lcdc-mfd" }, > + { .compatible = "atmel,at91sam9m11-lcdc-mfd" }, > + { .compatible = "atmel,at91sam9rl-lcdc-mfd" }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, lcdc_match); > + > +static struct platform_driver lcdc_driver = { > + .probe = lcdc_probe, > + .driver = { > + .name = "atmel-lcdc", > + .of_match_table = lcdc_match, > + }, > +}; > +module_platform_driver(lcdc_driver); > + > +MODULE_ALIAS("platform:atmel-lcdc"); > +MODULE_AUTHOR("Sam Ravnborg <sam@ravnborg.org>"); > +MODULE_DESCRIPTION("Atmel LCDC mfd driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/mfd/atmel-lcdc.h b/include/linux/mfd/atmel-lcdc.h > new file mode 100644 > index 000000000000..fdab269baa8e > --- /dev/null > +++ b/include/linux/mfd/atmel-lcdc.h > @@ -0,0 +1,184 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 Sam Ravnborg > + * > + * Author: Sam Ravnborg <sam@ravnborg.org> > + */ > + > +#ifndef __LINUX_MFD_LCDC_H > +#define __LINUX_MFD_LCDC_H > + > +#include <linux/regmap.h> > +#include <linux/clk.h> > + > +/** > + * Structure shared by the Atmel LCD Controller device and its sub-devices. > + * > + * @regmap: register map used to access LCDC IP registers > + * @lcdc_clk: the lcd controller peripheral clock > + * @bus_clk: the bus clock clock (often the same as lcdc_clk) > + * @irq: the lcdc irq > + */ > +struct atmel_mfd_lcdc { > + struct regmap *regmap; > + struct clk *lcdc_clk; > + struct clk *bus_clk; > + int irq; > +}; > + > +#define ATMEL_LCDC_DMABADDR1 0x00 > +#define ATMEL_LCDC_DMABADDR2 0x04 > +#define ATMEL_LCDC_DMAFRMPT1 0x08 > +#define ATMEL_LCDC_DMAFRMPT2 0x0c > +#define ATMEL_LCDC_DMAFRMADD1 0x10 > +#define ATMEL_LCDC_DMAFRMADD2 0x14 > + > +#define ATMEL_LCDC_DMAFRMCFG 0x18 > +#define ATMEL_LCDC_FRSIZE (0x7fffff << 0) > +#define ATMEL_LCDC_BLENGTH_OFFSET 24 > +#define ATMEL_LCDC_BLENGTH (0x7f << ATMEL_LCDC_BLENGTH_OFFSET) > + > +#define ATMEL_LCDC_DMACON 0x1c > +#define ATMEL_LCDC_DMAEN (0x1 << 0) > +#define ATMEL_LCDC_DMARST (0x1 << 1) > +#define ATMEL_LCDC_DMABUSY (0x1 << 2) > +#define ATMEL_LCDC_DMAUPDT (0x1 << 3) > +#define ATMEL_LCDC_DMA2DEN (0x1 << 4) > + > +#define ATMEL_LCDC_DMA2DCFG 0x20 > +#define ATMEL_LCDC_ADDRINC_OFFSET 0 > +#define ATMEL_LCDC_ADDRINC (0xffff) > +#define ATMEL_LCDC_PIXELOFF_OFFSET 24 > +#define ATMEL_LCDC_PIXELOFF (0x1f << 24) > + > +#define ATMEL_LCDC_LCDCON1 0x0800 > +#define ATMEL_LCDC_BYPASS (1 << 0) > +#define ATMEL_LCDC_CLKVAL_OFFSET 12 > +#define ATMEL_LCDC_CLKVAL (0x1ff << ATMEL_LCDC_CLKVAL_OFFSET) > +#define ATMEL_LCDC_LINCNT (0x7ff << 21) > + > +#define ATMEL_LCDC_LCDCON2 0x0804 > +#define ATMEL_LCDC_DISTYPE (3 << 0) > +#define ATMEL_LCDC_DISTYPE_STNMONO (0 << 0) > +#define ATMEL_LCDC_DISTYPE_STNCOLOR (1 << 0) > +#define ATMEL_LCDC_DISTYPE_TFT (2 << 0) > +#define ATMEL_LCDC_SCANMOD (1 << 2) > +#define ATMEL_LCDC_SCANMOD_SINGLE (0 << 2) > +#define ATMEL_LCDC_SCANMOD_DUAL (1 << 2) > +#define ATMEL_LCDC_IFWIDTH (3 << 3) > +#define ATMEL_LCDC_IFWIDTH_4 (0 << 3) > +#define ATMEL_LCDC_IFWIDTH_8 (1 << 3) > +#define ATMEL_LCDC_IFWIDTH_16 (2 << 3) > +#define ATMEL_LCDC_PIXELSIZE (7 << 5) > +#define ATMEL_LCDC_PIXELSIZE_1 (0 << 5) > +#define ATMEL_LCDC_PIXELSIZE_2 (1 << 5) > +#define ATMEL_LCDC_PIXELSIZE_4 (2 << 5) > +#define ATMEL_LCDC_PIXELSIZE_8 (3 << 5) > +#define ATMEL_LCDC_PIXELSIZE_16 (4 << 5) > +#define ATMEL_LCDC_PIXELSIZE_24 (5 << 5) > +#define ATMEL_LCDC_PIXELSIZE_32 (6 << 5) > +#define ATMEL_LCDC_INVVD (1 << 8) > +#define ATMEL_LCDC_INVVD_NORMAL (0 << 8) > +#define ATMEL_LCDC_INVVD_INVERTED (1 << 8) > +#define ATMEL_LCDC_INVFRAME (1 << 9) > +#define ATMEL_LCDC_INVFRAME_NORMAL (0 << 9) > +#define ATMEL_LCDC_INVFRAME_INVERTED (1 << 9) > +#define ATMEL_LCDC_INVLINE (1 << 10) > +#define ATMEL_LCDC_INVLINE_NORMAL (0 << 10) > +#define ATMEL_LCDC_INVLINE_INVERTED (1 << 10) > +#define ATMEL_LCDC_INVCLK (1 << 11) > +#define ATMEL_LCDC_INVCLK_NORMAL (0 << 11) > +#define ATMEL_LCDC_INVCLK_INVERTED (1 << 11) > +#define ATMEL_LCDC_INVDVAL (1 << 12) > +#define ATMEL_LCDC_INVDVAL_NORMAL (0 << 12) > +#define ATMEL_LCDC_INVDVAL_INVERTED (1 << 12) > +#define ATMEL_LCDC_CLKMOD (1 << 15) > +#define ATMEL_LCDC_CLKMOD_ACTIVEDISPLAY (0 << 15) > +#define ATMEL_LCDC_CLKMOD_ALWAYSACTIVE (1 << 15) > +#define ATMEL_LCDC_MEMOR (1 << 31) > +#define ATMEL_LCDC_MEMOR_BIG (0 << 31) > +#define ATMEL_LCDC_MEMOR_LITTLE (1 << 31) > + > +#define ATMEL_LCDC_TIM1 0x0808 > +#define ATMEL_LCDC_VFP_OFFSET 0 > +#define ATMEL_LCDC_VFP (0xffU << 0) > +#define ATMEL_LCDC_VBP_OFFSET 8 > +#define ATMEL_LCDC_VBP (0xffU << ATMEL_LCDC_VBP_OFFSET) > +#define ATMEL_LCDC_VPW_OFFSET 16 > +#define ATMEL_LCDC_VPW (0x3fU << ATMEL_LCDC_VPW_OFFSET) > +#define ATMEL_LCDC_VHDLY_OFFSET 24 > +#define ATMEL_LCDC_VHDLY (0xfU << ATMEL_LCDC_VHDLY_OFFSET) > + > +#define ATMEL_LCDC_TIM2 0x080c > +#define ATMEL_LCDC_HBP_OFFSET 0 > +#define ATMEL_LCDC_HBP (0xffU << 0) > +#define ATMEL_LCDC_HPW_OFFSET 8 > +#define ATMEL_LCDC_HPW (0x3fU << ATMEL_LCDC_HPW_OFFSET) > +#define ATMEL_LCDC_HFP_OFFSET 21 > +#define ATMEL_LCDC_HFP (0x7ffU << ATMEL_LCDC_HFP_OFFSET) > + > +#define ATMEL_LCDC_LCDFRMCFG 0x0810 > +#define ATMEL_LCDC_LINEVAL (0x7ff << 0) > +#define ATMEL_LCDC_HOZVAL_OFFSET 21 > +#define ATMEL_LCDC_HOZVAL (0x7ff << ATMEL_LCDC_HOZVAL_OFFSET) > + > +#define ATMEL_LCDC_FIFO 0x0814 > +#define ATMEL_LCDC_FIFOTH (0xffff) > + > +#define ATMEL_LCDC_MVAL 0x0818 > + > +#define ATMEL_LCDC_DP1_2 0x081c > +#define ATMEL_LCDC_DP4_7 0x0820 > +#define ATMEL_LCDC_DP3_5 0x0824 > +#define ATMEL_LCDC_DP2_3 0x0828 > +#define ATMEL_LCDC_DP5_7 0x082c > +#define ATMEL_LCDC_DP3_4 0x0830 > +#define ATMEL_LCDC_DP4_5 0x0834 > +#define ATMEL_LCDC_DP6_7 0x0838 > +#define ATMEL_LCDC_DP1_2_VAL (0xff) > +#define ATMEL_LCDC_DP4_7_VAL (0xfffffff) > +#define ATMEL_LCDC_DP3_5_VAL (0xfffff) > +#define ATMEL_LCDC_DP2_3_VAL (0xfff) > +#define ATMEL_LCDC_DP5_7_VAL (0xfffffff) > +#define ATMEL_LCDC_DP3_4_VAL (0xffff) > +#define ATMEL_LCDC_DP4_5_VAL (0xfffff) > +#define ATMEL_LCDC_DP6_7_VAL (0xfffffff) > + > +#define ATMEL_LCDC_PWRCON 0x083c > +#define ATMEL_LCDC_PWR (1 << 0) > +#define ATMEL_LCDC_GUARDT_OFFSET 1 > +#define ATMEL_LCDC_GUARDT (0x7f << ATMEL_LCDC_GUARDT_OFFSET) > +#define ATMEL_LCDC_BUSY (1 << 31) > + > +#define ATMEL_LCDC_CONTRAST_CTR 0x0840 > +#define ATMEL_LCDC_PS (3 << 0) > +#define ATMEL_LCDC_PS_DIV1 (0 << 0) > +#define ATMEL_LCDC_PS_DIV2 (1 << 0) > +#define ATMEL_LCDC_PS_DIV4 (2 << 0) > +#define ATMEL_LCDC_PS_DIV8 (3 << 0) > +#define ATMEL_LCDC_POL (1 << 2) > +#define ATMEL_LCDC_POL_NEGATIVE (0 << 2) > +#define ATMEL_LCDC_POL_POSITIVE (1 << 2) > +#define ATMEL_LCDC_ENA (1 << 3) > +#define ATMEL_LCDC_ENA_PWMDISABLE (0 << 3) > +#define ATMEL_LCDC_ENA_PWMENABLE (1 << 3) > + > +#define ATMEL_LCDC_CONTRAST_VAL 0x0844 > +#define ATMEL_LCDC_CVAL (0xff) > + > +#define ATMEL_LCDC_IER 0x0848 > +#define ATMEL_LCDC_IDR 0x084c > +#define ATMEL_LCDC_IMR 0x0850 > +#define ATMEL_LCDC_ISR 0x0854 > +#define ATMEL_LCDC_ICR 0x0858 > +#define ATMEL_LCDC_LNI (1 << 0) > +#define ATMEL_LCDC_LSTLNI (1 << 1) > +#define ATMEL_LCDC_EOFI (1 << 2) > +#define ATMEL_LCDC_UFLWI (1 << 4) > +#define ATMEL_LCDC_OWRI (1 << 5) > +#define ATMEL_LCDC_MERI (1 << 6) > + > +#define ATMEL_LCDC_LUT(n) (0x0c00 + ((n)*4)) > +#define ATMEL_LCDC_LUT_SIZE 256 > + > +#endif /* __LINUX_MFD_LCDC_H */ _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v1 4/7] dt-bindings: add bindings for Atmel LCDC pwm 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg ` (2 preceding siblings ...) 2018-08-12 18:46 ` [PATCH v1 3/7] mfd: add atmel-lcdc driver Sam Ravnborg @ 2018-08-12 18:46 ` Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 5/7] pwm: add pwm-atmel-lcdc driver Sam Ravnborg ` (5 subsequent siblings) 9 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-12 18:46 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, linux-pwm, Boris Brezillon, dri-devel, Thierry Reding, Sam Ravnborg, linux-arm-kernel The LCDC IP used by some Atmel SOC's have a multifunction device that include two sub-devices: - pwm - display controller This binding describe the pwm binding and refer back to the mfd device that this must be a child node of. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Boris Brezillon <boris.brezillon@free-electrons.com> --- .../devicetree/bindings/pwm/atmel-lcdc-pwm.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/atmel-lcdc-pwm.txt diff --git a/Documentation/devicetree/bindings/pwm/atmel-lcdc-pwm.txt b/Documentation/devicetree/bindings/pwm/atmel-lcdc-pwm.txt new file mode 100644 index 000000000000..a7f11ac6972a --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/atmel-lcdc-pwm.txt @@ -0,0 +1,30 @@ +Device-Tree bindings for Atmel's LCDC (LCD Controller) PWM driver + +The Atmel LCDC PWM is subdevice of the LCDC MFD device. +See ../mfd/atmel-lcdc.txt for more details. + +Required properties: + - compatible: value should be one of the following: + "atmel,lcdc-pwm" + - pinctr-names: the pin control state names. Should contain "default". + - pinctrl-0: should contain the pinctrl states described by pinctrl + default. + - #pwm-cells: should be set to 3. This PWM chip use the default 3 cells + bindings defined in pwm.txt in this directory. + +Example: + lcdc0: lcdc@700000 { + compatible = "atmel,at91sam9263-lcdc-mfd"; + reg = <0x700000 0x1000>; + interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; + clocks = <&lcd_clk>, <&lcd_clk>; + clock-names = "lcdc_clk", "hclk"; + + lcdc_pwm: lcdc-pwm { + compatible = "atmel,lcdc-pwm"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_lcdc_pwm>; + #pwm-cells = <3>; + }; + + }; -- 2.12.0 ^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH v1 5/7] pwm: add pwm-atmel-lcdc driver 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg ` (3 preceding siblings ...) 2018-08-12 18:46 ` [PATCH v1 4/7] dt-bindings: add bindings for Atmel LCDC pwm Sam Ravnborg @ 2018-08-12 18:46 ` Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 6/7] dt-bindings: add bindings for Atmel lcdc-display-controller Sam Ravnborg ` (4 subsequent siblings) 9 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-12 18:46 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, linux-pwm, Boris Brezillon, dri-devel, Thierry Reding, Sam Ravnborg, linux-arm-kernel The LCDC IP used by some Atmel SOC's have a multifunction device that include two sub-devices: - pwm - display controller This driver add support for the pwm sub-device exposing a single PWM device that can be used for backlight. The driver is based on the pwm-atmel-hlcdc driver. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Boris Brezillon <boris.brezillon@free-electrons.com> --- drivers/pwm/Kconfig | 13 ++++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-atmel-lcdc.c | 178 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 192 insertions(+) create mode 100644 drivers/pwm/pwm-atmel-lcdc.c diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index a4d262db9945..3976c1840778 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -62,6 +62,19 @@ config PWM_ATMEL_HLCDC_PWM To compile this driver as a module, choose M here: the module will be called pwm-atmel-hlcdc. +config PWM_ATMEL_LCDC_PWM + tristate "Atmel LCDC PWM support" + depends on MFD_ATMEL_LCDC + depends on HAVE_CLK + help + Generic PWM framework driver for the PWM output of the LCDC + (Atmel LCD Controller). This PWM output is mainly used + to control the LCD backlight. + The Atmel LCD Controller is found in the at91sam9x family. + + To compile this driver as a module, choose M here: the module + will be called pwm-atmel-lcdc. + config PWM_ATMEL_TCB tristate "Atmel TC Block PWM support" depends on ATMEL_TCLIB && OF diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 9c676a0dadf5..85d884cb0977 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_PWM_SYSFS) += sysfs.o obj-$(CONFIG_PWM_AB8500) += pwm-ab8500.o obj-$(CONFIG_PWM_ATMEL) += pwm-atmel.o obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM) += pwm-atmel-hlcdc.o +obj-$(CONFIG_PWM_ATMEL_LCDC_PWM) += pwm-atmel-lcdc.o obj-$(CONFIG_PWM_ATMEL_TCB) += pwm-atmel-tcb.o obj-$(CONFIG_PWM_BCM_IPROC) += pwm-bcm-iproc.o obj-$(CONFIG_PWM_BCM_KONA) += pwm-bcm-kona.o diff --git a/drivers/pwm/pwm-atmel-lcdc.c b/drivers/pwm/pwm-atmel-lcdc.c new file mode 100644 index 000000000000..3d528e75cf9f --- /dev/null +++ b/drivers/pwm/pwm-atmel-lcdc.c @@ -0,0 +1,178 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Sam Ravnborg + * + * Author: Sam Ravnborg <sam@ravnborg.org> + * + * PWM embedded in the LCD Controller. + * A sub-device of the Atmel LCDC driver. + * + * Based on pwm-atmel-hlcdc which is: + * Copyright (C) 2014 Free Electrons + * Copyright (C) 2014 Atmel + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> + */ + +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/clk.h> +#include <linux/pwm.h> + +#include <linux/platform_device.h> +#include <linux/mfd/atmel-lcdc.h> + +struct lcdc_pwm { + struct pwm_chip chip; + struct atmel_mfd_lcdc *mfd_lcdc; +}; + +static inline struct lcdc_pwm *to_lcdc_pwm(struct pwm_chip *chip) +{ + return container_of(chip, struct lcdc_pwm, chip); +} + +static int lcdc_pwm_apply(struct pwm_chip *pwm_chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct lcdc_pwm *chip; + int contrast_ctr; + int contrast_val; + int ret; + + chip = to_lcdc_pwm(pwm_chip); + + if (state->enabled) { + contrast_val = pwm_get_relative_duty_cycle(state, + ATMEL_LCDC_CVAL); + ret = regmap_write(chip->mfd_lcdc->regmap, + ATMEL_LCDC_CONTRAST_VAL, contrast_val); + if (ret) + return ret; + + contrast_ctr = ATMEL_LCDC_ENA_PWMENABLE | ATMEL_LCDC_PS_DIV8; + if (state->polarity == PWM_POLARITY_NORMAL) + contrast_ctr |= ATMEL_LCDC_POL_POSITIVE; + else + contrast_ctr |= ATMEL_LCDC_POL_NEGATIVE; + + ret = regmap_write(chip->mfd_lcdc->regmap, + ATMEL_LCDC_CONTRAST_CTR, contrast_ctr); + } else { + contrast_ctr = ATMEL_LCDC_ENA_PWMDISABLE; + ret = regmap_write(chip->mfd_lcdc->regmap, + ATMEL_LCDC_CONTRAST_CTR, contrast_ctr); + } + + return ret; +} + +static const struct pwm_ops lcdc_pwm_ops = { + .apply = lcdc_pwm_apply, + .owner = THIS_MODULE, +}; + +#ifdef CONFIG_PM_SLEEP +static int lcdc_pwm_suspend(struct device *dev) +{ + struct lcdc_pwm *chip = dev_get_drvdata(dev); + + /* Keep the lcdc clock enabled if the PWM is still running. */ + if (pwm_is_enabled(&chip->chip.pwms[0])) + clk_disable_unprepare(chip->mfd_lcdc->lcdc_clk); + + return 0; +} + +static int lcdc_pwm_resume(struct device *dev) +{ + struct lcdc_pwm *chip = dev_get_drvdata(dev); + struct pwm_state state; + int ret; + + pwm_get_state(&chip->chip.pwms[0], &state); + + /* Re-enable the lcdc clock if it was stopped during suspend. */ + if (!state.enabled) { + ret = clk_prepare_enable(chip->mfd_lcdc->lcdc_clk); + if (ret) + return ret; + } + + return lcdc_pwm_apply(&chip->chip, &chip->chip.pwms[0], &state); +} +#endif + +static SIMPLE_DEV_PM_OPS(lcdc_pwm_pm_ops, + lcdc_pwm_suspend, lcdc_pwm_resume); + +static int lcdc_pwm_probe(struct platform_device *pdev) +{ + struct atmel_mfd_lcdc *mfd_lcdc; + struct lcdc_pwm *chip; + struct device *dev; + int ret; + + dev = &pdev->dev; + mfd_lcdc = dev_get_drvdata(dev->parent); + + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL); + if (!chip) + return -ENOMEM; + + ret = clk_prepare_enable(mfd_lcdc->lcdc_clk); + if (ret) + return ret; + + chip->mfd_lcdc = mfd_lcdc; + chip->chip.ops = &lcdc_pwm_ops; + chip->chip.dev = dev; + chip->chip.base = -1; + chip->chip.npwm = 1; + chip->chip.of_xlate = of_pwm_xlate_with_flags; + chip->chip.of_pwm_n_cells = 3; + + ret = pwmchip_add_with_polarity(&chip->chip, PWM_POLARITY_INVERSED); + if (ret) { + clk_disable_unprepare(mfd_lcdc->lcdc_clk); + return ret; + } + + platform_set_drvdata(pdev, chip); + + return 0; +} + +static int lcdc_pwm_remove(struct platform_device *pdev) +{ + struct lcdc_pwm *chip = platform_get_drvdata(pdev); + int ret; + + ret = pwmchip_remove(&chip->chip); + if (ret) + return ret; + + clk_disable_unprepare(chip->mfd_lcdc->lcdc_clk); + + return 0; +} + +static const struct of_device_id lcdc_pwm_dt_ids[] = { + { .compatible = "atmel,lcdc-pwm" }, + { /* sentinel */ }, +}; + +static struct platform_driver lcdc_pwm_driver = { + .driver = { + .name = "atmel-lcdc-pwm", + .of_match_table = lcdc_pwm_dt_ids, + .pm = &lcdc_pwm_pm_ops, + }, + .probe = lcdc_pwm_probe, + .remove = lcdc_pwm_remove, +}; +module_platform_driver(lcdc_pwm_driver); + +MODULE_ALIAS("platform:pwm-atmel-lcdc"); +MODULE_AUTHOR("Sam Ravnborg <sam@ravnborg.org>"); +MODULE_DESCRIPTION("Atmel LCDC PWM driver"); +MODULE_LICENSE("GPL v2"); -- 2.12.0 ^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH v1 6/7] dt-bindings: add bindings for Atmel lcdc-display-controller 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg ` (4 preceding siblings ...) 2018-08-12 18:46 ` [PATCH v1 5/7] pwm: add pwm-atmel-lcdc driver Sam Ravnborg @ 2018-08-12 18:46 ` Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 7/7] drm: add Atmel LCDC display controller support Sam Ravnborg ` (3 subsequent siblings) 9 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-12 18:46 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, Boris Brezillon, dri-devel, linux-pwm, Sam Ravnborg, linux-arm-kernel The LCDC IP used by some Atmel SOC's have a multifunction device that include two sub-devices: - pwm - display controller This binding describe the lcdc display controller and refer back to the mfd device that this must be a child node of. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Boris Brezillon <boris.brezillon@free-electrons.com> --- .../display/atmel/lcdc-display-controller.txt | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/atmel/lcdc-display-controller.txt diff --git a/Documentation/devicetree/bindings/display/atmel/lcdc-display-controller.txt b/Documentation/devicetree/bindings/display/atmel/lcdc-display-controller.txt new file mode 100644 index 000000000000..508a49b5d8b7 --- /dev/null +++ b/Documentation/devicetree/bindings/display/atmel/lcdc-display-controller.txt @@ -0,0 +1,40 @@ +Atmel LCD Controller (LCDC) Display Controller + +Required properties: + - compatible: value should be one of the following: + "atmel,lcdc-display-controller" + +Optional properties: +- lcd-supply: phandle to a regulator that supply the display + +Required children nodes: + One child node that references the panel connected. + (See ../graph.txt) + At least one port node is required. + + +Example: + lcdc0: lcdc@700000 { + compatible = "atmel,at91sam9263-lcdc-mfd"; + reg = <0x700000 0x1000>; + interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; + clocks = <&lcd_clk>, <&lcd_clk>; + clock-names = "lcdc_clk", "hclk"; + + lcdc-display-controller { + compatible = "atmel,lcdc-display-controller"; + lcd-supply = <&lcdc_reg>; + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + lcdc_panel_output: endpoint@0 { + reg = <0>; + remote-endpoint = <&panel_input>; + }; + }; + }; + }; -- 2.12.0 ^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH v1 7/7] drm: add Atmel LCDC display controller support 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg ` (5 preceding siblings ...) 2018-08-12 18:46 ` [PATCH v1 6/7] dt-bindings: add bindings for Atmel lcdc-display-controller Sam Ravnborg @ 2018-08-12 18:46 ` Sam Ravnborg 2018-08-24 12:31 ` Boris Brezillon 2018-08-26 14:28 ` Noralf Trønnes 2018-08-12 19:55 ` [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg ` (2 subsequent siblings) 9 siblings, 2 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-12 18:46 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, linux-pwm, Boris Brezillon, Nicolas Ferre, dri-devel, Sam Ravnborg, linux-arm-kernel This is a DRM based driver for the Atmel LCDC IP. There exist today a framebuffer based driver and this is a re-implmentation of the same on top of DRM. The rewrite was based on the original fbdev driver but the driver has also seen inspiration from the atmel-hlcdc_dc driver and others. The driver is not a full replacement: - STN displays are not supported Binding support is missing but most of the STN specific functionality is otherwise ported from the fbdev driver. - gamma support is missing The driver utilises drm_simple_kms_helper and this helper lacks support for settting up gamma - modesetting is not checked (see TODO in file) - support for extra modes as applicable (and lcd-wiring-mode) - support for AVR32 (is it relevant?) Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Nicolas Ferre <nicolas.ferre@atmel.com> Cc: Boris Brezillon <boris.brezillon@free-electrons.com> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> --- MAINTAINERS | 7 + drivers/gpu/drm/atmel/Kconfig | 12 + drivers/gpu/drm/atmel/Makefile | 2 + drivers/gpu/drm/atmel/atmel_lcdc-dc.c | 1094 +++++++++++++++++++++++++++++++++ 4 files changed, 1115 insertions(+) create mode 100644 drivers/gpu/drm/atmel/atmel_lcdc-dc.c diff --git a/MAINTAINERS b/MAINTAINERS index 09ce76a9a1dc..0a594d02a7c0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4685,6 +4685,13 @@ F: drivers/gpu/drm/atmel/atmel-hlcdc* F: Documentation/devicetree/bindings/display/atmel/ T: git git://anongit.freedesktop.org/drm/drm-misc +DRM DRIVERS FOR ATMEL LCDC +M: Sam Ravnborg <sam@ravnborg.org> +L: dri-devel@lists.freedesktop.org +S: Maintained +F: drivers/gpu/drm/atmel/atmel-lcdc* +F: Documentation/devicetree/bindings/display/atmel/ + DRM DRIVERS FOR BRIDGE CHIPS M: Archit Taneja <architt@codeaurora.org> M: Andrzej Hajda <a.hajda@samsung.com> diff --git a/drivers/gpu/drm/atmel/Kconfig b/drivers/gpu/drm/atmel/Kconfig index 7cd3862f9d18..c4c6150f271a 100644 --- a/drivers/gpu/drm/atmel/Kconfig +++ b/drivers/gpu/drm/atmel/Kconfig @@ -14,3 +14,15 @@ config DRM_ATMEL_HLCDC help Choose this option if you have an ATMEL SoC with an HLCDC display controller (i.e. at91sam9n12, at91sam9x5 family or sama5d3 family). + +config DRM_ATMEL_LCDC + tristate "DRM Support for ATMEL LCDC Display Controller" + depends on DRM && OF && COMMON_CLK && MFD_ATMEL_LCDC && ARM + select DRM_ATMEL + select DRM_GEM_CMA_HELPER + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DRM_PANEL + help + Choose this option if you have an ATMEL SoC with an LCDC display + controller (only at91sam9263 or at91sam9rl). diff --git a/drivers/gpu/drm/atmel/Makefile b/drivers/gpu/drm/atmel/Makefile index 49dc89f36b73..9fdfada613d2 100644 --- a/drivers/gpu/drm/atmel/Makefile +++ b/drivers/gpu/drm/atmel/Makefile @@ -5,3 +5,5 @@ atmel-hlcdc-dc-y := atmel_hlcdc_crtc.o \ atmel_hlcdc_plane.o obj-$(CONFIG_DRM_ATMEL_HLCDC) += atmel-hlcdc-dc.o + +obj-$(CONFIG_DRM_ATMEL_LCDC) += atmel_lcdc-dc.o diff --git a/drivers/gpu/drm/atmel/atmel_lcdc-dc.c b/drivers/gpu/drm/atmel/atmel_lcdc-dc.c new file mode 100644 index 000000000000..275a09e2e100 --- /dev/null +++ b/drivers/gpu/drm/atmel/atmel_lcdc-dc.c @@ -0,0 +1,1094 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Sam Ravnborg + * + * The driver is based on atmel_lcdfb which is: + * Copyright (C) 2007 Atmel Corporation + * + * Atmel LCD Controller Display Controller. + * A sub-device of the Atmel LCDC IP. + * + * The Atmel LCD Controller supports in the following configuration: + * - TFT only, with BGR565, 8 bits/pixel + * - Resolution up to 2048x2048 + * - Single plane, crtc, one fixed output + * + * Features not (yet) ported from atmel_lcdfb: + * - Support for extra modes (and configurable intensify bit) + * - Check modesetting support - lcdc_dc_display_check() + * - set color / palette handling + * - support for STN displays (partly implemented) + * - AVR32 support (relevant?) + */ + +#include <linux/regulator/consumer.h> +#include <linux/mfd/atmel-lcdc.h> +#include <linux/of_platform.h> +#include <linux/pm_runtime.h> +#include <linux/irqreturn.h> +#include <linux/device.h> +#include <linux/regmap.h> + +#include <video/videomode.h> + +#include <drm/drm_gem_framebuffer_helper.h> +#include <drm/drm_simple_kms_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_fourcc.h> +#include <drm/drm_vblank.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h> +#include <drm/drm_print.h> +#include <drm/drm_drv.h> +#include <drm/drm_of.h> + +/* Parameters */ +#define ATMEL_LCDC_DMA_BURST_LEN 8 /* words */ + +/** + * struct atmel_lcdc_dc_desc - CPU specific configuration properties + */ +struct atmel_lcdc_dc_desc { + int guard_time; + int fifo_size; + int min_width; + int min_height; + int max_width; + int max_height; + bool have_hozval; + bool have_alt_pixclock; +}; + +/* private data */ +struct lcdc_dc { + const struct atmel_lcdc_dc_desc *desc; + struct atmel_mfd_lcdc *mfd_lcdc; + struct regulator *lcd_supply; + struct drm_fbdev_cma *fbdev; + struct drm_bridge *bridge; + struct drm_panel *panel; + struct regmap *regmap; + struct device *dev; + + struct drm_simple_display_pipe pipe; + struct work_struct reset_lcdc_work; + struct drm_connector connector; +}; + +/* Configuration of individual CPU's */ +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9261 = { + .guard_time = 1, + .fifo_size = 512, + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .have_hozval = true, + .have_alt_pixclock = false, +}; + +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9263 = { + .guard_time = 1, + .fifo_size = 2048, + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .have_hozval = false, + .have_alt_pixclock = false, +}; + +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9g10 = { + .guard_time = 1, + .fifo_size = 512, + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .have_hozval = true, + .have_alt_pixclock = false, +}; + +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9g45 = { + .guard_time = 1, + .fifo_size = 512, + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .have_hozval = false, + .have_alt_pixclock = true, +}; + +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9g46 = { + .guard_time = 1, + .fifo_size = 512, + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .have_hozval = false, + .have_alt_pixclock = false, +}; + +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9m10 = { + .guard_time = 1, + .fifo_size = 512, + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .have_hozval = false, + .have_alt_pixclock = false, +}; + +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9m11 = { + .guard_time = 1, + .fifo_size = 512, + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .have_hozval = false, + .have_alt_pixclock = false, +}; + +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9rl = { + .guard_time = 1, + .fifo_size = 512, + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .have_hozval = false, + .have_alt_pixclock = false, +}; + +static const struct of_device_id atmel_lcdc_of_match[] = { + { + .compatible = "atmel,at91sam9261-lcdc-mfd", + .data = &atmel_lcdc_dc_at91sam9261, + }, { + .compatible = "atmel,at91sam9263-lcdc-mfd", + .data = &atmel_lcdc_dc_at91sam9263, + }, { + .compatible = "atmel,at91sam9g10-lcdc-mfd", + .data = &atmel_lcdc_dc_at91sam9g10, + }, { + .compatible = "atmel,at91sam9g45-lcdc-mfd", + .data = &atmel_lcdc_dc_at91sam9g45, + }, { + .compatible = "atmel,at91sam9g46-lcdc-mfd", + .data = &atmel_lcdc_dc_at91sam9g46, + }, { + .compatible = "atmel,at91sam9m10-lcdc-mfd", + .data = &atmel_lcdc_dc_at91sam9m10, + }, { + .compatible = "atmel,at91sam9m11-lcdc-mfd", + .data = &atmel_lcdc_dc_at91sam9m11, + }, { + .compatible = "atmel,at91sam9rl-lcdc-mfd", + .data = &atmel_lcdc_dc_at91sam9rl, + }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, atmel_lcdc_of_match); + +/* + * The Atmel LCD controller display-controller supports several formats but + * this driver supports only a small subset. + * TODO: atmel_lcdfb supports more - port it over + * Maybe actual wiring will impact mode support? + */ +static const u32 lcdc_dc_formats[] = { + DRM_FORMAT_BGR565, +}; + +/* Start LCD Controller (DMA + PWR) */ +static void lcdc_dc_start(struct lcdc_dc *lcdc_dc) +{ + // Enable DMA + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_DMACON, ATMEL_LCDC_DMAEN); + // Enable LCD + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_PWRCON, + (lcdc_dc->desc->guard_time << ATMEL_LCDC_GUARDT_OFFSET) + | ATMEL_LCDC_PWR); +} + +/* Stop LCD Controller (PWR + DMA) */ +static void lcdc_dc_stop(struct lcdc_dc *lcdc_dc) +{ + unsigned int pwrcon; + + might_sleep(); + + /* Turn off the LCD controller and the DMA controller */ + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_PWRCON, + lcdc_dc->desc->guard_time << ATMEL_LCDC_GUARDT_OFFSET); + + /* Wait for the LCDC core to become idle */ + regmap_read_poll_timeout(lcdc_dc->regmap, ATMEL_LCDC_PWRCON, pwrcon, + !(pwrcon & ATMEL_LCDC_BUSY), 100, 10000); + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_DMACON, !(ATMEL_LCDC_DMAEN)); +} + +static void lcdc_dc_start_clock(struct lcdc_dc *lcdc_dc) +{ + clk_prepare_enable(lcdc_dc->mfd_lcdc->bus_clk); + clk_prepare_enable(lcdc_dc->mfd_lcdc->lcdc_clk); +} + +static void lcdc_dc_stop_clock(struct lcdc_dc *lcdc_dc) +{ + clk_disable_unprepare(lcdc_dc->mfd_lcdc->bus_clk); + clk_disable_unprepare(lcdc_dc->mfd_lcdc->lcdc_clk); +} + +static int lcdc_dc_display_check(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *pstate, + struct drm_crtc_state *cstate) +{ + const struct drm_display_mode *dmode; + struct drm_framebuffer *old_fb; + struct drm_framebuffer *fb; + + dmode = &cstate->mode; + old_fb = pipe->plane.state->fb; + fb = pstate->fb; + + /* Check timing? */ + /* TODO */ + + return 0; +} + +/* Horizontal size of LCD module - configuration dependent */ +static unsigned int compute_hozval(struct lcdc_dc *lcdc_dc, unsigned int width) +{ + unsigned int valid_lcdd_data_line; + unsigned int hoz_display_size; + unsigned int disptype; + unsigned int scanmode; + unsigned int ifwidth; + unsigned int lcdcon2; + + if (!lcdc_dc->desc->have_hozval) + return width; + + regmap_read(lcdc_dc->regmap, ATMEL_LCDC_LCDCON2, &lcdcon2); + disptype = lcdcon2 & ATMEL_LCDC_DISTYPE; + + if (disptype == ATMEL_LCDC_DISTYPE_TFT) + return width; + + ifwidth = lcdcon2 & ATMEL_LCDC_IFWIDTH; + scanmode = lcdcon2 & ATMEL_LCDC_SCANMOD; + + /* + * STN display + * Based on algorithm from datasheet calculate hozval + */ + if (disptype == ATMEL_LCDC_DISTYPE_STNCOLOR) + hoz_display_size = width * 3; + else + hoz_display_size = width; + + switch (ifwidth) { + case ATMEL_LCDC_IFWIDTH_4: + valid_lcdd_data_line = 4; + break; + case ATMEL_LCDC_IFWIDTH_8: + if (scanmode == ATMEL_LCDC_SCANMOD_DUAL) + valid_lcdd_data_line = 4; + else + valid_lcdd_data_line = 8; + break; + default: + valid_lcdd_data_line = 8; + break; + } + + return DIV_ROUND_UP(hoz_display_size, valid_lcdd_data_line); +} + +static void set_vertical_timing(struct lcdc_dc *lcdc_dc, + struct drm_display_mode *dmode) +{ + unsigned int tim1; + unsigned int vfp; + unsigned int vbp; + unsigned int vpw; + unsigned int vhdly; + + /* VFP: Vertical Front Porch */ + vfp = dmode->vsync_start - dmode->vdisplay; + + /* VBP: Vertical Back Porch */ + vbp = dmode->vtotal - dmode->vsync_end; + + /* VPW: Vertical Synchronization pulse width */ + vpw = dmode->vsync_end - dmode->vsync_start - 1; + + /* VHDLY: Vertical to horizontal delay */ + vhdly = 0; + + tim1 = vfp << ATMEL_LCDC_VFP_OFFSET | + vbp << ATMEL_LCDC_VBP_OFFSET | + vpw << ATMEL_LCDC_VPW_OFFSET | + vhdly << ATMEL_LCDC_VHDLY_OFFSET; + + DRM_DEV_DEBUG(lcdc_dc->dev, " TIM1 = %08x\n", tim1); + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_TIM1, tim1); +} + +static void set_horizontal_timing(struct lcdc_dc *lcdc_dc, + struct drm_display_mode *dmode) +{ + unsigned int tim2; + unsigned int hbp; + unsigned int hpw; + unsigned int hfp; + + /* HBP: Horizontal Back Porch */ + hbp = dmode->htotal - dmode->hsync_end - 1; + + /* HPW: Horizontal synchronization pulse width */ + hpw = dmode->hsync_end - dmode->hsync_start - 1; + + /* HFP: Horizontal Front Porch */ + hfp = dmode->hsync_start - dmode->hdisplay - 2; + + tim2 = hbp << ATMEL_LCDC_HBP_OFFSET | + hpw << ATMEL_LCDC_HPW_OFFSET | + hfp << ATMEL_LCDC_HFP_OFFSET; + + DRM_DEV_DEBUG(lcdc_dc->dev, " TIM2 = %08x\n", tim2); + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_TIM2, tim2); +} + +static void lcdc_dc_crtc_mode_set_nofb(struct lcdc_dc *lcdc_dc) +{ + struct drm_display_mode *dmode; + unsigned int hozval_linesz; + unsigned int value; + + dmode = &lcdc_dc->pipe.crtc.state->adjusted_mode; + + /* Vertical & horizontal timing */ + set_vertical_timing(lcdc_dc, dmode); + set_horizontal_timing(lcdc_dc, dmode); + + /* Horizontal value (aka line size) */ + hozval_linesz = compute_hozval(lcdc_dc, dmode->crtc_hdisplay); + + /* Display size */ + value = (hozval_linesz - 1) << ATMEL_LCDC_HOZVAL_OFFSET; + value |= dmode->crtc_vdisplay - 1; + DRM_DEV_DEBUG(lcdc_dc->dev, " LCDFRMCFG = %08x\n", value); + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_LCDFRMCFG, value); + + /* FIFO Threshold: Use formula from data sheet */ + value = lcdc_dc->desc->fifo_size - (2 * ATMEL_LCDC_DMA_BURST_LEN + 3); + DRM_DEV_DEBUG(lcdc_dc->dev, " FIFO = %08x\n", value); + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_FIFO, value); + + /* + * Toggle LCD_MODE every frame + * Note: register not documented, this is from atmel_lcdfb + */ + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_MVAL, 0); +} + +static void lcdc_dc_enable(struct lcdc_dc *lcdc_dc, + struct drm_device *drm, + struct drm_crtc *crtc) +{ + const struct drm_format_info *format; + struct drm_display_mode *dmode; + unsigned long clk_value_khz; + unsigned int pix_factor; + unsigned int lcdcon1; + unsigned int lcdcon2; + u32 bus_flags; + + dmode = &lcdc_dc->pipe.crtc.state->adjusted_mode; + format = crtc->primary->state->fb->format; + + /* Control register 1 */ + + /* Set pixel clock */ + if (lcdc_dc->desc->have_alt_pixclock) + pix_factor = 1; + else + pix_factor = 2; + + clk_value_khz = clk_get_rate(lcdc_dc->mfd_lcdc->lcdc_clk) / 1000; + lcdcon1 = DIV_ROUND_UP(clk_value_khz, dmode->clock); + + if (lcdcon1 < pix_factor) { + DRM_DEV_INFO(lcdc_dc->dev, "Bypassing pixel clock divider\n"); + regmap_write(lcdc_dc->regmap, + ATMEL_LCDC_LCDCON1, ATMEL_LCDC_BYPASS); + } else { + + lcdcon1 = (lcdcon1 / pix_factor) - 1; + DRM_DEV_DEBUG(lcdc_dc->dev, "CLKVAL = 0x%08x\n", lcdcon1); + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_LCDCON1, + lcdcon1 << ATMEL_LCDC_CLKVAL_OFFSET); + dmode->clock = clk_value_khz / (pix_factor * (lcdcon1 + 1)); + DRM_DEV_DEBUG(lcdc_dc->dev, "updated pixclk: %u KHz\n", + dmode->clock); + } + + /* Control register 2 */ + /* Only TFT supported (Controller supports STN too) */ + lcdcon2 = ATMEL_LCDC_DISTYPE_TFT; + + /* scan mode (STN only) */ + if (dmode->flags & DRM_MODE_FLAG_DBLSCAN) + lcdcon2 |= ATMEL_LCDC_SCANMOD_DUAL; + else + lcdcon2 |= ATMEL_LCDC_SCANMOD_SINGLE; + + /* Interface width 4 bits (STN only) */ + lcdcon2 |= ATMEL_LCDC_IFWIDTH_4; + + /* bits per pixel */ + switch (format->depth) { + case 1: + lcdcon2 |= ATMEL_LCDC_PIXELSIZE_1; + break; + case 2: + lcdcon2 |= ATMEL_LCDC_PIXELSIZE_2; + break; + case 4: + lcdcon2 |= ATMEL_LCDC_PIXELSIZE_4; + break; + case 8: + lcdcon2 |= ATMEL_LCDC_PIXELSIZE_8; + break; + case 15: + /* fall through */ + case 16: + lcdcon2 |= ATMEL_LCDC_PIXELSIZE_16; + break; + case 24: + lcdcon2 |= ATMEL_LCDC_PIXELSIZE_24; + break; + case 32: + lcdcon2 |= ATMEL_LCDC_PIXELSIZE_32; + break; + default: + DRM_DEV_ERROR(lcdc_dc->dev, "Unexpected depth (%d)", + format->depth); + break; + } + + /* Polarity normal */ + lcdcon2 |= ATMEL_LCDC_INVVD_NORMAL; + + /* vsync polarity */ + if (dmode->flags & DRM_MODE_FLAG_PVSYNC) + lcdcon2 |= ATMEL_LCDC_INVFRAME_INVERTED; + else + lcdcon2 |= ATMEL_LCDC_INVFRAME_NORMAL; + + /* hsync polarity */ + if (dmode->flags & DRM_MODE_FLAG_PHSYNC) + lcdcon2 |= ATMEL_LCDC_INVLINE_INVERTED; + else + lcdcon2 |= ATMEL_LCDC_INVLINE_NORMAL; + + bus_flags = lcdc_dc->connector.display_info.bus_flags; + + /* dot clock (pix clock) polarity */ + if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) + lcdcon2 |= ATMEL_LCDC_INVCLK_INVERTED; + else + lcdcon2 |= ATMEL_LCDC_INVCLK_NORMAL; + + /* Date Enable polarity */ + if (bus_flags & DRM_BUS_FLAG_DE_LOW) + lcdcon2 |= ATMEL_LCDC_INVDVAL_INVERTED; + else + lcdcon2 |= ATMEL_LCDC_INVDVAL_NORMAL; + + /* Clock is always active */ + lcdcon2 |= ATMEL_LCDC_CLKMOD_ALWAYSACTIVE; + + /* Memory layout */ + if (format->format & DRM_FORMAT_BIG_ENDIAN) + lcdcon2 |= ATMEL_LCDC_MEMOR_BIG; + else + lcdcon2 |= ATMEL_LCDC_MEMOR_LITTLE; + + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_LCDCON2, lcdcon2); + + lcdc_dc_start(lcdc_dc); + + drm_crtc_vblank_on(crtc); +} + +static void lcdc_dc_display_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *cstate, + struct drm_plane_state *plane_state) +{ + struct lcdc_dc *lcdc_dc; + struct drm_device *drm; + struct drm_crtc *crtc; + int ret; + + crtc = &pipe->crtc; + drm = crtc->dev; + lcdc_dc = drm->dev_private; + + ret = regulator_enable(lcdc_dc->lcd_supply); + if (ret) + DRM_DEV_ERROR(lcdc_dc->dev, "regulator_enable failed (%d)", + ret); + + drm_panel_prepare(lcdc_dc->panel); + lcdc_dc_crtc_mode_set_nofb(lcdc_dc); + + /* + * drm_simple_kms_helper have no support for gamma setup. + * Add: lcdc_dc_setcolor(lcdc_dc, crtc); when we have it + */ + lcdc_dc_enable(lcdc_dc, drm, crtc); + + drm_panel_enable(lcdc_dc->panel); +} + +static void lcdc_dc_display_disable(struct drm_simple_display_pipe *pipe) +{ + struct lcdc_dc *lcdc_dc; + struct drm_device *drm; + struct drm_crtc *crtc; + + crtc = &pipe->crtc; + drm = crtc->dev; + lcdc_dc = drm->dev_private; + + drm_crtc_vblank_off(crtc); + + drm_panel_disable(lcdc_dc->panel); + + lcdc_dc_stop(lcdc_dc); + + drm_panel_unprepare(lcdc_dc->panel); + regulator_disable(lcdc_dc->lcd_supply); +} + +/* Update DMA config */ +static void lcdc_dc_update_dma(struct lcdc_dc *lcdc_dc, + struct drm_simple_display_pipe *pipe) +{ + struct drm_plane_state *plane_state; + struct drm_framebuffer *fb; + unsigned int burst_length; + unsigned int frame_size; + dma_addr_t dma_addr; + + plane_state = pipe->plane.state; + fb = plane_state->fb; + + if (fb) { + unsigned int dmafrmcfg; + + dma_addr = drm_fb_cma_get_gem_addr(fb, pipe->plane.state, 0); + + /* Set frame buffer DMA base address */ + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_DMABADDR1, dma_addr); + + /* + * Set frame size and burst length + * Frame_size equals size of visible area * bits / 32 + * (size in 32 bit words) + */ + frame_size = plane_state->crtc_w * plane_state->crtc_h; + frame_size = (frame_size * fb->format->depth) / 32; + + burst_length = ATMEL_LCDC_DMA_BURST_LEN - 1; + + dmafrmcfg = frame_size; + dmafrmcfg |= burst_length << ATMEL_LCDC_BLENGTH_OFFSET; + + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_DMAFRMCFG, dmafrmcfg); + } +} + +static void lcdc_dc_update_event(struct drm_simple_display_pipe *pipe) +{ + struct drm_pending_vblank_event *event; + struct drm_crtc *crtc; + + crtc = &pipe->crtc; + if (!crtc) + return; + + spin_lock_irq(&crtc->dev->event_lock); + event = crtc->state->event; + if (event) { + crtc->state->event = NULL; + + if (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); +} + +static void lcdc_dc_display_update(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *old_pstate) +{ + struct drm_device *drm; + struct lcdc_dc *lcdc_dc; + + drm = pipe->crtc.dev; + lcdc_dc = drm->dev_private; + + /* Re-initialize the DMA engine... */ + lcdc_dc_update_dma(lcdc_dc, pipe); + + /* vblank event handling */ + lcdc_dc_update_event(pipe); +} + +static int lcdc_dc_display_enable_vblank(struct drm_simple_display_pipe *pipe) +{ + struct drm_crtc *crtc; + struct lcdc_dc *lcdc_dc; + + crtc = &pipe->crtc; + lcdc_dc = crtc->dev->dev_private; + + /* Last line interrupt enable */ + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_IER, ATMEL_LCDC_LSTLNI); + + return 0; +} + +static void lcdc_dc_display_disable_vblank(struct drm_simple_display_pipe *pipe) +{ + struct drm_crtc *crtc; + struct lcdc_dc *lcdc_dc; + + crtc = &pipe->crtc; + lcdc_dc = crtc->dev->dev_private; + + /* Last line interrupt disable */ + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_IDR, ATMEL_LCDC_LSTLNI); +} + +static const struct drm_simple_display_pipe_funcs lcdc_dc_display_funcs = { + .check = lcdc_dc_display_check, + .enable = lcdc_dc_display_enable, + .disable = lcdc_dc_display_disable, + .update = lcdc_dc_display_update, + .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, + .enable_vblank = lcdc_dc_display_enable_vblank, + .disable_vblank = lcdc_dc_display_disable_vblank, +}; + +static const struct drm_mode_config_funcs mode_config_funcs = { + .fb_create = drm_gem_fb_create, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + +/* scheduled worker to reset LCD */ +static void reset_lcdc_work(struct work_struct *work) +{ + struct lcdc_dc *lcdc_dc; + + lcdc_dc = container_of(work, struct lcdc_dc, reset_lcdc_work); + + lcdc_dc_stop(lcdc_dc); + lcdc_dc_start(lcdc_dc); +} + +static irqreturn_t lcdc_dc_irq_handler(int irq, void *arg) +{ + struct lcdc_dc *lcdc_dc; + struct drm_device *drm; + unsigned int status; + struct device *dev; + unsigned int imr; + unsigned int isr; + + drm = arg; + lcdc_dc = drm->dev_private; + dev = lcdc_dc->dev; + + regmap_read(lcdc_dc->regmap, ATMEL_LCDC_IMR, &imr); + regmap_read(lcdc_dc->regmap, ATMEL_LCDC_ISR, &isr); + status = imr & isr; + if (!status) + return IRQ_NONE; + + if (status & ATMEL_LCDC_LSTLNI) + drm_crtc_handle_vblank(&lcdc_dc->pipe.crtc); + + if (status & ATMEL_LCDC_UFLWI) { + DRM_DEV_INFO(dev, "FIFO underflow %#x\n", status); + /* reset DMA and FIFO to avoid screen shifting */ + schedule_work(&lcdc_dc->reset_lcdc_work); + } + if (status & ATMEL_LCDC_OWRI) + DRM_DEV_INFO(dev, "FIFO overwrite interrupt"); + + if (status & ATMEL_LCDC_MERI) + DRM_DEV_INFO(dev, "DMA memory error"); + + /* Clear all reported (from ISR) interrupts */ + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_ICR, isr); + + return IRQ_HANDLED; +} + +static int lcdc_dc_irq_postinstall(struct drm_device *dev) +{ + struct lcdc_dc *lcdc_dc; + unsigned int ier; + + lcdc_dc = dev->dev_private; + + ier = 0; + /* FIFO underflow interrupt enable */ + ier |= ATMEL_LCDC_UFLWI; + /* FIFO overwrite interrupt enable */ + ier |= ATMEL_LCDC_OWRI; + /* DMA memory error interrupt enable */ + ier |= ATMEL_LCDC_MERI; + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_IER, ier); + + return 0; +} + +static void lcdc_dc_irq_uninstall(struct drm_device *dev) +{ + struct lcdc_dc *lcdc_dc; + unsigned int isr; + + lcdc_dc = dev->dev_private; + + /* disable all interrupts */ + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_IDR, ~0); + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_ICR, ~0); + + /* Clear any pending interrupts */ + regmap_read(lcdc_dc->regmap, ATMEL_LCDC_ISR, &isr); +} + +DEFINE_DRM_GEM_CMA_FOPS(lcdc_dc_drm_fops); + +static struct drm_driver lcdc_dc_drm_driver = { + .driver_features = DRIVER_HAVE_IRQ | + DRIVER_GEM | + DRIVER_MODESET | + DRIVER_PRIME | + DRIVER_ATOMIC, + .name = "atmel-lcdc", + .desc = "Atmel LCD Display Controller DRM", + .date = "20180808", + .major = 1, + .minor = 0, + .patchlevel = 0, + + .lastclose = drm_fb_helper_lastclose, + .fops = &lcdc_dc_drm_fops, + + .irq_handler = lcdc_dc_irq_handler, + .irq_preinstall = lcdc_dc_irq_uninstall, + .irq_postinstall = lcdc_dc_irq_postinstall, + .irq_uninstall = lcdc_dc_irq_uninstall, + + .dumb_create = drm_gem_cma_dumb_create, + + .gem_print_info = drm_gem_cma_print_info, + .gem_vm_ops = &drm_gem_cma_vm_ops, + + .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_export = drm_gem_prime_export, + + .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, + .gem_free_object_unlocked = drm_gem_cma_free_object, + + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, + .gem_prime_vmap = drm_gem_cma_prime_vmap, + .gem_prime_vunmap = drm_gem_cma_prime_vunmap, + .gem_prime_mmap = drm_gem_cma_prime_mmap, +}; + +static int lcdc_dc_modeset_init(struct lcdc_dc *lcdc_dc, struct drm_device *drm) +{ + struct drm_bridge *bridge; + struct drm_panel *panel; + struct device_node *np; + struct device *dev; + int ret; + + dev = drm->dev; + + drm_mode_config_init(drm); + drm->mode_config.min_width = lcdc_dc->desc->min_width; + drm->mode_config.min_height = lcdc_dc->desc->min_height; + drm->mode_config.max_width = lcdc_dc->desc->max_width; + drm->mode_config.max_height = lcdc_dc->desc->max_height; + drm->mode_config.funcs = &mode_config_funcs; + + np = dev->of_node; + /* port@0 is the output port */ + ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); + if (ret && ret != -ENODEV) { + DRM_DEV_ERROR(dev, "Failed to find panel (%d)\n", ret); + goto err_out; + } + + bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown); + if (IS_ERR(bridge)) { + ret = PTR_ERR(bridge); + DRM_DEV_ERROR(dev, "Failed to add bridge (%d)", ret); + goto err_panel_remove; + } + + lcdc_dc->panel = panel; + lcdc_dc->bridge = bridge; + + ret = drm_simple_display_pipe_init(drm, + &lcdc_dc->pipe, + &lcdc_dc_display_funcs, + lcdc_dc_formats, + ARRAY_SIZE(lcdc_dc_formats), + NULL, + &lcdc_dc->connector); + if (ret) { + DRM_DEV_ERROR(dev, "Failed to init display pipe (%d)\n", ret); + goto err_panel_remove; + } + + ret = drm_simple_display_pipe_attach_bridge(&lcdc_dc->pipe, bridge); + if (ret) { + DRM_DEV_ERROR(dev, "failed to attach bridge (%d)", ret); + goto err_panel_remove; + } + + drm_mode_config_reset(drm); + + return 0; + +err_panel_remove: + if (panel) + drm_panel_bridge_remove(bridge); + +err_out: + return ret; +} + +static int lcdc_dc_load(struct drm_device *drm) +{ + const struct of_device_id *match; + struct platform_device *pdev; + struct lcdc_dc *lcdc_dc; + struct device *dev; + int ret; + + dev = drm->dev; + pdev = to_platform_device(dev); + + match = of_match_node(atmel_lcdc_of_match, dev->parent->of_node); + if (!match) { + DRM_DEV_ERROR(dev, "invalid compatible string (node=%s)", + dev->parent->of_node->name); + return -ENODEV; + } + + if (!match->data) { + DRM_DEV_ERROR(dev, "invalid lcdc_dc description\n"); + return -EINVAL; + } + + lcdc_dc = devm_kzalloc(dev, sizeof(*lcdc_dc), GFP_KERNEL); + if (!lcdc_dc) { + DRM_DEV_ERROR(dev, "Failed to allocate lcdc_dc\n"); + return -ENOMEM; + } + + /* reset of lcdc might sleep and require a preemptible task context */ + INIT_WORK(&lcdc_dc->reset_lcdc_work, reset_lcdc_work); + + platform_set_drvdata(pdev, drm); + dev_set_drvdata(dev, lcdc_dc); + + lcdc_dc->mfd_lcdc = dev_get_drvdata(dev->parent); + drm->dev_private = lcdc_dc; + + lcdc_dc->regmap = lcdc_dc->mfd_lcdc->regmap; + lcdc_dc->desc = match->data; + lcdc_dc->dev = dev; + + lcdc_dc->lcd_supply = devm_regulator_get(dev, "lcd"); + if (IS_ERR(lcdc_dc->lcd_supply)) { + DRM_DEV_ERROR(dev, "Failed to get lcd-supply (%ld)\n", + PTR_ERR(lcdc_dc->lcd_supply)); + lcdc_dc->lcd_supply = NULL; + } + + lcdc_dc_start_clock(lcdc_dc); + + pm_runtime_enable(dev); + + ret = drm_vblank_init(drm, 1); + if (ret) { + DRM_DEV_ERROR(dev, "failed to initialize vblank (%d)\n", + ret); + goto err_pm_runtime_disable; + } + + ret = lcdc_dc_modeset_init(lcdc_dc, drm); + if (ret) { + DRM_DEV_ERROR(dev, "modeset_init failed (%d)", ret); + goto err_pm_runtime_disable; + } + + pm_runtime_get_sync(dev); + ret = drm_irq_install(drm, lcdc_dc->mfd_lcdc->irq); + pm_runtime_put_sync(dev); + if (ret < 0) { + DRM_DEV_ERROR(dev, "Failed to install IRQ (%d)\n", ret); + + goto err_pm_runtime_disable; + } + + /* + * Passing in 16 here will make the RGB656 mode the default + * Passing in 32 will use XRGB8888 mode + */ + drm_fb_cma_fbdev_init(drm, 16, 0); + + drm_kms_helper_poll_init(drm); + + lcdc_dc->fbdev = drm_fbdev_cma_init(drm, 8, 1); + if (IS_ERR(lcdc_dc->fbdev)) { + ret = PTR_ERR(lcdc_dc->fbdev); + DRM_DEV_ERROR(dev, "Failed to init FB CMA area (%d)", ret); + goto err_irq_uninstall; + } + + drm_helper_hpd_irq_event(drm); + + return 0; + +err_irq_uninstall: + pm_runtime_get_sync(dev); + drm_irq_uninstall(drm); + pm_runtime_put_sync(dev); + +err_pm_runtime_disable: + pm_runtime_disable(dev); + lcdc_dc_stop_clock(lcdc_dc); + + cancel_work_sync(&lcdc_dc->reset_lcdc_work); + + return ret; +} + +static void lcdc_dc_unload(struct drm_device *dev) +{ + struct lcdc_dc *lcdc_dc = dev->dev_private; + + drm_fb_cma_fbdev_fini(dev); + flush_work(&lcdc_dc->reset_lcdc_work); + drm_kms_helper_poll_fini(dev); + if (lcdc_dc->panel) + drm_panel_bridge_remove(lcdc_dc->bridge); + drm_mode_config_cleanup(dev); + + pm_runtime_get_sync(dev->dev); + drm_irq_uninstall(dev); + pm_runtime_put_sync(dev->dev); + + dev->dev_private = NULL; + + pm_runtime_disable(dev->dev); + lcdc_dc_stop_clock(lcdc_dc); + cancel_work_sync(&lcdc_dc->reset_lcdc_work); +} + + +static int lcdc_dc_probe(struct platform_device *pdev) +{ + struct drm_device *drm; + struct device *dev; + int ret; + + dev = &pdev->dev; + + drm = drm_dev_alloc(&lcdc_dc_drm_driver, dev); + if (IS_ERR(drm)) { + DRM_DEV_ERROR(dev, "Failed to allocate drm device\n"); + return PTR_ERR(drm); + } + + ret = lcdc_dc_load(drm); + if (ret) + goto err_put_ref; + + ret = drm_dev_register(drm, 0); + if (ret) { + DRM_DEV_ERROR(dev, "Failed to register drm (%d)\n", ret); + goto err_unload; + } + + return 0; + +err_unload: + lcdc_dc_unload(drm); + +err_put_ref: + drm_dev_put(drm); + return ret; +} + +static int lcdc_dc_remove(struct platform_device *pdev) +{ + struct drm_device *drm; + + drm = platform_get_drvdata(pdev); + + drm_dev_unregister(drm); + lcdc_dc_unload(drm); + drm_dev_unref(drm); + + return 0; +} + +static const struct of_device_id lcdc_dc_dt_ids[] = { + { .compatible = "atmel,lcdc-display-controller", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, lcdc_dc_dt_ids); + +static struct platform_driver lcdc_dc_driver = { + .probe = lcdc_dc_probe, + .remove = lcdc_dc_remove, + .driver = { + .of_match_table = lcdc_dc_dt_ids, + .name = "atmel-lcdc-dc", + }, +}; + +module_platform_driver(lcdc_dc_driver); + +MODULE_AUTHOR("Sam Ravnborg <sam@ravnborg.org>"); +MODULE_DESCRIPTION("Atmel LCDC Display Controller DRM Driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:atmel-lcdc-dc"); -- 2.12.0 ^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH v1 7/7] drm: add Atmel LCDC display controller support 2018-08-12 18:46 ` [PATCH v1 7/7] drm: add Atmel LCDC display controller support Sam Ravnborg @ 2018-08-24 12:31 ` Boris Brezillon 2018-08-26 18:41 ` Sam Ravnborg 2018-08-26 14:28 ` Noralf Trønnes 1 sibling, 1 reply; 50+ messages in thread From: Boris Brezillon @ 2018-08-24 12:31 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, Nicolas Ferre, Nicolas Ferre, dri-devel, Rob Herring, Lee Jones, linux-arm-kernel +Noralf Hi Sam, On Sun, 12 Aug 2018 20:46:29 +0200 Sam Ravnborg <sam@ravnborg.org> wrote: > This is a DRM based driver for the Atmel LCDC IP. > There exist today a framebuffer based driver and > this is a re-implmentation of the same on top of DRM. > > The rewrite was based on the original fbdev driver > but the driver has also seen inspiration from > the atmel-hlcdc_dc driver and others. > > The driver is not a full replacement: > - STN displays are not supported > Binding support is missing but most of the > STN specific functionality is otherwise ported > from the fbdev driver. > - gamma support is missing > The driver utilises drm_simple_kms_helper and > this helper lacks support for settting up gamma > - modesetting is not checked (see TODO in file) > - support for extra modes as applicable (and lcd-wiring-mode) > - support for AVR32 (is it relevant?) > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > MAINTAINERS | 7 + > drivers/gpu/drm/atmel/Kconfig | 12 + > drivers/gpu/drm/atmel/Makefile | 2 + > drivers/gpu/drm/atmel/atmel_lcdc-dc.c | 1094 +++++++++++++++++++++++++++++++++ > 4 files changed, 1115 insertions(+) > create mode 100644 drivers/gpu/drm/atmel/atmel_lcdc-dc.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 09ce76a9a1dc..0a594d02a7c0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4685,6 +4685,13 @@ F: drivers/gpu/drm/atmel/atmel-hlcdc* > F: Documentation/devicetree/bindings/display/atmel/ > T: git git://anongit.freedesktop.org/drm/drm-misc > > +DRM DRIVERS FOR ATMEL LCDC > +M: Sam Ravnborg <sam@ravnborg.org> > +L: dri-devel@lists.freedesktop.org > +S: Maintained > +F: drivers/gpu/drm/atmel/atmel-lcdc* > +F: Documentation/devicetree/bindings/display/atmel/ > + As stated in one of my previous replies, I think we should keep a single entry for both drivers, just update the existing one and we should be good. > DRM DRIVERS FOR BRIDGE CHIPS > M: Archit Taneja <architt@codeaurora.org> > M: Andrzej Hajda <a.hajda@samsung.com> [...] > +/* > + * The Atmel LCD controller display-controller supports several formats but > + * this driver supports only a small subset. > + * TODO: atmel_lcdfb supports more - port it over > + * Maybe actual wiring will impact mode support? > + */ > +static const u32 lcdc_dc_formats[] = { > + DRM_FORMAT_BGR565, > +}; > + > +/* Start LCD Controller (DMA + PWR) */ > +static void lcdc_dc_start(struct lcdc_dc *lcdc_dc) > +{ > + // Enable DMA Avoid using C++ style comments. > + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_DMACON, ATMEL_LCDC_DMAEN); > + // Enable LCD > + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_PWRCON, > + (lcdc_dc->desc->guard_time << ATMEL_LCDC_GUARDT_OFFSET) > + | ATMEL_LCDC_PWR); > +} > + > +static int lcdc_dc_display_check(struct drm_simple_display_pipe *pipe, > + struct drm_plane_state *pstate, > + struct drm_crtc_state *cstate) > +{ > + const struct drm_display_mode *dmode; > + struct drm_framebuffer *old_fb; > + struct drm_framebuffer *fb; > + > + dmode = &cstate->mode; > + old_fb = pipe->plane.state->fb; > + fb = pstate->fb; > + > + /* Check timing? */ Did you look at what other simple DRM drivers check in this hook? > + /* TODO */ > + > + return 0; > +} > + > +/* scheduled worker to reset LCD */ > +static void reset_lcdc_work(struct work_struct *work) > +{ > + struct lcdc_dc *lcdc_dc; > + > + lcdc_dc = container_of(work, struct lcdc_dc, reset_lcdc_work); > + Hm, you need a lock to protect this section and you have to check the LCDC state, because it might have been disabled through the ->disable() hook just after you have scheduled this reset operation. > + lcdc_dc_stop(lcdc_dc); > + lcdc_dc_start(lcdc_dc); > +} > + > +static irqreturn_t lcdc_dc_irq_handler(int irq, void *arg) > +{ > + struct lcdc_dc *lcdc_dc; > + struct drm_device *drm; > + unsigned int status; > + struct device *dev; > + unsigned int imr; > + unsigned int isr; > + > + drm = arg; > + lcdc_dc = drm->dev_private; > + dev = lcdc_dc->dev; > + > + regmap_read(lcdc_dc->regmap, ATMEL_LCDC_IMR, &imr); > + regmap_read(lcdc_dc->regmap, ATMEL_LCDC_ISR, &isr); > + status = imr & isr; > + if (!status) > + return IRQ_NONE; > + > + if (status & ATMEL_LCDC_LSTLNI) > + drm_crtc_handle_vblank(&lcdc_dc->pipe.crtc); > + > + if (status & ATMEL_LCDC_UFLWI) { > + DRM_DEV_INFO(dev, "FIFO underflow %#x\n", status); > + /* reset DMA and FIFO to avoid screen shifting */ > + schedule_work(&lcdc_dc->reset_lcdc_work); > + } Add a blank line here. > + if (status & ATMEL_LCDC_OWRI) > + DRM_DEV_INFO(dev, "FIFO overwrite interrupt"); > + > + if (status & ATMEL_LCDC_MERI) > + DRM_DEV_INFO(dev, "DMA memory error"); > + > + /* Clear all reported (from ISR) interrupts */ > + regmap_write(lcdc_dc->regmap, ATMEL_LCDC_ICR, isr); > + > + return IRQ_HANDLED; > +} > + [...] > +static int lcdc_dc_modeset_init(struct lcdc_dc *lcdc_dc, struct drm_device *drm) > +{ > + struct drm_bridge *bridge; > + struct drm_panel *panel; > + struct device_node *np; > + struct device *dev; > + int ret; > + > + dev = drm->dev; > + > + drm_mode_config_init(drm); > + drm->mode_config.min_width = lcdc_dc->desc->min_width; > + drm->mode_config.min_height = lcdc_dc->desc->min_height; > + drm->mode_config.max_width = lcdc_dc->desc->max_width; > + drm->mode_config.max_height = lcdc_dc->desc->max_height; > + drm->mode_config.funcs = &mode_config_funcs; > + > + np = dev->of_node; > + /* port@0 is the output port */ > + ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); > + if (ret && ret != -ENODEV) { > + DRM_DEV_ERROR(dev, "Failed to find panel (%d)\n", ret); > + goto err_out; return ret; > + } > + > + bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown); drm_of_find_panel_or_bridge() might directly return a bridge. And maybe we should declare a DPI connector. And finally, you can might want to use devm_drm_panel_bridge_add() instead of drm_panel_bridge_add() if (panel) bridge = devm_drm_panel_bridge_add(dev, panel, DRM_MODE_CONNECTOR_DPI); > + if (IS_ERR(bridge)) { > + ret = PTR_ERR(bridge); > + DRM_DEV_ERROR(dev, "Failed to add bridge (%d)", ret); > + goto err_panel_remove; Hm, if drm_panel_bridge_add() failed, you shouldn't call drm_panel_bridge_remove(bridge). You can just do return ret; > + } > + > + lcdc_dc->panel = panel; > + lcdc_dc->bridge = bridge; > + > + ret = drm_simple_display_pipe_init(drm, > + &lcdc_dc->pipe, > + &lcdc_dc_display_funcs, > + lcdc_dc_formats, > + ARRAY_SIZE(lcdc_dc_formats), > + NULL, > + &lcdc_dc->connector); > + if (ret) { > + DRM_DEV_ERROR(dev, "Failed to init display pipe (%d)\n", ret); > + goto err_panel_remove; If you use devm_drm_panel_bridge_add(), you can return directly. > + } > + > + ret = drm_simple_display_pipe_attach_bridge(&lcdc_dc->pipe, bridge); > + if (ret) { > + DRM_DEV_ERROR(dev, "failed to attach bridge (%d)", ret); > + goto err_panel_remove; Ditto. > + } > + > + drm_mode_config_reset(drm); > + > + return 0; > + > +err_panel_remove: > + if (panel) > + drm_panel_bridge_remove(bridge); > + > +err_out: > + return ret; And now you should be able to get rid of the error path entirely. > +} > + > +static int lcdc_dc_load(struct drm_device *drm) > +{ > + const struct of_device_id *match; > + struct platform_device *pdev; > + struct lcdc_dc *lcdc_dc; > + struct device *dev; > + int ret; > + > + dev = drm->dev; > + pdev = to_platform_device(dev); > + > + match = of_match_node(atmel_lcdc_of_match, dev->parent->of_node); > + if (!match) { > + DRM_DEV_ERROR(dev, "invalid compatible string (node=%s)", > + dev->parent->of_node->name); > + return -ENODEV; > + } > + > + if (!match->data) { > + DRM_DEV_ERROR(dev, "invalid lcdc_dc description\n"); > + return -EINVAL; > + } > + > + lcdc_dc = devm_kzalloc(dev, sizeof(*lcdc_dc), GFP_KERNEL); > + if (!lcdc_dc) { > + DRM_DEV_ERROR(dev, "Failed to allocate lcdc_dc\n"); > + return -ENOMEM; > + } > + > + /* reset of lcdc might sleep and require a preemptible task context */ > + INIT_WORK(&lcdc_dc->reset_lcdc_work, reset_lcdc_work); > + > + platform_set_drvdata(pdev, drm); > + dev_set_drvdata(dev, lcdc_dc); > + > + lcdc_dc->mfd_lcdc = dev_get_drvdata(dev->parent); > + drm->dev_private = lcdc_dc; > + > + lcdc_dc->regmap = lcdc_dc->mfd_lcdc->regmap; > + lcdc_dc->desc = match->data; > + lcdc_dc->dev = dev; > + > + lcdc_dc->lcd_supply = devm_regulator_get(dev, "lcd"); > + if (IS_ERR(lcdc_dc->lcd_supply)) { > + DRM_DEV_ERROR(dev, "Failed to get lcd-supply (%ld)\n", > + PTR_ERR(lcdc_dc->lcd_supply)); > + lcdc_dc->lcd_supply = NULL; > + } > + > + lcdc_dc_start_clock(lcdc_dc); Hm, do you really need to call that here? I'd make it part of the runtime PM resume hook, and put a lcdc_dc_stop_clock() in the suspend hook. > + > + pm_runtime_enable(dev); > + > + ret = drm_vblank_init(drm, 1); > + if (ret) { > + DRM_DEV_ERROR(dev, "failed to initialize vblank (%d)\n", > + ret); > + goto err_pm_runtime_disable; > + } > + > + ret = lcdc_dc_modeset_init(lcdc_dc, drm); > + if (ret) { > + DRM_DEV_ERROR(dev, "modeset_init failed (%d)", ret); > + goto err_pm_runtime_disable; > + } > + > + pm_runtime_get_sync(dev); This call will automatically call the runtime PM resume hook, so if you need the clk to be enabled before that point you should put it earlier. Also, you should call pm_runtime_get_sync() in the ->enable() path and pm_runtime_put() in the ->disable() path. > + ret = drm_irq_install(drm, lcdc_dc->mfd_lcdc->irq); > + pm_runtime_put_sync(dev); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "Failed to install IRQ (%d)\n", ret); > + > + goto err_pm_runtime_disable; > + } > + > + /* > + * Passing in 16 here will make the RGB656 mode the default > + * Passing in 32 will use XRGB8888 mode > + */ > + drm_fb_cma_fbdev_init(drm, 16, 0); > + > + drm_kms_helper_poll_init(drm); > + > + lcdc_dc->fbdev = drm_fbdev_cma_init(drm, 8, 1); > + if (IS_ERR(lcdc_dc->fbdev)) { > + ret = PTR_ERR(lcdc_dc->fbdev); > + DRM_DEV_ERROR(dev, "Failed to init FB CMA area (%d)", ret); > + goto err_irq_uninstall; > + } > + > + drm_helper_hpd_irq_event(drm); > + > + return 0; > + > +err_irq_uninstall: > + pm_runtime_get_sync(dev); > + drm_irq_uninstall(drm); > + pm_runtime_put_sync(dev); > + > +err_pm_runtime_disable: > + pm_runtime_disable(dev); > + lcdc_dc_stop_clock(lcdc_dc); > + > + cancel_work_sync(&lcdc_dc->reset_lcdc_work); Isn't there a race here? What if there was a work queued but you disabled the clk? I guess reg read/write accesses might fail (don't know how bad this can go though). > + > + return ret; > +} > + > +static void lcdc_dc_unload(struct drm_device *dev) > +{ > + struct lcdc_dc *lcdc_dc = dev->dev_private; > + > + drm_fb_cma_fbdev_fini(dev); > + flush_work(&lcdc_dc->reset_lcdc_work); > + drm_kms_helper_poll_fini(dev); > + if (lcdc_dc->panel) > + drm_panel_bridge_remove(lcdc_dc->bridge); You can drop that one if you use the devm_ version. > + drm_mode_config_cleanup(dev); > + > + pm_runtime_get_sync(dev->dev); > + drm_irq_uninstall(dev); > + pm_runtime_put_sync(dev->dev); > + > + dev->dev_private = NULL; > + > + pm_runtime_disable(dev->dev); > + lcdc_dc_stop_clock(lcdc_dc); > + cancel_work_sync(&lcdc_dc->reset_lcdc_work); And again, it might be too late. Should be moved just after disabling the IRQ, since this is the only path where you queue this work. > +} > + > + > +static int lcdc_dc_probe(struct platform_device *pdev) > +{ > + struct drm_device *drm; > + struct device *dev; > + int ret; > + > + dev = &pdev->dev; > + > + drm = drm_dev_alloc(&lcdc_dc_drm_driver, dev); > + if (IS_ERR(drm)) { > + DRM_DEV_ERROR(dev, "Failed to allocate drm device\n"); > + return PTR_ERR(drm); > + } > + > + ret = lcdc_dc_load(drm); > + if (ret) > + goto err_put_ref; > + > + ret = drm_dev_register(drm, 0); > + if (ret) { > + DRM_DEV_ERROR(dev, "Failed to register drm (%d)\n", ret); > + goto err_unload; > + } > + > + return 0; > + > +err_unload: > + lcdc_dc_unload(drm); > + > +err_put_ref: > + drm_dev_put(drm); > + return ret; > +} That's all I see for now, but keep in mind that I don't know much about the DRM simple interface, so you'd better wait for a review from someone who knows better (Noralf?). Regards, Boris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 7/7] drm: add Atmel LCDC display controller support 2018-08-24 12:31 ` Boris Brezillon @ 2018-08-26 18:41 ` Sam Ravnborg 0 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-26 18:41 UTC (permalink / raw) To: Boris Brezillon Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, Nicolas Ferre, Nicolas Ferre, dri-devel, Rob Herring, Lee Jones, linux-arm-kernel Hi Boris. Very usefull feedback! I am working on v2, and have addressed your review items. Just a few comments below to a few items. The rest is processed/done. I hope to post v2 in the week to come. > > +static int lcdc_dc_load(struct drm_device *drm) > > +{ > > + const struct of_device_id *match; > > + struct platform_device *pdev; > > + struct lcdc_dc *lcdc_dc; > > + struct device *dev; > > + int ret; > > + > > + dev = drm->dev; > > + pdev = to_platform_device(dev); > > + > > + match = of_match_node(atmel_lcdc_of_match, dev->parent->of_node); > > + if (!match) { > > + DRM_DEV_ERROR(dev, "invalid compatible string (node=%s)", > > + dev->parent->of_node->name); > > + return -ENODEV; > > + } > > + > > + if (!match->data) { > > + DRM_DEV_ERROR(dev, "invalid lcdc_dc description\n"); > > + return -EINVAL; > > + } > > + > > + lcdc_dc = devm_kzalloc(dev, sizeof(*lcdc_dc), GFP_KERNEL); > > + if (!lcdc_dc) { > > + DRM_DEV_ERROR(dev, "Failed to allocate lcdc_dc\n"); > > + return -ENOMEM; > > + } > > + > > + /* reset of lcdc might sleep and require a preemptible task context */ > > + INIT_WORK(&lcdc_dc->reset_lcdc_work, reset_lcdc_work); > > + > > + platform_set_drvdata(pdev, drm); > > + dev_set_drvdata(dev, lcdc_dc); > > + > > + lcdc_dc->mfd_lcdc = dev_get_drvdata(dev->parent); > > + drm->dev_private = lcdc_dc; > > + > > + lcdc_dc->regmap = lcdc_dc->mfd_lcdc->regmap; > > + lcdc_dc->desc = match->data; > > + lcdc_dc->dev = dev; > > + > > + lcdc_dc->lcd_supply = devm_regulator_get(dev, "lcd"); > > + if (IS_ERR(lcdc_dc->lcd_supply)) { > > + DRM_DEV_ERROR(dev, "Failed to get lcd-supply (%ld)\n", > > + PTR_ERR(lcdc_dc->lcd_supply)); > > + lcdc_dc->lcd_supply = NULL; > > + } > > + > > + lcdc_dc_start_clock(lcdc_dc); > > Hm, do you really need to call that here? I'd make it part of the > runtime PM resume hook, and put a lcdc_dc_stop_clock() in the suspend > hook. As suggested I have introduced suspend/resume hooks and have included start/stop clock calls. But as the suspend/resume hooks depends on CONFIG_PM_SLEEP I still need the explicit call in this function. > > + > > + pm_runtime_enable(dev); > > + > > + ret = drm_vblank_init(drm, 1); > > + if (ret) { > > + DRM_DEV_ERROR(dev, "failed to initialize vblank (%d)\n", > > + ret); > > + goto err_pm_runtime_disable; > > + } > > + > > + ret = lcdc_dc_modeset_init(lcdc_dc, drm); > > + if (ret) { > > + DRM_DEV_ERROR(dev, "modeset_init failed (%d)", ret); > > + goto err_pm_runtime_disable; > > + } > > + > > + pm_runtime_get_sync(dev); > > This call will automatically call the runtime PM resume hook, so if you > need the clk to be enabled before that point you should put it earlier. > > Also, you should call pm_runtime_get_sync() in the ->enable() path and > pm_runtime_put() in the ->disable() path. Good catch! Added. Sam _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 7/7] drm: add Atmel LCDC display controller support 2018-08-12 18:46 ` [PATCH v1 7/7] drm: add Atmel LCDC display controller support Sam Ravnborg 2018-08-24 12:31 ` Boris Brezillon @ 2018-08-26 14:28 ` Noralf Trønnes 2018-08-26 14:58 ` Sam Ravnborg 1 sibling, 1 reply; 50+ messages in thread From: Noralf Trønnes @ 2018-08-26 14:28 UTC (permalink / raw) To: Sam Ravnborg, Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, Boris Brezillon, Nicolas Ferre, dri-devel, linux-pwm, linux-arm-kernel Den 12.08.2018 20.46, skrev Sam Ravnborg: > This is a DRM based driver for the Atmel LCDC IP. > There exist today a framebuffer based driver and > this is a re-implmentation of the same on top of DRM. > > The rewrite was based on the original fbdev driver > but the driver has also seen inspiration from > the atmel-hlcdc_dc driver and others. > > The driver is not a full replacement: > - STN displays are not supported > Binding support is missing but most of the > STN specific functionality is otherwise ported > from the fbdev driver. > - gamma support is missing > The driver utilises drm_simple_kms_helper and > this helper lacks support for settting up gamma > - modesetting is not checked (see TODO in file) > - support for extra modes as applicable (and lcd-wiring-mode) > - support for AVR32 (is it relevant?) > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > MAINTAINERS | 7 + > drivers/gpu/drm/atmel/Kconfig | 12 + > drivers/gpu/drm/atmel/Makefile | 2 + > drivers/gpu/drm/atmel/atmel_lcdc-dc.c | 1094 +++++++++++++++++++++++++++++++++ > 4 files changed, 1115 insertions(+) > create mode 100644 drivers/gpu/drm/atmel/atmel_lcdc-dc.c > <snip> > diff --git a/drivers/gpu/drm/atmel/atmel_lcdc-dc.c b/drivers/gpu/drm/atmel/atmel_lcdc-dc.c > new file mode 100644 > index 000000000000..275a09e2e100 > --- /dev/null > +++ b/drivers/gpu/drm/atmel/atmel_lcdc-dc.c > @@ -0,0 +1,1094 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Sam Ravnborg > + * > + * The driver is based on atmel_lcdfb which is: > + * Copyright (C) 2007 Atmel Corporation > + * > + * Atmel LCD Controller Display Controller. > + * A sub-device of the Atmel LCDC IP. > + * > + * The Atmel LCD Controller supports in the following configuration: > + * - TFT only, with BGR565, 8 bits/pixel > + * - Resolution up to 2048x2048 > + * - Single plane, crtc, one fixed output > + * > + * Features not (yet) ported from atmel_lcdfb: > + * - Support for extra modes (and configurable intensify bit) > + * - Check modesetting support - lcdc_dc_display_check() > + * - set color / palette handling > + * - support for STN displays (partly implemented) > + * - AVR32 support (relevant?) > + */ > + > +#include <linux/regulator/consumer.h> > +#include <linux/mfd/atmel-lcdc.h> > +#include <linux/of_platform.h> > +#include <linux/pm_runtime.h> > +#include <linux/irqreturn.h> > +#include <linux/device.h> > +#include <linux/regmap.h> > + > +#include <video/videomode.h> > + > +#include <drm/drm_gem_framebuffer_helper.h> > +#include <drm/drm_simple_kms_helper.h> > +#include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_fb_cma_helper.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_fb_helper.h> > +#include <drm/drm_fourcc.h> > +#include <drm/drm_vblank.h> > +#include <drm/drm_modes.h> > +#include <drm/drm_panel.h> > +#include <drm/drm_print.h> > +#include <drm/drm_drv.h> > +#include <drm/drm_of.h> > + > +/* Parameters */ > +#define ATMEL_LCDC_DMA_BURST_LEN 8 /* words */ > + > +/** > + * struct atmel_lcdc_dc_desc - CPU specific configuration properties > + */ > +struct atmel_lcdc_dc_desc { > + int guard_time; > + int fifo_size; > + int min_width; > + int min_height; > + int max_width; > + int max_height; > + bool have_hozval; > + bool have_alt_pixclock; > +}; > + > +/* private data */ > +struct lcdc_dc { > + const struct atmel_lcdc_dc_desc *desc; > + struct atmel_mfd_lcdc *mfd_lcdc; > + struct regulator *lcd_supply; > + struct drm_fbdev_cma *fbdev; There's generic fbdev emulaton now, so you can drop this. More info further down. > + struct drm_bridge *bridge; > + struct drm_panel *panel; > + struct regmap *regmap; > + struct device *dev; > + > + struct drm_simple_display_pipe pipe; > + struct work_struct reset_lcdc_work; > + struct drm_connector connector; > +}; > + > +/* Configuration of individual CPU's */ > +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9261 = { > + .guard_time = 1, > + .fifo_size = 512, > + .min_width = 0, > + .min_height = 0, Looks like the minimum is always zero, maybe drop it? > + .max_width = 2048, > + .max_height = 2048, > + .have_hozval = true, > + .have_alt_pixclock = false, > +}; > + > +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9263 = { > + .guard_time = 1, > + .fifo_size = 2048, > + .min_width = 0, > + .min_height = 0, > + .max_width = 2048, > + .max_height = 2048, > + .have_hozval = false, > + .have_alt_pixclock = false, > +}; > + > +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9g10 = { > + .guard_time = 1, > + .fifo_size = 512, > + .min_width = 0, > + .min_height = 0, > + .max_width = 2048, > + .max_height = 2048, > + .have_hozval = true, > + .have_alt_pixclock = false, > +}; > + > +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9g45 = { > + .guard_time = 1, > + .fifo_size = 512, > + .min_width = 0, > + .min_height = 0, > + .max_width = 2048, > + .max_height = 2048, > + .have_hozval = false, > + .have_alt_pixclock = true, > +}; > + > +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9g46 = { > + .guard_time = 1, > + .fifo_size = 512, > + .min_width = 0, > + .min_height = 0, > + .max_width = 2048, > + .max_height = 2048, > + .have_hozval = false, > + .have_alt_pixclock = false, > +}; > + > +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9m10 = { > + .guard_time = 1, > + .fifo_size = 512, > + .min_width = 0, > + .min_height = 0, > + .max_width = 2048, > + .max_height = 2048, > + .have_hozval = false, > + .have_alt_pixclock = false, > +}; > + > +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9m11 = { > + .guard_time = 1, > + .fifo_size = 512, > + .min_width = 0, > + .min_height = 0, > + .max_width = 2048, > + .max_height = 2048, > + .have_hozval = false, > + .have_alt_pixclock = false, > +}; > + > +static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9rl = { > + .guard_time = 1, > + .fifo_size = 512, > + .min_width = 0, > + .min_height = 0, > + .max_width = 2048, > + .max_height = 2048, > + .have_hozval = false, > + .have_alt_pixclock = false, > +}; <snip> > +DEFINE_DRM_GEM_CMA_FOPS(lcdc_dc_drm_fops); > + > +static struct drm_driver lcdc_dc_drm_driver = { > + .driver_features = DRIVER_HAVE_IRQ | > + DRIVER_GEM | > + DRIVER_MODESET | > + DRIVER_PRIME | > + DRIVER_ATOMIC, > + .name = "atmel-lcdc", > + .desc = "Atmel LCD Display Controller DRM", > + .date = "20180808", > + .major = 1, > + .minor = 0, > + .patchlevel = 0, > + > + .lastclose = drm_fb_helper_lastclose, You can drop this, lastclose is handled by the generic fbdev emulation. > + .fops = &lcdc_dc_drm_fops, > + > + .irq_handler = lcdc_dc_irq_handler, > + .irq_preinstall = lcdc_dc_irq_uninstall, > + .irq_postinstall = lcdc_dc_irq_postinstall, > + .irq_uninstall = lcdc_dc_irq_uninstall, > + > + .dumb_create = drm_gem_cma_dumb_create, > + > + .gem_print_info = drm_gem_cma_print_info, > + .gem_vm_ops = &drm_gem_cma_vm_ops, > + > + .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_export = drm_gem_prime_export, > + > + .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, > + .gem_free_object_unlocked = drm_gem_cma_free_object, > + > + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > + .gem_prime_vmap = drm_gem_cma_prime_vmap, > + .gem_prime_vunmap = drm_gem_cma_prime_vunmap, > + .gem_prime_mmap = drm_gem_cma_prime_mmap, > +}; > + > +static int lcdc_dc_modeset_init(struct lcdc_dc *lcdc_dc, struct drm_device *drm) > +{ > + struct drm_bridge *bridge; > + struct drm_panel *panel; > + struct device_node *np; > + struct device *dev; > + int ret; > + > + dev = drm->dev; > + > + drm_mode_config_init(drm); > + drm->mode_config.min_width = lcdc_dc->desc->min_width; > + drm->mode_config.min_height = lcdc_dc->desc->min_height; > + drm->mode_config.max_width = lcdc_dc->desc->max_width; > + drm->mode_config.max_height = lcdc_dc->desc->max_height; > + drm->mode_config.funcs = &mode_config_funcs; > + > + np = dev->of_node; > + /* port@0 is the output port */ > + ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); > + if (ret && ret != -ENODEV) { > + DRM_DEV_ERROR(dev, "Failed to find panel (%d)\n", ret); > + goto err_out; > + } > + > + bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown); > + if (IS_ERR(bridge)) { > + ret = PTR_ERR(bridge); > + DRM_DEV_ERROR(dev, "Failed to add bridge (%d)", ret); > + goto err_panel_remove; > + } > + > + lcdc_dc->panel = panel; > + lcdc_dc->bridge = bridge; > + > + ret = drm_simple_display_pipe_init(drm, > + &lcdc_dc->pipe, > + &lcdc_dc_display_funcs, > + lcdc_dc_formats, > + ARRAY_SIZE(lcdc_dc_formats), > + NULL, > + &lcdc_dc->connector); > + if (ret) { > + DRM_DEV_ERROR(dev, "Failed to init display pipe (%d)\n", ret); > + goto err_panel_remove; > + } > + > + ret = drm_simple_display_pipe_attach_bridge(&lcdc_dc->pipe, bridge); > + if (ret) { > + DRM_DEV_ERROR(dev, "failed to attach bridge (%d)", ret); > + goto err_panel_remove; > + } > + > + drm_mode_config_reset(drm); > + > + return 0; > + > +err_panel_remove: > + if (panel) > + drm_panel_bridge_remove(bridge); > + > +err_out: > + return ret; > +} > + > +static int lcdc_dc_load(struct drm_device *drm) > +{ > + const struct of_device_id *match; > + struct platform_device *pdev; > + struct lcdc_dc *lcdc_dc; > + struct device *dev; > + int ret; > + > + dev = drm->dev; > + pdev = to_platform_device(dev); > + > + match = of_match_node(atmel_lcdc_of_match, dev->parent->of_node); > + if (!match) { > + DRM_DEV_ERROR(dev, "invalid compatible string (node=%s)", > + dev->parent->of_node->name); > + return -ENODEV; > + } > + > + if (!match->data) { > + DRM_DEV_ERROR(dev, "invalid lcdc_dc description\n"); > + return -EINVAL; > + } > + > + lcdc_dc = devm_kzalloc(dev, sizeof(*lcdc_dc), GFP_KERNEL); > + if (!lcdc_dc) { > + DRM_DEV_ERROR(dev, "Failed to allocate lcdc_dc\n"); > + return -ENOMEM; > + } > + > + /* reset of lcdc might sleep and require a preemptible task context */ > + INIT_WORK(&lcdc_dc->reset_lcdc_work, reset_lcdc_work); > + > + platform_set_drvdata(pdev, drm); > + dev_set_drvdata(dev, lcdc_dc); > + > + lcdc_dc->mfd_lcdc = dev_get_drvdata(dev->parent); > + drm->dev_private = lcdc_dc; > + > + lcdc_dc->regmap = lcdc_dc->mfd_lcdc->regmap; > + lcdc_dc->desc = match->data; > + lcdc_dc->dev = dev; > + > + lcdc_dc->lcd_supply = devm_regulator_get(dev, "lcd"); > + if (IS_ERR(lcdc_dc->lcd_supply)) { > + DRM_DEV_ERROR(dev, "Failed to get lcd-supply (%ld)\n", > + PTR_ERR(lcdc_dc->lcd_supply)); > + lcdc_dc->lcd_supply = NULL; > + } > + > + lcdc_dc_start_clock(lcdc_dc); > + > + pm_runtime_enable(dev); > + > + ret = drm_vblank_init(drm, 1); > + if (ret) { > + DRM_DEV_ERROR(dev, "failed to initialize vblank (%d)\n", > + ret); > + goto err_pm_runtime_disable; > + } > + > + ret = lcdc_dc_modeset_init(lcdc_dc, drm); > + if (ret) { > + DRM_DEV_ERROR(dev, "modeset_init failed (%d)", ret); > + goto err_pm_runtime_disable; > + } > + > + pm_runtime_get_sync(dev); > + ret = drm_irq_install(drm, lcdc_dc->mfd_lcdc->irq); > + pm_runtime_put_sync(dev); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "Failed to install IRQ (%d)\n", ret); > + > + goto err_pm_runtime_disable; > + } > + > + /* > + * Passing in 16 here will make the RGB656 mode the default > + * Passing in 32 will use XRGB8888 mode > + */ > + drm_fb_cma_fbdev_init(drm, 16, 0); You are using both drm_fb_cma_fbdev_init() and drm_fbdev_cma_init() which does the same. I'm to blame for the confusion, the generic fbdev emulation idea came up during refactoring, so there's an old way and a new way, which is now obsolete (both will be removed when drivers have been converted to drm_fbdev_generic_setup()). You can use drm_fbdev_generic_setup(drm, 16) instead, and that's all you need to get fbdev emulation. I see that many drivers register fbdev before they register the DRM device. Personally I find this strange to make the emulated device available before the real one. I suggest you call drm_fbdev_generic_setup() after drm_dev_register() and if it fails, just put a note in the log and let the driver succeed in probing. > + > + drm_kms_helper_poll_init(drm); > + > + lcdc_dc->fbdev = drm_fbdev_cma_init(drm, 8, 1); > + if (IS_ERR(lcdc_dc->fbdev)) { > + ret = PTR_ERR(lcdc_dc->fbdev); > + DRM_DEV_ERROR(dev, "Failed to init FB CMA area (%d)", ret); > + goto err_irq_uninstall; > + } > + > + drm_helper_hpd_irq_event(drm); > + > + return 0; > + > +err_irq_uninstall: > + pm_runtime_get_sync(dev); > + drm_irq_uninstall(drm); > + pm_runtime_put_sync(dev); > + > +err_pm_runtime_disable: > + pm_runtime_disable(dev); > + lcdc_dc_stop_clock(lcdc_dc); > + > + cancel_work_sync(&lcdc_dc->reset_lcdc_work); > + > + return ret; > +} > + > +static void lcdc_dc_unload(struct drm_device *dev) > +{ > + struct lcdc_dc *lcdc_dc = dev->dev_private; > + > + drm_fb_cma_fbdev_fini(dev); You can remove this. fbdev is automatically unregistered by drm_dev_unregister(). Noralf. > + flush_work(&lcdc_dc->reset_lcdc_work); > + drm_kms_helper_poll_fini(dev); > + if (lcdc_dc->panel) > + drm_panel_bridge_remove(lcdc_dc->bridge); > + drm_mode_config_cleanup(dev); > + > + pm_runtime_get_sync(dev->dev); > + drm_irq_uninstall(dev); > + pm_runtime_put_sync(dev->dev); > + > + dev->dev_private = NULL; > + > + pm_runtime_disable(dev->dev); > + lcdc_dc_stop_clock(lcdc_dc); > + cancel_work_sync(&lcdc_dc->reset_lcdc_work); > +} > + > + > +static int lcdc_dc_probe(struct platform_device *pdev) > +{ > + struct drm_device *drm; > + struct device *dev; > + int ret; > + > + dev = &pdev->dev; > + > + drm = drm_dev_alloc(&lcdc_dc_drm_driver, dev); > + if (IS_ERR(drm)) { > + DRM_DEV_ERROR(dev, "Failed to allocate drm device\n"); > + return PTR_ERR(drm); > + } > + > + ret = lcdc_dc_load(drm); > + if (ret) > + goto err_put_ref; > + > + ret = drm_dev_register(drm, 0); > + if (ret) { > + DRM_DEV_ERROR(dev, "Failed to register drm (%d)\n", ret); > + goto err_unload; > + } > + > + return 0; > + > +err_unload: > + lcdc_dc_unload(drm); > + > +err_put_ref: > + drm_dev_put(drm); > + return ret; > +} > + > +static int lcdc_dc_remove(struct platform_device *pdev) > +{ > + struct drm_device *drm; > + > + drm = platform_get_drvdata(pdev); > + > + drm_dev_unregister(drm); > + lcdc_dc_unload(drm); > + drm_dev_unref(drm); > + > + return 0; > +} > + > +static const struct of_device_id lcdc_dc_dt_ids[] = { > + { .compatible = "atmel,lcdc-display-controller", }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, lcdc_dc_dt_ids); > + > +static struct platform_driver lcdc_dc_driver = { > + .probe = lcdc_dc_probe, > + .remove = lcdc_dc_remove, > + .driver = { > + .of_match_table = lcdc_dc_dt_ids, > + .name = "atmel-lcdc-dc", > + }, > +}; > + > +module_platform_driver(lcdc_dc_driver); > + > +MODULE_AUTHOR("Sam Ravnborg <sam@ravnborg.org>"); > +MODULE_DESCRIPTION("Atmel LCDC Display Controller DRM Driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:atmel-lcdc-dc"); _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v1 7/7] drm: add Atmel LCDC display controller support 2018-08-26 14:28 ` Noralf Trønnes @ 2018-08-26 14:58 ` Sam Ravnborg 0 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-26 14:58 UTC (permalink / raw) To: Noralf Trønnes Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Boris Brezillon, Nicolas Ferre, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel Hi Noralf. > >+/* Configuration of individual CPU's */ > >+static const struct atmel_lcdc_dc_desc atmel_lcdc_dc_at91sam9261 = { > >+ .guard_time = 1, > >+ .fifo_size = 512, > >+ .min_width = 0, > >+ .min_height = 0, > > Looks like the minimum is always zero, maybe drop it? I kept is at in reality 0 is not an OK value. Will revisit the datasheets and check what they say. As for all the other comments - thanks! I will address them in v2. Currently working on a larger refactoring to address all the feedback. Things becomes simpler which is always good. Sam _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg ` (6 preceding siblings ...) 2018-08-12 18:46 ` [PATCH v1 7/7] drm: add Atmel LCDC display controller support Sam Ravnborg @ 2018-08-12 19:55 ` Sam Ravnborg 2018-08-13 14:47 ` Nicolas Ferre 2018-08-13 15:54 ` Nicolas Ferre 2018-08-24 8:22 ` Boris Brezillon 9 siblings, 1 reply; 50+ messages in thread From: Sam Ravnborg @ 2018-08-12 19:55 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Nicolas Ferre, Alexandre Belloni Cc: devicetree, linux-arm-kernel, dri-devel, linux-pwm The at91sam9263 has a few interesting "GPU" features: - 2D memory addressing Data sheet says: The LCDC can be configured to work on a frame buffer larger than the actual screen size. By changing the values in a few registers, it is easy to move the displayed area along the frame buffer width and height - 2D Graphics controller Data sheet says: The Two D Graphics Controller (TDGC) features a hardware accelerator which highly simplifies drawing tasks and graphic management operations. The hardware accelerator makes it easy to draw lines and complex polygons and to perform block transfers within the frame buffer. The TDGC also features a draw command queue that automatically executes a more complex drawing function that is composed of several register accesses. The datasheet text is from: chapter 43.9 + chapter 44: http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-6249-32-bit-ARM926EJ-S-Embedded-Microprocessor-SAM9263_Datasheet.pdf (No NDA required) Based on the above, would it be possible to utilise some of these features without any dedicated userspace (mesa) support? Any other driver that has something similar that can be used for inspiration? Or is this in reality a simple GPU that requires a dedicated GPU driver? Sam ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-12 19:55 ` [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg @ 2018-08-13 14:47 ` Nicolas Ferre 2018-08-14 8:41 ` Daniel Vetter 0 siblings, 1 reply; 50+ messages in thread From: Nicolas Ferre @ 2018-08-13 14:47 UTC (permalink / raw) To: Sam Ravnborg, Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Alexandre Belloni, Joshua Henderson Cc: devicetree, linux-arm-kernel, dri-devel, linux-pwm On 12/08/2018 at 21:55, Sam Ravnborg wrote: > The at91sam9263 has a few interesting "GPU" features: > > - 2D memory addressing > Data sheet says: > The LCDC can be configured to work on a frame buffer > larger than the actual screen size. By changing the > values in a few registers, it is easy to move the > displayed area along the frame buffer width and height > > - 2D Graphics controller > Data sheet says: > The Two D Graphics Controller (TDGC) features a > hardware accelerator which highly simplifies drawing > tasks and graphic management operations. > The hardware accelerator makes it easy to draw lines > and complex polygons and to perform block transfers > within the frame buffer. > The TDGC also features a draw command queue that > automatically executes a more complex drawing function > that is composed of several register accesses. > > The datasheet text is from: chapter 43.9 + chapter 44: > http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-6249-32-bit-ARM926EJ-S-Embedded-Microprocessor-SAM9263_Datasheet.pdf > (No NDA required) (old memories) this 2D engine prove itself of being quite limited as the fill polygon and clipping functions are not working. > Based on the above, would it be possible to utilise some > of these features without any dedicated userspace (mesa) support? > Any other driver that has something similar that can be used > for inspiration? This is really an interesting question indeed. > Or is this in reality a simple GPU that requires a > dedicated GPU driver? > > Sam > -- Nicolas Ferre ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-13 14:47 ` Nicolas Ferre @ 2018-08-14 8:41 ` Daniel Vetter 2018-08-22 20:12 ` Sam Ravnborg 0 siblings, 1 reply; 50+ messages in thread From: Daniel Vetter @ 2018-08-14 8:41 UTC (permalink / raw) To: Nicolas Ferre Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Sam Ravnborg, Joshua Henderson, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel On Mon, Aug 13, 2018 at 04:47:26PM +0200, Nicolas Ferre wrote: > On 12/08/2018 at 21:55, Sam Ravnborg wrote: > > The at91sam9263 has a few interesting "GPU" features: > > > > - 2D memory addressing > > Data sheet says: > > The LCDC can be configured to work on a frame buffer > > larger than the actual screen size. By changing the > > values in a few registers, it is easy to move the > > displayed area along the frame buffer width and height > > > > - 2D Graphics controller > > Data sheet says: > > The Two D Graphics Controller (TDGC) features a > > hardware accelerator which highly simplifies drawing > > tasks and graphic management operations. > > The hardware accelerator makes it easy to draw lines > > and complex polygons and to perform block transfers > > within the frame buffer. > > The TDGC also features a draw command queue that > > automatically executes a more complex drawing function > > that is composed of several register accesses. > > > > The datasheet text is from: chapter 43.9 + chapter 44: > > http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-6249-32-bit-ARM926EJ-S-Embedded-Microprocessor-SAM9263_Datasheet.pdf > > (No NDA required) > > (old memories) this 2D engine prove itself of being quite limited as the > fill polygon and clipping functions are not working. > > > Based on the above, would it be possible to utilise some > > of these features without any dedicated userspace (mesa) support? > > Any other driver that has something similar that can be used > > for inspiration? > > This is really an interesting question indeed. Needs userspace like everything else. There's unfortunately no real standard for 2d apis in userspace, which means none of these efforts go very far. Mostly it's just some custom-made X drivers. Adding a generic 2d api to drm is a FAQ, and the answer is "no". -Daniel > > > Or is this in reality a simple GPU that requires a > > dedicated GPU driver? > > > > Sam > > > > > -- > Nicolas Ferre > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-14 8:41 ` Daniel Vetter @ 2018-08-22 20:12 ` Sam Ravnborg 2018-08-23 6:16 ` Daniel Vetter 0 siblings, 1 reply; 50+ messages in thread From: Sam Ravnborg @ 2018-08-22 20:12 UTC (permalink / raw) To: Daniel Vetter Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Joshua Henderson, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel Hi Daniel. > > > Based on the above, would it be possible to utilise some > > > of these features without any dedicated userspace (mesa) support? > > > Any other driver that has something similar that can be used > > > for inspiration? > > > > This is really an interesting question indeed. > > Needs userspace like everything else. There's unfortunately no real > standard for 2d apis in userspace, which means none of these efforts go > very far. Mostly it's just some custom-made X drivers. > > Adding a generic 2d api to drm is a FAQ, and the answer is "no". And then you went off writing a blog post about it - thanks. The blog post provided a lot of useful answers - great. (Or rather you will write the blog in 5 days, seems to be some time machine involved. (Date is the 27th below the title)). Sam _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-22 20:12 ` Sam Ravnborg @ 2018-08-23 6:16 ` Daniel Vetter 0 siblings, 0 replies; 50+ messages in thread From: Daniel Vetter @ 2018-08-23 6:16 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Joshua Henderson, Nicolas Ferre, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, Linux ARM On Wed, Aug 22, 2018 at 10:12 PM, Sam Ravnborg <sam@ravnborg.org> wrote: > Hi Daniel. > >> > > Based on the above, would it be possible to utilise some >> > > of these features without any dedicated userspace (mesa) support? >> > > Any other driver that has something similar that can be used >> > > for inspiration? >> > >> > This is really an interesting question indeed. >> >> Needs userspace like everything else. There's unfortunately no real >> standard for 2d apis in userspace, which means none of these efforts go >> very far. Mostly it's just some custom-made X drivers. >> >> Adding a generic 2d api to drm is a FAQ, and the answer is "no". > > And then you went off writing a blog post about it - thanks. > The blog post provided a lot of useful answers - great. Yeah I figured more detail than this here would be good, but also wanted to avoid having to retype that ever again. > (Or rather you will write the blog in 5 days, seems to be some time > machine involved. (Date is the 27th below the title)). My blog platform pulled one on me - pushed it out with a future date, with the idea to proof-read a bit more. Didn't work, so everyone read the slightly draft-ish version :-/ In the past this did work, and was very neat. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg ` (7 preceding siblings ...) 2018-08-12 19:55 ` [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg @ 2018-08-13 15:54 ` Nicolas Ferre 2018-08-13 18:18 ` Sam Ravnborg 2018-08-24 8:22 ` Boris Brezillon 9 siblings, 1 reply; 50+ messages in thread From: Nicolas Ferre @ 2018-08-13 15:54 UTC (permalink / raw) To: Sam Ravnborg, Boris Brezillon, Rob Herring, Mark Rutland, Lee Jones, Alexandre Belloni Cc: devicetree, Joshua Henderson, linux-arm-kernel, dri-devel, linux-pwm On 12/08/2018 at 20:41, Sam Ravnborg wrote: > New DRM based driver for at91sam9 SOC's that uses the > Atmel LCDC IP core. I'm delighted to see this work: Thanks a lot Sam! > This is first version of a patch set that adds > drivers for the Atmel LCDC IP core. > Posted for review as the basics works now. > > The LCDC IP core contains two devices: > - a PWM often used for backlight > - a LCD display controller > > Both devices are supported today by the atmel_lcdfb driver. > For this new set of drivers the compatible strings was > selected to avoid clash with the existing compatible > strings used for the atmel_lcdfb driver to allow them > to co-exist. Would be good to have a plan to phase-out the old atmel_lcdfb fbdev driver when this one addresses some TODO items that make sense. The mfd suffix seems strange to me. What about "atmel,at91sam9263-lcdc-mfd" => "atmel,at91sam9263-lcd" (or "microchip,at91sam9263-lcdc"). > This patchset implements three drivers. > - A MFD driver that include the generic parts. > - A PWM driver. > - A DRM display controller driver. > This is the same split as used for the Atmel hlcdc IP. Good, this is why I would use the same type of compatibility string for the split mfd/pwm/lcd (just without the "h"). > The hlcdc and lcdc has only a few things in common and > trying to share the code for them was not a viable solution. Yes, I agree. > The DRM implementation has a few shortcomings compared to the > existing fbdev based driver: > - STN displays are not supported > Binding support is missing but most of the > STN specific functionality is otherwise ported > from the fbdev driver. > I assume the info should come from the panel > but as I lack HW I have not looked too much > into what is required. Yes, I'm not even sure if STN displays are still available those days... Might not be worth it spending time on this. > - gamma support is missing > The driver utilises drm_simple_kms_helper and > this helper lacks support for setting up gamma. > If this is useful please let me know and I > will extend drm_simple_kms_helper to support this > and update the driver. > - modesetting is not checked (see TODO in file) > Is this required for such a simple setup? > - support for extra modes as applicable (and lcd-wiring-mode) > - support for AVR32 (is it relevant?) No, AVR32 is not relevant anymore as the core and SoC were removed some years ago from Linux. All mention of AT32 or AVR32 can be remove then. > The first patch renames .../drm/atmel-hlcdc to .../drm/atmel > to have a nice home for both drivers. > > The drivers _works_ on a proprietary at91sam9263 based board > and I will eventually migrate the at91sam9263ek over > to use it as well. We'll be able to test it on other SoCs and boards. > Works is maybe a stretch - the following was tested: > modetest shows reasonable output > modetest -s shows some nice colored squares You must see something like this (without the overlay additional black square): http://www.at91.com/linux4sam/bin/view/Linux4SAM/UsingAtmelDRMDriver#Some_modetest_commands > vbltest shows 45.70 Hz (after terminating modetest -s with SIGINT) > drmdevice shows reasonable output > brightness can be controlled > > Anything else that can be recommended for some basic tests? > How to test suspend/resume? > > REVIEW > Please give feedback on general structure/architecture > Please check if the drm framework is used in the optimal way > And then the usual review from spelling errors, names, style etc. > > All feedback (from spelling errors to general structure) appreciated! On my side, it's mostly Nitpicking ;-) Now that we're Microchip for a little bit more than 2 years, I tend to make this name prevalent compared to Atmel for new work around our products... But I know this driver is for older SoCs and that it got inspired by two former drivers that have this "atmel" naming everywhere. MAINTAINERS and Kconfig changes could include Microchip name, so that it's obvious for the newcomers... > Sam > > Sam Ravnborg (7): > atmel-hlcdc: renamed directory to drm/atmel/ > dt-binding: add bindings for Atmel LCDC mfd > mfd: add atmel-lcdc driver > dt-bindings: add bindings for Atmel LCDC pwm > pwm: add pwm-atmel-lcdc driver > dt-bindings: add bindings for Atmel lcdc-display-controller > drm: add Atmel LCDC display controller support > > .../display/atmel/lcdc-display-controller.txt | 40 + > .../devicetree/bindings/mfd/atmel-lcdc.txt | 75 ++ > .../devicetree/bindings/pwm/atmel-lcdc-pwm.txt | 30 + > MAINTAINERS | 9 +- > drivers/gpu/drm/Kconfig | 2 +- > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/atmel-hlcdc/Kconfig | 10 - > drivers/gpu/drm/atmel/Kconfig | 28 + > drivers/gpu/drm/{atmel-hlcdc => atmel}/Makefile | 2 + > .../drm/{atmel-hlcdc => atmel}/atmel_hlcdc_crtc.c | 0 > .../drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.c | 0 > .../drm/{atmel-hlcdc => atmel}/atmel_hlcdc_dc.h | 0 > .../{atmel-hlcdc => atmel}/atmel_hlcdc_output.c | 0 > .../drm/{atmel-hlcdc => atmel}/atmel_hlcdc_plane.c | 0 > drivers/gpu/drm/atmel/atmel_lcdc-dc.c | 1094 ++++++++++++++++++++ > drivers/mfd/Kconfig | 10 + > drivers/mfd/Makefile | 1 + > drivers/mfd/atmel-lcdc.c | 158 +++ > drivers/pwm/Kconfig | 13 + > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-atmel-lcdc.c | 178 ++++ > include/linux/mfd/atmel-lcdc.h | 184 ++++ > 22 files changed, 1824 insertions(+), 13 deletions(-) > -- Nicolas Ferre ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-13 15:54 ` Nicolas Ferre @ 2018-08-13 18:18 ` Sam Ravnborg 2018-08-13 22:04 ` Rob Herring 2018-08-14 14:36 ` Alexandre Belloni 0 siblings, 2 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-13 18:18 UTC (permalink / raw) To: Nicolas Ferre Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Joshua Henderson, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel Hi Nicholas. On Mon, Aug 13, 2018 at 05:54:48PM +0200, Nicolas Ferre wrote: > On 12/08/2018 at 20:41, Sam Ravnborg wrote: > >New DRM based driver for at91sam9 SOC's that uses the > >Atmel LCDC IP core. > > I'm delighted to see this work: Thanks a lot Sam! Glad to hear. I was a bit worried that the response would be "this is waste of time as we have a working driver already". > > >This is first version of a patch set that adds > >drivers for the Atmel LCDC IP core. > >Posted for review as the basics works now. > > > >The LCDC IP core contains two devices: > >- a PWM often used for backlight > >- a LCD display controller > > > >Both devices are supported today by the atmel_lcdfb driver. > >For this new set of drivers the compatible strings was > >selected to avoid clash with the existing compatible > >strings used for the atmel_lcdfb driver to allow them > >to co-exist. > > Would be good to have a plan to phase-out the old atmel_lcdfb fbdev > driver when this one addresses some TODO items that make sense. Agree on this. One approach could be to say that when all in-kernel users of atmel_lcdfb are ported, then the old driver could be dropped after a kernel release. > The mfd suffix seems strange to me. What about "atmel,at91sam9263-lcdc-mfd" > => "atmel,at91sam9263-lcd" (or "microchip,at91sam9263-lcdc"). The "-mfd" suffix was added to avoid clashing with the current compatible string used by the atmel_lcdfb driver. I susggest we do the following: - use the microchip prefix, as this is now owned by microchip - and add the driver to a drm/microchip/ directory (Then we can only hope that microchip do not change name or are purchased by someone else). > > >The DRM implementation has a few shortcomings compared to the > >existing fbdev based driver: > > - STN displays are not supported > > Binding support is missing but most of the > > STN specific functionality is otherwise ported > > from the fbdev driver. > > I assume the info should come from the panel > > but as I lack HW I have not looked too much > > into what is required. > > Yes, I'm not even sure if STN displays are still available those > days... Might not be worth it spending time on this. Then I will delete all the STN bits that was ported over. If we need them later they can be found on the mailign list. > > > - gamma support is missing > > The driver utilises drm_simple_kms_helper and > > this helper lacks support for setting up gamma. > > If this is useful please let me know and I > > will extend drm_simple_kms_helper to support this > > and update the driver. > > - modesetting is not checked (see TODO in file) > > Is this required for such a simple setup? > > - support for extra modes as applicable (and lcd-wiring-mode) > > - support for AVR32 (is it relevant?) > > No, AVR32 is not relevant anymore as the core and SoC were removed > some years ago from Linux. > All mention of AT32 or AVR32 can be remove then. Ok, I will delete these. > >The drivers _works_ on a proprietary at91sam9263 based board > >and I will eventually migrate the at91sam9263ek over > >to use it as well. > > We'll be able to test it on other SoCs and boards. Thanks! > > >Works is maybe a stretch - the following was tested: > > modetest shows reasonable output > > modetest -s shows some nice colored squares > > You must see something like this (without the overlay additional > black square): > http://www.at91.com/linux4sam/bin/view/Linux4SAM/UsingAtmelDRMDriver#Some_modetest_commands I posted a picture to G+ here: https://plus.google.com/+SamRavnborg/posts/YBt4jUGLgWa They look similar but are not equal. For now I assume this is OK. I will do some testing with a Qt app, and will test colors with this too. > >All feedback (from spelling errors to general structure) appreciated! > > On my side, it's mostly Nitpicking ;-) > Now that we're Microchip for a little bit more than 2 years, I tend > to make this name prevalent compared to Atmel for new work around > our products... But I know this driver is for older SoCs and that it > got inspired by two former drivers that have this "atmel" naming > everywhere. MAINTAINERS and Kconfig changes could include Microchip > name, so that it's obvious for the newcomers... Will fix, as described above. Thanks for the inputs! Sam ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-13 18:18 ` Sam Ravnborg @ 2018-08-13 22:04 ` Rob Herring 2018-08-14 16:43 ` Sam Ravnborg 2018-08-14 14:36 ` Alexandre Belloni 1 sibling, 1 reply; 50+ messages in thread From: Rob Herring @ 2018-08-13 22:04 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Joshua Henderson, Nicolas Ferre, dri-devel, Boris Brezillon, Lee Jones, linux-arm-kernel On Mon, Aug 13, 2018 at 08:18:08PM +0200, Sam Ravnborg wrote: > Hi Nicholas. > > On Mon, Aug 13, 2018 at 05:54:48PM +0200, Nicolas Ferre wrote: > > On 12/08/2018 at 20:41, Sam Ravnborg wrote: > > >New DRM based driver for at91sam9 SOC's that uses the > > >Atmel LCDC IP core. > > > > I'm delighted to see this work: Thanks a lot Sam! > Glad to hear. > I was a bit worried that the response would be "this is > waste of time as we have a working driver already". > > > > > >This is first version of a patch set that adds > > >drivers for the Atmel LCDC IP core. > > >Posted for review as the basics works now. > > > > > >The LCDC IP core contains two devices: > > >- a PWM often used for backlight > > >- a LCD display controller > > > > > >Both devices are supported today by the atmel_lcdfb driver. > > >For this new set of drivers the compatible strings was > > >selected to avoid clash with the existing compatible > > >strings used for the atmel_lcdfb driver to allow them > > >to co-exist. > > > > Would be good to have a plan to phase-out the old atmel_lcdfb fbdev > > driver when this one addresses some TODO items that make sense. > Agree on this. > One approach could be to say that when all in-kernel users of atmel_lcdfb > are ported, then the old driver could be dropped after a kernel release. > > > The mfd suffix seems strange to me. What about "atmel,at91sam9263-lcdc-mfd" > > => "atmel,at91sam9263-lcd" (or "microchip,at91sam9263-lcdc"). > The "-mfd" suffix was added to avoid clashing with the current > compatible string used by the atmel_lcdfb driver. I don't know that 2 registers for a backlight PWM constitute an MFD. A single node can be both an LCD controller and a PWM. The fact that the OS has 2 drivers is irrelevant to the binding and DT is not a way to select drivers. Your choice with Linux is either kconfig or manual module loading. > I susggest we do the following: > - use the microchip prefix, as this is now owned by microchip > - and add the driver to a drm/microchip/ directory > (Then we can only hope that microchip do not change name or > are purchased by someone else). I would not do that. Then you have a dts with a mixture based on when you got around to writing each binding. i.MX has continued using 'fsl' even on new chips today. Probably a good choice had the QCom acq had gone thru. Rob _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-13 22:04 ` Rob Herring @ 2018-08-14 16:43 ` Sam Ravnborg 2018-08-14 22:42 ` Rob Herring 0 siblings, 1 reply; 50+ messages in thread From: Sam Ravnborg @ 2018-08-14 16:43 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Joshua Henderson, dri-devel, Boris Brezillon, Lee Jones, linux-arm-kernel Hi Rob. > I don't know that 2 registers for a backlight PWM constitute an MFD. A > single node can be both an LCD controller and a PWM. Current suggestion from v1 patchset looks like this: lcdc0: lcdc@700000 { compatible = "atmel,at91sam9263-lcdc-mfd"; reg = <0x700000 0x1000>; interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; clocks = <&lcd_clk>, <&lcd_clk>; clock-names = "lcdc_clk", "hclk"; lcdc-display-controller { compatible = "atmel,lcdc-display-controller"; lcd-supply = <&lcdc_reg>; port@0 { }; }; lcdc_pwm: lcdc-pwm { compatible = "atmel,lcdc-pwm"; #pwm-cells = <3>; }; }; backlight: backlight { compatible = "pwm-backlight"; pwms = <&lcdc_pwm 0 50000 0>; }; We could have described the full binding in one file, rather than in three files like it is done in v1. But the structure was done so it matched what was done for hlcdc. If I understand the proposal from you correct an example binding would look like this: lcdc0: lcdc@700000 { compatible = "atmel,at91sam9263-lcdc"; reg = <0x700000 0x1000>; interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; clocks = <&lcd_clk>, <&lcd_clk>; clock-names = "lcdc_clk", "hclk"; lcd-supply = <&lcdc_reg>; port@0 { }; #pwm-cells = <3>; }; backlight: backlight { compatible = "pwm-backlight"; pwms = <&lcd0 0 50000 0>; }; This is doable, but IMO it is less obvious that the LCDC IP core implements two different features - a PWM and a LCD controller. Right now the preference is to stay with the v1 approach: - It is a mirror of what we do today for hlcdc, so no suprises - It shows in a nice way that the LCDC IP core implments both an LCD controller and a PWM - The pwm functionality is not hiddin inside the lcdc stuff, and it is thus simpler to add good pin-ctrl handles with nice names that matches the usage. (I could always add more pin-ctrl, but it is common to refer to a single pin-ctrl. One DT related Q: The LCD Controller supports BGR565, but as this is less common some HW implmentations exchange R and B, expessed in the old binding as wiring-mode like this: atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG" How can we express this wiring-mode in a generic way, both in DT and in code? Is it something that in DRM belongs to the panel, the encoder, the connector, or? And can any of the exisitng flags be used? Thanks in advance, Sam ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-14 16:43 ` Sam Ravnborg @ 2018-08-14 22:42 ` Rob Herring 2018-08-15 4:48 ` Sam Ravnborg 0 siblings, 1 reply; 50+ messages in thread From: Rob Herring @ 2018-08-14 22:42 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Joshua Henderson, Nicolas Ferre, dri-devel, Boris Brezillon, Lee Jones, linux-arm-kernel On Tue, Aug 14, 2018 at 06:43:43PM +0200, Sam Ravnborg wrote: > Hi Rob. > > > I don't know that 2 registers for a backlight PWM constitute an MFD. A > > single node can be both an LCD controller and a PWM. > > Current suggestion from v1 patchset looks like this: > lcdc0: lcdc@700000 { > compatible = "atmel,at91sam9263-lcdc-mfd"; > reg = <0x700000 0x1000>; > interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; > clocks = <&lcd_clk>, <&lcd_clk>; > clock-names = "lcdc_clk", "hclk"; > > lcdc-display-controller { > compatible = "atmel,lcdc-display-controller"; > lcd-supply = <&lcdc_reg>; > > port@0 { > }; > }; > > lcdc_pwm: lcdc-pwm { > compatible = "atmel,lcdc-pwm"; > #pwm-cells = <3>; > }; > > }; > > backlight: backlight { > compatible = "pwm-backlight"; > pwms = <&lcdc_pwm 0 50000 0>; > }; > > > We could have described the full binding in one file, rather than in > three files like it is done in v1. But the structure was done so it matched > what was done for hlcdc. > > > If I understand the proposal from you correct an example binding would > look like this: > > lcdc0: lcdc@700000 { > compatible = "atmel,at91sam9263-lcdc"; > reg = <0x700000 0x1000>; > interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; > clocks = <&lcd_clk>, <&lcd_clk>; > clock-names = "lcdc_clk", "hclk"; > > lcd-supply = <&lcdc_reg>; > > port@0 { > }; > > #pwm-cells = <3>; > }; > > backlight: backlight { > compatible = "pwm-backlight"; > pwms = <&lcd0 0 50000 0>; > }; > > > This is doable, but IMO it is less obvious that the LCDC IP core implements > two different features - a PWM and a LCD controller. Features don't equate to nodes. If the sub-blocks can be separately instantiated or have their own resources then sub-nodes make sense. Otherwise, it's a single device (node) with multiple providers. > Right now the preference is to stay with the v1 approach: > - It is a mirror of what we do today for hlcdc, so no suprises Which BTW doesn't appear to have actually been reviewed. Do these IP blocks actually share anything? Consistency is nice, but keeping compatibility with the existing binding by extending things in a backwards compatible way is more important. Implementing a new driver is not license to change the binding. Shall I let u-boot or *BSD developers change the binding too for their driver? > - It shows in a nice way that the LCDC IP core implments both an LCD controller and a PWM > - The pwm functionality is not hiddin inside the lcdc stuff, and it is thus > simpler to add good pin-ctrl handles with nice names that matches the usage. > (I could always add more pin-ctrl, but it is common to refer to a single pin-ctrl. Does anyone actually use this for a non-backlight PWM? I'm not sure the abstraction is worth it. The driver could just register itself as a backlight provider rather than a PWM provider. > One DT related Q: > The LCD Controller supports BGR565, but as this is less common some HW implmentations > exchange R and B, expessed in the old binding as wiring-mode like this: > > atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG" > > How can we express this wiring-mode in a generic way, both in DT and in code? > Is it something that in DRM belongs to the panel, the encoder, the connector, or? > And can any of the exisitng flags be used? I thought we had come up with a common definition, but I guess it didn't make it upstream. It's definitely needed and I've been rejecting anything new that's vendor specific. Rob _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-14 22:42 ` Rob Herring @ 2018-08-15 4:48 ` Sam Ravnborg 2018-08-15 14:45 ` Rob Herring 0 siblings, 1 reply; 50+ messages in thread From: Sam Ravnborg @ 2018-08-15 4:48 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Joshua Henderson, dri-devel, Boris Brezillon, Lee Jones, linux-arm-kernel Hi Rob. Thanks for the feedback, as I am rather new to this DT stuff a few iterations are no suprise. > > If I understand the proposal from you correct an example binding would > > look like this: > > > > lcdc0: lcdc@700000 { > > compatible = "atmel,at91sam9263-lcdc"; > > reg = <0x700000 0x1000>; > > interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>; > > clocks = <&lcd_clk>, <&lcd_clk>; > > clock-names = "lcdc_clk", "hclk"; > > > > lcd-supply = <&lcdc_reg>; > > > > port@0 { > > }; > > > > #pwm-cells = <3>; > > }; > > > > backlight: backlight { > > compatible = "pwm-backlight"; > > pwms = <&lcd0 0 50000 0>; > > }; > > > > > > This is doable, but IMO it is less obvious that the LCDC IP core implements > > two different features - a PWM and a LCD controller. > > Features don't equate to nodes. If the sub-blocks can be separately > instantiated or have their own resources then sub-nodes make sense. > Otherwise, it's a single device (node) with multiple providers. > > > Right now the preference is to stay with the v1 approach: > > - It is a mirror of what we do today for hlcdc, so no suprises > > Which BTW doesn't appear to have actually been reviewed. > > Do these IP blocks actually share anything? They have registers in the same memory area - and I think that it. > Consistency is nice, but keeping compatibility with the existing binding > by extending things in a backwards compatible way is more important. > Implementing a new driver is not license to change the binding. Shall I > let u-boot or *BSD developers change the binding too for their driver? So in other words - the existing binding in atmel,lcdc.txt needs to be extended in a backward compatible way. I will take a look at that and post a proposal in v2. > > - It shows in a nice way that the LCDC IP core implments both an LCD controller and a PWM > > - The pwm functionality is not hiddin inside the lcdc stuff, and it is thus > > simpler to add good pin-ctrl handles with nice names that matches the usage. > > (I could always add more pin-ctrl, but it is common to refer to a single pin-ctrl. > > Does anyone actually use this for a non-backlight PWM? I'm not sure the > abstraction is worth it. The driver could just register itself as a > backlight provider rather than a PWM provider. Makes sense. > > > One DT related Q: > > The LCD Controller supports BGR565, but as this is less common some HW implmentations > > exchange R and B, expessed in the old binding as wiring-mode like this: > > > > atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG" > > > > How can we express this wiring-mode in a generic way, both in DT and in code? > > Is it something that in DRM belongs to the panel, the encoder, the connector, or? > > And can any of the exisitng flags be used? > > I thought we had come up with a common definition, but I guess it didn't > make it upstream. It's definitely needed and I've been rejecting > anything new that's vendor specific. I found this: https://patchwork.ozlabs.org/patch/659570/ The suggestion with a boolean seems simple and I will try that. Sam ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-15 4:48 ` Sam Ravnborg @ 2018-08-15 14:45 ` Rob Herring 2018-08-15 15:04 ` Daniel Vetter 0 siblings, 1 reply; 50+ messages in thread From: Rob Herring @ 2018-08-15 14:45 UTC (permalink / raw) To: Sam Ravnborg, Peter Rosin Cc: Mark Rutland, devicetree, Alexandre Belloni, Linux PWM List, Joshua Henderson, Nicolas Ferre, dri-devel, Boris Brezillon, Lee Jones, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Tue, Aug 14, 2018 at 10:48 PM Sam Ravnborg <sam@ravnborg.org> wrote: > [...] > > > One DT related Q: > > > The LCD Controller supports BGR565, but as this is less common some HW implmentations > > > exchange R and B, expessed in the old binding as wiring-mode like this: > > > > > > atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG" > > > > > > How can we express this wiring-mode in a generic way, both in DT and in code? > > > Is it something that in DRM belongs to the panel, the encoder, the connector, or? > > > And can any of the exisitng flags be used? > > > > I thought we had come up with a common definition, but I guess it didn't > > make it upstream. It's definitely needed and I've been rejecting > > anything new that's vendor specific. > I found this: https://patchwork.ozlabs.org/patch/659570/ > The suggestion with a boolean seems simple and I will try that. That's really too simple to be common. There's been a few other attempts[1][2][3] (for Atmel HLCDC in fact) with [3] probably being the closest to merging. The binding looked fine to me, but looks like the DRM implementation had some issues. Rob [1] https://lists.freedesktop.org/archives/dri-devel/2018-April/173089.html [2] https://patchwork.kernel.org/patch/9965079/ [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/568731.html _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-15 14:45 ` Rob Herring @ 2018-08-15 15:04 ` Daniel Vetter 2018-08-15 15:41 ` Peter Rosin 0 siblings, 1 reply; 50+ messages in thread From: Daniel Vetter @ 2018-08-15 15:04 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, devicetree, Alexandre Belloni, Linux PWM List, Lee Jones, Joshua Henderson, Nicolas Ferre, dri-devel, Boris Brezillon, Sam Ravnborg, Peter Rosin, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Wed, Aug 15, 2018 at 4:45 PM, Rob Herring <robh@kernel.org> wrote: > On Tue, Aug 14, 2018 at 10:48 PM Sam Ravnborg <sam@ravnborg.org> wrote: >> > > [...] > >> > > One DT related Q: >> > > The LCD Controller supports BGR565, but as this is less common some HW implmentations >> > > exchange R and B, expessed in the old binding as wiring-mode like this: >> > > >> > > atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG" >> > > >> > > How can we express this wiring-mode in a generic way, both in DT and in code? >> > > Is it something that in DRM belongs to the panel, the encoder, the connector, or? >> > > And can any of the exisitng flags be used? >> > >> > I thought we had come up with a common definition, but I guess it didn't >> > make it upstream. It's definitely needed and I've been rejecting >> > anything new that's vendor specific. >> I found this: https://patchwork.ozlabs.org/patch/659570/ >> The suggestion with a boolean seems simple and I will try that. > > That's really too simple to be common. There's been a few other > attempts[1][2][3] (for Atmel HLCDC in fact) with [3] probably being > the closest to merging. The binding looked fine to me, but looks like > the DRM implementation had some issues. > > Rob > > [1] https://lists.freedesktop.org/archives/dri-devel/2018-April/173089.html > [2] https://patchwork.kernel.org/patch/9965079/ > [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/568731.html There's a standardized bus_format for drm_panel. IIrc there's been discussions to add something similar to drm_bridge, but didn't go all that far for reasons. Lots of drivers just handle this internally in some way. So no idea why this all got stalled. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-15 15:04 ` Daniel Vetter @ 2018-08-15 15:41 ` Peter Rosin 2018-08-15 20:48 ` Sam Ravnborg 0 siblings, 1 reply; 50+ messages in thread From: Peter Rosin @ 2018-08-15 15:41 UTC (permalink / raw) To: Daniel Vetter, Rob Herring Cc: Mark Rutland, devicetree, Alexandre Belloni, Linux PWM List, Lee Jones, Joshua Henderson, dri-devel, Boris Brezillon, Sam Ravnborg, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On 2018-08-15 17:04, Daniel Vetter wrote: > On Wed, Aug 15, 2018 at 4:45 PM, Rob Herring <robh@kernel.org> wrote: >> On Tue, Aug 14, 2018 at 10:48 PM Sam Ravnborg <sam@ravnborg.org> wrote: >>> >> >> [...] >> >>>>> One DT related Q: >>>>> The LCD Controller supports BGR565, but as this is less common some HW implmentations >>>>> exchange R and B, expessed in the old binding as wiring-mode like this: >>>>> >>>>> atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG" >>>>> >>>>> How can we express this wiring-mode in a generic way, both in DT and in code? >>>>> Is it something that in DRM belongs to the panel, the encoder, the connector, or? >>>>> And can any of the exisitng flags be used? >>>> >>>> I thought we had come up with a common definition, but I guess it didn't >>>> make it upstream. It's definitely needed and I've been rejecting >>>> anything new that's vendor specific. >>> I found this: https://patchwork.ozlabs.org/patch/659570/ >>> The suggestion with a boolean seems simple and I will try that. >> >> That's really too simple to be common. There's been a few other >> attempts[1][2][3] (for Atmel HLCDC in fact) with [3] probably being >> the closest to merging. The binding looked fine to me, but looks like >> the DRM implementation had some issues. >> >> Rob >> >> [1] https://lists.freedesktop.org/archives/dri-devel/2018-April/173089.html >> [2] https://patchwork.kernel.org/patch/9965079/ >> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/568731.html > > There's a standardized bus_format for drm_panel. IIrc there's been > discussions to add something similar to drm_bridge, but didn't go all > that far for reasons. > > Lots of drivers just handle this internally in some way. So no idea > why this all got stalled. I stopped pushing that patch for reasons mentioned in https://lkml.org/lkml/2018/4/9/108 However, since then, the component approach mentioned in that mail was shot down and instead the tda998x driver is now a bridge (or soon, I expect the series from Russell King that to land in 4.19-rc1) which means that the binding from that series is back on the table. At least I guess so? However, in that series the approach is that the bridge states its expected input and the output then adjusts to what is needed. However, the "problem" is really in the atmel-hlcdc output (it moves the MSB for the RGB colors around depending on the output mode) so I no longer subscribe to all ideas in that series and think it is cleaner to state the needed bus format closer to the atmel- hlcdc endpoint as is done e.g. in this latest series: https://lkml.org/lkml/2018/8/10/309 This latest series uses the media/video-interface approach and specifies the bus-width in the endpoint. However, the bus-width is alone obviously not enough to differentiate rgb565 and brg565, so will not help Sam (or is it bgr565 that Sam needs? The above quoted text is ambiguous). I think something like my bus-format binding in [3] is generic enough to also help Sam. Cheers, Peter ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-15 15:41 ` Peter Rosin @ 2018-08-15 20:48 ` Sam Ravnborg 0 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-15 20:48 UTC (permalink / raw) To: Peter Rosin Cc: Mark Rutland, Rob Herring, Alexandre Belloni, Linux PWM List, Joshua Henderson, dri-devel, Boris Brezillon, devicetree, Daniel Vetter, Lee Jones, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE Hi Peter. > > I stopped pushing that patch for reasons mentioned in > https://lkml.org/lkml/2018/4/9/108 > > However, since then, the component approach mentioned in that mail was > shot down and instead the tda998x driver is now a bridge (or soon, I expect > the series from Russell King that to land in 4.19-rc1) which means that the > binding from that series is back on the table. At least I guess so? However, > in that series the approach is that the bridge states its expected input and > the output then adjusts to what is needed. However, the "problem" is really in > the atmel-hlcdc output (it moves the MSB for the RGB colors around depending > on the output mode) so I no longer subscribe to all ideas in that series > and think it is cleaner to state the needed bus format closer to the atmel- > hlcdc endpoint as is done e.g. in this latest series: > > https://lkml.org/lkml/2018/8/10/309 > > This latest series uses the media/video-interface approach and specifies > the bus-width in the endpoint. However, the bus-width is alone obviously not > enough to differentiate rgb565 and brg565, so will not help Sam Some digging in diverse threads required. I may take the easy way in v2 and postpone this feature. But you all confirmed that doing something locally in the driver is the wrong approach so that is ruled out. > (or is it bgr565 that Sam needs? The above quoted text is ambiguous). My bad. I need either rgb565 or bgr565 (at lest this is how I understood the application note from Atmel/Microchip - need to re-read this too). Some of the IP cores have an intensify bit which I do not yet understand how to adapt to the supported formats. This is also on the todo list. > I think something like my bus-format binding in [3] is generic enough to > also help Sam. Thanks, if I find time I will look into this, but first priority is to get the bindinns correct, and then the overall structure of the driver. Sam ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-13 18:18 ` Sam Ravnborg 2018-08-13 22:04 ` Rob Herring @ 2018-08-14 14:36 ` Alexandre Belloni 2018-08-14 16:16 ` Sam Ravnborg 1 sibling, 1 reply; 50+ messages in thread From: Alexandre Belloni @ 2018-08-14 14:36 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, linux-pwm, Joshua Henderson, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel On 13/08/2018 20:18:08+0200, Sam Ravnborg wrote: > > Would be good to have a plan to phase-out the old atmel_lcdfb fbdev > > driver when this one addresses some TODO items that make sense. > Agree on this. > One approach could be to say that when all in-kernel users of atmel_lcdfb > are ported, then the old driver could be dropped after a kernel release. > I would drop it only after an LTS release. > > The mfd suffix seems strange to me. What about "atmel,at91sam9263-lcdc-mfd" > > => "atmel,at91sam9263-lcd" (or "microchip,at91sam9263-lcdc"). > The "-mfd" suffix was added to avoid clashing with the current > compatible string used by the atmel_lcdfb driver. > > I susggest we do the following: > - use the microchip prefix, as this is now owned by microchip > - and add the driver to a drm/microchip/ directory > (Then we can only hope that microchip do not change name or > are purchased by someone else). > The compatible string should remain the same but the drivers have to be mutually exclusive in Kconfig. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-14 14:36 ` Alexandre Belloni @ 2018-08-14 16:16 ` Sam Ravnborg 0 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-14 16:16 UTC (permalink / raw) To: Alexandre Belloni Cc: Mark Rutland, devicetree, linux-pwm, Joshua Henderson, dri-devel, Boris Brezillon, Rob Herring, Lee Jones, linux-arm-kernel On Tue, Aug 14, 2018 at 04:36:03PM +0200, Alexandre Belloni wrote: > On 13/08/2018 20:18:08+0200, Sam Ravnborg wrote: > > > Would be good to have a plan to phase-out the old atmel_lcdfb fbdev > > > driver when this one addresses some TODO items that make sense. > > Agree on this. > > One approach could be to say that when all in-kernel users of atmel_lcdfb > > are ported, then the old driver could be dropped after a kernel release. > > > > I would drop it only after an LTS release. Much better, agreed. > > > > The mfd suffix seems strange to me. What about "atmel,at91sam9263-lcdc-mfd" > > > => "atmel,at91sam9263-lcd" (or "microchip,at91sam9263-lcdc"). > > The "-mfd" suffix was added to avoid clashing with the current > > compatible string used by the atmel_lcdfb driver. > > > > I susggest we do the following: > > - use the microchip prefix, as this is now owned by microchip > > - and add the driver to a drm/microchip/ directory > > (Then we can only hope that microchip do not change name or > > are purchased by someone else). > > > > The compatible string should remain the same but the drivers have to be > mutually exclusive in Kconfig. OK, will do so in v2. I had planned to keep both in the DT file but then I will just replace one with the other. Sam ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg ` (8 preceding siblings ...) 2018-08-13 15:54 ` Nicolas Ferre @ 2018-08-24 8:22 ` Boris Brezillon 2018-08-24 15:52 ` Sam Ravnborg 9 siblings, 1 reply; 50+ messages in thread From: Boris Brezillon @ 2018-08-24 8:22 UTC (permalink / raw) To: Sam Ravnborg Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Nicolas Ferre, dri-devel, Rob Herring, Lee Jones, linux-arm-kernel Hi Sam, On Sun, 12 Aug 2018 20:41:52 +0200 Sam Ravnborg <sam@ravnborg.org> wrote: > New DRM based driver for at91sam9 SOC's that uses the > Atmel LCDC IP core. First of all, thanks for this contribution. > > This is first version of a patch set that adds > drivers for the Atmel LCDC IP core. > Posted for review as the basics works now. > > The LCDC IP core contains two devices: > - a PWM often used for backlight > - a LCD display controller > > Both devices are supported today by the atmel_lcdfb driver. > For this new set of drivers the compatible strings was > selected to avoid clash with the existing compatible > strings used for the atmel_lcdfb driver to allow them > to co-exist. Hm, I think Rob commented on that already, but we usually try to stay compatible with the exisiting/old bindings when introducing a new one. Don't know how feasible this is in this particular case though. > > This patchset implements three drivers. > - A MFD driver that include the generic parts. > - A PWM driver. > - A DRM display controller driver. > This is the same split as used for the Atmel hlcdc IP. > > The hlcdc and lcdc has only a few things in common and > trying to share the code for them was not a viable solution. > > The DRM implementation has a few shortcomings compared to the > existing fbdev based driver: > - STN displays are not supported > Binding support is missing but most of the > STN specific functionality is otherwise ported > from the fbdev driver. > I assume the info should come from the panel > but as I lack HW I have not looked too much > into what is required. > - gamma support is missing > The driver utilises drm_simple_kms_helper and > this helper lacks support for setting up gamma. > If this is useful please let me know and I > will extend drm_simple_kms_helper to support this > and update the driver. I guess you can skip that for now. > - modesetting is not checked (see TODO in file) > Is this required for such a simple setup? Well, that's always better if you can check that the requested display mode is supported before trying to apply it. > - support for extra modes as applicable (and lcd-wiring-mode) Peter already suggested something I think. > - support for AVR32 (is it relevant?) It is, AVR32 is no longer supported in mainline. > > The first patch renames .../drm/atmel-hlcdc to .../drm/atmel > to have a nice home for both drivers. Sounds good. Regards, Boris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver 2018-08-24 8:22 ` Boris Brezillon @ 2018-08-24 15:52 ` Sam Ravnborg 0 siblings, 0 replies; 50+ messages in thread From: Sam Ravnborg @ 2018-08-24 15:52 UTC (permalink / raw) To: Boris Brezillon Cc: Mark Rutland, devicetree, Alexandre Belloni, linux-pwm, Nicolas Ferre, dri-devel, Rob Herring, Lee Jones, linux-arm-kernel Hi Boris. > > > > Both devices are supported today by the atmel_lcdfb driver. > > For this new set of drivers the compatible strings was > > selected to avoid clash with the existing compatible > > strings used for the atmel_lcdfb driver to allow them > > to co-exist. > > Hm, I think Rob commented on that already, but we usually try to stay > compatible with the exisiting/old bindings when introducing a new one. > Don't know how feasible this is in this particular case though. I v2 I am working with a backward compatible approach. This is better that what I cam up with initially. > > The DRM implementation has a few shortcomings compared to the > > existing fbdev based driver: > > - STN displays are not supported > > Binding support is missing but most of the > > STN specific functionality is otherwise ported > > from the fbdev driver. > > I assume the info should come from the panel > > but as I lack HW I have not looked too much > > into what is required. > > - gamma support is missing > > The driver utilises drm_simple_kms_helper and > > this helper lacks support for setting up gamma. > > If this is useful please let me know and I > > will extend drm_simple_kms_helper to support this > > and update the driver. > > I guess you can skip that for now. Also based on feedback from Nicholas the STN parts will be dropped in v2. For the gamma stuff this looks feasible with a small extension to drm_simple_kms_helper - but I dunno if this is something that userspace will actually use. So that will wait until there is some good reason to implement it, and I know how to test it too. > > > - modesetting is not checked (see TODO in file) > > Is this required for such a simple setup? > > Well, that's always better if you can check that the requested display > mode is supported before trying to apply it. I will try to cook up something, have learned a little since posting v1. > > > - support for extra modes as applicable (and lcd-wiring-mode) > > Peter already suggested something I think. Yep, plenty of links to read. Sam _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
end of thread, other threads:[~2018-08-26 18:41 UTC | newest] Thread overview: 50+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-12 18:41 [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 1/7] atmel-hlcdc: renamed directory to drm/atmel/ Sam Ravnborg 2018-08-14 8:39 ` Daniel Vetter 2018-08-14 16:19 ` Sam Ravnborg 2018-08-16 7:41 ` Daniel Vetter 2018-08-22 20:09 ` Sam Ravnborg 2018-08-22 20:22 ` Daniel Vetter 2018-08-24 8:28 ` Boris Brezillon 2018-08-24 15:43 ` Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 2/7] dt-binding: add bindings for Atmel LCDC mfd Sam Ravnborg 2018-08-24 8:45 ` Boris Brezillon 2018-08-24 15:58 ` Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 3/7] mfd: add atmel-lcdc driver Sam Ravnborg 2018-08-14 11:09 ` kbuild test robot 2018-08-15 5:24 ` Lee Jones 2018-08-15 20:40 ` Sam Ravnborg 2018-08-16 8:28 ` Nicolas Ferre 2018-08-16 8:42 ` Lee Jones 2018-08-24 8:37 ` Boris Brezillon 2018-08-24 8:15 ` Boris Brezillon 2018-08-24 10:58 ` Lee Jones 2018-08-15 8:11 ` kbuild test robot 2018-08-24 8:48 ` Boris Brezillon 2018-08-12 18:46 ` [PATCH v1 4/7] dt-bindings: add bindings for Atmel LCDC pwm Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 5/7] pwm: add pwm-atmel-lcdc driver Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 6/7] dt-bindings: add bindings for Atmel lcdc-display-controller Sam Ravnborg 2018-08-12 18:46 ` [PATCH v1 7/7] drm: add Atmel LCDC display controller support Sam Ravnborg 2018-08-24 12:31 ` Boris Brezillon 2018-08-26 18:41 ` Sam Ravnborg 2018-08-26 14:28 ` Noralf Trønnes 2018-08-26 14:58 ` Sam Ravnborg 2018-08-12 19:55 ` [RFC PATCH 0/7] add at91sam9 LCDC DRM driver Sam Ravnborg 2018-08-13 14:47 ` Nicolas Ferre 2018-08-14 8:41 ` Daniel Vetter 2018-08-22 20:12 ` Sam Ravnborg 2018-08-23 6:16 ` Daniel Vetter 2018-08-13 15:54 ` Nicolas Ferre 2018-08-13 18:18 ` Sam Ravnborg 2018-08-13 22:04 ` Rob Herring 2018-08-14 16:43 ` Sam Ravnborg 2018-08-14 22:42 ` Rob Herring 2018-08-15 4:48 ` Sam Ravnborg 2018-08-15 14:45 ` Rob Herring 2018-08-15 15:04 ` Daniel Vetter 2018-08-15 15:41 ` Peter Rosin 2018-08-15 20:48 ` Sam Ravnborg 2018-08-14 14:36 ` Alexandre Belloni 2018-08-14 16:16 ` Sam Ravnborg 2018-08-24 8:22 ` Boris Brezillon 2018-08-24 15:52 ` Sam Ravnborg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).