| Message ID | 1621517811-64765-1-git-send-email-tiantao6@hisilicon.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/exynos: Use pm_runtime_resume_and_get() to replace open coding | expand |
Hi, 21. 5. 20. 오후 10:36에 Tian Tao 이(가) 쓴 글: > use pm_runtime_resume_and_get() to replace pm_runtime_get_sync and > pm_runtime_put_noidle. it also avoids the problem of positive return > values so we can change if (ret < 0) to if (ret). Could you tell me why did you change the condition? pm_runtime_resume_and_get() can return only 0 or negative value. So I think you don't have to change the condition. Could you drop this description? > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > --- > drivers/gpu/drm/exynos/exynos_drm_mic.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c > index 3821ea7..6d94eae 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c > @@ -268,11 +268,9 @@ static void mic_pre_enable(struct drm_bridge *bridge) > if (mic->enabled) > goto unlock; > > - ret = pm_runtime_get_sync(mic->dev); > - if (ret < 0) { > - pm_runtime_put_noidle(mic->dev); > + ret = pm_runtime_resume_and_get(mic->dev); Right, we can use pm_runtime_resume_and_get function because pm_runtime_resume_and_get function does exactly same thing as existing code does. > + if (ret) Seems unnecessary change. Thanks, Inki Dae > goto unlock; > - } > > mic_set_path(mic, 1); > >
在 2021/5/21 12:47, Inki Dae 写道: > Hi, > > 21. 5. 20. 오후 10:36에 Tian Tao 이(가) 쓴 글: >> use pm_runtime_resume_and_get() to replace pm_runtime_get_sync and >> pm_runtime_put_noidle. it also avoids the problem of positive return >> values so we can change if (ret < 0) to if (ret). > Could you tell me why did you change the condition? pm_runtime_resume_and_get() can return only 0 or negative value. > So I think you don't have to change the condition. Could you drop this description? > >> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> >> --- >> drivers/gpu/drm/exynos/exynos_drm_mic.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c >> index 3821ea7..6d94eae 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c >> @@ -268,11 +268,9 @@ static void mic_pre_enable(struct drm_bridge *bridge) >> if (mic->enabled) >> goto unlock; >> >> - ret = pm_runtime_get_sync(mic->dev); >> - if (ret < 0) { >> - pm_runtime_put_noidle(mic->dev); >> + ret = pm_runtime_resume_and_get(mic->dev); > Right, we can use pm_runtime_resume_and_get function because pm_runtime_resume_and_get function does exactly same thing as existing code does. > >> + if (ret) > Seems unnecessary change. as you can see，If pm_runtime_resume_and_get returns 0, it means that the function was executed successfully and should not be executed in an if condition. There is no error in continuing to use if (ret < 0), but it is not as concise as using if (ret) directly static inline int pm_runtime_resume_and_get(struct device *dev) { int ret; ret = __pm_runtime_resume(dev, RPM_GET_PUT); if (ret < 0) { pm_runtime_put_noidle(dev); return ret; } return 0; } > > Thanks, > Inki Dae > >> goto unlock; >> - } >> >> mic_set_path(mic, 1); >> >> > . >
在 2021/5/21 16:36, Inki Dae 写道: > > 21. 5. 21. 오후 3:08에 tiantao (H) 이(가) 쓴 글: >> 在 2021/5/21 12:47, Inki Dae 写道: >>> Hi, >>> >>> 21. 5. 20. 오후 10:36에 Tian Tao 이(가) 쓴 글: >>>> use pm_runtime_resume_and_get() to replace pm_runtime_get_sync and >>>> pm_runtime_put_noidle. it also avoids the problem of positive return >>>> values so we can change if (ret < 0) to if (ret). >>> Could you tell me why did you change the condition? pm_runtime_resume_and_get() can return only 0 or negative value. >>> So I think you don't have to change the condition. Could you drop this description? >>> >>>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> >>>> --- >>>> drivers/gpu/drm/exynos/exynos_drm_mic.c | 6 ++---- >>>> 1 file changed, 2 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c >>>> index 3821ea7..6d94eae 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c >>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c >>>> @@ -268,11 +268,9 @@ static void mic_pre_enable(struct drm_bridge *bridge) >>>> if (mic->enabled) >>>> goto unlock; >>>> - ret = pm_runtime_get_sync(mic->dev); >>>> - if (ret < 0) { >>>> - pm_runtime_put_noidle(mic->dev); >>>> + ret = pm_runtime_resume_and_get(mic->dev); >>> Right, we can use pm_runtime_resume_and_get function because pm_runtime_resume_and_get function does exactly same thing as existing code does. >>> >>>> + if (ret) >>> Seems unnecessary change. >> as you can see，If pm_runtime_resume_and_get returns 0, it means that the function was executed successfully and should not be executed in an if condition. >> >> There is no error in continuing to use if (ret < 0), but it is not as concise as using if (ret) directly >> > I don't see why positive value should be considered because pm_runtime_resume_and_get function never return positive value as you can see. > Is there something I'm missing? sorry， I misunderstand you, I wil send v2 to drop this. > >> static inline int pm_runtime_resume_and_get(struct device *dev) >> { >> int ret; >> >> ret = __pm_runtime_resume(dev, RPM_GET_PUT); >> if (ret < 0) { >> pm_runtime_put_noidle(dev); >> return ret; >> } >> >> return 0; >> } >>> Thanks, >>> Inki Dae >>> >>>> goto unlock; >>>> - } >>>> mic_set_path(mic, 1); >>>> >>> . >>> >> > . >
21. 5. 21. 오후 3:08에 tiantao (H) 이(가) 쓴 글: > > 在 2021/5/21 12:47, Inki Dae 写道: >> Hi, >> >> 21. 5. 20. 오후 10:36에 Tian Tao 이(가) 쓴 글: >>> use pm_runtime_resume_and_get() to replace pm_runtime_get_sync and >>> pm_runtime_put_noidle. it also avoids the problem of positive return >>> values so we can change if (ret < 0) to if (ret). >> Could you tell me why did you change the condition? pm_runtime_resume_and_get() can return only 0 or negative value. >> So I think you don't have to change the condition. Could you drop this description? >> >>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_mic.c | 6 ++---- >>> 1 file changed, 2 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c >>> index 3821ea7..6d94eae 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c >>> @@ -268,11 +268,9 @@ static void mic_pre_enable(struct drm_bridge *bridge) >>> if (mic->enabled) >>> goto unlock; >>> - ret = pm_runtime_get_sync(mic->dev); >>> - if (ret < 0) { >>> - pm_runtime_put_noidle(mic->dev); >>> + ret = pm_runtime_resume_and_get(mic->dev); >> Right, we can use pm_runtime_resume_and_get function because pm_runtime_resume_and_get function does exactly same thing as existing code does. >> >>> + if (ret) >> Seems unnecessary change. > > as you can see，If pm_runtime_resume_and_get returns 0, it means that the function was executed successfully and should not be executed in an if condition. > > There is no error in continuing to use if (ret < 0), but it is not as concise as using if (ret) directly > I don't see why positive value should be considered because pm_runtime_resume_and_get function never return positive value as you can see. Is there something I'm missing? > static inline int pm_runtime_resume_and_get(struct device *dev) > { > int ret; > > ret = __pm_runtime_resume(dev, RPM_GET_PUT); > if (ret < 0) { > pm_runtime_put_noidle(dev); > return ret; > } > > return 0; > } >> >> Thanks, >> Inki Dae >> >>> goto unlock; >>> - } >>> mic_set_path(mic, 1); >>> >> . >> > >
diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c index 3821ea7..6d94eae 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c @@ -268,11 +268,9 @@ static void mic_pre_enable(struct drm_bridge *bridge) if (mic->enabled) goto unlock; - ret = pm_runtime_get_sync(mic->dev); - if (ret < 0) { - pm_runtime_put_noidle(mic->dev); + ret = pm_runtime_resume_and_get(mic->dev); + if (ret) goto unlock; - } mic_set_path(mic, 1);
use pm_runtime_resume_and_get() to replace pm_runtime_get_sync and pm_runtime_put_noidle. it also avoids the problem of positive return values so we can change if (ret < 0) to if (ret). Signed-off-by: Tian Tao <tiantao6@hisilicon.com> --- drivers/gpu/drm/exynos/exynos_drm_mic.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)