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 
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 torvalds#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>
  • Loading branch information
@daeinki
Andrzej Hajda authored and daeinki committed Mar 21, 2019
1 parent 9e98c67 commit 6a3b45a
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 6a3b45a

Please sign in to comment.