Message ID | 20211117154009.261787-1-eugen.hristev@microchip.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | media: i2c: imx274: fix s_frame_interval runtime resume not requested | expand |
Hi Eugen, On Wed, Nov 17, 2021 at 05:40:09PM +0200, Eugen Hristev wrote: > pm_runtime_resume_and_get should be called when the s_frame_interval > is called. > > The driver will try to access device registers to configure VMAX, coarse > time and exposure. > > Currently if the runtime is not resumed, this fails: > # media-ctl -d /dev/media0 --set-v4l2 '"IMX274 1-001a":0[fmt:SRGGB10_1X10/3840x2 > 160@1/10]' > > IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 3840x2160, goodness 0 > IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1920x1080, goodness -3000 > IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x720, goodness -4000 > IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x540, goodness -4180 > IMX274 1-001a: __imx274_change_compose: selected 1x1 binning > IMX274 1-001a: imx274_set_frame_interval: input frame interval = 1 / 10 > IMX274 1-001a: imx274_read_mbreg : addr 0x300e, val=0x1 (2 bytes) > IMX274 1-001a: imx274_set_frame_interval : register SVR = 1 > IMX274 1-001a: imx274_read_mbreg : addr 0x30f6, val=0x6a8 (2 bytes) > IMX274 1-001a: imx274_set_frame_interval : register HMAX = 1704 > IMX274 1-001a: imx274_set_frame_length : input length = 2112 > IMX274 1-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 884 (3 bytes) > IMX274 1-001a: imx274_set_frame_length error = -121 > IMX274 1-001a: imx274_set_frame_interval error = -121 > Unable to setup formats: Remote I/O error (121) > > The device is not resumed thus the remote I/O error. > > Setting the frame interval works at streaming time, because > pm_runtime_resume_and_get is called at s_stream time before sensor setup. > The failure happens when only the s_frame_interval is called separately > independently on streaming time. > > Fixes: ad97bc37426c ("media: i2c: imx274: Add IMX274 power on and off sequence" > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com> > --- > drivers/media/i2c/imx274.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c > index e89ef35a71c5..6e63fdcc5e46 100644 > --- a/drivers/media/i2c/imx274.c > +++ b/drivers/media/i2c/imx274.c > @@ -1420,6 +1420,10 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, > int min, max, def; > int ret; > > + ret = pm_runtime_resume_and_get(&imx274->client->dev); > + if (ret < 0) > + return ret; > + > mutex_lock(&imx274->lock); > ret = imx274_set_frame_interval(imx274, fi->interval); > > @@ -1451,6 +1455,7 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, > > unlock: > mutex_unlock(&imx274->lock); > + pm_runtime_put(&imx274->client->dev); > > return ret; > } If the device is powered off in the end, could you instead not power it on in the first place? I.e. see how this works for the s_ctrl() callback.
On 11/17/21 6:11 PM, Sakari Ailus wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Eugen, > > On Wed, Nov 17, 2021 at 05:40:09PM +0200, Eugen Hristev wrote: >> pm_runtime_resume_and_get should be called when the s_frame_interval >> is called. >> >> The driver will try to access device registers to configure VMAX, coarse >> time and exposure. >> >> Currently if the runtime is not resumed, this fails: >> # media-ctl -d /dev/media0 --set-v4l2 '"IMX274 1-001a":0[fmt:SRGGB10_1X10/3840x2 >> 160@1/10]' >> >> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 3840x2160, goodness 0 >> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1920x1080, goodness -3000 >> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x720, goodness -4000 >> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x540, goodness -4180 >> IMX274 1-001a: __imx274_change_compose: selected 1x1 binning >> IMX274 1-001a: imx274_set_frame_interval: input frame interval = 1 / 10 >> IMX274 1-001a: imx274_read_mbreg : addr 0x300e, val=0x1 (2 bytes) >> IMX274 1-001a: imx274_set_frame_interval : register SVR = 1 >> IMX274 1-001a: imx274_read_mbreg : addr 0x30f6, val=0x6a8 (2 bytes) >> IMX274 1-001a: imx274_set_frame_interval : register HMAX = 1704 >> IMX274 1-001a: imx274_set_frame_length : input length = 2112 >> IMX274 1-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 884 (3 bytes) >> IMX274 1-001a: imx274_set_frame_length error = -121 >> IMX274 1-001a: imx274_set_frame_interval error = -121 >> Unable to setup formats: Remote I/O error (121) >> >> The device is not resumed thus the remote I/O error. >> >> Setting the frame interval works at streaming time, because >> pm_runtime_resume_and_get is called at s_stream time before sensor setup. >> The failure happens when only the s_frame_interval is called separately >> independently on streaming time. >> >> Fixes: ad97bc37426c ("media: i2c: imx274: Add IMX274 power on and off sequence" >> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com> >> --- >> drivers/media/i2c/imx274.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c >> index e89ef35a71c5..6e63fdcc5e46 100644 >> --- a/drivers/media/i2c/imx274.c >> +++ b/drivers/media/i2c/imx274.c >> @@ -1420,6 +1420,10 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, >> int min, max, def; >> int ret; >> >> + ret = pm_runtime_resume_and_get(&imx274->client->dev); >> + if (ret < 0) >> + return ret; >> + >> mutex_lock(&imx274->lock); >> ret = imx274_set_frame_interval(imx274, fi->interval); >> >> @@ -1451,6 +1455,7 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, >> >> unlock: >> mutex_unlock(&imx274->lock); >> + pm_runtime_put(&imx274->client->dev); >> >> return ret; >> } > > If the device is powered off in the end, could you instead not power it on > in the first place? I.e. see how this works for the s_ctrl() callback. Hi Sakari, I tried this initially, as in s_ctrl, if (!pm_runtime_get_if_in_use(&imx274->client->dev)) return 0; However, if the device is powered off, the s_frame_interval does not do anything (return 0), and the frame interval is not changed. Not even the internal structure frame_interval is updated (as this is updated after configuring the actual device). And in consequence media-ctl -p will still print the old frame interval. So either we power on the device to set everything, or, things have to be set in the software struct and written once streaming starts. I am in favor of the first option (hence the patch), to avoid having configuration that was requested but not written to the device itself. The second option would require some rework to move the software part before the hardware part, and to assume that the hardware part never fails in bounds or by other reason (or the software part would be no longer consistent) What do you think ? Eugen > > -- > Kind regards, > > Sakari Ailus >
Hi Eugen, On Wed, Nov 17, 2021 at 04:52:40PM +0000, Eugen.Hristev@microchip.com wrote: > On 11/17/21 6:11 PM, Sakari Ailus wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Hi Eugen, > > > > On Wed, Nov 17, 2021 at 05:40:09PM +0200, Eugen Hristev wrote: > >> pm_runtime_resume_and_get should be called when the s_frame_interval > >> is called. > >> > >> The driver will try to access device registers to configure VMAX, coarse > >> time and exposure. > >> > >> Currently if the runtime is not resumed, this fails: > >> # media-ctl -d /dev/media0 --set-v4l2 '"IMX274 1-001a":0[fmt:SRGGB10_1X10/3840x2 > >> 160@1/10]' > >> > >> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 3840x2160, goodness 0 > >> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1920x1080, goodness -3000 > >> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x720, goodness -4000 > >> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x540, goodness -4180 > >> IMX274 1-001a: __imx274_change_compose: selected 1x1 binning > >> IMX274 1-001a: imx274_set_frame_interval: input frame interval = 1 / 10 > >> IMX274 1-001a: imx274_read_mbreg : addr 0x300e, val=0x1 (2 bytes) > >> IMX274 1-001a: imx274_set_frame_interval : register SVR = 1 > >> IMX274 1-001a: imx274_read_mbreg : addr 0x30f6, val=0x6a8 (2 bytes) > >> IMX274 1-001a: imx274_set_frame_interval : register HMAX = 1704 > >> IMX274 1-001a: imx274_set_frame_length : input length = 2112 > >> IMX274 1-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 884 (3 bytes) > >> IMX274 1-001a: imx274_set_frame_length error = -121 > >> IMX274 1-001a: imx274_set_frame_interval error = -121 > >> Unable to setup formats: Remote I/O error (121) > >> > >> The device is not resumed thus the remote I/O error. > >> > >> Setting the frame interval works at streaming time, because > >> pm_runtime_resume_and_get is called at s_stream time before sensor setup. > >> The failure happens when only the s_frame_interval is called separately > >> independently on streaming time. > >> > >> Fixes: ad97bc37426c ("media: i2c: imx274: Add IMX274 power on and off sequence" > >> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com> > >> --- > >> drivers/media/i2c/imx274.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c > >> index e89ef35a71c5..6e63fdcc5e46 100644 > >> --- a/drivers/media/i2c/imx274.c > >> +++ b/drivers/media/i2c/imx274.c > >> @@ -1420,6 +1420,10 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, > >> int min, max, def; > >> int ret; > >> > >> + ret = pm_runtime_resume_and_get(&imx274->client->dev); > >> + if (ret < 0) > >> + return ret; > >> + > >> mutex_lock(&imx274->lock); > >> ret = imx274_set_frame_interval(imx274, fi->interval); > >> > >> @@ -1451,6 +1455,7 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, > >> > >> unlock: > >> mutex_unlock(&imx274->lock); > >> + pm_runtime_put(&imx274->client->dev); > >> > >> return ret; > >> } > > > > If the device is powered off in the end, could you instead not power it on > > in the first place? I.e. see how this works for the s_ctrl() callback. > > > Hi Sakari, > > I tried this initially, as in s_ctrl, > > if (!pm_runtime_get_if_in_use(&imx274->client->dev)) > > return 0; > > > However, if the device is powered off, the s_frame_interval does not do > anything (return 0), and the frame interval is not changed. Not even the > internal structure frame_interval is updated (as this is updated after > configuring the actual device). > And in consequence media-ctl -p will still print the old frame interval. > > So either we power on the device to set everything, or, things have to > be set in the software struct and written once streaming starts. > I am in favor of the first option (hence the patch), to avoid having > configuration that was requested but not written to the device itself. > The second option would require some rework to move the software part > before the hardware part, and to assume that the hardware part never > fails in bounds or by other reason (or the software part would be no > longer consistent) > > What do you think ? Seems reasonable, but the driver is hardly doing this in an exemplary way. Still the rework might not worth the small gain. I'll take this one then.
On 11/17/21 11:03 PM, Sakari Ailus wrote: > Hi Eugen, > > On Wed, Nov 17, 2021 at 04:52:40PM +0000, Eugen.Hristev@microchip.com wrote: >> On 11/17/21 6:11 PM, Sakari Ailus wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> Hi Eugen, >>> >>> On Wed, Nov 17, 2021 at 05:40:09PM +0200, Eugen Hristev wrote: >>>> pm_runtime_resume_and_get should be called when the s_frame_interval >>>> is called. >>>> >>>> The driver will try to access device registers to configure VMAX, coarse >>>> time and exposure. >>>> >>>> Currently if the runtime is not resumed, this fails: >>>> # media-ctl -d /dev/media0 --set-v4l2 '"IMX274 1-001a":0[fmt:SRGGB10_1X10/3840x2 >>>> 160@1/10]' >>>> >>>> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 3840x2160, goodness 0 >>>> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1920x1080, goodness -3000 >>>> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x720, goodness -4000 >>>> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x540, goodness -4180 >>>> IMX274 1-001a: __imx274_change_compose: selected 1x1 binning >>>> IMX274 1-001a: imx274_set_frame_interval: input frame interval = 1 / 10 >>>> IMX274 1-001a: imx274_read_mbreg : addr 0x300e, val=0x1 (2 bytes) >>>> IMX274 1-001a: imx274_set_frame_interval : register SVR = 1 >>>> IMX274 1-001a: imx274_read_mbreg : addr 0x30f6, val=0x6a8 (2 bytes) >>>> IMX274 1-001a: imx274_set_frame_interval : register HMAX = 1704 >>>> IMX274 1-001a: imx274_set_frame_length : input length = 2112 >>>> IMX274 1-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 884 (3 bytes) >>>> IMX274 1-001a: imx274_set_frame_length error = -121 >>>> IMX274 1-001a: imx274_set_frame_interval error = -121 >>>> Unable to setup formats: Remote I/O error (121) >>>> >>>> The device is not resumed thus the remote I/O error. >>>> >>>> Setting the frame interval works at streaming time, because >>>> pm_runtime_resume_and_get is called at s_stream time before sensor setup. >>>> The failure happens when only the s_frame_interval is called separately >>>> independently on streaming time. >>>> >>>> Fixes: ad97bc37426c ("media: i2c: imx274: Add IMX274 power on and off sequence" >>>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com> >>>> --- >>>> drivers/media/i2c/imx274.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c >>>> index e89ef35a71c5..6e63fdcc5e46 100644 >>>> --- a/drivers/media/i2c/imx274.c >>>> +++ b/drivers/media/i2c/imx274.c >>>> @@ -1420,6 +1420,10 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, >>>> int min, max, def; >>>> int ret; >>>> >>>> + ret = pm_runtime_resume_and_get(&imx274->client->dev); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> mutex_lock(&imx274->lock); >>>> ret = imx274_set_frame_interval(imx274, fi->interval); >>>> >>>> @@ -1451,6 +1455,7 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, >>>> >>>> unlock: >>>> mutex_unlock(&imx274->lock); >>>> + pm_runtime_put(&imx274->client->dev); >>>> >>>> return ret; >>>> } >>> >>> If the device is powered off in the end, could you instead not power it on >>> in the first place? I.e. see how this works for the s_ctrl() callback. >> >> >> Hi Sakari, >> >> I tried this initially, as in s_ctrl, >> >> if (!pm_runtime_get_if_in_use(&imx274->client->dev)) >> >> return 0; >> >> >> However, if the device is powered off, the s_frame_interval does not do >> anything (return 0), and the frame interval is not changed. Not even the >> internal structure frame_interval is updated (as this is updated after >> configuring the actual device). >> And in consequence media-ctl -p will still print the old frame interval. >> >> So either we power on the device to set everything, or, things have to >> be set in the software struct and written once streaming starts. >> I am in favor of the first option (hence the patch), to avoid having >> configuration that was requested but not written to the device itself. >> The second option would require some rework to move the software part >> before the hardware part, and to assume that the hardware part never >> fails in bounds or by other reason (or the software part would be no >> longer consistent) >> >> What do you think ? > > Seems reasonable, but the driver is hardly doing this in an exemplary way. > Still the rework might not worth the small gain. I'll take this one then. Okay, thank you. I noticed that the fixes tag in the commit message misses the last closing bracket ')' . Might break automated checkers and shout out a warning. Maybe it's possible to amend it ? Thanks again, Eugen > > -- > Kind regards, > > Sakari Ailus >
On Thu, Nov 18, 2021 at 07:16:16AM +0000, Eugen.Hristev@microchip.com wrote: > On 11/17/21 11:03 PM, Sakari Ailus wrote: > > Hi Eugen, > > > > On Wed, Nov 17, 2021 at 04:52:40PM +0000, Eugen.Hristev@microchip.com wrote: > >> On 11/17/21 6:11 PM, Sakari Ailus wrote: > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>> > >>> Hi Eugen, > >>> > >>> On Wed, Nov 17, 2021 at 05:40:09PM +0200, Eugen Hristev wrote: > >>>> pm_runtime_resume_and_get should be called when the s_frame_interval > >>>> is called. > >>>> > >>>> The driver will try to access device registers to configure VMAX, coarse > >>>> time and exposure. > >>>> > >>>> Currently if the runtime is not resumed, this fails: > >>>> # media-ctl -d /dev/media0 --set-v4l2 '"IMX274 1-001a":0[fmt:SRGGB10_1X10/3840x2 > >>>> 160@1/10]' > >>>> > >>>> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 3840x2160, goodness 0 > >>>> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1920x1080, goodness -3000 > >>>> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x720, goodness -4000 > >>>> IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x540, goodness -4180 > >>>> IMX274 1-001a: __imx274_change_compose: selected 1x1 binning > >>>> IMX274 1-001a: imx274_set_frame_interval: input frame interval = 1 / 10 > >>>> IMX274 1-001a: imx274_read_mbreg : addr 0x300e, val=0x1 (2 bytes) > >>>> IMX274 1-001a: imx274_set_frame_interval : register SVR = 1 > >>>> IMX274 1-001a: imx274_read_mbreg : addr 0x30f6, val=0x6a8 (2 bytes) > >>>> IMX274 1-001a: imx274_set_frame_interval : register HMAX = 1704 > >>>> IMX274 1-001a: imx274_set_frame_length : input length = 2112 > >>>> IMX274 1-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 884 (3 bytes) > >>>> IMX274 1-001a: imx274_set_frame_length error = -121 > >>>> IMX274 1-001a: imx274_set_frame_interval error = -121 > >>>> Unable to setup formats: Remote I/O error (121) > >>>> > >>>> The device is not resumed thus the remote I/O error. > >>>> > >>>> Setting the frame interval works at streaming time, because > >>>> pm_runtime_resume_and_get is called at s_stream time before sensor setup. > >>>> The failure happens when only the s_frame_interval is called separately > >>>> independently on streaming time. > >>>> > >>>> Fixes: ad97bc37426c ("media: i2c: imx274: Add IMX274 power on and off sequence" > >>>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com> > >>>> --- > >>>> drivers/media/i2c/imx274.c | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c > >>>> index e89ef35a71c5..6e63fdcc5e46 100644 > >>>> --- a/drivers/media/i2c/imx274.c > >>>> +++ b/drivers/media/i2c/imx274.c > >>>> @@ -1420,6 +1420,10 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, > >>>> int min, max, def; > >>>> int ret; > >>>> > >>>> + ret = pm_runtime_resume_and_get(&imx274->client->dev); > >>>> + if (ret < 0) > >>>> + return ret; > >>>> + > >>>> mutex_lock(&imx274->lock); > >>>> ret = imx274_set_frame_interval(imx274, fi->interval); > >>>> > >>>> @@ -1451,6 +1455,7 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, > >>>> > >>>> unlock: > >>>> mutex_unlock(&imx274->lock); > >>>> + pm_runtime_put(&imx274->client->dev); > >>>> > >>>> return ret; > >>>> } > >>> > >>> If the device is powered off in the end, could you instead not power it on > >>> in the first place? I.e. see how this works for the s_ctrl() callback. > >> > >> > >> Hi Sakari, > >> > >> I tried this initially, as in s_ctrl, > >> > >> if (!pm_runtime_get_if_in_use(&imx274->client->dev)) > >> > >> return 0; > >> > >> > >> However, if the device is powered off, the s_frame_interval does not do > >> anything (return 0), and the frame interval is not changed. Not even the > >> internal structure frame_interval is updated (as this is updated after > >> configuring the actual device). > >> And in consequence media-ctl -p will still print the old frame interval. > >> > >> So either we power on the device to set everything, or, things have to > >> be set in the software struct and written once streaming starts. > >> I am in favor of the first option (hence the patch), to avoid having > >> configuration that was requested but not written to the device itself. > >> The second option would require some rework to move the software part > >> before the hardware part, and to assume that the hardware part never > >> fails in bounds or by other reason (or the software part would be no > >> longer consistent) > >> > >> What do you think ? > > > > Seems reasonable, but the driver is hardly doing this in an exemplary way. > > Still the rework might not worth the small gain. I'll take this one then. > > > Okay, thank you. > I noticed that the fixes tag in the commit message misses the last > closing bracket ')' . Might break automated checkers and shout out a > warning. Maybe it's possible to amend it ? Thanks, fixed it.
diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c index e89ef35a71c5..6e63fdcc5e46 100644 --- a/drivers/media/i2c/imx274.c +++ b/drivers/media/i2c/imx274.c @@ -1420,6 +1420,10 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, int min, max, def; int ret; + ret = pm_runtime_resume_and_get(&imx274->client->dev); + if (ret < 0) + return ret; + mutex_lock(&imx274->lock); ret = imx274_set_frame_interval(imx274, fi->interval); @@ -1451,6 +1455,7 @@ static int imx274_s_frame_interval(struct v4l2_subdev *sd, unlock: mutex_unlock(&imx274->lock); + pm_runtime_put(&imx274->client->dev); return ret; }
pm_runtime_resume_and_get should be called when the s_frame_interval is called. The driver will try to access device registers to configure VMAX, coarse time and exposure. Currently if the runtime is not resumed, this fails: # media-ctl -d /dev/media0 --set-v4l2 '"IMX274 1-001a":0[fmt:SRGGB10_1X10/3840x2 160@1/10]' IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 3840x2160, goodness 0 IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1920x1080, goodness -3000 IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x720, goodness -4000 IMX274 1-001a: imx274_binning_goodness: ask 3840x2160, size 1280x540, goodness -4180 IMX274 1-001a: __imx274_change_compose: selected 1x1 binning IMX274 1-001a: imx274_set_frame_interval: input frame interval = 1 / 10 IMX274 1-001a: imx274_read_mbreg : addr 0x300e, val=0x1 (2 bytes) IMX274 1-001a: imx274_set_frame_interval : register SVR = 1 IMX274 1-001a: imx274_read_mbreg : addr 0x30f6, val=0x6a8 (2 bytes) IMX274 1-001a: imx274_set_frame_interval : register HMAX = 1704 IMX274 1-001a: imx274_set_frame_length : input length = 2112 IMX274 1-001a: imx274_write_mbreg : i2c bulk write failed, 30f8 = 884 (3 bytes) IMX274 1-001a: imx274_set_frame_length error = -121 IMX274 1-001a: imx274_set_frame_interval error = -121 Unable to setup formats: Remote I/O error (121) The device is not resumed thus the remote I/O error. Setting the frame interval works at streaming time, because pm_runtime_resume_and_get is called at s_stream time before sensor setup. The failure happens when only the s_frame_interval is called separately independently on streaming time. Fixes: ad97bc37426c ("media: i2c: imx274: Add IMX274 power on and off sequence" Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com> --- drivers/media/i2c/imx274.c | 5 +++++ 1 file changed, 5 insertions(+)