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.reic...@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.reic...@collabora.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkei...@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to