media: adv7180: Power down decoder when configuring the device

Some variants of the chip (ADV7180) have it's decoder powered up after
reset, while others (ADV7280, ADV7281, ADV7282, ADV7283) have it powered
down.

This is tracked by the feature flag ADV7180_FLAG_RESET_POWERED. At probe
this flag is used to initialize the state variable powered which keeps
track of if the decoder is powered on, or off, for the resume callback.

This however misses that the decoder needs to be powered off for some
configuration of the device to take hold. So for devices where it's left
on (ADV7180) the format configuration at probe time have little effect.
This worked as the .set_fmt callback powers down the decoder, updates
the format, and powers back on the decoder.

Before moving all configuration to .s_stream this needs to be fixed.
Instead of tracking if the decoder is powered on or off, use the
flag to determine if needs to be powered down after a reset to do the
configuration.

To keep the behavior consistent with the currents implementation switch
the decoder back on for devices where this is the reset behavior. The
primary reason for this is that if not done the first 35+ frames or so
of the capture session is garbage.

To keep the support of starting the decoder when resuming from sleep on
devices where the reset behavior is to start with the decoder powered
off, use the state variable streaming. If it is set the decoder was
powered on when the system suspended so we know to start it again when
resuming.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
[hverkuil: fix typo in comment: where -> were]

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 8409ee9acc4f71283d4a9b1c15d2f7d825c23bf6..bb94b9a7b3c7fda7bdb61e30564cbb2e28e4d0f7 100644 (file)
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -214,7 +214,6 @@ struct adv7180_state {
        struct gpio_desc        *pwdn_gpio;
        struct gpio_desc        *rst_gpio;
        v4l2_std_id             curr_norm;
-       bool                    powered;
        bool                    streaming;
        u8                      input;
 
@@ -556,8 +555,6 @@ static int adv7180_s_power(struct v4l2_subdev *sd, int on)
                return ret;
 
        ret = adv7180_set_power(state, on);
-       if (ret == 0)
-               state->powered = on;
 
        mutex_unlock(&state->mutex);
        return ret;
@@ -887,6 +884,13 @@ static int init_device(struct adv7180_state *state)
        adv7180_write(state, ADV7180_REG_PWR_MAN, ADV7180_PWR_MAN_RES);
        usleep_range(5000, 10000);
 
+       /*
+        * If the devices decoder is power on after reset, power off so the
+        * device can be configured.
+        */
+       if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
+               adv7180_set_power(state, false);
+
        ret = state->chip_info->init(state);
        if (ret)
                return ret;
@@ -927,6 +931,14 @@ static int init_device(struct adv7180_state *state)
                        return ret;
        }
 
+       /*
+        * If the devices decoder is power on after reset, restore the power
+        * after configuration. This is to preserve the behavior of the driver,
+        * not doing this result in the first 35+ frames captured being garbage.
+        */
+       if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
+               adv7180_set_power(state, true);
+
        return 0;
 }
 
@@ -1457,10 +1469,7 @@ static int adv7180_probe(struct i2c_client *client)
        state->irq = client->irq;
        mutex_init(&state->mutex);
        state->curr_norm = V4L2_STD_NTSC;
-       if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
-               state->powered = true;
-       else
-               state->powered = false;
+
        state->input = 0;
        sd = &state->sd;
        v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
@@ -1568,11 +1577,12 @@ static int adv7180_resume(struct device *dev)
        if (ret < 0)
                return ret;
 
-       guard(mutex)(&state->mutex);
-
-       ret = adv7180_set_power(state, state->powered);
-       if (ret)
-               return ret;
+       /* If we were streaming when suspending, start decoder. */
+       if (state->streaming) {
+               ret = adv7180_set_power(state, true);
+               if (ret)
+                       return ret;
+       }
 
        return 0;
 }