[PATCH 0/4] drm/fsl-dcu: add overlay and cursor plane support

[PATCH 0/4] drm/fsl-dcu: add overlay and cursor plane support

[Stefan Agner]

This patchset adds overlay and cursor plane support. It also
fixes some issues uncovered during implementation of this.

However, the plane updates currently causes the display to flicker for
unknown reasons. As far as I can tell, the CRTC atomic_flush should
trigger the update correctly via READREG, which according to
documentation:
The READREG bit causes a single transfer to begin at the next frame
blanking period. This bit is cleared when the transfer is complete.

I made a video how that looks:
https://cloud.agner.ch/index.php/s/Yfqa2u7UBEWUT8N

Any ideas?

Stefan Agner (4):
  drm/fsl-dcu: support overlay and cursor planes
  drm/fsl-dcu: respect pos/size register sizes
  drm/fsl-dcu: update all registers on flush
  drm/fsl-dcu: do not update when modifying irq registers

 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 50 ++++++++++++++++++++++++-----
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   |  4 ---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  8 ++---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 42 +++++++++++-------------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
 5 files changed, 67 insertions(+), 40 deletions(-)

-- 
2.9.3

[PATCH 0/4] drm/fsl-dcu: add overlay and cursor plane support

[Stefan Agner]

This patchset adds overlay and cursor plane support. It also
fixes some issues uncovered during implementation of this.

However, the plane updates currently causes the display to flicker for
unknown reasons. As far as I can tell, the CRTC atomic_flush should
trigger the update correctly via READREG, which according to
documentation:
The READREG bit causes a single transfer to begin at the next frame
blanking period. This bit is cleared when the transfer is complete.

I made a video how that looks:
https://cloud.agner.ch/index.php/s/Yfqa2u7UBEWUT8N

Any ideas?

Stefan Agner (4):
  drm/fsl-dcu: support overlay and cursor planes
  drm/fsl-dcu: respect pos/size register sizes
  drm/fsl-dcu: update all registers on flush
  drm/fsl-dcu: do not update when modifying irq registers

 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 50 ++++++++++++++++++++++++-----
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   |  4 ---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  8 ++---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 42 +++++++++++-------------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
 5 files changed, 67 insertions(+), 40 deletions(-)

-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/fsl-dcu: support overlay and cursor planes

[Stefan Agner]

