| Message ID | 20200211072858.30784-3-samuel@sholland.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/sun4i: dsi: Remove unused drv from driver context | expand |
Maxime, On 2/11/20 1:28 AM, Samuel Holland wrote: > Currently, the DSI host blocks binding the display pipeline until the > panel is available. This unnecessarily prevents other display outpus > from working, and adds logspam to dmesg when the panel driver is built > as a module (the component master is unsuccessfully brought up several > times during boot). > > Flip the dependency, instead requiring the host to be bound before the > panel is attached. The panel driver provides no functionality outside of > the display pipeline anyway. > > Since the panel is now probed after the DRM connector, we need a hotplug > event to turn on the connector after the panel is attached. > > This has the added benefit of fixing panel module removal/insertion. > Previously, the panel would be turned off when its module was removed. > But because the connector state was hardcoded, nothing knew to turn the > panel back on when it was re-attached. Now, with hotplug events > available, the connector state will follow the panel module state, and > the panel will be re-enabled properly. > > Fixes: 133add5b5ad4 ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller support") > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 22 ++++++++++++++++------ > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 1 + > 2 files changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > index 019fdf4ec274..ef35ce5a9bb0 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > @@ -804,7 +804,10 @@ static struct drm_connector_helper_funcs sun6i_dsi_connector_helper_funcs = { > static enum drm_connector_status > sun6i_dsi_connector_detect(struct drm_connector *connector, bool force) > { > - return connector_status_connected; > + struct sun6i_dsi *dsi = connector_to_sun6i_dsi(connector); > + > + return dsi->panel ? connector_status_connected : > + connector_status_disconnected; > } > > static const struct drm_connector_funcs sun6i_dsi_connector_funcs = { > @@ -945,10 +948,15 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host, > > if (IS_ERR(panel)) > return PTR_ERR(panel); > + if (!dsi->drm) > + return -EPROBE_DEFER; There's actually a bug here. If the panel and DSI drivers are loaded in parallel, sun6i_dsi_attach() can be called after sun6i_dsi_bind() but before sun4i_framebuffer_init() initializes drm->mode_config.funcs, causing the hotplug call to crash. This check also needs to consider dsi->drm->registered before allowing the panel to be added. I can send a v2 or a follow-up, whichever you prefer. Thanks, Samuel > dsi->panel = panel; > dsi->device = device; > > + drm_panel_attach(dsi->panel, &dsi->connector); > + drm_kms_helper_hotplug_event(dsi->drm); > + > dev_info(host->dev, "Attached device %s\n", device->name); > > return 0; > @@ -958,10 +966,14 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host, > struct mipi_dsi_device *device) > { > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > + struct drm_panel *panel = dsi->panel; > > dsi->panel = NULL; > dsi->device = NULL; > > + drm_panel_detach(panel); > + drm_kms_helper_hotplug_event(dsi->drm); > + > return 0; > } > > @@ -1026,9 +1038,6 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, > struct sun6i_dsi *dsi = dev_get_drvdata(dev); > int ret; > > - if (!dsi->panel) > - return -EPROBE_DEFER; > - > drm_encoder_helper_add(&dsi->encoder, > &sun6i_dsi_enc_helper_funcs); > ret = drm_encoder_init(drm, > @@ -1054,7 +1063,8 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, > } > > drm_connector_attach_encoder(&dsi->connector, &dsi->encoder); > - drm_panel_attach(dsi->panel, &dsi->connector); > + > + dsi->drm = drm; > > return 0; > > @@ -1068,7 +1078,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master, > { > struct sun6i_dsi *dsi = dev_get_drvdata(dev); > > - drm_panel_detach(dsi->panel); > + dsi->drm = NULL; > } > > static const struct component_ops sun6i_dsi_ops = { > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > index 61e88ea6044d..c863900ae3b4 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > @@ -29,6 +29,7 @@ struct sun6i_dsi { > > struct device *dev; > struct mipi_dsi_device *device; > + struct drm_device *drm; > struct drm_panel *panel; > }; > >
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 019fdf4ec274..ef35ce5a9bb0 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -804,7 +804,10 @@ static struct drm_connector_helper_funcs sun6i_dsi_connector_helper_funcs = { static enum drm_connector_status sun6i_dsi_connector_detect(struct drm_connector *connector, bool force) { - return connector_status_connected; + struct sun6i_dsi *dsi = connector_to_sun6i_dsi(connector); + + return dsi->panel ? connector_status_connected : + connector_status_disconnected; } static const struct drm_connector_funcs sun6i_dsi_connector_funcs = { @@ -945,10 +948,15 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host, if (IS_ERR(panel)) return PTR_ERR(panel); + if (!dsi->drm) + return -EPROBE_DEFER; dsi->panel = panel; dsi->device = device; + drm_panel_attach(dsi->panel, &dsi->connector); + drm_kms_helper_hotplug_event(dsi->drm); + dev_info(host->dev, "Attached device %s\n", device->name); return 0; @@ -958,10 +966,14 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) { struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); + struct drm_panel *panel = dsi->panel; dsi->panel = NULL; dsi->device = NULL; + drm_panel_detach(panel); + drm_kms_helper_hotplug_event(dsi->drm); + return 0; } @@ -1026,9 +1038,6 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, struct sun6i_dsi *dsi = dev_get_drvdata(dev); int ret; - if (!dsi->panel) - return -EPROBE_DEFER; - drm_encoder_helper_add(&dsi->encoder, &sun6i_dsi_enc_helper_funcs); ret = drm_encoder_init(drm, @@ -1054,7 +1063,8 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, } drm_connector_attach_encoder(&dsi->connector, &dsi->encoder); - drm_panel_attach(dsi->panel, &dsi->connector); + + dsi->drm = drm; return 0; @@ -1068,7 +1078,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master, { struct sun6i_dsi *dsi = dev_get_drvdata(dev); - drm_panel_detach(dsi->panel); + dsi->drm = NULL; } static const struct component_ops sun6i_dsi_ops = { diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h index 61e88ea6044d..c863900ae3b4 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h @@ -29,6 +29,7 @@ struct sun6i_dsi { struct device *dev; struct mipi_dsi_device *device; + struct drm_device *drm; struct drm_panel *panel; };
Currently, the DSI host blocks binding the display pipeline until the panel is available. This unnecessarily prevents other display outpus from working, and adds logspam to dmesg when the panel driver is built as a module (the component master is unsuccessfully brought up several times during boot). Flip the dependency, instead requiring the host to be bound before the panel is attached. The panel driver provides no functionality outside of the display pipeline anyway. Since the panel is now probed after the DRM connector, we need a hotplug event to turn on the connector after the panel is attached. This has the added benefit of fixing panel module removal/insertion. Previously, the panel would be turned off when its module was removed. But because the connector state was hardcoded, nothing knew to turn the panel back on when it was re-attached. Now, with hotplug events available, the connector state will follow the panel module state, and the panel will be re-enabled properly. Fixes: 133add5b5ad4 ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller support") Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 22 ++++++++++++++++------ drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 1 + 2 files changed, 17 insertions(+), 6 deletions(-)