| Message ID | 20180821140515.22246-8-heiko@sntech.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/rockchip: migrate to common dw-mipi-dsi bridge and dual-dsi | expand |
On 21.08.2018 16:05, Heiko Stuebner wrote: > Add the Rockchip-sepcific dual-dsi setup and hook it into the VOP as well. > As described in the general dual-dsi devicetree binding, the panel should > define two input ports and point each of them to one of the used dsi- > controllers, as well as declare one of them as clock-master. > This is used to determine the dual-dsi state and get access to both > controller instances. > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > v4: > add component directly in probe when adding empty dsi slave controller > v5: > use driver-internal mechanism to find dual dsi slave > --- > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 105 ++++++++++++++++++ > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 + > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 4 + > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + > 5 files changed, 114 insertions(+) > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > index b3aae8439aa3..e4aee2ccbf4d 100644 > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > @@ -218,6 +218,10 @@ struct dw_mipi_dsi_rockchip { > struct clk *grf_clk; > struct clk *phy_cfg_clk; > > + /* dual-channel */ > + bool is_slave; > + struct dw_mipi_dsi_rockchip *slave; > + > unsigned int lane_mbps; /* per lane */ > u16 input_div; > u16 feedback_div; > @@ -226,6 +230,7 @@ struct dw_mipi_dsi_rockchip { > struct dw_mipi_dsi *dmd; > const struct rockchip_dw_dsi_chip_data *cdata; > struct dw_mipi_dsi_plat_data pdata; > + int devcnt; > }; > > struct dphy_pll_parameter_map { > @@ -602,6 +607,8 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder, > } > > s->output_type = DRM_MODE_CONNECTOR_DSI; > + if (dsi->slave) > + s->output_flags = ROCKCHIP_OUTPUT_DSI_DUAL; > > return 0; > } > @@ -617,6 +624,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) > return; > > pm_runtime_get_sync(dsi->dev); > + if (dsi->slave) > + pm_runtime_get_sync(dsi->slave->dev); > > /* > * For the RK3399, the clk of grf must be enabled before writing grf > @@ -630,6 +639,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) > } > > dw_mipi_dsi_rockchip_config(dsi, mux); > + if (dsi->slave) > + dw_mipi_dsi_rockchip_config(dsi->slave, mux); > > clk_disable_unprepare(dsi->grf_clk); > } > @@ -638,6 +649,8 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) > { > struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); > > + if (dsi->slave) > + pm_runtime_put(dsi->slave->dev); > pm_runtime_put(dsi->dev); > } > > @@ -673,14 +686,76 @@ static int rockchip_dsi_drm_create_encoder(struct dw_mipi_dsi_rockchip *dsi, > return 0; > } > > +static int dw_mipi_dsi_rockchip_match_second(struct device *dev, void *data) > +{ > + struct dw_mipi_dsi_rockchip *dsi = data; > + struct drm_bridge *bridge1, *bridge2; > + struct drm_panel *panel1, *panel2; > + int ret; > + > + if (dsi->dev->of_node == dev->of_node) > + return 0; > + > + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, > + &panel1, &bridge1); > + if (ret) > + return ret; > + > + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, > + &panel2, &bridge2); > + if (ret) > + return ret; > + > + return (panel1 == panel2) && (bridge1 == bridge2); > +} > + > static int dw_mipi_dsi_rockchip_bind(struct device *dev, > struct device *master, > void *data) > { > struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); > struct drm_device *drm_dev = data; > + struct device *second; > + bool master1, master2; > int ret; > > + second = driver_find_device(dsi->dev->driver, NULL, dsi, > + dw_mipi_dsi_rockchip_match_second); This function performs get_device on the matched device, so you should probably call somewhere put_device to make the counter balanced. I hope second device is somehow guarded by component framework from being unbound in unexpected moments (ie. when in use by master). > + if (IS_ERR(second)) > + return PTR_ERR(second); > + > + if (second) { > + master1 = of_property_read_bool(dsi->dev->of_node, > + "clock-master"); > + master2 = of_property_read_bool(second->of_node, > + "clock-master"); > + > + if (master1 && master2) { > + DRM_DEV_ERROR(dsi->dev, "only one clock-master allowed\n"); > + return -EINVAL; > + } > + > + if (!master1 && !master2) { > + DRM_DEV_ERROR(dsi->dev, "no clock-master defined\n"); > + return -EINVAL; > + } > + > + /* we are the slave in dual-DSI */ > + if (!master1) { > + dsi->is_slave = true; > + return 0; > + } > + > + dsi->slave = dev_get_drvdata(second); > + if (!dsi->slave) { > + DRM_DEV_ERROR(dev, "could not get slaves data\n"); > + return -ENODEV; > + } > + > + dsi->slave->is_slave = true; > + dw_mipi_dsi_set_slave(dsi->dmd, dsi->slave->dmd); > + } > + > ret = clk_prepare_enable(dsi->pllref_clk); > if (ret) { > DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret); > @@ -708,6 +783,9 @@ static void dw_mipi_dsi_rockchip_unbind(struct device *dev, > { > struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); > > + if (dsi->is_slave) > + return; > + > dw_mipi_dsi_unbind(dsi->dmd); > > clk_disable_unprepare(dsi->pllref_clk); > @@ -749,6 +827,14 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = { > .detach = dw_mipi_dsi_rockchip_host_detach, > }; > > +static int dw_mipi_dsi_rockchip_cnt_dev(struct device *dev, void *data) > +{ > + struct dw_mipi_dsi_rockchip *dsi = data; > + > + dsi->devcnt++; > + return 0; > +} > + > static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -835,6 +921,22 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) > goto err_clkdisable; > } > > + /* > + * All dsi child devices will have been created, so we can check > + * their number and add the component here if there will be no > + * host-attach call. I guess you mean that in this point all child devices have been created (remove 'will'), but it may be false assumption, there is always possibility to create device on the fly - see mipi_dsi_device_register_full. You cannot assume that in certain point in time there will be no more devices on the bus. I am not sure of the purpose of this code. Why do you want to behave differently if there are no child devs? Regards Andrzej > + */ > + device_for_each_child(dsi->dev, dsi, dw_mipi_dsi_rockchip_cnt_dev); > + if (dsi->devcnt == 0) { > + ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_ops); > + if (ret) { > + DRM_DEV_ERROR(dsi->dev, "Failed to register component: %d\n", > + ret); > + dw_mipi_dsi_remove(dsi->dmd); > + goto err_clkdisable; > + } > + } > + > return 0; > > err_clkdisable: > @@ -846,6 +948,9 @@ static int dw_mipi_dsi_rockchip_remove(struct platform_device *pdev) > { > struct dw_mipi_dsi_rockchip *dsi = platform_get_drvdata(pdev); > > + if (dsi->devcnt == 0) > + component_del(dsi->dev, &dw_mipi_dsi_rockchip_ops); > + > dw_mipi_dsi_remove(dsi->dmd); > > return 0; > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > index 67bbd274b71f..acdd4facca67 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > @@ -37,6 +37,7 @@ struct rockchip_crtc_state { > int output_type; > int output_mode; > int output_bpc; > + int output_flags; > }; > #define to_rockchip_crtc_state(s) \ > container_of(s, struct rockchip_crtc_state, base) > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index c9a5ea38e86b..e4c39356f79e 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -910,6 +910,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, > pin_pol |= (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC) ? > BIT(VSYNC_POSITIVE) : 0; > VOP_REG_SET(vop, output, pin_pol, pin_pol); > + VOP_REG_SET(vop, output, mipi_dual_channel_en, 0); > > switch (s->output_type) { > case DRM_MODE_CONNECTOR_LVDS: > @@ -927,6 +928,8 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, > case DRM_MODE_CONNECTOR_DSI: > VOP_REG_SET(vop, output, mipi_pin_pol, pin_pol); > VOP_REG_SET(vop, output, mipi_en, 1); > + VOP_REG_SET(vop, output, mipi_dual_channel_en, > + !!(s->output_flags & ROCKCHIP_OUTPUT_DSI_DUAL)); > break; > case DRM_MODE_CONNECTOR_DisplayPort: > pin_pol &= ~BIT(DCLK_INVERT); > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h > index fcb91041a666..361e4c7d225c 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h > @@ -60,6 +60,7 @@ struct vop_output { > struct vop_reg edp_en; > struct vop_reg hdmi_en; > struct vop_reg mipi_en; > + struct vop_reg mipi_dual_channel_en; > struct vop_reg rgb_en; > }; > > @@ -213,6 +214,9 @@ struct vop_data { > /* for use special outface */ > #define ROCKCHIP_OUT_MODE_AAAA 15 > > +/* output flags */ > +#define ROCKCHIP_OUTPUT_DSI_DUAL BIT(0) > + > enum alpha_mode { > ALPHA_STRAIGHT, > ALPHA_INVERSE, > diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c > index d824ca60c1a4..44d73c7beb5c 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c > +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c > @@ -543,6 +543,7 @@ static const struct vop_output rk3399_output = { > .hdmi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 13), > .edp_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 14), > .mipi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 15), > + .mipi_dual_channel_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 3), > }; > > static const struct vop_data rk3399_vop_big = {
Hi Andrzej, Am Montag, 27. August 2018, 13:34:07 CEST schrieb Andrzej Hajda: > On 21.08.2018 16:05, Heiko Stuebner wrote: > > Add the Rockchip-sepcific dual-dsi setup and hook it into the VOP as well. > > As described in the general dual-dsi devicetree binding, the panel should > > define two input ports and point each of them to one of the used dsi- > > controllers, as well as declare one of them as clock-master. > > This is used to determine the dual-dsi state and get access to both > > controller instances. > > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > v4: > > add component directly in probe when adding empty dsi slave controller > > v5: > > use driver-internal mechanism to find dual dsi slave > > --- > > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 105 ++++++++++++++++++ > > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + > > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 + > > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 4 + > > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + > > 5 files changed, 114 insertions(+) > > > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > index b3aae8439aa3..e4aee2ccbf4d 100644 > > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > @@ -218,6 +218,10 @@ struct dw_mipi_dsi_rockchip { > > struct clk *grf_clk; > > struct clk *phy_cfg_clk; > > > > + /* dual-channel */ > > + bool is_slave; > > + struct dw_mipi_dsi_rockchip *slave; > > + > > unsigned int lane_mbps; /* per lane */ > > u16 input_div; > > u16 feedback_div; > > @@ -226,6 +230,7 @@ struct dw_mipi_dsi_rockchip { > > struct dw_mipi_dsi *dmd; > > const struct rockchip_dw_dsi_chip_data *cdata; > > struct dw_mipi_dsi_plat_data pdata; > > + int devcnt; > > }; > > > > struct dphy_pll_parameter_map { > > @@ -602,6 +607,8 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder, > > } > > > > s->output_type = DRM_MODE_CONNECTOR_DSI; > > + if (dsi->slave) > > + s->output_flags = ROCKCHIP_OUTPUT_DSI_DUAL; > > > > return 0; > > } > > @@ -617,6 +624,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) > > return; > > > > pm_runtime_get_sync(dsi->dev); > > + if (dsi->slave) > > + pm_runtime_get_sync(dsi->slave->dev); > > > > /* > > * For the RK3399, the clk of grf must be enabled before writing grf > > @@ -630,6 +639,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) > > } > > > > dw_mipi_dsi_rockchip_config(dsi, mux); > > + if (dsi->slave) > > + dw_mipi_dsi_rockchip_config(dsi->slave, mux); > > > > clk_disable_unprepare(dsi->grf_clk); > > } > > @@ -638,6 +649,8 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) > > { > > struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); > > > > + if (dsi->slave) > > + pm_runtime_put(dsi->slave->dev); > > pm_runtime_put(dsi->dev); > > } > > > > @@ -673,14 +686,76 @@ static int rockchip_dsi_drm_create_encoder(struct dw_mipi_dsi_rockchip *dsi, > > return 0; > > } > > > > +static int dw_mipi_dsi_rockchip_match_second(struct device *dev, void *data) > > +{ > > + struct dw_mipi_dsi_rockchip *dsi = data; > > + struct drm_bridge *bridge1, *bridge2; > > + struct drm_panel *panel1, *panel2; > > + int ret; > > + > > + if (dsi->dev->of_node == dev->of_node) > > + return 0; > > + > > + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, > > + &panel1, &bridge1); > > + if (ret) > > + return ret; > > + > > + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, > > + &panel2, &bridge2); > > + if (ret) > > + return ret; > > + > > + return (panel1 == panel2) && (bridge1 == bridge2); > > +} > > + > > static int dw_mipi_dsi_rockchip_bind(struct device *dev, > > struct device *master, > > void *data) > > { > > struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); > > struct drm_device *drm_dev = data; > > + struct device *second; > > + bool master1, master2; > > int ret; > > > > + second = driver_find_device(dsi->dev->driver, NULL, dsi, > > + dw_mipi_dsi_rockchip_match_second); > > This function performs get_device on the matched device, so you should > probably call somewhere put_device to make the counter balanced. > I hope second device is somehow guarded by component framework from > being unbound in unexpected moments (ie. when in use by master). Thanks for that catch, I'll add the put_device. > > + if (IS_ERR(second)) > > + return PTR_ERR(second); > > + > > + if (second) { > > + master1 = of_property_read_bool(dsi->dev->of_node, > > + "clock-master"); > > + master2 = of_property_read_bool(second->of_node, > > + "clock-master"); > > + > > + if (master1 && master2) { > > + DRM_DEV_ERROR(dsi->dev, "only one clock-master allowed\n"); > > + return -EINVAL; > > + } > > + > > + if (!master1 && !master2) { > > + DRM_DEV_ERROR(dsi->dev, "no clock-master defined\n"); > > + return -EINVAL; > > + } > > + > > + /* we are the slave in dual-DSI */ > > + if (!master1) { > > + dsi->is_slave = true; > > + return 0; > > + } > > + > > + dsi->slave = dev_get_drvdata(second); > > + if (!dsi->slave) { > > + DRM_DEV_ERROR(dev, "could not get slaves data\n"); > > + return -ENODEV; > > + } > > + > > + dsi->slave->is_slave = true; > > + dw_mipi_dsi_set_slave(dsi->dmd, dsi->slave->dmd); > > + } > > + > > ret = clk_prepare_enable(dsi->pllref_clk); > > if (ret) { > > DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret); > > @@ -708,6 +783,9 @@ static void dw_mipi_dsi_rockchip_unbind(struct device *dev, > > { > > struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); > > > > + if (dsi->is_slave) > > + return; > > + > > dw_mipi_dsi_unbind(dsi->dmd); > > > > clk_disable_unprepare(dsi->pllref_clk); > > @@ -749,6 +827,14 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = { > > .detach = dw_mipi_dsi_rockchip_host_detach, > > }; > > > > +static int dw_mipi_dsi_rockchip_cnt_dev(struct device *dev, void *data) > > +{ > > + struct dw_mipi_dsi_rockchip *dsi = data; > > + > > + dsi->devcnt++; > > + return 0; > > +} > > + > > static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) > > { > > struct device *dev = &pdev->dev; > > @@ -835,6 +921,22 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) > > goto err_clkdisable; > > } > > > > + /* > > + * All dsi child devices will have been created, so we can check > > + * their number and add the component here if there will be no > > + * host-attach call. > > I guess you mean that in this point all child devices have been created > (remove 'will'), but it may be false assumption, there is always > possibility to create device on the fly - see mipi_dsi_device_register_full. > You cannot assume that in certain point in time there will be no more > devices on the bus. > > I am not sure of the purpose of this code. Why do you want to behave > differently if there are no child devs? That's all related to the suggestion of doing component_add in the host-attach callback. If the device doesn't have active children its component will never get added and the whole drm component structure will wait forever and never probe the drm driver, hence add the component when no child will be present. [This is of course part of handling my empty dsi slave]. I don't think the whole dw-dsi driver currently expects more than one device to exist, while the DSI bus can address multiple devices. With the changes in this series with component_add being called from dsi-attach, this creates even more issues with multiple devices, as component_add and component_del would be called multiple times for each dsi device on the bus. Should I simply go back to my deferral if no panel found, or do you have another sugestion? Thanks Heiko
On 20.09.2018 13:20, Heiko Stuebner wrote: > Hi Andrzej, > > Am Montag, 27. August 2018, 13:34:07 CEST schrieb Andrzej Hajda: >> On 21.08.2018 16:05, Heiko Stuebner wrote: >>> Add the Rockchip-sepcific dual-dsi setup and hook it into the VOP as well. >>> As described in the general dual-dsi devicetree binding, the panel should >>> define two input ports and point each of them to one of the used dsi- >>> controllers, as well as declare one of them as clock-master. >>> This is used to determine the dual-dsi state and get access to both >>> controller instances. >>> >>> Signed-off-by: Heiko Stuebner <heiko@sntech.de> >>> v4: >>> add component directly in probe when adding empty dsi slave controller >>> v5: >>> use driver-internal mechanism to find dual dsi slave >>> --- >>> .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 105 ++++++++++++++++++ >>> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + >>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 + >>> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 4 + >>> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + >>> 5 files changed, 114 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c >>> index b3aae8439aa3..e4aee2ccbf4d 100644 >>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c >>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c >>> @@ -218,6 +218,10 @@ struct dw_mipi_dsi_rockchip { >>> struct clk *grf_clk; >>> struct clk *phy_cfg_clk; >>> >>> + /* dual-channel */ >>> + bool is_slave; >>> + struct dw_mipi_dsi_rockchip *slave; >>> + >>> unsigned int lane_mbps; /* per lane */ >>> u16 input_div; >>> u16 feedback_div; >>> @@ -226,6 +230,7 @@ struct dw_mipi_dsi_rockchip { >>> struct dw_mipi_dsi *dmd; >>> const struct rockchip_dw_dsi_chip_data *cdata; >>> struct dw_mipi_dsi_plat_data pdata; >>> + int devcnt; >>> }; >>> >>> struct dphy_pll_parameter_map { >>> @@ -602,6 +607,8 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder, >>> } >>> >>> s->output_type = DRM_MODE_CONNECTOR_DSI; >>> + if (dsi->slave) >>> + s->output_flags = ROCKCHIP_OUTPUT_DSI_DUAL; >>> >>> return 0; >>> } >>> @@ -617,6 +624,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) >>> return; >>> >>> pm_runtime_get_sync(dsi->dev); >>> + if (dsi->slave) >>> + pm_runtime_get_sync(dsi->slave->dev); >>> >>> /* >>> * For the RK3399, the clk of grf must be enabled before writing grf >>> @@ -630,6 +639,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) >>> } >>> >>> dw_mipi_dsi_rockchip_config(dsi, mux); >>> + if (dsi->slave) >>> + dw_mipi_dsi_rockchip_config(dsi->slave, mux); >>> >>> clk_disable_unprepare(dsi->grf_clk); >>> } >>> @@ -638,6 +649,8 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) >>> { >>> struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); >>> >>> + if (dsi->slave) >>> + pm_runtime_put(dsi->slave->dev); >>> pm_runtime_put(dsi->dev); >>> } >>> >>> @@ -673,14 +686,76 @@ static int rockchip_dsi_drm_create_encoder(struct dw_mipi_dsi_rockchip *dsi, >>> return 0; >>> } >>> >>> +static int dw_mipi_dsi_rockchip_match_second(struct device *dev, void *data) >>> +{ >>> + struct dw_mipi_dsi_rockchip *dsi = data; >>> + struct drm_bridge *bridge1, *bridge2; >>> + struct drm_panel *panel1, *panel2; >>> + int ret; >>> + >>> + if (dsi->dev->of_node == dev->of_node) >>> + return 0; >>> + >>> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, >>> + &panel1, &bridge1); >>> + if (ret) >>> + return ret; >>> + >>> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, >>> + &panel2, &bridge2); >>> + if (ret) >>> + return ret; >>> + >>> + return (panel1 == panel2) && (bridge1 == bridge2); >>> +} >>> + >>> static int dw_mipi_dsi_rockchip_bind(struct device *dev, >>> struct device *master, >>> void *data) >>> { >>> struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); >>> struct drm_device *drm_dev = data; >>> + struct device *second; >>> + bool master1, master2; >>> int ret; >>> >>> + second = driver_find_device(dsi->dev->driver, NULL, dsi, >>> + dw_mipi_dsi_rockchip_match_second); >> This function performs get_device on the matched device, so you should >> probably call somewhere put_device to make the counter balanced. >> I hope second device is somehow guarded by component framework from >> being unbound in unexpected moments (ie. when in use by master). > Thanks for that catch, I'll add the put_device. > > >>> + if (IS_ERR(second)) >>> + return PTR_ERR(second); >>> + >>> + if (second) { >>> + master1 = of_property_read_bool(dsi->dev->of_node, >>> + "clock-master"); >>> + master2 = of_property_read_bool(second->of_node, >>> + "clock-master"); >>> + >>> + if (master1 && master2) { >>> + DRM_DEV_ERROR(dsi->dev, "only one clock-master allowed\n"); >>> + return -EINVAL; >>> + } >>> + >>> + if (!master1 && !master2) { >>> + DRM_DEV_ERROR(dsi->dev, "no clock-master defined\n"); >>> + return -EINVAL; >>> + } >>> + >>> + /* we are the slave in dual-DSI */ >>> + if (!master1) { >>> + dsi->is_slave = true; >>> + return 0; >>> + } >>> + >>> + dsi->slave = dev_get_drvdata(second); >>> + if (!dsi->slave) { >>> + DRM_DEV_ERROR(dev, "could not get slaves data\n"); >>> + return -ENODEV; >>> + } >>> + >>> + dsi->slave->is_slave = true; >>> + dw_mipi_dsi_set_slave(dsi->dmd, dsi->slave->dmd); >>> + } >>> + >>> ret = clk_prepare_enable(dsi->pllref_clk); >>> if (ret) { >>> DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret); >>> @@ -708,6 +783,9 @@ static void dw_mipi_dsi_rockchip_unbind(struct device *dev, >>> { >>> struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); >>> >>> + if (dsi->is_slave) >>> + return; >>> + >>> dw_mipi_dsi_unbind(dsi->dmd); >>> >>> clk_disable_unprepare(dsi->pllref_clk); >>> @@ -749,6 +827,14 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = { >>> .detach = dw_mipi_dsi_rockchip_host_detach, >>> }; >>> >>> +static int dw_mipi_dsi_rockchip_cnt_dev(struct device *dev, void *data) >>> +{ >>> + struct dw_mipi_dsi_rockchip *dsi = data; >>> + >>> + dsi->devcnt++; >>> + return 0; >>> +} >>> + >>> static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) >>> { >>> struct device *dev = &pdev->dev; >>> @@ -835,6 +921,22 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) >>> goto err_clkdisable; >>> } >>> >>> + /* >>> + * All dsi child devices will have been created, so we can check >>> + * their number and add the component here if there will be no >>> + * host-attach call. >> I guess you mean that in this point all child devices have been created >> (remove 'will'), but it may be false assumption, there is always >> possibility to create device on the fly - see mipi_dsi_device_register_full. >> You cannot assume that in certain point in time there will be no more >> devices on the bus. >> >> I am not sure of the purpose of this code. Why do you want to behave >> differently if there are no child devs? > That's all related to the suggestion of doing component_add in the > host-attach callback. If the device doesn't have active children its > component will never get added and the whole drm component structure > will wait forever and never probe the drm driver, hence add the component > when no child will be present. What wrong with it? If the resource is required, then consumer should wait for it, if there is a problem with provider it can be forever, I think it is clear and fair rule. > [This is of course part of handling my empty dsi slave]. Could you elaborate it? If dsi slave has no downstream device why is it active at all? > > I don't think the whole dw-dsi driver currently expects more than one > device to exist, while the DSI bus can address multiple devices. > > With the changes in this series with component_add being called from > dsi-attach, this creates even more issues with multiple devices, as > component_add and component_del would be called multiple times > for each dsi device on the bus. No, there is no rule of calling component_add every time from attach. The rule is to call component_add just when all required resources are gathered. If it can happen in dsi-attach call then it should be called from there. If you expect more devices on dsi bus you should call it on gathering the last one expected. But do you really have more than one dsi device attached to the same dsi master, if not I wouldn't care too much about only theoretical issues. Regards Andrzej > > Should I simply go back to my deferral if no panel found, or do you > have another sugestion? > > Thanks > Heiko > > > >
Hi Andrzej Am Freitag, 21. September 2018, 08:11:16 CEST schrieb Andrzej Hajda: > On 20.09.2018 13:20, Heiko Stuebner wrote: > > Hi Andrzej, > > > > Am Montag, 27. August 2018, 13:34:07 CEST schrieb Andrzej Hajda: > >> On 21.08.2018 16:05, Heiko Stuebner wrote: > >>> Add the Rockchip-sepcific dual-dsi setup and hook it into the VOP as well. > >>> As described in the general dual-dsi devicetree binding, the panel should > >>> define two input ports and point each of them to one of the used dsi- > >>> controllers, as well as declare one of them as clock-master. > >>> This is used to determine the dual-dsi state and get access to both > >>> controller instances. > >>> > >>> Signed-off-by: Heiko Stuebner <heiko@sntech.de> > >>> v4: > >>> add component directly in probe when adding empty dsi slave controller > >>> v5: > >>> use driver-internal mechanism to find dual dsi slave > >>> --- > >>> .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 105 ++++++++++++++++++ > >>> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + > >>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 + > >>> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 4 + > >>> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + > >>> 5 files changed, 114 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > >>> index b3aae8439aa3..e4aee2ccbf4d 100644 > >>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > >>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > >>> @@ -218,6 +218,10 @@ struct dw_mipi_dsi_rockchip { > >>> struct clk *grf_clk; > >>> struct clk *phy_cfg_clk; > >>> > >>> + /* dual-channel */ > >>> + bool is_slave; > >>> + struct dw_mipi_dsi_rockchip *slave; > >>> + > >>> unsigned int lane_mbps; /* per lane */ > >>> u16 input_div; > >>> u16 feedback_div; > >>> @@ -226,6 +230,7 @@ struct dw_mipi_dsi_rockchip { > >>> struct dw_mipi_dsi *dmd; > >>> const struct rockchip_dw_dsi_chip_data *cdata; > >>> struct dw_mipi_dsi_plat_data pdata; > >>> + int devcnt; > >>> }; > >>> > >>> struct dphy_pll_parameter_map { > >>> @@ -602,6 +607,8 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder, > >>> } > >>> > >>> s->output_type = DRM_MODE_CONNECTOR_DSI; > >>> + if (dsi->slave) > >>> + s->output_flags = ROCKCHIP_OUTPUT_DSI_DUAL; > >>> > >>> return 0; > >>> } > >>> @@ -617,6 +624,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) > >>> return; > >>> > >>> pm_runtime_get_sync(dsi->dev); > >>> + if (dsi->slave) > >>> + pm_runtime_get_sync(dsi->slave->dev); > >>> > >>> /* > >>> * For the RK3399, the clk of grf must be enabled before writing grf > >>> @@ -630,6 +639,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) > >>> } > >>> > >>> dw_mipi_dsi_rockchip_config(dsi, mux); > >>> + if (dsi->slave) > >>> + dw_mipi_dsi_rockchip_config(dsi->slave, mux); > >>> > >>> clk_disable_unprepare(dsi->grf_clk); > >>> } > >>> @@ -638,6 +649,8 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) > >>> { > >>> struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); > >>> > >>> + if (dsi->slave) > >>> + pm_runtime_put(dsi->slave->dev); > >>> pm_runtime_put(dsi->dev); > >>> } > >>> > >>> @@ -673,14 +686,76 @@ static int rockchip_dsi_drm_create_encoder(struct dw_mipi_dsi_rockchip *dsi, > >>> return 0; > >>> } > >>> > >>> +static int dw_mipi_dsi_rockchip_match_second(struct device *dev, void *data) > >>> +{ > >>> + struct dw_mipi_dsi_rockchip *dsi = data; > >>> + struct drm_bridge *bridge1, *bridge2; > >>> + struct drm_panel *panel1, *panel2; > >>> + int ret; > >>> + > >>> + if (dsi->dev->of_node == dev->of_node) > >>> + return 0; > >>> + > >>> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, > >>> + &panel1, &bridge1); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, > >>> + &panel2, &bridge2); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + return (panel1 == panel2) && (bridge1 == bridge2); > >>> +} > >>> + > >>> static int dw_mipi_dsi_rockchip_bind(struct device *dev, > >>> struct device *master, > >>> void *data) > >>> { > >>> struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); > >>> struct drm_device *drm_dev = data; > >>> + struct device *second; > >>> + bool master1, master2; > >>> int ret; > >>> > >>> + second = driver_find_device(dsi->dev->driver, NULL, dsi, > >>> + dw_mipi_dsi_rockchip_match_second); > >> This function performs get_device on the matched device, so you should > >> probably call somewhere put_device to make the counter balanced. > >> I hope second device is somehow guarded by component framework from > >> being unbound in unexpected moments (ie. when in use by master). > > Thanks for that catch, I'll add the put_device. > > > > > >>> + if (IS_ERR(second)) > >>> + return PTR_ERR(second); > >>> + > >>> + if (second) { > >>> + master1 = of_property_read_bool(dsi->dev->of_node, > >>> + "clock-master"); > >>> + master2 = of_property_read_bool(second->of_node, > >>> + "clock-master"); > >>> + > >>> + if (master1 && master2) { > >>> + DRM_DEV_ERROR(dsi->dev, "only one clock-master allowed\n"); > >>> + return -EINVAL; > >>> + } > >>> + > >>> + if (!master1 && !master2) { > >>> + DRM_DEV_ERROR(dsi->dev, "no clock-master defined\n"); > >>> + return -EINVAL; > >>> + } > >>> + > >>> + /* we are the slave in dual-DSI */ > >>> + if (!master1) { > >>> + dsi->is_slave = true; > >>> + return 0; > >>> + } > >>> + > >>> + dsi->slave = dev_get_drvdata(second); > >>> + if (!dsi->slave) { > >>> + DRM_DEV_ERROR(dev, "could not get slaves data\n"); > >>> + return -ENODEV; > >>> + } > >>> + > >>> + dsi->slave->is_slave = true; > >>> + dw_mipi_dsi_set_slave(dsi->dmd, dsi->slave->dmd); > >>> + } > >>> + > >>> ret = clk_prepare_enable(dsi->pllref_clk); > >>> if (ret) { > >>> DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret); > >>> @@ -708,6 +783,9 @@ static void dw_mipi_dsi_rockchip_unbind(struct device *dev, > >>> { > >>> struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); > >>> > >>> + if (dsi->is_slave) > >>> + return; > >>> + > >>> dw_mipi_dsi_unbind(dsi->dmd); > >>> > >>> clk_disable_unprepare(dsi->pllref_clk); > >>> @@ -749,6 +827,14 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = { > >>> .detach = dw_mipi_dsi_rockchip_host_detach, > >>> }; > >>> > >>> +static int dw_mipi_dsi_rockchip_cnt_dev(struct device *dev, void *data) > >>> +{ > >>> + struct dw_mipi_dsi_rockchip *dsi = data; > >>> + > >>> + dsi->devcnt++; > >>> + return 0; > >>> +} > >>> + > >>> static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) > >>> { > >>> struct device *dev = &pdev->dev; > >>> @@ -835,6 +921,22 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) > >>> goto err_clkdisable; > >>> } > >>> > >>> + /* > >>> + * All dsi child devices will have been created, so we can check > >>> + * their number and add the component here if there will be no > >>> + * host-attach call. > >> I guess you mean that in this point all child devices have been created > >> (remove 'will'), but it may be false assumption, there is always > >> possibility to create device on the fly - see mipi_dsi_device_register_full. > >> You cannot assume that in certain point in time there will be no more > >> devices on the bus. > >> > >> I am not sure of the purpose of this code. Why do you want to behave > >> differently if there are no child devs? > > That's all related to the suggestion of doing component_add in the > > host-attach callback. If the device doesn't have active children its > > component will never get added and the whole drm component structure > > will wait forever and never probe the drm driver, hence add the component > > when no child will be present. > > What wrong with it? If the resource is required, then consumer should > wait for it, if there is a problem with provider it can be forever, I > think it is clear and fair rule. > > > [This is of course part of handling my empty dsi slave]. > > Could you elaborate it? If dsi slave has no downstream device why is it > active at all? As in all instances before, you were right on pointing out the flaw here :-) DSI devices can of course also sit on completely other buses, like i2c and attach from there by creating their own dsi device on the bus. I've prepared v6 which should address that in a more elegant manner while keeping the component_attach in the host-attach callback. Thanks Heiko
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c index b3aae8439aa3..e4aee2ccbf4d 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c @@ -218,6 +218,10 @@ struct dw_mipi_dsi_rockchip { struct clk *grf_clk; struct clk *phy_cfg_clk; + /* dual-channel */ + bool is_slave; + struct dw_mipi_dsi_rockchip *slave; + unsigned int lane_mbps; /* per lane */ u16 input_div; u16 feedback_div; @@ -226,6 +230,7 @@ struct dw_mipi_dsi_rockchip { struct dw_mipi_dsi *dmd; const struct rockchip_dw_dsi_chip_data *cdata; struct dw_mipi_dsi_plat_data pdata; + int devcnt; }; struct dphy_pll_parameter_map { @@ -602,6 +607,8 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder, } s->output_type = DRM_MODE_CONNECTOR_DSI; + if (dsi->slave) + s->output_flags = ROCKCHIP_OUTPUT_DSI_DUAL; return 0; } @@ -617,6 +624,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) return; pm_runtime_get_sync(dsi->dev); + if (dsi->slave) + pm_runtime_get_sync(dsi->slave->dev); /* * For the RK3399, the clk of grf must be enabled before writing grf @@ -630,6 +639,8 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) } dw_mipi_dsi_rockchip_config(dsi, mux); + if (dsi->slave) + dw_mipi_dsi_rockchip_config(dsi->slave, mux); clk_disable_unprepare(dsi->grf_clk); } @@ -638,6 +649,8 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) { struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); + if (dsi->slave) + pm_runtime_put(dsi->slave->dev); pm_runtime_put(dsi->dev); } @@ -673,14 +686,76 @@ static int rockchip_dsi_drm_create_encoder(struct dw_mipi_dsi_rockchip *dsi, return 0; } +static int dw_mipi_dsi_rockchip_match_second(struct device *dev, void *data) +{ + struct dw_mipi_dsi_rockchip *dsi = data; + struct drm_bridge *bridge1, *bridge2; + struct drm_panel *panel1, *panel2; + int ret; + + if (dsi->dev->of_node == dev->of_node) + return 0; + + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, + &panel1, &bridge1); + if (ret) + return ret; + + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, + &panel2, &bridge2); + if (ret) + return ret; + + return (panel1 == panel2) && (bridge1 == bridge2); +} + static int dw_mipi_dsi_rockchip_bind(struct device *dev, struct device *master, void *data) { struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); struct drm_device *drm_dev = data; + struct device *second; + bool master1, master2; int ret; + second = driver_find_device(dsi->dev->driver, NULL, dsi, + dw_mipi_dsi_rockchip_match_second); + if (IS_ERR(second)) + return PTR_ERR(second); + + if (second) { + master1 = of_property_read_bool(dsi->dev->of_node, + "clock-master"); + master2 = of_property_read_bool(second->of_node, + "clock-master"); + + if (master1 && master2) { + DRM_DEV_ERROR(dsi->dev, "only one clock-master allowed\n"); + return -EINVAL; + } + + if (!master1 && !master2) { + DRM_DEV_ERROR(dsi->dev, "no clock-master defined\n"); + return -EINVAL; + } + + /* we are the slave in dual-DSI */ + if (!master1) { + dsi->is_slave = true; + return 0; + } + + dsi->slave = dev_get_drvdata(second); + if (!dsi->slave) { + DRM_DEV_ERROR(dev, "could not get slaves data\n"); + return -ENODEV; + } + + dsi->slave->is_slave = true; + dw_mipi_dsi_set_slave(dsi->dmd, dsi->slave->dmd); + } + ret = clk_prepare_enable(dsi->pllref_clk); if (ret) { DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret); @@ -708,6 +783,9 @@ static void dw_mipi_dsi_rockchip_unbind(struct device *dev, { struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); + if (dsi->is_slave) + return; + dw_mipi_dsi_unbind(dsi->dmd); clk_disable_unprepare(dsi->pllref_clk); @@ -749,6 +827,14 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = { .detach = dw_mipi_dsi_rockchip_host_detach, }; +static int dw_mipi_dsi_rockchip_cnt_dev(struct device *dev, void *data) +{ + struct dw_mipi_dsi_rockchip *dsi = data; + + dsi->devcnt++; + return 0; +} + static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -835,6 +921,22 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) goto err_clkdisable; } + /* + * All dsi child devices will have been created, so we can check + * their number and add the component here if there will be no + * host-attach call. + */ + device_for_each_child(dsi->dev, dsi, dw_mipi_dsi_rockchip_cnt_dev); + if (dsi->devcnt == 0) { + ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_ops); + if (ret) { + DRM_DEV_ERROR(dsi->dev, "Failed to register component: %d\n", + ret); + dw_mipi_dsi_remove(dsi->dmd); + goto err_clkdisable; + } + } + return 0; err_clkdisable: @@ -846,6 +948,9 @@ static int dw_mipi_dsi_rockchip_remove(struct platform_device *pdev) { struct dw_mipi_dsi_rockchip *dsi = platform_get_drvdata(pdev); + if (dsi->devcnt == 0) + component_del(dsi->dev, &dw_mipi_dsi_rockchip_ops); + dw_mipi_dsi_remove(dsi->dmd); return 0; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 67bbd274b71f..acdd4facca67 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -37,6 +37,7 @@ struct rockchip_crtc_state { int output_type; int output_mode; int output_bpc; + int output_flags; }; #define to_rockchip_crtc_state(s) \ container_of(s, struct rockchip_crtc_state, base) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index c9a5ea38e86b..e4c39356f79e 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -910,6 +910,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, pin_pol |= (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC) ? BIT(VSYNC_POSITIVE) : 0; VOP_REG_SET(vop, output, pin_pol, pin_pol); + VOP_REG_SET(vop, output, mipi_dual_channel_en, 0); switch (s->output_type) { case DRM_MODE_CONNECTOR_LVDS: @@ -927,6 +928,8 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, case DRM_MODE_CONNECTOR_DSI: VOP_REG_SET(vop, output, mipi_pin_pol, pin_pol); VOP_REG_SET(vop, output, mipi_en, 1); + VOP_REG_SET(vop, output, mipi_dual_channel_en, + !!(s->output_flags & ROCKCHIP_OUTPUT_DSI_DUAL)); break; case DRM_MODE_CONNECTOR_DisplayPort: pin_pol &= ~BIT(DCLK_INVERT); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index fcb91041a666..361e4c7d225c 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -60,6 +60,7 @@ struct vop_output { struct vop_reg edp_en; struct vop_reg hdmi_en; struct vop_reg mipi_en; + struct vop_reg mipi_dual_channel_en; struct vop_reg rgb_en; }; @@ -213,6 +214,9 @@ struct vop_data { /* for use special outface */ #define ROCKCHIP_OUT_MODE_AAAA 15 +/* output flags */ +#define ROCKCHIP_OUTPUT_DSI_DUAL BIT(0) + enum alpha_mode { ALPHA_STRAIGHT, ALPHA_INVERSE, diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c index d824ca60c1a4..44d73c7beb5c 100644 --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c @@ -543,6 +543,7 @@ static const struct vop_output rk3399_output = { .hdmi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 13), .edp_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 14), .mipi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 15), + .mipi_dual_channel_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 3), }; static const struct vop_data rk3399_vop_big = {
Add the Rockchip-sepcific dual-dsi setup and hook it into the VOP as well. As described in the general dual-dsi devicetree binding, the panel should define two input ports and point each of them to one of the used dsi- controllers, as well as declare one of them as clock-master. This is used to determine the dual-dsi state and get access to both controller instances. Signed-off-by: Heiko Stuebner <heiko@sntech.de> v4: add component directly in probe when adding empty dsi slave controller v5: use driver-internal mechanism to find dual dsi slave --- .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 105 ++++++++++++++++++ drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 + drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 4 + drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + 5 files changed, 114 insertions(+)