| Message ID | 20200821074506.32359-1-dinghao.liu@zju.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/omap: Fix runtime PM imbalance in dsi_runtime_get | expand |
> Hi, > > On 21/08/2020 10:45, Dinghao Liu wrote: > > pm_runtime_get_sync() increments the runtime PM usage counter > > even when it returns an error code. However, users of > > dsi_runtime_get(), a direct wrapper of pm_runtime_get_sync(), > > assume that PM usage counter will not change on error. Thus a > > pairing decrement is needed on the error handling path to keep > > the counter balanced. > > > > Fixes: 4fbafaf371be7 ("OMAP: DSS2: Use PM runtime & HWMOD support") > > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn> > > --- > > drivers/gpu/drm/omapdrm/dss/dsi.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > > index eeccf40bae41..973bfa14a104 100644 > > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > > @@ -1112,8 +1112,11 @@ static int dsi_runtime_get(struct dsi_data *dsi) > > DSSDBG("dsi_runtime_get\n"); > > > > r = pm_runtime_get_sync(dsi->dev); > > - WARN_ON(r < 0); > > - return r < 0 ? r : 0; > > + if (WARN_ON(r < 0)) { > > + pm_runtime_put_noidle(dsi->dev); > > + return r; > > + } > > + return 0; > > } > > Thanks! Good catch. I think this is broken in all the other modules in omapdrm too (e.g. dispc.c, > venc.c, etc). > > Would you like to update the patch to cover the whole omapdrm? > Sure. I will fix this soon. Regards, Dinghao
Hi Tomi, On Fri, Aug 21, 2020 at 03:06:59PM +0300, Tomi Valkeinen wrote: > On 21/08/2020 10:45, Dinghao Liu wrote: > > pm_runtime_get_sync() increments the runtime PM usage counter > > even when it returns an error code. However, users of > > dsi_runtime_get(), a direct wrapper of pm_runtime_get_sync(), > > assume that PM usage counter will not change on error. Thus a > > pairing decrement is needed on the error handling path to keep > > the counter balanced. > > > > Fixes: 4fbafaf371be7 ("OMAP: DSS2: Use PM runtime & HWMOD support") > > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn> > > --- > > drivers/gpu/drm/omapdrm/dss/dsi.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > > index eeccf40bae41..973bfa14a104 100644 > > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > > @@ -1112,8 +1112,11 @@ static int dsi_runtime_get(struct dsi_data *dsi) > > DSSDBG("dsi_runtime_get\n"); > > > > r = pm_runtime_get_sync(dsi->dev); > > - WARN_ON(r < 0); > > - return r < 0 ? r : 0; > > + if (WARN_ON(r < 0)) { > > + pm_runtime_put_noidle(dsi->dev); > > + return r; > > + } > > + return 0; > > } > > Thanks! Good catch. I think this is broken in all the other modules in omapdrm too (e.g. dispc.c, > venc.c, etc). > > Would you like to update the patch to cover the whole omapdrm? Just for yoru information, there has been quite a few similar patches submitted all across the kernel. I believe this is an issue of the pm_runtime_get_sync() API, which really shouldn't require a put() when it fails. For drivers that really don't expect pm_runtime_get_sync() to fail (no I2C access to a regulator for instance, only SoC-internal operations) I've instead decided to ignore the error completely. I don't think poluting the whole kernel code base with this kind of "fixes" is a good idea.
diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index eeccf40bae41..973bfa14a104 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -1112,8 +1112,11 @@ static int dsi_runtime_get(struct dsi_data *dsi) DSSDBG("dsi_runtime_get\n"); r = pm_runtime_get_sync(dsi->dev); - WARN_ON(r < 0); - return r < 0 ? r : 0; + if (WARN_ON(r < 0)) { + pm_runtime_put_noidle(dsi->dev); + return r; + } + return 0; } static void dsi_runtime_put(struct dsi_data *dsi)
pm_runtime_get_sync() increments the runtime PM usage counter even when it returns an error code. However, users of dsi_runtime_get(), a direct wrapper of pm_runtime_get_sync(), assume that PM usage counter will not change on error. Thus a pairing decrement is needed on the error handling path to keep the counter balanced. Fixes: 4fbafaf371be7 ("OMAP: DSS2: Use PM runtime & HWMOD support") Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn> --- drivers/gpu/drm/omapdrm/dss/dsi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)