| Message ID | 20210330173348.30135-6-p.yadav@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | CSI2RX support on J721E | expand |
On Tue, 2021-03-30 at 23:03 +0530, Pratyush Yadav wrote: > Some platforms like TI's J721E can have the CSI2RX paired with an > external DPHY. Add support to enable and configure the DPHY using the > generic PHY framework. > > Get the pixel rate and bpp from the subdev and pass them on to the DPHY > along with the number of lanes. All other settings are left to their > default values. > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > --- > drivers/media/platform/cadence/cdns-csi2rx.c | 147 +++++++++++++++++-- > 1 file changed, 137 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c > index c68a3eac62cd..31bd80e3f780 100644 > --- a/drivers/media/platform/cadence/cdns-csi2rx.c > +++ b/drivers/media/platform/cadence/cdns-csi2rx.c > @@ -30,6 +30,12 @@ > #define CSI2RX_STATIC_CFG_DLANE_MAP(llane, plane) ((plane) << (16 + (llane) * 4)) > #define CSI2RX_STATIC_CFG_LANES_MASK GENMASK(11, 8) > > +#define CSI2RX_DPHY_LANE_CTRL_REG 0x40 > +#define CSI2RX_DPHY_CL_RST BIT(16) > +#define CSI2RX_DPHY_DL_RST(i) BIT((i) + 12) > +#define CSI2RX_DPHY_CL_EN BIT(4) > +#define CSI2RX_DPHY_DL_EN(i) BIT(i) > + > #define CSI2RX_STREAM_BASE(n) (((n) + 1) * 0x100) > > #define CSI2RX_STREAM_CTRL_REG(n) (CSI2RX_STREAM_BASE(n) + 0x000) > @@ -54,6 +60,11 @@ enum csi2rx_pads { > CSI2RX_PAD_MAX, > }; > > +struct csi2rx_fmt { > + u32 code; > + u8 bpp; > +}; > + > struct csi2rx_priv { > struct device *dev; > unsigned int count; > @@ -85,6 +96,52 @@ struct csi2rx_priv { > int source_pad; > }; > > +static const struct csi2rx_fmt formats[] = { > + { > + .code = MEDIA_BUS_FMT_YUYV8_2X8, > + .bpp = 16, > + }, > + { > + .code = MEDIA_BUS_FMT_UYVY8_2X8, > + .bpp = 16, > + }, > + { > + .code = MEDIA_BUS_FMT_YVYU8_2X8, > + .bpp = 16, > + }, > + { > + .code = MEDIA_BUS_FMT_VYUY8_2X8, > + .bpp = 16, > + }, > +}; > + > +static u8 csi2rx_get_bpp(u32 code) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(formats); i++) { > + if (formats[i].code == code) > + return formats[i].bpp; > + } > + > + return 0; > +} > + > +static s64 csi2rx_get_pixel_rate(struct csi2rx_priv *csi2rx) > +{ > + struct v4l2_ctrl *ctrl; > + > + ctrl = v4l2_ctrl_find(csi2rx->source_subdev->ctrl_handler, > + V4L2_CID_PIXEL_RATE); > + if (!ctrl) { > + dev_err(csi2rx->dev, "no pixel rate control in subdev: %s\n", > + csi2rx->source_subdev->name); > + return -EINVAL; > + } > + > + return v4l2_ctrl_g_ctrl_int64(ctrl); > +} > + > static inline > struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev) > { > @@ -101,6 +158,55 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx) > writel(0, csi2rx->base + CSI2RX_SOFT_RESET_REG); > } > > +static int csi2rx_configure_external_dphy(struct csi2rx_priv *csi2rx) > +{ > + union phy_configure_opts opts = { }; > + struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy; > + struct v4l2_subdev_format sd_fmt; > + s64 pixel_rate; > + int ret; > + u8 bpp; > + > + sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; > + sd_fmt.pad = 0; > + > + ret = v4l2_subdev_call(csi2rx->source_subdev, pad, get_fmt, NULL, > + &sd_fmt); > + if (ret) > + return ret; > + > + bpp = csi2rx_get_bpp(sd_fmt.format.code); > + if (!bpp) > + return -EINVAL; > + > + pixel_rate = csi2rx_get_pixel_rate(csi2rx); > + if (pixel_rate < 0) > + return pixel_rate; > + > + ret = phy_mipi_dphy_get_default_config(pixel_rate, bpp, > + csi2rx->num_lanes, cfg); > + if (ret) > + return ret; > + > + ret = phy_set_mode_ext(csi2rx->dphy, PHY_MODE_MIPI_DPHY, > + PHY_MIPI_DPHY_SUBMODE_RX); > + if (ret) > + return ret; > + > + ret = phy_power_on(csi2rx->dphy); > + if (ret) > + return ret; Seems phy_power_on, then phy_set_mode_ext? > + > + ret = phy_configure(csi2rx->dphy, &opts); > + if (ret) { > + /* Can't do anything if it fails. Ignore the return value. */ > + phy_power_off(csi2rx->dphy); > + return ret; > + } > + > + return 0; > +} > + > static int csi2rx_start(struct csi2rx_priv *csi2rx) > { > unsigned int i; > @@ -139,6 +245,17 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx) > if (ret) > goto err_disable_pclk; > > + /* Enable DPHY clk and data lanes. */ > + if (csi2rx->dphy) { > + reg = CSI2RX_DPHY_CL_EN | CSI2RX_DPHY_CL_RST; > + for (i = 0; i < csi2rx->num_lanes; i++) { > + reg |= CSI2RX_DPHY_DL_EN(csi2rx->lanes[i] - 1); > + reg |= CSI2RX_DPHY_DL_RST(csi2rx->lanes[i] - 1); > + } > + > + writel(reg, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG); > + } > + > /* > * Create a static mapping between the CSI virtual channels > * and the output stream. > @@ -169,10 +286,21 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx) > if (ret) > goto err_disable_pixclk; > > + if (csi2rx->dphy) { > + ret = csi2rx_configure_external_dphy(csi2rx); > + if (ret) { > + dev_err(csi2rx->dev, > + "Failed to configure external DPHY: %d\n", ret); > + goto err_disable_sysclk; > + } > + } > + > clk_disable_unprepare(csi2rx->p_clk); > > return 0; > > +err_disable_sysclk: > + clk_disable_unprepare(csi2rx->sys_clk); > err_disable_pixclk: > for (; i > 0; i--) > clk_disable_unprepare(csi2rx->pixel_clk[i - 1]); > @@ -200,6 +328,13 @@ static void csi2rx_stop(struct csi2rx_priv *csi2rx) > > if (v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, false)) > dev_warn(csi2rx->dev, "Couldn't disable our subdev\n"); > + > + if (csi2rx->dphy) { > + writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG); > + > + if (phy_power_off(csi2rx->dphy)) > + dev_warn(csi2rx->dev, "Couldn't power off DPHY\n"); > + } > } > > static int csi2rx_s_stream(struct v4l2_subdev *subdev, int enable) > @@ -306,15 +441,6 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx, > return PTR_ERR(csi2rx->dphy); > } > > - /* > - * FIXME: Once we'll have external D-PHY support, the check > - * will need to be removed. > - */ > - if (csi2rx->dphy) { > - dev_err(&pdev->dev, "External D-PHY not supported yet\n"); > - return -EINVAL; > - } > - > clk_prepare_enable(csi2rx->p_clk); > dev_cfg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG); > clk_disable_unprepare(csi2rx->p_clk); > @@ -339,7 +465,7 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx, > * FIXME: Once we'll have internal D-PHY support, the check > * will need to be removed. > */ > - if (csi2rx->has_internal_dphy) { > + if (!csi2rx->dphy && csi2rx->has_internal_dphy) { > dev_err(&pdev->dev, "Internal D-PHY not supported yet\n"); > return -EINVAL; > } > @@ -460,6 +586,7 @@ static int csi2rx_probe(struct platform_device *pdev) > dev_info(&pdev->dev, > "Probed CSI2RX with %u/%u lanes, %u streams, %s D-PHY\n", > csi2rx->num_lanes, csi2rx->max_lanes, csi2rx->max_streams, > + csi2rx->dphy ? "external" : > csi2rx->has_internal_dphy ? "internal" : "no"); > > return 0;
On 31/03/21 05:24PM, Chunfeng Yun wrote: > On Tue, 2021-03-30 at 23:03 +0530, Pratyush Yadav wrote: > > Some platforms like TI's J721E can have the CSI2RX paired with an > > external DPHY. Add support to enable and configure the DPHY using the > > generic PHY framework. > > > > Get the pixel rate and bpp from the subdev and pass them on to the DPHY > > along with the number of lanes. All other settings are left to their > > default values. > > > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > > --- > > drivers/media/platform/cadence/cdns-csi2rx.c | 147 +++++++++++++++++-- > > 1 file changed, 137 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c > > index c68a3eac62cd..31bd80e3f780 100644 > > --- a/drivers/media/platform/cadence/cdns-csi2rx.c > > +++ b/drivers/media/platform/cadence/cdns-csi2rx.c > > @@ -30,6 +30,12 @@ > > #define CSI2RX_STATIC_CFG_DLANE_MAP(llane, plane) ((plane) << (16 + (llane) * 4)) > > #define CSI2RX_STATIC_CFG_LANES_MASK GENMASK(11, 8) > > > > +#define CSI2RX_DPHY_LANE_CTRL_REG 0x40 > > +#define CSI2RX_DPHY_CL_RST BIT(16) > > +#define CSI2RX_DPHY_DL_RST(i) BIT((i) + 12) > > +#define CSI2RX_DPHY_CL_EN BIT(4) > > +#define CSI2RX_DPHY_DL_EN(i) BIT(i) > > + > > #define CSI2RX_STREAM_BASE(n) (((n) + 1) * 0x100) > > > > #define CSI2RX_STREAM_CTRL_REG(n) (CSI2RX_STREAM_BASE(n) + 0x000) > > @@ -54,6 +60,11 @@ enum csi2rx_pads { > > CSI2RX_PAD_MAX, > > }; > > > > +struct csi2rx_fmt { > > + u32 code; > > + u8 bpp; > > +}; > > + > > struct csi2rx_priv { > > struct device *dev; > > unsigned int count; > > @@ -85,6 +96,52 @@ struct csi2rx_priv { > > int source_pad; > > }; > > > > +static const struct csi2rx_fmt formats[] = { > > + { > > + .code = MEDIA_BUS_FMT_YUYV8_2X8, > > + .bpp = 16, > > + }, > > + { > > + .code = MEDIA_BUS_FMT_UYVY8_2X8, > > + .bpp = 16, > > + }, > > + { > > + .code = MEDIA_BUS_FMT_YVYU8_2X8, > > + .bpp = 16, > > + }, > > + { > > + .code = MEDIA_BUS_FMT_VYUY8_2X8, > > + .bpp = 16, > > + }, > > +}; > > + > > +static u8 csi2rx_get_bpp(u32 code) > > +{ > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(formats); i++) { > > + if (formats[i].code == code) > > + return formats[i].bpp; > > + } > > + > > + return 0; > > +} > > + > > +static s64 csi2rx_get_pixel_rate(struct csi2rx_priv *csi2rx) > > +{ > > + struct v4l2_ctrl *ctrl; > > + > > + ctrl = v4l2_ctrl_find(csi2rx->source_subdev->ctrl_handler, > > + V4L2_CID_PIXEL_RATE); > > + if (!ctrl) { > > + dev_err(csi2rx->dev, "no pixel rate control in subdev: %s\n", > > + csi2rx->source_subdev->name); > > + return -EINVAL; > > + } > > + > > + return v4l2_ctrl_g_ctrl_int64(ctrl); > > +} > > + > > static inline > > struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev) > > { > > @@ -101,6 +158,55 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx) > > writel(0, csi2rx->base + CSI2RX_SOFT_RESET_REG); > > } > > > > +static int csi2rx_configure_external_dphy(struct csi2rx_priv *csi2rx) > > +{ > > + union phy_configure_opts opts = { }; > > + struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy; > > + struct v4l2_subdev_format sd_fmt; > > + s64 pixel_rate; > > + int ret; > > + u8 bpp; > > + > > + sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; > > + sd_fmt.pad = 0; > > + > > + ret = v4l2_subdev_call(csi2rx->source_subdev, pad, get_fmt, NULL, > > + &sd_fmt); > > + if (ret) > > + return ret; > > + > > + bpp = csi2rx_get_bpp(sd_fmt.format.code); > > + if (!bpp) > > + return -EINVAL; > > + > > + pixel_rate = csi2rx_get_pixel_rate(csi2rx); > > + if (pixel_rate < 0) > > + return pixel_rate; > > + > > + ret = phy_mipi_dphy_get_default_config(pixel_rate, bpp, > > + csi2rx->num_lanes, cfg); > > + if (ret) > > + return ret; > > + > > + ret = phy_set_mode_ext(csi2rx->dphy, PHY_MODE_MIPI_DPHY, > > + PHY_MIPI_DPHY_SUBMODE_RX); > > + if (ret) > > + return ret; > > + > > + ret = phy_power_on(csi2rx->dphy); > > + if (ret) > > + return ret; > Seems phy_power_on, then phy_set_mode_ext? Shouldn't the mode be set before the PHY is powered on so the correct power on procedure can be performed based on the mode of operation? > > > + > > + ret = phy_configure(csi2rx->dphy, &opts); > > + if (ret) { > > + /* Can't do anything if it fails. Ignore the return value. */ > > + phy_power_off(csi2rx->dphy); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > static int csi2rx_start(struct csi2rx_priv *csi2rx) > > { > > unsigned int i; > > @@ -139,6 +245,17 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx) > > if (ret) > > goto err_disable_pclk; > > > > + /* Enable DPHY clk and data lanes. */ > > + if (csi2rx->dphy) { > > + reg = CSI2RX_DPHY_CL_EN | CSI2RX_DPHY_CL_RST; > > + for (i = 0; i < csi2rx->num_lanes; i++) { > > + reg |= CSI2RX_DPHY_DL_EN(csi2rx->lanes[i] - 1); > > + reg |= CSI2RX_DPHY_DL_RST(csi2rx->lanes[i] - 1); > > + } > > + > > + writel(reg, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG); > > + } > > + > > /* > > * Create a static mapping between the CSI virtual channels > > * and the output stream. > > @@ -169,10 +286,21 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx) > > if (ret) > > goto err_disable_pixclk; > > > > + if (csi2rx->dphy) { > > + ret = csi2rx_configure_external_dphy(csi2rx); > > + if (ret) { > > + dev_err(csi2rx->dev, > > + "Failed to configure external DPHY: %d\n", ret); > > + goto err_disable_sysclk; > > + } > > + } > > + > > clk_disable_unprepare(csi2rx->p_clk); > > > > return 0; > > > > +err_disable_sysclk: > > + clk_disable_unprepare(csi2rx->sys_clk); > > err_disable_pixclk: > > for (; i > 0; i--) > > clk_disable_unprepare(csi2rx->pixel_clk[i - 1]); > > @@ -200,6 +328,13 @@ static void csi2rx_stop(struct csi2rx_priv *csi2rx) > > > > if (v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, false)) > > dev_warn(csi2rx->dev, "Couldn't disable our subdev\n"); > > + > > + if (csi2rx->dphy) { > > + writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG); > > + > > + if (phy_power_off(csi2rx->dphy)) > > + dev_warn(csi2rx->dev, "Couldn't power off DPHY\n"); > > + } > > } > > > > static int csi2rx_s_stream(struct v4l2_subdev *subdev, int enable) > > @@ -306,15 +441,6 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx, > > return PTR_ERR(csi2rx->dphy); > > } > > > > - /* > > - * FIXME: Once we'll have external D-PHY support, the check > > - * will need to be removed. > > - */ > > - if (csi2rx->dphy) { > > - dev_err(&pdev->dev, "External D-PHY not supported yet\n"); > > - return -EINVAL; > > - } > > - > > clk_prepare_enable(csi2rx->p_clk); > > dev_cfg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG); > > clk_disable_unprepare(csi2rx->p_clk); > > @@ -339,7 +465,7 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx, > > * FIXME: Once we'll have internal D-PHY support, the check > > * will need to be removed. > > */ > > - if (csi2rx->has_internal_dphy) { > > + if (!csi2rx->dphy && csi2rx->has_internal_dphy) { > > dev_err(&pdev->dev, "Internal D-PHY not supported yet\n"); > > return -EINVAL; > > } > > @@ -460,6 +586,7 @@ static int csi2rx_probe(struct platform_device *pdev) > > dev_info(&pdev->dev, > > "Probed CSI2RX with %u/%u lanes, %u streams, %s D-PHY\n", > > csi2rx->num_lanes, csi2rx->max_lanes, csi2rx->max_streams, > > + csi2rx->dphy ? "external" : > > csi2rx->has_internal_dphy ? "internal" : "no"); > > > > return 0; >
On Wed, 2021-04-07 at 00:24 +0530, Pratyush Yadav wrote: > On 31/03/21 05:24PM, Chunfeng Yun wrote: > > On Tue, 2021-03-30 at 23:03 +0530, Pratyush Yadav wrote: > > > Some platforms like TI's J721E can have the CSI2RX paired with an > > > external DPHY. Add support to enable and configure the DPHY using the > > > generic PHY framework. > > > > > > Get the pixel rate and bpp from the subdev and pass them on to the DPHY > > > along with the number of lanes. All other settings are left to their > > > default values. > > > > > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > > > --- > > > drivers/media/platform/cadence/cdns-csi2rx.c | 147 +++++++++++++++++-- > > > 1 file changed, 137 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c > > > index c68a3eac62cd..31bd80e3f780 100644 > > > --- a/drivers/media/platform/cadence/cdns-csi2rx.c > > > +++ b/drivers/media/platform/cadence/cdns-csi2rx.c > > > @@ -30,6 +30,12 @@ > > > #define CSI2RX_STATIC_CFG_DLANE_MAP(llane, plane) ((plane) << (16 + (llane) * 4)) > > > #define CSI2RX_STATIC_CFG_LANES_MASK GENMASK(11, 8) > > > > > > +#define CSI2RX_DPHY_LANE_CTRL_REG 0x40 > > > +#define CSI2RX_DPHY_CL_RST BIT(16) > > > +#define CSI2RX_DPHY_DL_RST(i) BIT((i) + 12) > > > +#define CSI2RX_DPHY_CL_EN BIT(4) > > > +#define CSI2RX_DPHY_DL_EN(i) BIT(i) > > > + > > > #define CSI2RX_STREAM_BASE(n) (((n) + 1) * 0x100) > > > > > > #define CSI2RX_STREAM_CTRL_REG(n) (CSI2RX_STREAM_BASE(n) + 0x000) > > > @@ -54,6 +60,11 @@ enum csi2rx_pads { > > > CSI2RX_PAD_MAX, > > > }; > > > > > > +struct csi2rx_fmt { > > > + u32 code; > > > + u8 bpp; > > > +}; > > > + > > > struct csi2rx_priv { > > > struct device *dev; > > > unsigned int count; > > > @@ -85,6 +96,52 @@ struct csi2rx_priv { > > > int source_pad; > > > }; > > > > > > +static const struct csi2rx_fmt formats[] = { > > > + { > > > + .code = MEDIA_BUS_FMT_YUYV8_2X8, > > > + .bpp = 16, > > > + }, > > > + { > > > + .code = MEDIA_BUS_FMT_UYVY8_2X8, > > > + .bpp = 16, > > > + }, > > > + { > > > + .code = MEDIA_BUS_FMT_YVYU8_2X8, > > > + .bpp = 16, > > > + }, > > > + { > > > + .code = MEDIA_BUS_FMT_VYUY8_2X8, > > > + .bpp = 16, > > > + }, > > > +}; > > > + > > > +static u8 csi2rx_get_bpp(u32 code) > > > +{ > > > + int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(formats); i++) { > > > + if (formats[i].code == code) > > > + return formats[i].bpp; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static s64 csi2rx_get_pixel_rate(struct csi2rx_priv *csi2rx) > > > +{ > > > + struct v4l2_ctrl *ctrl; > > > + > > > + ctrl = v4l2_ctrl_find(csi2rx->source_subdev->ctrl_handler, > > > + V4L2_CID_PIXEL_RATE); > > > + if (!ctrl) { > > > + dev_err(csi2rx->dev, "no pixel rate control in subdev: %s\n", > > > + csi2rx->source_subdev->name); > > > + return -EINVAL; > > > + } > > > + > > > + return v4l2_ctrl_g_ctrl_int64(ctrl); > > > +} > > > + > > > static inline > > > struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev) > > > { > > > @@ -101,6 +158,55 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx) > > > writel(0, csi2rx->base + CSI2RX_SOFT_RESET_REG); > > > } > > > > > > +static int csi2rx_configure_external_dphy(struct csi2rx_priv *csi2rx) > > > +{ > > > + union phy_configure_opts opts = { }; > > > + struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy; > > > + struct v4l2_subdev_format sd_fmt; > > > + s64 pixel_rate; > > > + int ret; > > > + u8 bpp; > > > + > > > + sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; > > > + sd_fmt.pad = 0; > > > + > > > + ret = v4l2_subdev_call(csi2rx->source_subdev, pad, get_fmt, NULL, > > > + &sd_fmt); > > > + if (ret) > > > + return ret; > > > + > > > + bpp = csi2rx_get_bpp(sd_fmt.format.code); > > > + if (!bpp) > > > + return -EINVAL; > > > + > > > + pixel_rate = csi2rx_get_pixel_rate(csi2rx); > > > + if (pixel_rate < 0) > > > + return pixel_rate; > > > + > > > + ret = phy_mipi_dphy_get_default_config(pixel_rate, bpp, > > > + csi2rx->num_lanes, cfg); > > > + if (ret) > > > + return ret; > > > + > > > + ret = phy_set_mode_ext(csi2rx->dphy, PHY_MODE_MIPI_DPHY, > > > + PHY_MIPI_DPHY_SUBMODE_RX); > > > + if (ret) > > > + return ret; > > > + > > > + ret = phy_power_on(csi2rx->dphy); > > > + if (ret) > > > + return ret; > > Seems phy_power_on, then phy_set_mode_ext? > > Shouldn't the mode be set before the PHY is powered on so the correct > power on procedure can be performed based on the mode of operation? Of course, it is fine for cnds-dphy. But it depends on HW design and also phy driver; if the mode is controlled in PHY IP register, we can't access it before phy_power_on if no phy_init called (e.g. clock/power is not enabled). Just let you pay attention on the phy sequence. Thanks > > > > > > + > > > + ret = phy_configure(csi2rx->dphy, &opts); > > > + if (ret) { > > > + /* Can't do anything if it fails. Ignore the return value. */ > > > + phy_power_off(csi2rx->dphy); > > > + return ret; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > static int csi2rx_start(struct csi2rx_priv *csi2rx) > > > { > > > unsigned int i; > > > @@ -139,6 +245,17 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx) > > > if (ret) > > > goto err_disable_pclk; > > > > > > + /* Enable DPHY clk and data lanes. */ > > > + if (csi2rx->dphy) { > > > + reg = CSI2RX_DPHY_CL_EN | CSI2RX_DPHY_CL_RST; > > > + for (i = 0; i < csi2rx->num_lanes; i++) { > > > + reg |= CSI2RX_DPHY_DL_EN(csi2rx->lanes[i] - 1); > > > + reg |= CSI2RX_DPHY_DL_RST(csi2rx->lanes[i] - 1); > > > + } > > > + > > > + writel(reg, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG); > > > + } > > > + > > > /* > > > * Create a static mapping between the CSI virtual channels > > > * and the output stream. > > > @@ -169,10 +286,21 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx) > > > if (ret) > > > goto err_disable_pixclk; > > > > > > + if (csi2rx->dphy) { > > > + ret = csi2rx_configure_external_dphy(csi2rx); > > > + if (ret) { > > > + dev_err(csi2rx->dev, > > > + "Failed to configure external DPHY: %d\n", ret); > > > + goto err_disable_sysclk; > > > + } > > > + } > > > + > > > clk_disable_unprepare(csi2rx->p_clk); > > > > > > return 0; > > > > > > +err_disable_sysclk: > > > + clk_disable_unprepare(csi2rx->sys_clk); > > > err_disable_pixclk: > > > for (; i > 0; i--) > > > clk_disable_unprepare(csi2rx->pixel_clk[i - 1]); > > > @@ -200,6 +328,13 @@ static void csi2rx_stop(struct csi2rx_priv *csi2rx) > > > > > > if (v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, false)) > > > dev_warn(csi2rx->dev, "Couldn't disable our subdev\n"); > > > + > > > + if (csi2rx->dphy) { > > > + writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG); > > > + > > > + if (phy_power_off(csi2rx->dphy)) > > > + dev_warn(csi2rx->dev, "Couldn't power off DPHY\n"); > > > + } > > > } > > > > > > static int csi2rx_s_stream(struct v4l2_subdev *subdev, int enable) > > > @@ -306,15 +441,6 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx, > > > return PTR_ERR(csi2rx->dphy); > > > } > > > > > > - /* > > > - * FIXME: Once we'll have external D-PHY support, the check > > > - * will need to be removed. > > > - */ > > > - if (csi2rx->dphy) { > > > - dev_err(&pdev->dev, "External D-PHY not supported yet\n"); > > > - return -EINVAL; > > > - } > > > - > > > clk_prepare_enable(csi2rx->p_clk); > > > dev_cfg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG); > > > clk_disable_unprepare(csi2rx->p_clk); > > > @@ -339,7 +465,7 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx, > > > * FIXME: Once we'll have internal D-PHY support, the check > > > * will need to be removed. > > > */ > > > - if (csi2rx->has_internal_dphy) { > > > + if (!csi2rx->dphy && csi2rx->has_internal_dphy) { > > > dev_err(&pdev->dev, "Internal D-PHY not supported yet\n"); > > > return -EINVAL; > > > } > > > @@ -460,6 +586,7 @@ static int csi2rx_probe(struct platform_device *pdev) > > > dev_info(&pdev->dev, > > > "Probed CSI2RX with %u/%u lanes, %u streams, %s D-PHY\n", > > > csi2rx->num_lanes, csi2rx->max_lanes, csi2rx->max_streams, > > > + csi2rx->dphy ? "external" : > > > csi2rx->has_internal_dphy ? "internal" : "no"); > > > > > > return 0; > > >
On 08/04/2021 11:13, Chunfeng Yun wrote: > On Wed, 2021-04-07 at 00:24 +0530, Pratyush Yadav wrote: >> On 31/03/21 05:24PM, Chunfeng Yun wrote: >>> On Tue, 2021-03-30 at 23:03 +0530, Pratyush Yadav wrote: >>>> Some platforms like TI's J721E can have the CSI2RX paired with an >>>> external DPHY. Add support to enable and configure the DPHY using the >>>> generic PHY framework. >>>> >>>> Get the pixel rate and bpp from the subdev and pass them on to the DPHY >>>> along with the number of lanes. All other settings are left to their >>>> default values. >>>> >>>> Signed-off-by: Pratyush Yadav <p.yadav@ti.com> >>>> --- >>>> drivers/media/platform/cadence/cdns-csi2rx.c | 147 +++++++++++++++++-- >>>> 1 file changed, 137 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c >>>> index c68a3eac62cd..31bd80e3f780 100644 >>>> --- a/drivers/media/platform/cadence/cdns-csi2rx.c >>>> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c >>>> @@ -30,6 +30,12 @@ >>>> #define CSI2RX_STATIC_CFG_DLANE_MAP(llane, plane) ((plane) << (16 + (llane) * 4)) >>>> #define CSI2RX_STATIC_CFG_LANES_MASK GENMASK(11, 8) >>>> >>>> +#define CSI2RX_DPHY_LANE_CTRL_REG 0x40 >>>> +#define CSI2RX_DPHY_CL_RST BIT(16) >>>> +#define CSI2RX_DPHY_DL_RST(i) BIT((i) + 12) >>>> +#define CSI2RX_DPHY_CL_EN BIT(4) >>>> +#define CSI2RX_DPHY_DL_EN(i) BIT(i) >>>> + >>>> #define CSI2RX_STREAM_BASE(n) (((n) + 1) * 0x100) >>>> >>>> #define CSI2RX_STREAM_CTRL_REG(n) (CSI2RX_STREAM_BASE(n) + 0x000) >>>> @@ -54,6 +60,11 @@ enum csi2rx_pads { >>>> CSI2RX_PAD_MAX, >>>> }; >>>> >>>> +struct csi2rx_fmt { >>>> + u32 code; >>>> + u8 bpp; >>>> +}; >>>> + >>>> struct csi2rx_priv { >>>> struct device *dev; >>>> unsigned int count; >>>> @@ -85,6 +96,52 @@ struct csi2rx_priv { >>>> int source_pad; >>>> }; >>>> >>>> +static const struct csi2rx_fmt formats[] = { >>>> + { >>>> + .code = MEDIA_BUS_FMT_YUYV8_2X8, >>>> + .bpp = 16, >>>> + }, >>>> + { >>>> + .code = MEDIA_BUS_FMT_UYVY8_2X8, >>>> + .bpp = 16, >>>> + }, >>>> + { >>>> + .code = MEDIA_BUS_FMT_YVYU8_2X8, >>>> + .bpp = 16, >>>> + }, >>>> + { >>>> + .code = MEDIA_BUS_FMT_VYUY8_2X8, >>>> + .bpp = 16, >>>> + }, >>>> +}; >>>> + >>>> +static u8 csi2rx_get_bpp(u32 code) >>>> +{ >>>> + int i; >>>> + >>>> + for (i = 0; i < ARRAY_SIZE(formats); i++) { >>>> + if (formats[i].code == code) >>>> + return formats[i].bpp; >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static s64 csi2rx_get_pixel_rate(struct csi2rx_priv *csi2rx) >>>> +{ >>>> + struct v4l2_ctrl *ctrl; >>>> + >>>> + ctrl = v4l2_ctrl_find(csi2rx->source_subdev->ctrl_handler, >>>> + V4L2_CID_PIXEL_RATE); >>>> + if (!ctrl) { >>>> + dev_err(csi2rx->dev, "no pixel rate control in subdev: %s\n", >>>> + csi2rx->source_subdev->name); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + return v4l2_ctrl_g_ctrl_int64(ctrl); >>>> +} >>>> + >>>> static inline >>>> struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev) >>>> { >>>> @@ -101,6 +158,55 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx) >>>> writel(0, csi2rx->base + CSI2RX_SOFT_RESET_REG); >>>> } >>>> >>>> +static int csi2rx_configure_external_dphy(struct csi2rx_priv *csi2rx) >>>> +{ >>>> + union phy_configure_opts opts = { }; >>>> + struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy; >>>> + struct v4l2_subdev_format sd_fmt; >>>> + s64 pixel_rate; >>>> + int ret; >>>> + u8 bpp; >>>> + >>>> + sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; >>>> + sd_fmt.pad = 0; >>>> + >>>> + ret = v4l2_subdev_call(csi2rx->source_subdev, pad, get_fmt, NULL, >>>> + &sd_fmt); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + bpp = csi2rx_get_bpp(sd_fmt.format.code); >>>> + if (!bpp) >>>> + return -EINVAL; >>>> + >>>> + pixel_rate = csi2rx_get_pixel_rate(csi2rx); >>>> + if (pixel_rate < 0) >>>> + return pixel_rate; >>>> + >>>> + ret = phy_mipi_dphy_get_default_config(pixel_rate, bpp, >>>> + csi2rx->num_lanes, cfg); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + ret = phy_set_mode_ext(csi2rx->dphy, PHY_MODE_MIPI_DPHY, >>>> + PHY_MIPI_DPHY_SUBMODE_RX); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + ret = phy_power_on(csi2rx->dphy); >>>> + if (ret) >>>> + return ret; >>> Seems phy_power_on, then phy_set_mode_ext? >> >> Shouldn't the mode be set before the PHY is powered on so the correct >> power on procedure can be performed based on the mode of operation? > Of course, it is fine for cnds-dphy. > But it depends on HW design and also phy driver; > if the mode is controlled in PHY IP register, we can't access it before > phy_power_on if no phy_init called (e.g. clock/power is not enabled). > > Just let you pay attention on the phy sequence. I don't think the phy configuration should depend on phy_power_on, but the runtime PM. I guess this could be solved with: phy_pm_runtime_get_sync(); phy_set_mode_ext(); phy_power_on(); phy_pm_runtime_put(); Tomi
diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c index c68a3eac62cd..31bd80e3f780 100644 --- a/drivers/media/platform/cadence/cdns-csi2rx.c +++ b/drivers/media/platform/cadence/cdns-csi2rx.c @@ -30,6 +30,12 @@ #define CSI2RX_STATIC_CFG_DLANE_MAP(llane, plane) ((plane) << (16 + (llane) * 4)) #define CSI2RX_STATIC_CFG_LANES_MASK GENMASK(11, 8) +#define CSI2RX_DPHY_LANE_CTRL_REG 0x40 +#define CSI2RX_DPHY_CL_RST BIT(16) +#define CSI2RX_DPHY_DL_RST(i) BIT((i) + 12) +#define CSI2RX_DPHY_CL_EN BIT(4) +#define CSI2RX_DPHY_DL_EN(i) BIT(i) + #define CSI2RX_STREAM_BASE(n) (((n) + 1) * 0x100) #define CSI2RX_STREAM_CTRL_REG(n) (CSI2RX_STREAM_BASE(n) + 0x000) @@ -54,6 +60,11 @@ enum csi2rx_pads { CSI2RX_PAD_MAX, }; +struct csi2rx_fmt { + u32 code; + u8 bpp; +}; + struct csi2rx_priv { struct device *dev; unsigned int count; @@ -85,6 +96,52 @@ struct csi2rx_priv { int source_pad; }; +static const struct csi2rx_fmt formats[] = { + { + .code = MEDIA_BUS_FMT_YUYV8_2X8, + .bpp = 16, + }, + { + .code = MEDIA_BUS_FMT_UYVY8_2X8, + .bpp = 16, + }, + { + .code = MEDIA_BUS_FMT_YVYU8_2X8, + .bpp = 16, + }, + { + .code = MEDIA_BUS_FMT_VYUY8_2X8, + .bpp = 16, + }, +}; + +static u8 csi2rx_get_bpp(u32 code) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(formats); i++) { + if (formats[i].code == code) + return formats[i].bpp; + } + + return 0; +} + +static s64 csi2rx_get_pixel_rate(struct csi2rx_priv *csi2rx) +{ + struct v4l2_ctrl *ctrl; + + ctrl = v4l2_ctrl_find(csi2rx->source_subdev->ctrl_handler, + V4L2_CID_PIXEL_RATE); + if (!ctrl) { + dev_err(csi2rx->dev, "no pixel rate control in subdev: %s\n", + csi2rx->source_subdev->name); + return -EINVAL; + } + + return v4l2_ctrl_g_ctrl_int64(ctrl); +} + static inline struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev) { @@ -101,6 +158,55 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx) writel(0, csi2rx->base + CSI2RX_SOFT_RESET_REG); } +static int csi2rx_configure_external_dphy(struct csi2rx_priv *csi2rx) +{ + union phy_configure_opts opts = { }; + struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy; + struct v4l2_subdev_format sd_fmt; + s64 pixel_rate; + int ret; + u8 bpp; + + sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; + sd_fmt.pad = 0; + + ret = v4l2_subdev_call(csi2rx->source_subdev, pad, get_fmt, NULL, + &sd_fmt); + if (ret) + return ret; + + bpp = csi2rx_get_bpp(sd_fmt.format.code); + if (!bpp) + return -EINVAL; + + pixel_rate = csi2rx_get_pixel_rate(csi2rx); + if (pixel_rate < 0) + return pixel_rate; + + ret = phy_mipi_dphy_get_default_config(pixel_rate, bpp, + csi2rx->num_lanes, cfg); + if (ret) + return ret; + + ret = phy_set_mode_ext(csi2rx->dphy, PHY_MODE_MIPI_DPHY, + PHY_MIPI_DPHY_SUBMODE_RX); + if (ret) + return ret; + + ret = phy_power_on(csi2rx->dphy); + if (ret) + return ret; + + ret = phy_configure(csi2rx->dphy, &opts); + if (ret) { + /* Can't do anything if it fails. Ignore the return value. */ + phy_power_off(csi2rx->dphy); + return ret; + } + + return 0; +} + static int csi2rx_start(struct csi2rx_priv *csi2rx) { unsigned int i; @@ -139,6 +245,17 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx) if (ret) goto err_disable_pclk; + /* Enable DPHY clk and data lanes. */ + if (csi2rx->dphy) { + reg = CSI2RX_DPHY_CL_EN | CSI2RX_DPHY_CL_RST; + for (i = 0; i < csi2rx->num_lanes; i++) { + reg |= CSI2RX_DPHY_DL_EN(csi2rx->lanes[i] - 1); + reg |= CSI2RX_DPHY_DL_RST(csi2rx->lanes[i] - 1); + } + + writel(reg, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG); + } + /* * Create a static mapping between the CSI virtual channels * and the output stream. @@ -169,10 +286,21 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx) if (ret) goto err_disable_pixclk; + if (csi2rx->dphy) { + ret = csi2rx_configure_external_dphy(csi2rx); + if (ret) { + dev_err(csi2rx->dev, + "Failed to configure external DPHY: %d\n", ret); + goto err_disable_sysclk; + } + } + clk_disable_unprepare(csi2rx->p_clk); return 0; +err_disable_sysclk: + clk_disable_unprepare(csi2rx->sys_clk); err_disable_pixclk: for (; i > 0; i--) clk_disable_unprepare(csi2rx->pixel_clk[i - 1]); @@ -200,6 +328,13 @@ static void csi2rx_stop(struct csi2rx_priv *csi2rx) if (v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, false)) dev_warn(csi2rx->dev, "Couldn't disable our subdev\n"); + + if (csi2rx->dphy) { + writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG); + + if (phy_power_off(csi2rx->dphy)) + dev_warn(csi2rx->dev, "Couldn't power off DPHY\n"); + } } static int csi2rx_s_stream(struct v4l2_subdev *subdev, int enable) @@ -306,15 +441,6 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx, return PTR_ERR(csi2rx->dphy); } - /* - * FIXME: Once we'll have external D-PHY support, the check - * will need to be removed. - */ - if (csi2rx->dphy) { - dev_err(&pdev->dev, "External D-PHY not supported yet\n"); - return -EINVAL; - } - clk_prepare_enable(csi2rx->p_clk); dev_cfg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG); clk_disable_unprepare(csi2rx->p_clk); @@ -339,7 +465,7 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx, * FIXME: Once we'll have internal D-PHY support, the check * will need to be removed. */ - if (csi2rx->has_internal_dphy) { + if (!csi2rx->dphy && csi2rx->has_internal_dphy) { dev_err(&pdev->dev, "Internal D-PHY not supported yet\n"); return -EINVAL; } @@ -460,6 +586,7 @@ static int csi2rx_probe(struct platform_device *pdev) dev_info(&pdev->dev, "Probed CSI2RX with %u/%u lanes, %u streams, %s D-PHY\n", csi2rx->num_lanes, csi2rx->max_lanes, csi2rx->max_streams, + csi2rx->dphy ? "external" : csi2rx->has_internal_dphy ? "internal" : "no"); return 0;
Some platforms like TI's J721E can have the CSI2RX paired with an external DPHY. Add support to enable and configure the DPHY using the generic PHY framework. Get the pixel rate and bpp from the subdev and pass them on to the DPHY along with the number of lanes. All other settings are left to their default values. Signed-off-by: Pratyush Yadav <p.yadav@ti.com> --- drivers/media/platform/cadence/cdns-csi2rx.c | 147 +++++++++++++++++-- 1 file changed, 137 insertions(+), 10 deletions(-)