[PATCH 0/2] Add writeback scaling

[PATCH 0/2] Add writeback scaling

[Ben Davis]

Add support for scaling on writeback. To do this add writeback_w and
writeback_h as writeback connector properties to specify the desired
output dimensions.
Then implement downscaling on writeback for Malidp-550 and Malidp-650
(upscaling on writeback is not supported on these devices).

Ben Davis (2):
  drm: Add writeback_w,h properties
  drm/malidp: Enable writeback scaling

 drivers/gpu/drm/arm/malidp_crtc.c | 49 ++++++++++++----------
 drivers/gpu/drm/arm/malidp_crtc.h | 12 ++++++
 drivers/gpu/drm/arm/malidp_drv.c  | 10 +++--
 drivers/gpu/drm/arm/malidp_hw.c   | 45 ++++++++++++++------
 drivers/gpu/drm/arm/malidp_hw.h   | 19 +++++++--
 drivers/gpu/drm/arm/malidp_mw.c   | 86 ++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/arm/malidp_regs.h |  1 +
 drivers/gpu/drm/drm_atomic_uapi.c |  8 ++++
 drivers/gpu/drm/drm_writeback.c   | 28 +++++++++++++
 include/drm/drm_connector.h       |  4 ++
 include/drm/drm_mode_config.h     | 10 +++++
 11 files changed, 211 insertions(+), 61 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/malidp_crtc.h

-- 
2.7.4

[PATCH 1/2] drm: Add writeback_w,h properties

[Ben Davis]

Add new properties to specify width and height for writeback.

Signed-off-by: Ben Davis <ben.davis@arm.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  8 ++++++++
 drivers/gpu/drm/drm_writeback.c   | 28 ++++++++++++++++++++++++++++
 include/drm/drm_connector.h       |  4 ++++
 include/drm/drm_mode_config.h     | 10 ++++++++++
 4 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index d520a04..1f68dce 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -765,6 +765,10 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 			return -EINVAL;
 		}
 		state->content_protection = val;
+	} else if (property == config->prop_writeback_w) {
+		state->writeback_w = val;
+	} else if (property == config->prop_writeback_h) {
+		state->writeback_h = val;
 	} else if (property == config->writeback_fb_id_property) {
 		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
 		int ret = drm_atomic_set_writeback_fb_for_connector(state, fb);
@@ -837,6 +841,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->scaling_mode;
 	} else if (property == connector->content_protection_property) {
 		*val = state->content_protection;
+	} else if (property == config->prop_writeback_w) {
+		*val = state->writeback_w;
+	} else if (property == config->prop_writeback_h) {
+		*val = state->writeback_h;
 	} else if (property == config->writeback_fb_id_property) {
 		/* Writeback framebuffer is one-shot, write and forget */
 		*val = 0;
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index c20e6fe..be42073 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -74,6 +74,12 @@
  *	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.
+ *
+ *  "WRITEBACK_W":
+ *	The width of the writeback buffer to write back.
+ *
+ *  "WRITEBACK_H":
+ *	The height of the writeback buffer to write back.
  */
 
 #define fence_to_wb_connector(x) container_of(x->lock, \
@@ -141,6 +147,22 @@ static int create_writeback_properties(struct drm_device *dev)
 		dev->mode_config.writeback_out_fence_ptr_property = prop;
 	}
 
+	if (!dev->mode_config.prop_writeback_w) {
+		prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+						 "WRITEBACK_W", 0, INT_MAX);
+		if (!prop)
+			return -ENOMEM;
+		dev->mode_config.prop_writeback_w = prop;
+	}
+
+	if (!dev->mode_config.prop_writeback_h) {
+		prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+						 "WRITEBACK_H", 0, INT_MAX);
+		if (!prop)
+			return -ENOMEM;
+		dev->mode_config.prop_writeback_h = prop;
+	}
+
 	return 0;
 }
 
