Hi Maxime, mripard@xxxxxxxxxx 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@xxxxxxxxxxx> > > --- > > .../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