| Message ID | 20200911135413.3654800-9-m.tretter@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/exynos: Convert driver to drm bridge | expand |
W dniu 11.09.2020 o 15:54, Michael Tretter pisze: > Platform drivers need to be aware if a mipi dsi device attaches or > detaches. Add host_ops to the driver_data for attach and detach > callbacks and move the i80 mode selection and the hotplug handling into > the callback, because these depend on the drm driver. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- > v2: > - new patch > --- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++----- > 1 file changed, 53 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index 1a15ae71205d..684a2fbef08a 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -239,6 +239,12 @@ struct exynos_dsi_transfer { > #define DSIM_STATE_CMD_LPM BIT(2) > #define DSIM_STATE_VIDOUT_AVAILABLE BIT(3) > > +struct exynos_dsi; > +struct exynos_dsi_host_ops { > + int (*attach)(struct device *dev, struct mipi_dsi_device *device); > + int (*detach)(struct device *dev, struct mipi_dsi_device *device); > +}; > + > enum exynos_reg_offset { > EXYNOS_REG_OFS, > EXYNOS5433_REG_OFS > @@ -254,6 +260,7 @@ struct exynos_dsi_driver_data { > unsigned int wait_for_reset; > unsigned int num_bits_resol; > const unsigned int *reg_values; > + const struct exynos_dsi_host_ops *host_ops; > }; > > struct exynos_dsi { > @@ -467,6 +474,41 @@ static const unsigned int exynos5433_reg_values[] = { > [PHYTIMING_HS_TRAIL] = 0x0c, > }; > > +static int __exynos_dsi_host_attach(struct device *dev, > + struct mipi_dsi_device *device) > +{ > + struct exynos_dsi *dsi = dev_get_drvdata(dev); > + struct drm_device *drm = dsi->encoder.dev; As I wrote yesterday drm device was guaranteed to be present here only if mipi dsi host registration was performed in component bind. In case it is performed in probe it will be always NULL, and the code does not make sense. Regards Andrzej > + struct exynos_drm_crtc *crtc; > + > + mutex_lock(&drm->mode_config.mutex); > + crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD); > + crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO); > + mutex_unlock(&drm->mode_config.mutex); > + > + if (drm->mode_config.poll_enabled) > + drm_kms_helper_hotplug_event(drm); > + > + return 0; > +} > + > +static int __exynos_dsi_host_detach(struct device *dev, > + struct mipi_dsi_device *device) > +{ > + struct exynos_dsi *dsi = dev_get_drvdata(dev); > + struct drm_device *drm = dsi->encoder.dev; > + > + if (drm->mode_config.poll_enabled) > + drm_kms_helper_hotplug_event(drm); > + > + return 0; > +} > + > +static const struct exynos_dsi_host_ops exynos_dsi_host_ops = { > + .attach = __exynos_dsi_host_attach, > + .detach = __exynos_dsi_host_detach, > +}; > + > static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { > .reg_ofs = EXYNOS_REG_OFS, > .plltmr_reg = 0x50, > @@ -477,6 +519,7 @@ static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { > .wait_for_reset = 1, > .num_bits_resol = 11, > .reg_values = reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { > @@ -489,6 +532,7 @@ static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { > .wait_for_reset = 1, > .num_bits_resol = 11, > .reg_values = reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { > @@ -499,6 +543,7 @@ static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { > .wait_for_reset = 1, > .num_bits_resol = 11, > .reg_values = reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { > @@ -510,6 +555,7 @@ static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { > .wait_for_reset = 0, > .num_bits_resol = 12, > .reg_values = exynos5433_reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { > @@ -521,6 +567,7 @@ static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { > .wait_for_reset = 1, > .num_bits_resol = 12, > .reg_values = exynos5422_reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct of_device_id exynos_dsi_of_match[] = { > @@ -1551,8 +1598,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > struct mipi_dsi_device *device) > { > struct exynos_dsi *dsi = host_to_dsi(host); > + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; > struct drm_encoder *encoder = &dsi->encoder; > - struct drm_device *drm = encoder->dev; > struct drm_bridge *out_bridge; > > out_bridge = of_drm_find_bridge(device->dev.of_node); > @@ -1590,18 +1637,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > return ret; > } > > - mutex_lock(&drm->mode_config.mutex); > - > dsi->lanes = device->lanes; > dsi->format = device->format; > dsi->mode_flags = device->mode_flags; > - exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode = > - !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO); > > - mutex_unlock(&drm->mode_config.mutex); > - > - if (drm->mode_config.poll_enabled) > - drm_kms_helper_hotplug_event(drm); > + if (ops && ops->attach) > + ops->attach(dsi->dsi_host.dev, device); > > return 0; > } > @@ -1610,6 +1651,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, > struct mipi_dsi_device *device) > { > struct exynos_dsi *dsi = host_to_dsi(host); > + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; > struct drm_device *drm = dsi->encoder.dev; > > if (dsi->panel) { > @@ -1625,8 +1667,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, > INIT_LIST_HEAD(&dsi->bridge_chain); > } > > - if (drm->mode_config.poll_enabled) > - drm_kms_helper_hotplug_event(drm); > + if (ops && ops->detach) > + ops->detach(dsi->dsi_host.dev, device); > > exynos_dsi_unregister_te_irq(dsi); >
On Tue, 15 Sep 2020 19:07:59 +0200, Andrzej Hajda wrote: > > W dniu 11.09.2020 o 15:54, Michael Tretter pisze: > > Platform drivers need to be aware if a mipi dsi device attaches or > > detaches. Add host_ops to the driver_data for attach and detach > > callbacks and move the i80 mode selection and the hotplug handling into > > the callback, because these depend on the drm driver. > > > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > > --- > > v2: > > - new patch > > --- > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++----- > > 1 file changed, 53 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > index 1a15ae71205d..684a2fbef08a 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > @@ -239,6 +239,12 @@ struct exynos_dsi_transfer { > > #define DSIM_STATE_CMD_LPM BIT(2) > > #define DSIM_STATE_VIDOUT_AVAILABLE BIT(3) > > > > +struct exynos_dsi; > > +struct exynos_dsi_host_ops { > > + int (*attach)(struct device *dev, struct mipi_dsi_device *device); > > + int (*detach)(struct device *dev, struct mipi_dsi_device *device); > > +}; > > + > > enum exynos_reg_offset { > > EXYNOS_REG_OFS, > > EXYNOS5433_REG_OFS > > @@ -254,6 +260,7 @@ struct exynos_dsi_driver_data { > > unsigned int wait_for_reset; > > unsigned int num_bits_resol; > > const unsigned int *reg_values; > > + const struct exynos_dsi_host_ops *host_ops; > > }; > > > > struct exynos_dsi { > > @@ -467,6 +474,41 @@ static const unsigned int exynos5433_reg_values[] = { > > [PHYTIMING_HS_TRAIL] = 0x0c, > > }; > > > > +static int __exynos_dsi_host_attach(struct device *dev, > > + struct mipi_dsi_device *device) > > +{ > > + struct exynos_dsi *dsi = dev_get_drvdata(dev); > > + struct drm_device *drm = dsi->encoder.dev; > > > As I wrote yesterday drm device was guaranteed to be present here only > if mipi dsi host registration was performed in component bind. In case > it is performed in probe it will be always NULL, and the code does not > make sense. Correct, but if the driver is used as a drm bridge, there won't be an encoder until bridge_attach. Mipi dsi devices defer their probe until the mipi dsi host is available. If I move the mipi dsi host registration into bridge_attach, the driver does not know if the next device is another bridge or a panel during bridge_attach, but the driver cannot successfully return from bridge_attach, because then the drm driver expects a connector. I guess that the encoder should be initialized before registering the mipi dsi host during probe instead of bind. The probe won't be rolled back on PROBE_DEFER during bind and the encoder will be available in host_attach. When splitting the driver into the exynos platform specific code and the more generic code, there won't be an encoder during host_attach in the generic code, but the host_ops callback could (and will) use the platform specific encoder, which is available before bridge_attach. Does this make sense to you? Michael > > > Regards > > Andrzej > > > > > + struct exynos_drm_crtc *crtc; > > + > > + mutex_lock(&drm->mode_config.mutex); > > + crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD); > > + crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO); > > + mutex_unlock(&drm->mode_config.mutex); > > + > > + if (drm->mode_config.poll_enabled) > > + drm_kms_helper_hotplug_event(drm); > > + > > + return 0; > > +} > > + > > +static int __exynos_dsi_host_detach(struct device *dev, > > + struct mipi_dsi_device *device) > > +{ > > + struct exynos_dsi *dsi = dev_get_drvdata(dev); > > + struct drm_device *drm = dsi->encoder.dev; > > + > > + if (drm->mode_config.poll_enabled) > > + drm_kms_helper_hotplug_event(drm); > > + > > + return 0; > > +} > > + > > +static const struct exynos_dsi_host_ops exynos_dsi_host_ops = { > > + .attach = __exynos_dsi_host_attach, > > + .detach = __exynos_dsi_host_detach, > > +}; > > + > > static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { > > .reg_ofs = EXYNOS_REG_OFS, > > .plltmr_reg = 0x50, > > @@ -477,6 +519,7 @@ static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { > > .wait_for_reset = 1, > > .num_bits_resol = 11, > > .reg_values = reg_values, > > + .host_ops = &exynos_dsi_host_ops, > > }; > > > > static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { > > @@ -489,6 +532,7 @@ static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { > > .wait_for_reset = 1, > > .num_bits_resol = 11, > > .reg_values = reg_values, > > + .host_ops = &exynos_dsi_host_ops, > > }; > > > > static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { > > @@ -499,6 +543,7 @@ static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { > > .wait_for_reset = 1, > > .num_bits_resol = 11, > > .reg_values = reg_values, > > + .host_ops = &exynos_dsi_host_ops, > > }; > > > > static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { > > @@ -510,6 +555,7 @@ static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { > > .wait_for_reset = 0, > > .num_bits_resol = 12, > > .reg_values = exynos5433_reg_values, > > + .host_ops = &exynos_dsi_host_ops, > > }; > > > > static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { > > @@ -521,6 +567,7 @@ static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { > > .wait_for_reset = 1, > > .num_bits_resol = 12, > > .reg_values = exynos5422_reg_values, > > + .host_ops = &exynos_dsi_host_ops, > > }; > > > > static const struct of_device_id exynos_dsi_of_match[] = { > > @@ -1551,8 +1598,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > struct mipi_dsi_device *device) > > { > > struct exynos_dsi *dsi = host_to_dsi(host); > > + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; > > struct drm_encoder *encoder = &dsi->encoder; > > - struct drm_device *drm = encoder->dev; > > struct drm_bridge *out_bridge; > > > > out_bridge = of_drm_find_bridge(device->dev.of_node); > > @@ -1590,18 +1637,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > return ret; > > } > > > > - mutex_lock(&drm->mode_config.mutex); > > - > > dsi->lanes = device->lanes; > > dsi->format = device->format; > > dsi->mode_flags = device->mode_flags; > > - exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode = > > - !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO); > > > > - mutex_unlock(&drm->mode_config.mutex); > > - > > - if (drm->mode_config.poll_enabled) > > - drm_kms_helper_hotplug_event(drm); > > + if (ops && ops->attach) > > + ops->attach(dsi->dsi_host.dev, device); > > > > return 0; > > } > > @@ -1610,6 +1651,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, > > struct mipi_dsi_device *device) > > { > > struct exynos_dsi *dsi = host_to_dsi(host); > > + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; > > struct drm_device *drm = dsi->encoder.dev; > > > > if (dsi->panel) { > > @@ -1625,8 +1667,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, > > INIT_LIST_HEAD(&dsi->bridge_chain); > > } > > > > - if (drm->mode_config.poll_enabled) > > - drm_kms_helper_hotplug_event(drm); > > + if (ops && ops->detach) > > + ops->detach(dsi->dsi_host.dev, device); > > > > exynos_dsi_unregister_te_irq(dsi); > > >
W dniu 15.09.2020 o 20:02, Michael Tretter pisze: > On Tue, 15 Sep 2020 19:07:59 +0200, Andrzej Hajda wrote: >> W dniu 11.09.2020 o 15:54, Michael Tretter pisze: >>> Platform drivers need to be aware if a mipi dsi device attaches or >>> detaches. Add host_ops to the driver_data for attach and detach >>> callbacks and move the i80 mode selection and the hotplug handling into >>> the callback, because these depend on the drm driver. >>> >>> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> >>> --- >>> v2: >>> - new patch >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++----- >>> 1 file changed, 53 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> index 1a15ae71205d..684a2fbef08a 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> @@ -239,6 +239,12 @@ struct exynos_dsi_transfer { >>> #define DSIM_STATE_CMD_LPM BIT(2) >>> #define DSIM_STATE_VIDOUT_AVAILABLE BIT(3) >>> >>> +struct exynos_dsi; >>> +struct exynos_dsi_host_ops { >>> + int (*attach)(struct device *dev, struct mipi_dsi_device *device); >>> + int (*detach)(struct device *dev, struct mipi_dsi_device *device); >>> +}; >>> + >>> enum exynos_reg_offset { >>> EXYNOS_REG_OFS, >>> EXYNOS5433_REG_OFS >>> @@ -254,6 +260,7 @@ struct exynos_dsi_driver_data { >>> unsigned int wait_for_reset; >>> unsigned int num_bits_resol; >>> const unsigned int *reg_values; >>> + const struct exynos_dsi_host_ops *host_ops; >>> }; >>> >>> struct exynos_dsi { >>> @@ -467,6 +474,41 @@ static const unsigned int exynos5433_reg_values[] = { >>> [PHYTIMING_HS_TRAIL] = 0x0c, >>> }; >>> >>> +static int __exynos_dsi_host_attach(struct device *dev, >>> + struct mipi_dsi_device *device) >>> +{ >>> + struct exynos_dsi *dsi = dev_get_drvdata(dev); >>> + struct drm_device *drm = dsi->encoder.dev; >> >> As I wrote yesterday drm device was guaranteed to be present here only >> if mipi dsi host registration was performed in component bind. In case >> it is performed in probe it will be always NULL, and the code does not >> make sense. > Correct, but if the driver is used as a drm bridge, there won't be an encoder > until bridge_attach. Mipi dsi devices defer their probe until the mipi dsi > host is available. If I move the mipi dsi host registration into > bridge_attach, the driver does not know if the next device is another bridge > or a panel during bridge_attach, but the driver cannot successfully return > from bridge_attach, because then the drm driver expects a connector. Hmm, I am not sure why do you think driver expects connector on return of briddge_attach. > > I guess that the encoder should be initialized before registering the mipi dsi > host during probe instead of bind. But you should have already drm dev on encoder init which in probe is unavailable. > The probe won't be rolled back on > PROBE_DEFER during bind and the encoder will be available in host_attach. > When splitting the driver into the exynos platform specific code and the more > generic code, there won't be an encoder during host_attach in the generic > code, but the host_ops callback could (and will) use the platform specific > encoder, which is available before bridge_attach. > > Does this make sense to you? Nope :) But maybe I am missing sth. Generally I see two ways (which I have already described in different e-mail, in different words): A. Static - we wait for every part of display stack to be probed, then create drm_dev - typical approach, but slow (deferred probe causes late drm creation), and racy - only(?) component framework and DSI bus have possibility to signal driver unbind, so we can react on it properly. B. Dynamic - drm framework requires only crtcs and encoders to be attached to drm on init, connectors, and hidden parts (drm_bridges, drm_panels) can be created/destroyed and attached/detached at any time (almost), so lets take advantage of it - create drm_dev ASAP and attach other parts when they become available, the only issue is that there is no generic way to be notified when given parts becomes available - in interesting area only mipi devices have such notifications via attach callbacks. So either we convert exynos_dsi to A either we continue B approach. In second case we should assure mipi_dsi host is created if drm_dev is available. Regards Andrzej > > Michael > >> >> Regards >> >> Andrzej >> >> >> >>> + struct exynos_drm_crtc *crtc; >>> + >>> + mutex_lock(&drm->mode_config.mutex); >>> + crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD); >>> + crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO); >>> + mutex_unlock(&drm->mode_config.mutex); >>> + >>> + if (drm->mode_config.poll_enabled) >>> + drm_kms_helper_hotplug_event(drm); >>> + >>> + return 0; >>> +} >>> + >>> +static int __exynos_dsi_host_detach(struct device *dev, >>> + struct mipi_dsi_device *device) >>> +{ >>> + struct exynos_dsi *dsi = dev_get_drvdata(dev); >>> + struct drm_device *drm = dsi->encoder.dev; >>> + >>> + if (drm->mode_config.poll_enabled) >>> + drm_kms_helper_hotplug_event(drm); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct exynos_dsi_host_ops exynos_dsi_host_ops = { >>> + .attach = __exynos_dsi_host_attach, >>> + .detach = __exynos_dsi_host_detach, >>> +}; >>> + >>> static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { >>> .reg_ofs = EXYNOS_REG_OFS, >>> .plltmr_reg = 0x50, >>> @@ -477,6 +519,7 @@ static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { >>> .wait_for_reset = 1, >>> .num_bits_resol = 11, >>> .reg_values = reg_values, >>> + .host_ops = &exynos_dsi_host_ops, >>> }; >>> >>> static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { >>> @@ -489,6 +532,7 @@ static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { >>> .wait_for_reset = 1, >>> .num_bits_resol = 11, >>> .reg_values = reg_values, >>> + .host_ops = &exynos_dsi_host_ops, >>> }; >>> >>> static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { >>> @@ -499,6 +543,7 @@ static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { >>> .wait_for_reset = 1, >>> .num_bits_resol = 11, >>> .reg_values = reg_values, >>> + .host_ops = &exynos_dsi_host_ops, >>> }; >>> >>> static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { >>> @@ -510,6 +555,7 @@ static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { >>> .wait_for_reset = 0, >>> .num_bits_resol = 12, >>> .reg_values = exynos5433_reg_values, >>> + .host_ops = &exynos_dsi_host_ops, >>> }; >>> >>> static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { >>> @@ -521,6 +567,7 @@ static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { >>> .wait_for_reset = 1, >>> .num_bits_resol = 12, >>> .reg_values = exynos5422_reg_values, >>> + .host_ops = &exynos_dsi_host_ops, >>> }; >>> >>> static const struct of_device_id exynos_dsi_of_match[] = { >>> @@ -1551,8 +1598,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, >>> struct mipi_dsi_device *device) >>> { >>> struct exynos_dsi *dsi = host_to_dsi(host); >>> + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; >>> struct drm_encoder *encoder = &dsi->encoder; >>> - struct drm_device *drm = encoder->dev; >>> struct drm_bridge *out_bridge; >>> >>> out_bridge = of_drm_find_bridge(device->dev.of_node); >>> @@ -1590,18 +1637,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, >>> return ret; >>> } >>> >>> - mutex_lock(&drm->mode_config.mutex); >>> - >>> dsi->lanes = device->lanes; >>> dsi->format = device->format; >>> dsi->mode_flags = device->mode_flags; >>> - exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode = >>> - !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO); >>> >>> - mutex_unlock(&drm->mode_config.mutex); >>> - >>> - if (drm->mode_config.poll_enabled) >>> - drm_kms_helper_hotplug_event(drm); >>> + if (ops && ops->attach) >>> + ops->attach(dsi->dsi_host.dev, device); >>> >>> return 0; >>> } >>> @@ -1610,6 +1651,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, >>> struct mipi_dsi_device *device) >>> { >>> struct exynos_dsi *dsi = host_to_dsi(host); >>> + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; >>> struct drm_device *drm = dsi->encoder.dev; >>> >>> if (dsi->panel) { >>> @@ -1625,8 +1667,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, >>> INIT_LIST_HEAD(&dsi->bridge_chain); >>> } >>> >>> - if (drm->mode_config.poll_enabled) >>> - drm_kms_helper_hotplug_event(drm); >>> + if (ops && ops->detach) >>> + ops->detach(dsi->dsi_host.dev, device); >>> >>> exynos_dsi_unregister_te_irq(dsi); >>> > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://protect2.fireeye.com/v1/url?k=80c78954-dd15920c-80c6021b-0cc47a31c8b4-c39fad41cb70b194&q=1&e=b51d6682-ba72-48c0-b0c9-013866ba39ab&u=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 1a15ae71205d..684a2fbef08a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -239,6 +239,12 @@ struct exynos_dsi_transfer { #define DSIM_STATE_CMD_LPM BIT(2) #define DSIM_STATE_VIDOUT_AVAILABLE BIT(3) +struct exynos_dsi; +struct exynos_dsi_host_ops { + int (*attach)(struct device *dev, struct mipi_dsi_device *device); + int (*detach)(struct device *dev, struct mipi_dsi_device *device); +}; + enum exynos_reg_offset { EXYNOS_REG_OFS, EXYNOS5433_REG_OFS @@ -254,6 +260,7 @@ struct exynos_dsi_driver_data { unsigned int wait_for_reset; unsigned int num_bits_resol; const unsigned int *reg_values; + const struct exynos_dsi_host_ops *host_ops; }; struct exynos_dsi { @@ -467,6 +474,41 @@ static const unsigned int exynos5433_reg_values[] = { [PHYTIMING_HS_TRAIL] = 0x0c, }; +static int __exynos_dsi_host_attach(struct device *dev, + struct mipi_dsi_device *device) +{ + struct exynos_dsi *dsi = dev_get_drvdata(dev); + struct drm_device *drm = dsi->encoder.dev; + struct exynos_drm_crtc *crtc; + + mutex_lock(&drm->mode_config.mutex); + crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD); + crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO); + mutex_unlock(&drm->mode_config.mutex); + + if (drm->mode_config.poll_enabled) + drm_kms_helper_hotplug_event(drm); + + return 0; +} + +static int __exynos_dsi_host_detach(struct device *dev, + struct mipi_dsi_device *device) +{ + struct exynos_dsi *dsi = dev_get_drvdata(dev); + struct drm_device *drm = dsi->encoder.dev; + + if (drm->mode_config.poll_enabled) + drm_kms_helper_hotplug_event(drm); + + return 0; +} + +static const struct exynos_dsi_host_ops exynos_dsi_host_ops = { + .attach = __exynos_dsi_host_attach, + .detach = __exynos_dsi_host_detach, +}; + static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { .reg_ofs = EXYNOS_REG_OFS, .plltmr_reg = 0x50, @@ -477,6 +519,7 @@ static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { .wait_for_reset = 1, .num_bits_resol = 11, .reg_values = reg_values, + .host_ops = &exynos_dsi_host_ops, }; static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { @@ -489,6 +532,7 @@ static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { .wait_for_reset = 1, .num_bits_resol = 11, .reg_values = reg_values, + .host_ops = &exynos_dsi_host_ops, }; static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { @@ -499,6 +543,7 @@ static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { .wait_for_reset = 1, .num_bits_resol = 11, .reg_values = reg_values, + .host_ops = &exynos_dsi_host_ops, }; static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { @@ -510,6 +555,7 @@ static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { .wait_for_reset = 0, .num_bits_resol = 12, .reg_values = exynos5433_reg_values, + .host_ops = &exynos_dsi_host_ops, }; static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { @@ -521,6 +567,7 @@ static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { .wait_for_reset = 1, .num_bits_resol = 12, .reg_values = exynos5422_reg_values, + .host_ops = &exynos_dsi_host_ops, }; static const struct of_device_id exynos_dsi_of_match[] = { @@ -1551,8 +1598,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) { struct exynos_dsi *dsi = host_to_dsi(host); + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; struct drm_encoder *encoder = &dsi->encoder; - struct drm_device *drm = encoder->dev; struct drm_bridge *out_bridge; out_bridge = of_drm_find_bridge(device->dev.of_node); @@ -1590,18 +1637,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, return ret; } - mutex_lock(&drm->mode_config.mutex); - dsi->lanes = device->lanes; dsi->format = device->format; dsi->mode_flags = device->mode_flags; - exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode = - !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO); - mutex_unlock(&drm->mode_config.mutex); - - if (drm->mode_config.poll_enabled) - drm_kms_helper_hotplug_event(drm); + if (ops && ops->attach) + ops->attach(dsi->dsi_host.dev, device); return 0; } @@ -1610,6 +1651,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) { struct exynos_dsi *dsi = host_to_dsi(host); + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; struct drm_device *drm = dsi->encoder.dev; if (dsi->panel) { @@ -1625,8 +1667,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, INIT_LIST_HEAD(&dsi->bridge_chain); } - if (drm->mode_config.poll_enabled) - drm_kms_helper_hotplug_event(drm); + if (ops && ops->detach) + ops->detach(dsi->dsi_host.dev, device); exynos_dsi_unregister_te_irq(dsi);
Platform drivers need to be aware if a mipi dsi device attaches or detaches. Add host_ops to the driver_data for attach and detach callbacks and move the i80 mode selection and the hotplug handling into the callback, because these depend on the drm driver. Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> --- v2: - new patch --- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++----- 1 file changed, 53 insertions(+), 11 deletions(-)