mbox series

[0/2] media: i2c: adv7604: Fix handling of video adjustments

Message ID 20230210225622.24411-1-laurent.pinchart+renesas@ideasonboard.com
Headers show
Series media: i2c: adv7604: Fix handling of video adjustments | expand

Message

Laurent Pinchart Feb. 10, 2023, 10:56 p.m. UTC
Hello,

This small series fixes two issues with video adjustments (brightness,
contrast, saturation and hue) in the adv7604 driver. Patch 1/2 makes
those controls effective (they currently have no effect), and patch 2/2
fixes the range of the hue control.

I have successfully tested the series with an ADV7612. The ADV7604 and
ADV7611 documentation of the hue control differs from the ADV7612, but I
believe that's because earlier documentation was incorrect. It could
still be useful to test the series with an ADV7604 or ADV7611.

In patch 2/2 I've decided to represent the hue value as an unsigned
8-bit integer, mapping to the [0°, 360°[ range. Using a signed value
would map to the [-180°, 180°[ range instead, without making any other
difference (and without requiring any modification to the patch other
than changing the range). I don't have a strong preference between the
two options, and I'm pretty sure we can freely pick one without any fear
of an impact on existing userspace applications as patch 1/2 shows that
the hue control has currently no effect.

Laurent Pinchart (2):
  media: i2c: adv7604: Enable video adjustment
  media: i2c: adv7604: Fix range of hue control

 drivers/media/i2c/adv7604.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Hans Verkuil Feb. 15, 2023, 12:49 p.m. UTC | #1
On 10/02/2023 23:56, Laurent Pinchart wrote:
> The video adjustments (contrast, brightness, saturation and hue) are
> ignored by default by the device when the VID_ADJ_EN bit is clear. The
> corresponding V4L2 controls exposed by the drivers have thus no effect.
> Fix this by setting the VID_ADJ_EN bit.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/i2c/adv7604.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 9d218962d7c8..3af0e67f9edb 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1805,6 +1805,9 @@ static void select_input(struct v4l2_subdev *sd)
>  		v4l2_dbg(2, debug, sd, "%s: Unknown port %d selected\n",
>  				__func__, state->selected_input);
>  	}
> +
> +	/* Enable video adjustment (contrast, saturation, brightness and hue) */
> +	cp_write_clr_set(sd, 0x3e, 0x80, 0x80);
>  }
>  
>  static int adv76xx_s_routing(struct v4l2_subdev *sd,

Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>