Hi Paul,
Thank you for the patch.
On Wed, Jun 15, 2022 at 04:11:13AM +0900, Paul Elder wrote:
> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> Connect the CSI receiver subdevice into the rest of the driver. This
> includes:
> - calling the subdevice via the v4l2 subdev API
> - moving the async notifier for the sensor from the ISP to the CSI
> receiver
> - in the ISP, create a media link to the CSI receiver, and remove the
> media link creation to the sensor
> - in the CSI receiver, create a media link to the sensor
>
> Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> .../platform/rockchip/rkisp1/rkisp1-csi.c | 34 ++++++++++++++++--
> .../platform/rockchip/rkisp1/rkisp1-csi.h | 6 ++--
> .../platform/rockchip/rkisp1/rkisp1-dev.c | 36 +++++++++----------
> .../platform/rockchip/rkisp1/rkisp1-isp.c | 21 ++---------
> 4 files changed, 53 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> index 8182694a6fe0..96712b467dde 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c
> @@ -43,6 +43,34 @@ rkisp1_csi_get_pad_fmt(struct rkisp1_csi *csi,
> return v4l2_subdev_get_try_format(&csi->sd, &state, pad);
> }
>
> +int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
> + struct rkisp1_sensor_async *s_asd,
> + unsigned int source_pad)
> +{
> + struct rkisp1_csi *csi = &rkisp1->csi;
> + int ret;
There's trailing whitespace here.
> +
> + s_asd->pixel_rate_ctrl = v4l2_ctrl_find(sd->ctrl_handler,
> + V4L2_CID_PIXEL_RATE);
> + if (!s_asd->pixel_rate_ctrl) {
> + dev_err(rkisp1->dev, "No pixel rate control in subdev %s\n",
> + sd->name);
> + return -EINVAL;
> + }
> +
> + /* Create the link from the sensor to the CSI receiver. */
> + ret = media_create_pad_link(&sd->entity, source_pad,
> + &csi->sd.entity, RKISP1_CSI_PAD_SINK,
> + !s_asd->index ? MEDIA_LNK_FL_ENABLED : 0);
> + if (ret) {
> + dev_err(csi->rkisp1->dev, "failed to link src pad of %s\n",
> + sd->name);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int rkisp1_csi_config(struct rkisp1_csi *csi,
> const struct rkisp1_sensor_async *sensor)
> {
> @@ -118,8 +146,8 @@ static void rkisp1_csi_disable(struct rkisp1_csi *csi)
> val & (~RKISP1_CIF_MIPI_CTRL_OUTPUT_ENA));
> }
>
> -int rkisp1_csi_start(struct rkisp1_csi *csi,
> - const struct rkisp1_sensor_async *sensor)
> +static int rkisp1_csi_start(struct rkisp1_csi *csi,
> + const struct rkisp1_sensor_async *sensor)
> {
> struct rkisp1_device *rkisp1 = csi->rkisp1;
> union phy_configure_opts opts;
> @@ -155,7 +183,7 @@ int rkisp1_csi_start(struct rkisp1_csi *csi,
> return 0;
> }
>
> -void rkisp1_csi_stop(struct rkisp1_csi *csi)
> +static void rkisp1_csi_stop(struct rkisp1_csi *csi)
> {
> rkisp1_csi_disable(csi);
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h
> index ddf8e5e08f55..eadcd24f65fb 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.h
> @@ -21,8 +21,8 @@ void rkisp1_csi_cleanup(struct rkisp1_device *rkisp1);
> int rkisp1_csi_register(struct rkisp1_device *rkisp1);
> void rkisp1_csi_unregister(struct rkisp1_device *rkisp1);
>
> -int rkisp1_csi_start(struct rkisp1_csi *csi,
> - const struct rkisp1_sensor_async *sensor);
> -void rkisp1_csi_stop(struct rkisp1_csi *csi);
> +int rkisp1_csi_link_sensor(struct rkisp1_device *rkisp1, struct v4l2_subdev *sd,
> + struct rkisp1_sensor_async *s_asd,
> + unsigned int source_pad);
>
> #endif /* _RKISP1_CSI_H */
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index faf2cd4c8149..a3e182c86bdd 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -17,6 +17,7 @@
> #include <linux/pinctrl/consumer.h>
> #include <linux/pm_runtime.h>
> #include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
>
> #include "rkisp1-common.h"
> #include "rkisp1-csi.h"
> @@ -119,17 +120,8 @@ static int rkisp1_subdev_notifier_bound(struct v4l2_async_notifier *notifier,
> container_of(asd, struct rkisp1_sensor_async, asd);
> int source_pad;
>
> - s_asd->pixel_rate_ctrl = v4l2_ctrl_find(sd->ctrl_handler,
> - V4L2_CID_PIXEL_RATE);
> - if (!s_asd->pixel_rate_ctrl) {
> - dev_err(rkisp1->dev, "No pixel rate control in subdev %s\n",
> - sd->name);
> - return -EINVAL;
> - }
> -
> s_asd->sd = sd;
>
> - /* Create the link to the sensor. */
> source_pad = media_entity_get_fwnode_pad(&sd->entity, s_asd->source_ep,
> MEDIA_PAD_FL_SOURCE);
> if (source_pad < 0) {
> @@ -138,10 +130,7 @@ static int rkisp1_subdev_notifier_bound(struct v4l2_async_notifier *notifier,
> return source_pad;
> }
>
> - return media_create_pad_link(&sd->entity, source_pad,
> - &rkisp1->isp.sd.entity,
> - RKISP1_ISP_PAD_SINK_VIDEO,
> - !s_asd->index ? MEDIA_LNK_FL_ENABLED : 0);
> + return rkisp1_csi_link_sensor(rkisp1, sd, s_asd, source_pad);
> }
>
> static int rkisp1_subdev_notifier_complete(struct v4l2_async_notifier *notifier)
> @@ -283,6 +272,14 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
> unsigned int i;
> int ret;
>
> + /* Link the CSI receiver to the ISP. */
> + ret = media_create_pad_link(&rkisp1->csi.sd.entity, RKISP1_CSI_PAD_SRC,
> + &rkisp1->isp.sd.entity,
> + RKISP1_ISP_PAD_SINK_VIDEO,
> + MEDIA_LNK_FL_ENABLED);
> + if (ret)
> + return ret;
> +
> /* create ISP->RSZ->CAP links */
> for (i = 0; i < 2; i++) {
> struct media_entity *resizer =
> @@ -364,13 +361,6 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1)
> if (ret)
> goto error;
>
> - ret = rkisp1_subdev_notifier_register(rkisp1);
> - if (ret) {
> - dev_err(rkisp1->dev,
> - "Failed to register subdev notifier(%d)\n", ret);
> - goto error;
> - }
> -
> return 0;
>
> error:
> @@ -534,10 +524,16 @@ static int rkisp1_probe(struct platform_device *pdev)
> if (ret)
> goto err_cleanup_csi;
>
> + ret = rkisp1_subdev_notifier_register(rkisp1);
> + if (ret)
> + goto err_unreg_entities;
> +
> rkisp1_debug_init(rkisp1);
>
> return 0;
>
> +err_unreg_entities:
> + rkisp1_entities_unregister(rkisp1);
> err_cleanup_csi:
> rkisp1_csi_cleanup(rkisp1);
> err_unreg_media_dev:
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 5afb8be311c7..260c9ce0dca4 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -16,7 +16,6 @@
> #include <media/v4l2-event.h>
>
> #include "rkisp1-common.h"
> -#include "rkisp1-csi.h"
>
> #define RKISP1_DEF_SINK_PAD_FMT MEDIA_BUS_FMT_SRGGB10_1X10
> #define RKISP1_DEF_SRC_PAD_FMT MEDIA_BUS_FMT_YUYV8_2X8
> @@ -728,16 +727,12 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct rkisp1_isp *isp = to_rkisp1_isp(sd);
> struct rkisp1_device *rkisp1 = isp->rkisp1;
> - const struct rkisp1_sensor_async *asd;
> struct media_pad *source_pad;
> int ret;
>
> if (!enable) {
> v4l2_subdev_call(rkisp1->source, video, s_stream, false);
> -
> - rkisp1_csi_stop(&rkisp1->csi);
> rkisp1_isp_stop(isp);
> -
> return 0;
> }
>
> @@ -754,30 +749,20 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
> return -EPIPE;
> }
>
> - asd = container_of(rkisp1->source->asd, struct rkisp1_sensor_async,
> - asd);
> -
> - if (asd->mbus_type != V4L2_MBUS_CSI2_DPHY)
> - return -EINVAL;
> + if (rkisp1->source != &rkisp1->csi.sd)
> + return -EPIPE;
>
> isp->frame_sequence = -1;
> mutex_lock(&isp->ops_lock);
> - ret = rkisp1_config_cif(isp, asd->mbus_type, asd->mbus_flags);
> + ret = rkisp1_config_cif(isp, V4L2_MBUS_CSI2_DPHY, 0);
> if (ret)
> goto mutex_unlock;
>
> rkisp1_isp_start(isp);
>
> - ret = rkisp1_csi_start(&rkisp1->csi, asd);
> - if (ret) {
> - rkisp1_isp_stop(isp);
> - goto mutex_unlock;
> - }
> -
> ret = v4l2_subdev_call(rkisp1->source, video, s_stream, true);
> if (ret) {
> rkisp1_isp_stop(isp);
> - rkisp1_csi_stop(&rkisp1->csi);
> goto mutex_unlock;
> }
>
> --
> 2.30.2
>
--
Regards,
Laurent Pinchart
