Hi Frederico!
On Thu April 12 2012 18:39:36 Federico Vaga wrote:
> Video user controls such as brightness, contrast, saturation, and
> hue are now handled.
I just saw this patch series being merged, and I wonder if you could make a
follow-up patch for 3.6 where you implement the control framework for adv7180
and sta2x11. See Documentation/video4linux/v4l2-controls.txt and the many
drivers that use it now.
I missed this patch series or I would have requested it at the time. Unfortunately
I had to deal with other things in March and April so I was for the most part
absent from the list during those months.
My goal is to convert all subdevice drivers like adv7180 and as many bridge and
platform drivers as is possible to the control framework. I try to prevent new
drivers from getting in that do not use that framework to prevent double work,
but I didn't catch this one.
If you could do that work, then that would be much appreciated. You have the
hardware, after all, so that makes it easier for you.
Regards,
Hans
>
> Signed-off-by: Federico Vaga <federico.vaga@xxxxxxxxx>
> Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@xxxxxx>
> Cc: Alan Cox <alan@xxxxxxxxxxxxxxx>
> ---
> drivers/media/video/adv7180.c | 417 ++++++++++++++++++++++++++++++++++-------
> 1 files changed, 350 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/media/video/adv7180.c b/drivers/media/video/adv7180.c
> index b8b6c4b..174bffa 100644
> --- a/drivers/media/video/adv7180.c
> +++ b/drivers/media/video/adv7180.c
> @@ -48,6 +48,7 @@
> #define ADV7180_INPUT_CONTROL_PAL_COMB_N_PED 0xd0
> #define ADV7180_INPUT_CONTROL_PAL_SECAM 0xe0
> #define ADV7180_INPUT_CONTROL_PAL_SECAM_PED 0xf0
> +#define ADV7180_INPUT_CONTROL_INSEL_MASK 0x0f
>
> #define ADV7180_EXTENDED_OUTPUT_CONTROL_REG 0x04
> #define ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS 0xC5
> @@ -55,9 +56,29 @@
> #define ADV7180_AUTODETECT_ENABLE_REG 0x07
> #define ADV7180_AUTODETECT_DEFAULT 0x7f
>
> +#define ADV7180_CON_REG 0x08 /*Unsigned */
> +#define CON_REG_MIN 0
> +#define CON_REG_DEF 128
> +#define CON_REG_MAX 255
> +
> +#define ADV7180_BRI_REG 0x0a /*Signed */
> +#define BRI_REG_MIN -128
> +#define BRI_REG_DEF 0
> +#define BRI_REG_MAX 127
> +
> +#define ADV7180_HUE_REG 0x0b /*Signed, inverted */
> +#define HUE_REG_MIN -127
> +#define HUE_REG_DEF 0
> +#define HUE_REG_MAX 128
> +
> #define ADV7180_ADI_CTRL_REG 0x0e
> #define ADV7180_ADI_CTRL_IRQ_SPACE 0x20
>
> +#define ADV7180_PWR_MAN_REG 0x0f
> +#define ADV7180_PWR_MAN_ON 0x04
> +#define ADV7180_PWR_MAN_OFF 0x24
> +#define ADV7180_PWR_MAN_RES 0x80
> +
> #define ADV7180_STATUS1_REG 0x10
> #define ADV7180_STATUS1_IN_LOCK 0x01
> #define ADV7180_STATUS1_AUTOD_MASK 0x70
> @@ -78,6 +99,12 @@
> #define ADV7180_ICONF1_PSYNC_ONLY 0x10
> #define ADV7180_ICONF1_ACTIVE_TO_CLR 0xC0
>
> +#define ADV7180_SD_SAT_CB_REG 0xe3 /*Unsigned */
> +#define ADV7180_SD_SAT_CR_REG 0xe4 /*Unsigned */
> +#define SAT_REG_MIN 0
> +#define SAT_REG_DEF 128
> +#define SAT_REG_MAX 255
> +
> #define ADV7180_IRQ1_LOCK 0x01
> #define ADV7180_IRQ1_UNLOCK 0x02
> #define ADV7180_ISR1_ADI 0x42
> @@ -90,6 +117,9 @@
> #define ADV7180_IMR3_ADI 0x4C
> #define ADV7180_IMR4_ADI 0x50
>
> +#define ADV7180_NTSC_V_BIT_END_REG 0xE6
> +#define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND 0x4F
> +
> struct adv7180_state {
> struct v4l2_subdev sd;
> struct work_struct work;
> @@ -97,6 +127,11 @@ struct adv7180_state {
> int irq;
> v4l2_std_id curr_norm;
> bool autodetect;
> + s8 brightness;
> + s16 hue;
> + u8 contrast;
> + u8 saturation;
> + u8 input;
> };
>
> static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
> @@ -155,7 +190,7 @@ static u32 adv7180_status_to_v4l2(u8 status1)
> }
>
> static int __adv7180_status(struct i2c_client *client, u32 *status,
> - v4l2_std_id *std)
> + v4l2_std_id *std)
> {
> int status1 = i2c_smbus_read_byte_data(client, ADV7180_STATUS1_REG);
>
> @@ -192,6 +227,36 @@ static int adv7180_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
> return err;
> }
>
> +static int adv7180_s_routing(struct v4l2_subdev *sd, u32 input,
> + u32 output, u32 config)
> +{
> + struct adv7180_state *state = to_state(sd);
> + int ret = mutex_lock_interruptible(&state->mutex);
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> + if (ret)
> + return ret;
> +
> + /*We cannot discriminate between LQFP and 40-pin LFCSP, so accept
> + * all inputs and let the card driver take care of validation
> + */
> + if ((input & ADV7180_INPUT_CONTROL_INSEL_MASK) != input)
> + goto out;
> +
> + ret = i2c_smbus_read_byte_data(client, ADV7180_INPUT_CONTROL_REG);
> +
> + if (ret < 0)
> + goto out;
> +
> + ret &= ~ADV7180_INPUT_CONTROL_INSEL_MASK;
> + ret = i2c_smbus_write_byte_data(client,
> + ADV7180_INPUT_CONTROL_REG, ret | input);
> + state->input = input;
> +out:
> + mutex_unlock(&state->mutex);
> + return ret;
> +}
> +
> static int adv7180_g_input_status(struct v4l2_subdev *sd, u32 *status)
> {
> struct adv7180_state *state = to_state(sd);
> @@ -205,7 +270,7 @@ static int adv7180_g_input_status(struct v4l2_subdev *sd, u32 *status)
> }
>
> static int adv7180_g_chip_ident(struct v4l2_subdev *sd,
> - struct v4l2_dbg_chip_ident *chip)
> + struct v4l2_dbg_chip_ident *chip)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> @@ -222,9 +287,10 @@ static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
>
> /* all standards -> autodetect */
> if (std == V4L2_STD_ALL) {
> - ret = i2c_smbus_write_byte_data(client,
> - ADV7180_INPUT_CONTROL_REG,
> - ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM);
> + ret =
> + i2c_smbus_write_byte_data(client, ADV7180_INPUT_CONTROL_REG,
> + ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
> + | state->input);
> if (ret < 0)
> goto out;
>
> @@ -236,7 +302,8 @@ static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
> goto out;
>
> ret = i2c_smbus_write_byte_data(client,
> - ADV7180_INPUT_CONTROL_REG, ret);
> + ADV7180_INPUT_CONTROL_REG,
> + ret | state->input);
> if (ret < 0)
> goto out;
>
> @@ -249,14 +316,138 @@ out:
> return ret;
> }
>
> +static int adv7180_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl *qc)
> +{
> + switch (qc->id) {
> + case V4L2_CID_BRIGHTNESS:
> + return v4l2_ctrl_query_fill(qc, BRI_REG_MIN, BRI_REG_MAX,
> + 1, BRI_REG_DEF);
> + case V4L2_CID_HUE:
> + return v4l2_ctrl_query_fill(qc, HUE_REG_MIN, HUE_REG_MAX,
> + 1, HUE_REG_DEF);
> + case V4L2_CID_CONTRAST:
> + return v4l2_ctrl_query_fill(qc, CON_REG_MIN, CON_REG_MAX,
> + 1, CON_REG_DEF);
> + case V4L2_CID_SATURATION:
> + return v4l2_ctrl_query_fill(qc, SAT_REG_MIN, SAT_REG_MAX,
> + 1, SAT_REG_DEF);
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int adv7180_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +{
> + struct adv7180_state *state = to_state(sd);
> + int ret = mutex_lock_interruptible(&state->mutex);
> + if (ret)
> + return ret;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_BRIGHTNESS:
> + ctrl->value = state->brightness;
> + break;
> + case V4L2_CID_HUE:
> + ctrl->value = state->hue;
> + break;
> + case V4L2_CID_CONTRAST:
> + ctrl->value = state->contrast;
> + break;
> + case V4L2_CID_SATURATION:
> + ctrl->value = state->saturation;
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + mutex_unlock(&state->mutex);
> + return ret;
> +}
> +
> +static int adv7180_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +{
> + struct adv7180_state *state = to_state(sd);
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> + int ret = mutex_lock_interruptible(&state->mutex);
> + if (ret)
> + return ret;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_BRIGHTNESS:
> + if ((ctrl->value > BRI_REG_MAX)
> + || (ctrl->value < BRI_REG_MIN)) {
> + ret = -ERANGE;
> + break;
> + }
> + state->brightness = ctrl->value;
> + ret = i2c_smbus_write_byte_data(client,
> + ADV7180_BRI_REG,
> + state->brightness);
> + break;
> + case V4L2_CID_HUE:
> + if ((ctrl->value > HUE_REG_MAX)
> + || (ctrl->value < HUE_REG_MIN)) {
> + ret = -ERANGE;
> + break;
> + }
> + state->hue = ctrl->value;
> + /*Hue is inverted according to HSL chart */
> + ret = i2c_smbus_write_byte_data(client,
> + ADV7180_HUE_REG, -state->hue);
> + break;
> + case V4L2_CID_CONTRAST:
> + if ((ctrl->value > CON_REG_MAX)
> + || (ctrl->value < CON_REG_MIN)) {
> + ret = -ERANGE;
> + break;
> + }
> + state->contrast = ctrl->value;
> + ret = i2c_smbus_write_byte_data(client,
> + ADV7180_CON_REG,
> + state->contrast);
> + break;
> + case V4L2_CID_SATURATION:
> + if ((ctrl->value > SAT_REG_MAX)
> + || (ctrl->value < SAT_REG_MIN)) {
> + ret = -ERANGE;
> + break;
> + }
> + /*
> + *This could be V4L2_CID_BLUE_BALANCE/V4L2_CID_RED_BALANCE
> + *Let's not confuse the user, everybody understands saturation
> + */
> + state->saturation = ctrl->value;
> + ret = i2c_smbus_write_byte_data(client,
> + ADV7180_SD_SAT_CB_REG,
> + state->saturation);
> + if (ret < 0)
> + break;
> + ret = i2c_smbus_write_byte_data(client,
> + ADV7180_SD_SAT_CR_REG,
> + state->saturation);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + mutex_unlock(&state->mutex);
> + return ret;
> +}
> +
> static const struct v4l2_subdev_video_ops adv7180_video_ops = {
> .querystd = adv7180_querystd,
> .g_input_status = adv7180_g_input_status,
> + .s_routing = adv7180_s_routing,
> };
>
> static const struct v4l2_subdev_core_ops adv7180_core_ops = {
> .g_chip_ident = adv7180_g_chip_ident,
> .s_std = adv7180_s_std,
> + .queryctrl = adv7180_queryctrl,
> + .g_ctrl = adv7180_g_ctrl,
> + .s_ctrl = adv7180_s_ctrl,
> };
>
> static const struct v4l2_subdev_ops adv7180_ops = {
> @@ -267,13 +458,13 @@ static const struct v4l2_subdev_ops adv7180_ops = {
> static void adv7180_work(struct work_struct *work)
> {
> struct adv7180_state *state = container_of(work, struct adv7180_state,
> - work);
> + work);
> struct i2c_client *client = v4l2_get_subdevdata(&state->sd);
> u8 isr3;
>
> mutex_lock(&state->mutex);
> i2c_smbus_write_byte_data(client, ADV7180_ADI_CTRL_REG,
> - ADV7180_ADI_CTRL_IRQ_SPACE);
> + ADV7180_ADI_CTRL_IRQ_SPACE);
> isr3 = i2c_smbus_read_byte_data(client, ADV7180_ISR3_ADI);
> /* clear */
> i2c_smbus_write_byte_data(client, ADV7180_ICR3_ADI, isr3);
> @@ -297,56 +488,51 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
> return IRQ_HANDLED;
> }
>
> -/*
> - * Generic i2c probe
> - * concerning the addresses: i2c wants 7 bit (without the r/w bit), so '>>1'
> - */
> -
> -static __devinit int adv7180_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> +static int init_device(struct i2c_client *client, struct adv7180_state *state)
> {
> - struct adv7180_state *state;
> - struct v4l2_subdev *sd;
> int ret;
>
> - /* Check if the adapter supports the needed features */
> - if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> - return -EIO;
> -
> - v4l_info(client, "chip found @ 0x%02x (%s)\n",
> - client->addr << 1, client->adapter->name);
> -
> - state = kzalloc(sizeof(struct adv7180_state), GFP_KERNEL);
> - if (state == NULL) {
> - ret = -ENOMEM;
> - goto err;
> - }
> -
> - state->irq = client->irq;
> - INIT_WORK(&state->work, adv7180_work);
> - mutex_init(&state->mutex);
> - state->autodetect = true;
> - sd = &state->sd;
> - v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
> -
> /* Initialize adv7180 */
> /* Enable autodetection */
> - ret = i2c_smbus_write_byte_data(client, ADV7180_INPUT_CONTROL_REG,
> - ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM);
> - if (ret < 0)
> - goto err_unreg_subdev;
> + if (state->autodetect) {
> + ret =
> + i2c_smbus_write_byte_data(client, ADV7180_INPUT_CONTROL_REG,
> + ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM
> + | state->input);
> + if (ret < 0)
> + return ret;
>
> - ret = i2c_smbus_write_byte_data(client, ADV7180_AUTODETECT_ENABLE_REG,
> - ADV7180_AUTODETECT_DEFAULT);
> - if (ret < 0)
> - goto err_unreg_subdev;
> + ret =
> + i2c_smbus_write_byte_data(client,
> + ADV7180_AUTODETECT_ENABLE_REG,
> + ADV7180_AUTODETECT_DEFAULT);
> + if (ret < 0)
> + return ret;
> + } else {
> + ret = v4l2_std_to_adv7180(state->curr_norm);
> + if (ret < 0)
> + return ret;
>
> + ret =
> + i2c_smbus_write_byte_data(client, ADV7180_INPUT_CONTROL_REG,
> + ret | state->input);
> + if (ret < 0)
> + return ret;
> +
> + }
> /* ITU-R BT.656-4 compatible */
> ret = i2c_smbus_write_byte_data(client,
> - ADV7180_EXTENDED_OUTPUT_CONTROL_REG,
> - ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS);
> + ADV7180_EXTENDED_OUTPUT_CONTROL_REG,
> + ADV7180_EXTENDED_OUTPUT_CONTROL_NTSCDIS);
> if (ret < 0)
> - goto err_unreg_subdev;
> + return ret;
> +
> + /* Manually set V bit end position in NTSC mode */
> + ret = i2c_smbus_write_byte_data(client,
> + ADV7180_NTSC_V_BIT_END_REG,
> + ADV7180_NTSC_V_BIT_END_MANUAL_NVEND);
> + if (ret < 0)
> + return ret;
>
> /* read current norm */
> __adv7180_status(client, NULL, &state->curr_norm);
> @@ -354,45 +540,109 @@ static __devinit int adv7180_probe(struct i2c_client *client,
> /* register for interrupts */
> if (state->irq > 0) {
> ret = request_irq(state->irq, adv7180_irq, 0, DRIVER_NAME,
> - state);
> + state);
> if (ret)
> - goto err_unreg_subdev;
> + return ret;
>
> ret = i2c_smbus_write_byte_data(client, ADV7180_ADI_CTRL_REG,
> - ADV7180_ADI_CTRL_IRQ_SPACE);
> + ADV7180_ADI_CTRL_IRQ_SPACE);
> if (ret < 0)
> - goto err_unreg_subdev;
> + return ret;
>
> /* config the Interrupt pin to be active low */
> ret = i2c_smbus_write_byte_data(client, ADV7180_ICONF1_ADI,
> - ADV7180_ICONF1_ACTIVE_LOW | ADV7180_ICONF1_PSYNC_ONLY);
> + ADV7180_ICONF1_ACTIVE_LOW |
> + ADV7180_ICONF1_PSYNC_ONLY);
> if (ret < 0)
> - goto err_unreg_subdev;
> + return ret;
>
> ret = i2c_smbus_write_byte_data(client, ADV7180_IMR1_ADI, 0);
> if (ret < 0)
> - goto err_unreg_subdev;
> + return ret;
>
> ret = i2c_smbus_write_byte_data(client, ADV7180_IMR2_ADI, 0);
> if (ret < 0)
> - goto err_unreg_subdev;
> + return ret;
>
> /* enable AD change interrupts interrupts */
> ret = i2c_smbus_write_byte_data(client, ADV7180_IMR3_ADI,
> - ADV7180_IRQ3_AD_CHANGE);
> + ADV7180_IRQ3_AD_CHANGE);
> if (ret < 0)
> - goto err_unreg_subdev;
> + return ret;
>
> ret = i2c_smbus_write_byte_data(client, ADV7180_IMR4_ADI, 0);
> if (ret < 0)
> - goto err_unreg_subdev;
> + return ret;
>
> ret = i2c_smbus_write_byte_data(client, ADV7180_ADI_CTRL_REG,
> - 0);
> + 0);
> if (ret < 0)
> - goto err_unreg_subdev;
> + return ret;
> }
>
> + /*Set default value for controls */
> + ret = i2c_smbus_write_byte_data(client, ADV7180_BRI_REG,
> + state->brightness);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ADV7180_HUE_REG, state->hue);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ADV7180_CON_REG,
> + state->contrast);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ADV7180_SD_SAT_CB_REG,
> + state->saturation);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ADV7180_SD_SAT_CR_REG,
> + state->saturation);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static __devinit int adv7180_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct adv7180_state *state;
> + struct v4l2_subdev *sd;
> + int ret;
> +
> + /* Check if the adapter supports the needed features */
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> + return -EIO;
> +
> + v4l_info(client, "chip found @ 0x%02x (%s)\n",
> + client->addr, client->adapter->name);
> +
> + state = kzalloc(sizeof(struct adv7180_state), GFP_KERNEL);
> + if (state == NULL) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + state->irq = client->irq;
> + INIT_WORK(&state->work, adv7180_work);
> + mutex_init(&state->mutex);
> + state->autodetect = true;
> + state->brightness = BRI_REG_DEF;
> + state->hue = HUE_REG_DEF;
> + state->contrast = CON_REG_DEF;
> + state->saturation = SAT_REG_DEF;
> + state->input = 0;
> + sd = &state->sd;
> + v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
> +
> + ret = init_device(client, state);
> + if (0 != ret)
> + goto err_unreg_subdev;
> return 0;
>
> err_unreg_subdev:
> @@ -432,16 +682,49 @@ static const struct i2c_device_id adv7180_id[] = {
> {},
> };
>
> +#ifdef CONFIG_PM
> +static int adv7180_suspend(struct i2c_client *client, pm_message_t state)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG,
> + ADV7180_PWR_MAN_OFF);
> + if (ret < 0)
> + return ret;
> + return 0;
> +}
> +
> +static int adv7180_resume(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct adv7180_state *state = to_state(sd);
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG,
> + ADV7180_PWR_MAN_ON);
> + if (ret < 0)
> + return ret;
> + ret = init_device(client, state);
> + if (ret < 0)
> + return ret;
> + return 0;
> +}
> +#endif
> +
> MODULE_DEVICE_TABLE(i2c, adv7180_id);
>
> static struct i2c_driver adv7180_driver = {
> .driver = {
> - .owner = THIS_MODULE,
> - .name = DRIVER_NAME,
> - },
> - .probe = adv7180_probe,
> - .remove = __devexit_p(adv7180_remove),
> - .id_table = adv7180_id,
> + .owner = THIS_MODULE,
> + .name = DRIVER_NAME,
> + },
> + .probe = adv7180_probe,
> + .remove = __devexit_p(adv7180_remove),
> +#ifdef CONFIG_PM
> + .suspend = adv7180_suspend,
> + .resume = adv7180_resume,
> +#endif
> + .id_table = adv7180_id,
> };
>
> module_i2c_driver(adv7180_driver);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
