| Message ID | 20191213181051.25983-10-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add PX30 LVDS support | expand |
Hi, On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote: > +static int px30_lvds_grf_config(struct drm_encoder *encoder, > + struct drm_display_mode *mode) > +{ > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > + u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC); > + u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC); > + u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC); > + int ret; > + > + if (lvds->output != DISPLAY_OUTPUT_LVDS) { > + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n", > + lvds->output); > + return -EINVAL; > + } > + > + if (nhsync ^ nvsync) { > + DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n"); > + return -EINVAL; > + } > + > + /* Set format */ > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > + PX30_LVDS_FORMAT(lvds->format), > + PX30_LVDS_FORMAT(lvds->format)); > + if (ret) > + return ret; > + > + /* Control Hsync/Vsync polarity */ > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > + PX30_LVDS_TIE_CLKS(1), > + PX30_LVDS_TIE_CLKS(1)); > + if (ret) > + return ret; > + > + /* Set Hsync/Vsync polarity */ > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > + PX30_LVDS_INVERT_CLKS(1), > + PX30_LVDS_INVERT_CLKS(nhsync)); > + if (ret) > + return ret; I don't know the hardware but it seems pretty weird to me. hsync and vsync in LVDS are not clocks (or even signals), they're a bit in the payload. Is there any explanation in the datasheet (or even a datasheet in the first place)? Maxime
Hi Maxime, Maxime Ripard <maxime@cerno.tech> wrote on Mon, 16 Dec 2019 11:58:27 +0100: > Hi, > > On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote: > > +static int px30_lvds_grf_config(struct drm_encoder *encoder, > > + struct drm_display_mode *mode) > > +{ > > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > > + u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC); > > + u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC); > > + u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC); > > + int ret; > > + > > + if (lvds->output != DISPLAY_OUTPUT_LVDS) { > > + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n", > > + lvds->output); > > + return -EINVAL; > > + } > > + > > + if (nhsync ^ nvsync) { > > + DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n"); > > + return -EINVAL; > > + } > > + > > + /* Set format */ > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > > + PX30_LVDS_FORMAT(lvds->format), > > + PX30_LVDS_FORMAT(lvds->format)); > > + if (ret) > > + return ret; > > + > > + /* Control Hsync/Vsync polarity */ > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > > + PX30_LVDS_TIE_CLKS(1), > > + PX30_LVDS_TIE_CLKS(1)); > > + if (ret) > > + return ret; > > + > > + /* Set Hsync/Vsync polarity */ > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > > + PX30_LVDS_INVERT_CLKS(1), > > + PX30_LVDS_INVERT_CLKS(nhsync)); > > + if (ret) > > + return ret; > > I don't know the hardware but it seems pretty weird to me. hsync and > vsync in LVDS are not clocks (or even signals), they're a bit in the > payload. Is there any explanation in the datasheet (or even a > datasheet in the first place)? There is no explanation about this in the PX30 TRM part 1 (public). But you are right the naming is weird. Could the "tie clocks" thing above mean something to you/people knowing the LVDS world? Cheers, Miquèl
On Mon, Dec 16, 2019 at 12:03:12PM +0100, Miquel Raynal wrote: > Maxime Ripard <maxime@cerno.tech> wrote on Mon, 16 Dec 2019 11:58:27 +0100: > > Hi, > > > > On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote: > > > +static int px30_lvds_grf_config(struct drm_encoder *encoder, > > > + struct drm_display_mode *mode) > > > +{ > > > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > > > + u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC); > > > + u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC); > > > + u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC); > > > + int ret; > > > + > > > + if (lvds->output != DISPLAY_OUTPUT_LVDS) { > > > + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n", > > > + lvds->output); > > > + return -EINVAL; > > > + } > > > + > > > + if (nhsync ^ nvsync) { > > > + DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n"); > > > + return -EINVAL; > > > + } > > > + > > > + /* Set format */ > > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > > > + PX30_LVDS_FORMAT(lvds->format), > > > + PX30_LVDS_FORMAT(lvds->format)); > > > + if (ret) > > > + return ret; > > > + > > > + /* Control Hsync/Vsync polarity */ > > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > > > + PX30_LVDS_TIE_CLKS(1), > > > + PX30_LVDS_TIE_CLKS(1)); > > > + if (ret) > > > + return ret; > > > + > > > + /* Set Hsync/Vsync polarity */ > > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > > > + PX30_LVDS_INVERT_CLKS(1), > > > + PX30_LVDS_INVERT_CLKS(nhsync)); > > > + if (ret) > > > + return ret; > > > > I don't know the hardware but it seems pretty weird to me. hsync and > > vsync in LVDS are not clocks (or even signals), they're a bit in the > > payload. Is there any explanation in the datasheet (or even a > > datasheet in the first place)? > > There is no explanation about this in the PX30 TRM part 1 (public). But > you are right the naming is weird. Could the "tie clocks" thing above > mean something to you/people knowing the LVDS world? I have no idea what that could mean :) Maxime
Hi Miquel: Thanks for your work here. A discussion about the grf write macro bellow. On 12/14/19 2:10 AM, Miquel Raynal wrote: > Introduce PX30 LVDS support. This means adding the relevant helper > functions, a specific probe and also the initialization of a specific > PHY. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/gpu/drm/rockchip/rockchip_lvds.c | 173 +++++++++++++++++++++++ > drivers/gpu/drm/rockchip/rockchip_lvds.h | 14 ++ > 2 files changed, 187 insertions(+) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c > index a0c203dcd66f..e550c2f102e0 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c > +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c > @@ -10,6 +10,7 @@ > #include <linux/component.h> > #include <linux/mfd/syscon.h> > #include <linux/of_graph.h> > +#include <linux/phy/phy.h> > #include <linux/pinctrl/devinfo.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > @@ -54,6 +55,7 @@ struct rockchip_lvds { > void __iomem *regs; > struct regmap *grf; > struct clk *pclk; > + struct phy *dphy; > const struct rockchip_lvds_soc_data *soc_data; > int output; /* rgb lvds or dual lvds output */ > int format; /* vesa or jeida format */ > @@ -322,6 +324,133 @@ static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder) > drm_panel_unprepare(lvds->panel); > } > > +static int px30_lvds_poweron(struct rockchip_lvds *lvds) > +{ > + int ret; > + > + ret = pm_runtime_get_sync(lvds->dev); > + if (ret < 0) { > + DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret); > + return ret; > + } > + > + /* Enable LVDS mode */ > + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1), > + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1)); > +} > + > +static void px30_lvds_poweroff(struct rockchip_lvds *lvds) > +{ > + regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1), > + PX30_LVDS_MODE_EN(0) | PX30_LVDS_P2S_EN(0)); > + > + pm_runtime_put(lvds->dev); > +} > + > +static int px30_lvds_grf_config(struct drm_encoder *encoder, > + struct drm_display_mode *mode) > +{ > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > + u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC); > + u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC); > + u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC); > + int ret; > + > + if (lvds->output != DISPLAY_OUTPUT_LVDS) { > + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n", > + lvds->output); > + return -EINVAL; > + } > + > + if (nhsync ^ nvsync) { > + DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n"); > + return -EINVAL; > + } > + > + /* Set format */ > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > + PX30_LVDS_FORMAT(lvds->format), > + PX30_LVDS_FORMAT(lvds->format)); > + if (ret) > + return ret; > + > + /* Control Hsync/Vsync polarity */ > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > + PX30_LVDS_TIE_CLKS(1), > + PX30_LVDS_TIE_CLKS(1)); > + if (ret) > + return ret; > + > + /* Set Hsync/Vsync polarity */ > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > + PX30_LVDS_INVERT_CLKS(1), > + PX30_LVDS_INVERT_CLKS(nhsync)); > + if (ret) > + return ret; > + > + /* Set dclk polarity */ > + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > + PX30_LVDS_INVERT_DCLK(1), > + PX30_LVDS_INVERT_DCLK(ndclk)); > +} > + > +static int px30_lvds_set_vop_source(struct rockchip_lvds *lvds, > + struct drm_encoder *encoder) > +{ > + int vop; > + > + vop = drm_of_encoder_active_endpoint_id(lvds->dev->of_node, encoder); > + if (vop < 0) > + return vop; > + > + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > + PX30_LVDS_VOP_SEL(1), > + PX30_LVDS_VOP_SEL(vop)); > +} > + > +static void px30_lvds_encoder_enable(struct drm_encoder *encoder) > +{ > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; > + int ret; > + > + drm_panel_prepare(lvds->panel); > + > + ret = px30_lvds_poweron(lvds); > + if (ret) { > + DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret); > + drm_panel_unprepare(lvds->panel); > + return; > + } > + > + ret = px30_lvds_grf_config(encoder, mode); > + if (ret) { > + DRM_DEV_ERROR(lvds->dev, "failed to configure LVDS: %d\n", ret); > + drm_panel_unprepare(lvds->panel); > + return; > + } > + > + ret = px30_lvds_set_vop_source(lvds, encoder); > + if (ret) { > + DRM_DEV_ERROR(lvds->dev, "failed to set VOP source: %d\n", ret); > + drm_panel_unprepare(lvds->panel); > + return; > + } > + > + drm_panel_enable(lvds->panel); > +} > + > +static void px30_lvds_encoder_disable(struct drm_encoder *encoder) > +{ > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > + > + drm_panel_disable(lvds->panel); > + px30_lvds_poweroff(lvds); > + drm_panel_unprepare(lvds->panel); > +} > + > static const > struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = { > .enable = rk3288_lvds_encoder_enable, > @@ -329,6 +458,13 @@ struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = { > .atomic_check = rockchip_lvds_encoder_atomic_check, > }; > > +static const > +struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = { > + .enable = px30_lvds_encoder_enable, > + .disable = px30_lvds_encoder_disable, > + .atomic_check = rockchip_lvds_encoder_atomic_check, > +}; > + > static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = { > .destroy = drm_encoder_cleanup, > }; > @@ -379,16 +515,53 @@ static int rk3288_lvds_probe(struct platform_device *pdev, > return 0; > } > > +static int px30_lvds_probe(struct platform_device *pdev, > + struct rockchip_lvds *lvds) > +{ > + int ret; > + > + /* MSB */ > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > + PX30_LVDS_MSBSEL(1), > + PX30_LVDS_MSBSEL(1)); > + if (ret) > + return ret; > + > + /* PHY */ > + lvds->dphy = devm_phy_get(&pdev->dev, "dphy"); > + if (IS_ERR(lvds->dphy)) > + return PTR_ERR(lvds->dphy); > + > + phy_init(lvds->dphy); > + if (ret) > + return ret; > + > + phy_set_mode(lvds->dphy, PHY_MODE_LVDS); > + if (ret) > + return ret; > + > + return phy_power_on(lvds->dphy); > +} > + > static const struct rockchip_lvds_soc_data rk3288_lvds_data = { > .probe = rk3288_lvds_probe, > .helper_funcs = &rk3288_lvds_encoder_helper_funcs, > }; > > +static const struct rockchip_lvds_soc_data px30_lvds_data = { > + .probe = px30_lvds_probe, > + .helper_funcs = &px30_lvds_encoder_helper_funcs, > +}; > + > static const struct of_device_id rockchip_lvds_dt_ids[] = { > { > .compatible = "rockchip,rk3288-lvds", > .data = &rk3288_lvds_data > }, > + { > + .compatible = "rockchip,px30-lvds", > + .data = &px30_lvds_data > + }, > {} > }; > MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids); > diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h > index e41e9ab3c306..7cfb102b4854 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_lvds.h > +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h > @@ -106,4 +106,18 @@ > #define LVDS_VESA_18 2 > #define LVDS_JEIDA_18 3 > > +#define WRITE_EN(v, h, l) ((GENMASK(h, l) << 16) | (v << l)) How about rename WRITE_EN to HIWORD_UPDATE to keep align with other modules that write grf: such as dwmac-rk.c/dw-mipi-dsi-rockhip.c/dw-hdmi-rockchip.c > + > +#define PX30_LVDS_GRF_PD_VO_CON0 0x434 > +#define PX30_LVDS_TIE_CLKS(val) WRITE_EN(val, 8, 8) > +#define PX30_LVDS_INVERT_CLKS(val) WRITE_EN(val, 9, 9) > +#define PX30_LVDS_INVERT_DCLK(val) WRITE_EN(val, 5, 5) > + > +#define PX30_LVDS_GRF_PD_VO_CON1 0x438 > +#define PX30_LVDS_FORMAT(val) WRITE_EN(val, 14, 13) > +#define PX30_LVDS_MODE_EN(val) WRITE_EN(val, 12, 12) > +#define PX30_LVDS_MSBSEL(val) WRITE_EN(val, 11, 11) > +#define PX30_LVDS_P2S_EN(val) WRITE_EN(val, 6, 6) > +#define PX30_LVDS_VOP_SEL(val) WRITE_EN(val, 1, 1) > + > #endif /* _ROCKCHIP_LVDS_ */
Hi Andy, Andy Yan <andy.yan@rock-chips.com> wrote on Mon, 16 Dec 2019 20:00:31 +0800: > Hi Miquel: > > Thanks for your work here. > > A discussion about the grf write macro bellow. > > On 12/14/19 2:10 AM, Miquel Raynal wrote: > > Introduce PX30 LVDS support. This means adding the relevant helper > > functions, a specific probe and also the initialization of a specific > > PHY. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > drivers/gpu/drm/rockchip/rockchip_lvds.c | 173 +++++++++++++++++++++++ > > drivers/gpu/drm/rockchip/rockchip_lvds.h | 14 ++ > > 2 files changed, 187 insertions(+) > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c > > index a0c203dcd66f..e550c2f102e0 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c > > +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c > > @@ -10,6 +10,7 @@ > > #include <linux/component.h> > > #include <linux/mfd/syscon.h> > > #include <linux/of_graph.h> > > +#include <linux/phy/phy.h> > > #include <linux/pinctrl/devinfo.h> > > #include <linux/platform_device.h> > > #include <linux/pm_runtime.h> > > @@ -54,6 +55,7 @@ struct rockchip_lvds { > > void __iomem *regs; > > struct regmap *grf; > > struct clk *pclk; > > + struct phy *dphy; > > const struct rockchip_lvds_soc_data *soc_data; > > int output; /* rgb lvds or dual lvds output */ > > int format; /* vesa or jeida format */ > > @@ -322,6 +324,133 @@ static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder) > > drm_panel_unprepare(lvds->panel); > > } > > > +static int px30_lvds_poweron(struct rockchip_lvds *lvds) > > +{ > > + int ret; > > + > > + ret = pm_runtime_get_sync(lvds->dev); > > + if (ret < 0) { > > + DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret); > > + return ret; > > + } > > + > > + /* Enable LVDS mode */ > > + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > > + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1), > > + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1)); > > +} > > + > > +static void px30_lvds_poweroff(struct rockchip_lvds *lvds) > > +{ > > + regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > > + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1), > > + PX30_LVDS_MODE_EN(0) | PX30_LVDS_P2S_EN(0)); > > + > > + pm_runtime_put(lvds->dev); > > +} > > + > > +static int px30_lvds_grf_config(struct drm_encoder *encoder, > > + struct drm_display_mode *mode) > > +{ > > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > > + u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC); > > + u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC); > > + u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC); > > + int ret; > > + > > + if (lvds->output != DISPLAY_OUTPUT_LVDS) { > > + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n", > > + lvds->output); > > + return -EINVAL; > > + } > > + > > + if (nhsync ^ nvsync) { > > + DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n"); > > + return -EINVAL; > > + } > > + > > + /* Set format */ > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > > + PX30_LVDS_FORMAT(lvds->format), > > + PX30_LVDS_FORMAT(lvds->format)); > > + if (ret) > > + return ret; > > + > > + /* Control Hsync/Vsync polarity */ > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > > + PX30_LVDS_TIE_CLKS(1), > > + PX30_LVDS_TIE_CLKS(1)); > > + if (ret) > > + return ret; > > + > > + /* Set Hsync/Vsync polarity */ > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > > + PX30_LVDS_INVERT_CLKS(1), > > + PX30_LVDS_INVERT_CLKS(nhsync)); > > + if (ret) > > + return ret; > > + > > + /* Set dclk polarity */ > > + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, > > + PX30_LVDS_INVERT_DCLK(1), > > + PX30_LVDS_INVERT_DCLK(ndclk)); > > +} > > + > > +static int px30_lvds_set_vop_source(struct rockchip_lvds *lvds, > > + struct drm_encoder *encoder) > > +{ > > + int vop; > > + > > + vop = drm_of_encoder_active_endpoint_id(lvds->dev->of_node, encoder); > > + if (vop < 0) > > + return vop; > > + > > + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > > + PX30_LVDS_VOP_SEL(1), > > + PX30_LVDS_VOP_SEL(vop)); > > +} > > + > > +static void px30_lvds_encoder_enable(struct drm_encoder *encoder) > > +{ > > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > > + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; > > + int ret; > > + > > + drm_panel_prepare(lvds->panel); > > + > > + ret = px30_lvds_poweron(lvds); > > + if (ret) { > > + DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret); > > + drm_panel_unprepare(lvds->panel); > > + return; > > + } > > + > > + ret = px30_lvds_grf_config(encoder, mode); > > + if (ret) { > > + DRM_DEV_ERROR(lvds->dev, "failed to configure LVDS: %d\n", ret); > > + drm_panel_unprepare(lvds->panel); > > + return; > > + } > > + > > + ret = px30_lvds_set_vop_source(lvds, encoder); > > + if (ret) { > > + DRM_DEV_ERROR(lvds->dev, "failed to set VOP source: %d\n", ret); > > + drm_panel_unprepare(lvds->panel); > > + return; > > + } > > + > > + drm_panel_enable(lvds->panel); > > +} > > + > > +static void px30_lvds_encoder_disable(struct drm_encoder *encoder) > > +{ > > + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); > > + > > + drm_panel_disable(lvds->panel); > > + px30_lvds_poweroff(lvds); > > + drm_panel_unprepare(lvds->panel); > > +} > > + > > static const > > struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = { > > .enable = rk3288_lvds_encoder_enable, > > @@ -329,6 +458,13 @@ struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = { > > .atomic_check = rockchip_lvds_encoder_atomic_check, > > }; > > > +static const > > +struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = { > > + .enable = px30_lvds_encoder_enable, > > + .disable = px30_lvds_encoder_disable, > > + .atomic_check = rockchip_lvds_encoder_atomic_check, > > +}; > > + > > static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = { > > .destroy = drm_encoder_cleanup, > > }; > > @@ -379,16 +515,53 @@ static int rk3288_lvds_probe(struct platform_device *pdev, > > return 0; > > } > > > +static int px30_lvds_probe(struct platform_device *pdev, > > + struct rockchip_lvds *lvds) > > +{ > > + int ret; > > + > > + /* MSB */ > > + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, > > + PX30_LVDS_MSBSEL(1), > > + PX30_LVDS_MSBSEL(1)); > > + if (ret) > > + return ret; > > + > > + /* PHY */ > > + lvds->dphy = devm_phy_get(&pdev->dev, "dphy"); > > + if (IS_ERR(lvds->dphy)) > > + return PTR_ERR(lvds->dphy); > > + > > + phy_init(lvds->dphy); > > + if (ret) > > + return ret; > > + > > + phy_set_mode(lvds->dphy, PHY_MODE_LVDS); > > + if (ret) > > + return ret; > > + > > + return phy_power_on(lvds->dphy); > > +} > > + > > static const struct rockchip_lvds_soc_data rk3288_lvds_data = { > > .probe = rk3288_lvds_probe, > > .helper_funcs = &rk3288_lvds_encoder_helper_funcs, > > }; > > > +static const struct rockchip_lvds_soc_data px30_lvds_data = { > > + .probe = px30_lvds_probe, > > + .helper_funcs = &px30_lvds_encoder_helper_funcs, > > +}; > > + > > static const struct of_device_id rockchip_lvds_dt_ids[] = { > > { > > .compatible = "rockchip,rk3288-lvds", > > .data = &rk3288_lvds_data > > }, > > + { > > + .compatible = "rockchip,px30-lvds", > > + .data = &px30_lvds_data > > + }, > > {} > > }; > > MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids); > > diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h > > index e41e9ab3c306..7cfb102b4854 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_lvds.h > > +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h > > @@ -106,4 +106,18 @@ > > #define LVDS_VESA_18 2 > > #define LVDS_JEIDA_18 3 > > > +#define WRITE_EN(v, h, l) ((GENMASK(h, l) << 16) | (v << l)) > > > How about rename WRITE_EN to HIWORD_UPDATE to keep align with other modules that write grf: such as dwmac-rk.c/dw-mipi-dsi-rockhip.c/dw-hdmi-rockchip.c Sure. It is also the name of this macro in the BSP but I found it so undescriptive that I changed it. I don't like very much its new name neither so I'll go back to the original one. Thanks, Miquèl
Hi Maxime & Miquel, 在 2019/12/16 下午7:14, Maxime Ripard 写道: > On Mon, Dec 16, 2019 at 12:03:12PM +0100, Miquel Raynal wrote: >> Maxime Ripard <maxime@cerno.tech> wrote on Mon, 16 Dec 2019 11:58:27 +0100: >>> Hi, >>> >>> On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote: >>>> +static int px30_lvds_grf_config(struct drm_encoder *encoder, >>>> + struct drm_display_mode *mode) >>>> +{ >>>> + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); >>>> + u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC); >>>> + u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC); >>>> + u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC); >>>> + int ret; >>>> + >>>> + if (lvds->output != DISPLAY_OUTPUT_LVDS) { >>>> + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n", >>>> + lvds->output); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + if (nhsync ^ nvsync) { >>>> + DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n"); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + /* Set format */ >>>> + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, >>>> + PX30_LVDS_FORMAT(lvds->format), >>>> + PX30_LVDS_FORMAT(lvds->format)); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* Control Hsync/Vsync polarity */ >>>> + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, >>>> + PX30_LVDS_TIE_CLKS(1), >>>> + PX30_LVDS_TIE_CLKS(1)); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* Set Hsync/Vsync polarity */ >>>> + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, >>>> + PX30_LVDS_INVERT_CLKS(1), >>>> + PX30_LVDS_INVERT_CLKS(nhsync)); >>>> + if (ret) >>>> + return ret; >>> I don't know the hardware but it seems pretty weird to me. hsync and >>> vsync in LVDS are not clocks (or even signals), they're a bit in the >>> payload. Is there any explanation in the datasheet (or even a >>> datasheet in the first place)? >> There is no explanation about this in the PX30 TRM part 1 (public). But >> you are right the naming is weird. Could the "tie clocks" thing above >> mean something to you/people knowing the LVDS world? > I have no idea what that could mean :) This two bit(GRF_CON0[9,8]) is not for hsync/vsync polarity config, the polarity is done at vop_crtc_atomic_enable@rockchip_drm_vop.c. Before px30, all rockchip platform lvds output payload is include hsync,vsync and den clock signal. About years ago, we meet a lvds panel can't work normally at rk3288, from the panel spec we know the panel need lvds work den mode only, the hsync/vsync need to set 0 at blank time, so we add this two bit(GRF_CON0[9,8]) for this mode. but now we can't get in touch with the customer and get the panel to test this function, so we can't verify the panel work unnormally is because of the den mode only and the hsync vsync signal level at blank time. I recommend not including this part of configuration before we test this funcion at den mode only lvds panel,thanks. > > Maxime > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi some mistakes with last mail, so resend this mail. 在 2019/12/18 上午11:17, sandy.huang 写道: > Hi Maxime & Miquel, > > 在 2019/12/16 下午7:14, Maxime Ripard 写道: >> On Mon, Dec 16, 2019 at 12:03:12PM +0100, Miquel Raynal wrote: >>> Maxime Ripard <maxime@cerno.tech> wrote on Mon, 16 Dec 2019 11:58:27 >>> +0100: >>>> Hi, >>>> >>>> On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote: >>>>> +static int px30_lvds_grf_config(struct drm_encoder *encoder, >>>>> + struct drm_display_mode *mode) >>>>> +{ >>>>> + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); >>>>> + u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC); >>>>> + u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC); >>>>> + u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC); >>>>> + int ret; >>>>> + >>>>> + if (lvds->output != DISPLAY_OUTPUT_LVDS) { >>>>> + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n", >>>>> + lvds->output); >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + if (nhsync ^ nvsync) { >>>>> + DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync >>>>> polarity\n"); >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + /* Set format */ >>>>> + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, >>>>> + PX30_LVDS_FORMAT(lvds->format), >>>>> + PX30_LVDS_FORMAT(lvds->format)); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + /* Control Hsync/Vsync polarity */ >>>>> + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, >>>>> + PX30_LVDS_TIE_CLKS(1), >>>>> + PX30_LVDS_TIE_CLKS(1)); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + /* Set Hsync/Vsync polarity */ >>>>> + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, >>>>> + PX30_LVDS_INVERT_CLKS(1), >>>>> + PX30_LVDS_INVERT_CLKS(nhsync)); >>>>> + if (ret) >>>>> + return ret; >>>> I don't know the hardware but it seems pretty weird to me. hsync and >>>> vsync in LVDS are not clocks (or even signals), they're a bit in the >>>> payload. Is there any explanation in the datasheet (or even a >>>> datasheet in the first place)? >>> There is no explanation about this in the PX30 TRM part 1 (public). But >>> you are right the naming is weird. Could the "tie clocks" thing above >>> mean something to you/people knowing the LVDS world? >> I have no idea what that could mean :) > > This two bit(GRF_CON0[9,8]) is not for hsync/vsync polarity config, > the polarity is done at vop_crtc_atomic_enable@rockchip_drm_vop.c. > > Before px30, all rockchip platform lvds output payload is include > hsync,vsync and den clock signal. About years ago, we meet a lvds > panel can't work normally at rk3288, from the panel spec we know the > panel need lvds work den mode only, the hsync/vsync need to set 0 at > blank time, so we add this two bit(GRF_CON0[9,8]) for this mode. > > but now we can't get in touch with the customer and get the panel to > test this function, so we can't verify the panel work unnormally is > because of the den mode only and the hsync vsync signal level at > blank time. > > I recommend not including this part of configuration before we test > this funcion at den mode only lvds panel,thanks. > >> >> Maxime >> >> _______________________________________________ >> Linux-rockchip mailing list >> Linux-rockchip@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-rockchip >
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c index a0c203dcd66f..e550c2f102e0 100644 --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c @@ -10,6 +10,7 @@ #include <linux/component.h> #include <linux/mfd/syscon.h> #include <linux/of_graph.h> +#include <linux/phy/phy.h> #include <linux/pinctrl/devinfo.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> @@ -54,6 +55,7 @@ struct rockchip_lvds { void __iomem *regs; struct regmap *grf; struct clk *pclk; + struct phy *dphy; const struct rockchip_lvds_soc_data *soc_data; int output; /* rgb lvds or dual lvds output */ int format; /* vesa or jeida format */ @@ -322,6 +324,133 @@ static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder) drm_panel_unprepare(lvds->panel); } +static int px30_lvds_poweron(struct rockchip_lvds *lvds) +{ + int ret; + + ret = pm_runtime_get_sync(lvds->dev); + if (ret < 0) { + DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret); + return ret; + } + + /* Enable LVDS mode */ + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1), + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1)); +} + +static void px30_lvds_poweroff(struct rockchip_lvds *lvds) +{ + regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, + PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1), + PX30_LVDS_MODE_EN(0) | PX30_LVDS_P2S_EN(0)); + + pm_runtime_put(lvds->dev); +} + +static int px30_lvds_grf_config(struct drm_encoder *encoder, + struct drm_display_mode *mode) +{ + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); + u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC); + u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC); + u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC); + int ret; + + if (lvds->output != DISPLAY_OUTPUT_LVDS) { + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n", + lvds->output); + return -EINVAL; + } + + if (nhsync ^ nvsync) { + DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n"); + return -EINVAL; + } + + /* Set format */ + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, + PX30_LVDS_FORMAT(lvds->format), + PX30_LVDS_FORMAT(lvds->format)); + if (ret) + return ret; + + /* Control Hsync/Vsync polarity */ + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, + PX30_LVDS_TIE_CLKS(1), + PX30_LVDS_TIE_CLKS(1)); + if (ret) + return ret; + + /* Set Hsync/Vsync polarity */ + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, + PX30_LVDS_INVERT_CLKS(1), + PX30_LVDS_INVERT_CLKS(nhsync)); + if (ret) + return ret; + + /* Set dclk polarity */ + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0, + PX30_LVDS_INVERT_DCLK(1), + PX30_LVDS_INVERT_DCLK(ndclk)); +} + +static int px30_lvds_set_vop_source(struct rockchip_lvds *lvds, + struct drm_encoder *encoder) +{ + int vop; + + vop = drm_of_encoder_active_endpoint_id(lvds->dev->of_node, encoder); + if (vop < 0) + return vop; + + return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, + PX30_LVDS_VOP_SEL(1), + PX30_LVDS_VOP_SEL(vop)); +} + +static void px30_lvds_encoder_enable(struct drm_encoder *encoder) +{ + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; + int ret; + + drm_panel_prepare(lvds->panel); + + ret = px30_lvds_poweron(lvds); + if (ret) { + DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret); + drm_panel_unprepare(lvds->panel); + return; + } + + ret = px30_lvds_grf_config(encoder, mode); + if (ret) { + DRM_DEV_ERROR(lvds->dev, "failed to configure LVDS: %d\n", ret); + drm_panel_unprepare(lvds->panel); + return; + } + + ret = px30_lvds_set_vop_source(lvds, encoder); + if (ret) { + DRM_DEV_ERROR(lvds->dev, "failed to set VOP source: %d\n", ret); + drm_panel_unprepare(lvds->panel); + return; + } + + drm_panel_enable(lvds->panel); +} + +static void px30_lvds_encoder_disable(struct drm_encoder *encoder) +{ + struct rockchip_lvds *lvds = encoder_to_lvds(encoder); + + drm_panel_disable(lvds->panel); + px30_lvds_poweroff(lvds); + drm_panel_unprepare(lvds->panel); +} + static const struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = { .enable = rk3288_lvds_encoder_enable, @@ -329,6 +458,13 @@ struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = { .atomic_check = rockchip_lvds_encoder_atomic_check, }; +static const +struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = { + .enable = px30_lvds_encoder_enable, + .disable = px30_lvds_encoder_disable, + .atomic_check = rockchip_lvds_encoder_atomic_check, +}; + static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = { .destroy = drm_encoder_cleanup, }; @@ -379,16 +515,53 @@ static int rk3288_lvds_probe(struct platform_device *pdev, return 0; } +static int px30_lvds_probe(struct platform_device *pdev, + struct rockchip_lvds *lvds) +{ + int ret; + + /* MSB */ + ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1, + PX30_LVDS_MSBSEL(1), + PX30_LVDS_MSBSEL(1)); + if (ret) + return ret; + + /* PHY */ + lvds->dphy = devm_phy_get(&pdev->dev, "dphy"); + if (IS_ERR(lvds->dphy)) + return PTR_ERR(lvds->dphy); + + phy_init(lvds->dphy); + if (ret) + return ret; + + phy_set_mode(lvds->dphy, PHY_MODE_LVDS); + if (ret) + return ret; + + return phy_power_on(lvds->dphy); +} + static const struct rockchip_lvds_soc_data rk3288_lvds_data = { .probe = rk3288_lvds_probe, .helper_funcs = &rk3288_lvds_encoder_helper_funcs, }; +static const struct rockchip_lvds_soc_data px30_lvds_data = { + .probe = px30_lvds_probe, + .helper_funcs = &px30_lvds_encoder_helper_funcs, +}; + static const struct of_device_id rockchip_lvds_dt_ids[] = { { .compatible = "rockchip,rk3288-lvds", .data = &rk3288_lvds_data }, + { + .compatible = "rockchip,px30-lvds", + .data = &px30_lvds_data + }, {} }; MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids); diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h index e41e9ab3c306..7cfb102b4854 100644 --- a/drivers/gpu/drm/rockchip/rockchip_lvds.h +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h @@ -106,4 +106,18 @@ #define LVDS_VESA_18 2 #define LVDS_JEIDA_18 3 +#define WRITE_EN(v, h, l) ((GENMASK(h, l) << 16) | (v << l)) + +#define PX30_LVDS_GRF_PD_VO_CON0 0x434 +#define PX30_LVDS_TIE_CLKS(val) WRITE_EN(val, 8, 8) +#define PX30_LVDS_INVERT_CLKS(val) WRITE_EN(val, 9, 9) +#define PX30_LVDS_INVERT_DCLK(val) WRITE_EN(val, 5, 5) + +#define PX30_LVDS_GRF_PD_VO_CON1 0x438 +#define PX30_LVDS_FORMAT(val) WRITE_EN(val, 14, 13) +#define PX30_LVDS_MODE_EN(val) WRITE_EN(val, 12, 12) +#define PX30_LVDS_MSBSEL(val) WRITE_EN(val, 11, 11) +#define PX30_LVDS_P2S_EN(val) WRITE_EN(val, 6, 6) +#define PX30_LVDS_VOP_SEL(val) WRITE_EN(val, 1, 1) + #endif /* _ROCKCHIP_LVDS_ */
Introduce PX30 LVDS support. This means adding the relevant helper functions, a specific probe and also the initialization of a specific PHY. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/gpu/drm/rockchip/rockchip_lvds.c | 173 +++++++++++++++++++++++ drivers/gpu/drm/rockchip/rockchip_lvds.h | 14 ++ 2 files changed, 187 insertions(+)