Skip to content

Commit

Permalink
drm/exynos/mixer: fix MIXER shadow registry synchronisation code
Browse files Browse the repository at this point in the history 
BugLink: https://bugs.launchpad.net/bugs/1830906

[ Upstream commit 6a3b45a ]

MIXER on Exynos5 SoCs uses different synchronisation method than Exynos4
to update internal state (shadow registers).
Apparently the driver implements it incorrectly. The rule should be
as follows:
- do not request updating registers until previous request was finished,
  ie. MXR_CFG_LAYER_UPDATE_COUNT must be 0.
- before setting registers synchronisation on VSYNC should be turned off,
  ie. MXR_STATUS_SYNC_ENABLE should be reset,
- after finishing MXR_STATUS_SYNC_ENABLE should be set again.
The patch hopefully implements it correctly.
Below sample kernel log from page fault caused by the bug:

[   25.670038] exynos-sysmmu 14650000.sysmmu: 14450000.mixer: PAGE FAULT occurred at 0x2247b800
[   25.677888] ------------[ cut here ]------------
[   25.682164] kernel BUG at ../drivers/iommu/exynos-iommu.c:450!
[   25.687971] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
[   25.693778] Modules linked in:
[   25.696816] CPU: 5 PID: 1553 Comm: fb-release_test Not tainted 5.0.0-rc7-01157-g5f86b1566bdd #136
[   25.705646] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   25.711710] PC is at exynos_sysmmu_irq+0x1c0/0x264
[   25.716470] LR is at lock_is_held_type+0x44/0x64

v2: added missing MXR_CFG_LAYER_UPDATE bit setting in mixer_enable_sync

Reported-by: Marian Mihailescu <mihailescu2m@gmail.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
  • Loading branch information
@smb49
Andrzej Hajda authored and smb49 committed Jul 2, 2019
1 parent de12dd1 commit 6abd50f
Showing 1 changed file with 66 additions and 44 deletions.
110 changes: 66 additions & 44 deletions drivers/gpu/drm/exynos/exynos_mixer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "regs-vp.h"

#include <linux/kernel.h>
#include <linux/ktime.h>
#include <linux/spinlock.h>
#include <linux/wait.h>
#include <linux/i2c.h>
Expand Down Expand Up @@ -352,15 +353,62 @@ static void mixer_cfg_vp_blend(struct mixer_context *ctx, unsigned int alpha)
mixer_reg_write(ctx, MXR_VIDEO_CFG, val);
}

static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
static bool mixer_is_synced(struct mixer_context *ctx)
{
/* block update on vsync */
mixer_reg_writemask(ctx, MXR_STATUS, enable ?
MXR_STATUS_SYNC_ENABLE : 0, MXR_STATUS_SYNC_ENABLE);
u32 base, shadow;

if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
ctx->mxr_ver == MXR_VER_128_0_0_184)
return !(mixer_reg_read(ctx, MXR_CFG) &
MXR_CFG_LAYER_UPDATE_COUNT_MASK);

if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags) &&
vp_reg_read(ctx, VP_SHADOW_UPDATE))
return false;

base = mixer_reg_read(ctx, MXR_CFG);
shadow = mixer_reg_read(ctx, MXR_CFG_S);
if (base != shadow)
return false;

base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(0));
shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(0));
if (base != shadow)
return false;

base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(1));
shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(1));
if (base != shadow)
return false;

return true;
}

static int mixer_wait_for_sync(struct mixer_context *ctx)
{
ktime_t timeout = ktime_add_us(ktime_get(), 100000);

while (!mixer_is_synced(ctx)) {
usleep_range(1000, 2000);
if (ktime_compare(ktime_get(), timeout) > 0)
return -ETIMEDOUT;
}
return 0;
}

static void mixer_disable_sync(struct mixer_context *ctx)
{
mixer_reg_writemask(ctx, MXR_STATUS, 0, MXR_STATUS_SYNC_ENABLE);
}

static void mixer_enable_sync(struct mixer_context *ctx)
{
if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
ctx->mxr_ver == MXR_VER_128_0_0_184)
mixer_reg_writemask(ctx, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
mixer_reg_writemask(ctx, MXR_STATUS, ~0, MXR_STATUS_SYNC_ENABLE);
if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags))
vp_reg_write(ctx, VP_SHADOW_UPDATE, enable ?
VP_SHADOW_UPDATE_ENABLE : 0);
vp_reg_write(ctx, VP_SHADOW_UPDATE, VP_SHADOW_UPDATE_ENABLE);
}

