Hi Inki,
This patchset contains various fixes and improvements to exynos_drm components: mixer, hdmi and crtc.
Patchset is based on exynos-drm-next-todo.
Regards Andrzej
Andrzej Hajda (7): drm/exynos/hdmi: fix edid memory leak drm/exynos/hdmi: fix interrupt clearing drm/exynos/mixer: correct vsync configuration sequence drm/exynos/mixer: always update INT_EN cache drm/exynos/mixer: simplify poweron flag drm/exynos/mixer: replace MXR_INT_EN register cache with flag drm/exynos/crtc: add NULL checks before accessing crtc
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 11 +++-- drivers/gpu/drm/exynos/exynos_hdmi.c | 7 ++- drivers/gpu/drm/exynos/exynos_mixer.c | 79 ++++++++++++-------------------- 3 files changed, 44 insertions(+), 53 deletions(-)
edid returned by drm_get_edid should be freed. The patch fixes it.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_hdmi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 563a19e..65a2285 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -1055,6 +1055,7 @@ static int hdmi_get_modes(struct drm_connector *connector) { struct hdmi_context *hdata = ctx_from_connector(connector); struct edid *edid; + int ret;
if (!hdata->ddc_adpt) return -ENODEV; @@ -1070,7 +1071,11 @@ static int hdmi_get_modes(struct drm_connector *connector)
drm_mode_connector_update_edid_property(connector, edid);
- return drm_add_edid_modes(connector, edid); + ret = drm_add_edid_modes(connector, edid); + + kfree(edid); + + return ret; }
static int hdmi_find_phy_conf(struct hdmi_context *hdata, u32 pixel_clock)
The driver used incorrect flags to clear interrupt status. The patch fixes it.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index a41c84e..52275b4 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -718,6 +718,9 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
/* handling VSYNC */ if (val & MXR_INT_STATUS_VSYNC) { + /* vsync interrupt use different bit for read and clear */ + val |= MXR_INT_CLEAR_VSYNC; + /* interlace scan need to check shadow register */ if (ctx->interlace) { base = mixer_reg_read(res, MXR_GRAPHIC_BASE(0)); @@ -743,11 +746,6 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
out: /* clear interrupts */ - if (~val & MXR_INT_EN_VSYNC) { - /* vsync interrupt use different bit for read and clear */ - val &= ~MXR_INT_EN_VSYNC; - val |= MXR_INT_CLEAR_VSYNC; - } mixer_reg_write(res, MXR_INT_STATUS, val);
spin_unlock(&res->reg_slock);
Specification advises to clear vsync indicator before configuring vsync.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 52275b4..2df1bb2 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -900,8 +900,8 @@ static int mixer_enable_vblank(struct exynos_drm_manager *mgr) }
/* enable vsync interrupt */ - mixer_reg_writemask(res, MXR_INT_EN, MXR_INT_EN_VSYNC, - MXR_INT_EN_VSYNC); + mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC); + mixer_reg_writemask(res, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC);
return 0; } @@ -912,6 +912,7 @@ static void mixer_disable_vblank(struct exynos_drm_manager *mgr) struct mixer_resources *res = &mixer_ctx->mixer_res;
/* disable vsync interrupt */ + mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC); mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC); }
@@ -1101,6 +1102,8 @@ static void mixer_poweron(struct exynos_drm_manager *mgr)
mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);
+ if (ctx->int_en & MXR_INT_EN_VSYNC) + mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC); mixer_reg_write(res, MXR_INT_EN, ctx->int_en); mixer_win_reset(ctx);
INT_EN cache field was updated only by mixer_enable_vblank. The patch adds update also by mixer_disable_vblank function.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 2df1bb2..360a40b 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -911,6 +911,11 @@ static void mixer_disable_vblank(struct exynos_drm_manager *mgr) struct mixer_context *mixer_ctx = mgr->ctx; struct mixer_resources *res = &mixer_ctx->mixer_res;
+ if (!mixer_ctx->powered) { + mixer_ctx->int_en &= MXR_INT_EN_VSYNC; + return; + } + /* disable vsync interrupt */ mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC); mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
The driver uses bool protected by mutex to track power state. The patch replaces this combo with single bit and atomic bitops.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 51 ++++++++++------------------------- 1 file changed, 14 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 360a40b..2465b1d 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -85,18 +85,21 @@ enum mixer_version_id { MXR_VER_128_0_0_184, };
+enum mixer_flag_bits { + MXR_BIT_POWERED, +}; + struct mixer_context { struct platform_device *pdev; struct device *dev; struct drm_device *drm_dev; int pipe; + unsigned long flags; bool interlace; - bool powered; bool vp_enabled; bool has_sclk; u32 int_en;
- struct mutex mixer_mutex; struct mixer_resources mixer_res; struct hdmi_win_data win_data[MIXER_WIN_NR]; enum mixer_version_id mxr_ver; @@ -894,7 +897,7 @@ static int mixer_enable_vblank(struct exynos_drm_manager *mgr) struct mixer_context *mixer_ctx = mgr->ctx; struct mixer_resources *res = &mixer_ctx->mixer_res;
- if (!mixer_ctx->powered) { + if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) { mixer_ctx->int_en |= MXR_INT_EN_VSYNC; return 0; } @@ -911,7 +914,7 @@ static void mixer_disable_vblank(struct exynos_drm_manager *mgr) struct mixer_context *mixer_ctx = mgr->ctx; struct mixer_resources *res = &mixer_ctx->mixer_res;
- if (!mixer_ctx->powered) { + if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) { mixer_ctx->int_en &= MXR_INT_EN_VSYNC; return; } @@ -980,12 +983,8 @@ static void mixer_win_commit(struct exynos_drm_manager *mgr, int zpos)
DRM_DEBUG_KMS("win: %d\n", win);
- mutex_lock(&mixer_ctx->mixer_mutex); - if (!mixer_ctx->powered) { - mutex_unlock(&mixer_ctx->mixer_mutex); + if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) return; - } - mutex_unlock(&mixer_ctx->mixer_mutex);
if (win > 1 && mixer_ctx->vp_enabled) vp_video_buffer(mixer_ctx, win); @@ -1004,13 +1003,10 @@ static void mixer_win_disable(struct exynos_drm_manager *mgr, int zpos)
DRM_DEBUG_KMS("win: %d\n", win);
- mutex_lock(&mixer_ctx->mixer_mutex); - if (!mixer_ctx->powered) { - mutex_unlock(&mixer_ctx->mixer_mutex); + if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) { mixer_ctx->win_data[win].resume = false; return; } - mutex_unlock(&mixer_ctx->mixer_mutex);
spin_lock_irqsave(&res->reg_slock, flags); mixer_vsync_set_update(mixer_ctx, false); @@ -1027,12 +1023,8 @@ static void mixer_wait_for_vblank(struct exynos_drm_manager *mgr) { struct mixer_context *mixer_ctx = mgr->ctx;
- mutex_lock(&mixer_ctx->mixer_mutex); - if (!mixer_ctx->powered) { - mutex_unlock(&mixer_ctx->mixer_mutex); + if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) return; - } - mutex_unlock(&mixer_ctx->mixer_mutex);
drm_vblank_get(mgr->crtc->dev, mixer_ctx->pipe);
@@ -1084,13 +1076,8 @@ static void mixer_poweron(struct exynos_drm_manager *mgr) struct mixer_context *ctx = mgr->ctx; struct mixer_resources *res = &ctx->mixer_res;
- mutex_lock(&ctx->mixer_mutex); - if (ctx->powered) { - mutex_unlock(&ctx->mixer_mutex); + if (test_bit(MXR_BIT_POWERED, &ctx->flags)) return; - } - - mutex_unlock(&ctx->mixer_mutex);
pm_runtime_get_sync(ctx->dev);
@@ -1101,9 +1088,7 @@ static void mixer_poweron(struct exynos_drm_manager *mgr) clk_prepare_enable(res->sclk_mixer); }
- mutex_lock(&ctx->mixer_mutex); - ctx->powered = true; - mutex_unlock(&ctx->mixer_mutex); + set_bit(MXR_BIT_POWERED, &ctx->flags);
mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);
@@ -1120,21 +1105,15 @@ static void mixer_poweroff(struct exynos_drm_manager *mgr) struct mixer_context *ctx = mgr->ctx; struct mixer_resources *res = &ctx->mixer_res;
- mutex_lock(&ctx->mixer_mutex); - if (!ctx->powered) { - mutex_unlock(&ctx->mixer_mutex); + if (!test_bit(MXR_BIT_POWERED, &ctx->flags)) return; - } - mutex_unlock(&ctx->mixer_mutex);
mixer_stop(ctx); mixer_window_suspend(mgr);
ctx->int_en = mixer_reg_read(res, MXR_INT_EN);
- mutex_lock(&ctx->mixer_mutex); - ctx->powered = false; - mutex_unlock(&ctx->mixer_mutex); + clear_bit(MXR_BIT_POWERED, &ctx->flags);
clk_disable_unprepare(res->mixer); if (ctx->vp_enabled) { @@ -1269,8 +1248,6 @@ static int mixer_bind(struct device *dev, struct device *manager, void *data) return -ENOMEM; }
- mutex_init(&ctx->mixer_mutex); - if (dev->of_node) { const struct of_device_id *match; match = of_match_node(mixer_match_types, dev->of_node);
Driver uses only VSYNC interrupts, so we need to cache VSYNC bit state only.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 2465b1d..4858170e 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -87,6 +87,7 @@ enum mixer_version_id {
enum mixer_flag_bits { MXR_BIT_POWERED, + MXR_BIT_VSYNC, };
struct mixer_context { @@ -98,7 +99,6 @@ struct mixer_context { bool interlace; bool vp_enabled; bool has_sclk; - u32 int_en;
struct mixer_resources mixer_res; struct hdmi_win_data win_data[MIXER_WIN_NR]; @@ -897,10 +897,9 @@ static int mixer_enable_vblank(struct exynos_drm_manager *mgr) struct mixer_context *mixer_ctx = mgr->ctx; struct mixer_resources *res = &mixer_ctx->mixer_res;
- if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) { - mixer_ctx->int_en |= MXR_INT_EN_VSYNC; + __set_bit(MXR_BIT_VSYNC, &mixer_ctx->flags); + if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) return 0; - }
/* enable vsync interrupt */ mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC); @@ -914,10 +913,10 @@ static void mixer_disable_vblank(struct exynos_drm_manager *mgr) struct mixer_context *mixer_ctx = mgr->ctx; struct mixer_resources *res = &mixer_ctx->mixer_res;
- if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) { - mixer_ctx->int_en &= MXR_INT_EN_VSYNC; + __clear_bit(MXR_BIT_VSYNC, &mixer_ctx->flags); + + if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) return; - }
/* disable vsync interrupt */ mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC); @@ -1092,9 +1091,10 @@ static void mixer_poweron(struct exynos_drm_manager *mgr)
mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);
- if (ctx->int_en & MXR_INT_EN_VSYNC) + if (test_bit(MXR_BIT_VSYNC, &ctx->flags)) { mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC); - mixer_reg_write(res, MXR_INT_EN, ctx->int_en); + mixer_reg_writemask(res, MXR_INT_EN, ~0, MXR_INT_EN_VSYNC); + } mixer_win_reset(ctx);
mixer_window_resume(mgr); @@ -1111,8 +1111,6 @@ static void mixer_poweroff(struct exynos_drm_manager *mgr) mixer_stop(ctx); mixer_window_suspend(mgr);
- ctx->int_en = mixer_reg_read(res, MXR_INT_EN); - clear_bit(MXR_BIT_POWERED, &ctx->flags);
clk_disable_unprepare(res->mixer);
exynos_drm_crtc_disable_vblank can be called when drm initialization fails. In such case some structures are not initialized, so the function should check it. The patch adds necessary checks. It fixes following Oops: [ 1.521834] Unable to handle kernel NULL pointer dereference at virtual address 00000348 ... [ 1.801740] [<c02687a0>] (exynos_drm_crtc_disable_vblank) from [<c0252200>] (vblank_disable_and_save+0x74/0x1e8) [ 1.811893] [<c0252200>] (vblank_disable_and_save) from [<c02535c0>] (drm_vblank_cleanup+0x50/0x80) [ 1.820918] [<c02535c0>] (drm_vblank_cleanup) from [<c026791c>] (exynos_drm_load+0xe8/0x118) [ 1.829338] [<c026791c>] (exynos_drm_load) from [<c025472c>] (drm_dev_register+0xa0/0x100) [ 1.837585] [<c025472c>] (drm_dev_register) from [<c02563a8>] (drm_platform_init+0x40/0xd0) [ 1.845919] [<c02563a8>] (drm_platform_init) from [<c0276968>] (try_to_bring_up_master.part.2+0xc8/0x104) [ 1.855467] [<c0276968>] (try_to_bring_up_master.part.2) from [<c0276a48>] (component_master_add_with_match+0xa4/0x124) [ 1.866227] [<c0276a48>] (component_master_add_with_match) from [<c0267a58>] (exynos_drm_platform_probe+0x10c/0x158) [ 1.876731] [<c0267a58>] (exynos_drm_platform_probe) from [<c027bc70>] (platform_drv_probe+0x48/0xa4) [ 1.885932] [<c027bc70>] (platform_drv_probe) from [<c027a840>] (driver_probe_device+0x10c/0x22c) [ 1.894784] [<c027a840>] (driver_probe_device) from [<c027a9ec>] (__driver_attach+0x8c/0x90) [ 1.903203] [<c027a9ec>] (__driver_attach) from [<c0279080>] (bus_for_each_dev+0x54/0x88) [ 1.911363] [<c0279080>] (bus_for_each_dev) from [<c027a040>] (bus_add_driver+0xd4/0x1d0) [ 1.919522] [<c027a040>] (bus_add_driver) from [<c027b014>] (driver_register+0x78/0xf4) [ 1.927507] [<c027b014>] (driver_register) from [<c0267b08>] (exynos_drm_init+0x64/0x8c) [ 1.935580] [<c0267b08>] (exynos_drm_init) from [<c0008924>] (do_one_initcall+0x80/0x1b8) [ 1.943743] [<c0008924>] (do_one_initcall) from [<c060cd40>] (kernel_init_freeable+0xfc/0x1c8) [ 1.952334] [<c060cd40>] (kernel_init_freeable) from [<c0447e04>] (kernel_init+0x8/0xec) [ 1.960406] [<c0447e04>] (kernel_init) from [<c000e738>] (ret_from_fork+0x14/0x3c)
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index 45026e6..e05fe12 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -406,9 +406,14 @@ int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int pipe) void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe) { struct exynos_drm_private *private = dev->dev_private; - struct exynos_drm_crtc *exynos_crtc = - to_exynos_crtc(private->crtc[pipe]); - struct exynos_drm_manager *manager = exynos_crtc->manager; + struct exynos_drm_crtc *exynos_crtc; + struct exynos_drm_manager *manager; + + if (!private->crtc[pipe]) + return; + + exynos_crtc = to_exynos_crtc(private->crtc[pipe]); + manager = exynos_crtc->manager;
if (exynos_crtc->dpms != DRM_MODE_DPMS_ON) return;
dri-devel@lists.freedesktop.org
-
Andrzej Hajda