linux-kernel - Re: [PATCH v3 3/7] drm/tinydrm: Add MIPI DBI support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fa27da04-8ce4-3163-540b-db5038503741@tronnes.org>
Date:   Mon, 6 Feb 2017 16:45:47 +0100
From:   Noralf Trønnes <noralf@...nnes.org>
To:     Andrzej Hajda <a.hajda@...sung.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc:     thomas.petazzoni@...e-electrons.com, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v3 3/7] drm/tinydrm: Add MIPI DBI support

(Adding Maxime)

Den 06.02.2017 13.34, skrev Andrzej Hajda:
> On 06.02.2017 12:53, Thierry Reding wrote:
>> On Mon, Feb 06, 2017 at 01:30:09PM +0200, Jani Nikula wrote:
>>> On Mon, 06 Feb 2017, Thierry Reding <thierry.reding@...il.com> wrote:
>>>> On Tue, Jan 31, 2017 at 05:03:15PM +0100, Noralf Trønnes wrote:
>>>>> Add support for MIPI DBI compatible controllers.
>>>>> Interface type C option 1 and 3 are supported (SPI).
>>>>>
>>>>> Signed-off-by: Noralf Trønnes <noralf@...nnes.org>
>>>>> ---
>>>>>   Documentation/gpu/tinydrm.rst      |   12 +
>>>>>   drivers/gpu/drm/tinydrm/Kconfig    |    3 +
>>>>>   drivers/gpu/drm/tinydrm/Makefile   |    3 +
>>>>>   drivers/gpu/drm/tinydrm/mipi-dbi.c | 1005 ++++++++++++++++++++++++++++++++++++
>>>>>   include/drm/tinydrm/mipi-dbi.h     |  107 ++++
>>>>>   5 files changed, 1130 insertions(+)
>>>>>   create mode 100644 drivers/gpu/drm/tinydrm/mipi-dbi.c
>>>>>   create mode 100644 include/drm/tinydrm/mipi-dbi.h
>>>> Any reason why this is in the tinydrm subdirectory? Looks like this
>>>> could be useful to drivers outside of it.

I did consider having it outside, but I couldn't find any users in drm
that could benefit from it (there is one backlight driver).
But now there's Maxime's panel driver.

How about something like this:
(I have not included the framebuffer dirty function since it will only
be used in tinydrm anyway)

include/drm/drm_mipi_dbi.h:

struct mipi_dbi_device;

/**
  * mipi_dbi_dcs_write - MIPI DCS command with optional parameter(s)
  * @dbi: MIPI DBI structure
  * @cmd: Command
  * @seq...: Optional parameter(s)
  *
  * Send MIPI DCS command to the controller. Use 
mipi_dbi_dcs_read_buffer() for
  * get/read commands.
  *
  * Returns:
  * Zero on success, negative error code on failure.
  */
#define mipi_dbi_dcs_write(dbi, cmd, seq...) \
({ \
     const u8 d[] = { seq }; \
     BUILD_BUG_ON_MSG(ARRAY_SIZE(d) > 64, "DCS sequence too big for 
stack");\
     mipi_dbi_dcs_write_buffer(dbi, cmd, d, ARRAY_SIZE(d)); \
})

...

drivers/gpu/drm/drm_mipi_dbi.c:

struct mipi_dbi_device {
     struct mutex lock;
     int (*write)(struct mipi_dbi_device *dbi, u8 cmd, const u8 *par,
              size_t num);
     int (*read)(struct mipi_dbi_device *dbi, u8 cmd, u8 *par, size_t num);
     bool swap16;
};

/* MIPI DBI Type C options 1 and 3 */
struct mipi_dbi_spi_device {
     struct mipi_dbi_device dbi;
     struct spi_device *spi;
     struct gpio_desc *dc;
};

/*
  * MIPI DBI Type B - Intel 8080 type parallel bus
  * I need this to fully convert staging/fbtft to drm
  */
struct mipi_dbi_i80_device {
     struct mipi_dbi_device dbi;
     struct device *dev;
     struct gpio_desc *cs;
     struct gpio_desc *dc;
     struct gpio_desc *wr;
     struct gpio_descs *db;
};

/**
  * mipi_dbi_get_swap16 - Is byteswapping 16-bit pixel data needed?
  * @dbi: MIPI DBI device
  *
  * Byte swapping 16-bit pixel data is necessary if the bus can't 
support 16-bit
  * big endian transfers (e.g. if SPI can only do 8-bit and the machine is
  * little endian). This applies to the MIPI_DCS_WRITE_MEMORY_START command.
  *
  * Returns:
  * True if it's neccesary to swap bytes, false otherwise.
  */
bool mipi_dbi_get_swap16(struct mipi_dbi_device *dbi)
{
     return dbi->swap16;
}

/**
  * mipi_dbi_spi_init - Initialize MIPI DBI SPI device
  * @spi: SPI device
  * @dc: D/C gpio (optional)
  * @writeonly: True if it's not possible to read from the controller.
  *
  * If @dc is set, a Type C Option 3 interface is assumed, if not
  * Type C Option 1 (9-bit).
  *
  * If the SPI master driver doesn't support the necessary bits per word,
  * the following transformation is used:
  *
  * - 9-bit: reorder buffer as 9x 8-bit words, padded with no-op command.
  * - 16-bit (pixel data): if machine is big endian send as 8-bit, if little
  *   endian the user is responsible for swapping the bytes.
  *   See mipi_dbi_get_swap_pixel_bytes().
  *
  * Returns:
  * Pointer to &mipi_dbi_device on success, ERR_PTR on failure.
  */
struct mipi_dbi_device *mipi_dbi_spi_init(struct spi_device *spi,
                       struct gpio_desc *dc, bool writeonly)
{
...
}

struct mipi_dbi_device *mipi_dbi_i80_init(...)


Noralf.