@@ -225,6 +247,12 @@ int drm_writeback_connector_init(struct drm_device *dev,
 	drm_object_attach_property(&connector->base,
 				   config->writeback_pixel_formats_property,
 				   blob->base.id);
+
+	drm_object_attach_property(&connector->base,
+				   config->prop_writeback_w, 0);
+	drm_object_attach_property(&connector->base,
+				   config->prop_writeback_h, 0);
+
 	wb_connector->pixel_formats_blob_ptr = blob;
 
 	return 0;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8fe22ab..51c4cb2 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -515,6 +515,10 @@ struct drm_connector_state {
 	 */
 	struct drm_writeback_job *writeback_job;
 
+	/** @writeback_w: width of plane to write to wb buffer */
+	/** @writeback_h: height of plane to write to wb buffer */
+	uint32_t writeback_w, writeback_h;
+
 	/**
 	 * @max_requested_bpc: Connector property to limit the maximum bit
 	 * depth of the pixels.
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 7f60e8e..a0c2133 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -622,6 +622,16 @@ struct drm_mode_config {
 	 */
 	struct drm_property *prop_crtc_h;
 	/**
+	 * @prop_writeback_w: Writeback connector property for the plane
+	 * destination position in the writeback buffer.
+	 */
+	struct drm_property *prop_writeback_w;
+	/**
+	 * @prop_writeback_h: Writeback connector property for the plane
+	 * destination position in the writeback buffer.
+	 */
+	struct drm_property *prop_writeback_h;
+	/**
 	 * @prop_fb_id: Default atomic plane property to specify the
 	 * &drm_framebuffer.
 	 */
-- 
2.7.4

[PATCH 2/2] drm/malidp: Enable writeback scaling

[Ben Davis]

The phase setting part of malidp_crtc_atomic_check_scaling is refactored
to allow use in writeback scaling.

Also the enable_memwrite function prototype is simplified by directely
passing mw_state.

Signed-off-by: Ben Davis <ben.davis@arm.com>
---
 drivers/gpu/drm/arm/malidp_crtc.c | 49 ++++++++++++----------
 drivers/gpu/drm/arm/malidp_crtc.h | 12 ++++++
 drivers/gpu/drm/arm/malidp_drv.c  | 10 +++--
 drivers/gpu/drm/arm/malidp_hw.c   | 45 ++++++++++++++------
 drivers/gpu/drm/arm/malidp_hw.h   | 19 +++++++--
 drivers/gpu/drm/arm/malidp_mw.c   | 86 ++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/arm/malidp_regs.h |  1 +
 7 files changed, 161 insertions(+), 61 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/malidp_crtc.h

diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index 3f65996..aa66457 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -265,6 +265,31 @@ static int malidp_crtc_atomic_check_ctm(struct drm_crtc *crtc,
 	return 0;
 }
 
+void malidp_set_se_config_phase(struct malidp_se_config *s)
+{
+#define SE_N_PHASE 4
+#define SE_SHIFT_N_PHASE 12
+	u32 phase;
+
+	/* Calculate initial_phase and delta_phase for horizontal. */
+	phase = s->input_w;
+	s->h_init_phase = ((phase << SE_N_PHASE) / s->output_w + 1) / 2;
+
+	phase = s->input_w;
+	phase <<= (SE_SHIFT_N_PHASE + SE_N_PHASE);
+	s->h_delta_phase = phase / s->output_w;
+
+	/* Same for vertical. */
+	phase = s->input_h;
+	s->v_init_phase = ((phase << SE_N_PHASE) / s->output_h + 1) / 2;
+
+	phase = s->input_h;
+	phase <<= (SE_SHIFT_N_PHASE + SE_N_PHASE);
+	s->v_delta_phase = phase / s->output_h;
+#undef SE_N_PHASE
+#undef SE_SHIFT_N_PHASE
+}
+
 static int malidp_crtc_atomic_check_scaling(struct drm_crtc *crtc,
 					    struct drm_crtc_state *state)
 {
@@ -291,7 +316,6 @@ static int malidp_crtc_atomic_check_scaling(struct drm_crtc *crtc,
 
 	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
 		struct malidp_plane *mp = to_malidp_plane(plane);
-		u32 phase;
 
 		if (!(mp->layer->id & scaling))
 			continue;
@@ -319,27 +343,8 @@ static int malidp_crtc_atomic_check_scaling(struct drm_crtc *crtc,
 		s->output_w = pstate->crtc_w;
 		s->output_h = pstate->crtc_h;
 
-#define SE_N_PHASE 4
-#define SE_SHIFT_N_PHASE 12
-		/* Calculate initial_phase and delta_phase for horizontal. */
-		phase = s->input_w;
-		s->h_init_phase =
-				((phase << SE_N_PHASE) / s->output_w + 1) / 2;
-
-		phase = s->input_w;
-		phase <<= (SE_SHIFT_N_PHASE + SE_N_PHASE);
-		s->h_delta_phase = phase / s->output_w;
-
-		/* Same for vertical. */
-		phase = s->input_h;
-		s->v_init_phase =
-				((phase << SE_N_PHASE) / s->output_h + 1) / 2;
-
-		phase = s->input_h;
-		phase <<= (SE_SHIFT_N_PHASE + SE_N_PHASE);
-		s->v_delta_phase = phase / s->output_h;
-#undef SE_N_PHASE
-#undef SE_SHIFT_N_PHASE
+
+		malidp_set_se_config_phase(s);
 		s->plane_src_id = mp->layer->id;
 	}
 
diff --git a/drivers/gpu/drm/arm/malidp_crtc.h b/drivers/gpu/drm/arm/malidp_crtc.h
new file mode 100644
index 0000000..0f056f8
--- /dev/null
+++ b/drivers/gpu/drm/arm/malidp_crtc.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * (C) COPYRIGHT 2019 ARM Limited. All rights reserved.
+ *
+ */
+
+#include "malidp_hw.h"
+
+#ifndef __MALIDP_CRTC_H__
+#define __MALIDP_CRTC_H__
+void malidp_set_se_config_phase(struct malidp_se_config *s);
+#endif
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index d37ff9d..e2465e9 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -138,7 +138,7 @@ static void malidp_atomic_commit_se_config(struct drm_crtc *crtc,
 	u32 val;
 
 	/* Set SE_CONTROL */
-	if (!s->scale_enable) {
+	if (!s->scale_enable && !s->wb_scale_enable) {
 		val = malidp_hw_read(hwdev, se_control);
 		val &= ~MALIDP_SE_SCALING_EN;
 		malidp_hw_write(hwdev, val, se_control);
@@ -147,12 +147,16 @@ static void malidp_atomic_commit_se_config(struct drm_crtc *crtc,
 
 	hwdev->hw->se_set_scaling_coeffs(hwdev, s, old_s);
 	val = malidp_hw_read(hwdev, se_control);
-	val |= MALIDP_SE_SCALING_EN | MALIDP_SE_ALPHA_EN;
+	val |= MALIDP_SE_SCALING_EN;
 
 	val &= ~MALIDP_SE_ENH(MALIDP_SE_ENH_MASK);
 	val |= s->enhancer_enable ? MALIDP_SE_ENH(3) : 0;
 
-	val |= MALIDP_SE_RGBO_IF_EN;
+	if (s->scale_enable)
+		val |= (MALIDP_SE_RGBO_IF_EN | MALIDP_SE_ALPHA_EN);
+	else
+		val &= ~((MALIDP_SE_RGBO_IF_EN | MALIDP_SE_ALPHA_EN));
+
 	malidp_hw_write(hwdev, val, se_control);
 
 	/* Set IN_SIZE & OUT_SIZE. */
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 1a36718..6f59c0e 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -497,13 +497,18 @@ static long malidp500_se_calc_mclk(struct malidp_hw_device *hwdev,
 	return ret;
 }
 
-static int malidp500_enable_memwrite(struct malidp_hw_device *hwdev,
-				     dma_addr_t *addrs, s32 *pitches,
-				     int num_planes, u16 w, u16 h, u32 fmt_id,
-				     const s16 *rgb2yuv_coeffs)
+static int malidp500_enable_memwrite(struct malidp_hw_device *hwdev, u16 w, u16 h,
+				     struct malidp_mw_connector_state *mw_state)
 {
+
+	int num_planes = mw_state->n_planes;
+	dma_addr_t *addrs = mw_state->addrs;
+	s32 *pitches = mw_state->pitches;
+	u32 fmt_id = mw_state->format;
 	u32 base = MALIDP500_SE_MEMWRITE_BASE;
 	u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
+	const s16 *rgb2yuv_coeffs = !mw_state->rgb2yuv_initialized ?
+					mw_state->rgb2yuv_coeffs : NULL;
 
 	/* enable the scaling engine block */
 	malidp_hw_setbits(hwdev, MALIDP_SCALE_ENGINE_EN, de_base + MALIDP_DE_DISPLAY_FUNC);
@@ -847,13 +852,18 @@ static long malidp550_se_calc_mclk(struct malidp_hw_device *hwdev,
 	return ret;
 }
 
-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,
-				     const s16 *rgb2yuv_coeffs)
+static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev, u16 w, u16 h,
+				     struct malidp_mw_connector_state *mw_state)
 {
+	int num_planes = mw_state->n_planes;
+	bool scaling = mw_state->wb_scale_enable;
+	dma_addr_t *addrs = mw_state->addrs;
+	s32 *pitches = mw_state->pitches;
+	u32 fmt_id = mw_state->format;
 	u32 base = MALIDP550_SE_MEMWRITE_BASE;
 	u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
+	const s16 *rgb2yuv_coeffs = !mw_state->rgb2yuv_initialized ?
+					mw_state->rgb2yuv_coeffs : NULL;
 
 	/* enable the scaling engine block */
 	malidp_hw_setbits(hwdev, MALIDP_SCALE_ENGINE_EN, de_base + MALIDP_DE_DISPLAY_FUNC);
@@ -876,10 +886,18 @@ static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev,
 		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);
+	malidp_hw_clearbits(hwdev, MALIDP_SE_MEMWRITE_EN | MALIDP_SE_MEMWRITE_SCL_EN,
+			    MALIDP550_SE_CONTROL);
+
+	if (scaling) {
+		malidp_hw_setbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_SCL_EN,
+				  MALIDP550_SE_CONTROL);
+	} else {
+		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);
+	}
 
 	if (rgb2yuv_coeffs) {
 		int i;
@@ -897,7 +915,8 @@ 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,
+	malidp_hw_clearbits(hwdev,
+			    MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_EN | MALIDP_SE_MEMWRITE_SCL_EN,
 			    MALIDP550_SE_CONTROL);
 	malidp_hw_clearbits(hwdev, MALIDP_SCALE_ENGINE_EN, base + MALIDP_DE_DISPLAY_FUNC);
 }
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 8352a2e..61ddcb5 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -85,6 +85,7 @@ enum malidp_scaling_coeff_set {
 struct malidp_se_config {
 	u8 scale_enable : 1;
 	u8 enhancer_enable : 1;
+	u8 wb_scale_enable : 1;
 	u8 hcoeff : 3;
 	u8 vcoeff : 3;
 	u8 plane_src_id;
@@ -94,6 +95,19 @@ struct malidp_se_config {
 	u32 v_init_phase, v_delta_phase;
 };
 
+#define to_mw_state(_state) (struct malidp_mw_connector_state *)(_state)
+
+struct malidp_mw_connector_state {
+	struct drm_connector_state base;
+	dma_addr_t addrs[2];
+	s32 pitches[2];
+	u8 format;
+	u8 n_planes;
+	bool rgb2yuv_initialized;
+	const s16 *rgb2yuv_coeffs;
+	bool wb_scale_enable;
+};
+
 /* regmap features */
 #define MALIDP_REGMAP_HAS_CLEARIRQ				BIT(0)
 #define MALIDP_DEVICE_AFBC_SUPPORT_SPLIT			BIT(1)
@@ -206,9 +220,8 @@ struct malidp_hw {
 	 * @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,
-			       const s16 *rgb2yuv_coeffs);
+	int (*enable_memwrite)(struct malidp_hw_device *hwdev, u16 w, u16 h,
+			       struct malidp_mw_connector_state *mw_state);
 
 	/*
 	 * Disable the writing to memory of the next frame's content.
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index 2865f7a..2120ef1 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -13,23 +13,13 @@
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drmP.h>
 #include <drm/drm_writeback.h>
+#include <video/videomode.h>
 
+#include "malidp_crtc.h"
 #include "malidp_drv.h"
 #include "malidp_hw.h"
 #include "malidp_mw.h"
 
-#define to_mw_state(_state) (struct malidp_mw_connector_state *)(_state)
-
-struct malidp_mw_connector_state {
-	struct drm_connector_state base;
-	dma_addr_t addrs[2];
-	s32 pitches[2];
-	u8 format;
-	u8 n_planes;
-	bool rgb2yuv_initialized;
-	const s16 *rgb2yuv_coeffs;
-};
-
 static int malidp_mw_connector_get_modes(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
@@ -126,16 +116,76 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
 			       struct drm_connector_state *conn_state)
 {
 	struct malidp_mw_connector_state *mw_state = to_mw_state(conn_state);
+	struct malidp_crtc_state *cs = to_malidp_crtc_state(crtc_state);
+	struct malidp_se_config *s = &cs->scaler_config;
 	struct malidp_drm *malidp = encoder->dev->dev_private;
 	struct drm_framebuffer *fb;
-	int i, n_planes;
+	int i, n_planes, ret;
+	struct malidp_hw_device *hwdev = malidp->dev;
+	struct videomode vm;
+	u32 h_upscale_factor = 0; /* U16.16 */
+	u32 v_upscale_factor = 0; /* U16.16 */
+
+	s->wb_scale_enable = 0;
 
 	if (!conn_state->writeback_job || !conn_state->writeback_job->fb)
 		return 0;
 
+
+	if ((conn_state->writeback_w != crtc_state->mode.hdisplay) ||
+	    (conn_state->writeback_h != crtc_state->mode.vdisplay)) {
+		s->wb_scale_enable = 1;
+		mw_state->wb_scale_enable = true;
+	}
+
 	fb = conn_state->writeback_job->fb;
-	if ((fb->width != crtc_state->mode.hdisplay) ||
-	    (fb->height != crtc_state->mode.vdisplay)) {
+	if (s->wb_scale_enable) {
+		if (s->scale_enable) {
+			DRM_DEBUG_KMS("Attempting to scale on writeback while scaling a layer\n");
+			return -EINVAL;
+		}
+
+		s->input_w = crtc_state->mode.hdisplay;
+		s->input_h = crtc_state->mode.vdisplay;
+		s->output_w = conn_state->writeback_w;
+		s->output_h = conn_state->writeback_h;
+
+		if (fb->width < s->output_w || fb->height < s->output_h) {
+			DRM_DEBUG_KMS("Invalid framebuffer size %ux%u\n",
+					fb->width, fb->height);
+			return -EINVAL;
+		}
+
+		if ((s->output_w > s->input_w) || (s->output_h > s->input_h)) {
+			DRM_DEBUG_KMS("Upscaling on writeback is not supported\n");
+			return -EINVAL;
+		}
+
+		/*
+		 * Convert crtc_[w|h] to U32.32, then divide by U16.16 src_[w|h]
+		 * to get the U16.16 result.
+		 */
+		h_upscale_factor = div_u64((u64)s->output_w << 32,
+					   (u32)s->input_w << 16);
+		v_upscale_factor = div_u64((u64)s->output_h << 32,
+					   (u32)s->input_h << 16);
+
+		/* enhancer is for upscaling */
+		s->enhancer_enable = 0;
+
+		malidp_set_se_config_phase(s);
+
+		s->hcoeff = malidp_se_select_coeffs(h_upscale_factor);
+		s->vcoeff = malidp_se_select_coeffs(v_upscale_factor);
+
+		drm_display_mode_to_videomode(&crtc_state->adjusted_mode, &vm);
+		ret = hwdev->hw->se_calc_mclk(hwdev, s, &vm);
+		if (ret < 0)
+			return -EINVAL;
+	}
+
+	if (!s->wb_scale_enable && ((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;
@@ -259,11 +309,7 @@ void malidp_mw_atomic_commit(struct drm_device *drm,
 
 		drm_writeback_queue_job(mw_conn, conn_state->writeback_job);
 		conn_state->writeback_job = NULL;
-		hwdev->hw->enable_memwrite(hwdev, mw_state->addrs,
-					   mw_state->pitches, mw_state->n_planes,
-					   fb->width, fb->height, mw_state->format,
-					   !mw_state->rgb2yuv_initialized ?
-					   mw_state->rgb2yuv_coeffs : NULL);
+		hwdev->hw->enable_memwrite(hwdev, fb->width, fb->height, mw_state);
 		mw_state->rgb2yuv_initialized = !!mw_state->rgb2yuv_coeffs;
 	} else {
 		DRM_DEV_DEBUG_DRIVER(drm->dev, "Disable memwrite\n");
diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h
index a3363f3..254aa98 100644
--- a/drivers/gpu/drm/arm/malidp_regs.h
+++ b/drivers/gpu/drm/arm/malidp_regs.h
@@ -73,6 +73,7 @@
 #define   MALIDP_DISP_FUNC_CADJ		(1 << 4)
 #define   MALIDP_DISP_FUNC_ILACED	(1 << 8)
 #define   MALIDP_SCALE_ENGINE_EN	(1 << 16)
+#define   MALIDP_SE_MEMWRITE_SCL_EN	(1 << 5)
 #define   MALIDP_SE_MEMWRITE_EN		(2 << 5)
 
 /* register offsets for IRQ management */
-- 
2.7.4

Re: [PATCH 1/2] drm: Add writeback_w,h properties

[Maarten Lankhorst]

Op 10-04-2019 om 13:11 schreef Ben Davis:
> Add new properties to specify width and height for writeback.
>
> Signed-off-by: Ben Davis <ben.davis@arm.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  8 ++++++++
>  drivers/gpu/drm/drm_writeback.c   | 28 ++++++++++++++++++++++++++++
>  include/drm/drm_connector.h       |  4 ++++
>  include/drm/drm_mode_config.h     | 10 ++++++++++
>  4 files changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index d520a04..1f68dce 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -765,6 +765,10 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  			return -EINVAL;
>  		}
>  		state->content_protection = val;
> +	} else if (property == config->prop_writeback_w) {
> +		state->writeback_w = val;
> +	} else if (property == config->prop_writeback_h) {
> +		state->writeback_h = val;
>  	} else if (property == config->writeback_fb_id_property) {
>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
>  		int ret = drm_atomic_set_writeback_fb_for_connector(state, fb);
> @@ -837,6 +841,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = state->scaling_mode;
>  	} else if (property == connector->content_protection_property) {
>  		*val = state->content_protection;
> +	} else if (property == config->prop_writeback_w) {
> +		*val = state->writeback_w;
> +	} else if (property == config->prop_writeback_h) {
> +		*val = state->writeback_h;
>  	} else if (property == config->writeback_fb_id_property) {
>  		/* Writeback framebuffer is one-shot, write and forget */
>  		*val = 0;
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index c20e6fe..be42073 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -74,6 +74,12 @@
>   *	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.
> + *
> + *  "WRITEBACK_W":
> + *	The width of the writeback buffer to write back.
> + *
> + *  "WRITEBACK_H":
> + *	The height of the writeback buffer to write back.

What happens when they are not set, and left to default 0?

~Maarten

Re: [PATCH 1/2] drm: Add writeback_w,h properties

[Ben Davis]

On Wed, Apr 10, 2019 at 01:48:51PM +0200, Maarten Lankhorst wrote:
> Op 10-04-2019 om 13:11 schreef Ben Davis:
> > Add new properties to specify width and height for writeback.
> >
> > Signed-off-by: Ben Davis <ben.davis@arm.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  8 ++++++++
> >  drivers/gpu/drm/drm_writeback.c   | 28 ++++++++++++++++++++++++++++
> >  include/drm/drm_connector.h       |  4 ++++
> >  include/drm/drm_mode_config.h     | 10 ++++++++++
> >  4 files changed, 50 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index d520a04..1f68dce 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -765,6 +765,10 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> >  			return -EINVAL;
> >  		}
> >  		state->content_protection = val;
> > +	} else if (property == config->prop_writeback_w) {
> > +		state->writeback_w = val;
> > +	} else if (property == config->prop_writeback_h) {
> > +		state->writeback_h = val;
> >  	} else if (property == config->writeback_fb_id_property) {
> >  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
> >  		int ret = drm_atomic_set_writeback_fb_for_connector(state, fb);
> > @@ -837,6 +841,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >  		*val = state->scaling_mode;
> >  	} else if (property == connector->content_protection_property) {
> >  		*val = state->content_protection;
> > +	} else if (property == config->prop_writeback_w) {
> > +		*val = state->writeback_w;
> > +	} else if (property == config->prop_writeback_h) {
> > +		*val = state->writeback_h;
> >  	} else if (property == config->writeback_fb_id_property) {
> >  		/* Writeback framebuffer is one-shot, write and forget */
> >  		*val = 0;
> > diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> > index c20e6fe..be42073 100644
> > --- a/drivers/gpu/drm/drm_writeback.c
> > +++ b/drivers/gpu/drm/drm_writeback.c
> > @@ -74,6 +74,12 @@
> >   *	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.
> > + *
> > + *  "WRITEBACK_W":
> > + *	The width of the writeback buffer to write back.
> > + *
> > + *  "WRITEBACK_H":
> > + *	The height of the writeback buffer to write back.
> 
> What happens when they are not set, and left to default 0?
> 
> ~Maarten

Hi Maarten,

If they're not set they should be ignored and if writeback is enabled
it should not be scaled. However you probably shouldn't be able to set
them as 0, I can add a restriction on that.

Thanks, Ben

Re: [PATCH 1/2] drm: Add writeback_w,h properties

[Maarten Lankhorst]

Op 10-04-2019 om 18:46 schreef Ben Davis:
> On Wed, Apr 10, 2019 at 01:48:51PM +0200, Maarten Lankhorst wrote:
>> Op 10-04-2019 om 13:11 schreef Ben Davis:
>>> Add new properties to specify width and height for writeback.
>>>
>>> Signed-off-by: Ben Davis <ben.davis@arm.com>
>>> ---
>>>  drivers/gpu/drm/drm_atomic_uapi.c |  8 ++++++++
>>>  drivers/gpu/drm/drm_writeback.c   | 28 ++++++++++++++++++++++++++++
>>>  include/drm/drm_connector.h       |  4 ++++
>>>  include/drm/drm_mode_config.h     | 10 ++++++++++
>>>  4 files changed, 50 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>>> index d520a04..1f68dce 100644
>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>> @@ -765,6 +765,10 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>>>  			return -EINVAL;
>>>  		}
>>>  		state->content_protection = val;
>>> +	} else if (property == config->prop_writeback_w) {
>>> +		state->writeback_w = val;
>>> +	} else if (property == config->prop_writeback_h) {
>>> +		state->writeback_h = val;
>>>  	} else if (property == config->writeback_fb_id_property) {
>>>  		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
>>>  		int ret = drm_atomic_set_writeback_fb_for_connector(state, fb);
>>> @@ -837,6 +841,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>>>  		*val = state->scaling_mode;
>>>  	} else if (property == connector->content_protection_property) {
>>>  		*val = state->content_protection;
>>> +	} else if (property == config->prop_writeback_w) {
>>> +		*val = state->writeback_w;
>>> +	} else if (property == config->prop_writeback_h) {
>>> +		*val = state->writeback_h;
>>>  	} else if (property == config->writeback_fb_id_property) {
>>>  		/* Writeback framebuffer is one-shot, write and forget */
>>>  		*val = 0;
>>> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
>>> index c20e6fe..be42073 100644
>>> --- a/drivers/gpu/drm/drm_writeback.c
>>> +++ b/drivers/gpu/drm/drm_writeback.c
>>> @@ -74,6 +74,12 @@
>>>   *	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.
>>> + *
>>> + *  "WRITEBACK_W":
>>> + *	The width of the writeback buffer to write back.
>>> + *
>>> + *  "WRITEBACK_H":
>>> + *	The height of the writeback buffer to write back.
>> What happens when they are not set, and left to default 0?
>>
>> ~Maarten
> Hi Maarten,
>
> If they're not set they should be ignored and if writeback is enabled
> it should not be scaled. However you probably shouldn't be able to set
> them as 0, I can add a restriction on that.
I would use 0 as ignored and treat it as the default value.