* [PATCH 0/2] drm/meson: fix primary plane disabling @ 2019-06-05 14:12 Neil Armstrong 2019-06-05 14:12 ` [PATCH 1/2] " Neil Armstrong 2019-06-05 14:12 ` [PATCH 2/2] drm/meson: fix G12A " Neil Armstrong 0 siblings, 2 replies; 7+ messages in thread From: Neil Armstrong @ 2019-06-05 14:12 UTC (permalink / raw) To: dri-devel; +Cc: linux-amlogic, linux-arm-kernel, linux-kernel, Neil Armstrong The primary plane disabling logic is broken on all supported Amlogic SoCs, and the G12A primary plane disable register write is wrong. This patchset solves thse issues, and has been tested with the Baylibre ffmpeg-drm tool and modetest. Neil Armstrong (2): drm/meson: fix primary plane disabling drm/meson: fix G12A primary plane disabling drivers/gpu/drm/meson/meson_crtc.c | 6 ++---- drivers/gpu/drm/meson/meson_plane.c | 8 +++++--- drivers/gpu/drm/meson/meson_viu.c | 3 +-- 3 files changed, 8 insertions(+), 9 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] drm/meson: fix primary plane disabling 2019-06-05 14:12 [PATCH 0/2] drm/meson: fix primary plane disabling Neil Armstrong @ 2019-06-05 14:12 ` Neil Armstrong 2019-06-06 17:25 ` Kevin Hilman 2019-06-05 14:12 ` [PATCH 2/2] drm/meson: fix G12A " Neil Armstrong 1 sibling, 1 reply; 7+ messages in thread From: Neil Armstrong @ 2019-06-05 14:12 UTC (permalink / raw) To: dri-devel; +Cc: linux-amlogic, linux-arm-kernel, linux-kernel, Neil Armstrong The primary plane disable logic is flawed, when the primary plane is disabled, it is re-enabled in the vsync irq when another plane is updated. Handle the plane disabling correctly by handling the primary plane enable flag in the primary plane update & disable callbacks. Fixes: 490f50c109d1 ("drm/meson: Add G12A support for OSD1 Plane") Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/gpu/drm/meson/meson_crtc.c | 4 ---- drivers/gpu/drm/meson/meson_plane.c | 4 +++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c index 685715144156..50a9a96720b9 100644 --- a/drivers/gpu/drm/meson/meson_crtc.c +++ b/drivers/gpu/drm/meson/meson_crtc.c @@ -107,8 +107,6 @@ static void meson_g12a_crtc_atomic_enable(struct drm_crtc *crtc, priv->io_base + _REG(VPP_OUT_H_V_SIZE)); drm_crtc_vblank_on(crtc); - - priv->viu.osd1_enabled = true; } static void meson_crtc_atomic_enable(struct drm_crtc *crtc, @@ -137,8 +135,6 @@ static void meson_crtc_atomic_enable(struct drm_crtc *crtc, priv->io_base + _REG(VPP_MISC)); drm_crtc_vblank_on(crtc); - - priv->viu.osd1_enabled = true; } static void meson_g12a_crtc_atomic_disable(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c index 22490047932e..b788280895c6 100644 --- a/drivers/gpu/drm/meson/meson_plane.c +++ b/drivers/gpu/drm/meson/meson_plane.c @@ -305,6 +305,8 @@ static void meson_plane_atomic_update(struct drm_plane *plane, meson_plane->enabled = true; } + priv->viu.osd1_enabled = true; + spin_unlock_irqrestore(&priv->drm->event_lock, flags); } @@ -323,7 +325,7 @@ static void meson_plane_atomic_disable(struct drm_plane *plane, priv->io_base + _REG(VPP_MISC)); meson_plane->enabled = false; - + priv->viu.osd1_enabled = false; } static const struct drm_plane_helper_funcs meson_plane_helper_funcs = { -- 2.21.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/meson: fix primary plane disabling 2019-06-05 14:12 ` [PATCH 1/2] " Neil Armstrong @ 2019-06-06 17:25 ` Kevin Hilman 2019-06-07 8:07 ` Neil Armstrong 0 siblings, 1 reply; 7+ messages in thread From: Kevin Hilman @ 2019-06-06 17:25 UTC (permalink / raw) To: Neil Armstrong, dri-devel Cc: linux-amlogic, linux-kernel, linux-arm-kernel, Neil Armstrong Neil Armstrong <narmstrong@baylibre.com> writes: > The primary plane disable logic is flawed, when the primary plane is > disabled, it is re-enabled in the vsync irq when another plane is updated. > > Handle the plane disabling correctly by handling the primary plane > enable flag in the primary plane update & disable callbacks. > > Fixes: 490f50c109d1 ("drm/meson: Add G12A support for OSD1 Plane") > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> Reviewed-by: Kevin Hilman <khilman@baylibre.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/meson: fix primary plane disabling 2019-06-06 17:25 ` Kevin Hilman @ 2019-06-07 8:07 ` Neil Armstrong 0 siblings, 0 replies; 7+ messages in thread From: Neil Armstrong @ 2019-06-07 8:07 UTC (permalink / raw) To: Kevin Hilman, dri-devel; +Cc: linux-amlogic, linux-kernel, linux-arm-kernel On 06/06/2019 19:25, Kevin Hilman wrote: > Neil Armstrong <narmstrong@baylibre.com> writes: > >> The primary plane disable logic is flawed, when the primary plane is >> disabled, it is re-enabled in the vsync irq when another plane is updated. >> >> Handle the plane disabling correctly by handling the primary plane >> enable flag in the primary plane update & disable callbacks. >> >> Fixes: 490f50c109d1 ("drm/meson: Add G12A support for OSD1 Plane") >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > > Reviewed-by: Kevin Hilman <khilman@baylibre.com> > Applying to drm-misc-fixes Thanks, Neil ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/meson: fix G12A primary plane disabling 2019-06-05 14:12 [PATCH 0/2] drm/meson: fix primary plane disabling Neil Armstrong 2019-06-05 14:12 ` [PATCH 1/2] " Neil Armstrong @ 2019-06-05 14:12 ` Neil Armstrong 2019-06-06 17:30 ` Kevin Hilman 1 sibling, 1 reply; 7+ messages in thread From: Neil Armstrong @ 2019-06-05 14:12 UTC (permalink / raw) To: dri-devel; +Cc: linux-amlogic, linux-arm-kernel, linux-kernel, Neil Armstrong The G12A Primary plane was disabled by writing in the OSD1 configuration registers, but this caused the plane blender to stall instead of continuing blended only the overlay plane. Fix this by disabling the OSD1 plane in the blender registers, and also enabling it back using the same register. Fixes: 490f50c109d1 ("drm/meson: Add G12A support for OSD1 Plane") Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/gpu/drm/meson/meson_crtc.c | 2 ++ drivers/gpu/drm/meson/meson_plane.c | 4 ++-- drivers/gpu/drm/meson/meson_viu.c | 3 +-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c index 50a9a96720b9..aa8ea107524e 100644 --- a/drivers/gpu/drm/meson/meson_crtc.c +++ b/drivers/gpu/drm/meson/meson_crtc.c @@ -252,6 +252,8 @@ static void meson_g12a_crtc_enable_osd1(struct meson_drm *priv) writel_relaxed(priv->viu.osb_blend1_size, priv->io_base + _REG(VIU_OSD_BLEND_BLEND1_SIZE)); + writel_bits_relaxed(3 << 8, 3 << 8, + priv->io_base + _REG(OSD1_BLEND_SRC_CTRL)); } static void meson_crtc_enable_vd1(struct meson_drm *priv) diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c index b788280895c6..d90427b93a51 100644 --- a/drivers/gpu/drm/meson/meson_plane.c +++ b/drivers/gpu/drm/meson/meson_plane.c @@ -318,8 +318,8 @@ static void meson_plane_atomic_disable(struct drm_plane *plane, /* Disable OSD1 */ if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) - writel_bits_relaxed(BIT(0) | BIT(21), 0, - priv->io_base + _REG(VIU_OSD1_CTRL_STAT)); + writel_bits_relaxed(3 << 8, 0, + priv->io_base + _REG(OSD1_BLEND_SRC_CTRL)); else writel_bits_relaxed(VPP_OSD1_POSTBLEND, 0, priv->io_base + _REG(VPP_MISC)); diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c index 462c7cb3e1bd..4b2b3024d371 100644 --- a/drivers/gpu/drm/meson/meson_viu.c +++ b/drivers/gpu/drm/meson/meson_viu.c @@ -405,8 +405,7 @@ void meson_viu_init(struct meson_drm *priv) 0 << 16 | 1, priv->io_base + _REG(VIU_OSD_BLEND_CTRL)); - writel_relaxed(3 << 8 | - 1 << 20, + writel_relaxed(1 << 20, priv->io_base + _REG(OSD1_BLEND_SRC_CTRL)); writel_relaxed(1 << 20, priv->io_base + _REG(OSD2_BLEND_SRC_CTRL)); -- 2.21.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/meson: fix G12A primary plane disabling 2019-06-05 14:12 ` [PATCH 2/2] drm/meson: fix G12A " Neil Armstrong @ 2019-06-06 17:30 ` Kevin Hilman 2019-06-07 8:07 ` Neil Armstrong 0 siblings, 1 reply; 7+ messages in thread From: Kevin Hilman @ 2019-06-06 17:30 UTC (permalink / raw) To: Neil Armstrong, dri-devel Cc: linux-amlogic, linux-kernel, linux-arm-kernel, Neil Armstrong Neil Armstrong <narmstrong@baylibre.com> writes: > The G12A Primary plane was disabled by writing in the OSD1 configuration > registers, but this caused the plane blender to stall instead of continuing > blended only the overlay plane. grammar nit: "...instead of continuing to blend only the overlay plane." > Fix this by disabling the OSD1 plane in the blender registers, and also > enabling it back using the same register. > > Fixes: 490f50c109d1 ("drm/meson: Add G12A support for OSD1 Plane") > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> Reviewed-by: Kevin Hilman <khilman@baylibre.com> As noted elsewhere, this driver is also full of magic constants used in register writes which makes reviewing this kind of change for correctness that much more difficult, but since that's already been pointed out elsewhere, and it's already on your TODO list, it should not block this important fix. Kevin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/meson: fix G12A primary plane disabling 2019-06-06 17:30 ` Kevin Hilman @ 2019-06-07 8:07 ` Neil Armstrong 0 siblings, 0 replies; 7+ messages in thread From: Neil Armstrong @ 2019-06-07 8:07 UTC (permalink / raw) To: Kevin Hilman, dri-devel; +Cc: linux-amlogic, linux-kernel, linux-arm-kernel On 06/06/2019 19:30, Kevin Hilman wrote: > Neil Armstrong <narmstrong@baylibre.com> writes: > >> The G12A Primary plane was disabled by writing in the OSD1 configuration >> registers, but this caused the plane blender to stall instead of continuing >> blended only the overlay plane. > > grammar nit: "...instead of continuing to blend only the overlay plane." Fixed while applying on drm-misc-fixes > >> Fix this by disabling the OSD1 plane in the blender registers, and also >> enabling it back using the same register. >> >> Fixes: 490f50c109d1 ("drm/meson: Add G12A support for OSD1 Plane") >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > > Reviewed-by: Kevin Hilman <khilman@baylibre.com> > > As noted elsewhere, this driver is also full of magic constants used in > register writes which makes reviewing this kind of change for > correctness that much more difficult, but since that's already been > pointed out elsewhere, and it's already on your TODO list, it should not > block this important fix. Yep, it's the top priority now. Thanks, Neil > > Kevin > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-06-07 8:07 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-05 14:12 [PATCH 0/2] drm/meson: fix primary plane disabling Neil Armstrong 2019-06-05 14:12 ` [PATCH 1/2] " Neil Armstrong 2019-06-06 17:25 ` Kevin Hilman 2019-06-07 8:07 ` Neil Armstrong 2019-06-05 14:12 ` [PATCH 2/2] drm/meson: fix G12A " Neil Armstrong 2019-06-06 17:30 ` Kevin Hilman 2019-06-07 8:07 ` Neil Armstrong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).