| Message ID | 20201105120333.947408-23-tomi.valkeinen@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Convert DSI code to use drm_mipi_dsi and drm_panel | expand |
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@collabora.com> > > 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@collabora.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > .../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; > } >
On 09/11/2020 10:49, Laurent Pinchart wrote: > 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@collabora.com> >> >> 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@collabora.com> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> .../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. The driver does not (and has not) support multiple DSI peripherals, even if the plumbing has been there. Yes, the VC handling can be made simpler. I would prefer to do that after this series. As I see it, the main point of this series is to move to DRM model while keeping the current mainline drivers working (dsi and panel-dsi-cm). That will enable many cleanups also outside the dsi driver. The series adds some shortcuts in places where they don't affect the supported setup. When we get to the end, we'll be using DRM bridge and panel model, and re-writing the VC handling (and some other parts) should fall into place much more neatly than doing them either before the series (on top of omapdrm's custom APIs) or inside the series. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
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; + 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; }