On 26.08.2019 17:26, Boris Brezillon wrote:
The encoder->enable() can't report errors and is expected to always succeed. If we call pm_runtime_put() in the exynos_dsi_enable() error path (as currently done) we'll have unbalanced get/put calls when encoder->disable() is called.
True
The situation is not ideal since drm_panel_{prepare,enable}() can theoretically return an error (even if in practice I don't think any panel driver does that).
So why do you want to fix it at all, if you think return value is always 0 :) ?
git grep -p -A30 '_prepare' drivers/gpu/drm/panel/ shows that many of them can return errors.
Putting a WARN_ON() is the best we can do, unfortunately.
I guess optimally we should use DRM_MODE_LINK_STATUS_BAD, but I am not sure how exactly it should be handled.
Note that -ENOSYS is actually a valid case, it just means the panel driver does not implement the hook.
It would be good then to fix it in panel framework, ie without hook drm_panel_* function should return 0, ENOSYS makes no sense here.
Regards
Andrzej
Signed-off-by: Boris Brezillon boris.brezillon@collabora.com
Changes in v2:
- New patch
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 8e655ae1fb0c..c555cecfe1f5 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1387,8 +1387,7 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
if (dsi->panel) { ret = drm_panel_prepare(dsi->panel);
if (ret < 0)goto err_put_sync;
WARN_ON(ret && ret != -ENOSYS);@@ -1398,22 +1397,13 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
if (dsi->panel) { ret = drm_panel_enable(dsi->panel);
if (ret < 0)goto err_display_disable;
WARN_ON(ret && ret != -ENOSYS);} else { drm_bridge_enable(dsi->out_bridge); }
dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE; return;
-err_display_disable:
- exynos_dsi_set_display_enable(dsi, false);
- drm_panel_unprepare(dsi->panel);
-err_put_sync:
- dsi->state &= ~DSIM_STATE_ENABLED;
- pm_runtime_put(dsi->dev);
}
static void exynos_dsi_disable(struct drm_encoder *encoder)