lkml.org 
[lkml]   [2022]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v6 03/14] media: cadence: csi2rx: Add get_fmt and set_fmt pad ops
    Hi Pratyush,

    Thank you for the patch.

    On Fri, Jan 21, 2022 at 07:58:53PM +0530, Pratyush Yadav wrote:
    > The format is needed to calculate the link speed for the external DPHY
    > configuration. It is not right to query the format from the source
    > subdev. Add get_fmt and set_fmt pad operations so that the format can be
    > configured and correct bpp be selected.
    >
    > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>

    Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

    > ---
    >
    > Changes in v6:
    > - Move the lock around the dereference for framefmt in
    > csi2rx_{get,set}_fmt() instead of when we get the pointer.
    > - Do not return an error when an unsupported format is set. Instead
    > adjust the code to the first format in the list.
    > - Not adding Laurent's R-by since I am making changes in
    > csi2rx_set_fmt() that he didn't explicitly recommend.
    >
    > Changes in v5:
    > - Use YUV 1X16 formats instead of 2X8.
    > - New in v5.
    >
    > drivers/media/platform/cadence/cdns-csi2rx.c | 131 +++++++++++++++++++
    > 1 file changed, 131 insertions(+)
    >
    > diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
    > index 2547903f2e8e..ae3ebdb3890d 100644
    > --- a/drivers/media/platform/cadence/cdns-csi2rx.c
    > +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
    > @@ -54,6 +54,11 @@ enum csi2rx_pads {
    > CSI2RX_PAD_MAX,
    > };
    >
    > +struct csi2rx_fmt {
    > + u32 code;
    > + u8 bpp;
    > +};
    > +
    > struct csi2rx_priv {
    > struct device *dev;
    > unsigned int count;
    > @@ -79,12 +84,43 @@ struct csi2rx_priv {
    > struct v4l2_subdev subdev;
    > struct v4l2_async_notifier notifier;
    > struct media_pad pads[CSI2RX_PAD_MAX];
    > + struct v4l2_mbus_framefmt fmt;
    >
    > /* Remote source */
    > struct v4l2_subdev *source_subdev;
    > int source_pad;
    > };
    >
    > +static const struct csi2rx_fmt formats[] = {
    > + {
    > + .code = MEDIA_BUS_FMT_YUYV8_1X16,
    > + .bpp = 16,
    > + },
    > + {
    > + .code = MEDIA_BUS_FMT_UYVY8_1X16,
    > + .bpp = 16,
    > + },
    > + {
    > + .code = MEDIA_BUS_FMT_YVYU8_1X16,
    > + .bpp = 16,
    > + },
    > + {
    > + .code = MEDIA_BUS_FMT_VYUY8_1X16,
    > + .bpp = 16,
    > + },
    > +};
    > +
    > +static const struct csi2rx_fmt *csi2rx_get_fmt_by_code(u32 code)
    > +{
    > + unsigned int i;
    > +
    > + for (i = 0; i < ARRAY_SIZE(formats); i++)
    > + if (formats[i].code == code)
    > + return &formats[i];
    > +
    > + return NULL;
    > +}
    > +
    > static inline
    > struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev)
    > {
    > @@ -236,12 +272,103 @@ static int csi2rx_s_stream(struct v4l2_subdev *subdev, int enable)
    > return ret;
    > }
    >
    > +static struct v4l2_mbus_framefmt *
    > +csi2rx_get_pad_format(struct csi2rx_priv *csi2rx,
    > + struct v4l2_subdev_state *state,
    > + unsigned int pad, u32 which)
    > +{
    > + switch (which) {
    > + case V4L2_SUBDEV_FORMAT_TRY:
    > + return v4l2_subdev_get_try_format(&csi2rx->subdev, state, pad);
    > + case V4L2_SUBDEV_FORMAT_ACTIVE:
    > + return &csi2rx->fmt;
    > + default:
    > + return NULL;
    > + }
    > +}
    > +
    > +static int csi2rx_get_fmt(struct v4l2_subdev *subdev,
    > + struct v4l2_subdev_state *state,
    > + struct v4l2_subdev_format *format)
    > +{
    > + struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
    > + struct v4l2_mbus_framefmt *framefmt;
    > +
    > + framefmt = csi2rx_get_pad_format(csi2rx, state, format->pad,
    > + format->which);
    > + if (!framefmt)
    > + return -EINVAL;
    > +
    > + mutex_lock(&csi2rx->lock);
    > + format->format = *framefmt;
    > + mutex_unlock(&csi2rx->lock);
    > +
    > + return 0;
    > +}
    > +
    > +static int csi2rx_set_fmt(struct v4l2_subdev *subdev,
    > + struct v4l2_subdev_state *state,
    > + struct v4l2_subdev_format *format)
    > +{
    > + struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
    > + struct v4l2_mbus_framefmt *framefmt;
    > +
    > + /* No transcoding, source and sink formats must match. */
    > + if (format->pad != CSI2RX_PAD_SINK)
    > + return csi2rx_get_fmt(subdev, state, format);
    > +
    > + if (!csi2rx_get_fmt_by_code(format->format.code))
    > + format->format.code = formats[0].code;
    > +
    > + format->format.field = V4L2_FIELD_NONE;
    > +
    > + framefmt = csi2rx_get_pad_format(csi2rx, state, format->pad,
    > + format->which);
    > + if (!framefmt)
    > + return -EINVAL;
    > +
    > + mutex_lock(&csi2rx->lock);
    > + *framefmt = format->format;
    > + mutex_unlock(&csi2rx->lock);
    > +
    > + return 0;
    > +}
    > +
    > +static int csi2rx_init_cfg(struct v4l2_subdev *subdev,
    > + struct v4l2_subdev_state *state)
    > +{
    > + struct v4l2_subdev_format format = {
    > + .which = state ? V4L2_SUBDEV_FORMAT_TRY
    > + : V4L2_SUBDEV_FORMAT_ACTIVE,
    > + .pad = CSI2RX_PAD_SINK,
    > + .format = {
    > + .width = 640,
    > + .height = 480,
    > + .code = MEDIA_BUS_FMT_UYVY8_1X16,
    > + .field = V4L2_FIELD_NONE,
    > + .colorspace = V4L2_COLORSPACE_SRGB,
    > + .ycbcr_enc = V4L2_YCBCR_ENC_601,
    > + .quantization = V4L2_QUANTIZATION_LIM_RANGE,
    > + .xfer_func = V4L2_XFER_FUNC_SRGB,
    > + },
    > + };
    > +
    > + return csi2rx_set_fmt(subdev, state, &format);
    > +}
    > +
    > +static const struct v4l2_subdev_pad_ops csi2rx_pad_ops = {
    > + .get_fmt = csi2rx_get_fmt,
    > + .set_fmt = csi2rx_set_fmt,
    > + .init_cfg = csi2rx_init_cfg,
    > +};
    > +
    > static const struct v4l2_subdev_video_ops csi2rx_video_ops = {
    > .s_stream = csi2rx_s_stream,
    > };
    >
    > static const struct v4l2_subdev_ops csi2rx_subdev_ops = {
    > .video = &csi2rx_video_ops,
    > + .pad = &csi2rx_pad_ops,
    > };
    >
    > static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
    > @@ -457,6 +584,10 @@ static int csi2rx_probe(struct platform_device *pdev)
    > if (ret)
    > goto err_cleanup;
    >
    > + ret = csi2rx_init_cfg(&csi2rx->subdev, NULL);
    > + if (ret)
    > + goto err_cleanup;
    > +
    > ret = v4l2_async_register_subdev(&csi2rx->subdev);
    > if (ret < 0)
    > goto err_cleanup;

    --
    Regards,

    Laurent Pinchart

    \
     
     \ /
      Last update: 2022-01-23 16:15    [W:3.893 / U:0.704 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site