Message ID | 20201122163637.3590465-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
---|---|
Headers | show |
Series | adv748x: Add support for s2ram | expand |
On 22.11.2020 19:36, Niklas Söderlund wrote: > This series enables usage of the ADV748x after the system have been > suspended to ram. During s2ram the ADV748x may be powered down and thus > lose its configuration from probe time. The configuration contains > among other things the i2c slave address mappings for the different > blocks inside the ADV748x. If this is lost the hardware listens to the > "wrong" i2c addresses and becomes inaccessible. > > Example trying to read the analog standard before and after s2ram with > and without this this series. > > Without this series, > > # subdev=$(grep -l "adv748x 4-0070 afe" /sys/class/video4linux/*/name | sed 's#.*video4linux\(.*\)/name#/dev\1#g') > # v4l2-ctl --get-detected-standard -d $subdev > Video Standard = 0x000000ff > PAL-B/B1/G/H/I/D/D1/K > # echo on > /sys/bus/i2c/drivers/bd9571mwv/*/bd9571mwv-regulator*/backup_mode > ** flipp SW23 off ** flip? > # echo mem > /sys/power/state > ** flipp SW23 on ** flip? > # v4l2-ctl --get-detected-standard -d $subdev > [ 502.753723] adv748x 4-0070: error reading 63, 02 > [ 502.866437] adv748x 4-0070: error reading 63, 02 > VIDIOC_QUERYSTD: failed: No such device or address > > With this series, > > # subdev=$(grep -l "adv748x 4-0070 afe" /sys/class/video4linux/*/name | sed 's#.*video4linux\(.*\)/name#/dev\1#g') > # v4l2-ctl --get-detected-standard -d $subdev > Video Standard = 0x000000ff > PAL-B/B1/G/H/I/D/D1/K > # echo on > /sys/bus/i2c/drivers/bd9571mwv/*/bd9571mwv-regulator*/backup_mode > ** flipp SW23 off ** > # echo mem > /sys/power/state > ** flipp SW23 on ** Here as well... > # v4l2-ctl --get-detected-standard -d $subdev > Video Standard = 0x000000ff > PAL-B/B1/G/H/I/D/D1/K [...] MBR, Sergei
Hi Niklas, On 22/11/2020 16:36, Niklas Söderlund wrote: > Hello, > > This series enables usage of the ADV748x after the system have been > suspended to ram. During s2ram the ADV748x may be powered down and thus > lose its configuration from probe time. The configuration contains > among other things the i2c slave address mappings for the different > blocks inside the ADV748x. If this is lost the hardware listens to the > "wrong" i2c addresses and becomes inaccessible. > > Example trying to read the analog standard before and after s2ram with > and without this this series. > Should we be considering runtime_pm for this instead? -- Kieran > Without this series, > > # subdev=$(grep -l "adv748x 4-0070 afe" /sys/class/video4linux/*/name | sed 's#.*video4linux\(.*\)/name#/dev\1#g') > # v4l2-ctl --get-detected-standard -d $subdev > Video Standard = 0x000000ff > PAL-B/B1/G/H/I/D/D1/K > # echo on > /sys/bus/i2c/drivers/bd9571mwv/*/bd9571mwv-regulator*/backup_mode > ** flipp SW23 off ** > # echo mem > /sys/power/state > ** flipp SW23 on ** > # v4l2-ctl --get-detected-standard -d $subdev > [ 502.753723] adv748x 4-0070: error reading 63, 02 > [ 502.866437] adv748x 4-0070: error reading 63, 02 > VIDIOC_QUERYSTD: failed: No such device or address > > With this series, > > # subdev=$(grep -l "adv748x 4-0070 afe" /sys/class/video4linux/*/name | sed 's#.*video4linux\(.*\)/name#/dev\1#g') > # v4l2-ctl --get-detected-standard -d $subdev > Video Standard = 0x000000ff > PAL-B/B1/G/H/I/D/D1/K > # echo on > /sys/bus/i2c/drivers/bd9571mwv/*/bd9571mwv-regulator*/backup_mode > ** flipp SW23 off ** > # echo mem > /sys/power/state > ** flipp SW23 on ** > # v4l2-ctl --get-detected-standard -d $subdev > Video Standard = 0x000000ff > PAL-B/B1/G/H/I/D/D1/K > > Also any streaming while the system is suspended to ram fails to resume > without this series due to the issue demonstrated above. This series is > tested on R-Car M3-N on-top of latest media-tree. > > Niklas Söderlund (3): > adv748x: afe: Select input port when device is reset > adv748x: csi2: Set virtual channel when device is reset > adv748x: Configure device when resuming from sleep > > drivers/media/i2c/adv748x/adv748x-afe.c | 6 +---- > drivers/media/i2c/adv748x/adv748x-core.c | 29 ++++++++++++++++++++++-- > drivers/media/i2c/adv748x/adv748x-csi2.c | 6 +---- > drivers/media/i2c/adv748x/adv748x.h | 2 ++ > 4 files changed, 31 insertions(+), 12 deletions(-) >
Hi Kieran, On 2020-11-25 13:09:39 +0000, Kieran Bingham wrote: > Hi Niklas, > > On 22/11/2020 16:36, Niklas Söderlund wrote: > > Hello, > > > > This series enables usage of the ADV748x after the system have been > > suspended to ram. During s2ram the ADV748x may be powered down and thus > > lose its configuration from probe time. The configuration contains > > among other things the i2c slave address mappings for the different > > blocks inside the ADV748x. If this is lost the hardware listens to the > > "wrong" i2c addresses and becomes inaccessible. > > > > Example trying to read the analog standard before and after s2ram with > > and without this this series. > > > > Should we be considering runtime_pm for this instead? I don't think so, why do you think we should? I opted for this solution because we need fine grain control of when the registers are restored when resuming from s2ram. If they are not restored before (in my case) the VIN driver is resumed and it was streaming at suspend time it will fail as the i2c address map is wrong at this time. For this reason the registers are restored in the early resume callback. Second I'm unsure how we could properly test such a solution as I don't think we can powerdown the ADV7482 without also s2ram the whole SoC on our test platforms as it's power is not controllable by the SoC. For example it's not powered down in s2idel. > > -- > Kieran > > > > Without this series, > > > > # subdev=$(grep -l "adv748x 4-0070 afe" /sys/class/video4linux/*/name | sed 's#.*video4linux\(.*\)/name#/dev\1#g') > > # v4l2-ctl --get-detected-standard -d $subdev > > Video Standard = 0x000000ff > > PAL-B/B1/G/H/I/D/D1/K > > # echo on > /sys/bus/i2c/drivers/bd9571mwv/*/bd9571mwv-regulator*/backup_mode > > ** flipp SW23 off ** > > # echo mem > /sys/power/state > > ** flipp SW23 on ** > > # v4l2-ctl --get-detected-standard -d $subdev > > [ 502.753723] adv748x 4-0070: error reading 63, 02 > > [ 502.866437] adv748x 4-0070: error reading 63, 02 > > VIDIOC_QUERYSTD: failed: No such device or address > > > > With this series, > > > > # subdev=$(grep -l "adv748x 4-0070 afe" /sys/class/video4linux/*/name | sed 's#.*video4linux\(.*\)/name#/dev\1#g') > > # v4l2-ctl --get-detected-standard -d $subdev > > Video Standard = 0x000000ff > > PAL-B/B1/G/H/I/D/D1/K > > # echo on > /sys/bus/i2c/drivers/bd9571mwv/*/bd9571mwv-regulator*/backup_mode > > ** flipp SW23 off ** > > # echo mem > /sys/power/state > > ** flipp SW23 on ** > > # v4l2-ctl --get-detected-standard -d $subdev > > Video Standard = 0x000000ff > > PAL-B/B1/G/H/I/D/D1/K > > > > Also any streaming while the system is suspended to ram fails to resume > > without this series due to the issue demonstrated above. This series is > > tested on R-Car M3-N on-top of latest media-tree. > > > > Niklas Söderlund (3): > > adv748x: afe: Select input port when device is reset > > adv748x: csi2: Set virtual channel when device is reset > > adv748x: Configure device when resuming from sleep > > > > drivers/media/i2c/adv748x/adv748x-afe.c | 6 +---- > > drivers/media/i2c/adv748x/adv748x-core.c | 29 ++++++++++++++++++++++-- > > drivers/media/i2c/adv748x/adv748x-csi2.c | 6 +---- > > drivers/media/i2c/adv748x/adv748x.h | 2 ++ > > 4 files changed, 31 insertions(+), 12 deletions(-) > > >
Hi Niklas, Kieran, On Wed, Nov 25, 2020 at 2:40 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > On 2020-11-25 13:09:39 +0000, Kieran Bingham wrote: > > On 22/11/2020 16:36, Niklas Söderlund wrote: > > > This series enables usage of the ADV748x after the system have been > > > suspended to ram. During s2ram the ADV748x may be powered down and thus > > > lose its configuration from probe time. The configuration contains > > > among other things the i2c slave address mappings for the different > > > blocks inside the ADV748x. If this is lost the hardware listens to the > > > "wrong" i2c addresses and becomes inaccessible. > > > > > > Example trying to read the analog standard before and after s2ram with > > > and without this this series. > > > > Should we be considering runtime_pm for this instead? > > I don't think so, why do you think we should? > > I opted for this solution because we need fine grain control of when the > registers are restored when resuming from s2ram. If they are not > restored before (in my case) the VIN driver is resumed and it was > streaming at suspend time it will fail as the i2c address map is wrong > at this time. For this reason the registers are restored in the early > resume callback. > > Second I'm unsure how we could properly test such a solution as I don't > think we can powerdown the ADV7482 without also s2ram the whole SoC on > our test platforms as it's power is not controllable by the SoC. For > example it's not powered down in s2idel. Also, there is no need to reset the ADV7482 every time it is runtime-resumed. In fact such needless reset may impact performance, as runtime-resume happens much more frequently than system resume during the system's lifetime. Gr{oetje,eeting}s, Geert