Add support for overlay plane and a cursor plane. The driver uses
the topmost plane as cursor plane. The DCU IP would have dedicated
cursor support, but that lacks proper color support and hence is
not practical to use for Linux systems.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 47 ++++++++++++++++++++++++-----
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 37 ++++++++++++-----------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
 3 files changed, 60 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 3371635..d30b61e 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -139,20 +139,51 @@ static const struct drm_crtc_funcs fsl_dcu_drm_crtc_funcs = {
 
 int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
 {
-	struct drm_plane *primary;
+	struct drm_device *drm = fsl_dev->drm;
+	struct drm_plane *primary, *cursor;
 	struct drm_crtc *crtc = &fsl_dev->crtc;
-	int ret;
+	int total_layer = fsl_dev->soc->total_layer;
+	int ret, i;
 
-	fsl_dcu_drm_init_planes(fsl_dev->drm);
+	fsl_dcu_drm_init_planes(drm);
 
-	primary = fsl_dcu_drm_primary_create_plane(fsl_dev->drm);
-	if (!primary)
-		return -ENOMEM;
+	primary = fsl_dcu_drm_create_plane(drm, DRM_PLANE_TYPE_PRIMARY);
+	if (IS_ERR(primary)) {
+		dev_err(fsl_dev->dev, "failed to construct primary plane\n");
+		ret = PTR_ERR(primary);
+		return ret;
+	}
+
+	/*
+	 * Initialize overlay layers. The hardware does not have specific
+	 * layer types, we just happen to use one layer as primary layer
+	 * and one layer as cursor layer, hence total_layer - 2 = overlays.
+	 */
+	for (i = 0; i < total_layer - 2; i++) {
+		struct drm_plane *plane =
+			fsl_dcu_drm_create_plane(drm, DRM_PLANE_TYPE_OVERLAY);
+
+		if (IS_ERR(plane))
+			continue;
+	}
 
-	ret = drm_crtc_init_with_planes(fsl_dev->drm, crtc, primary, NULL,
+	cursor = fsl_dcu_drm_create_plane(drm, DRM_PLANE_TYPE_CURSOR);
+	if (IS_ERR(cursor)) {
+		dev_warn(fsl_dev->dev, "failed to construct cursor plane\n");
+		cursor = NULL;
+	}
+
+	/*
+	 * Initialize cursor plane after overlay planes since the initialization
+	 * order is crucial to the layer id (and hence layer stacking order).
+	 */
+	ret = drm_crtc_init_with_planes(drm, crtc, primary, cursor,
 					&fsl_dcu_drm_crtc_funcs, NULL);
 	if (ret) {
-		primary->funcs->destroy(primary);
+		struct drm_plane *plane;
+
+		list_for_each_entry(plane, &drm->mode_config.plane_list, head)
+			drm_plane_cleanup(plane);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index a7e5486..a6af3d9 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -178,7 +178,6 @@ static const struct drm_plane_helper_funcs fsl_dcu_drm_plane_helper_funcs = {
 static void fsl_dcu_drm_plane_destroy(struct drm_plane *plane)
 {
 	drm_plane_cleanup(plane);
-	kfree(plane);
 }
 
 static const struct drm_plane_funcs fsl_dcu_drm_plane_funcs = {
@@ -218,28 +217,30 @@ void fsl_dcu_drm_init_planes(struct drm_device *dev)
 		     DCU_UPDATE_MODE_READREG);
 }
 
-struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
+struct drm_plane *fsl_dcu_drm_create_plane(struct drm_device *dev,
+					   enum drm_plane_type type)
 {
-	struct drm_plane *primary;
+	struct drm_plane *plane;
 	int ret;
 
-	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
-	if (!primary) {
-		DRM_DEBUG_KMS("Failed to allocate primary plane\n");
-		return NULL;
-	}
+	plane = devm_kzalloc(dev->dev, sizeof(struct drm_plane),
+			      GFP_KERNEL);
+	if (!plane)
+		return ERR_PTR(-ENOMEM);
 
-	/* possible_crtc's will be filled in later by crtc_init */
-	ret = drm_universal_plane_init(dev, primary, 0,
-				       &fsl_dcu_drm_plane_funcs,
+	ret = drm_universal_plane_init(dev, plane, 1, &fsl_dcu_drm_plane_funcs,
 				       fsl_dcu_drm_plane_formats,
 				       ARRAY_SIZE(fsl_dcu_drm_plane_formats),
-				       DRM_PLANE_TYPE_PRIMARY, NULL);
-	if (ret) {
-		kfree(primary);
-		primary = NULL;
-	}
-	drm_plane_helper_add(primary, &fsl_dcu_drm_plane_helper_funcs);
+				       type, NULL);
+	if (ret)
+		goto err_cleanup_plane;
+
+	drm_plane_helper_add(plane, &fsl_dcu_drm_plane_helper_funcs);
+
+	return plane;
+
+err_cleanup_plane:
+	drm_plane_cleanup(plane);
 
-	return primary;
+	return ERR_PTR(ret);
 }
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
index 8ee45f8..bd36166 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
@@ -13,6 +13,7 @@
 #define __FSL_DCU_DRM_PLANE_H__
 
 void fsl_dcu_drm_init_planes(struct drm_device *dev);
-struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev);
+struct drm_plane *fsl_dcu_drm_create_plane(struct drm_device *dev,
+					   enum drm_plane_type type);
 
 #endif /* __FSL_DCU_DRM_PLANE_H__ */
-- 
2.9.3

[PATCH 1/4] drm/fsl-dcu: support overlay and cursor planes

[Stefan Agner]

Add support for overlay plane and a cursor plane. The driver uses
the topmost plane as cursor plane. The DCU IP would have dedicated
cursor support, but that lacks proper color support and hence is
not practical to use for Linux systems.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 47 ++++++++++++++++++++++++-----
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 37 ++++++++++++-----------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
 3 files changed, 60 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 3371635..d30b61e 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -139,20 +139,51 @@ static const struct drm_crtc_funcs fsl_dcu_drm_crtc_funcs = {
 
 int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
 {
-	struct drm_plane *primary;
+	struct drm_device *drm = fsl_dev->drm;
+	struct drm_plane *primary, *cursor;
 	struct drm_crtc *crtc = &fsl_dev->crtc;
-	int ret;
+	int total_layer = fsl_dev->soc->total_layer;
+	int ret, i;
 
-	fsl_dcu_drm_init_planes(fsl_dev->drm);
+	fsl_dcu_drm_init_planes(drm);
 
-	primary = fsl_dcu_drm_primary_create_plane(fsl_dev->drm);
-	if (!primary)
-		return -ENOMEM;
+	primary = fsl_dcu_drm_create_plane(drm, DRM_PLANE_TYPE_PRIMARY);
+	if (IS_ERR(primary)) {
+		dev_err(fsl_dev->dev, "failed to construct primary plane\n");
+		ret = PTR_ERR(primary);
+		return ret;
+	}
+
+	/*
+	 * Initialize overlay layers. The hardware does not have specific
+	 * layer types, we just happen to use one layer as primary layer
+	 * and one layer as cursor layer, hence total_layer - 2 = overlays.
+	 */
+	for (i = 0; i < total_layer - 2; i++) {
+		struct drm_plane *plane =
+			fsl_dcu_drm_create_plane(drm, DRM_PLANE_TYPE_OVERLAY);
+
+		if (IS_ERR(plane))
+			continue;
+	}
 
-	ret = drm_crtc_init_with_planes(fsl_dev->drm, crtc, primary, NULL,
+	cursor = fsl_dcu_drm_create_plane(drm, DRM_PLANE_TYPE_CURSOR);
+	if (IS_ERR(cursor)) {
+		dev_warn(fsl_dev->dev, "failed to construct cursor plane\n");
+		cursor = NULL;
+	}
+
+	/*
+	 * Initialize cursor plane after overlay planes since the initialization
+	 * order is crucial to the layer id (and hence layer stacking order).
+	 */
+	ret = drm_crtc_init_with_planes(drm, crtc, primary, cursor,
 					&fsl_dcu_drm_crtc_funcs, NULL);
 	if (ret) {
-		primary->funcs->destroy(primary);
+		struct drm_plane *plane;
+
+		list_for_each_entry(plane, &drm->mode_config.plane_list, head)
+			drm_plane_cleanup(plane);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index a7e5486..a6af3d9 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -178,7 +178,6 @@ static const struct drm_plane_helper_funcs fsl_dcu_drm_plane_helper_funcs = {
 static void fsl_dcu_drm_plane_destroy(struct drm_plane *plane)
 {
 	drm_plane_cleanup(plane);
-	kfree(plane);
 }
 
 static const struct drm_plane_funcs fsl_dcu_drm_plane_funcs = {
@@ -218,28 +217,30 @@ void fsl_dcu_drm_init_planes(struct drm_device *dev)
 		     DCU_UPDATE_MODE_READREG);
 }
 
-struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
+struct drm_plane *fsl_dcu_drm_create_plane(struct drm_device *dev,
+					   enum drm_plane_type type)
 {
-	struct drm_plane *primary;
+	struct drm_plane *plane;
 	int ret;
 
-	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
-	if (!primary) {
-		DRM_DEBUG_KMS("Failed to allocate primary plane\n");
-		return NULL;
-	}
+	plane = devm_kzalloc(dev->dev, sizeof(struct drm_plane),
+			      GFP_KERNEL);
+	if (!plane)
+		return ERR_PTR(-ENOMEM);
 
-	/* possible_crtc's will be filled in later by crtc_init */
-	ret = drm_universal_plane_init(dev, primary, 0,
-				       &fsl_dcu_drm_plane_funcs,
+	ret = drm_universal_plane_init(dev, plane, 1, &fsl_dcu_drm_plane_funcs,
 				       fsl_dcu_drm_plane_formats,
 				       ARRAY_SIZE(fsl_dcu_drm_plane_formats),
-				       DRM_PLANE_TYPE_PRIMARY, NULL);
-	if (ret) {
-		kfree(primary);
-		primary = NULL;
-	}
-	drm_plane_helper_add(primary, &fsl_dcu_drm_plane_helper_funcs);
+				       type, NULL);
+	if (ret)
+		goto err_cleanup_plane;
+
+	drm_plane_helper_add(plane, &fsl_dcu_drm_plane_helper_funcs);
+
+	return plane;
+
+err_cleanup_plane:
+	drm_plane_cleanup(plane);
 
-	return primary;
+	return ERR_PTR(ret);
 }
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
index 8ee45f8..bd36166 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
@@ -13,6 +13,7 @@
 #define __FSL_DCU_DRM_PLANE_H__
 
 void fsl_dcu_drm_init_planes(struct drm_device *dev);
-struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev);
+struct drm_plane *fsl_dcu_drm_create_plane(struct drm_device *dev,
+					   enum drm_plane_type type);
 
 #endif /* __FSL_DCU_DRM_PLANE_H__ */
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/fsl-dcu: respect pos/size register sizes

[Stefan Agner]

Mask the size and position values to avoid mutual overwriting.
Especially, a negative X position caused the Y position to be
overwritten with 0xfff too. This has been observed when using
a layer as cursor layer.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
index 3b371fe7..060f881 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -118,11 +118,11 @@
 
 #define DCU_CTRLDESCLN(layer, reg)	(0x200 + (reg - 1) * 4 + (layer) * 0x40)
 
-#define DCU_LAYER_HEIGHT(x)		((x) << 16)
-#define DCU_LAYER_WIDTH(x)		(x)
+#define DCU_LAYER_HEIGHT(x)		(((x) & 0x7ff) << 16)
+#define DCU_LAYER_WIDTH(x)		((x) & 0x7ff)
 
-#define DCU_LAYER_POSY(x)		((x) << 16)
-#define DCU_LAYER_POSX(x)		(x)
+#define DCU_LAYER_POSY(x)		(((x) & 0xfff) << 16)
+#define DCU_LAYER_POSX(x)		((x) & 0xfff)
 
 #define DCU_LAYER_EN			BIT(31)
 #define DCU_LAYER_TILE_EN		BIT(30)
-- 
2.9.3

[PATCH 2/4] drm/fsl-dcu: respect pos/size register sizes

[Stefan Agner]

Mask the size and position values to avoid mutual overwriting.
Especially, a negative X position caused the Y position to be
overwritten with 0xfff too. This has been observed when using
a layer as cursor layer.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
index 3b371fe7..060f881 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -118,11 +118,11 @@
 
 #define DCU_CTRLDESCLN(layer, reg)	(0x200 + (reg - 1) * 4 + (layer) * 0x40)
 
-#define DCU_LAYER_HEIGHT(x)		((x) << 16)
-#define DCU_LAYER_WIDTH(x)		(x)
+#define DCU_LAYER_HEIGHT(x)		(((x) & 0x7ff) << 16)
+#define DCU_LAYER_WIDTH(x)		((x) & 0x7ff)
 
-#define DCU_LAYER_POSY(x)		((x) << 16)
-#define DCU_LAYER_POSX(x)		(x)
+#define DCU_LAYER_POSY(x)		(((x) & 0xfff) << 16)
+#define DCU_LAYER_POSX(x)		((x) & 0xfff)
 
 #define DCU_LAYER_EN			BIT(31)
 #define DCU_LAYER_TILE_EN		BIT(30)
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/fsl-dcu: update all registers on flush

[Stefan Agner]

Use the UPDATE_MODE READREG bit to initiate a register transfer
on flush. This makes sure that we flush all registers only once
for all planes.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 3 +++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 -----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index d30b61e..62eb284 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -27,6 +27,9 @@ static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 {
 	struct drm_pending_vblank_event *event = crtc->state->event;
 
+	regmap_write(fsl_dev->regmap,
+		     DCU_UPDATE_MODE, DCU_UPDATE_MODE_READREG);
+
 	if (event) {
 		crtc->state->event = NULL;
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index a6af3d9..d7412ff 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -160,11 +160,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
 			     DCU_LAYER_POST_SKIP(0) |
 			     DCU_LAYER_PRE_SKIP(0));
 	}
-	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
-			   DCU_MODE_DCU_MODE_MASK,
-			   DCU_MODE_DCU_MODE(DCU_MODE_NORMAL));
-	regmap_write(fsl_dev->regmap,
-		     DCU_UPDATE_MODE, DCU_UPDATE_MODE_READREG);
 
 	return;
 }
-- 
2.9.3

[PATCH 3/4] drm/fsl-dcu: update all registers on flush

[Stefan Agner]

Use the UPDATE_MODE READREG bit to initiate a register transfer
on flush. This makes sure that we flush all registers only once
for all planes.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 3 +++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 -----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index d30b61e..62eb284 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -27,6 +27,9 @@ static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 {
 	struct drm_pending_vblank_event *event = crtc->state->event;
 
+	regmap_write(fsl_dev->regmap,
+		     DCU_UPDATE_MODE, DCU_UPDATE_MODE_READREG);
+
 	if (event) {
 		crtc->state->event = NULL;
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index a6af3d9..d7412ff 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -160,11 +160,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
 			     DCU_LAYER_POST_SKIP(0) |
 			     DCU_LAYER_PRE_SKIP(0));
 	}
-	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
-			   DCU_MODE_DCU_MODE_MASK,
-			   DCU_MODE_DCU_MODE(DCU_MODE_NORMAL));
-	regmap_write(fsl_dev->regmap,
-		     DCU_UPDATE_MODE, DCU_UPDATE_MODE_READREG);
 
 	return;
 }
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 3/4] drm/fsl-dcu: update all registers on flush

[Meng Yi]

Hi Stefan,

Some comments below

> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-
> dcu/fsl_dcu_drm_crtc.c
> index d30b61e..62eb284 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> @@ -27,6 +27,9 @@ static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc
> *crtc,  {
>  	struct drm_pending_vblank_event *event = crtc->state->event;
> 
> +	regmap_write(fsl_dev->regmap,
> +		     DCU_UPDATE_MODE, DCU_UPDATE_MODE_READREG);

fsl_dev is not declared.
I had added these two lines when testing the patches.

       struct drm_device *dev = crtc->dev;
       struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;

> +
>  	if (event) {
>  		crtc->state->event = NULL;
> 

Best Regards,
Meng

[PATCH 4/4] drm/fsl-dcu: do not update when modifying irq registers

[Stefan Agner]

The IRQ status and mask registers are not "double buffered" according
to the reference manual. Hence, there is no extra transfer/update
write needed when modifying these registers.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 092aaec..4e700bc4 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -59,8 +59,6 @@ static int fsl_dcu_drm_irq_init(struct drm_device *dev)
 
 	regmap_write(fsl_dev->regmap, DCU_INT_STATUS, 0);
 	regmap_write(fsl_dev->regmap, DCU_INT_MASK, ~0);
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
 
 	return ret;
 }
@@ -139,8 +137,6 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
 		drm_handle_vblank(dev, 0);
 
 	regmap_write(fsl_dev->regmap, DCU_INT_STATUS, int_status);
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
 
 	return IRQ_HANDLED;
 }
-- 
2.9.3

[PATCH 4/4] drm/fsl-dcu: do not update when modifying irq registers

[Stefan Agner]

The IRQ status and mask registers are not "double buffered" according
to the reference manual. Hence, there is no extra transfer/update
write needed when modifying these registers.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 092aaec..4e700bc4 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -59,8 +59,6 @@ static int fsl_dcu_drm_irq_init(struct drm_device *dev)
 
 	regmap_write(fsl_dev->regmap, DCU_INT_STATUS, 0);
 	regmap_write(fsl_dev->regmap, DCU_INT_MASK, ~0);
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
 
 	return ret;
 }
