diff mbox series

[2/2] drm/meson: fix G12A primary plane disabling

Message ID 20190605141253.24165-3-narmstrong@baylibre.com (mailing list archive)
State New, archived
Headers show
Series drm/meson: fix primary plane disabling | expand

Commit Message

Neil Armstrong June 5, 2019, 2:12 p.m. UTC
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(-)

Comments

Kevin Hilman June 6, 2019, 5:30 p.m. UTC | #1
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
Neil Armstrong June 7, 2019, 8:07 a.m. UTC | #2
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
>
diff mbox series

Patch

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));