Hi Laurent,
On 21/04/18 13:44, Laurent Pinchart wrote:
> The pixel rate, as reported by the V4L2_CID_PIXEL_RATE control, must
> include both horizontal and vertical blanking. Both the AFE and HDMI
> receiver program it incorrectly:
>
> - The HDMI receiver goes to the trouble of removing blanking to compute
> the rate of active pixels. This is easy to fix by removing the
> computation and returning the incoming pixel clock rate directly.
>
> - The AFE performs similar calculation, while it should simply return
> the fixed pixel rate for analog sources, mandated by the ADV748x to be
> 28.63636 MHz.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
This looks quite reasonable, and simplifies the code. Win win.
I presume this will have implications on the pixel receiver side (VIN in our
case)... are there changes required there, or was it 'just wrong' here.
Either way,
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/adv748x/adv748x-afe.c | 11 +++++------
> drivers/media/i2c/adv748x/adv748x-hdmi.c | 8 +-------
> 2 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c b/drivers/media/i2c/adv748x/adv748x-afe.c
> index 61514bae7e5c..3e18d5ae813b 100644
> --- a/drivers/media/i2c/adv748x/adv748x-afe.c
> +++ b/drivers/media/i2c/adv748x/adv748x-afe.c
> @@ -321,17 +321,16 @@ static const struct v4l2_subdev_video_ops adv748x_afe_video_ops = {
> static int adv748x_afe_propagate_pixelrate(struct adv748x_afe *afe)
> {
> struct v4l2_subdev *tx;
> - unsigned int width, height, fps;
>
> tx = adv748x_get_remote_sd(&afe->pads[ADV748X_AFE_SOURCE]);
> if (!tx)
> return -ENOLINK;
>
> - width = 720;
> - height = afe->curr_norm & V4L2_STD_525_60 ? 480 : 576;
> - fps = afe->curr_norm & V4L2_STD_525_60 ? 30 : 25;
> -
> - return adv748x_csi2_set_pixelrate(tx, width * height * fps);
> + /*
> + * The ADV748x samples analog video signals using an externally supplied
> + * clock whose frequency is required to be 28.63636 MHz.
> + */
> + return adv748x_csi2_set_pixelrate(tx, 28636360);
> }
>
> static int adv748x_afe_enum_mbus_code(struct v4l2_subdev *sd,
> diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c
> index 10d229a4f088..aecc2a84dfec 100644
> --- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
> +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
> @@ -402,8 +402,6 @@ static int adv748x_hdmi_propagate_pixelrate(struct adv748x_hdmi *hdmi)
> {
> struct v4l2_subdev *tx;
> struct v4l2_dv_timings timings;
> - struct v4l2_bt_timings *bt = &timings.bt;
> - unsigned int fps;
>
> tx = adv748x_get_remote_sd(&hdmi->pads[ADV748X_HDMI_SOURCE]);
> if (!tx)
> @@ -411,11 +409,7 @@ static int adv748x_hdmi_propagate_pixelrate(struct adv748x_hdmi *hdmi)
>
> adv748x_hdmi_query_dv_timings(&hdmi->sd, &timings);
>
> - fps = DIV_ROUND_CLOSEST_ULL(bt->pixelclock,
> - V4L2_DV_BT_FRAME_WIDTH(bt) *
> - V4L2_DV_BT_FRAME_HEIGHT(bt));
> -
> - return adv748x_csi2_set_pixelrate(tx, bt->width * bt->height * fps);
> + return adv748x_csi2_set_pixelrate(tx, timings.bt.pixelclock);
> }
>
> static int adv748x_hdmi_enum_mbus_code(struct v4l2_subdev *sd,
>