@@ -139,8 +137,6 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
 		drm_handle_vblank(dev, 0);
 
 	regmap_write(fsl_dev->regmap, DCU_INT_STATUS, int_status);
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
 
 	return IRQ_HANDLED;
 }
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 0/4] drm/fsl-dcu: add overlay and cursor plane support

[Meng Yi]

Hi Stefan,

I had tested the patches on LS1021A-TWR board using drmlib.

Like set three overlays:
root@ls1021atwr:~# ./modetest -P 39:900x100+10+10@RG24 -P 39:200x200+300+0@RG24 -P 39:200x200+400+300@RG24

How did you test the overlays and cursor layer, I mean I see you using x-window like thing in the  video.

Best Regards,
Meng

> This patchset adds overlay and cursor plane support. It also fixes some issues
> uncovered during implementation of this.
> 
> However, the plane updates currently causes the display to flicker for unknown
> reasons. As far as I can tell, the CRTC atomic_flush should trigger the update
> correctly via READREG, which according to
> documentation:
> The READREG bit causes a single transfer to begin at the next frame blanking
> period. This bit is cleared when the transfer is complete.
> 
> I made a video how that looks:
> https://cloud.agner.ch/index.php/s/Yfqa2u7UBEWUT8N
> 
> Any ideas?
> 
> Stefan Agner (4):
>   drm/fsl-dcu: support overlay and cursor planes
>   drm/fsl-dcu: respect pos/size register sizes
>   drm/fsl-dcu: update all registers on flush
>   drm/fsl-dcu: do not update when modifying irq registers
> 
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 50
> ++++++++++++++++++++++++-----
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   |  4 ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  8 ++---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 42 +++++++++++-------------
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
>  5 files changed, 67 insertions(+), 40 deletions(-)
> 
> --
> 2.9.3

