linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Anholt <eric@anholt.net>
To: "Noralf Trønnes" <noralf@tronnes.org>,
	dri-devel@lists.freedesktop.org,
	"Rob Herring" <robh+dt@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH 2/3] drm: Add an hx8367d tinydrm driver.
Date: Tue, 30 Oct 2018 15:46:34 -0700	[thread overview]
Message-ID: <87efc7m4s5.fsf@anholt.net> (raw)
In-Reply-To: <a3995168-3d9f-07b3-1e93-7ee5bd14e5a8@tronnes.org>

[-- Attachment #1: Type: text/plain, Size: 4827 bytes --]

Noralf Trønnes <noralf@tronnes.org> writes:

> Den 24.10.2018 20.43, skrev Eric Anholt:
>> I want to sort out support for tinydrm in vc4, so I needed to get a
>> tinydrm-appropriate panel working and this is what I had on hand.
>> This is derived from a combination of ili9341.c from tinydrm and
>> fb_hx8357d.c from staging's fbtft.  The register header is copied
>> directly from staging's fbtft, on the assumption that we will delete
>> that copy later.
>>
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>>   MAINTAINERS                       |   7 +
>>   drivers/gpu/drm/tinydrm/Kconfig   |  11 ++
>>   drivers/gpu/drm/tinydrm/Makefile  |   1 +
>>   drivers/gpu/drm/tinydrm/hx8357d.c | 261 ++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/tinydrm/hx8357d.h |  71 ++++++++
>>   5 files changed, 351 insertions(+)
>>   create mode 100644 drivers/gpu/drm/tinydrm/hx8357d.c
>>   create mode 100644 drivers/gpu/drm/tinydrm/hx8357d.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 39c3f6682ace..e78971e20a11 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4623,6 +4623,13 @@ S:	Maintained
>>   F:	drivers/gpu/drm/tinydrm/ili9225.c
>>   F:	Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>>   
>> +DRM DRIVER FOR HX8357D PANELS
>> +M:	Eric Anholt <eric@anholt.net>
>> +T:	git git://anongit.freedesktop.org/drm/drm-misc
>> +S:	Maintained
>> +F:	drivers/gpu/drm/tinydrm/hx8357d.c
>> +F:	Documentation/devicetree/bindings/display/himax,hx8357d.txt
>> +
>>   DRM DRIVER FOR INTEL I810 VIDEO CARDS
>>   S:	Orphan / Obsolete
>>   F:	drivers/gpu/drm/i810/
>> diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
>> index 16f4b5c91f1b..2c408ac1a900 100644
>> --- a/drivers/gpu/drm/tinydrm/Kconfig
>> +++ b/drivers/gpu/drm/tinydrm/Kconfig
>> @@ -10,6 +10,17 @@ menuconfig DRM_TINYDRM
>>   config TINYDRM_MIPI_DBI
>>   	tristate
>>   
>> +config TINYDRM_HX8357D
>> +	tristate "DRM support for HX8357D display panels"
>> +	depends on DRM_TINYDRM && SPI
>> +	depends on BACKLIGHT_CLASS_DEVICE
>> +	select TINYDRM_MIPI_DBI
>> +	help
>> +	  DRM driver for the following HX8357D panels:
>> +	  * YX350HV15-T 3.5" 340x350 TFT (Adafruit 3.5")
>> +
>> +	  If M is selected the module will be called hx8357d.
>> +
>>   config TINYDRM_ILI9225
>>   	tristate "DRM support for ILI9225 display panels"
>>   	depends on DRM_TINYDRM && SPI
>> diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
>> index 14d99080665a..f823066f7743 100644
>> --- a/drivers/gpu/drm/tinydrm/Makefile
>> +++ b/drivers/gpu/drm/tinydrm/Makefile
>> @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_TINYDRM)		+= core/
>>   obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
>>   
>>   # Displays
>> +obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
>>   obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
>>   obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
>>   obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
>> diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
>> new file mode 100644
>> index 000000000000..51d4da624d57
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tinydrm/hx8357d.c
>> @@ -0,0 +1,261 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * DRM driver for the HX8357D LCD controller
>> + *
>> + * Copyright 2018 Broadcom
>> + * Copyright 2018 David Lechner <david@lechnology.com>
>> + * Copyright 2016 Noralf Trønnes
>> + * Copyright (C) 2015 Adafruit Industries
>> + * Copyright (C) 2013 Christian Vogelgsang
>> + */
>> +
>> +#include <linux/backlight.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/module.h>
>> +#include <linux/property.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#include <drm/drm_fb_helper.h>
>> +#include <drm/drm_gem_framebuffer_helper.h>
>> +#include <drm/drm_modeset_helper.h>
>> +#include <drm/tinydrm/mipi-dbi.h>
>> +#include <drm/tinydrm/tinydrm-helpers.h>
>> +#include <video/mipi_display.h>
>> +#include "hx8357d.h"
>
> I prefer to have the defines in the driver instead of an extra header file.
> The reason is that usually only a handful of defines are actually used,
> in this case it's 9.

Yeah.  Also, that license on the header is pretty sketchy.  I've written
my own defines based on reading the HX8357D spec and included it in the
driver instead.

>> +
>> +static const struct spi_device_id hx8357d_id[] = {
>> +	{ "hx8357d", 0 },
>
> Last time I tried, module autoloading didn't work unless this contains
> the last part of the compatible. In this case: "yx350hv15".
> Have you checked that autoloading does work?

I hadn't since I changed the compatible string to adafruit.  Thanks!

> Otherwise this looks good:
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2018-10-30 22:46 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-24 18:43 [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen Eric Anholt
2018-10-24 18:43 ` [PATCH 1/3] dt-bindings: new binding for Himax HX8357D display panels Eric Anholt
2018-10-25 21:42   ` Rob Herring
2018-10-27 16:10   ` Noralf Trønnes
2018-10-24 18:43 ` [PATCH 2/3] drm: Add an hx8367d tinydrm driver Eric Anholt
2018-10-27 16:12   ` Noralf Trønnes
2018-10-30 22:46     ` Eric Anholt [this message]
2018-10-24 18:43 ` [PATCH 3/3] drm/tinydrm: Fix setting of the column/page end addresses Eric Anholt
2018-10-27 16:13   ` Noralf Trønnes
2018-10-25 16:29 ` [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen Eric Anholt
2018-10-25 21:52   ` Noralf Trønnes
2018-10-26  2:30     ` Eric Anholt
2018-10-26 19:16       ` Noralf Trønnes
2018-10-26 20:57         ` Noralf Trønnes
2018-10-31 16:27   ` Noralf Trønnes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87efc7m4s5.fsf@anholt.net \
    --to=eric@anholt.net \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hkallweit1@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=noralf@tronnes.org \
    --cc=robh+dt@kernel.org \
    --subject='Re: [PATCH 2/3] drm: Add an hx8367d tinydrm driver.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).