| Message ID | 20240831055328.22482-1-zhi.mao@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: improve suspend/resume switch performance for GT9769 VCM driver | expand |
ping... On Sat, 2024-08-31 at 13:52 +0800, Zhi Mao wrote: > Chromebook camera AP can switch between video and photo mode, > the behavior corresponding to VCM is suspend and resume, > it will cause camera preview is not smooth during switching > operation. > We use autosuspend function can fix this issue. > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com> > --- > drivers/media/i2c/dw9768.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c > index 18ef2b35c9aa..7449d29df901 100644 > --- a/drivers/media/i2c/dw9768.c > +++ b/drivers/media/i2c/dw9768.c > @@ -374,7 +374,7 @@ static int dw9768_open(struct v4l2_subdev *sd, > struct v4l2_subdev_fh *fh) > > static int dw9768_close(struct v4l2_subdev *sd, struct > v4l2_subdev_fh *fh) > { > - pm_runtime_put(sd->dev); > + pm_runtime_put_sync_autosuspend(sd->dev); > > return 0; > } > @@ -490,6 +490,7 @@ static int dw9768_probe(struct i2c_client > *client) > goto err_power_off; > } > > + pm_runtime_use_autosuspend(dev); > pm_runtime_idle(dev); > > return 0;
Hi Zhi, On Sat, Aug 31, 2024 at 1:54 PM Zhi Mao <zhi.mao@mediatek.com> wrote: Suggestion for the new patch title: media: i2c: dw9768: Use runtime PM autosuspend to better describe "what" this patch does to the DW9768 driver, not "why". Also there's nothing to do with GT9769 in this patch. > > Chromebook camera AP can switch between video and photo mode, > the behavior corresponding to VCM is suspend and resume, > it will cause camera preview is not smooth during switching operation. > We use autosuspend function can fix this issue. > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com> > --- > drivers/media/i2c/dw9768.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c > index 18ef2b35c9aa..7449d29df901 100644 > --- a/drivers/media/i2c/dw9768.c > +++ b/drivers/media/i2c/dw9768.c > @@ -374,7 +374,7 @@ static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > > static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > { > - pm_runtime_put(sd->dev); Call pm_runtime_mark_last_busy() to update the power.last_busy field for autosuspend to work properly. > + pm_runtime_put_sync_autosuspend(sd->dev); Use pm_runtime_put_autosuspend(). Don't switch to the synchronized variant without reasons. > > return 0; > } > @@ -490,6 +490,7 @@ static int dw9768_probe(struct i2c_client *client) > goto err_power_off; > } > > + pm_runtime_use_autosuspend(dev); Add pm_runtime_set_autosuspend_delay() before calling pm_runtime_use_autosuspend() to avoid rapid power state bouncing. Regards, Fei > pm_runtime_idle(dev); > > return 0; > -- > 2.46.0 > >
Hi Fei, Thanks for your review. On Mon, 2024-09-23 at 17:07 +0800, Fei Shao wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > Hi Zhi, > > On Sat, Aug 31, 2024 at 1:54 PM Zhi Mao <zhi.mao@mediatek.com> wrote: > > Suggestion for the new patch title: > media: i2c: dw9768: Use runtime PM autosuspend > > to better describe "what" this patch does to the DW9768 driver, not > "why". > Also there's nothing to do with GT9769 in this patch. > > > > > Chromebook camera AP can switch between video and photo mode, > > the behavior corresponding to VCM is suspend and resume, > > it will cause camera preview is not smooth during switching > operation. > > We use autosuspend function can fix this issue. > > > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com> > > --- > > drivers/media/i2c/dw9768.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/dw9768.c > b/drivers/media/i2c/dw9768.c > > index 18ef2b35c9aa..7449d29df901 100644 > > --- a/drivers/media/i2c/dw9768.c > > +++ b/drivers/media/i2c/dw9768.c > > @@ -374,7 +374,7 @@ static int dw9768_open(struct v4l2_subdev *sd, > struct v4l2_subdev_fh *fh) > > > > static int dw9768_close(struct v4l2_subdev *sd, struct > v4l2_subdev_fh *fh) > > { > > - pm_runtime_put(sd->dev); > > Call pm_runtime_mark_last_busy() to update the power.last_busy field > for autosuspend to work properly. > > > + pm_runtime_put_sync_autosuspend(sd->dev); > > Use pm_runtime_put_autosuspend(). > Don't switch to the synchronized variant without reasons. > > > > > return 0; > > } > > @@ -490,6 +490,7 @@ static int dw9768_probe(struct i2c_client > *client) > > goto err_power_off; > > } > > > > + pm_runtime_use_autosuspend(dev); > > Add pm_runtime_set_autosuspend_delay() before calling > pm_runtime_use_autosuspend() to avoid rapid power state bouncing. > I have fixed these suggestion above. please review patch: https://lore.kernel.org/all/20241012032805.23545-1-zhi.mao@mediatek.com/ > Regards, > Fei > > > > > pm_runtime_idle(dev); > > > > return 0; > > -- > > 2.46.0 > > > >
diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c index 18ef2b35c9aa..7449d29df901 100644 --- a/drivers/media/i2c/dw9768.c +++ b/drivers/media/i2c/dw9768.c @@ -374,7 +374,7 @@ static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) { - pm_runtime_put(sd->dev); + pm_runtime_put_sync_autosuspend(sd->dev); return 0; } @@ -490,6 +490,7 @@ static int dw9768_probe(struct i2c_client *client) goto err_power_off; } + pm_runtime_use_autosuspend(dev); pm_runtime_idle(dev); return 0;
Chromebook camera AP can switch between video and photo mode, the behavior corresponding to VCM is suspend and resume, it will cause camera preview is not smooth during switching operation. We use autosuspend function can fix this issue. Signed-off-by: Zhi Mao <zhi.mao@mediatek.com> --- drivers/media/i2c/dw9768.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)