Hi Thomas,
Thank you for the review.
On Wed, Nov 29, 2023 at 05:21:19PM +0100, Thomas Zimmermann wrote:
> Hi,
>
> thanks for the patches.
>
> Am 29.11.23 um 15:29 schrieb Mehdi Djait:
> > Introduce a DRM driver for the sharp LS027B7DH01 Memory LCD.
> >
> > LS027B7DH01 is a 2.7" 400x240 monochrome display connected to a SPI bus.
> > The drivers implements the Multiple Lines Data Update Mode.
> > External COM inversion is enabled using a PWM signal as input.
> >
> > Signed-off-by: Mehdi Djait <mehdi.djait@xxxxxxxxxxx>
> > ---
> > MAINTAINERS | 7 +
> > drivers/gpu/drm/tiny/Kconfig | 8 +
> > drivers/gpu/drm/tiny/Makefile | 1 +
> > drivers/gpu/drm/tiny/sharp-ls027b7dh01.c | 411 +++++++++++++++++++++++
> > 4 files changed, 427 insertions(+)
> > create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 012df8ccf34e..fb859698bd3d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6832,6 +6832,13 @@ S: Maintained
> > F: Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
> > F: drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
> > +DRM DRIVER FOR SHARP LS027B7DH01 Memory LCD
> > +M: Mehdi Djait <mehdi.djait@xxxxxxxxxxx>
> > +S: Maintained
> > +T: git git://anongit.freedesktop.org/drm/drm-misc
> > +F: Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
> > +F: drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> > +
> > DRM DRIVER FOR SITRONIX ST7586 PANELS
> > M: David Lechner <david@xxxxxxxxxxxxxx>
> > S: Maintained
> > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> > index f6889f649bc1..a2ade06403ca 100644
> > --- a/drivers/gpu/drm/tiny/Kconfig
> > +++ b/drivers/gpu/drm/tiny/Kconfig
> > @@ -186,6 +186,14 @@ config TINYDRM_REPAPER
> > If M is selected the module will be called repaper.
> > +config TINYDRM_SHARP_LS027B7DH01
> > + tristate "DRM support for SHARP LS027B7DH01 display"
> > + depends on DRM && SPI
> > + select DRM_KMS_HELPER
> > + select DRM_GEM_DMA_HELPER
> > + help
> > + DRM driver for the SHARP LS027B7DD01 LCD display.
> > +
> > config TINYDRM_ST7586
> > tristate "DRM support for Sitronix ST7586 display panels"
> > depends on DRM && SPI
> > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> > index 76dde89a044b..b05df3afb231 100644
> > --- a/drivers/gpu/drm/tiny/Makefile
> > +++ b/drivers/gpu/drm/tiny/Makefile
> > @@ -14,5 +14,6 @@ obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o
> > obj-$(CONFIG_TINYDRM_ILI9486) += ili9486.o
> > obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o
> > obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o
> > +obj-$(CONFIG_TINYDRM_SHARP_LS027B7DH01) += sharp-ls027b7dh01.o
> > obj-$(CONFIG_TINYDRM_ST7586) += st7586.o
> > obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o
> > diff --git a/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> > new file mode 100644
> > index 000000000000..2f58865a5c78
> > --- /dev/null
> > +++ b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> > @@ -0,0 +1,411 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Sharp LS027B7DH01 Memory Display Driver
> > + *
> > + * Copyright (C) 2023 Andrew D'Angelo
> > + * Copyright (C) 2023 Mehdi Djait <mehdi.djait@xxxxxxxxxxx>
> > + *
> > + * The Sharp Memory LCD requires an alternating signal to prevent the buildup of
> > + * a DC bias that would result in a Display that no longer can be updated. Two
> > + * modes for the generation of this signal are supported:
> > + *
> > + * Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
> > + * least once a second to the display.
> > + *
> > + * Hardware, EXTMODE = High: the alternating signal should be supplied on the
> > + * EXTCOMIN pin.
> > + *
> > + * In this driver the Hardware mode is implemented with a PWM signal.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/pwm.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_damage_helper.h>
> > +#include <drm/drm_drv.h>
> > +#include <drm/drm_fbdev_generic.h>
> > +#include <drm/drm_fb_dma_helper.h>
> > +#include <drm/drm_format_helper.h>
> > +#include <drm/drm_framebuffer.h>
> > +#include <drm/drm_gem_atomic_helper.h>
> > +#include <drm/drm_gem_dma_helper.h>
> > +#include <drm/drm_gem_framebuffer_helper.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_probe_helper.h>
> > +#include <drm/drm_simple_kms_helper.h>
> > +
> > +#define CMD_WRITE BIT(0)
> > +#define CMD_CLEAR BIT(2)
> > +
> > +struct sharp_ls027b7dh01 {
> > + struct spi_device *spi;
> > +
> > + struct drm_device drm;
> > + struct drm_connector connector;
> > + struct drm_simple_display_pipe pipe;
>
> Could you please replace the simple pipe and its helpers with regular
> DRMhelpers. It should no tbe used in new drivers. It's an unnecessary
> indirection. Replacing is simple: copy the content of the data structure and
> its helpers into the driver. Maybe clean up the result, if necessary.
Could you please further explain where to copy the helper functions or give me
an example driver ? This is my first DRM driver and grepping did not help me much.
(Sorry for the delayed response)
--
Kind Regards
Mehdi Djait