RE: [PATCH 0/4] drm/fsl-dcu: add overlay and cursor plane support

[Meng Yi]

Hi Stefan,

I had tested the patches on LS1021A-TWR board using drmlib.

Like set three overlays:
root@ls1021atwr:~# ./modetest -P 39:900x100+10+10@RG24 -P 39:200x200+300+0@RG24 -P 39:200x200+400+300@RG24

How did you test the overlays and cursor layer, I mean I see you using x-window like thing in the  video.

Best Regards,
Meng

> This patchset adds overlay and cursor plane support. It also fixes some issues
> uncovered during implementation of this.
> 
> However, the plane updates currently causes the display to flicker for unknown
> reasons. As far as I can tell, the CRTC atomic_flush should trigger the update
> correctly via READREG, which according to
> documentation:
> The READREG bit causes a single transfer to begin at the next frame blanking
> period. This bit is cleared when the transfer is complete.
> 
> I made a video how that looks:
> https://cloud.agner.ch/index.php/s/Yfqa2u7UBEWUT8N
> 
> Any ideas?
> 
> Stefan Agner (4):
>   drm/fsl-dcu: support overlay and cursor planes
>   drm/fsl-dcu: respect pos/size register sizes
>   drm/fsl-dcu: update all registers on flush
>   drm/fsl-dcu: do not update when modifying irq registers
> 
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 50
> ++++++++++++++++++++++++-----
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   |  4 ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  8 ++---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 42 +++++++++++-------------
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
>  5 files changed, 67 insertions(+), 40 deletions(-)
> 
> --
> 2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 0/4] drm/fsl-dcu: add overlay and cursor plane support

[Stefan Agner]

On 2016-09-07 01:43, Meng Yi wrote:
> Hi Stefan,
> 
> I had tested the patches on LS1021A-TWR board using drmlib.
> 
> Like set three overlays:
> root@ls1021atwr:~# ./modetest -P 39:900x100+10+10@RG24 -P
> 39:200x200+300+0@RG24 -P 39:200x200+400+300@RG24
> 
> How did you test the overlays and cursor layer, I mean I see you using
> x-window like thing in the  video.

I did use X with the modesetting driver. You just need to choose
modesetting in your xorg.conf, and it should make use of the cursor
layer automatically.

>> This patchset adds overlay and cursor plane support. It also fixes some issues
>> uncovered during implementation of this.
>>
>> However, the plane updates currently causes the display to flicker for unknown
>> reasons. As far as I can tell, the CRTC atomic_flush should trigger the update
>> correctly via READREG, which according to
>> documentation:
>> The READREG bit causes a single transfer to begin at the next frame blanking
>> period. This bit is cleared when the transfer is complete.
>>
>> I made a video how that looks:
>> https://cloud.agner.ch/index.php/s/Yfqa2u7UBEWUT8N

It would be interesting whether you see that on LS1021a too.

--
Stefan

>>
>> Any ideas?
>>
>> Stefan Agner (4):
>>   drm/fsl-dcu: support overlay and cursor planes
>>   drm/fsl-dcu: respect pos/size register sizes
>>   drm/fsl-dcu: update all registers on flush
>>   drm/fsl-dcu: do not update when modifying irq registers
>>
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 50
>> ++++++++++++++++++++++++-----
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   |  4 ---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  8 ++---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 42 +++++++++++-------------
>> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
>>  5 files changed, 67 insertions(+), 40 deletions(-)
>>
>> --
>> 2.9.3

