Hi Sakari,
On Saturday, 11 November 2017 00:32:27 EET Sakari Ailus wrote:
> On Fri, Nov 10, 2017 at 02:31:37PM +0100, Niklas Söderlund wrote:
> > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> > supports the rcar-vin driver on R-Car Gen3 SoCs where separate CSI-2
> > hardware blocks are connected between the video sources and the video
> > grabbers (VIN).
> >
> > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> >
> > drivers/media/platform/rcar-vin/Kconfig | 12 +
> > drivers/media/platform/rcar-vin/Makefile | 1 +
> > drivers/media/platform/rcar-vin/rcar-csi2.c | 934 +++++++++++++++++++++++
> > 3 files changed, 947 insertions(+)
> > create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
[snip]
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c new file mode 100644
> > index 0000000000000000..27d09da191a09b39
> > --- /dev/null
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
[snip]
> > +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv,
> > + struct v4l2_subdev *source,
> > + struct v4l2_mbus_framefmt *mf,
> > + u32 *phypll)
> > +{
> > + const struct phypll_hsfreqrange *hsfreq;
> > + const struct rcar_csi2_format *format;
> > + struct v4l2_ctrl *ctrl;
> > + u64 mbps;
> > +
> > + ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
>
> How about LINK_FREQ instead? It'd be more straightforward to calculate
> this. Up to you.
This probably needs to be documented, but I think the official API is
V4L2_CID_PIXEL_RATE. The link frequency control is not mandatory.
> > + if (!ctrl) {
> > + dev_err(priv->dev, "no link freq control in subdev %s\n",
> > + source->name);
> > + return -EINVAL;
> > + }
> > +
> > + format = rcar_csi2_code_to_fmt(mf->code);
> > + if (!format) {
> > + dev_err(priv->dev, "Unknown format: %d\n", mf->code);
> > + return -EINVAL;
> > + }
> > +
> > + mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> > + do_div(mbps, priv->lanes * 1000000);
> > +
> > + for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> > + if (hsfreq->mbps >= mbps)
> > + break;
> > +
> > + if (!hsfreq->mbps) {
> > + dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> > + return -ERANGE;
> > + }
> > +
> > + dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n",
mbps,
> > + hsfreq->mbps);
> > +
> > + *phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> > +
> > + return 0;
> > +}
[snip]
> > +static int rcar_csi2_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > + struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +
> > + if (on)
> > + pm_runtime_get_sync(priv->dev);
> > + else
> > + pm_runtime_put(priv->dev);
>
> The pipeline you have is already rather complex. Would it be an option to
> power the hardware on when streaming is started? The smiapp driver does
> this, without even implementing the s_power callback. (You'd still need to
> call it on the image source, as long as we have drivers that need it.)
We've briefly discussed this before, and I agree that pipeline power
management needs to be redesigned, but we still haven't agreed on a proper
architecture for that. Feel free to propose an RFC :-) In the meantime I
wouldn't try to enforce one specific model.
> > +
> > + return 0;
> > +}
[snip]
--
Regards,
Laurent Pinchart
