Re: [PATCH v2] media: i2c: adv748x: Fix pixel rate values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Niklas, Laurent,

Thanks for the updated patch and detailed investigation to get this right.

On 11/05/18 15:04, Niklas Söderlund wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> 
> 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
> 14.3180180 MHz.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> [Niklas: Update AFE fixed pixel rate]
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>

Does this still require changes to the CSI2 driver to be synchronised?
(or does it not matter as the CSI2 isn't upstream yet)...

--
Kieran


> ---
> 
> * Changes since v1
> - Update AFE fixed pixel rate.
> ---
>  drivers/media/i2c/adv748x/adv748x-afe.c  | 12 ++++++------
>  drivers/media/i2c/adv748x/adv748x-hdmi.c |  8 +-------
>  2 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c b/drivers/media/i2c/adv748x/adv748x-afe.c
> index 61514bae7e5ceb42..edd25e895e5dec3c 100644
> --- a/drivers/media/i2c/adv748x/adv748x-afe.c
> +++ b/drivers/media/i2c/adv748x/adv748x-afe.c
> @@ -321,17 +321,17 @@ 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 ADC sampling frequency is twice the externally supplied
> +	 * clock whose frequency is required to be 28.63636 MHz. It oversamples
> +	 * with a factor of 4 resulting in a pixel rate of 14.3180180 MHz.
> +	 */
> +	return adv748x_csi2_set_pixelrate(tx, 14318180);
>  }
>  
>  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 10d229a4f08868f7..aecc2a84dfecbec8 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,
> 

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux