Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1508955 > unrolled thread

[RFC PATCH v2 0/9] Introduce writeback connectors

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH v2 0/9] Introduce writeback connectors Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:00 +0200
    [RFC PATCH v2 4/9] drm: mali-dp: Add RGB writeback formats for DP550/DP650 Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:00 +0200
    [RFC PATCH v2 3/9] drm: mali-dp: Rename malidp_input_format Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:00 +0200
    [RFC PATCH v2 2/9] drm: mali-dp: Clear CVAL when leaving config mode Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:00 +0200
    [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:10 +0200
      Re: [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support Gustavo Padovan <gustavo@padovan.org> - 2016-10-26 23:50 +0200
        Re: [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support Brian Starkey <brian.starkey@arm.com> - 2016-10-27 16:00 +0200
          Re: [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support Daniel Vetter <daniel@ffwll.ch> - 2016-10-27 16:20 +0200
          Re: [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support Gustavo Padovan <gustavo@padovan.org> - 2016-10-27 16:50 +0200
    [RFC PATCH v2 5/9] drm: mali-dp: Add support for writeback on DP550/DP650 Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:10 +0200
    [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:10 +0200
      Re: [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence  operations Daniel Vetter <daniel@ffwll.ch> - 2016-10-26 13:10 +0200
      Re: [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence  operations Gustavo Padovan <gustavo@padovan.org> - 2016-10-26 23:40 +0200
      Re: [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence  operations Gustavo Padovan <gustavo@padovan.org> - 2016-10-26 23:50 +0200
        Re: [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence  operations Brian Starkey <brian.starkey@arm.com> - 2016-10-27 16:30 +0200
    [RFC PATCH v2 6/9] drm: mali-dp: Add writeback connector Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:10 +0200
    [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback connectors Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:10 +0200
      Re: [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback  connectors Daniel Vetter <daniel@ffwll.ch> - 2016-10-26 13:10 +0200
      Re: [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback  connectors Brian Starkey <brian.starkey@arm.com> - 2016-10-26 14:50 +0200
      Re: [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback  connectors Gustavo Padovan <gustavo@padovan.org> - 2016-10-26 23:50 +0200
        Re: [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback  connectors Brian Starkey <brian.starkey@arm.com> - 2016-10-27 17:00 +0200
    [RFC PATCH v2 1/9] drm: Add writeback connector type Brian Starkey <brian.starkey@arm.com> - 2016-10-26 11:10 +0200
      Re: [RFC PATCH v2 1/9] drm: Add writeback connector type Daniel Vetter <daniel@ffwll.ch> - 2016-10-26 13:10 +0200
        Re: [RFC PATCH v2 1/9] drm: Add writeback connector type Brian Starkey <brian.starkey@arm.com> - 2016-10-26 14:50 +0200
          Re: [RFC PATCH v2 1/9] drm: Add writeback connector type Daniel Vetter <daniel@ffwll.ch> - 2016-10-26 15:10 +0200

Page 1 of 2  [1] 2  Next page →

#1508955 — [RFC PATCH v2 0/9] Introduce writeback connectors

Hi,

This is an updated RFC series introducing a new connector type:
 DRM_MODE_CONNECTOR_WRITEBACK
See v1 here: [1]

Writeback connectors are used to expose the memory writeback engines
found in some display controllers, which can write a CRTC's
composition result to a memory buffer.
This is useful e.g. for testing, screen-recording, screenshots,
wireless display, display cloning, memory-to-memory composition.

Writeback connectors are given a WRITEBACK_FB_ID property (which acts
slightly differently to FB_ID, so gets a new name), as well as
PIXEL_FORMATS and PIXEL_FORMATS_SIZE to list the supported writeback
formats, and OUT_FENCE_PTR to be used for out-fences.

The semantics of writeback connectors have been changed significantly
since v1, based largely on Daniel's feedback. Now, a writeback
connector can only be attached to a CRTC if it has a framebuffer
attached and vice-versa. The writeback framebuffer applies only to a
single atomic commit, and userspace can never read back the value of
WRITEBACK_FB_ID. This makes writeback a "one-shot" operation, it must
be re-armed every time it is to be used.

Patch 1 introduces the actual connector type and the infrastructure
around it. Patches 2-6 add a writeback connector for mali-dp.

Patches 7-9 add support for writeback out-fences, based on Gustavo
Padovan's v5 series [2] for adding explicit fencing.

As always, I look forward to any comments.

Thanks,
Brian

[1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1247574.html
[2] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1253822.html

Changes since v1, based on Daniel and Eric's comments:
 - The writeback framebuffer is no longer persistent across commits
 - Removed the client cap, made the connector report disconnected
   instead
 - Added drm_writeback.c for central connector initialization and
   documentation
 - Added support for out-fences
 - Added core checks for writeback connectors, e.g. disallowing
   a framebuffer with no CRTC
 - Mali-DP doesn't require a full modeset to enable/disable the
   writeback connector

---

Brian Starkey (8):
  drm: Add writeback connector type
  drm: mali-dp: Clear CVAL when leaving config mode
  drm: mali-dp: Rename malidp_input_format
  drm: mali-dp: Add RGB writeback formats for DP550/DP650
  drm: mali-dp: Add writeback connector
  drm: atomic: factor out common out-fence operations
  drm: writeback: Add out-fences for writeback connectors
  drm: mali-dp: Add writeback out-fence support

Liviu Dudau (1):
  drm: mali-dp: Add support for writeback on DP550/DP650

 Documentation/gpu/drm-kms.rst       |    9 +
 drivers/gpu/drm/Makefile            |    2 +-
 drivers/gpu/drm/arm/Makefile        |    1 +
 drivers/gpu/drm/arm/malidp_crtc.c   |   21 +++
 drivers/gpu/drm/arm/malidp_drv.c    |   28 +++-
 drivers/gpu/drm/arm/malidp_drv.h    |    7 +
 drivers/gpu/drm/arm/malidp_hw.c     |  149 +++++++++++++----
 drivers/gpu/drm/arm/malidp_hw.h     |   27 ++-
 drivers/gpu/drm/arm/malidp_mw.c     |  313 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/arm/malidp_mw.h     |   28 ++++
 drivers/gpu/drm/arm/malidp_planes.c |    8 +-
 drivers/gpu/drm/arm/malidp_regs.h   |   15 ++
 drivers/gpu/drm/drm_atomic.c        |  218 +++++++++++++++++++++---
 drivers/gpu/drm/drm_atomic_helper.c |    8 +
 drivers/gpu/drm/drm_connector.c     |    4 +-
 drivers/gpu/drm/drm_writeback.c     |  237 ++++++++++++++++++++++++++
 include/drm/drm_atomic.h            |    3 +
 include/drm/drm_connector.h         |   26 +++
 include/drm/drm_crtc.h              |   20 +++
 include/drm/drm_writeback.h         |   21 +++
 include/uapi/drm/drm_mode.h         |    1 +
 21 files changed, 1076 insertions(+), 70 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/malidp_mw.c
 create mode 100644 drivers/gpu/drm/arm/malidp_mw.h
 create mode 100644 drivers/gpu/drm/drm_writeback.c
 create mode 100644 include/drm/drm_writeback.h

-- 
1.7.9.5

[toc] | [next] | [standalone]

#1508957 — [RFC PATCH v2 4/9] drm: mali-dp: Add RGB writeback formats for DP550/DP650

Add a layer bit for the SE memory-write, and add it to the pixel format
matrix for DP550/DP650.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/arm/malidp_hw.c |   28 ++++++++++++++--------------
 drivers/gpu/drm/arm/malidp_hw.h |    1 +
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index eb8a4bf..df1fce0 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -46,20 +46,20 @@ static const struct malidp_format_id malidp500_de_formats[] = {
 
 #define MALIDP_COMMON_FORMATS \
 	/*    fourcc,   layers supporting the format,      internal id   */ \
-	{ DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(0, 0) }, \
-	{ DRM_FORMAT_ABGR2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(0, 1) }, \
-	{ DRM_FORMAT_RGBA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(0, 2) }, \
-	{ DRM_FORMAT_BGRA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(0, 3) }, \
-	{ DRM_FORMAT_ARGB8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(1, 0) }, \
-	{ DRM_FORMAT_ABGR8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(1, 1) }, \
-	{ DRM_FORMAT_RGBA8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(1, 2) }, \
-	{ DRM_FORMAT_BGRA8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(1, 3) }, \
-	{ DRM_FORMAT_XRGB8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(2, 0) }, \
-	{ DRM_FORMAT_XBGR8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(2, 1) }, \
-	{ DRM_FORMAT_RGBX8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(2, 2) }, \
-	{ DRM_FORMAT_BGRX8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(2, 3) }, \
-	{ DRM_FORMAT_RGB888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(3, 0) }, \
-	{ DRM_FORMAT_BGR888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(3, 1) }, \
+	{ DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(0, 0) }, \
+	{ DRM_FORMAT_ABGR2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(0, 1) }, \
+	{ DRM_FORMAT_RGBA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(0, 2) }, \
+	{ DRM_FORMAT_BGRA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(0, 3) }, \
+	{ DRM_FORMAT_ARGB8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(1, 0) }, \
+	{ DRM_FORMAT_ABGR8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(1, 1) }, \
+	{ DRM_FORMAT_RGBA8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(1, 2) }, \
+	{ DRM_FORMAT_BGRA8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(1, 3) }, \
+	{ DRM_FORMAT_XRGB8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(2, 0) }, \
+	{ DRM_FORMAT_XBGR8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(2, 1) }, \
+	{ DRM_FORMAT_RGBX8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(2, 2) }, \
+	{ DRM_FORMAT_BGRX8888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(2, 3) }, \
+	{ DRM_FORMAT_RGB888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(3, 0) }, \
+	{ DRM_FORMAT_BGR888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(3, 1) }, \
 	{ DRM_FORMAT_RGBA5551, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4, 0) }, \
 	{ DRM_FORMAT_ABGR1555, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4, 1) }, \
 	{ DRM_FORMAT_RGB565, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4, 2) }, \
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 4f8c884..ce4ea55 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -33,6 +33,7 @@ enum {
 	DE_GRAPHICS2 = BIT(2), /* used only in DP500 */
 	DE_VIDEO2 = BIT(3),
 	DE_SMART = BIT(4),
+	SE_MEMWRITE = BIT(5),
 };
 
 struct malidp_format_id {
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1508958 — [RFC PATCH v2 3/9] drm: mali-dp: Rename malidp_input_format

We're going to use the same format list for output formats, so rename
everything related to input formats to avoid confusion.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/malidp_hw.c     |   24 ++++++++++++------------
 drivers/gpu/drm/arm/malidp_hw.h     |    8 ++++----
 drivers/gpu/drm/arm/malidp_planes.c |    8 ++++----
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 65e667b..eb8a4bf 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -21,7 +21,7 @@
 #include "malidp_drv.h"
 #include "malidp_hw.h"
 
-static const struct malidp_input_format malidp500_de_formats[] = {
+static const struct malidp_format_id malidp500_de_formats[] = {
 	/*    fourcc,   layers supporting the format,     internal id  */
 	{ DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_GRAPHICS2,  0 },
 	{ DRM_FORMAT_ABGR2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_GRAPHICS2,  1 },
@@ -69,7 +69,7 @@ static const struct malidp_input_format malidp500_de_formats[] = {
 	{ DRM_FORMAT_NV12, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 6) },	\
 	{ DRM_FORMAT_YUV420, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 7) }
 
-static const struct malidp_input_format malidp550_de_formats[] = {
+static const struct malidp_format_id malidp550_de_formats[] = {
 	MALIDP_COMMON_FORMATS,
 };
 
@@ -441,8 +441,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
 				.irq_mask = MALIDP500_DE_IRQ_CONF_VALID,
 				.vsync_irq = MALIDP500_DE_IRQ_CONF_VALID,
 			},
-			.input_formats = malidp500_de_formats,
-			.n_input_formats = ARRAY_SIZE(malidp500_de_formats),
+			.pixel_formats = malidp500_de_formats,
+			.n_pixel_formats = ARRAY_SIZE(malidp500_de_formats),
 			.bus_align_bytes = 8,
 		},
 		.query_hw = malidp500_query_hw,
@@ -474,8 +474,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
 				.irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
 				.vsync_irq = MALIDP550_DC_IRQ_CONF_VALID,
 			},
-			.input_formats = malidp550_de_formats,
-			.n_input_formats = ARRAY_SIZE(malidp550_de_formats),
+			.pixel_formats = malidp550_de_formats,
+			.n_pixel_formats = ARRAY_SIZE(malidp550_de_formats),
 			.bus_align_bytes = 8,
 		},
 		.query_hw = malidp550_query_hw,
@@ -508,8 +508,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
 				.irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
 				.vsync_irq = MALIDP550_DC_IRQ_CONF_VALID,
 			},
-			.input_formats = malidp550_de_formats,
-			.n_input_formats = ARRAY_SIZE(malidp550_de_formats),
+			.pixel_formats = malidp550_de_formats,
+			.n_pixel_formats = ARRAY_SIZE(malidp550_de_formats),
 			.bus_align_bytes = 16,
 		},
 		.query_hw = malidp650_query_hw,
@@ -527,10 +527,10 @@ u8 malidp_hw_get_format_id(const struct malidp_hw_regmap *map,
 {
 	unsigned int i;
 
-	for (i = 0; i < map->n_input_formats; i++) {
-		if (((map->input_formats[i].layer & layer_id) == layer_id) &&
-		    (map->input_formats[i].format == format))
-			return map->input_formats[i].id;
+	for (i = 0; i < map->n_pixel_formats; i++) {
+		if (((map->pixel_formats[i].layer & layer_id) == layer_id) &&
+		    (map->pixel_formats[i].format == format))
+			return map->pixel_formats[i].id;
 	}
 
 	return MALIDP_INVALID_FORMAT_ID;
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 087e1202..4f8c884 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -35,7 +35,7 @@ enum {
 	DE_SMART = BIT(4),
 };
 
-struct malidp_input_format {
+struct malidp_format_id {
 	u32 format;		/* DRM fourcc */
 	u8 layer;		/* bitmask of layers supporting it */
 	u8 id;			/* used internally */
@@ -85,9 +85,9 @@ struct malidp_hw_regmap {
 	const struct malidp_irq_map se_irq_map;
 	const struct malidp_irq_map dc_irq_map;
 
-	/* list of supported input formats for each layer */
-	const struct malidp_input_format *input_formats;
-	const u8 n_input_formats;
+	/* list of supported pixel formats for each layer */
+	const struct malidp_format_id *pixel_formats;
+	const u8 n_pixel_formats;
 
 	/* pitch alignment requirement in bytes */
 	const u8 bus_align_bytes;
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 9020c0d..f44a1cb 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -255,7 +255,7 @@ int malidp_de_planes_init(struct drm_device *drm)
 	u32 *formats;
 	int ret, i, j, n;
 
-	formats = kcalloc(map->n_input_formats, sizeof(*formats), GFP_KERNEL);
+	formats = kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KERNEL);
 	if (!formats) {
 		ret = -ENOMEM;
 		goto cleanup;
@@ -271,9 +271,9 @@ int malidp_de_planes_init(struct drm_device *drm)
 		}
 
 		/* build the list of DRM supported formats based on the map */
-		for (n = 0, j = 0;  j < map->n_input_formats; j++) {
-			if ((map->input_formats[j].layer & id) == id)
-				formats[n++] = map->input_formats[j].format;
+		for (n = 0, j = 0;  j < map->n_pixel_formats; j++) {
+			if ((map->pixel_formats[j].layer & id) == id)
+				formats[n++] = map->pixel_formats[j].format;
 		}
 
 		plane_type = (i == 0) ? DRM_PLANE_TYPE_PRIMARY :
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1508960 — [RFC PATCH v2 2/9] drm: mali-dp: Clear CVAL when leaving config mode

It's possible for CVAL to get set whilst we are in config mode. If this
happens, afer we leave config mode the HW will latch whatever
configuration is in the registers at the next vsync. Most likely this
will be a partial configuration, as we'll be racing against the ongoing
atomic_commit.

To avoid this, clear CVAL before leaving config mode.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/arm/malidp_hw.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 7f4a0bd..65e667b 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -125,6 +125,7 @@ static void malidp500_leave_config_mode(struct malidp_hw_device *hwdev)
 {
 	u32 status, count = 100;
 
+	malidp_hw_clearbits(hwdev, MALIDP_CFG_VALID, MALIDP500_CONFIG_VALID);
 	malidp_hw_clearbits(hwdev, MALIDP500_DC_CONFIG_REQ, MALIDP500_DC_CONTROL);
 	while (count) {
 		status = malidp_hw_read(hwdev, hwdev->map.dc_base + MALIDP_REG_STATUS);
@@ -271,6 +272,7 @@ static void malidp550_leave_config_mode(struct malidp_hw_device *hwdev)
 {
 	u32 status, count = 100;
 
+	malidp_hw_clearbits(hwdev, MALIDP_CFG_VALID, MALIDP550_CONFIG_VALID);
 	malidp_hw_clearbits(hwdev, MALIDP550_DC_CONFIG_REQ, MALIDP550_DC_CONTROL);
 	while (count) {
 		status = malidp_hw_read(hwdev, hwdev->map.dc_base + MALIDP_REG_STATUS);
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1508965 — [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support

If userspace has asked for an out-fence for the writeback, we add a
fence to malidp_mw_job, to be signaled when the writeback job has
completed.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/arm/malidp_hw.c |    5 ++++-
 drivers/gpu/drm/arm/malidp_mw.c |   18 +++++++++++++++++-
 drivers/gpu/drm/arm/malidp_mw.h |    3 +++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 1689547..3032226 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
 		unsigned long irqflags;
 		/*
 		 * We can't unreference the framebuffer here, so we queue it
-		 * up on our threaded handler.
+		 * up on our threaded handler. However, signal the fence
+		 * as soon as possible
 		 */
+		malidp_mw_job_signal(drm, malidp->current_mw, 0);
+
 		spin_lock_irqsave(&malidp->mw_lock, irqflags);
 		list_add_tail(&malidp->current_mw->list,
 			      &malidp->finished_mw_jobs);
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index 69e423c..05880b1 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -34,11 +34,24 @@ struct malidp_mw_connector_state {
 	u8 n_planes;
 };
 
+void malidp_mw_job_signal(struct drm_device *drm,
+			  struct malidp_mw_job *job, int status)
+{
+	DRM_DEV_DEBUG_DRIVER(drm->dev, "MW job signal %p\n", job);
+
+	if (!job->fence)
+		return;
+
+	job->fence->status = status;
+	fence_signal(job->fence);
+}
+
 void malidp_mw_job_cleanup(struct drm_device *drm,
 				  struct malidp_mw_job *job)
 {
 	DRM_DEV_DEBUG_DRIVER(drm->dev, "MW job cleanup %p\n", job);
 	drm_framebuffer_unreference(job->fb);
+	fence_put(job->fence);
 	kfree(job);
 }
 
@@ -107,8 +120,10 @@ static void malidp_mw_connector_destroy_state(struct drm_connector *connector,
 	struct malidp_mw_connector_state *mw_state = to_mw_state(state);
 
 	__drm_atomic_helper_connector_destroy_state(&mw_state->base);
-	if (mw_state->job)
+	if (mw_state->job) {
+		malidp_mw_job_signal(connector->dev, mw_state->job, -EPIPE);
 		malidp_mw_job_cleanup(connector->dev, mw_state->job);
+	}
 	kfree(mw_state);
 }
 
@@ -177,6 +192,7 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
 	/* We can take ownership of the framebuffer reference in the job. */
 	mw_state->job->fb = conn_state->fb;
 	conn_state->fb = NULL;
+	mw_state->job->fence = fence_get(conn_state->out_fence);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/arm/malidp_mw.h b/drivers/gpu/drm/arm/malidp_mw.h
index db7b2b0..5d0bd1d 100644
--- a/drivers/gpu/drm/arm/malidp_mw.h
+++ b/drivers/gpu/drm/arm/malidp_mw.h
@@ -15,11 +15,14 @@
 struct malidp_mw_job {
 	struct list_head list;
 	struct drm_framebuffer *fb;
+	struct fence *fence;
 };
 
 int malidp_mw_connector_init(struct drm_device *drm);
 void malidp_mw_atomic_commit(struct drm_device *drm,
 			     struct drm_atomic_state *old_state);
+void malidp_mw_job_signal(struct drm_device *drm,
+			  struct malidp_mw_job *job, int status);
 void malidp_mw_job_cleanup(struct drm_device *drm,
 				  struct malidp_mw_job *job);
 #endif
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1509870 — Re: [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support

2016-10-26 Brian Starkey <brian.starkey@arm.com>:

> If userspace has asked for an out-fence for the writeback, we add a
> fence to malidp_mw_job, to be signaled when the writeback job has
> completed.
> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
>  drivers/gpu/drm/arm/malidp_hw.c |    5 ++++-
>  drivers/gpu/drm/arm/malidp_mw.c |   18 +++++++++++++++++-
>  drivers/gpu/drm/arm/malidp_mw.h |    3 +++
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index 1689547..3032226 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
>  		unsigned long irqflags;
>  		/*
>  		 * We can't unreference the framebuffer here, so we queue it
> -		 * up on our threaded handler.
> +		 * up on our threaded handler. However, signal the fence
> +		 * as soon as possible
>  		 */
> +		malidp_mw_job_signal(drm, malidp->current_mw, 0);

Drivers should not deal with fences directly. We need some sort of 
drm_writeback_finished() that will do the signalling for you.

Gustavo

[toc] | [prev] | [next] | [standalone]

#1510176 — Re: [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support

On Wed, Oct 26, 2016 at 07:43:57PM -0200, Gustavo Padovan wrote:
>2016-10-26 Brian Starkey <brian.starkey@arm.com>:
>
>> If userspace has asked for an out-fence for the writeback, we add a
>> fence to malidp_mw_job, to be signaled when the writeback job has
>> completed.
>>
>> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
>> ---
>>  drivers/gpu/drm/arm/malidp_hw.c |    5 ++++-
>>  drivers/gpu/drm/arm/malidp_mw.c |   18 +++++++++++++++++-
>>  drivers/gpu/drm/arm/malidp_mw.h |    3 +++
>>  3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
>> index 1689547..3032226 100644
>> --- a/drivers/gpu/drm/arm/malidp_hw.c
>> +++ b/drivers/gpu/drm/arm/malidp_hw.c
>> @@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
>>  		unsigned long irqflags;
>>  		/*
>>  		 * We can't unreference the framebuffer here, so we queue it
>> -		 * up on our threaded handler.
>> +		 * up on our threaded handler. However, signal the fence
>> +		 * as soon as possible
>>  		 */
>> +		malidp_mw_job_signal(drm, malidp->current_mw, 0);
>
>Drivers should not deal with fences directly. We need some sort of
>drm_writeback_finished() that will do the signalling for you.
>

With a signature like this?
	drm_writeback_finished(struct drm_connector_state *state);

I'll have to think about how to achieve that. The state isn't
refcounted and the driver isn't in charge of it's lifetime. I'm not
sure how/where to ensure the state doesn't get destroyed before its
been signaled.

-Brian

>Gustavo
>

[toc] | [prev] | [next] | [standalone]

#1510233 — Re: [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support

On Thu, Oct 27, 2016 at 09:25:19AM -0200, Gustavo Padovan wrote:
> 2016-10-27 Brian Starkey <brian.starkey@arm.com>:
> 
> > On Wed, Oct 26, 2016 at 07:43:57PM -0200, Gustavo Padovan wrote:
> > > 2016-10-26 Brian Starkey <brian.starkey@arm.com>:
> > > 
> > > > If userspace has asked for an out-fence for the writeback, we add a
> > > > fence to malidp_mw_job, to be signaled when the writeback job has
> > > > completed.
> > > > 
> > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > > > ---
> > > >  drivers/gpu/drm/arm/malidp_hw.c |    5 ++++-
> > > >  drivers/gpu/drm/arm/malidp_mw.c |   18 +++++++++++++++++-
> > > >  drivers/gpu/drm/arm/malidp_mw.h |    3 +++
> > > >  3 files changed, 24 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> > > > index 1689547..3032226 100644
> > > > --- a/drivers/gpu/drm/arm/malidp_hw.c
> > > > +++ b/drivers/gpu/drm/arm/malidp_hw.c
> > > > @@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
> > > >  		unsigned long irqflags;
> > > >  		/*
> > > >  		 * We can't unreference the framebuffer here, so we queue it
> > > > -		 * up on our threaded handler.
> > > > +		 * up on our threaded handler. However, signal the fence
> > > > +		 * as soon as possible
> > > >  		 */
> > > > +		malidp_mw_job_signal(drm, malidp->current_mw, 0);
> > > 
> > > Drivers should not deal with fences directly. We need some sort of
> > > drm_writeback_finished() that will do the signalling for you.
> > > 
> > 
> > With a signature like this?
> > 	drm_writeback_finished(struct drm_connector_state *state);
> > 
> > I'll have to think about how to achieve that. The state isn't
> > refcounted and the driver isn't in charge of it's lifetime. I'm not
> > sure how/where to ensure the state doesn't get destroyed before its
> > been signaled.
> 
> Maybe we should do something similar to the crtc vblank handlers and
> even hide the connector_state. Those helpers only take the crtc.
> They are able to hold ref to the event as well.

I guess we could reuse the drm_event stuff, but not sure that's too much
overkill. It would at least be a consistent driver interface, and drivers
could reuse stuff like arm_event and similar functions. Which might be
rather beneficial.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1510324 — Re: [RFC PATCH v2 9/9] drm: mali-dp: Add writeback out-fence support

2016-10-27 Brian Starkey <brian.starkey@arm.com>:

> On Wed, Oct 26, 2016 at 07:43:57PM -0200, Gustavo Padovan wrote:
> > 2016-10-26 Brian Starkey <brian.starkey@arm.com>:
> > 
> > > If userspace has asked for an out-fence for the writeback, we add a
> > > fence to malidp_mw_job, to be signaled when the writeback job has
> > > completed.
> > > 
> > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > > ---
> > >  drivers/gpu/drm/arm/malidp_hw.c |    5 ++++-
> > >  drivers/gpu/drm/arm/malidp_mw.c |   18 +++++++++++++++++-
> > >  drivers/gpu/drm/arm/malidp_mw.h |    3 +++
> > >  3 files changed, 24 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> > > index 1689547..3032226 100644
> > > --- a/drivers/gpu/drm/arm/malidp_hw.c
> > > +++ b/drivers/gpu/drm/arm/malidp_hw.c
> > > @@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
> > >  		unsigned long irqflags;
> > >  		/*
> > >  		 * We can't unreference the framebuffer here, so we queue it
> > > -		 * up on our threaded handler.
> > > +		 * up on our threaded handler. However, signal the fence
> > > +		 * as soon as possible
> > >  		 */
> > > +		malidp_mw_job_signal(drm, malidp->current_mw, 0);
> > 
> > Drivers should not deal with fences directly. We need some sort of
> > drm_writeback_finished() that will do the signalling for you.
> > 
> 
> With a signature like this?
> 	drm_writeback_finished(struct drm_connector_state *state);
> 
> I'll have to think about how to achieve that. The state isn't
> refcounted and the driver isn't in charge of it's lifetime. I'm not
> sure how/where to ensure the state doesn't get destroyed before its
> been signaled.

Maybe we should do something similar to the crtc vblank handlers and
even hide the connector_state. Those helpers only take the crtc.
They are able to hold ref to the event as well.

Gustavo

[toc] | [prev] | [next] | [standalone]

#1508967 — [RFC PATCH v2 5/9] drm: mali-dp: Add support for writeback on DP550/DP650

From: Liviu Dudau <Liviu.Dudau@arm.com>

Mali-DP display processors are able to write the composition result to a
memory buffer via the SE.

Add entry points in the HAL for enabling/disabling this feature, and
implement support for it on DP650 and DP550. DP500 acts differently and
so is omitted from this change.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/arm/malidp_hw.c   |   52 +++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/arm/malidp_hw.h   |   18 +++++++++++++
 drivers/gpu/drm/arm/malidp_regs.h |   15 +++++++++++
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index df1fce0..5004988 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -389,6 +389,48 @@ static int malidp550_rotmem_required(struct malidp_hw_device *hwdev, u16 w, u16
 	return w * bytes_per_col;
 }
 
+static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev,
+				     dma_addr_t *addrs, s32 *pitches,
+				     int num_planes, u16 w, u16 h, u32 fmt_id)
+{
+	u32 base = MALIDP550_SE_MEMWRITE_BASE;
+	u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
+
+	/* enable the scaling engine block */
+	malidp_hw_setbits(hwdev, MALIDP_SCALE_ENGINE_EN, de_base + MALIDP_DE_DISPLAY_FUNC);
+
+	malidp_hw_write(hwdev, fmt_id, base + MALIDP_MW_FORMAT);
+	switch (num_planes) {
+	case 2:
+		malidp_hw_write(hwdev, lower_32_bits(addrs[1]), base + MALIDP_MW_P2_PTR_LOW);
+		malidp_hw_write(hwdev, upper_32_bits(addrs[1]), base + MALIDP_MW_P2_PTR_HIGH);
+		malidp_hw_write(hwdev, pitches[1], base + MALIDP_MW_P2_STRIDE);
+		/* fall through */
+	case 1:
+		malidp_hw_write(hwdev, lower_32_bits(addrs[0]), base + MALIDP_MW_P1_PTR_LOW);
+		malidp_hw_write(hwdev, upper_32_bits(addrs[0]), base + MALIDP_MW_P1_PTR_HIGH);
+		malidp_hw_write(hwdev, pitches[0], base + MALIDP_MW_P1_STRIDE);
+		break;
+	default:
+		WARN(1, "Invalid number of planes");
+	}
+
+	malidp_hw_write(hwdev, MALIDP_DE_H_ACTIVE(w) | MALIDP_DE_V_ACTIVE(h),
+			MALIDP550_SE_MEMWRITE_OUT_SIZE);
+	malidp_hw_setbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_EN,
+			  MALIDP550_SE_CONTROL);
+
+	return 0;
+}
+
+static void malidp550_disable_memwrite(struct malidp_hw_device *hwdev)
+{
+	u32 base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
+	malidp_hw_clearbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_EN,
+			    MALIDP550_SE_CONTROL);
+	malidp_hw_clearbits(hwdev, MALIDP_SCALE_ENGINE_EN, base + MALIDP_DE_DISPLAY_FUNC);
+}
+
 static int malidp650_query_hw(struct malidp_hw_device *hwdev)
 {
 	u32 conf = malidp_hw_read(hwdev, MALIDP550_CONFIG_ID);
@@ -471,7 +513,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
 					    MALIDP550_SE_IRQ_AXI_ERR,
 			},
 			.dc_irq_map = {
-				.irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
+				.irq_mask = MALIDP550_DC_IRQ_CONF_VALID |
+					    MALIDP550_DC_IRQ_SE,
 				.vsync_irq = MALIDP550_DC_IRQ_CONF_VALID,
 			},
 			.pixel_formats = malidp550_de_formats,
@@ -485,6 +528,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
 		.set_config_valid = malidp550_set_config_valid,
 		.modeset = malidp550_modeset,
 		.rotmem_required = malidp550_rotmem_required,
+		.enable_memwrite = malidp550_enable_memwrite,
+		.disable_memwrite = malidp550_disable_memwrite,
 	},
 	[MALIDP_650] = {
 		.map = {
@@ -505,7 +550,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
 					    MALIDP550_SE_IRQ_AXI_ERR,
 			},
 			.dc_irq_map = {
-				.irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
+				.irq_mask = MALIDP550_DC_IRQ_CONF_VALID |
+					    MALIDP550_DC_IRQ_SE,
 				.vsync_irq = MALIDP550_DC_IRQ_CONF_VALID,
 			},
 			.pixel_formats = malidp550_de_formats,
@@ -519,6 +565,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
 		.set_config_valid = malidp550_set_config_valid,
 		.modeset = malidp550_modeset,
 		.rotmem_required = malidp550_rotmem_required,
+		.enable_memwrite = malidp550_enable_memwrite,
+		.disable_memwrite = malidp550_disable_memwrite,
 	},
 };
 
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index ce4ea55..8056efa 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -147,6 +147,24 @@ struct malidp_hw_device {
 	 */
 	int (*rotmem_required)(struct malidp_hw_device *hwdev, u16 w, u16 h, u32 fmt);
 
+	/**
+	 * Enable writing to memory the content of the next frame
+	 * @param hwdev - malidp_hw_device structure containing the HW description
+	 * @param addrs - array of addresses for each plane
+	 * @param pitches - array of pitches for each plane
+	 * @param num_planes - number of planes to be written
+	 * @param w - width of the output frame
+	 * @param h - height of the output frame
+	 * @param fmt_id - internal format ID of output buffer
+	 */
+	int (*enable_memwrite)(struct malidp_hw_device *hwdev, dma_addr_t *addrs,
+			       s32 *pitches, int num_planes, u16 w, u16 h, u32 fmt_id);
+
+	/*
+	 * Disable the writing to memory of the next frame's content.
+	 */
+	void (*disable_memwrite)(struct malidp_hw_device *hwdev);
+
 	u8 features;
 
 	u8 min_line_size;
diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h
index 73fecb3..cab086c 100644
--- a/drivers/gpu/drm/arm/malidp_regs.h
+++ b/drivers/gpu/drm/arm/malidp_regs.h
@@ -64,6 +64,8 @@
 /* bit masks that are common between products */
 #define   MALIDP_CFG_VALID		(1 << 0)
 #define   MALIDP_DISP_FUNC_ILACED	(1 << 8)
+#define   MALIDP_SCALE_ENGINE_EN	(1 << 16)
+#define   MALIDP_SE_MEMWRITE_EN		(2 << 5)
 
 /* register offsets for IRQ management */
 #define MALIDP_REG_STATUS		0x00000
@@ -92,6 +94,15 @@
 #define MALIDP_DE_H_ACTIVE(x)		(((x) & 0x1fff) << 0)
 #define MALIDP_DE_V_ACTIVE(x)		(((x) & 0x1fff) << 16)
 
+/* register offsets relative to MALIDP5x0_SE_MEMWRITE_BASE */
+#define MALIDP_MW_FORMAT		0x00000
+#define MALIDP_MW_P1_STRIDE		0x00004
+#define MALIDP_MW_P2_STRIDE		0x00008
+#define MALIDP_MW_P1_PTR_LOW		0x0000c
+#define MALIDP_MW_P1_PTR_HIGH		0x00010
+#define MALIDP_MW_P2_PTR_LOW		0x0002c
+#define MALIDP_MW_P2_PTR_HIGH		0x00030
+
 /* register offsets and bits specific to DP500 */
 #define MALIDP500_DC_BASE		0x00000
 #define MALIDP500_DC_CONTROL		0x0000c
@@ -149,6 +160,10 @@
 #define MALIDP550_DE_LS_PTR_BASE	0x0042c
 #define MALIDP550_DE_PERF_BASE		0x00500
 #define MALIDP550_SE_BASE		0x08000
+#define MALIDP550_SE_CONTROL		0x08010
+#define   MALIDP550_SE_MEMWRITE_ONESHOT	(1 << 7)
+#define MALIDP550_SE_MEMWRITE_OUT_SIZE	0x08030
+#define MALIDP550_SE_MEMWRITE_BASE	0x08100
 #define MALIDP550_DC_BASE		0x0c000
 #define MALIDP550_DC_CONTROL		0x0c010
 #define   MALIDP550_DC_CONFIG_REQ	(1 << 16)
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1508968 — [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

Some parts of setting up the CRTC out-fence can be re-used for
writeback out-fences. Factor this out into a separate function.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/drm_atomic.c |   64 +++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f434f34..3f8fc97 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_atomic_clean_old_fb);
 
-static int crtc_setup_out_fence(struct drm_crtc *crtc,
-				struct drm_crtc_state *crtc_state,
-				struct drm_out_fence_state *fence_state)
+static struct fence *get_crtc_fence(struct drm_crtc *crtc,
+				    struct drm_crtc_state *crtc_state)
 {
 	struct fence *fence;
 
-	fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
-	if (fence_state->fd < 0) {
-		return fence_state->fd;
-	}
-
-	fence_state->out_fence_ptr = crtc_state->out_fence_ptr;
-	crtc_state->out_fence_ptr = NULL;
-
-	if (put_user(fence_state->fd, fence_state->out_fence_ptr))
-		return -EFAULT;
-
 	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
 	if (!fence)
-		return -ENOMEM;
+		return NULL;
 
 	fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
 		   crtc->fence_context, ++crtc->fence_seqno);
 
+	crtc_state->event->base.fence = fence_get(fence);
+
+	return fence;
+}
+
+static int setup_out_fence(struct drm_out_fence_state *fence_state,
+			   u64 __user *out_fence_ptr,
+			   struct fence *fence)
+{
+	int ret;
+
+	DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n",
+			 fence, out_fence_ptr);
+
+	fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
+	if (fence_state->fd < 0)
+		return fence_state->fd;
+
+	ret = put_user(fence_state->fd, out_fence_ptr);
+	if (ret)
+		return ret;
+
+	fence_state->out_fence_ptr = out_fence_ptr;
+
 	fence_state->sync_file = sync_file_create(fence);
-	if(!fence_state->sync_file) {
-		fence_put(fence);
+	if (!fence_state->sync_file)
 		return -ENOMEM;
-	}
 
-	crtc_state->event->base.fence = fence_get(fence);
 	return 0;
 }
 
@@ -1896,10 +1905,21 @@ retry:
 		}
 
 		if (crtc_state->out_fence_ptr) {
-			ret = crtc_setup_out_fence(crtc, crtc_state,
-						   &fence_state[fence_idx++]);
-			if (ret)
+			struct fence *fence = get_crtc_fence(crtc, crtc_state);
+			if (!fence) {
+				ret = -ENOMEM;
+				goto out;
+			}
+
+			ret = setup_out_fence(&fence_state[fence_idx++],
+					      crtc_state->out_fence_ptr,
+					      fence);
+			if (ret) {
+				fence_put(fence);
 				goto out;
+			}
+
+			crtc_state->out_fence_ptr = NULL;
 		}
 	}
 
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1509065 — Re: [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

On Wed, Oct 26, 2016 at 09:55:06AM +0100, Brian Starkey wrote:
> Some parts of setting up the CRTC out-fence can be re-used for
> writeback out-fences. Factor this out into a separate function.
> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>

Cc: Gustavo here pls, probably best if he reviews this one.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c |   64 +++++++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index f434f34..3f8fc97 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>  
> -static int crtc_setup_out_fence(struct drm_crtc *crtc,
> -				struct drm_crtc_state *crtc_state,
> -				struct drm_out_fence_state *fence_state)
> +static struct fence *get_crtc_fence(struct drm_crtc *crtc,
> +				    struct drm_crtc_state *crtc_state)
>  {
>  	struct fence *fence;
>  
> -	fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
> -	if (fence_state->fd < 0) {
> -		return fence_state->fd;
> -	}
> -
> -	fence_state->out_fence_ptr = crtc_state->out_fence_ptr;
> -	crtc_state->out_fence_ptr = NULL;
> -
> -	if (put_user(fence_state->fd, fence_state->out_fence_ptr))
> -		return -EFAULT;
> -
>  	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>  	if (!fence)
> -		return -ENOMEM;
> +		return NULL;
>  
>  	fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
>  		   crtc->fence_context, ++crtc->fence_seqno);
>  
> +	crtc_state->event->base.fence = fence_get(fence);
> +
> +	return fence;
> +}
> +
> +static int setup_out_fence(struct drm_out_fence_state *fence_state,
> +			   u64 __user *out_fence_ptr,
> +			   struct fence *fence)
> +{
> +	int ret;
> +
> +	DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n",
> +			 fence, out_fence_ptr);
> +
> +	fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
> +	if (fence_state->fd < 0)
> +		return fence_state->fd;
> +
> +	ret = put_user(fence_state->fd, out_fence_ptr);
> +	if (ret)
> +		return ret;
> +
> +	fence_state->out_fence_ptr = out_fence_ptr;
> +
>  	fence_state->sync_file = sync_file_create(fence);
> -	if(!fence_state->sync_file) {
> -		fence_put(fence);
> +	if (!fence_state->sync_file)
>  		return -ENOMEM;
> -	}
>  
> -	crtc_state->event->base.fence = fence_get(fence);
>  	return 0;
>  }
>  
> @@ -1896,10 +1905,21 @@ retry:
>  		}
>  
>  		if (crtc_state->out_fence_ptr) {
> -			ret = crtc_setup_out_fence(crtc, crtc_state,
> -						   &fence_state[fence_idx++]);
> -			if (ret)
> +			struct fence *fence = get_crtc_fence(crtc, crtc_state);
> +			if (!fence) {
> +				ret = -ENOMEM;
> +				goto out;
> +			}
> +
> +			ret = setup_out_fence(&fence_state[fence_idx++],
> +					      crtc_state->out_fence_ptr,
> +					      fence);
> +			if (ret) {
> +				fence_put(fence);
>  				goto out;
> +			}
> +
> +			crtc_state->out_fence_ptr = NULL;
>  		}
>  	}
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1509851 — Re: [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

2016-10-26 Brian Starkey <brian.starkey@arm.com>:

> Some parts of setting up the CRTC out-fence can be re-used for
> writeback out-fences. Factor this out into a separate function.
> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |   64 +++++++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index f434f34..3f8fc97 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>  
> -static int crtc_setup_out_fence(struct drm_crtc *crtc,
> -				struct drm_crtc_state *crtc_state,
> -				struct drm_out_fence_state *fence_state)
> +static struct fence *get_crtc_fence(struct drm_crtc *crtc,
> +				    struct drm_crtc_state *crtc_state)
>  {
>  	struct fence *fence;
>  
> -	fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
> -	if (fence_state->fd < 0) {
> -		return fence_state->fd;
> -	}
> -
> -	fence_state->out_fence_ptr = crtc_state->out_fence_ptr;
> -	crtc_state->out_fence_ptr = NULL;
> -
> -	if (put_user(fence_state->fd, fence_state->out_fence_ptr))
> -		return -EFAULT;
> -
>  	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>  	if (!fence)
> -		return -ENOMEM;
> +		return NULL;
>  
>  	fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
>  		   crtc->fence_context, ++crtc->fence_seqno);
>  
> +	crtc_state->event->base.fence = fence_get(fence);
> +
> +	return fence;
> +}
> +
> +static int setup_out_fence(struct drm_out_fence_state *fence_state,
> +			   u64 __user *out_fence_ptr,
> +			   struct fence *fence)
> +{
> +	int ret;
> +
> +	DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n",
> +			 fence, out_fence_ptr);
> +
> +	fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
> +	if (fence_state->fd < 0)
> +		return fence_state->fd;
> +
> +	ret = put_user(fence_state->fd, out_fence_ptr);
> +	if (ret)
> +		return ret;
> +
> +	fence_state->out_fence_ptr = out_fence_ptr;
> +
>  	fence_state->sync_file = sync_file_create(fence);
> -	if(!fence_state->sync_file) {
> -		fence_put(fence);
> +	if (!fence_state->sync_file)
>  		return -ENOMEM;
> -	}
>  
> -	crtc_state->event->base.fence = fence_get(fence);
>  	return 0;
>  }
>  
> @@ -1896,10 +1905,21 @@ retry:
>  		}
>  
>  		if (crtc_state->out_fence_ptr) {
> -			ret = crtc_setup_out_fence(crtc, crtc_state,
> -						   &fence_state[fence_idx++]);
> -			if (ret)
> +			struct fence *fence = get_crtc_fence(crtc, crtc_state);
> +			if (!fence) {
> +				ret = -ENOMEM;
> +				goto out;
> +			}
> +
> +			ret = setup_out_fence(&fence_state[fence_idx++],
> +					      crtc_state->out_fence_ptr,
> +					      fence);
> +			if (ret) {
> +				fence_put(fence);
>  				goto out;
> +			}
> +
> +			crtc_state->out_fence_ptr = NULL;

Yes, this split makes sense. I can break crtc_setup_out_fence() in
my out_fence patch already.

Gustavo

[toc] | [prev] | [next] | [standalone]

#1509874 — Re: [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

2016-10-26 Brian Starkey <brian.starkey@arm.com>:

> Some parts of setting up the CRTC out-fence can be re-used for
> writeback out-fences. Factor this out into a separate function.
> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |   64 +++++++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index f434f34..3f8fc97 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>  
> -static int crtc_setup_out_fence(struct drm_crtc *crtc,
> -				struct drm_crtc_state *crtc_state,
> -				struct drm_out_fence_state *fence_state)
> +static struct fence *get_crtc_fence(struct drm_crtc *crtc,
> +				    struct drm_crtc_state *crtc_state)
>  {
>  	struct fence *fence;
>  
> -	fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
> -	if (fence_state->fd < 0) {
> -		return fence_state->fd;
> -	}
> -
> -	fence_state->out_fence_ptr = crtc_state->out_fence_ptr;
> -	crtc_state->out_fence_ptr = NULL;
> -
> -	if (put_user(fence_state->fd, fence_state->out_fence_ptr))
> -		return -EFAULT;
> -
>  	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>  	if (!fence)
> -		return -ENOMEM;
> +		return NULL;
>  
>  	fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
>  		   crtc->fence_context, ++crtc->fence_seqno);
>  
> +	crtc_state->event->base.fence = fence_get(fence);
> +
> +	return fence;
> +}
> +
> +static int setup_out_fence(struct drm_out_fence_state *fence_state,
> +			   u64 __user *out_fence_ptr,
> +			   struct fence *fence)
> +{
> +	int ret;
> +
> +	DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n",
> +			 fence, out_fence_ptr);

%p should be kept for your internal debug only. Make sure to remove
anything that exposes kernel address when sending patches.

Gustavo

[toc] | [prev] | [next] | [standalone]

#1510266 — Re: [RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

On Wed, Oct 26, 2016 at 07:45:14PM -0200, Gustavo Padovan wrote:
>
>%p should be kept for your internal debug only. Make sure to remove
>anything that exposes kernel address when sending patches.
>
>Gustavo
>

Noted, thanks!

-Brian

[toc] | [prev] | [next] | [standalone]

#1508969 — [RFC PATCH v2 6/9] drm: mali-dp: Add writeback connector

Mali-DP has a memory writeback engine which can be used to write the
composition result to a memory buffer.
Expose this functionality as a DRM writeback connector on supported
hardware.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/arm/Makefile      |    1 +
 drivers/gpu/drm/arm/malidp_crtc.c |   21 +++
 drivers/gpu/drm/arm/malidp_drv.c  |   28 +++-
 drivers/gpu/drm/arm/malidp_drv.h  |    7 +
 drivers/gpu/drm/arm/malidp_hw.c   |   40 ++++-
 drivers/gpu/drm/arm/malidp_mw.c   |  297 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/arm/malidp_mw.h   |   25 ++++
 7 files changed, 413 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/malidp_mw.c
 create mode 100644 drivers/gpu/drm/arm/malidp_mw.h

diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
index bb8b158..3bf31d1 100644
--- a/drivers/gpu/drm/arm/Makefile
+++ b/drivers/gpu/drm/arm/Makefile
@@ -1,4 +1,5 @@
 hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
 obj-$(CONFIG_DRM_HDLCD)	+= hdlcd.o
 mali-dp-y := malidp_drv.o malidp_hw.o malidp_planes.o malidp_crtc.o
+mali-dp-y += malidp_mw.o
 obj-$(CONFIG_DRM_MALI_DISPLAY)	+= mali-dp.o
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index 08e6a71..aadc223 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -68,6 +68,18 @@ static void malidp_crtc_enable(struct drm_crtc *crtc)
 	clk_set_rate(hwdev->pxlclk, crtc->state->adjusted_mode.crtc_clock * 1000);
 
 	hwdev->modeset(hwdev, &vm);
+	/*
+	 * We should always disable the memory write when leaving config mode,
+	 * otherwise the hardware will start writing right away - possibly with
+	 * a stale config, and definitely before we've had a chance to configure
+	 * the planes.
+	 * If the memory write needs to be enabled, that will get taken care
+	 * of later during the atomic commit
+	 */
+	if (hwdev->disable_memwrite) {
+		DRM_DEV_DEBUG_DRIVER(crtc->dev->dev, "Disable memwrite\n");
+		hwdev->disable_memwrite(hwdev);
+	}
 	hwdev->leave_config_mode(hwdev);
 	drm_crtc_vblank_on(crtc);
 }
@@ -157,6 +169,15 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
 		}
 	}
 
+	/* If only the writeback routing has changed, we don't need a modeset */
+	if (state->connectors_changed) {
+		u32 old_mask = crtc->state->connector_mask;
+		u32 new_mask = state->connector_mask;
+		if ((old_mask ^ new_mask) ==
+		    (1 << drm_connector_index(&malidp->mw_connector)))
+			state->connectors_changed = false;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index f63a4df..cdefbab 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -28,6 +28,7 @@
 #include <drm/drm_of.h>
 
 #include "malidp_drv.h"
+#include "malidp_mw.h"
 #include "malidp_regs.h"
 #include "malidp_hw.h"
 
@@ -92,6 +93,14 @@ static void malidp_atomic_commit_tail(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_modeset_disables(drm, state);
 	drm_atomic_helper_commit_modeset_enables(drm, state);
+
+	/*
+	 * The order here is important. We must configure memory-write after
+	 * the CRTC is already enabled, so that its configuration update is
+	 * gated on the next CVAL.
+	 */
+	malidp_mw_atomic_commit(drm, state);
+
 	drm_atomic_helper_commit_planes(drm, state, 0);
 
 	malidp_atomic_commit_hw_done(state);
@@ -147,12 +156,20 @@ static int malidp_init(struct drm_device *drm)
 	drm->mode_config.helper_private = &malidp_mode_config_helpers;
 
 	ret = malidp_crtc_init(drm);
-	if (ret) {
-		drm_mode_config_cleanup(drm);
-		return ret;
-	}
+	if (ret)
+		goto crtc_fail;
+
+	ret = malidp_mw_connector_init(drm);
+	if (ret)
+		goto mw_fail;
 
 	return 0;
+
+mw_fail:
+	malidp_de_planes_destroy(drm);
+crtc_fail:
+	drm_mode_config_cleanup(drm);
+	return ret;
 }
 
 static void malidp_fini(struct drm_device *drm)
@@ -357,6 +374,9 @@ static int malidp_bind(struct device *dev)
 	atomic_set(&malidp->config_valid, 0);
 	init_waitqueue_head(&malidp->wq);
 
+	INIT_LIST_HEAD(&malidp->finished_mw_jobs);
+	spin_lock_init(&malidp->mw_lock);
+
 	ret = malidp_init(drm);
 	if (ret < 0)
 		goto init_fail;
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index 9fc8a2e..0dce0cf 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -22,8 +22,15 @@ struct malidp_drm {
 	struct drm_fbdev_cma *fbdev;
 	struct list_head event_list;
 	struct drm_crtc crtc;
+	struct drm_encoder mw_encoder;
+	struct drm_connector mw_connector;
 	wait_queue_head_t wq;
 	atomic_t config_valid;
+
+	struct malidp_mw_job *current_mw;
+	/* lock for protecting the finished_mw_jobs list */
+	spinlock_t mw_lock;
+	struct list_head finished_mw_jobs;
 };
 
 #define crtc_to_malidp_device(x) container_of(x, struct malidp_drm, crtc)
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 5004988..1689547 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -20,6 +20,7 @@
 
 #include "malidp_drv.h"
 #include "malidp_hw.h"
+#include "malidp_mw.h"
 
 static const struct malidp_format_id malidp500_de_formats[] = {
 	/*    fourcc,   layers supporting the format,     internal id  */
@@ -548,6 +549,7 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
 			.se_irq_map = {
 				.irq_mask = MALIDP550_SE_IRQ_EOW |
 					    MALIDP550_SE_IRQ_AXI_ERR,
+				.vsync_irq = MALIDP550_SE_IRQ_EOW,
 			},
 			.dc_irq_map = {
 				.irq_mask = MALIDP550_DC_IRQ_CONF_VALID |
@@ -689,7 +691,9 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
 	struct drm_device *drm = arg;
 	struct malidp_drm *malidp = drm->dev_private;
 	struct malidp_hw_device *hwdev = malidp->dev;
+	const struct malidp_irq_map *se = &hwdev->map.se_irq_map;
 	u32 status, mask;
+	irqreturn_t ret = IRQ_HANDLED;
 
 	status = malidp_hw_read(hwdev, hwdev->map.se_base + MALIDP_REG_STATUS);
 	if (!(status & hwdev->map.se_irq_map.irq_mask))
@@ -698,15 +702,47 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
 	mask = malidp_hw_read(hwdev, hwdev->map.se_base + MALIDP_REG_MASKIRQ);
 	status = malidp_hw_read(hwdev, hwdev->map.se_base + MALIDP_REG_STATUS);
 	status &= mask;
-	/* ToDo: status decoding and firing up of VSYNC and page flip events */
+
+	if (status & se->vsync_irq) {
+		unsigned long irqflags;
+		/*
+		 * We can't unreference the framebuffer here, so we queue it
+		 * up on our threaded handler.
+		 */
+		spin_lock_irqsave(&malidp->mw_lock, irqflags);
+		list_add_tail(&malidp->current_mw->list,
+			      &malidp->finished_mw_jobs);
+		malidp->current_mw = NULL;
+		spin_unlock_irqrestore(&malidp->mw_lock, irqflags);
+
+		ret = IRQ_WAKE_THREAD;
+	}
 
 	malidp_hw_clear_irq(hwdev, MALIDP_SE_BLOCK, status);
 
-	return IRQ_HANDLED;
+	return ret;
 }
 
 static irqreturn_t malidp_se_irq_thread_handler(int irq, void *arg)
 {
+	struct drm_device *drm = arg;
+	struct malidp_drm *malidp = drm->dev_private;
+	struct list_head finished;
+	struct malidp_mw_job *tmp, *pos;
+	unsigned long irqflags;
+
+	INIT_LIST_HEAD(&finished);
+
+	spin_lock_irqsave(&malidp->mw_lock, irqflags);
+	list_splice_tail_init(&malidp->finished_mw_jobs,
+			      &finished);
+	spin_unlock_irqrestore(&malidp->mw_lock, irqflags);
+
+	list_for_each_entry_safe(pos, tmp, &finished, list) {
+		list_del(&pos->list);
+		malidp_mw_job_cleanup(drm, pos);
+	}
+
 	return IRQ_HANDLED;
 }
 
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
new file mode 100644
index 0000000..69e423c
--- /dev/null
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -0,0 +1,297 @@
+/*
+ * (C) COPYRIGHT 2016 ARM Limited. All rights reserved.
+ * Author: Brian Starkey <brian.starkey@arm.com>
+ *
+ * This program is free software and is provided to you under the terms of the
+ * GNU General Public License version 2 as published by the Free Software
+ * Foundation, and any use by you of this program is subject to the terms
+ * of such GNU licence.
+ *
+ * ARM Mali DP Writeback connector implementation
+ */
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drmP.h>
+#include <drm/drm_writeback.h>
+
+#include "malidp_drv.h"
+#include "malidp_hw.h"
+#include "malidp_mw.h"
+
+#define mw_conn_to_malidp_device(x) container_of(x, struct malidp_drm, mw_connector)
+#define to_mw_state(_state) (struct malidp_mw_connector_state *)(_state)
+
+struct malidp_mw_connector_state {
+	struct drm_connector_state base;
+	struct malidp_mw_job *job;
+	dma_addr_t addrs[2];
+	s32 pitches[2];
+	u8 format;
+	u8 n_planes;
+};
+
+void malidp_mw_job_cleanup(struct drm_device *drm,
+				  struct malidp_mw_job *job)
+{
+	DRM_DEV_DEBUG_DRIVER(drm->dev, "MW job cleanup %p\n", job);
+	drm_framebuffer_unreference(job->fb);
+	kfree(job);
+}
+
+static int malidp_mw_connector_get_modes(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+
+	return drm_add_modes_noedid(connector, dev->mode_config.max_width,
+				    dev->mode_config.max_height);
+}
+
+static enum drm_mode_status
+malidp_mw_connector_mode_valid(struct drm_connector *connector,
+			       struct drm_display_mode *mode)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_mode_config *mode_config = &dev->mode_config;
+	int w = mode->hdisplay, h = mode->vdisplay;
+
+	if ((w < mode_config->min_width) || (w > mode_config->max_width))
+		return MODE_BAD_HVALUE;
+
+	if ((h < mode_config->min_height) || (h > mode_config->max_height))
+		return MODE_BAD_VVALUE;
+
+	return MODE_OK;
+}
+
+const struct drm_connector_helper_funcs malidp_mw_connector_helper_funcs = {
+	.get_modes = malidp_mw_connector_get_modes,
+	.mode_valid = malidp_mw_connector_mode_valid,
+};
+
+static enum drm_connector_status
+malidp_mw_connector_detect(struct drm_connector *connector, bool force)
+{
+	return connector_status_disconnected;
+}
+
+static void malidp_mw_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_cleanup(connector);
+}
+
+static struct drm_connector_state *
+malidp_mw_connector_duplicate_state(struct drm_connector *connector)
+{
+	struct malidp_mw_connector_state *mw_state;
+
+	if (WARN_ON(!connector->state))
+		return NULL;
+
+	mw_state = kzalloc(sizeof(*mw_state), GFP_KERNEL);
+	if (!mw_state)
+		return NULL;
+
+	/* No need to preserve any of our driver-local data */
+	__drm_atomic_helper_connector_duplicate_state(connector, &mw_state->base);
+
+	return &mw_state->base;
+}
+
+static void malidp_mw_connector_destroy_state(struct drm_connector *connector,
+					      struct drm_connector_state *state)
+{
+	struct malidp_mw_connector_state *mw_state = to_mw_state(state);
+
+	__drm_atomic_helper_connector_destroy_state(&mw_state->base);
+	if (mw_state->job)
+		malidp_mw_job_cleanup(connector->dev, mw_state->job);
+	kfree(mw_state);
+}
+
+static const struct drm_connector_funcs malidp_mw_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
+	.detect = malidp_mw_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = malidp_mw_connector_destroy,
+	.atomic_duplicate_state = malidp_mw_connector_duplicate_state,
+	.atomic_destroy_state = malidp_mw_connector_destroy_state,
+};
+
+static int
+malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
+			       struct drm_crtc_state *crtc_state,
+			       struct drm_connector_state *conn_state)
+{
+	struct drm_connector *conn = conn_state->connector;
+	struct drm_framebuffer *fb = conn_state->fb;
+	struct malidp_drm *malidp = mw_conn_to_malidp_device(conn);
+	struct malidp_mw_connector_state *mw_state;
+	int i, n_planes;
+
+	mw_state = (struct malidp_mw_connector_state *)conn_state;
+
+	if (!conn_state->fb)
+		return 0;
+
+	if ((fb->width != crtc_state->mode.hdisplay) ||
+	    (fb->height != crtc_state->mode.vdisplay)) {
+		DRM_DEBUG_KMS("Invalid framebuffer size %ux%u\n",
+				fb->width, fb->height);
+		return -EINVAL;
+	}
+
+	mw_state->format =
+		malidp_hw_get_format_id(&malidp->dev->map, SE_MEMWRITE,
+					fb->pixel_format);
+	if (mw_state->format == MALIDP_INVALID_FORMAT_ID) {
+		char *format_name = drm_get_format_name(fb->pixel_format);
+		DRM_DEBUG_KMS("Invalid pixel format %s\n", format_name);
+		kfree(format_name);
+		return -EINVAL;
+	}
+
+	n_planes = drm_format_num_planes(fb->pixel_format);
+	for (i = 0; i < n_planes; i++) {
+		struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, i);
+		if (!malidp_hw_pitch_valid(malidp->dev, fb->pitches[i])) {
+			DRM_DEBUG_KMS("Invalid pitch %u for plane %d\n",
+				      fb->pitches[i], i);
+			return -EINVAL;
+		}
+		mw_state->pitches[i] = fb->pitches[i];
+		mw_state->addrs[i] = obj->paddr + fb->offsets[i];
+	}
+	mw_state->n_planes = n_planes;
+
+	mw_state->job = kmalloc(sizeof(*mw_state->job), GFP_KERNEL);
+	if (!mw_state->job)
+		return -ENOMEM;
+
+	DRM_DEV_DEBUG_DRIVER(conn->dev->dev, "MW job create %p\n",
+			     mw_state->job);
+	/* We can take ownership of the framebuffer reference in the job. */
+	mw_state->job->fb = conn_state->fb;
+	conn_state->fb = NULL;
+
+	return 0;
+}
+
+static const struct drm_encoder_helper_funcs malidp_mw_encoder_helper_funcs = {
+	.atomic_check = malidp_mw_encoder_atomic_check,
+};
+
+static void malidp_mw_encoder_destroy(struct drm_encoder *encoder)
+{
+	drm_encoder_cleanup(encoder);
+}
+
+static const struct drm_encoder_funcs malidp_mw_encoder_funcs = {
+	.destroy = malidp_mw_encoder_destroy,
+};
+
+static u32 *get_writeback_formats(struct malidp_drm *malidp, int *n_formats)
+{
+	const struct malidp_hw_regmap *map = &malidp->dev->map;
+	u32 *formats;
+	int n, i;
+
+	formats = kcalloc(map->n_pixel_formats, sizeof(*formats),
+			  GFP_KERNEL);
+	if (!formats)
+		return NULL;
+
+	for (n = 0, i = 0;  i < map->n_pixel_formats; i++) {
+		if (map->pixel_formats[i].layer & SE_MEMWRITE)
+			formats[n++] = map->pixel_formats[i].format;
+	}
+
+	*n_formats = n;
+	return formats;
+}
+
+int malidp_mw_connector_init(struct drm_device *drm)
+{
+	struct malidp_drm *malidp = drm->dev_private;
+	u32 *formats;
+	int ret, n_formats;
+
+	if (!malidp->dev->enable_memwrite)
+		return 0;
+
+	drm_encoder_helper_add(&malidp->mw_encoder, &malidp_mw_encoder_helper_funcs);
+	malidp->mw_encoder.possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);
+	ret = drm_encoder_init(drm, &malidp->mw_encoder, &malidp_mw_encoder_funcs,
+			       DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (ret)
+		return ret;
+
+	drm_connector_helper_add(&malidp->mw_connector,
+				 &malidp_mw_connector_helper_funcs);
+	malidp->mw_connector.interlace_allowed = 0;
+
+	formats = get_writeback_formats(malidp, &n_formats);
+	if (!formats) {
+		ret = -ENOMEM;
+		goto err_encoder;
+	}
+
+	ret = drm_writeback_connector_init(drm, &malidp->mw_connector,
+					   &malidp_mw_connector_funcs,
+					   formats, n_formats);
+	kfree(formats);
+	if (ret)
+		goto err_encoder;
+
+	ret = drm_mode_connector_attach_encoder(&malidp->mw_connector,
+						&malidp->mw_encoder);
+	if (ret)
+		goto err_connector;
+
+	return 0;
+
+err_connector:
+	drm_connector_cleanup(&malidp->mw_connector);
+err_encoder:
+	drm_encoder_cleanup(&malidp->mw_encoder);
+	return ret;
+}
+
+void malidp_mw_atomic_commit(struct drm_device *drm,
+			     struct drm_atomic_state *old_state)
+{
+	struct malidp_mw_connector_state *mw_state;
+	struct malidp_drm *malidp = drm->dev_private;
+	struct malidp_hw_device *hwdev = malidp->dev;
+	struct drm_connector *mw_conn = &malidp->mw_connector;
+
+	mw_state = to_mw_state(mw_conn->state);
+
+	if (!mw_state)
+		return;
+
+	if (mw_state->job) {
+		struct drm_framebuffer *fb = mw_state->job->fb;
+		DRM_DEV_DEBUG_DRIVER(drm->dev, "Enable memwrite %ux%u:%d %pad fmt: %u\n",
+				     fb->width, fb->height, mw_state->pitches[0],
+				     &mw_state->addrs[0], mw_state->format);
+
+		/* Queue up the job for completion handling */
+		DRM_DEV_DEBUG_DRIVER(drm->dev, "MW job queue %p\n", mw_state->job);
+
+		WARN_ON(malidp->current_mw);
+		malidp->current_mw = mw_state->job;
+		mw_state->job = NULL;
+
+		hwdev->enable_memwrite(hwdev, mw_state->addrs, mw_state->pitches,
+				       mw_state->n_planes, fb->width, fb->height,
+				       mw_state->format);
+	} else {
+		DRM_DEV_DEBUG_DRIVER(drm->dev, "Disable memwrite\n");
+		hwdev->disable_memwrite(hwdev);
+	}
+}
diff --git a/drivers/gpu/drm/arm/malidp_mw.h b/drivers/gpu/drm/arm/malidp_mw.h
new file mode 100644
index 0000000..db7b2b0
--- /dev/null
+++ b/drivers/gpu/drm/arm/malidp_mw.h
@@ -0,0 +1,25 @@
+/*
+ * (C) COPYRIGHT 2016 ARM Limited. All rights reserved.
+ * Author: Brian Starkey <brian.starkey@arm.com>
+ *
+ * This program is free software and is provided to you under the terms of the
+ * GNU General Public License version 2 as published by the Free Software
+ * Foundation, and any use by you of this program is subject to the terms
+ * of such GNU licence.
+ *
+ */
+
+#ifndef __MALIDP_MW_H__
+#define __MALIDP_MW_H__
+
+struct malidp_mw_job {
+	struct list_head list;
+	struct drm_framebuffer *fb;
+};
+
+int malidp_mw_connector_init(struct drm_device *drm);
+void malidp_mw_atomic_commit(struct drm_device *drm,
+			     struct drm_atomic_state *old_state);
+void malidp_mw_job_cleanup(struct drm_device *drm,
+				  struct malidp_mw_job *job);
+#endif
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1508972 — [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback connectors

Add the OUT_FENCE_PTR property to writeback connectors, to enable
userspace to get a fence which will signal once the writeback is
complete.

A timeline is added to drm_connector for use by the writeback
out-fences. It is up to drivers to check for a fence in the
connector_state and signal the it appropriately when their writeback has
finished.

It is not allowed to request an out-fence without a framebuffer attached
to the connector.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/drm_atomic.c        |   60 +++++++++++++++++++++++---
 drivers/gpu/drm/drm_atomic_helper.c |    5 ++-
 drivers/gpu/drm/drm_writeback.c     |   80 +++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h         |   14 ++++++
 include/drm/drm_writeback.h         |    2 +
 5 files changed, 155 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3f8fc97..061ea13 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -30,6 +30,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_mode.h>
 #include <drm/drm_plane_helper.h>
+#include <drm/drm_writeback.h>
 #include <linux/sync_file.h>
 
 #include "drm_crtc_internal.h"
@@ -646,6 +647,12 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
 		return -EINVAL;
 	}
 
+	if (state->out_fence && !state->fb) {
+		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] requesting out-fence without framebuffer\n",
+				 connector->base.id, connector->name);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -1047,6 +1054,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
 		drm_atomic_set_fb_for_connector(state, fb);
 		if (fb)
 			drm_framebuffer_unreference(fb);
+	} else if (property == config->prop_out_fence_ptr) {
+		state->out_fence_ptr = u64_to_user_ptr(val);
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -1088,6 +1097,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 	} else if (property == config->writeback_fb_id_property) {
 		/* Writeback framebuffer is one-shot, write and forget */
 		*val = 0;
+	} else if (property == config->prop_out_fence_ptr) {
+		*val = 0;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
@@ -1736,6 +1747,39 @@ static int setup_out_fence(struct drm_out_fence_state *fence_state,
 	return 0;
 }
 
+static int setup_connector_out_fences(struct drm_atomic_state *state,
+				      struct drm_out_fence_state *fence_state,
+				      int *fence_idx)
+{
+	struct drm_connector *conn;
+	struct drm_connector_state *conn_state;
+	int i, ret;
+
+	for_each_connector_in_state(state, conn, conn_state, i) {
+		struct fence *fence;
+
+		if (!conn_state->out_fence_ptr)
+			continue;
+
+		fence = drm_writeback_get_out_fence(conn, conn_state);
+		if (!fence)
+			return -ENOMEM;
+
+		ret = setup_out_fence(&fence_state[(*fence_idx)++],
+				      conn_state->out_fence_ptr,
+				      fence);
+		if (ret) {
+			fence_put(fence);
+			return ret;
+		}
+
+		/* One-time usage only */
+		conn_state->out_fence_ptr = NULL;
+	}
+
+	return 0;
+}
+
 int drm_mode_atomic_ioctl(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv)
 {
@@ -1868,8 +1912,8 @@ retry:
 		drm_mode_object_unreference(obj);
 	}
 
-	fence_state = kcalloc(dev->mode_config.num_crtc, sizeof(*fence_state),
-			      GFP_KERNEL);
+	fence_state = kcalloc(dev->mode_config.num_crtc + state->num_connector,
+			      sizeof(*fence_state), GFP_KERNEL);
 	if (!fence_state) {
 		ret = -ENOMEM;
 		goto out;
@@ -1929,10 +1973,16 @@ retry:
 		 * Below we call drm_atomic_state_free for it.
 		 */
 		ret = drm_atomic_check_only(state);
-	} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
-		ret = drm_atomic_nonblocking_commit(state);
 	} else {
-		ret = drm_atomic_commit(state);
+		ret = setup_connector_out_fences(state, fence_state,
+						 &fence_idx);
+		if (ret)
+			goto out;
+
+		if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK)
+			ret = drm_atomic_nonblocking_commit(state);
+		else
+			ret = drm_atomic_commit(state);
 	}
 
 	if (!ret)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index bba8672..88da299 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3234,8 +3234,9 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
 	if (state->crtc)
 		drm_connector_reference(connector);
 
-	/* Don't copy over framebuffers, they are used only once */
+	/* Don't copy over framebuffers or fences, they are used only once */
 	state->fb = NULL;
+	state->out_fence = NULL;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
 
@@ -3365,6 +3366,8 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state)
 		drm_connector_unreference(state->connector);
 	if (state->fb)
 		drm_framebuffer_unreference(state->fb);
+
+	fence_put(state->out_fence);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 5a6e0ad..4a6ef30 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -11,6 +11,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_property.h>
 #include <drm/drmP.h>
+#include <linux/fence.h>
 
 /**
  * DOC: overview
@@ -32,6 +33,16 @@
  * explicitly cleared in order to make a subsequent commit which doesn't use
  * writeback.
  *
+ * Unlike with planes, when a writeback framebuffer is removed by userspace DRM
+ * makes no attempt to remove it from active use by the connector. This is
+ * because no method is provided to abort a writeback operation, and in any
+ * case making a new commit whilst a writeback is ongoing is undefined (see
+ * OUT_FENCE_PTR below). As soon as the current writeback is finished, the
+ * framebuffer will automatically no longer be in active use. As it will also
+ * have already been removed from the framebuffer list, there will be no way for
+ * any userspace application to retrieve a reference to it in the intervening
+ * period.
+ *
  * Writeback connectors have several additional properties, which userspace
  * can use to query and control them:
  *
@@ -52,8 +63,48 @@
  *  "PIXEL_FORMATS_SIZE":
  *	Immutable unsigned range property storing the number of entries in the
  *	PIXEL_FORMATS array.
+ *
+ *  "OUT_FENCE_PTR":
+ *	Userspace can provide the address of a 64-bit integer in this property,
+ *	which will be filled with a sync_file file descriptor by the kernel.
+ *	The sync_file will signal when the associated writeback has finished.
+ *	Userspace should wait for this fence to signal before making another
+ *	commit affecting any of the same CRTCs, Planes or Connectors.
+ *	**Failure to do so will result in undefined behaviour.**
+ *	For this reason it is strongly recommended that all userspace
+ *	applications making use of writeback connectors *always* retrieve an
+ *	out-fence for the commit and use it appropriately.
+ *	From userspace, this property will always read as zero.
  */
 
+#define fence_to_connector(x) container_of(x->lock, struct drm_connector, fence_lock)
+
+static const char *drm_writeback_fence_get_driver_name(struct fence *fence)
+{
+	struct drm_connector *connector = fence_to_connector(fence);
+
+	return connector->dev->driver->name;
+}
+
+static const char *drm_writeback_fence_get_timeline_name(struct fence *fence)
+{
+	struct drm_connector *connector = fence_to_connector(fence);
+
+	return connector->timeline_name;
+}
+
+static bool drm_writeback_fence_enable_signaling(struct fence *fence)
+{
+	return true;
+}
+
+static const struct fence_ops drm_writeback_fence_ops = {
+	.get_driver_name = drm_writeback_fence_get_driver_name,
+	.get_timeline_name = drm_writeback_fence_get_timeline_name,
+	.enable_signaling = drm_writeback_fence_enable_signaling,
+	.wait = fence_default_wait,
+};
+
 /**
  * create_writeback_properties - Create writeback connector-specific properties
  * @dev: DRM device
@@ -136,6 +187,14 @@ int drm_writeback_connector_init(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	connector->fence_context = fence_context_alloc(1);
+	spin_lock_init(&connector->fence_lock);
+	snprintf(connector->timeline_name, sizeof(connector->timeline_name),
+		 "CONNECTOR:%d", connector->base.id);
+
+	drm_object_attach_property(&connector->base,
+				   config->prop_out_fence_ptr, 0);
+
 	drm_object_attach_property(&connector->base,
 				   config->writeback_fb_id_property, 0);
 
@@ -155,3 +214,24 @@ fail:
 	return ret;
 }
 EXPORT_SYMBOL(drm_writeback_connector_init);
+
+struct fence *drm_writeback_get_out_fence(struct drm_connector *connector,
+					  struct drm_connector_state *conn_state)
+{
+	struct fence *fence;
+
+	if (WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK))
+		return NULL;
+
+	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
+	if (!fence)
+		return NULL;
+
+	fence_init(fence, &drm_writeback_fence_ops, &connector->fence_lock,
+		   connector->fence_context, ++connector->fence_seqno);
+
+	conn_state->out_fence = fence_get(fence);
+
+	return fence;
+}
+EXPORT_SYMBOL(drm_writeback_get_out_fence);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index a5e3778..7d40537 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -199,6 +199,7 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
  * @best_encoder: can be used by helpers and drivers to select the encoder
  * @state: backpointer to global drm_atomic_state
  * @fb: Writeback framebuffer, for DRM_MODE_CONNECTOR_WRITEBACK
+ * @out_fence: Fence which will clear when the framebuffer write has finished
  */
 struct drm_connector_state {
 	struct drm_connector *connector;
@@ -216,6 +217,9 @@ struct drm_connector_state {
 	struct drm_atomic_state *state;
 
 	struct drm_framebuffer *fb;  /* do not write directly, use drm_atomic_set_fb_for_connector() */
+
+	struct fence *out_fence;
+	u64 __user *out_fence_ptr;
 };
 
 /**
@@ -546,6 +550,10 @@ struct drm_cmdline_mode {
  * @tile_v_loc: vertical location of this tile
  * @tile_h_size: horizontal size of this tile.
  * @tile_v_size: vertical size of this tile.
+ * @fence_context: context for fence signalling
+ * @fence_lock: fence lock for the fence context
+ * @fence_seqno: seqno variable to create fences
+ * @timeline_name: fence timeline name
  *
  * Each connector may be connected to one or more CRTCs, or may be clonable by
  * another connector if they can share a CRTC.  Each connector also has a specific
@@ -694,6 +702,12 @@ struct drm_connector {
 	uint8_t num_h_tile, num_v_tile;
 	uint8_t tile_h_loc, tile_v_loc;
 	uint16_t tile_h_size, tile_v_size;
+
+	/* fence timelines info for DRM out-fences */
+	unsigned int fence_context;
+	spinlock_t fence_lock;
+	unsigned long fence_seqno;
+	char timeline_name[32];
 };
 
 #define obj_to_connector(x) container_of(x, struct drm_connector, base)
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index afdc2742..01f33bc 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -16,4 +16,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
 				 const struct drm_connector_funcs *funcs,
 				 u32 *formats, int n_formats);
 
+struct fence *drm_writeback_get_out_fence(struct drm_connector *connector,
+					  struct drm_connector_state *conn_state);
 #endif
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1509066 — Re: [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback connectors

On Wed, Oct 26, 2016 at 09:55:07AM +0100, Brian Starkey wrote:
> Add the OUT_FENCE_PTR property to writeback connectors, to enable
> userspace to get a fence which will signal once the writeback is
> complete.
> 
> A timeline is added to drm_connector for use by the writeback
> out-fences. It is up to drivers to check for a fence in the
> connector_state and signal the it appropriately when their writeback has
> finished.
> 
> It is not allowed to request an out-fence without a framebuffer attached
> to the connector.
> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>

Ah, here it is, so much for reading patches strictly in-order ;-) One
small comment below, otherwise I think this looks good. Again review from
Gustavo for the out fences stuff would be really good (so pls cc him). And
I think some igt testcases to exercise all the corner-cases in here.

> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index a5e3778..7d40537 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -199,6 +199,7 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
>   * @best_encoder: can be used by helpers and drivers to select the encoder
>   * @state: backpointer to global drm_atomic_state
>   * @fb: Writeback framebuffer, for DRM_MODE_CONNECTOR_WRITEBACK
> + * @out_fence: Fence which will clear when the framebuffer write has finished
>   */
>  struct drm_connector_state {
>  	struct drm_connector *connector;
> @@ -216,6 +217,9 @@ struct drm_connector_state {
>  	struct drm_atomic_state *state;
>  
>  	struct drm_framebuffer *fb;  /* do not write directly, use drm_atomic_set_fb_for_connector() */

btw if you feel like adding a 2nd comment in-line like above, then that's
a clear signal that you should move your kerneldoc struct member comments
to the inline style. You can freely mix&match inline with top-level struct
member documentation, so no need to change them all. You also missed the
doc for out_fence_ptr, 0day won't like that.

> +
> +	struct fence *out_fence;
> +	u64 __user *out_fence_ptr;

writeback_ prefix for both imo, like in patch 1.

>  };
>  
>  /**
> @@ -546,6 +550,10 @@ struct drm_cmdline_mode {
>   * @tile_v_loc: vertical location of this tile
>   * @tile_h_size: horizontal size of this tile.
>   * @tile_v_size: vertical size of this tile.
> + * @fence_context: context for fence signalling
> + * @fence_lock: fence lock for the fence context
> + * @fence_seqno: seqno variable to create fences
> + * @timeline_name: fence timeline name
>   *
>   * Each connector may be connected to one or more CRTCs, or may be clonable by
>   * another connector if they can share a CRTC.  Each connector also has a specific
> @@ -694,6 +702,12 @@ struct drm_connector {
>  	uint8_t num_h_tile, num_v_tile;
>  	uint8_t tile_h_loc, tile_v_loc;
>  	uint16_t tile_h_size, tile_v_size;
> +
> +	/* fence timelines info for DRM out-fences */
> +	unsigned int fence_context;
> +	spinlock_t fence_lock;
> +	unsigned long fence_seqno;
> +	char timeline_name[32];

Should all have writeout_ prefix. And at that point I wonder a bit whether
we shouldn't just go ahead and create a struct drm_writeout_connector, to
keep this stuff nicely separate. Only change visible to drivers would be
the type of drm_writeback_connector_init, and they'd need to
allocate/embedd a different struct. Worth it imo.
-Daniel

>  };
>  
>  #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index afdc2742..01f33bc 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -16,4 +16,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  				 const struct drm_connector_funcs *funcs,
>  				 u32 *formats, int n_formats);
>  
> +struct fence *drm_writeback_get_out_fence(struct drm_connector *connector,
> +					  struct drm_connector_state *conn_state);
>  #endif
> -- 
> 1.7.9.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1509275 — Re: [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback connectors

Hi Gustavo,

It would be great if you could cast your eye over this one as-well.
My intention was to be as similar to the CRTC out-fences as possible.

Thanks,
Brian

On Wed, Oct 26, 2016 at 09:55:07AM +0100, Brian Starkey wrote:
>Add the OUT_FENCE_PTR property to writeback connectors, to enable
>userspace to get a fence which will signal once the writeback is
>complete.
>
>A timeline is added to drm_connector for use by the writeback
>out-fences. It is up to drivers to check for a fence in the
>connector_state and signal the it appropriately when their writeback has
>finished.
>
>It is not allowed to request an out-fence without a framebuffer attached
>to the connector.
>
>Signed-off-by: Brian Starkey <brian.starkey@arm.com>
>---
> drivers/gpu/drm/drm_atomic.c        |   60 +++++++++++++++++++++++---
> drivers/gpu/drm/drm_atomic_helper.c |    5 ++-
> drivers/gpu/drm/drm_writeback.c     |   80 +++++++++++++++++++++++++++++++++++
> include/drm/drm_connector.h         |   14 ++++++
> include/drm/drm_writeback.h         |    2 +
> 5 files changed, 155 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 3f8fc97..061ea13 100644
>--- a/drivers/gpu/drm/drm_atomic.c
>+++ b/drivers/gpu/drm/drm_atomic.c
>@@ -30,6 +30,7 @@
> #include <drm/drm_atomic.h>
> #include <drm/drm_mode.h>
> #include <drm/drm_plane_helper.h>
>+#include <drm/drm_writeback.h>
> #include <linux/sync_file.h>
>
> #include "drm_crtc_internal.h"
>@@ -646,6 +647,12 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
> 		return -EINVAL;
> 	}
>
>+	if (state->out_fence && !state->fb) {
>+		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] requesting out-fence without framebuffer\n",
>+				 connector->base.id, connector->name);
>+		return -EINVAL;
>+	}
>+
> 	return 0;
> }
>
>@@ -1047,6 +1054,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
> 		drm_atomic_set_fb_for_connector(state, fb);
> 		if (fb)
> 			drm_framebuffer_unreference(fb);
>+	} else if (property == config->prop_out_fence_ptr) {
>+		state->out_fence_ptr = u64_to_user_ptr(val);
> 	} else if (connector->funcs->atomic_set_property) {
> 		return connector->funcs->atomic_set_property(connector,
> 				state, property, val);
>@@ -1088,6 +1097,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> 	} else if (property == config->writeback_fb_id_property) {
> 		/* Writeback framebuffer is one-shot, write and forget */
> 		*val = 0;
>+	} else if (property == config->prop_out_fence_ptr) {
>+		*val = 0;
> 	} else if (connector->funcs->atomic_get_property) {
> 		return connector->funcs->atomic_get_property(connector,
> 				state, property, val);
>@@ -1736,6 +1747,39 @@ static int setup_out_fence(struct drm_out_fence_state *fence_state,
> 	return 0;
> }
>
>+static int setup_connector_out_fences(struct drm_atomic_state *state,
>+				      struct drm_out_fence_state *fence_state,
>+				      int *fence_idx)
>+{
>+	struct drm_connector *conn;
>+	struct drm_connector_state *conn_state;
>+	int i, ret;
>+
>+	for_each_connector_in_state(state, conn, conn_state, i) {
>+		struct fence *fence;
>+
>+		if (!conn_state->out_fence_ptr)
>+			continue;
>+
>+		fence = drm_writeback_get_out_fence(conn, conn_state);
>+		if (!fence)
>+			return -ENOMEM;
>+
>+		ret = setup_out_fence(&fence_state[(*fence_idx)++],
>+				      conn_state->out_fence_ptr,
>+				      fence);
>+		if (ret) {
>+			fence_put(fence);
>+			return ret;
>+		}
>+
>+		/* One-time usage only */
>+		conn_state->out_fence_ptr = NULL;
>+	}
>+
>+	return 0;
>+}
>+
> int drm_mode_atomic_ioctl(struct drm_device *dev,
> 			  void *data, struct drm_file *file_priv)
> {
>@@ -1868,8 +1912,8 @@ retry:
> 		drm_mode_object_unreference(obj);
> 	}
>
>-	fence_state = kcalloc(dev->mode_config.num_crtc, sizeof(*fence_state),
>-			      GFP_KERNEL);
>+	fence_state = kcalloc(dev->mode_config.num_crtc + state->num_connector,
>+			      sizeof(*fence_state), GFP_KERNEL);
> 	if (!fence_state) {
> 		ret = -ENOMEM;
> 		goto out;
>@@ -1929,10 +1973,16 @@ retry:
> 		 * Below we call drm_atomic_state_free for it.
> 		 */
> 		ret = drm_atomic_check_only(state);
>-	} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
>-		ret = drm_atomic_nonblocking_commit(state);
> 	} else {
>-		ret = drm_atomic_commit(state);
>+		ret = setup_connector_out_fences(state, fence_state,
>+						 &fence_idx);
>+		if (ret)
>+			goto out;
>+
>+		if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK)
>+			ret = drm_atomic_nonblocking_commit(state);
>+		else
>+			ret = drm_atomic_commit(state);
> 	}
>
> 	if (!ret)
>diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>index bba8672..88da299 100644
>--- a/drivers/gpu/drm/drm_atomic_helper.c
>+++ b/drivers/gpu/drm/drm_atomic_helper.c
>@@ -3234,8 +3234,9 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
> 	if (state->crtc)
> 		drm_connector_reference(connector);
>
>-	/* Don't copy over framebuffers, they are used only once */
>+	/* Don't copy over framebuffers or fences, they are used only once */
> 	state->fb = NULL;
>+	state->out_fence = NULL;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
>
>@@ -3365,6 +3366,8 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state)
> 		drm_connector_unreference(state->connector);
> 	if (state->fb)
> 		drm_framebuffer_unreference(state->fb);
>+
>+	fence_put(state->out_fence);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
>
>diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
>index 5a6e0ad..4a6ef30 100644
>--- a/drivers/gpu/drm/drm_writeback.c
>+++ b/drivers/gpu/drm/drm_writeback.c
>@@ -11,6 +11,7 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_property.h>
> #include <drm/drmP.h>
>+#include <linux/fence.h>
>
> /**
>  * DOC: overview
>@@ -32,6 +33,16 @@
>  * explicitly cleared in order to make a subsequent commit which doesn't use
>  * writeback.
>  *
>+ * Unlike with planes, when a writeback framebuffer is removed by userspace DRM
>+ * makes no attempt to remove it from active use by the connector. This is
>+ * because no method is provided to abort a writeback operation, and in any
>+ * case making a new commit whilst a writeback is ongoing is undefined (see
>+ * OUT_FENCE_PTR below). As soon as the current writeback is finished, the
>+ * framebuffer will automatically no longer be in active use. As it will also
>+ * have already been removed from the framebuffer list, there will be no way for
>+ * any userspace application to retrieve a reference to it in the intervening
>+ * period.
>+ *
>  * Writeback connectors have several additional properties, which userspace
>  * can use to query and control them:
>  *
>@@ -52,8 +63,48 @@
>  *  "PIXEL_FORMATS_SIZE":
>  *	Immutable unsigned range property storing the number of entries in the
>  *	PIXEL_FORMATS array.
>+ *
>+ *  "OUT_FENCE_PTR":
>+ *	Userspace can provide the address of a 64-bit integer in this property,
>+ *	which will be filled with a sync_file file descriptor by the kernel.
>+ *	The sync_file will signal when the associated writeback has finished.
>+ *	Userspace should wait for this fence to signal before making another
>+ *	commit affecting any of the same CRTCs, Planes or Connectors.
>+ *	**Failure to do so will result in undefined behaviour.**
>+ *	For this reason it is strongly recommended that all userspace
>+ *	applications making use of writeback connectors *always* retrieve an
>+ *	out-fence for the commit and use it appropriately.
>+ *	From userspace, this property will always read as zero.
>  */
>
>+#define fence_to_connector(x) container_of(x->lock, struct drm_connector, fence_lock)
>+
>+static const char *drm_writeback_fence_get_driver_name(struct fence *fence)
>+{
>+	struct drm_connector *connector = fence_to_connector(fence);
>+
>+	return connector->dev->driver->name;
>+}
>+
>+static const char *drm_writeback_fence_get_timeline_name(struct fence *fence)
>+{
>+	struct drm_connector *connector = fence_to_connector(fence);
>+
>+	return connector->timeline_name;
>+}
>+
>+static bool drm_writeback_fence_enable_signaling(struct fence *fence)
>+{
>+	return true;
>+}
>+
>+static const struct fence_ops drm_writeback_fence_ops = {
>+	.get_driver_name = drm_writeback_fence_get_driver_name,
>+	.get_timeline_name = drm_writeback_fence_get_timeline_name,
>+	.enable_signaling = drm_writeback_fence_enable_signaling,
>+	.wait = fence_default_wait,
>+};
>+
> /**
>  * create_writeback_properties - Create writeback connector-specific properties
>  * @dev: DRM device
>@@ -136,6 +187,14 @@ int drm_writeback_connector_init(struct drm_device *dev,
> 	if (ret)
> 		goto fail;
>
>+	connector->fence_context = fence_context_alloc(1);
>+	spin_lock_init(&connector->fence_lock);
>+	snprintf(connector->timeline_name, sizeof(connector->timeline_name),
>+		 "CONNECTOR:%d", connector->base.id);
>+
>+	drm_object_attach_property(&connector->base,
>+				   config->prop_out_fence_ptr, 0);
>+
> 	drm_object_attach_property(&connector->base,
> 				   config->writeback_fb_id_property, 0);
>
>@@ -155,3 +214,24 @@ fail:
> 	return ret;
> }
> EXPORT_SYMBOL(drm_writeback_connector_init);
>+
>+struct fence *drm_writeback_get_out_fence(struct drm_connector *connector,
>+					  struct drm_connector_state *conn_state)
>+{
>+	struct fence *fence;
>+
>+	if (WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK))
>+		return NULL;
>+
>+	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>+	if (!fence)
>+		return NULL;
>+
>+	fence_init(fence, &drm_writeback_fence_ops, &connector->fence_lock,
>+		   connector->fence_context, ++connector->fence_seqno);
>+
>+	conn_state->out_fence = fence_get(fence);
>+
>+	return fence;
>+}
>+EXPORT_SYMBOL(drm_writeback_get_out_fence);
>diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>index a5e3778..7d40537 100644
>--- a/include/drm/drm_connector.h
>+++ b/include/drm/drm_connector.h
>@@ -199,6 +199,7 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
>  * @best_encoder: can be used by helpers and drivers to select the encoder
>  * @state: backpointer to global drm_atomic_state
>  * @fb: Writeback framebuffer, for DRM_MODE_CONNECTOR_WRITEBACK
>+ * @out_fence: Fence which will clear when the framebuffer write has finished
>  */
> struct drm_connector_state {
> 	struct drm_connector *connector;
>@@ -216,6 +217,9 @@ struct drm_connector_state {
> 	struct drm_atomic_state *state;
>
> 	struct drm_framebuffer *fb;  /* do not write directly, use drm_atomic_set_fb_for_connector() */
>+
>+	struct fence *out_fence;
>+	u64 __user *out_fence_ptr;
> };
>
> /**
>@@ -546,6 +550,10 @@ struct drm_cmdline_mode {
>  * @tile_v_loc: vertical location of this tile
>  * @tile_h_size: horizontal size of this tile.
>  * @tile_v_size: vertical size of this tile.
>+ * @fence_context: context for fence signalling
>+ * @fence_lock: fence lock for the fence context
>+ * @fence_seqno: seqno variable to create fences
>+ * @timeline_name: fence timeline name
>  *
>  * Each connector may be connected to one or more CRTCs, or may be clonable by
>  * another connector if they can share a CRTC.  Each connector also has a specific
>@@ -694,6 +702,12 @@ struct drm_connector {
> 	uint8_t num_h_tile, num_v_tile;
> 	uint8_t tile_h_loc, tile_v_loc;
> 	uint16_t tile_h_size, tile_v_size;
>+
>+	/* fence timelines info for DRM out-fences */
>+	unsigned int fence_context;
>+	spinlock_t fence_lock;
>+	unsigned long fence_seqno;
>+	char timeline_name[32];
> };
>
> #define obj_to_connector(x) container_of(x, struct drm_connector, base)
>diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
>index afdc2742..01f33bc 100644
>--- a/include/drm/drm_writeback.h
>+++ b/include/drm/drm_writeback.h
>@@ -16,4 +16,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
> 				 const struct drm_connector_funcs *funcs,
> 				 u32 *formats, int n_formats);
>
>+struct fence *drm_writeback_get_out_fence(struct drm_connector *connector,
>+					  struct drm_connector_state *conn_state);
> #endif
>-- 
>1.7.9.5
>

