Re: [PATCH RFC 2/9] drm: panel: Add panel for Epson Moverio BT200

[Andreas Kemnade <andreas@kemnade.info>]

On Tue, 22 Sep 2026 11:13:36 +0200
Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:

> Hallo Andreas,
> 
> On Tue, Sep 22, 2026 at 08:16:59AM +0200, Andreas Kemnade wrote:
> > diff --git a/drivers/gpu/drm/panel/panel-epson-bt200.c b/drivers/gpu/drm/panel/panel-epson-bt200.c
> > new file mode 100644
> > index 000000000000..c26a4f8ae15e
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-epson-bt200.c
> > @@ -0,0 +1,251 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +#include <linux/backlight.h>
> > +#include <linux/errno.h>
> > +#include <linux/delay.h>
> > +#include <linux/module.h>
> > +#include <linux/mod_devicetable.h>  
> 
> Please don't include <linux/mod_devicetable.h>. You can rely on
> <linux/spi/spi.h> to provide of_device_id and spi_device_id; or if you
> prefer full iwyu include <linux/device-id/of.h> and <.../spi.h>.
> 
> > +#include <linux/spi/spi.h>
> > +
> > +#include <drm/drm_connector.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_print.h>
> > +#include <drm/drm_probe_helper.h>
> > +
> > +static const struct drm_display_mode default_mode = {
> > +	.clock			= 41600,	/* kHz */
> > +#if 0  
> 
> huh, upstream code isn't supposed to have #if 0 blocks.
> 
correct, this will go away latest in the first non-rfc series.

> > +	/* values matching struct omap_video_timings in vendor kernel */
> > +	.hdisplay		= 960,
> > +	.hsync_start		= 960 + 176,
> > +	.hsync_end		= 960 + 176 + 20,
> > +	.htotal			= 960 + 176 + 20 + 86,
> > +#else
> > +	/* values from what is writted to LCDCTRL on vendor kernel */
> > +	.hdisplay		= 960,
> > +	.hsync_start		= 960 + 33,
> > +	.hsync_end		= 960 + 33 + 20,
> > +	.htotal			= 960 + 33 + 20 + 86,
> > +#endif
> > +	.vdisplay		= 540,
> > +	.vsync_start		= 540 + 9,
> > +	.vsync_end		= 540 + 9 + 3,
> > +	.vtotal			= 540 + 9 + 3 + 12,
> > +	.flags			= 0,
> > +};
> > +
> > +struct bt200_panel {
> > +	struct drm_panel panel;
> > +	struct spi_device *spi;
> > +};
> > +
> > +static int bt200_panel_write(struct bt200_panel *ctx, u8 addr, u8 data)
> > +{
> > +	u8 val[2];
> > +
> > +	val[0] = addr;
> > +	val[1] = data;
> > +
> > +	return spi_write(ctx->spi, val, sizeof(val));
> > +}
> > +
> > +static inline struct bt200_panel *panel_to_bt200(struct drm_panel *panel)
> > +{
> > +	return container_of(panel, struct bt200_panel, panel);
> > +}
> > +
> > +static int bt200_disable(struct drm_panel *panel)
> > +{
> > +	struct bt200_panel *ctx = panel_to_bt200(panel);
> > +
> > +	bt200_panel_write(ctx, 0x0A, 0);
> > +	return 0;
> > +}
> > +
> > +static int bt200_unprepare(struct drm_panel *panel)
> > +{
> > +	return 0;
> > +}
> > +
> > +/* found this table in vendor kernel */
> > +static struct {
> > +	u8 addr;
> > +	u8 data;
> > +} lcddr_init[] = {
> > +	{0x01, 0x00}, {0x02, 0x00}, {0x03, 0x00}, {0x05, 0x01},
> > +	{0x07, 0x00}, {0x0A, 0x00}, {0x10, 0x03}, {0x11, 0x44},
> > +	{0x12, 0x44}, {0x13, 0x55}, {0x14, 0x03}, {0x15, 0x00},
> > +	{0x16, 0x2A}, {0x17, 0x20}, {0x18, 0x00}, {0x19, 0x10},
> > +	{0x1A, 0x12}, {0x1B, 0x0E}, {0x1C, 0x0F}, {0x1D, 0x10},
> > +	{0x1E, 0x0F}, {0x1F, 0x1B}, {0x20, 0x0F}, {0x21, 0x00},
> > +	{0x22, 0x00}, {0x23, 0x00}, {0x24, 0x00}, {0x28, 0x14},
> > +	{0x29, 0x19}, {0x2A, 0x17}, {0x2B, 0x2B}, {0x2C, 0x99},
> > +	{0x2D, 0x13}, {0x2E, 0x2A}, {0x30, 0x0B}, {0x31, 0x00},
> > +	{0x32, 0x00}, {0x33, 0x01}, {0x34, 0x00}, {0x35, 0x0B},
> > +	{0x36, 0x04}, {0x37, 0x21}, {0x38, 0x00}, {0x39, 0x46},
> > +	{0x3A, 0x01}, {0x3B, 0x06}, {0x3C, 0x03}, {0x3D, 0x00},
> > +	{0x3E, 0x06}, {0x3F, 0x04}, {0x40, 0x00}, {0x41, 0x0D},
> > +	{0x42, 0x00}, {0x43, 0x2E}, {0x45, 0x08}, {0x46, 0x00},
> > +	{0x47, 0x01}, {0x48, 0x00}, {0x49, 0x00}, {0x4A, 0x0B},
> > +	{0x4B, 0x38}, {0x4C, 0x03}, {0x4D, 0x04}, {0x50, 0x0F},
> > +	{0x51, 0x04}, {0x52, 0x01}, {0x53, 0x0E}, {0x54, 0x11},
> > +	{0x55, 0x9F}, {0x56, 0x36}, {0x57, 0x00}, {0x58, 0x68},
> > +	{0x59, 0x01}, {0x5A, 0xE0}, {0x5B, 0x00}, {0x5C, 0x00},
> > +	{0x5D, 0x10}, {0x5E, 0x36}, {0x5F, 0x36}, {0x60, 0x00},
> > +	{0x61, 0x04}, {0x62, 0x12}, {0x63, 0x00}, {0x64, 0x80},
> > +	{0x65, 0x00}, {0x66, 0x80}, {0x67, 0x1C}, {0x68, 0x00},
> > +	{0x69, 0x00}, {0x6A, 0x00}, {0x6B, 0x00}, {0x6C, 0x00},
> > +	{0x70, 0x00}, {0x71, 0x55}, {0x72, 0x44}, {0x73, 0x33},
> > +	{0x74, 0x22}, {0x75, 0x33}, {0x76, 0x22}, {0x77, 0x22},
> > +	{0x78, 0x33}, {0x79, 0x22}, {0x7A, 0x44}, {0x7B, 0x55},
> > +	{0x7C, 0x00}, {0x80, 0x00}, {0x81, 0x66}, {0x82, 0x22},
> > +	{0x83, 0x33}, {0x84, 0x22}, {0x85, 0x33}, {0x86, 0x22},
> > +	{0x87, 0x22}, {0x88, 0x33}, {0x89, 0x44}, {0x8A, 0x44},
> > +	{0x8B, 0x44}, {0x8C, 0x00}, {0x90, 0x00}, {0x91, 0x77},
> > +	{0x92, 0x66}, {0x93, 0x33}, {0x94, 0x11}, {0x95, 0x33},
> > +	{0x96, 0x22}, {0x97, 0x22}, {0x98, 0x44}, {0x99, 0x44},
> > +	{0x9A, 0x33}, {0x9B, 0x44}, {0x9C, 0x00}, {0xA0, 0x88},
> > +	{0xA1, 0x88}, {0xA2, 0x88}, {0xA3, 0x88}, {0xA4, 0x88},
> > +	{0xA5, 0x88}, {0xA6, 0x88}, {0xA7, 0x88}, {0xA8, 0x88},
> > +	{0xA9, 0x88}, {0xAA, 0x88}, {0xAB, 0x88}, {0xAC, 0x88},
> > +	{0xAD, 0x88}, {0xAE, 0x88}, {0xAF, 0x88}, {0xB0, 0x88},
> > +	{0xB1, 0x88}, {0xB2, 0x88}, {0xB3, 0x88}, {0xB4, 0x88},
> > +	{0xB5, 0x88}, {0xB6, 0x88}, {0xB7, 0x88}, {0xB8, 0x88},
> > +	{0xB9, 0x88}, {0xBA, 0x88}, {0xBB, 0xA6}, {0xBC, 0x88},
> > +	{0xBD, 0x88}, {0xBE, 0x88}, {0xBF, 0x88}, {0xC0, 0x88},
> > +	{0xC1, 0x88}, {0xC2, 0x88}, {0xC3, 0x88}, {0xC4, 0x88},
> > +	{0xC5, 0x88}, {0xC6, 0x88}, {0xC7, 0x88}, {0xC8, 0x88},
> > +	{0xC9, 0x88}, {0xD0, 0x36}, {0xD1, 0x26}, {0xD2, 0x21},
> > +	{0xD3, 0x1F}, {0xD4, 0x17}, {0xD5, 0x15}, {0xD6, 0x13},
> > +	{0xD7, 0x10}, {0xD8, 0x08}, {0xD9, 0x08}, {0xDA, 0x18},
> > +	{0xDB, 0x1D}, {0xDC, 0x1F}, {0xDD, 0x27}, {0xDE, 0x29},
> > +	{0xDF, 0x2B}, {0xE0, 0x2E}, {0xE1, 0x36}, {0xE6, 0x00},
> > +	{0xF1, 0x00}, {0xF2, 0x00}, {0xF3, 0x00}  
> 
> Smells non-GPLly. Maybe ask the provider of the vendor kernel for "the
> preferred form of the work for making modifications to it".
> 
Well, this are address / data pairs. so probably the more
preferred form would be symbolic names. I did not manage to get u-boot sources
from Epson either. So I think it would be even harder to get anything here.
And since these things are very old, maybe more information
does not even exist anymore.
So how to progress here?

> I guess {0x0A, 0x00} disables the display, so taking over an enabled
> display setup by the bootloader doesn't work as is.
> 
The factory bootloader seems only to reset things. It seems not
to initialize anything regarding display.

Until just a few months my preferred way of booting this device
was to boot vendor kernel, reprogram boot order temporarily,
go through more recent u-boot and boot linux. During this procedure,
the bridge setup was left intact. I just read things back to check
if values are already set and happlily reinitialized display itself.
No issue found doing it that way.

[...]
> > +MODULE_DEVICE_TABLE(spi, bt200_ids);
> > +
> > +static struct spi_driver bt200_driver = {
> > +	.probe = bt200_probe,
> > +	.remove = bt200_remove,
> > +	.id_table = bt200_ids,
> > +	.driver = {
> > +		.name = "panel-bt200",
> > +		.of_match_table = bt200_of_match,
> > +		.suppress_bind_attrs = true,  
> 
> Why suppress bind attrs?
> 
That was not intentional, maybe I had something in mind that
it is common practice is certain areas, but not for panels.

Regards,
Andreas