Re: [PATCH v3 15/56] drm/omap: dsi: request VC via mipi_dsi_attach

Tomi Valkeinen
2020-11-09T11:17:00Z

On 09/11/2020 10:42, Laurent Pinchart wrote:
> Hi Tomi and Sebastian,
>
> Thank you for the patch.
>
> On Thu, Nov 05, 2020 at 02:02:52PM +0200, Tomi Valkeinen wrote:
>> From: Sebastian Reichel <sebastian.reichel@collabora.com>
>>
>> Drop custom request_vc/release_vc callbacks by using the
>> generic mipi_dsi_attach/mipi_dsi_detach functions.
>>
>> 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 | 24 +++++---
>> drivers/gpu/drm/omapdrm/dss/dsi.c | 61 ++++++++-----------
>> drivers/gpu/drm/omapdrm/dss/omapdss.h | 3 -
>> 3 files changed, 41 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
>> index ec36e62ddc3a..b1ca9b34ce17 100644
>> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
>> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
>> @@ -693,14 +693,6 @@ static int dsicm_connect(struct omap_dss_device *src,
>> struct omap_dss_device *dst)
>> {
>> struct panel_drv_data *ddata = to_panel_data(dst);
>> - struct device *dev = &ddata->dsi->dev;
>> - int r;
>> -
>> - r = src->ops->dsi.request_vc(src, ddata->channel);
>> - if (r) {
>> - dev_err(dev, "failed to get virtual channel\n");
>> - return r;
>> - }
>>
>> ddata->src = src;
>> return 0;
>> @@ -711,7 +703,6 @@ static void dsicm_disconnect(struct omap_dss_device *src,
>> {
>> struct panel_drv_data *ddata = to_panel_data(dst);
>>
>> - src->ops->dsi.release_vc(src, ddata->channel);
>> ddata->src = NULL;
>> }
>>
>> @@ -1276,8 +1267,21 @@ static int dsicm_probe(struct mipi_dsi_device *dsi)
>> goto err_bl;
>> }
>>
>> + dsi->lanes = 3;
>> + dsi->format = MIPI_DSI_FMT_RGB888;
>> + dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS |
>> + MIPI_DSI_MODE_EOT_PACKET;
>> + dsi->hs_rate = 300000000;
>> + dsi->lp_rate = 10000000;
>
> Should this change be explained in the commit message ?

I added:

To use mipi_dsi_attach() we need to fill in the mipi_dsi_device fields,
and some of these fields overlap with the fields in omap_dss_dsi_config.
In later patches the latter will get dropped.

>> +
>> + r = mipi_dsi_attach(dsi);
>> + if (r < 0)
>> + goto err_dsi_attach;
>> +
>> return 0;
>>
>> +err_dsi_attach:
>> + sysfs_remove_group(&dsi->dev.kobj, &dsicm_attr_group);
>> err_bl:
>> destroy_workqueue(ddata->workqueue);
>> err_reg:
>> @@ -1294,6 +1298,8 @@ static int __exit dsicm_remove(struct mipi_dsi_device *dsi)
>>
>> dev_dbg(&dsi->dev, "remove\n");
>>
>> + mipi_dsi_detach(dsi);
>> +
>> omapdss_device_unregister(dssdev);
>>
>> if (omapdss_device_is_enabled(dssdev))
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> index d1187a8af37f..a16427f3bc23 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> @@ -349,7 +349,7 @@ struct dsi_data {
>>
>> struct {
>> enum dsi_vc_source source;
>> - struct omap_dss_device *dssdev;
>> + struct mipi_dsi_device *dest;
>
> How about naming this dsidev ?

Maybe, but... dsidev was used to refer to the DSI host not that long ago. While going through these
patches, I thought about renaming "dest", but then I decided that "dest" a new name and perhaps a
good one to have during this transition period.

Tomi