RE: [PATCH 0/4] drm/fsl-dcu: add overlay and cursor plane support

[Stefan Agner]

On 2016-09-07 01:43, Meng Yi wrote:
> Hi Stefan,
> 
> I had tested the patches on LS1021A-TWR board using drmlib.
> 
> Like set three overlays:
> root@ls1021atwr:~# ./modetest -P 39:900x100+10+10@RG24 -P
> 39:200x200+300+0@RG24 -P 39:200x200+400+300@RG24
> 
> How did you test the overlays and cursor layer, I mean I see you using
> x-window like thing in the  video.

I did use X with the modesetting driver. You just need to choose
modesetting in your xorg.conf, and it should make use of the cursor
layer automatically.

>> This patchset adds overlay and cursor plane support. It also fixes some issues
>> uncovered during implementation of this.
>>
>> However, the plane updates currently causes the display to flicker for unknown
>> reasons. As far as I can tell, the CRTC atomic_flush should trigger the update
>> correctly via READREG, which according to
>> documentation:
>> The READREG bit causes a single transfer to begin at the next frame blanking
>> period. This bit is cleared when the transfer is complete.
>>
>> I made a video how that looks:
>> https://cloud.agner.ch/index.php/s/Yfqa2u7UBEWUT8N

It would be interesting whether you see that on LS1021a too.

--
Stefan

>>
>> Any ideas?
>>
>> Stefan Agner (4):
>>   drm/fsl-dcu: support overlay and cursor planes
>>   drm/fsl-dcu: respect pos/size register sizes
>>   drm/fsl-dcu: update all registers on flush
>>   drm/fsl-dcu: do not update when modifying irq registers
>>
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 50
>> ++++++++++++++++++++++++-----
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   |  4 ---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  8 ++---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 42 +++++++++++-------------
>> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
>>  5 files changed, 67 insertions(+), 40 deletions(-)
>>
>> --
>> 2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel