When we first enable the DSI encoder, we currently program some per-chip
configuration that we look up in rk3399_chip_data based on the device
tree compatible we match. This data configures various parameters of the
MIPI lanes, including on RK3399 whether DSI1 is slaved to DSI0 in a
dual-mode configuration. It also selects which LCDC (i.e. VOP) to scan
out from.
This causes a problem in RK3399 dual-mode configurations, though: panel
prepare() callbacks run before the encoder gets enabled and expect to be
able to write commands to the DSI bus, but the bus isn't fully
functional until the lane and master/slave configuration have been
programmed. As a result, dual-mode panels (and possibly others too) fail
to turn on when the rockchipdrm driver is initially loaded.
Because the LCDC mux is the only thing we don't know until enable time
(and is the only thing that can ever change), we can actually move most
of the initialization to bind() and get it out of the way early. That's
what this change does. (Rockchip's 4.4 BSP kernel does it in mode_set(),
which also avoids the issue, but bind() seems like the more correct
place to me.)
Tested on a Google Scarlet board (Acer Chromebook Tab 10), which has a
Kingdisplay KD097D04 dual-mode panel. Prior to this change, the panel's
backlight would turn on but no image would appear when initially loading
rockchipdrm. If I kept rockchipdrm loaded and reloaded the panel driver,
it would come on. With this change, the panel successfully turns on
during initial rockchipdrm load as expected.
Fixes: 2d4f7bdafd70 ("drm/rockchip: dsi: migrate to use dw-mipi-dsi bridge driver")
Signed-off-by: Thomas Hebb <[email protected]>
---
Resending since I wasn't subscribed to dri-devel
.../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 36 ++++++++++++++-----
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index ce044db8c97e..d0c9610ad220 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -691,13 +691,8 @@ static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_rockchip_phy_ops = {
.get_timing = dw_mipi_dsi_phy_get_timing,
};
-static void dw_mipi_dsi_rockchip_config(struct dw_mipi_dsi_rockchip *dsi,
- int mux)
+static void dw_mipi_dsi_rockchip_config(struct dw_mipi_dsi_rockchip *dsi)
{
- if (dsi->cdata->lcdsel_grf_reg)
- regmap_write(dsi->grf_regmap, dsi->cdata->lcdsel_grf_reg,
- mux ? dsi->cdata->lcdsel_lit : dsi->cdata->lcdsel_big);
-
if (dsi->cdata->lanecfg1_grf_reg)
regmap_write(dsi->grf_regmap, dsi->cdata->lanecfg1_grf_reg,
dsi->cdata->lanecfg1);
@@ -711,6 +706,13 @@ static void dw_mipi_dsi_rockchip_config(struct dw_mipi_dsi_rockchip *dsi,
dsi->cdata->enable);
}
+static void dw_mipi_dsi_rockchip_set_lcdsel(struct dw_mipi_dsi_rockchip *dsi,
+ int mux)
+{
+ regmap_write(dsi->grf_regmap, dsi->cdata->lcdsel_grf_reg,
+ mux ? dsi->cdata->lcdsel_lit : dsi->cdata->lcdsel_big);
+}
+
static int
dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
@@ -766,9 +768,9 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
return;
}
- dw_mipi_dsi_rockchip_config(dsi, mux);
+ dw_mipi_dsi_rockchip_set_lcdsel(dsi, mux);
if (dsi->slave)
- dw_mipi_dsi_rockchip_config(dsi->slave, mux);
+ dw_mipi_dsi_rockchip_set_lcdsel(dsi->slave, mux);
clk_disable_unprepare(dsi->grf_clk);
}
@@ -922,6 +924,24 @@ static int dw_mipi_dsi_rockchip_bind(struct device *dev,
return ret;
}
+ /*
+ * With the GRF clock running, write lane and dual-mode configurations
+ * that won't change immediately. If we waited until enable() to do
+ * this, things like panel preparation would not be able to send
+ * commands over DSI.
+ */
+ ret = clk_prepare_enable(dsi->grf_clk);
+ if (ret) {
+ DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
+ return ret;
+ }
+
+ dw_mipi_dsi_rockchip_config(dsi);
+ if (dsi->slave)
+ dw_mipi_dsi_rockchip_config(dsi->slave);
+
+ clk_disable_unprepare(dsi->grf_clk);
+
ret = rockchip_dsi_drm_create_encoder(dsi, drm_dev);
if (ret) {
DRM_DEV_ERROR(dev, "Failed to create drm encoder\n");
--
2.29.2
В Вс, 13/12/2020 в 12:58 -0800, Thomas Hebb пишет:
> When we first enable the DSI encoder, we currently program some per-chip
> configuration that we look up in rk3399_chip_data based on the device
> tree compatible we match. This data configures various parameters of the
> MIPI lanes, including on RK3399 whether DSI1 is slaved to DSI0 in a
> dual-mode configuration. It also selects which LCDC (i.e. VOP) to scan
> out from.
>
> This causes a problem in RK3399 dual-mode configurations, though: panel
> prepare() callbacks run before the encoder gets enabled and expect to be
> able to write commands to the DSI bus, but the bus isn't fully
> functional until the lane and master/slave configuration have been
> programmed. As a result, dual-mode panels (and possibly others too) fail
> to turn on when the rockchipdrm driver is initially loaded.
>
> Because the LCDC mux is the only thing we don't know until enable time
> (and is the only thing that can ever change), we can actually move most
> of the initialization to bind() and get it out of the way early. That's
> what this change does. (Rockchip's 4.4 BSP kernel does it in mode_set(),
> which also avoids the issue, but bind() seems like the more correct
> place to me.)
>
> Tested on a Google Scarlet board (Acer Chromebook Tab 10), which has a
> Kingdisplay KD097D04 dual-mode panel. Prior to this change, the panel's
> backlight would turn on but no image would appear when initially loading
> rockchipdrm. If I kept rockchipdrm loaded and reloaded the panel driver,
> it would come on. With this change, the panel successfully turns on
> during initial rockchipdrm load as expected.
>
> Fixes: 2d4f7bdafd70 ("drm/rockchip: dsi: migrate to use dw-mipi-dsi bridge driver")
> Signed-off-by: Thomas Hebb <[email protected]>
> ---
> Resending since I wasn't subscribed to dri-devel
>
> .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 36 ++++++++++++++-----
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index ce044db8c97e..d0c9610ad220 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -691,13 +691,8 @@ static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_rockchip_phy_ops = {
> .get_timing = dw_mipi_dsi_phy_get_timing,
> };
>
>
> -static void dw_mipi_dsi_rockchip_config(struct dw_mipi_dsi_rockchip *dsi,
> - int mux)
> +static void dw_mipi_dsi_rockchip_config(struct dw_mipi_dsi_rockchip *dsi)
> {
> - if (dsi->cdata->lcdsel_grf_reg)
> - regmap_write(dsi->grf_regmap, dsi->cdata->lcdsel_grf_reg,
> - mux ? dsi->cdata->lcdsel_lit : dsi->cdata->lcdsel_big);
> -
> if (dsi->cdata->lanecfg1_grf_reg)
> regmap_write(dsi->grf_regmap, dsi->cdata->lanecfg1_grf_reg,
> dsi->cdata->lanecfg1);
> @@ -711,6 +706,13 @@ static void dw_mipi_dsi_rockchip_config(struct dw_mipi_dsi_rockchip *dsi,
> dsi->cdata->enable);
> }
>
>
> +static void dw_mipi_dsi_rockchip_set_lcdsel(struct dw_mipi_dsi_rockchip *dsi,
> + int mux)
> +{
> + regmap_write(dsi->grf_regmap, dsi->cdata->lcdsel_grf_reg,
> + mux ? dsi->cdata->lcdsel_lit : dsi->cdata->lcdsel_big);
> +}
> +
> static int
> dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder,
> struct drm_crtc_state *crtc_state,
> @@ -766,9 +768,9 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
> return;
> }
>
>
> - dw_mipi_dsi_rockchip_config(dsi, mux);
> + dw_mipi_dsi_rockchip_set_lcdsel(dsi, mux);
> if (dsi->slave)
> - dw_mipi_dsi_rockchip_config(dsi->slave, mux);
> + dw_mipi_dsi_rockchip_set_lcdsel(dsi->slave, mux);
>
>
> clk_disable_unprepare(dsi->grf_clk);
> }
> @@ -922,6 +924,24 @@ static int dw_mipi_dsi_rockchip_bind(struct device *dev,
> return ret;
> }
>
>
> + /*
> + * With the GRF clock running, write lane and dual-mode configurations
> + * that won't change immediately. If we waited until enable() to do
> + * this, things like panel preparation would not be able to send
> + * commands over DSI.
> + */
> + ret = clk_prepare_enable(dsi->grf_clk);
> + if (ret) {
> + DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> + return ret;
> + }
> +
> + dw_mipi_dsi_rockchip_config(dsi);
> + if (dsi->slave)
> + dw_mipi_dsi_rockchip_config(dsi->slave);
> +
> + clk_disable_unprepare(dsi->grf_clk);
> +
> ret = rockchip_dsi_drm_create_encoder(dsi, drm_dev);
> if (ret) {
> DRM_DEV_ERROR(dev, "Failed to create drm encoder\n");
Have tested this patch on a Pine64 RockPro64 v2.1 with Linux v5.4.44
All works good when DRM_PANEL_KINGDISPLAY_KD097D04=m
Something bad happens when DRM_PANEL_KINGDISPLAY_KD097D04=y - the panel
driver starts again to fail to write prepare() commands to DSI:
[ 28.709049] 005: dw-mipi-dsi-rockchip ff960000.mipi: failed to write
command FIFO
[ 28.709056] 005: panel-kingdisplay-kd097d04 ff960000.mipi.0:
[drm:kingdisplay_panel_prepare] *ERROR* failed write init cmds: -110