| Message ID | 20230609145951.853533-7-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panel: sitronix-st7789v: Support ET028013DMA panel | expand |
On Fri, Jun 09, 2023 at 04:59:50PM +0200, Miquel Raynal wrote: > This panel from Emerging Display Technologies Corporation features an > ST7789V2 panel inside which is almost identical to what the Sitronix > panel driver supports. > > In practice, the module physical size is specific, and experiments show > that the display will malfunction if any of the following situation > occurs: > * Pixel clock is above 3MHz > * Pixel clock is not inverted > I could not properly identify the reasons behind these failures, scope > captures show valid input signals. The patch includes an additional change where the bus_flags are assigned to the connector. At least tell why, or maybe split it out. Other panels could learn to do the same - and I assume you did it because it was required in the atmel_hlcd driver. Sam > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > .../gpu/drm/panel/panel-sitronix-st7789v.c | 34 +++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > index 212bccc31804..7de192a3a8aa 100644 > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > @@ -30,7 +30,8 @@ > #define ST7789V_RGBCTRL_RCM(n) (((n) & 3) << 5) > #define ST7789V_RGBCTRL_VSYNC_HIGH BIT(3) > #define ST7789V_RGBCTRL_HSYNC_HIGH BIT(2) > -#define ST7789V_RGBCTRL_PCLK_HIGH BIT(1) > +#define ST7789V_RGBCTRL_PCLK_FALLING BIT(1) > +#define ST7789V_RGBCTRL_PCLK_RISING 0 > #define ST7789V_RGBCTRL_VBP(n) ((n) & 0x7f) > #define ST7789V_RGBCTRL_HBP(n) ((n) & 0x1f) > > @@ -117,6 +118,7 @@ struct st7789v_panel_info { > u16 width_mm; > u16 height_mm; > u32 bus_format; > + u32 bus_flags; > }; > > struct st7789v { > @@ -175,6 +177,18 @@ static const struct drm_display_mode default_mode = { > .vtotal = 320 + 8 + 4 + 4, > }; > > +static const struct drm_display_mode slow_mode = { > + .clock = 3000, > + .hdisplay = 240, > + .hsync_start = 240 + 38, > + .hsync_end = 240 + 38 + 10, > + .htotal = 240 + 38 + 10 + 10, > + .vdisplay = 320, > + .vsync_start = 320 + 8, > + .vsync_end = 320 + 8 + 4, > + .vtotal = 320 + 8 + 4 + 4, > +}; > + > static int st7789v_get_modes(struct drm_panel *panel, > struct drm_connector *connector) > { > @@ -197,6 +211,7 @@ static int st7789v_get_modes(struct drm_panel *panel, > > connector->display_info.width_mm = panel_info->width_mm; > connector->display_info.height_mm = panel_info->height_mm; > + connector->display_info.bus_flags = panel_info->bus_flags; > drm_display_info_set_bus_formats(&connector->display_info, > &panel_info->bus_format, 1); > > @@ -206,8 +221,13 @@ static int st7789v_get_modes(struct drm_panel *panel, > static int st7789v_prepare(struct drm_panel *panel) > { > struct st7789v *ctx = panel_to_st7789v(panel); > + const struct st7789v_panel_info *panel_info = ctx->panel_info; > + u8 pck = ST7789V_RGBCTRL_PCLK_FALLING; > int ret; > > + if (panel_info->bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE) > + pck = ST7789V_RGBCTRL_PCLK_RISING; > + > ret = regulator_enable(ctx->power); > if (ret) > return ret; > @@ -321,7 +341,7 @@ static int st7789v_prepare(struct drm_panel *panel) > ST7789V_RGBCTRL_RCM(2) | > ST7789V_RGBCTRL_VSYNC_HIGH | > ST7789V_RGBCTRL_HSYNC_HIGH | > - ST7789V_RGBCTRL_PCLK_HIGH)); > + pck)); > ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_VBP(8))); > ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_HBP(20))); > > @@ -422,14 +442,24 @@ static const struct st7789v_panel_info st7789v_info = { > .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > }; > > +static const struct st7789v_panel_info et028013dma_info = { > + .display_mode = &slow_mode, > + .width_mm = 43, > + .height_mm = 58, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > + .bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > +}; > + > static const struct of_device_id st7789v_of_match[] = { > { .compatible = "sitronix,st7789v", .data = &st7789v_info }, > + { .compatible = "edt,et028013dma", .data = &et028013dma_info }, > { } > }; > MODULE_DEVICE_TABLE(of, st7789v_of_match); > > static const struct spi_device_id st7789v_ids[] = { > { "st7789v", }, > + { "et028013dma", }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(spi, st7789v_ids); > -- > 2.34.1
On Fri, Jun 09, 2023 at 04:59:50PM +0200, Miquel Raynal wrote: > This panel from Emerging Display Technologies Corporation features an > ST7789V2 panel inside which is almost identical to what the Sitronix > panel driver supports. > > In practice, the module physical size is specific, and experiments show > that the display will malfunction if any of the following situation > occurs: > * Pixel clock is above 3MHz > * Pixel clock is not inverted > I could not properly identify the reasons behind these failures, scope > captures show valid input signals. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > .../gpu/drm/panel/panel-sitronix-st7789v.c | 34 +++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > index 212bccc31804..7de192a3a8aa 100644 > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > @@ -30,7 +30,8 @@ > #define ST7789V_RGBCTRL_RCM(n) (((n) & 3) << 5) > #define ST7789V_RGBCTRL_VSYNC_HIGH BIT(3) > #define ST7789V_RGBCTRL_HSYNC_HIGH BIT(2) > -#define ST7789V_RGBCTRL_PCLK_HIGH BIT(1) > +#define ST7789V_RGBCTRL_PCLK_FALLING BIT(1) > +#define ST7789V_RGBCTRL_PCLK_RISING 0 > #define ST7789V_RGBCTRL_VBP(n) ((n) & 0x7f) > #define ST7789V_RGBCTRL_HBP(n) ((n) & 0x1f) > > @@ -117,6 +118,7 @@ struct st7789v_panel_info { > u16 width_mm; > u16 height_mm; > u32 bus_format; > + u32 bus_flags; > }; > > struct st7789v { > @@ -175,6 +177,18 @@ static const struct drm_display_mode default_mode = { > .vtotal = 320 + 8 + 4 + 4, > }; > > +static const struct drm_display_mode slow_mode = { > + .clock = 3000, > + .hdisplay = 240, > + .hsync_start = 240 + 38, > + .hsync_end = 240 + 38 + 10, > + .htotal = 240 + 38 + 10 + 10, > + .vdisplay = 320, > + .vsync_start = 320 + 8, > + .vsync_end = 320 + 8 + 4, > + .vtotal = 320 + 8 + 4 + 4, > +}; Why is it supposed to be slow (and compared to what)? It looks like a fairly normal mode to me? Or is it because it's refreshed at 30Hz? Either way, the ST7789V is a panel controller and can be connected to a wide range of panels depending on the model, so it would be better to just use the model name there. > static int st7789v_get_modes(struct drm_panel *panel, > struct drm_connector *connector) > { > @@ -197,6 +211,7 @@ static int st7789v_get_modes(struct drm_panel *panel, > > connector->display_info.width_mm = panel_info->width_mm; > connector->display_info.height_mm = panel_info->height_mm; > + connector->display_info.bus_flags = panel_info->bus_flags; > drm_display_info_set_bus_formats(&connector->display_info, > &panel_info->bus_format, 1); > > @@ -206,8 +221,13 @@ static int st7789v_get_modes(struct drm_panel *panel, > static int st7789v_prepare(struct drm_panel *panel) > { > struct st7789v *ctx = panel_to_st7789v(panel); > + const struct st7789v_panel_info *panel_info = ctx->panel_info; > + u8 pck = ST7789V_RGBCTRL_PCLK_FALLING; > int ret; > > + if (panel_info->bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE) > + pck = ST7789V_RGBCTRL_PCLK_RISING; > + I guess it could be in a separate commit Maxime > ret = regulator_enable(ctx->power); > if (ret) > return ret; > @@ -321,7 +341,7 @@ static int st7789v_prepare(struct drm_panel *panel) > ST7789V_RGBCTRL_RCM(2) | > ST7789V_RGBCTRL_VSYNC_HIGH | > ST7789V_RGBCTRL_HSYNC_HIGH | > - ST7789V_RGBCTRL_PCLK_HIGH)); > + pck)); > ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_VBP(8))); > ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_HBP(20))); > > @@ -422,14 +442,24 @@ static const struct st7789v_panel_info st7789v_info = { > .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > }; > > +static const struct st7789v_panel_info et028013dma_info = { > + .display_mode = &slow_mode, > + .width_mm = 43, > + .height_mm = 58, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > + .bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > +}; > + > static const struct of_device_id st7789v_of_match[] = { > { .compatible = "sitronix,st7789v", .data = &st7789v_info }, > + { .compatible = "edt,et028013dma", .data = &et028013dma_info }, We should sort them by alphabetical order Maxime
Hi Maxime, mripard@kernel.org wrote on Mon, 12 Jun 2023 10:51:09 +0200: > On Fri, Jun 09, 2023 at 04:59:50PM +0200, Miquel Raynal wrote: > > This panel from Emerging Display Technologies Corporation features an > > ST7789V2 panel inside which is almost identical to what the Sitronix > > panel driver supports. > > > > In practice, the module physical size is specific, and experiments show > > that the display will malfunction if any of the following situation > > occurs: > > * Pixel clock is above 3MHz > > * Pixel clock is not inverted > > I could not properly identify the reasons behind these failures, scope > > captures show valid input signals. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > .../gpu/drm/panel/panel-sitronix-st7789v.c | 34 +++++++++++++++++-- > > 1 file changed, 32 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > > index 212bccc31804..7de192a3a8aa 100644 > > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c > > @@ -30,7 +30,8 @@ > > #define ST7789V_RGBCTRL_RCM(n) (((n) & 3) << 5) > > #define ST7789V_RGBCTRL_VSYNC_HIGH BIT(3) > > #define ST7789V_RGBCTRL_HSYNC_HIGH BIT(2) > > -#define ST7789V_RGBCTRL_PCLK_HIGH BIT(1) > > +#define ST7789V_RGBCTRL_PCLK_FALLING BIT(1) > > +#define ST7789V_RGBCTRL_PCLK_RISING 0 > > #define ST7789V_RGBCTRL_VBP(n) ((n) & 0x7f) > > #define ST7789V_RGBCTRL_HBP(n) ((n) & 0x1f) > > > > @@ -117,6 +118,7 @@ struct st7789v_panel_info { > > u16 width_mm; > > u16 height_mm; > > u32 bus_format; > > + u32 bus_flags; > > }; > > > > struct st7789v { > > @@ -175,6 +177,18 @@ static const struct drm_display_mode default_mode = { > > .vtotal = 320 + 8 + 4 + 4, > > }; > > > > +static const struct drm_display_mode slow_mode = { > > + .clock = 3000, > > + .hdisplay = 240, > > + .hsync_start = 240 + 38, > > + .hsync_end = 240 + 38 + 10, > > + .htotal = 240 + 38 + 10 + 10, > > + .vdisplay = 320, > > + .vsync_start = 320 + 8, > > + .vsync_end = 320 + 8 + 4, > > + .vtotal = 320 + 8 + 4 + 4, > > +}; > > Why is it supposed to be slow (and compared to what)? It looks like a > fairly normal mode to me? Or is it because it's refreshed at 30Hz? The initial support was using 60Hz and for a reason that I do not understand (scope capture look right), the panel I use is highly unstable at whatever frequency above 30Hz, so for me it was "slow" :-) But of course I'll use the panel name to qualify the mode. > Either way, the ST7789V is a panel controller and can be connected to a > wide range of panels depending on the model, so it would be better to > just use the model name there. Sure. > > static int st7789v_get_modes(struct drm_panel *panel, > > struct drm_connector *connector) > > { > > @@ -197,6 +211,7 @@ static int st7789v_get_modes(struct drm_panel *panel, > > > > connector->display_info.width_mm = panel_info->width_mm; > > connector->display_info.height_mm = panel_info->height_mm; > > + connector->display_info.bus_flags = panel_info->bus_flags; > > drm_display_info_set_bus_formats(&connector->display_info, > > &panel_info->bus_format, 1); > > > > @@ -206,8 +221,13 @@ static int st7789v_get_modes(struct drm_panel *panel, > > static int st7789v_prepare(struct drm_panel *panel) > > { > > struct st7789v *ctx = panel_to_st7789v(panel); > > + const struct st7789v_panel_info *panel_info = ctx->panel_info; > > + u8 pck = ST7789V_RGBCTRL_PCLK_FALLING; > > int ret; > > > > + if (panel_info->bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE) > > + pck = ST7789V_RGBCTRL_PCLK_RISING; > > + > > I guess it could be in a separate commit I will rebase on top of Sebastian's series who already addressed that point (as well as many others). [...] > > > > +static const struct st7789v_panel_info et028013dma_info = { > > + .display_mode = &slow_mode, > > + .width_mm = 43, > > + .height_mm = 58, > > + .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > > + .bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > > +}; > > + > > static const struct of_device_id st7789v_of_match[] = { > > { .compatible = "sitronix,st7789v", .data = &st7789v_info }, > > + { .compatible = "edt,et028013dma", .data = &et028013dma_info }, > > We should sort them by alphabetical order > > Maxime Ok! Thanks, Miquèl
diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c index 212bccc31804..7de192a3a8aa 100644 --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c @@ -30,7 +30,8 @@ #define ST7789V_RGBCTRL_RCM(n) (((n) & 3) << 5) #define ST7789V_RGBCTRL_VSYNC_HIGH BIT(3) #define ST7789V_RGBCTRL_HSYNC_HIGH BIT(2) -#define ST7789V_RGBCTRL_PCLK_HIGH BIT(1) +#define ST7789V_RGBCTRL_PCLK_FALLING BIT(1) +#define ST7789V_RGBCTRL_PCLK_RISING 0 #define ST7789V_RGBCTRL_VBP(n) ((n) & 0x7f) #define ST7789V_RGBCTRL_HBP(n) ((n) & 0x1f) @@ -117,6 +118,7 @@ struct st7789v_panel_info { u16 width_mm; u16 height_mm; u32 bus_format; + u32 bus_flags; }; struct st7789v { @@ -175,6 +177,18 @@ static const struct drm_display_mode default_mode = { .vtotal = 320 + 8 + 4 + 4, }; +static const struct drm_display_mode slow_mode = { + .clock = 3000, + .hdisplay = 240, + .hsync_start = 240 + 38, + .hsync_end = 240 + 38 + 10, + .htotal = 240 + 38 + 10 + 10, + .vdisplay = 320, + .vsync_start = 320 + 8, + .vsync_end = 320 + 8 + 4, + .vtotal = 320 + 8 + 4 + 4, +}; + static int st7789v_get_modes(struct drm_panel *panel, struct drm_connector *connector) { @@ -197,6 +211,7 @@ static int st7789v_get_modes(struct drm_panel *panel, connector->display_info.width_mm = panel_info->width_mm; connector->display_info.height_mm = panel_info->height_mm; + connector->display_info.bus_flags = panel_info->bus_flags; drm_display_info_set_bus_formats(&connector->display_info, &panel_info->bus_format, 1); @@ -206,8 +221,13 @@ static int st7789v_get_modes(struct drm_panel *panel, static int st7789v_prepare(struct drm_panel *panel) { struct st7789v *ctx = panel_to_st7789v(panel); + const struct st7789v_panel_info *panel_info = ctx->panel_info; + u8 pck = ST7789V_RGBCTRL_PCLK_FALLING; int ret; + if (panel_info->bus_flags & DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE) + pck = ST7789V_RGBCTRL_PCLK_RISING; + ret = regulator_enable(ctx->power); if (ret) return ret; @@ -321,7 +341,7 @@ static int st7789v_prepare(struct drm_panel *panel) ST7789V_RGBCTRL_RCM(2) | ST7789V_RGBCTRL_VSYNC_HIGH | ST7789V_RGBCTRL_HSYNC_HIGH | - ST7789V_RGBCTRL_PCLK_HIGH)); + pck)); ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_VBP(8))); ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_HBP(20))); @@ -422,14 +442,24 @@ static const struct st7789v_panel_info st7789v_info = { .bus_format = MEDIA_BUS_FMT_RGB666_1X18, }; +static const struct st7789v_panel_info et028013dma_info = { + .display_mode = &slow_mode, + .width_mm = 43, + .height_mm = 58, + .bus_format = MEDIA_BUS_FMT_RGB666_1X18, + .bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, +}; + static const struct of_device_id st7789v_of_match[] = { { .compatible = "sitronix,st7789v", .data = &st7789v_info }, + { .compatible = "edt,et028013dma", .data = &et028013dma_info }, { } }; MODULE_DEVICE_TABLE(of, st7789v_of_match); static const struct spi_device_id st7789v_ids[] = { { "st7789v", }, + { "et028013dma", }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(spi, st7789v_ids);
This panel from Emerging Display Technologies Corporation features an ST7789V2 panel inside which is almost identical to what the Sitronix panel driver supports. In practice, the module physical size is specific, and experiments show that the display will malfunction if any of the following situation occurs: * Pixel clock is above 3MHz * Pixel clock is not inverted I could not properly identify the reasons behind these failures, scope captures show valid input signals. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- .../gpu/drm/panel/panel-sitronix-st7789v.c | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)