[toc] | [prev] | [next] | [standalone]

#1509878 — Re: [RFC PATCH v2 8/9] drm: writeback: Add out-fences for writeback connectors

2016-10-26 Brian Starkey <brian.starkey@arm.com>:

> Add the OUT_FENCE_PTR property to writeback connectors, to enable
> userspace to get a fence which will signal once the writeback is
> complete.
> 
> A timeline is added to drm_connector for use by the writeback
> out-fences. It is up to drivers to check for a fence in the
> connector_state and signal the it appropriately when their writeback has
> finished.
> 
> It is not allowed to request an out-fence without a framebuffer attached
> to the connector.
> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
>  drivers/gpu/drm/drm_atomic.c        |   60 +++++++++++++++++++++++---
>  drivers/gpu/drm/drm_atomic_helper.c |    5 ++-
>  drivers/gpu/drm/drm_writeback.c     |   80 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h         |   14 ++++++
>  include/drm/drm_writeback.h         |    2 +
>  5 files changed, 155 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3f8fc97..061ea13 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -30,6 +30,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_mode.h>
>  #include <drm/drm_plane_helper.h>
> +#include <drm/drm_writeback.h>
>  #include <linux/sync_file.h>
>  
>  #include "drm_crtc_internal.h"
> @@ -646,6 +647,12 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
>  		return -EINVAL;
>  	}
>  
> +	if (state->out_fence && !state->fb) {
> +		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] requesting out-fence without framebuffer\n",
> +				 connector->base.id, connector->name);
> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1047,6 +1054,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		drm_atomic_set_fb_for_connector(state, fb);
>  		if (fb)
>  			drm_framebuffer_unreference(fb);
> +	} else if (property == config->prop_out_fence_ptr) {
> +		state->out_fence_ptr = u64_to_user_ptr(val);

We need to move this out of the conn_state. For my v6 on CRTC out fences
I added drm_atomic_set_out_fence_for_crtc() and
drm_atomic_get_out_fence_for_crtc(). See padovan/fences.

>  	} else if (connector->funcs->atomic_set_property) {
>  		return connector->funcs->atomic_set_property(connector,
>  				state, property, val);
> @@ -1088,6 +1097,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  	} else if (property == config->writeback_fb_id_property) {
>  		/* Writeback framebuffer is one-shot, write and forget */
>  		*val = 0;
> +	} else if (property == config->prop_out_fence_ptr) {
> +		*val = 0;
>  	} else if (connector->funcs->atomic_get_property) {
>  		return connector->funcs->atomic_get_property(connector,
>  				state, property, val);
> @@ -1736,6 +1747,39 @@ static int setup_out_fence(struct drm_out_fence_state *fence_state,
>  	return 0;
>  }
>  
> +static int setup_connector_out_fences(struct drm_atomic_state *state,
> +				      struct drm_out_fence_state *fence_state,
> +				      int *fence_idx)
> +{
> +	struct drm_connector *conn;
> +	struct drm_connector_state *conn_state;
> +	int i, ret;
> +
> +	for_each_connector_in_state(state, conn, conn_state, i) {
> +		struct fence *fence;
> +
> +		if (!conn_state->out_fence_ptr)
> +			continue;
> +
> +		fence = drm_writeback_get_out_fence(conn, conn_state);
> +		if (!fence)
> +			return -ENOMEM;
> +
> +		ret = setup_out_fence(&fence_state[(*fence_idx)++],
> +				      conn_state->out_fence_ptr,
> +				      fence);
> +		if (ret) {
> +			fence_put(fence);
> +			return ret;
> +		}
> +
> +		/* One-time usage only */
> +		conn_state->out_fence_ptr = NULL;
> +	}
> +
> +	return 0;
> +}
> +
>  int drm_mode_atomic_ioctl(struct drm_device *dev,
>  			  void *data, struct drm_file *file_priv)
>  {
> @@ -1868,8 +1912,8 @@ retry:
>  		drm_mode_object_unreference(obj);
>  	}
>  
> -	fence_state = kcalloc(dev->mode_config.num_crtc, sizeof(*fence_state),
> -			      GFP_KERNEL);
> +	fence_state = kcalloc(dev->mode_config.num_crtc + state->num_connector,
> +			      sizeof(*fence_state), GFP_KERNEL);
>  	if (!fence_state) {
>  		ret = -ENOMEM;
>  		goto out;
> @@ -1929,10 +1973,16 @@ retry:
>  		 * Below we call drm_atomic_state_free for it.
>  		 */
>  		ret = drm_atomic_check_only(state);
> -	} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
> -		ret = drm_atomic_nonblocking_commit(state);
>  	} else {
> -		ret = drm_atomic_commit(state);
> +		ret = setup_connector_out_fences(state, fence_state,
> +						 &fence_idx);
> +		if (ret)
> +			goto out;

I'd do this before this block. At the DRM_MODE_ATOMIC_TEST_ONLY or we
check or we commit. Moreover you should check if out_fences
are not set with TEST_ONLY.

> +
> +		if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK)
> +			ret = drm_atomic_nonblocking_commit(state);
> +		else
> +			ret = drm_atomic_commit(state);
>  	}
>  
>  	if (!ret)
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index bba8672..88da299 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3234,8 +3234,9 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
>  	if (state->crtc)
>  		drm_connector_reference(connector);
>  
> -	/* Don't copy over framebuffers, they are used only once */
> +	/* Don't copy over framebuffers or fences, they are used only once */
>  	state->fb = NULL;
> +	state->out_fence = NULL;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
>  
> @@ -3365,6 +3366,8 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state)
>  		drm_connector_unreference(state->connector);
>  	if (state->fb)
>  		drm_framebuffer_unreference(state->fb);
> +
> +	fence_put(state->out_fence);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index 5a6e0ad..4a6ef30 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -11,6 +11,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_property.h>
>  #include <drm/drmP.h>
> +#include <linux/fence.h>
>  
>  /**
>   * DOC: overview
> @@ -32,6 +33,16 @@
>   * explicitly cleared in order to make a subsequent commit which doesn't use
>   * writeback.
>   *
> + * Unlike with planes, when a writeback framebuffer is removed by userspace DRM
> + * makes no attempt to remove it from active use by the connector. This is
> + * because no method is provided to abort a writeback operation, and in any
> + * case making a new commit whilst a writeback is ongoing is undefined (see
> + * OUT_FENCE_PTR below). As soon as the current writeback is finished, the
> + * framebuffer will automatically no longer be in active use. As it will also
> + * have already been removed from the framebuffer list, there will be no way for
> + * any userspace application to retrieve a reference to it in the intervening
> + * period.
> + *
>   * Writeback connectors have several additional properties, which userspace
>   * can use to query and control them:
>   *
> @@ -52,8 +63,48 @@
>   *  "PIXEL_FORMATS_SIZE":
>   *	Immutable unsigned range property storing the number of entries in the
>   *	PIXEL_FORMATS array.
> + *
> + *  "OUT_FENCE_PTR":
> + *	Userspace can provide the address of a 64-bit integer in this property,
> + *	which will be filled with a sync_file file descriptor by the kernel.
> + *	The sync_file will signal when the associated writeback has finished.
> + *	Userspace should wait for this fence to signal before making another
> + *	commit affecting any of the same CRTCs, Planes or Connectors.
> + *	**Failure to do so will result in undefined behaviour.**
> + *	For this reason it is strongly recommended that all userspace
> + *	applications making use of writeback connectors *always* retrieve an
> + *	out-fence for the commit and use it appropriately.
> + *	From userspace, this property will always read as zero.
>   */
>  
> +#define fence_to_connector(x) container_of(x->lock, struct drm_connector, fence_lock)
> +
> +static const char *drm_writeback_fence_get_driver_name(struct fence *fence)
> +{
> +	struct drm_connector *connector = fence_to_connector(fence);
> +
> +	return connector->dev->driver->name;
> +}
> +
> +static const char *drm_writeback_fence_get_timeline_name(struct fence *fence)
> +{
> +	struct drm_connector *connector = fence_to_connector(fence);
> +
> +	return connector->timeline_name;
> +}
> +
> +static bool drm_writeback_fence_enable_signaling(struct fence *fence)
> +{
> +	return true;
> +}
> +
> +static const struct fence_ops drm_writeback_fence_ops = {
> +	.get_driver_name = drm_writeback_fence_get_driver_name,
> +	.get_timeline_name = drm_writeback_fence_get_timeline_name,
> +	.enable_signaling = drm_writeback_fence_enable_signaling,
> +	.wait = fence_default_wait,
> +};
> +
>  /**
>   * create_writeback_properties - Create writeback connector-specific properties
>   * @dev: DRM device
> @@ -136,6 +187,14 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	connector->fence_context = fence_context_alloc(1);
> +	spin_lock_init(&connector->fence_lock);
> +	snprintf(connector->timeline_name, sizeof(connector->timeline_name),
> +		 "CONNECTOR:%d", connector->base.id);
> +
> +	drm_object_attach_property(&connector->base,
> +				   config->prop_out_fence_ptr, 0);
> +
>  	drm_object_attach_property(&connector->base,
>  				   config->writeback_fb_id_property, 0);
>  
> @@ -155,3 +214,24 @@ fail:
>  	return ret;
>  }
>  EXPORT_SYMBOL(drm_writeback_connector_init);
> +
> +struct fence *drm_writeback_get_out_fence(struct drm_connector *connector,
> +					  struct drm_connector_state *conn_state)
> +{
> +	struct fence *fence;
> +
> +	if (WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK))
> +		return NULL;
> +
> +	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
> +	if (!fence)
> +		return NULL;
> +
> +	fence_init(fence, &drm_writeback_fence_ops, &connector->fence_lock,
> +		   connector->fence_context, ++connector->fence_seqno);
> +
> +	conn_state->out_fence = fence_get(fence);
> +
> +	return fence;
> +}
> +EXPORT_SYMBOL(drm_writeback_get_out_fence);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index a5e3778..7d40537 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -199,6 +199,7 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
>   * @best_encoder: can be used by helpers and drivers to select the encoder
>   * @state: backpointer to global drm_atomic_state
>   * @fb: Writeback framebuffer, for DRM_MODE_CONNECTOR_WRITEBACK
> + * @out_fence: Fence which will clear when the framebuffer write has finished
>   */
>  struct drm_connector_state {
>  	struct drm_connector *connector;
> @@ -216,6 +217,9 @@ struct drm_connector_state {
>  	struct drm_atomic_state *state;
>  
>  	struct drm_framebuffer *fb;  /* do not write directly, use drm_atomic_set_fb_for_connector() */
> +
> +	struct fence *out_fence;
> +	u64 __user *out_fence_ptr;
>  };
>  
>  /**
> @@ -546,6 +550,10 @@ struct drm_cmdline_mode {
>   * @tile_v_loc: vertical location of this tile
>   * @tile_h_size: horizontal size of this tile.
>   * @tile_v_size: vertical size of this tile.
> + * @fence_context: context for fence signalling
> + * @fence_lock: fence lock for the fence context
> + * @fence_seqno: seqno variable to create fences
> + * @timeline_name: fence timeline name
>   *
>   * Each connector may be connected to one or more CRTCs, or may be clonable by
>   * another connector if they can share a CRTC.  Each connector also has a specific
> @@ -694,6 +702,12 @@ struct drm_connector {
>  	uint8_t num_h_tile, num_v_tile;
>  	uint8_t tile_h_loc, tile_v_loc;
>  	uint16_t tile_h_size, tile_v_size;
> +
> +	/* fence timelines info for DRM out-fences */
> +	unsigned int fence_context;
> +	spinlock_t fence_lock;
> +	unsigned long fence_seqno;
> +	char timeline_name[32];
>  };
>  
>  #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index afdc2742..01f33bc 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -16,4 +16,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  				 const struct drm_connector_funcs *funcs,
>  				 u32 *formats, int n_formats);
>  
> +struct fence *drm_writeback_get_out_fence(struct drm_connector *connector,
> +					  struct drm_connector_state *conn_state);
>  #endif
> -- 
> 1.7.9.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web