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@xxxxxxxxxxxx> --- drivers/media/i2c/adv7180.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c index 3c7c896a21a4..eab3ae2331fd 100644 --- 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; @@ -900,6 +897,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; @@ -940,6 +944,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; } @@ -1471,10 +1483,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); @@ -1582,9 +1591,12 @@ static int adv7180_resume(struct device *dev) if (ret < 0) return ret; - ret = adv7180_set_power(state, state->powered); - if (ret) - return ret; + /* If we where streaming when suspending, start decoder. */ + if (state->streaming) { + ret = adv7180_set_power(state, true); + if (ret) + return ret; + } return 0; } -- 2.50.0