Hi Tomi and Sebastian, Thank you for the patch. On Thu, Nov 05, 2020 at 02:02:59PM +0200, Tomi Valkeinen wrote: > From: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > > In order to reduce the amount of custom functionality, this moves > handling of pixel format and DSI mode from set_config() to dsi > attach. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx> > --- > .../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 2 -- > drivers/gpu/drm/omapdrm/dss/dsi.c | 31 ++++++++++++------- > 2 files changed, 19 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > index a9609eed6bfa..2e9de33fc8d4 100644 > --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > @@ -550,8 +550,6 @@ static int dsicm_power_on(struct panel_drv_data *ddata) > u8 id1, id2, id3; > int r; > struct omap_dss_dsi_config dsi_config = { > - .mode = OMAP_DSS_DSI_CMD_MODE, > - .pixel_format = MIPI_DSI_FMT_RGB888, > .vm = &ddata->vm, > .hs_clk_min = 150000000, > .hs_clk_max = 300000000, > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index a16427f3bc23..e341aca92462 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -4579,24 +4579,19 @@ static int dsi_set_config(struct omap_dss_device *dssdev, > { > struct dsi_data *dsi = to_dsi_data(dssdev); > struct dsi_clk_calc_ctx ctx; > + struct omap_dss_dsi_config cfg = *config; > bool ok; > int r; > > mutex_lock(&dsi->lock); > > - dsi->pix_fmt = config->pixel_format; > - dsi->mode = config->mode; > + cfg.mode = dsi->mode; > + cfg.pixel_format = dsi->pix_fmt; > > - if (mipi_dsi_pixel_format_to_bpp(dsi->pix_fmt) < 0) { > - DSSERR("invalid pixel format\n"); > - r = -EINVAL; > - goto err; > - } > - > - if (config->mode == OMAP_DSS_DSI_VIDEO_MODE) > - ok = dsi_vm_calc(dsi, config, &ctx); > + if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) > + ok = dsi_vm_calc(dsi, &cfg, &ctx); > else > - ok = dsi_cm_calc(dsi, config, &ctx); > + ok = dsi_cm_calc(dsi, &cfg, &ctx); > > if (!ok) { > DSSERR("failed to find suitable DSI clock settings\n"); > @@ -4607,7 +4602,7 @@ static int dsi_set_config(struct omap_dss_device *dssdev, > dsi_pll_calc_dsi_fck(dsi, &ctx.dsi_cinfo); > > r = dsi_lp_clock_calc(ctx.dsi_cinfo.clkout[HSDIV_DSI], > - config->lp_clk_min, config->lp_clk_max, &dsi->user_lp_cinfo); > + cfg.lp_clk_min, cfg.lp_clk_max, &dsi->user_lp_cinfo); > if (r) { > DSSERR("failed to find suitable DSI LP clock settings\n"); > goto err; > @@ -4785,7 +4780,19 @@ static int omap_dsi_host_attach(struct mipi_dsi_host *host, > return -EBUSY; > } > > + if (mipi_dsi_pixel_format_to_bpp(client->format) < 0) { > + DSSERR("invalid pixel format\n"); > + return -EINVAL; > + } > + > dsi->vc[channel].dest = client; > + > + dsi->pix_fmt = client->format; Does this mean that all clients must use the same pixel format ? Do we even support multiple clients ? If no the VC allocation could be simplified. > + if (client->mode_flags & MIPI_DSI_MODE_VIDEO) > + dsi->mode = OMAP_DSS_DSI_VIDEO_MODE; > + else > + dsi->mode = OMAP_DSS_DSI_CMD_MODE; > + > return 0; > } > -- Regards, Laurent Pinchart