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

[Noralf TrÃnnes]

(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@xxxxxxxxx> 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@xxxxxxxxxxx>
> > > > > ---
> > > > >   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.