static void mixer_cfg_scan(struct mixer_context *ctx, int width, int height)
Expand Down Expand Up @@ -498,7 +546,6 @@ static void vp_video_buffer(struct mixer_context *ctx,

spin_lock_irqsave(&ctx->reg_slock, flags);

vp_reg_write(ctx, VP_SHADOW_UPDATE, 1);
/* interlace or progressive scan mode */
val = (test_bit(MXR_BIT_INTERLACE, &ctx->flags) ? ~0 : 0);
vp_reg_writemask(ctx, VP_MODE, val, VP_MODE_LINE_SKIP);
Expand Down Expand Up @@ -553,11 +600,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
vp_regs_dump(ctx);
}

static void mixer_layer_update(struct mixer_context *ctx)
{
mixer_reg_writemask(ctx, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
}

static void mixer_graph_buffer(struct mixer_context *ctx,
struct exynos_drm_plane *plane)
{
Expand Down Expand Up @@ -640,11 +682,6 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
mixer_cfg_layer(ctx, win, priority, true);
mixer_cfg_gfx_blend(ctx, win, pixel_alpha, state->base.alpha);

/* layer update mandatory for mixer 16.0.33.0 */
if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
ctx->mxr_ver == MXR_VER_128_0_0_184)
mixer_layer_update(ctx);

spin_unlock_irqrestore(&ctx->reg_slock, flags);

mixer_regs_dump(ctx);
Expand Down Expand Up @@ -709,7 +746,7 @@ static void mixer_win_reset(struct mixer_context *ctx)
static irqreturn_t mixer_irq_handler(int irq, void *arg)
{
struct mixer_context *ctx = arg;
u32 val, base, shadow;
u32 val;

spin_lock(&ctx->reg_slock);

Expand All @@ -723,26 +760,9 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
val &= ~MXR_INT_STATUS_VSYNC;

/* interlace scan need to check shadow register */
if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)) {
if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags) &&
vp_reg_read(ctx, VP_SHADOW_UPDATE))
goto out;

base = mixer_reg_read(ctx, MXR_CFG);
shadow = mixer_reg_read(ctx, MXR_CFG_S);
if (base != shadow)
goto out;

base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(0));
shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(0));
if (base != shadow)
goto out;

base = mixer_reg_read(ctx, MXR_GRAPHIC_BASE(1));
shadow = mixer_reg_read(ctx, MXR_GRAPHIC_BASE_S(1));
if (base != shadow)
goto out;
}
if (test_bit(MXR_BIT_INTERLACE, &ctx->flags)
&& !mixer_is_synced(ctx))
goto out;

drm_crtc_handle_vblank(&ctx->crtc->base);
}
Expand Down Expand Up @@ -917,12 +937,14 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)

static void mixer_atomic_begin(struct exynos_drm_crtc *crtc)
{
struct mixer_context *mixer_ctx = crtc->ctx;
struct mixer_context *ctx = crtc->ctx;

if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
if (!test_bit(MXR_BIT_POWERED, &ctx->flags))
return;

mixer_vsync_set_update(mixer_ctx, false);
if (mixer_wait_for_sync(ctx))
dev_err(ctx->dev, "timeout waiting for VSYNC\n");
mixer_disable_sync(ctx);
}

static void mixer_update_plane(struct exynos_drm_crtc *crtc,
Expand Down Expand Up @@ -964,7 +986,7 @@ static void mixer_atomic_flush(struct exynos_drm_crtc *crtc)
if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
return;

mixer_vsync_set_update(mixer_ctx, true);
mixer_enable_sync(mixer_ctx);
exynos_crtc_handle_event(crtc);
}

Expand All @@ -979,7 +1001,7 @@ static void mixer_enable(struct exynos_drm_crtc *crtc)

exynos_drm_pipe_clk_enable(crtc, true);

mixer_vsync_set_update(ctx, false);
mixer_disable_sync(ctx);

mixer_reg_writemask(ctx, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);

Expand All @@ -992,7 +1014,7 @@ static void mixer_enable(struct exynos_drm_crtc *crtc)

mixer_commit(ctx);

mixer_vsync_set_update(ctx, true);
mixer_enable_sync(ctx);

set_bit(MXR_BIT_POWERED, &ctx->flags);
}
Expand Down

0 comments on commit 6abd50f

Please sign in to comment.