Messages in this thread | | | Subject | Re: [PATCH 05/16] media: cadence: csi2rx: Add external DPHY support | From | Tomi Valkeinen <> | Date | Thu, 8 Apr 2021 11:22:39 +0300 |
| |
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
| |