linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] arm/komeda: Add side_by_side support
@ 2019-11-14  8:37 james qian wang (Arm Technology China)
  2019-11-14  8:37 ` [PATCH v3 1/6] drm/komeda: Add side by side assembling james qian wang (Arm Technology China)
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-14  8:37 UTC (permalink / raw)
  To: Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Mihail Atanassov
  Cc: Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China),
	james qian wang (Arm Technology China)

Komeda HW (two pipelines) can work on side by side mode, which splits the
internal display processing to two halves (LEFT/RIGHT) and handle them by
two pipelines separately and simultaneously.
And since one single pipeline only handles the half display frame, so the
main engine clock requirement can also be halved.

The data flow of side_by_side as blow:

 slave.layer0 ->\                  /-> slave.wb_layer -> mem.fb.right_part
     ...         -> slave.compiz ->
 slave.layer3 ->/                  \-> slave.improcessor->
                                                          \   /-> output-link0
 master.layer0 ->\                   /-> master.improcessor ->\-> output-link1
     ...          -> master.compiz ->
 master.layer3 ->/                   \-> master.wb_layer -> mem.fb.left_part

v3: Rebase

james qian wang (Arm Technology China) (6):
  drm/komeda: Add side by side assembling
  drm/komeda: Add side by side plane_state split
  drm/komeda: Build side by side display output pipeline
  drm/komeda: Add side by side support for writeback
  drm/komeda: Update writeback signal for side_by_side
  drm/komeda: Expose side_by_side by sysfs/config_id

 .../drm/arm/display/include/malidp_product.h  |   3 +-
 .../arm/display/komeda/d71/d71_component.c    |   4 +
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  54 ++--
 .../gpu/drm/arm/display/komeda/komeda_dev.c   |   4 +
 .../gpu/drm/arm/display/komeda/komeda_dev.h   |   9 +
 .../gpu/drm/arm/display/komeda/komeda_kms.h   |   8 +
 .../drm/arm/display/komeda/komeda_pipeline.c  |  50 +++-
 .../drm/arm/display/komeda/komeda_pipeline.h  |  39 ++-
 .../display/komeda/komeda_pipeline_state.c    | 277 +++++++++++++++++-
 .../gpu/drm/arm/display/komeda/komeda_plane.c |   7 +-
 .../arm/display/komeda/komeda_wb_connector.c  |  11 +-
 11 files changed, 421 insertions(+), 45 deletions(-)

--
2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/6] drm/komeda: Add side by side assembling
  2019-11-14  8:37 [PATCH v3 0/6] arm/komeda: Add side_by_side support james qian wang (Arm Technology China)
@ 2019-11-14  8:37 ` james qian wang (Arm Technology China)
  2019-11-15  0:02   ` Mihail Atanassov
  2019-11-14  8:37 ` [PATCH v3 2/6] drm/komeda: Add side by side plane_state split james qian wang (Arm Technology China)
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-14  8:37 UTC (permalink / raw)
  To: Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Mihail Atanassov
  Cc: Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China),
	james qian wang (Arm Technology China)

Komeda HW can support side by side, which splits the internal display
processing to two single halves (LEFT/RIGHT) and handle them by two
pipelines separately.
komeda "side by side" is enabled by DT property: "side_by_side_master",
once DT configured side by side, komeda need to verify it with HW's
configuration, and assemble it for the further usage.

v3: Correct a typo.

Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 13 ++++-
 .../gpu/drm/arm/display/komeda/komeda_dev.c   |  3 ++
 .../gpu/drm/arm/display/komeda/komeda_dev.h   |  9 ++++
 .../gpu/drm/arm/display/komeda/komeda_kms.h   |  3 ++
 .../drm/arm/display/komeda/komeda_pipeline.c  | 50 +++++++++++++++++--
 .../drm/arm/display/komeda/komeda_pipeline.h  |  1 +
 6 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 1c452ea75999..cee9a1692e71 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -561,21 +561,30 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
 	kms->n_crtcs = 0;
 
 	for (i = 0; i < mdev->n_pipelines; i++) {
+		/* if sbs, one komeda_dev only can represent one CRTC */
+		if (mdev->side_by_side && i != mdev->side_by_side_master)
+			continue;
+
 		crtc = &kms->crtcs[kms->n_crtcs];
 		master = mdev->pipelines[i];
 
 		crtc->master = master;
 		crtc->slave  = komeda_pipeline_get_slave(master);
+		crtc->side_by_side = mdev->side_by_side;
 
 		if (crtc->slave)
 			sprintf(str, "pipe-%d", crtc->slave->id);
 		else
 			sprintf(str, "None");
 
-		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
-			 kms->n_crtcs, master->id, str);
+		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s) sbs(%s).\n",
+			 kms->n_crtcs, master->id, str,
+			 crtc->side_by_side ? "On" : "Off");
 
 		kms->n_crtcs++;
+
+		if (mdev->side_by_side)
+			break;
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index 4e46f650fddf..c3fa4835cb8d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -178,6 +178,9 @@ static int komeda_parse_dt(struct device *dev, struct komeda_dev *mdev)
 		}
 	}
 
+	mdev->side_by_side = !of_property_read_u32(np, "side_by_side_master",
+						   &mdev->side_by_side_master);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
index d406a4d83352..471604b42431 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
@@ -183,6 +183,15 @@ struct komeda_dev {
 
 	/** @irq: irq number */
 	int irq;
+	/**
+	 * @side_by_side:
+	 *
+	 * on sbs the whole display frame will be split to two halves (1:2),
+	 * master pipeline handles the left part, slave for the right part
+	 */
+	bool side_by_side;
+	/** @side_by_side_master: master pipe id for side by side */
+	int side_by_side_master;
 
 	/** @lock: used to protect dpmode */
 	struct mutex lock;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 456f3c435719..ae6654fe95e2 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -76,6 +76,9 @@ struct komeda_crtc {
 	 */
 	struct komeda_pipeline *slave;
 
+	/** @side_by_side: if the master and slave works on side by side mode */
+	bool side_by_side;
+
 	/** @slave_planes: komeda slave planes mask */
 	u32 slave_planes;
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
index 452e505a1fd3..104e27cc1dc3 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
@@ -326,14 +326,56 @@ static void komeda_pipeline_assemble(struct komeda_pipeline *pipe)
 struct komeda_pipeline *
 komeda_pipeline_get_slave(struct komeda_pipeline *master)
 {
-	struct komeda_component *slave;
+	struct komeda_dev *mdev = master->mdev;
+	struct komeda_component *comp, *slave;
+	u32 avail_inputs;
+
+	/* on SBS, slave pipeline merge to master via image processor */
+	if (mdev->side_by_side) {
+		comp = &master->improc->base;
+		avail_inputs = KOMEDA_PIPELINE_IMPROCS;
+	} else {
+		comp = &master->compiz->base;
+		avail_inputs = KOMEDA_PIPELINE_COMPIZS;
+	}
 
-	slave = komeda_component_pickup_input(&master->compiz->base,
-					      KOMEDA_PIPELINE_COMPIZS);
+	slave = komeda_component_pickup_input(comp, avail_inputs);
 
 	return slave ? slave->pipeline : NULL;
 }
 
+static int komeda_assemble_side_by_side(struct komeda_dev *mdev)
+{
+	struct komeda_pipeline *master, *slave;
+	int i;
+
+	if (!mdev->side_by_side)
+		return 0;
+
+	if (mdev->side_by_side_master >= mdev->n_pipelines) {
+		DRM_ERROR("DT configured side by side master-%d is invalid.\n",
+			  mdev->side_by_side_master);
+		return -EINVAL;
+	}
+
+	master = mdev->pipelines[mdev->side_by_side_master];
+	slave = komeda_pipeline_get_slave(master);
+	if (!slave || slave->n_layers != master->n_layers) {
+		DRM_ERROR("Current HW doesn't support side by side.\n");
+		return -EINVAL;
+	}
+
+	if (!master->dual_link) {
+		DRM_DEBUG_ATOMIC("SBS can not work without dual link.\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < master->n_layers; i++)
+		master->layers[i]->sbs_slave = slave->layers[i];
+
+	return 0;
+}
+
 int komeda_assemble_pipelines(struct komeda_dev *mdev)
 {
 	struct komeda_pipeline *pipe;
@@ -346,7 +388,7 @@ int komeda_assemble_pipelines(struct komeda_dev *mdev)
 		komeda_pipeline_dump(pipe);
 	}
 
-	return 0;
+	return komeda_assemble_side_by_side(mdev);
 }
 
 void komeda_pipeline_dump_register(struct komeda_pipeline *pipe,
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index ac8725e24853..20a076cce635 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -237,6 +237,7 @@ struct komeda_layer {
 	 * not the source buffer.
 	 */
 	struct komeda_layer *right;
+	struct komeda_layer *sbs_slave;
 };
 
 struct komeda_layer_state {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/6] drm/komeda: Add side by side plane_state split
  2019-11-14  8:37 [PATCH v3 0/6] arm/komeda: Add side_by_side support james qian wang (Arm Technology China)
  2019-11-14  8:37 ` [PATCH v3 1/6] drm/komeda: Add side by side assembling james qian wang (Arm Technology China)
@ 2019-11-14  8:37 ` james qian wang (Arm Technology China)
  2019-11-15  0:00   ` Mihail Atanassov
  2019-11-14  8:37 ` [PATCH v3 3/6] drm/komeda: Build side by side display output pipeline james qian wang (Arm Technology China)
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-14  8:37 UTC (permalink / raw)
  To: Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Mihail Atanassov
  Cc: Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China),
	james qian wang (Arm Technology China)

On side by side mode, The full display frame will be split into two parts
(Left/Right), and each part will be handled by a single pipeline separately
master pipeline for left part, slave for right.

To simplify the usage and implementation, komeda use the following scheme
to do the side by side split
1. The planes also have been grouped into two classes:
   master-planes and slave-planes.
2. The master plane can display its image on any location of the final/full
   display frame, komeda will help to split the plane configuration to two
   parts and fed them into master and slave pipelines.
3. The slave plane only can put its display rect on the right part of the
   final display frame, and its data is only can be fed into the slave
   pipeline.

From the perspective of resource usage and assignment:
The master plane can use the resources from the master pipeline and slave
pipeline both, but slave plane only can use the slave pipeline resources.

With such scheme, the usage of master planes are same as the none
side_by_side mode. user can easily skip the slave planes and no need to
consider side_by_side for them.

Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../drm/arm/display/komeda/komeda_pipeline.h  |  33 ++-
 .../display/komeda/komeda_pipeline_state.c    | 188 ++++++++++++++++++
 .../gpu/drm/arm/display/komeda/komeda_plane.c |   7 +-
 3 files changed, 220 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index 20a076cce635..4c0946fbaac1 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -521,6 +521,20 @@ komeda_component_pickup_output(struct komeda_component *c, u32 avail_comps)
 	return komeda_pipeline_get_first_component(c->pipeline, avail_inputs);
 }
 
+static inline const char *
+komeda_data_flow_msg(struct komeda_data_flow_cfg *config)
+{
+	static char str[128];
+
+	snprintf(str, sizeof(str),
+		 "rot: %x src[x/y:%d/%d, w/h:%d/%d] disp[x/y:%d/%d, w/h:%d/%d]",
+		 config->rot,
+		 config->in_x, config->in_y, config->in_w, config->in_h,
+		 config->out_x, config->out_y, config->out_w, config->out_h);
+
+	return str;
+}
+
 struct komeda_plane_state;
 struct komeda_crtc_state;
 struct komeda_crtc;
@@ -532,22 +546,27 @@ int komeda_build_layer_data_flow(struct komeda_layer *layer,
 				 struct komeda_plane_state *kplane_st,
 				 struct komeda_crtc_state *kcrtc_st,
 				 struct komeda_data_flow_cfg *dflow);
-int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
-			      struct drm_connector_state *conn_st,
-			      struct komeda_crtc_state *kcrtc_st,
-			      struct komeda_data_flow_cfg *dflow);
-int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
-				   struct komeda_crtc_state *kcrtc_st);
-
 int komeda_build_layer_split_data_flow(struct komeda_layer *left,
 				       struct komeda_plane_state *kplane_st,
 				       struct komeda_crtc_state *kcrtc_st,
 				       struct komeda_data_flow_cfg *dflow);
+int komeda_build_layer_sbs_data_flow(struct komeda_layer *layer,
+				     struct komeda_plane_state *kplane_st,
+				     struct komeda_crtc_state *kcrtc_st,
+				     struct komeda_data_flow_cfg *dflow);
+
+int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
+			      struct drm_connector_state *conn_st,
+			      struct komeda_crtc_state *kcrtc_st,
+			      struct komeda_data_flow_cfg *dflow);
 int komeda_build_wb_split_data_flow(struct komeda_layer *wb_layer,
 				    struct drm_connector_state *conn_st,
 				    struct komeda_crtc_state *kcrtc_st,
 				    struct komeda_data_flow_cfg *dflow);
 
+int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
+				   struct komeda_crtc_state *kcrtc_st);
+
 int komeda_release_unclaimed_resources(struct komeda_pipeline *pipe,
 				       struct komeda_crtc_state *kcrtc_st);
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 0930234abb9d..5de0d231a1c3 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -1130,6 +1130,194 @@ int komeda_build_layer_split_data_flow(struct komeda_layer *left,
 	return err;
 }
 
+/* split func will split configuration of master plane to two layer data
+ * flows, which will be fed into master and slave pipeline then.
+ * NOTE: @m_dflow is first used as input argument to pass the configuration of
+ *	 master_plane. when the split is done, @*m_dflow @*s_dflow are the
+ *	 output data flow for pipeline.
+ */
+static int
+komeda_split_sbs_master_data_flow(struct komeda_crtc_state *kcrtc_st,
+				  struct komeda_data_flow_cfg **m_dflow,
+				  struct komeda_data_flow_cfg **s_dflow)
+{
+	struct komeda_data_flow_cfg *master = *m_dflow;
+	struct komeda_data_flow_cfg *slave = *s_dflow;
+	u32 disp_end = master->out_x + master->out_w;
+	u16 boundary;
+
+	pipeline_composition_size(kcrtc_st, &boundary, NULL);
+
+	if (disp_end <= boundary) {
+		/* the master viewport only located in master side, no need
+		 * slave anymore
+		 */
+		*s_dflow = NULL;
+	} else if ((master->out_x < boundary) && (disp_end > boundary)) {
+		/* the master viewport across two pipelines, split it */
+		bool flip_h = has_flip_h(master->rot);
+		bool r90  = drm_rotation_90_or_270(master->rot);
+		u32 src_x = master->in_x;
+		u32 src_y = master->in_y;
+		u32 src_w = master->in_w;
+		u32 src_h = master->in_h;
+
+		if (master->en_scaling || master->en_img_enhancement) {
+			DRM_DEBUG_ATOMIC("sbs doesn't support to split a scaling image.\n");
+			return -EINVAL;
+		}
+
+		memcpy(slave, master, sizeof(*master));
+
+		/* master for left part of display, slave for the right part */
+		/* split the disp_rect */
+		master->out_w = boundary - master->out_x;
+		slave->out_w  = disp_end - boundary;
+		slave->out_x  = 0;
+
+		if (r90) {
+			master->in_h = master->out_w;
+			slave->in_h = slave->out_w;
+
+			if (flip_h)
+				master->in_y = src_y + src_h - master->in_h;
+			else
+				slave->in_y = src_y + src_h - slave->in_h;
+		} else {
+			master->in_w = master->out_w;
+			slave->in_w = slave->out_w;
+
+			/* on flip_h, the left display content from the right-source */
+			if (flip_h)
+				master->in_x = src_w + src_x - master->in_w;
+			else
+				slave->in_x = src_w + src_x - slave->in_w;
+		}
+	} else if (master->out_x >= boundary) {
+		/* disp_rect only locate in right part, move the dflow to slave */
+		master->out_x -= boundary;
+		*s_dflow = master;
+		*m_dflow = NULL;
+	}
+
+	return 0;
+}
+
+static int
+komeda_split_sbs_slave_data_flow(struct komeda_crtc_state *kcrtc_st,
+				 struct komeda_data_flow_cfg *slave)
+{
+	u16 boundary;
+
+	pipeline_composition_size(kcrtc_st, &boundary, NULL);
+
+	if (slave->out_x < boundary) {
+		DRM_DEBUG_ATOMIC("SBS Slave plane is only allowed to configure the right part frame.\n");
+		return -EINVAL;
+	}
+
+	/* slave->disp_rect locate in the right part */
+	slave->out_x -= boundary;
+
+	return 0;
+}
+
+/* On side by side mode, The full display frame will be split to two parts
+ * (Left/Right), and each part will be handled by a single pipeline separately,
+ * master pipeline for left part, slave for right.
+ *
+ * To simplify the usage and implementation, komeda use the following scheme
+ * to do the side by side split
+ * 1. The planes also have been grouped into two classes:
+ *    master-planes and slave-planes.
+ * 2. The master plane can display its image on any location of the final/full
+ *    display frame, komeda will help to split the plane configuration to two
+ *    parts and fed them into master and slave pipelines.
+ * 3. The slave plane only can put its display rect on the right part of the
+ *    final display frame, and its data is only can be fed into the slave
+ *    pipeline.
+ *
+ * From the perspective of resource usage and assignment:
+ * The master plane can use the resources from the master pipeline and slave
+ * pipeline both, but slave plane only can use the slave pipeline resources.
+ *
+ * With such scheme, the usage of master planes are same as the none
+ * side_by_side mode. user can easily skip the slave planes and no need to
+ * consider side_by_side for them.
+ *
+ * NOTE: side_by_side split is occurred on pipeline level which split the plane
+ *	 data flow into pipelines, but the layer split is a pipeline
+ *	 internal split which splits the data flow into pipeline layers.
+ *	 So komeda still supports to apply a further layer split to the sbs
+ *	 split data flow.
+ */
+int komeda_build_layer_sbs_data_flow(struct komeda_layer *layer,
+				     struct komeda_plane_state *kplane_st,
+				     struct komeda_crtc_state *kcrtc_st,
+				     struct komeda_data_flow_cfg *dflow)
+{
+	struct komeda_crtc *kcrtc = to_kcrtc(kcrtc_st->base.crtc);
+	struct drm_plane *plane = kplane_st->base.plane;
+	struct komeda_data_flow_cfg temp, *master_dflow, *slave_dflow;
+	struct komeda_layer *master, *slave;
+	bool master_plane = layer->base.pipeline == kcrtc->master;
+	int err;
+
+	DRM_DEBUG_ATOMIC("SBS prepare %s-[PLANE:%d:%s]: %s.\n",
+			 master_plane ? "Master" : "Slave",
+			 plane->base.id, plane->name,
+			 komeda_data_flow_msg(dflow));
+
+	if (master_plane) {
+		master = layer;
+		slave = layer->sbs_slave;
+		master_dflow = dflow;
+		slave_dflow  = &temp;
+		err = komeda_split_sbs_master_data_flow(kcrtc_st,
+				&master_dflow, &slave_dflow);
+	} else {
+		master = NULL;
+		slave = layer;
+		master_dflow = NULL;
+		slave_dflow = dflow;
+		err = komeda_split_sbs_slave_data_flow(kcrtc_st, slave_dflow);
+	}
+
+	if (err)
+		return err;
+
+	if (master_dflow) {
+		DRM_DEBUG_ATOMIC("SBS Master-%s assigned: %s\n",
+			master->base.name, komeda_data_flow_msg(master_dflow));
+
+		if (master_dflow->en_split)
+			err = komeda_build_layer_split_data_flow(master,
+					kplane_st, kcrtc_st, master_dflow);
+		else
+			err = komeda_build_layer_data_flow(master,
+					kplane_st, kcrtc_st, master_dflow);
+
+		if (err)
+			return err;
+	}
+
+	if (slave_dflow) {
+		DRM_DEBUG_ATOMIC("SBS Slave-%s assigned: %s\n",
+			slave->base.name, komeda_data_flow_msg(slave_dflow));
+
+		if (slave_dflow->en_split)
+			err = komeda_build_layer_split_data_flow(slave,
+					kplane_st, kcrtc_st, slave_dflow);
+		else
+			err = komeda_build_layer_data_flow(slave,
+					kplane_st, kcrtc_st, slave_dflow);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 /* writeback data path: compiz -> scaler -> wb_layer -> memory */
 int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
 			      struct drm_connector_state *conn_st,
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index 98e915e325dd..2644f0727570 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -77,6 +77,7 @@ komeda_plane_atomic_check(struct drm_plane *plane,
 	struct komeda_plane_state *kplane_st = to_kplane_st(state);
 	struct komeda_layer *layer = kplane->layer;
 	struct drm_crtc_state *crtc_st;
+	struct komeda_crtc *kcrtc;
 	struct komeda_crtc_state *kcrtc_st;
 	struct komeda_data_flow_cfg dflow;
 	int err;
@@ -94,13 +95,17 @@ komeda_plane_atomic_check(struct drm_plane *plane,
 	if (!crtc_st->active)
 		return 0;
 
+	kcrtc = to_kcrtc(crtc_st->crtc);
 	kcrtc_st = to_kcrtc_st(crtc_st);
 
 	err = komeda_plane_init_data_flow(state, kcrtc_st, &dflow);
 	if (err)
 		return err;
 
-	if (dflow.en_split)
+	if (kcrtc->side_by_side)
+		err = komeda_build_layer_sbs_data_flow(layer,
+				kplane_st, kcrtc_st, &dflow);
+	else if (dflow.en_split)
 		err = komeda_build_layer_split_data_flow(layer,
 				kplane_st, kcrtc_st, &dflow);
 	else
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/6] drm/komeda: Build side by side display output pipeline
  2019-11-14  8:37 [PATCH v3 0/6] arm/komeda: Add side_by_side support james qian wang (Arm Technology China)
  2019-11-14  8:37 ` [PATCH v3 1/6] drm/komeda: Add side by side assembling james qian wang (Arm Technology China)
  2019-11-14  8:37 ` [PATCH v3 2/6] drm/komeda: Add side by side plane_state split james qian wang (Arm Technology China)
@ 2019-11-14  8:37 ` james qian wang (Arm Technology China)
  2019-11-14  8:37 ` [PATCH v3 4/6] drm/komeda: Add side by side support for writeback james qian wang (Arm Technology China)
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-14  8:37 UTC (permalink / raw)
  To: Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Mihail Atanassov
  Cc: Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China),
	james qian wang (Arm Technology China)

For side by side, the slave pipeline merges to master via image processor

 slave-layers -> slave-compiz-> slave-improc-
                                             \
 master-layers -> master-compiz -------------> master-improc ->

v3: Rebase.

Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../arm/display/komeda/d71/d71_component.c    |  4 ++
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 18 +++++--
 .../drm/arm/display/komeda/komeda_pipeline.h  |  1 +
 .../display/komeda/komeda_pipeline_state.c    | 51 ++++++++++++++-----
 .../arm/display/komeda/komeda_wb_connector.c  |  2 +-
 5 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index b6517c46e670..6dadf4413ef3 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -1085,6 +1085,10 @@ static void d71_improc_update(struct komeda_component *c,
 	else if (st->color_format == DRM_COLOR_FORMAT_YCRCB444)
 		ctrl |= IPS_CTRL_YUV;
 
+	/* slave input has been enabled, means side by side */
+	if (has_bit(1, state->active_inputs))
+		ctrl |= IPS_CTRL_SBS;
+
 	malidp_write32_mask(reg, BLK_CONTROL, mask, ctrl);
 }
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index cee9a1692e71..24928b922fbd 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -385,15 +385,23 @@ komeda_crtc_atomic_flush(struct drm_crtc *crtc,
 	komeda_crtc_do_flush(crtc, old);
 }
 
-/* Returns the minimum frequency of the aclk rate (main engine clock) in Hz */
+/*
+ * Returns the minimum frequency of the aclk rate (main engine clock) in Hz.
+ *
+ * The DPU output can be split into two halves, to stay within the bandwidth
+ * capabilities of the external link (dual-link mode).
+ * In these cases, each output link runs at half the pixel clock rate of the
+ * combined display, and has half the number of pixels.
+ * Beside split the output, the DPU internal pixel processing also can be split
+ * into two halves (LEFT/RIGHT) and handles by two pipelines simultaneously.
+ * So if side by side, the pipeline (main engine clock) also can run at half
+ * the clock rate of the combined display.
+ */
 static unsigned long
 komeda_calc_min_aclk_rate(struct komeda_crtc *kcrtc,
 			  unsigned long pxlclk)
 {
-	/* Once dual-link one display pipeline drives two display outputs,
-	 * the aclk needs run on the double rate of pxlclk
-	 */
-	if (kcrtc->master->dual_link)
+	if (kcrtc->master->dual_link && !kcrtc->side_by_side)
 		return pxlclk * 2;
 	else
 		return pxlclk;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index 4c0946fbaac1..59a81b4476df 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -540,6 +540,7 @@ struct komeda_crtc_state;
 struct komeda_crtc;
 
 void pipeline_composition_size(struct komeda_crtc_state *kcrtc_st,
+			       bool side_by_side,
 			       u16 *hsize, u16 *vsize);
 
 int komeda_build_layer_data_flow(struct komeda_layer *layer,
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 5de0d231a1c3..4dbf71455d1d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -654,12 +654,13 @@ komeda_merger_validate(struct komeda_merger *merger,
 }
 
 void pipeline_composition_size(struct komeda_crtc_state *kcrtc_st,
+			       bool side_by_side,
 			       u16 *hsize, u16 *vsize)
 {
 	struct drm_display_mode *m = &kcrtc_st->base.adjusted_mode;
 
 	if (hsize)
-		*hsize = m->hdisplay;
+		*hsize = side_by_side ? m->hdisplay / 2 : m->hdisplay;
 	if (vsize)
 		*vsize = m->vdisplay;
 }
@@ -670,12 +671,14 @@ komeda_compiz_set_input(struct komeda_compiz *compiz,
 			struct komeda_data_flow_cfg *dflow)
 {
 	struct drm_atomic_state *drm_st = kcrtc_st->base.state;
+	struct drm_crtc *crtc = kcrtc_st->base.crtc;
 	struct komeda_component_state *c_st, *old_st;
 	struct komeda_compiz_input_cfg *cin;
 	u16 compiz_w, compiz_h;
 	int idx = dflow->blending_zorder;
 
-	pipeline_composition_size(kcrtc_st, &compiz_w, &compiz_h);
+	pipeline_composition_size(kcrtc_st, to_kcrtc(crtc)->side_by_side,
+				  &compiz_w, &compiz_h);
 	/* check display rect */
 	if ((dflow->out_x + dflow->out_w > compiz_w) ||
 	    (dflow->out_y + dflow->out_h > compiz_h) ||
@@ -687,7 +690,7 @@ komeda_compiz_set_input(struct komeda_compiz *compiz,
 	}
 
 	c_st = komeda_component_get_state_and_set_user(&compiz->base, drm_st,
-			kcrtc_st->base.crtc, kcrtc_st->base.crtc);
+			crtc, crtc);
 	if (IS_ERR(c_st))
 		return PTR_ERR(c_st);
 
@@ -721,17 +724,19 @@ komeda_compiz_validate(struct komeda_compiz *compiz,
 		       struct komeda_crtc_state *state,
 		       struct komeda_data_flow_cfg *dflow)
 {
+	struct drm_crtc *crtc = state->base.crtc;
 	struct komeda_component_state *c_st;
 	struct komeda_compiz_state *st;
 
 	c_st = komeda_component_get_state_and_set_user(&compiz->base,
-			state->base.state, state->base.crtc, state->base.crtc);
+			state->base.state, crtc, crtc);
 	if (IS_ERR(c_st))
 		return PTR_ERR(c_st);
 
 	st = to_compiz_st(c_st);
 
-	pipeline_composition_size(state, &st->hsize, &st->vsize);
+	pipeline_composition_size(state, to_kcrtc(crtc)->side_by_side,
+				  &st->hsize, &st->vsize);
 
 	komeda_component_set_output(&dflow->input, &compiz->base, 0);
 
@@ -757,7 +762,8 @@ komeda_compiz_validate(struct komeda_compiz *compiz,
 static int
 komeda_improc_validate(struct komeda_improc *improc,
 		       struct komeda_crtc_state *kcrtc_st,
-		       struct komeda_data_flow_cfg *dflow)
+		       struct komeda_data_flow_cfg *m_dflow,
+		       struct komeda_data_flow_cfg *s_dflow)
 {
 	struct drm_crtc *crtc = kcrtc_st->base.crtc;
 	struct drm_crtc_state *crtc_st = &kcrtc_st->base;
@@ -771,8 +777,8 @@ komeda_improc_validate(struct komeda_improc *improc,
 
 	st = to_improc_st(c_st);
 
-	st->hsize = dflow->in_w;
-	st->vsize = dflow->in_h;
+	st->hsize = m_dflow->in_w;
+	st->vsize = m_dflow->in_h;
 
 	if (drm_atomic_crtc_needs_modeset(crtc_st)) {
 		u32 output_depths, output_formats;
@@ -808,8 +814,10 @@ komeda_improc_validate(struct komeda_improc *improc,
 		drm_ctm_to_coeffs(kcrtc_st->base.ctm, st->ctm_coeffs);
 	}
 
-	komeda_component_add_input(&st->base, &dflow->input, 0);
-	komeda_component_set_output(&dflow->input, &improc->base, 0);
+	komeda_component_add_input(&st->base, &m_dflow->input, 0);
+	if (s_dflow)
+		komeda_component_add_input(&st->base, &s_dflow->input, 1);
+	komeda_component_set_output(&m_dflow->input, &improc->base, 0);
 
 	return 0;
 }
@@ -1146,7 +1154,7 @@ komeda_split_sbs_master_data_flow(struct komeda_crtc_state *kcrtc_st,
 	u32 disp_end = master->out_x + master->out_w;
 	u16 boundary;
 
-	pipeline_composition_size(kcrtc_st, &boundary, NULL);
+	pipeline_composition_size(kcrtc_st, true, &boundary, NULL);
 
 	if (disp_end <= boundary) {
 		/* the master viewport only located in master side, no need
@@ -1209,7 +1217,7 @@ komeda_split_sbs_slave_data_flow(struct komeda_crtc_state *kcrtc_st,
 {
 	u16 boundary;
 
-	pipeline_composition_size(kcrtc_st, &boundary, NULL);
+	pipeline_composition_size(kcrtc_st, true, &boundary, NULL);
 
 	if (slave->out_x < boundary) {
 		DRM_DEBUG_ATOMIC("SBS Slave plane is only allowed to configure the right part frame.\n");
@@ -1384,7 +1392,20 @@ int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
 	memset(&m_dflow, 0, sizeof(m_dflow));
 	memset(&s_dflow, 0, sizeof(s_dflow));
 
-	if (slave && has_bit(slave->id, kcrtc_st->active_pipes)) {
+	/* build slave output data flow */
+	if (kcrtc->side_by_side) {
+		/* on side by side, the slave data flows into the improc of
+		 * itself first, and then merge it into master's image processor
+		 */
+		err = komeda_compiz_validate(slave->compiz, kcrtc_st, &s_dflow);
+		if (err)
+			return err;
+
+		err = komeda_improc_validate(slave->improc, kcrtc_st,
+					     &s_dflow, NULL);
+		if (err)
+			return err;
+	} else if (slave && has_bit(slave->id, kcrtc_st->active_pipes)) {
 		err = komeda_compiz_validate(slave->compiz, kcrtc_st, &s_dflow);
 		if (err)
 			return err;
@@ -1400,7 +1421,9 @@ int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
 	if (err)
 		return err;
 
-	err = komeda_improc_validate(master->improc, kcrtc_st, &m_dflow);
+	/* on side by side, merge the slave dflow into master */
+	err = komeda_improc_validate(master->improc, kcrtc_st, &m_dflow,
+				     kcrtc->side_by_side ? &s_dflow : NULL);
 	if (err)
 		return err;
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index e465cc4879c9..17ea021488aa 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -21,7 +21,7 @@ komeda_wb_init_data_flow(struct komeda_layer *wb_layer,
 	dflow->out_h = fb->height;
 
 	/* the write back data comes from the compiz */
-	pipeline_composition_size(kcrtc_st, &dflow->in_w, &dflow->in_h);
+	pipeline_composition_size(kcrtc_st, false, &dflow->in_w, &dflow->in_h);
 	dflow->input.component = &wb_layer->base.pipeline->compiz->base;
 	/* compiz doesn't output alpha */
 	dflow->pixel_blend_mode = DRM_MODE_BLEND_PIXEL_NONE;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/6] drm/komeda: Add side by side support for writeback
  2019-11-14  8:37 [PATCH v3 0/6] arm/komeda: Add side_by_side support james qian wang (Arm Technology China)
                   ` (2 preceding siblings ...)
  2019-11-14  8:37 ` [PATCH v3 3/6] drm/komeda: Build side by side display output pipeline james qian wang (Arm Technology China)
@ 2019-11-14  8:37 ` james qian wang (Arm Technology China)
  2019-11-14  8:37 ` [PATCH v3 5/6] drm/komeda: Update writeback signal for side_by_side james qian wang (Arm Technology China)
  2019-11-14  8:37 ` [PATCH v3 6/6] drm/komeda: Expose side_by_side by sysfs/config_id james qian wang (Arm Technology China)
  5 siblings, 0 replies; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-14  8:37 UTC (permalink / raw)
  To: Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Mihail Atanassov
  Cc: Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China),
	james qian wang (Arm Technology China)

In side by side mode, the master pipeline writeback the left frame and the
slave writeback the right part, the data flow as below:

  slave.compiz -> slave.wb_layer -> fb (right-part)
  master.compiz -> master.wb_layer -> fb (left-part)

Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../drm/arm/display/komeda/komeda_pipeline.h  |  4 ++
 .../display/komeda/komeda_pipeline_state.c    | 42 +++++++++++++++++++
 .../arm/display/komeda/komeda_wb_connector.c  |  6 ++-
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index 59a81b4476df..76621a972803 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -564,6 +564,10 @@ int komeda_build_wb_split_data_flow(struct komeda_layer *wb_layer,
 				    struct drm_connector_state *conn_st,
 				    struct komeda_crtc_state *kcrtc_st,
 				    struct komeda_data_flow_cfg *dflow);
+int komeda_build_wb_sbs_data_flow(struct komeda_crtc *kcrtc,
+				  struct drm_connector_state *conn_st,
+				  struct komeda_crtc_state *kcrtc_st,
+				  struct komeda_data_flow_cfg *wb_dflow);
 
 int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
 				   struct komeda_crtc_state *kcrtc_st);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 4dbf71455d1d..ab4d9ad79083 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -1377,6 +1377,48 @@ int komeda_build_wb_split_data_flow(struct komeda_layer *wb_layer,
 	return komeda_wb_layer_validate(wb_layer, conn_st, dflow);
 }
 
+/* writeback side by side split data path:
+ *
+ * slave.compiz -> slave.wb_layer - > fb (right-part)
+ * master.compiz -> master.wb_layer -> fb (left-part)
+ */
+int komeda_build_wb_sbs_data_flow(struct komeda_crtc *kcrtc,
+				  struct drm_connector_state *conn_st,
+				  struct komeda_crtc_state *kcrtc_st,
+				  struct komeda_data_flow_cfg *wb_dflow)
+{
+	struct komeda_pipeline *master = kcrtc->master;
+	struct komeda_pipeline *slave = kcrtc->slave;
+	struct komeda_data_flow_cfg m_dflow, s_dflow;
+	int err;
+
+	if (wb_dflow->en_scaling || wb_dflow->en_img_enhancement) {
+		DRM_DEBUG_ATOMIC("sbs doesn't support WB_scaling\n");
+		return -EINVAL;
+	}
+
+	memcpy(&m_dflow, wb_dflow, sizeof(*wb_dflow));
+	memcpy(&s_dflow, wb_dflow, sizeof(*wb_dflow));
+
+	/* master writeout the left part */
+	m_dflow.in_w >>= 1;
+	m_dflow.out_w >>= 1;
+	m_dflow.input.component = &master->compiz->base;
+
+	/* slave writeout the right part */
+	s_dflow.in_w >>= 1;
+	s_dflow.out_w >>= 1;
+	s_dflow.in_x += m_dflow.in_w;
+	s_dflow.out_x += m_dflow.out_w;
+	s_dflow.input.component = &slave->compiz->base;
+
+	err = komeda_wb_layer_validate(master->wb_layer, conn_st, &m_dflow);
+	if (err)
+		return err;
+
+	return komeda_wb_layer_validate(slave->wb_layer, conn_st, &s_dflow);
+}
+
 /* build display output data flow, the data path is:
  * compiz -> improc -> timing_ctrlr
  */
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index 17ea021488aa..44e628747654 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -37,6 +37,7 @@ komeda_wb_encoder_atomic_check(struct drm_encoder *encoder,
 			       struct drm_crtc_state *crtc_st,
 			       struct drm_connector_state *conn_st)
 {
+	struct komeda_crtc *kcrtc = to_kcrtc(crtc_st->crtc);
 	struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(crtc_st);
 	struct drm_writeback_job *writeback_job = conn_st->writeback_job;
 	struct komeda_layer *wb_layer;
@@ -65,7 +66,10 @@ komeda_wb_encoder_atomic_check(struct drm_encoder *encoder,
 	if (err)
 		return err;
 
-	if (dflow.en_split)
+	if (kcrtc->side_by_side)
+		err = komeda_build_wb_sbs_data_flow(kcrtc,
+				conn_st, kcrtc_st, &dflow);
+	else if (dflow.en_split)
 		err = komeda_build_wb_split_data_flow(wb_layer,
 				conn_st, kcrtc_st, &dflow);
 	else
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 5/6] drm/komeda: Update writeback signal for side_by_side
  2019-11-14  8:37 [PATCH v3 0/6] arm/komeda: Add side_by_side support james qian wang (Arm Technology China)
                   ` (3 preceding siblings ...)
  2019-11-14  8:37 ` [PATCH v3 4/6] drm/komeda: Add side by side support for writeback james qian wang (Arm Technology China)
@ 2019-11-14  8:37 ` james qian wang (Arm Technology China)
  2019-11-14  8:37 ` [PATCH v3 6/6] drm/komeda: Expose side_by_side by sysfs/config_id james qian wang (Arm Technology China)
  5 siblings, 0 replies; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-14  8:37 UTC (permalink / raw)
  To: Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Mihail Atanassov
  Cc: Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China),
	james qian wang (Arm Technology China)

In side by side mode, a writeback job is completed by two pipelines: left
by master and right by slave, we need to wait both pipeline finished (EOW),
then can signal the writeback job completion.

Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 23 ++++++++++---------
 .../gpu/drm/arm/display/komeda/komeda_kms.h   |  5 ++++
 .../arm/display/komeda/komeda_wb_connector.c  |  3 +++
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 24928b922fbd..78351b7135f8 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -193,27 +193,28 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
 	return err;
 }
 
-void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
+void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
 			      struct komeda_events *evts)
 {
 	struct drm_crtc *crtc = &kcrtc->base;
+	struct komeda_wb_connector *wb_conn = kcrtc->wb_conn;
 	u32 events = evts->pipes[kcrtc->master->id];
 
 	if (events & KOMEDA_EVENT_VSYNC)
 		drm_crtc_handle_vblank(crtc);
 
-	if (events & KOMEDA_EVENT_EOW) {
-		struct komeda_wb_connector *wb_conn = kcrtc->wb_conn;
+	/* handles writeback event */
+	if (events & KOMEDA_EVENT_EOW)
+		wb_conn->complete_pipes |= BIT(kcrtc->master->id);
 
-		if (wb_conn)
-			drm_writeback_signal_completion(&wb_conn->base, 0);
-		else
-			DRM_WARN("CRTC[%d]: EOW happen but no wb_connector.\n",
-				 drm_crtc_index(&kcrtc->base));
+	if (kcrtc->side_by_side &&
+	    (evts->pipes[kcrtc->slave->id] & KOMEDA_EVENT_EOW))
+		wb_conn->complete_pipes |= BIT(kcrtc->slave->id);
+
+	if (wb_conn->expected_pipes == wb_conn->complete_pipes) {
+		wb_conn->complete_pipes = 0;
+		drm_writeback_signal_completion(&wb_conn->base, 0);
 	}
-	/* will handle it together with the write back support */
-	if (events & KOMEDA_EVENT_EOW)
-		DRM_DEBUG("EOW.\n");
 
 	if (events & KOMEDA_EVENT_FLIP) {
 		unsigned long flags;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index ae6654fe95e2..174fb0a0b49b 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -58,6 +58,11 @@ struct komeda_wb_connector {
 
 	/** @wb_layer: represents associated writeback pipeline of komeda */
 	struct komeda_layer *wb_layer;
+
+	/** @expected_pipes: pipelines are used for the writeback job */
+	u32 expected_pipes;
+	/** @complete_pipes: pipelines which have finished writeback */
+	u32 complete_pipes;
 };
 
 /**
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index 44e628747654..d6833ea3b822 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -157,6 +157,9 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
 		return -ENOMEM;
 
 	kwb_conn->wb_layer = kcrtc->master->wb_layer;
+	kwb_conn->expected_pipes = BIT(kcrtc->master->id);
+	if (kcrtc->side_by_side)
+		kwb_conn->expected_pipes |= BIT(kcrtc->slave->id);
 
 	wb_conn = &kwb_conn->base;
 	wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(&kcrtc->base));
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 6/6] drm/komeda: Expose side_by_side by sysfs/config_id
  2019-11-14  8:37 [PATCH v3 0/6] arm/komeda: Add side_by_side support james qian wang (Arm Technology China)
                   ` (4 preceding siblings ...)
  2019-11-14  8:37 ` [PATCH v3 5/6] drm/komeda: Update writeback signal for side_by_side james qian wang (Arm Technology China)
@ 2019-11-14  8:37 ` james qian wang (Arm Technology China)
  5 siblings, 0 replies; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-14  8:37 UTC (permalink / raw)
  To: Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Mihail Atanassov
  Cc: Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China),
	james qian wang (Arm Technology China)

There are some restrictions if HW works on side_by_side, expose it via
config_id to user.

Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
---
 drivers/gpu/drm/arm/display/include/malidp_product.h | 3 ++-
 drivers/gpu/drm/arm/display/komeda/komeda_dev.c      | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h b/drivers/gpu/drm/arm/display/include/malidp_product.h
index 1053b11352eb..96e2e4016250 100644
--- a/drivers/gpu/drm/arm/display/include/malidp_product.h
+++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
@@ -27,7 +27,8 @@ union komeda_config_id {
 			n_scalers:2, /* number of scalers per pipeline */
 			n_layers:3, /* number of layers per pipeline */
 			n_richs:3, /* number of rich layers per pipeline */
-			reserved_bits:6;
+			side_by_side:1, /* if HW works on side_by_side mode */
+			reserved_bits:5;
 	};
 	__u32 value;
 };
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index c3fa4835cb8d..4dd4699d4e3d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -83,6 +83,7 @@ config_id_show(struct device *dev, struct device_attribute *attr, char *buf)
 	memset(&config_id, 0, sizeof(config_id));
 
 	config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
+	config_id.side_by_side = mdev->side_by_side;
 	config_id.n_pipelines = mdev->n_pipelines;
 	config_id.n_scalers = pipe->n_scalers;
 	config_id.n_layers = pipe->n_layers;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/6] drm/komeda: Add side by side plane_state split
  2019-11-14  8:37 ` [PATCH v3 2/6] drm/komeda: Add side by side plane_state split james qian wang (Arm Technology China)
@ 2019-11-15  0:00   ` Mihail Atanassov
  2019-11-19  8:42     ` james qian wang (Arm Technology China)
  0 siblings, 1 reply; 11+ messages in thread
From: Mihail Atanassov @ 2019-11-15  0:00 UTC (permalink / raw)
  To: james qian wang (Arm Technology China)
  Cc: nd, Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China)

On Thursday, 14 November 2019 08:37:31 GMT james qian wang (Arm Technology China) wrote:
> On side by side mode, The full display frame will be split into two parts
> (Left/Right), and each part will be handled by a single pipeline separately
> master pipeline for left part, slave for right.
> 
> To simplify the usage and implementation, komeda use the following scheme
> to do the side by side split
> 1. The planes also have been grouped into two classes:
>    master-planes and slave-planes.
> 2. The master plane can display its image on any location of the final/full
>    display frame, komeda will help to split the plane configuration to two
>    parts and fed them into master and slave pipelines.
> 3. The slave plane only can put its display rect on the right part of the
>    final display frame, and its data is only can be fed into the slave
>    pipeline.
> 
> From the perspective of resource usage and assignment:
> The master plane can use the resources from the master pipeline and slave
> pipeline both, but slave plane only can use the slave pipeline resources.
> 
> With such scheme, the usage of master planes are same as the none
> side_by_side mode. user can easily skip the slave planes and no need to
> consider side_by_side for them.
> 
> Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
> ---
>  .../drm/arm/display/komeda/komeda_pipeline.h  |  33 ++-
>  .../display/komeda/komeda_pipeline_state.c    | 188 ++++++++++++++++++
>  .../gpu/drm/arm/display/komeda/komeda_plane.c |   7 +-
>  3 files changed, 220 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index 20a076cce635..4c0946fbaac1 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -521,6 +521,20 @@ komeda_component_pickup_output(struct komeda_component *c, u32 avail_comps)
>  	return komeda_pipeline_get_first_component(c->pipeline, avail_inputs);
>  }
>  
> +static inline const char *
> +komeda_data_flow_msg(struct komeda_data_flow_cfg *config)
> +{
> +	static char str[128];
> +
> +	snprintf(str, sizeof(str),
> +		 "rot: %x src[x/y:%d/%d, w/h:%d/%d] disp[x/y:%d/%d, w/h:%d/%d]",
> +		 config->rot,
> +		 config->in_x, config->in_y, config->in_w, config->in_h,
> +		 config->out_x, config->out_y, config->out_w, config->out_h);
> +
> +	return str;
> +}
> +
>  struct komeda_plane_state;
>  struct komeda_crtc_state;
>  struct komeda_crtc;
> @@ -532,22 +546,27 @@ int komeda_build_layer_data_flow(struct komeda_layer *layer,
>  				 struct komeda_plane_state *kplane_st,
>  				 struct komeda_crtc_state *kcrtc_st,
>  				 struct komeda_data_flow_cfg *dflow);
> -int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
> -			      struct drm_connector_state *conn_st,
> -			      struct komeda_crtc_state *kcrtc_st,
> -			      struct komeda_data_flow_cfg *dflow);
> -int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
> -				   struct komeda_crtc_state *kcrtc_st);
> -
>  int komeda_build_layer_split_data_flow(struct komeda_layer *left,
>  				       struct komeda_plane_state *kplane_st,
>  				       struct komeda_crtc_state *kcrtc_st,
>  				       struct komeda_data_flow_cfg *dflow);
> +int komeda_build_layer_sbs_data_flow(struct komeda_layer *layer,
> +				     struct komeda_plane_state *kplane_st,
> +				     struct komeda_crtc_state *kcrtc_st,
> +				     struct komeda_data_flow_cfg *dflow);
> +
> +int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
> +			      struct drm_connector_state *conn_st,
> +			      struct komeda_crtc_state *kcrtc_st,
> +			      struct komeda_data_flow_cfg *dflow);
>  int komeda_build_wb_split_data_flow(struct komeda_layer *wb_layer,
>  				    struct drm_connector_state *conn_st,
>  				    struct komeda_crtc_state *kcrtc_st,
>  				    struct komeda_data_flow_cfg *dflow);
>  
> +int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
> +				   struct komeda_crtc_state *kcrtc_st);
> +
>  int komeda_release_unclaimed_resources(struct komeda_pipeline *pipe,
>  				       struct komeda_crtc_state *kcrtc_st);
>  
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 0930234abb9d..5de0d231a1c3 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -1130,6 +1130,194 @@ int komeda_build_layer_split_data_flow(struct komeda_layer *left,
>  	return err;
>  }
>  
> +/* split func will split configuration of master plane to two layer data
> + * flows, which will be fed into master and slave pipeline then.
> + * NOTE: @m_dflow is first used as input argument to pass the configuration of
> + *	 master_plane. when the split is done, @*m_dflow @*s_dflow are the
> + *	 output data flow for pipeline.
> + */
> +static int
> +komeda_split_sbs_master_data_flow(struct komeda_crtc_state *kcrtc_st,
> +				  struct komeda_data_flow_cfg **m_dflow,
> +				  struct komeda_data_flow_cfg **s_dflow)
> +{
> +	struct komeda_data_flow_cfg *master = *m_dflow;
> +	struct komeda_data_flow_cfg *slave = *s_dflow;
> +	u32 disp_end = master->out_x + master->out_w;
> +	u16 boundary;
> +
> +	pipeline_composition_size(kcrtc_st, &boundary, NULL);
> +
> +	if (disp_end <= boundary) {
> +		/* the master viewport only located in master side, no need
> +		 * slave anymore
> +		 */
> +		*s_dflow = NULL;
> +	} else if ((master->out_x < boundary) && (disp_end > boundary)) {
> +		/* the master viewport across two pipelines, split it */
> +		bool flip_h = has_flip_h(master->rot);
> +		bool r90  = drm_rotation_90_or_270(master->rot);
> +		u32 src_x = master->in_x;
> +		u32 src_y = master->in_y;
> +		u32 src_w = master->in_w;
> +		u32 src_h = master->in_h;
> +
> +		if (master->en_scaling || master->en_img_enhancement) {
> +			DRM_DEBUG_ATOMIC("sbs doesn't support to split a scaling image.\n");
> +			return -EINVAL;
> +		}
> +
> +		memcpy(slave, master, sizeof(*master));
> +
> +		/* master for left part of display, slave for the right part */
> +		/* split the disp_rect */
> +		master->out_w = boundary - master->out_x;
> +		slave->out_w  = disp_end - boundary;
> +		slave->out_x  = 0;
> +
> +		if (r90) {
> +			master->in_h = master->out_w;
> +			slave->in_h = slave->out_w;
> +
> +			if (flip_h)
> +				master->in_y = src_y + src_h - master->in_h;
> +			else
> +				slave->in_y = src_y + src_h - slave->in_h;
> +		} else {
> +			master->in_w = master->out_w;
> +			slave->in_w = slave->out_w;
> +
> +			/* on flip_h, the left display content from the right-source */
> +			if (flip_h)
> +				master->in_x = src_w + src_x - master->in_w;
> +			else
> +				slave->in_x = src_w + src_x - slave->in_w;

It really bugs me that the order here is w/x/w but in the block above
it's y/h/h.

> +		}
> +	} else if (master->out_x >= boundary) {
> +		/* disp_rect only locate in right part, move the dflow to slave */
> +		master->out_x -= boundary;
> +		*s_dflow = master;
> +		*m_dflow = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +komeda_split_sbs_slave_data_flow(struct komeda_crtc_state *kcrtc_st,
> +				 struct komeda_data_flow_cfg *slave)
> +{
> +	u16 boundary;
> +
> +	pipeline_composition_size(kcrtc_st, &boundary, NULL);
> +
> +	if (slave->out_x < boundary) {
> +		DRM_DEBUG_ATOMIC("SBS Slave plane is only allowed to configure the right part frame.\n");
> +		return -EINVAL;
> +	}
> +
> +	/* slave->disp_rect locate in the right part */
> +	slave->out_x -= boundary;
> +
> +	return 0;
> +}
> +
> +/* On side by side mode, The full display frame will be split to two parts
> + * (Left/Right), and each part will be handled by a single pipeline separately,
> + * master pipeline for left part, slave for right.
> + *
> + * To simplify the usage and implementation, komeda use the following scheme
> + * to do the side by side split
> + * 1. The planes also have been grouped into two classes:
> + *    master-planes and slave-planes.
> + * 2. The master plane can display its image on any location of the final/full
> + *    display frame, komeda will help to split the plane configuration to two
> + *    parts and fed them into master and slave pipelines.
> + * 3. The slave plane only can put its display rect on the right part of the
> + *    final display frame, and its data is only can be fed into the slave
> + *    pipeline.
> + *
> + * From the perspective of resource usage and assignment:
> + * The master plane can use the resources from the master pipeline and slave
> + * pipeline both, but slave plane only can use the slave pipeline resources.
> + *
> + * With such scheme, the usage of master planes are same as the none
> + * side_by_side mode. user can easily skip the slave planes and no need to
> + * consider side_by_side for them.
> + *
> + * NOTE: side_by_side split is occurred on pipeline level which split the plane
> + *	 data flow into pipelines, but the layer split is a pipeline
> + *	 internal split which splits the data flow into pipeline layers.
> + *	 So komeda still supports to apply a further layer split to the sbs
> + *	 split data flow.
> + */
> +int komeda_build_layer_sbs_data_flow(struct komeda_layer *layer,
> +				     struct komeda_plane_state *kplane_st,
> +				     struct komeda_crtc_state *kcrtc_st,
> +				     struct komeda_data_flow_cfg *dflow)
> +{
> +	struct komeda_crtc *kcrtc = to_kcrtc(kcrtc_st->base.crtc);
> +	struct drm_plane *plane = kplane_st->base.plane;
> +	struct komeda_data_flow_cfg temp, *master_dflow, *slave_dflow;
> +	struct komeda_layer *master, *slave;
> +	bool master_plane = layer->base.pipeline == kcrtc->master;
> +	int err;
> +
> +	DRM_DEBUG_ATOMIC("SBS prepare %s-[PLANE:%d:%s]: %s.\n",
> +			 master_plane ? "Master" : "Slave",
> +			 plane->base.id, plane->name,
> +			 komeda_data_flow_msg(dflow));
> +
> +	if (master_plane) {
> +		master = layer;
> +		slave = layer->sbs_slave;
> +		master_dflow = dflow;
> +		slave_dflow  = &temp;
> +		err = komeda_split_sbs_master_data_flow(kcrtc_st,
> +				&master_dflow, &slave_dflow);
> +	} else {
> +		master = NULL;
> +		slave = layer;
> +		master_dflow = NULL;
> +		slave_dflow = dflow;
> +		err = komeda_split_sbs_slave_data_flow(kcrtc_st, slave_dflow);
> +	}
> +
> +	if (err)
> +		return err;
> +
> +	if (master_dflow) {
> +		DRM_DEBUG_ATOMIC("SBS Master-%s assigned: %s\n",
> +			master->base.name, komeda_data_flow_msg(master_dflow));
> +
> +		if (master_dflow->en_split)
> +			err = komeda_build_layer_split_data_flow(master,
> +					kplane_st, kcrtc_st, master_dflow);
> +		else
> +			err = komeda_build_layer_data_flow(master,
> +					kplane_st, kcrtc_st, master_dflow);
> +
> +		if (err)
> +			return err;
> +	}
> +
> +	if (slave_dflow) {
> +		DRM_DEBUG_ATOMIC("SBS Slave-%s assigned: %s\n",
> +			slave->base.name, komeda_data_flow_msg(slave_dflow));
> +
> +		if (slave_dflow->en_split)
> +			err = komeda_build_layer_split_data_flow(slave,
> +					kplane_st, kcrtc_st, slave_dflow);
> +		else
> +			err = komeda_build_layer_data_flow(slave,
> +					kplane_st, kcrtc_st, slave_dflow);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
>  /* writeback data path: compiz -> scaler -> wb_layer -> memory */
>  int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
>  			      struct drm_connector_state *conn_st,
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> index 98e915e325dd..2644f0727570 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> @@ -77,6 +77,7 @@ komeda_plane_atomic_check(struct drm_plane *plane,
>  	struct komeda_plane_state *kplane_st = to_kplane_st(state);
>  	struct komeda_layer *layer = kplane->layer;
>  	struct drm_crtc_state *crtc_st;
> +	struct komeda_crtc *kcrtc;
>  	struct komeda_crtc_state *kcrtc_st;
>  	struct komeda_data_flow_cfg dflow;
>  	int err;
> @@ -94,13 +95,17 @@ komeda_plane_atomic_check(struct drm_plane *plane,
>  	if (!crtc_st->active)
>  		return 0;
>  
> +	kcrtc = to_kcrtc(crtc_st->crtc);
>  	kcrtc_st = to_kcrtc_st(crtc_st);
>  
>  	err = komeda_plane_init_data_flow(state, kcrtc_st, &dflow);
>  	if (err)
>  		return err;
>  
> -	if (dflow.en_split)
> +	if (kcrtc->side_by_side)
> +		err = komeda_build_layer_sbs_data_flow(layer,
> +				kplane_st, kcrtc_st, &dflow);
> +	else if (dflow.en_split)
>  		err = komeda_build_layer_split_data_flow(layer,
>  				kplane_st, kcrtc_st, &dflow);
>  	else
> 


-- 
Mihail




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/6] drm/komeda: Add side by side assembling
  2019-11-14  8:37 ` [PATCH v3 1/6] drm/komeda: Add side by side assembling james qian wang (Arm Technology China)
@ 2019-11-15  0:02   ` Mihail Atanassov
  2019-11-19  9:22     ` james qian wang (Arm Technology China)
  0 siblings, 1 reply; 11+ messages in thread
From: Mihail Atanassov @ 2019-11-15  0:02 UTC (permalink / raw)
  To: james qian wang (Arm Technology China)
  Cc: nd, Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	nd, linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China)

Hi James,

On Thursday, 14 November 2019 08:37:24 GMT james qian wang (Arm Technology China) wrote:
> Komeda HW can support side by side, which splits the internal display
> processing to two single halves (LEFT/RIGHT) and handle them by two
> pipelines separately.
> komeda "side by side" is enabled by DT property: "side_by_side_master",
> once DT configured side by side, komeda need to verify it with HW's
> configuration, and assemble it for the further usage.

A few problems I see with this approach:
 - This property doesn't scale to >2 pipes;
 - Our HW is capable of dynamically switching between SBS and non-SBS
modes, with this DT property you're effectively denying the opportunity
to use the second pipe when the first one can be satisfied with
4 planes and 1px/clk.

If we only want to fix the first problem, then at least we need this
to be a property of the pipeline node with a phandle linking slave to
master (or bidirectional).

For the second, why not do the SBS decision at modeset time?
If the first CRTC has dual-link output and the commit:
 - only drives one CRTC
 - uses up to 4 planes
 - doesn't meet clk requirements without SBS but does with SBS
then we can switch SBS on dynamically.

And we can tweak that decision with power use in mind later on since
there's no user-visible knob.

We can still keep a DT property if we have a use case for it (e.g.
forcing SBS on for some reason), but we might want to name it slightly
more conservatively then, so it doesn't imply that we never do SBS
when it's not there.

Lastly, maintaining that property in combination with the dynamic
modeset-time SBS decision tree means extra code for more or less the
same functionality. <2c>I'm not 100% sure it's worth it.</2c>

> 
> v3: Correct a typo.
> 
> Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
> ---
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 13 ++++-
>  .../gpu/drm/arm/display/komeda/komeda_dev.c   |  3 ++
>  .../gpu/drm/arm/display/komeda/komeda_dev.h   |  9 ++++
>  .../gpu/drm/arm/display/komeda/komeda_kms.h   |  3 ++
>  .../drm/arm/display/komeda/komeda_pipeline.c  | 50 +++++++++++++++++--
>  .../drm/arm/display/komeda/komeda_pipeline.h  |  1 +
>  6 files changed, 73 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 1c452ea75999..cee9a1692e71 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -561,21 +561,30 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
>  	kms->n_crtcs = 0;
>  
>  	for (i = 0; i < mdev->n_pipelines; i++) {
> +		/* if sbs, one komeda_dev only can represent one CRTC */
> +		if (mdev->side_by_side && i != mdev->side_by_side_master)
> +			continue;
> +
>  		crtc = &kms->crtcs[kms->n_crtcs];
>  		master = mdev->pipelines[i];
>  
>  		crtc->master = master;
>  		crtc->slave  = komeda_pipeline_get_slave(master);
> +		crtc->side_by_side = mdev->side_by_side;
>  
>  		if (crtc->slave)
>  			sprintf(str, "pipe-%d", crtc->slave->id);
>  		else
>  			sprintf(str, "None");
>  
> -		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
> -			 kms->n_crtcs, master->id, str);
> +		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s) sbs(%s).\n",
> +			 kms->n_crtcs, master->id, str,
> +			 crtc->side_by_side ? "On" : "Off");
>  
>  		kms->n_crtcs++;
> +
> +		if (mdev->side_by_side)
> +			break;
>  	}
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index 4e46f650fddf..c3fa4835cb8d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -178,6 +178,9 @@ static int komeda_parse_dt(struct device *dev, struct komeda_dev *mdev)
>  		}
>  	}
>  
> +	mdev->side_by_side = !of_property_read_u32(np, "side_by_side_master",
> +						   &mdev->side_by_side_master);
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index d406a4d83352..471604b42431 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -183,6 +183,15 @@ struct komeda_dev {
>  
>  	/** @irq: irq number */
>  	int irq;
> +	/**
> +	 * @side_by_side:
> +	 *
> +	 * on sbs the whole display frame will be split to two halves (1:2),
> +	 * master pipeline handles the left part, slave for the right part
> +	 */
> +	bool side_by_side;

That's a duplicate of the one in komeda_crtc. You don't need both.

> +	/** @side_by_side_master: master pipe id for side by side */
> +	int side_by_side_master;

As I detailed above, this should be on the crtc, otherwise we can't
scale to >2 pipes.

>  
>  	/** @lock: used to protect dpmode */
>  	struct mutex lock;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> index 456f3c435719..ae6654fe95e2 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> @@ -76,6 +76,9 @@ struct komeda_crtc {
>  	 */
>  	struct komeda_pipeline *slave;
>  
> +	/** @side_by_side: if the master and slave works on side by side mode */
> +	bool side_by_side;
> +
>  	/** @slave_planes: komeda slave planes mask */
>  	u32 slave_planes;
>  
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> index 452e505a1fd3..104e27cc1dc3 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> @@ -326,14 +326,56 @@ static void komeda_pipeline_assemble(struct komeda_pipeline *pipe)
>  struct komeda_pipeline *
>  komeda_pipeline_get_slave(struct komeda_pipeline *master)
>  {
> -	struct komeda_component *slave;
> +	struct komeda_dev *mdev = master->mdev;
> +	struct komeda_component *comp, *slave;
> +	u32 avail_inputs;
> +
> +	/* on SBS, slave pipeline merge to master via image processor */
> +	if (mdev->side_by_side) {
> +		comp = &master->improc->base;
> +		avail_inputs = KOMEDA_PIPELINE_IMPROCS;
> +	} else {
> +		comp = &master->compiz->base;
> +		avail_inputs = KOMEDA_PIPELINE_COMPIZS;
> +	}
>  
> -	slave = komeda_component_pickup_input(&master->compiz->base,
> -					      KOMEDA_PIPELINE_COMPIZS);
> +	slave = komeda_component_pickup_input(comp, avail_inputs);
>  
>  	return slave ? slave->pipeline : NULL;
>  }
>  
> +static int komeda_assemble_side_by_side(struct komeda_dev *mdev)
> +{
> +	struct komeda_pipeline *master, *slave;
> +	int i;
> +
> +	if (!mdev->side_by_side)
> +		return 0;
> +
> +	if (mdev->side_by_side_master >= mdev->n_pipelines) {
> +		DRM_ERROR("DT configured side by side master-%d is invalid.\n",
> +			  mdev->side_by_side_master);
> +		return -EINVAL;
> +	}
> +
> +	master = mdev->pipelines[mdev->side_by_side_master];
> +	slave = komeda_pipeline_get_slave(master);
> +	if (!slave || slave->n_layers != master->n_layers) {
> +		DRM_ERROR("Current HW doesn't support side by side.\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!master->dual_link) {
> +		DRM_DEBUG_ATOMIC("SBS can not work without dual link.\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < master->n_layers; i++)
> +		master->layers[i]->sbs_slave = slave->layers[i];
> +
> +	return 0;
> +}
> +
>  int komeda_assemble_pipelines(struct komeda_dev *mdev)
>  {
>  	struct komeda_pipeline *pipe;
> @@ -346,7 +388,7 @@ int komeda_assemble_pipelines(struct komeda_dev *mdev)
>  		komeda_pipeline_dump(pipe);
>  	}
>  
> -	return 0;
> +	return komeda_assemble_side_by_side(mdev);
>  }
>  
>  void komeda_pipeline_dump_register(struct komeda_pipeline *pipe,
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index ac8725e24853..20a076cce635 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -237,6 +237,7 @@ struct komeda_layer {
>  	 * not the source buffer.
>  	 */
>  	struct komeda_layer *right;
> +	struct komeda_layer *sbs_slave;
>  };
>  
>  struct komeda_layer_state {
> 


-- 
Mihail




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/6] drm/komeda: Add side by side plane_state split
  2019-11-15  0:00   ` Mihail Atanassov
@ 2019-11-19  8:42     ` james qian wang (Arm Technology China)
  0 siblings, 0 replies; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-19  8:42 UTC (permalink / raw)
  To: Mihail Atanassov
  Cc: nd, Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China)

On Fri, Nov 15, 2019 at 12:00:01AM +0000, Mihail Atanassov wrote:
> On Thursday, 14 November 2019 08:37:31 GMT james qian wang (Arm Technology China) wrote:
> > On side by side mode, The full display frame will be split into two parts
> > (Left/Right), and each part will be handled by a single pipeline separately
> > master pipeline for left part, slave for right.
> > 
> > To simplify the usage and implementation, komeda use the following scheme
> > to do the side by side split
> > 1. The planes also have been grouped into two classes:
> >    master-planes and slave-planes.
> > 2. The master plane can display its image on any location of the final/full
> >    display frame, komeda will help to split the plane configuration to two
> >    parts and fed them into master and slave pipelines.
> > 3. The slave plane only can put its display rect on the right part of the
> >    final display frame, and its data is only can be fed into the slave
> >    pipeline.
> > 
> > From the perspective of resource usage and assignment:
> > The master plane can use the resources from the master pipeline and slave
> > pipeline both, but slave plane only can use the slave pipeline resources.
> > 
> > With such scheme, the usage of master planes are same as the none
> > side_by_side mode. user can easily skip the slave planes and no need to
> > consider side_by_side for them.
> > 
> > Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
> > ---
> >  .../drm/arm/display/komeda/komeda_pipeline.h  |  33 ++-
> >  .../display/komeda/komeda_pipeline_state.c    | 188 ++++++++++++++++++
> >  .../gpu/drm/arm/display/komeda/komeda_plane.c |   7 +-
> >  3 files changed, 220 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > index 20a076cce635..4c0946fbaac1 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > @@ -521,6 +521,20 @@ komeda_component_pickup_output(struct komeda_component *c, u32 avail_comps)
> >  	return komeda_pipeline_get_first_component(c->pipeline, avail_inputs);
> >  }
> >  
> > +static inline const char *
> > +komeda_data_flow_msg(struct komeda_data_flow_cfg *config)
> > +{
> > +	static char str[128];
> > +
> > +	snprintf(str, sizeof(str),
> > +		 "rot: %x src[x/y:%d/%d, w/h:%d/%d] disp[x/y:%d/%d, w/h:%d/%d]",
> > +		 config->rot,
> > +		 config->in_x, config->in_y, config->in_w, config->in_h,
> > +		 config->out_x, config->out_y, config->out_w, config->out_h);
> > +
> > +	return str;
> > +}
> > +
> >  struct komeda_plane_state;
> >  struct komeda_crtc_state;
> >  struct komeda_crtc;
> > @@ -532,22 +546,27 @@ int komeda_build_layer_data_flow(struct komeda_layer *layer,
> >  				 struct komeda_plane_state *kplane_st,
> >  				 struct komeda_crtc_state *kcrtc_st,
> >  				 struct komeda_data_flow_cfg *dflow);
> > -int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
> > -			      struct drm_connector_state *conn_st,
> > -			      struct komeda_crtc_state *kcrtc_st,
> > -			      struct komeda_data_flow_cfg *dflow);
> > -int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
> > -				   struct komeda_crtc_state *kcrtc_st);
> > -
> >  int komeda_build_layer_split_data_flow(struct komeda_layer *left,
> >  				       struct komeda_plane_state *kplane_st,
> >  				       struct komeda_crtc_state *kcrtc_st,
> >  				       struct komeda_data_flow_cfg *dflow);
> > +int komeda_build_layer_sbs_data_flow(struct komeda_layer *layer,
> > +				     struct komeda_plane_state *kplane_st,
> > +				     struct komeda_crtc_state *kcrtc_st,
> > +				     struct komeda_data_flow_cfg *dflow);
> > +
> > +int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
> > +			      struct drm_connector_state *conn_st,
> > +			      struct komeda_crtc_state *kcrtc_st,
> > +			      struct komeda_data_flow_cfg *dflow);
> >  int komeda_build_wb_split_data_flow(struct komeda_layer *wb_layer,
> >  				    struct drm_connector_state *conn_st,
> >  				    struct komeda_crtc_state *kcrtc_st,
> >  				    struct komeda_data_flow_cfg *dflow);
> >  
> > +int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
> > +				   struct komeda_crtc_state *kcrtc_st);
> > +
> >  int komeda_release_unclaimed_resources(struct komeda_pipeline *pipe,
> >  				       struct komeda_crtc_state *kcrtc_st);
> >  
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > index 0930234abb9d..5de0d231a1c3 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > @@ -1130,6 +1130,194 @@ int komeda_build_layer_split_data_flow(struct komeda_layer *left,
> >  	return err;
> >  }
> >  
> > +/* split func will split configuration of master plane to two layer data
> > + * flows, which will be fed into master and slave pipeline then.
> > + * NOTE: @m_dflow is first used as input argument to pass the configuration of
> > + *	 master_plane. when the split is done, @*m_dflow @*s_dflow are the
> > + *	 output data flow for pipeline.
> > + */
> > +static int
> > +komeda_split_sbs_master_data_flow(struct komeda_crtc_state *kcrtc_st,
> > +				  struct komeda_data_flow_cfg **m_dflow,
> > +				  struct komeda_data_flow_cfg **s_dflow)
> > +{
> > +	struct komeda_data_flow_cfg *master = *m_dflow;
> > +	struct komeda_data_flow_cfg *slave = *s_dflow;
> > +	u32 disp_end = master->out_x + master->out_w;
> > +	u16 boundary;
> > +
> > +	pipeline_composition_size(kcrtc_st, &boundary, NULL);
> > +
> > +	if (disp_end <= boundary) {
> > +		/* the master viewport only located in master side, no need
> > +		 * slave anymore
> > +		 */
> > +		*s_dflow = NULL;
> > +	} else if ((master->out_x < boundary) && (disp_end > boundary)) {
> > +		/* the master viewport across two pipelines, split it */
> > +		bool flip_h = has_flip_h(master->rot);
> > +		bool r90  = drm_rotation_90_or_270(master->rot);
> > +		u32 src_x = master->in_x;
> > +		u32 src_y = master->in_y;
> > +		u32 src_w = master->in_w;
> > +		u32 src_h = master->in_h;
> > +
> > +		if (master->en_scaling || master->en_img_enhancement) {
> > +			DRM_DEBUG_ATOMIC("sbs doesn't support to split a scaling image.\n");
> > +			return -EINVAL;
> > +		}
> > +
> > +		memcpy(slave, master, sizeof(*master));
> > +
> > +		/* master for left part of display, slave for the right part */
> > +		/* split the disp_rect */
> > +		master->out_w = boundary - master->out_x;
> > +		slave->out_w  = disp_end - boundary;
> > +		slave->out_x  = 0;
> > +
> > +		if (r90) {
> > +			master->in_h = master->out_w;
> > +			slave->in_h = slave->out_w;
> > +
> > +			if (flip_h)
> > +				master->in_y = src_y + src_h - master->in_h;
> > +			else
> > +				slave->in_y = src_y + src_h - slave->in_h;
> > +		} else {
> > +			master->in_w = master->out_w;
> > +			slave->in_w = slave->out_w;
> > +
> > +			/* on flip_h, the left display content from the right-source */
> > +			if (flip_h)
> > +				master->in_x = src_w + src_x - master->in_w;
> > +			else
> > +				slave->in_x = src_w + src_x - slave->in_w;
> 
> It really bugs me that the order here is w/x/w but in the block above
> it's y/h/h.

will refine the order to x/w/w.

> > +		}
> > +	} else if (master->out_x >= boundary) {
> > +		/* disp_rect only locate in right part, move the dflow to slave */
> > +		master->out_x -= boundary;
> > +		*s_dflow = master;
> > +		*m_dflow = NULL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int
> > +komeda_split_sbs_slave_data_flow(struct komeda_crtc_state *kcrtc_st,
> > +				 struct komeda_data_flow_cfg *slave)
> > +{
> > +	u16 boundary;
> > +
> > +	pipeline_composition_size(kcrtc_st, &boundary, NULL);
> > +
> > +	if (slave->out_x < boundary) {
> > +		DRM_DEBUG_ATOMIC("SBS Slave plane is only allowed to configure the right part frame.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* slave->disp_rect locate in the right part */
> > +	slave->out_x -= boundary;
> > +
> > +	return 0;
> > +}
> > +
> > +/* On side by side mode, The full display frame will be split to two parts
> > + * (Left/Right), and each part will be handled by a single pipeline separately,
> > + * master pipeline for left part, slave for right.
> > + *
> > + * To simplify the usage and implementation, komeda use the following scheme
> > + * to do the side by side split
> > + * 1. The planes also have been grouped into two classes:
> > + *    master-planes and slave-planes.
> > + * 2. The master plane can display its image on any location of the final/full
> > + *    display frame, komeda will help to split the plane configuration to two
> > + *    parts and fed them into master and slave pipelines.
> > + * 3. The slave plane only can put its display rect on the right part of the
> > + *    final display frame, and its data is only can be fed into the slave
> > + *    pipeline.
> > + *
> > + * From the perspective of resource usage and assignment:
> > + * The master plane can use the resources from the master pipeline and slave
> > + * pipeline both, but slave plane only can use the slave pipeline resources.
> > + *
> > + * With such scheme, the usage of master planes are same as the none
> > + * side_by_side mode. user can easily skip the slave planes and no need to
> > + * consider side_by_side for them.
> > + *
> > + * NOTE: side_by_side split is occurred on pipeline level which split the plane
> > + *	 data flow into pipelines, but the layer split is a pipeline
> > + *	 internal split which splits the data flow into pipeline layers.
> > + *	 So komeda still supports to apply a further layer split to the sbs
> > + *	 split data flow.
> > + */
> > +int komeda_build_layer_sbs_data_flow(struct komeda_layer *layer,
> > +				     struct komeda_plane_state *kplane_st,
> > +				     struct komeda_crtc_state *kcrtc_st,
> > +				     struct komeda_data_flow_cfg *dflow)
> > +{
> > +	struct komeda_crtc *kcrtc = to_kcrtc(kcrtc_st->base.crtc);
> > +	struct drm_plane *plane = kplane_st->base.plane;
> > +	struct komeda_data_flow_cfg temp, *master_dflow, *slave_dflow;
> > +	struct komeda_layer *master, *slave;
> > +	bool master_plane = layer->base.pipeline == kcrtc->master;
> > +	int err;
> > +
> > +	DRM_DEBUG_ATOMIC("SBS prepare %s-[PLANE:%d:%s]: %s.\n",
> > +			 master_plane ? "Master" : "Slave",
> > +			 plane->base.id, plane->name,
> > +			 komeda_data_flow_msg(dflow));
> > +
> > +	if (master_plane) {
> > +		master = layer;
> > +		slave = layer->sbs_slave;
> > +		master_dflow = dflow;
> > +		slave_dflow  = &temp;
> > +		err = komeda_split_sbs_master_data_flow(kcrtc_st,
> > +				&master_dflow, &slave_dflow);
> > +	} else {
> > +		master = NULL;
> > +		slave = layer;
> > +		master_dflow = NULL;
> > +		slave_dflow = dflow;
> > +		err = komeda_split_sbs_slave_data_flow(kcrtc_st, slave_dflow);
> > +	}
> > +
> > +	if (err)
> > +		return err;
> > +
> > +	if (master_dflow) {
> > +		DRM_DEBUG_ATOMIC("SBS Master-%s assigned: %s\n",
> > +			master->base.name, komeda_data_flow_msg(master_dflow));
> > +
> > +		if (master_dflow->en_split)
> > +			err = komeda_build_layer_split_data_flow(master,
> > +					kplane_st, kcrtc_st, master_dflow);
> > +		else
> > +			err = komeda_build_layer_data_flow(master,
> > +					kplane_st, kcrtc_st, master_dflow);
> > +
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > +	if (slave_dflow) {
> > +		DRM_DEBUG_ATOMIC("SBS Slave-%s assigned: %s\n",
> > +			slave->base.name, komeda_data_flow_msg(slave_dflow));
> > +
> > +		if (slave_dflow->en_split)
> > +			err = komeda_build_layer_split_data_flow(slave,
> > +					kplane_st, kcrtc_st, slave_dflow);
> > +		else
> > +			err = komeda_build_layer_data_flow(slave,
> > +					kplane_st, kcrtc_st, slave_dflow);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /* writeback data path: compiz -> scaler -> wb_layer -> memory */
> >  int komeda_build_wb_data_flow(struct komeda_layer *wb_layer,
> >  			      struct drm_connector_state *conn_st,
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> > index 98e915e325dd..2644f0727570 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> > @@ -77,6 +77,7 @@ komeda_plane_atomic_check(struct drm_plane *plane,
> >  	struct komeda_plane_state *kplane_st = to_kplane_st(state);
> >  	struct komeda_layer *layer = kplane->layer;
> >  	struct drm_crtc_state *crtc_st;
> > +	struct komeda_crtc *kcrtc;
> >  	struct komeda_crtc_state *kcrtc_st;
> >  	struct komeda_data_flow_cfg dflow;
> >  	int err;
> > @@ -94,13 +95,17 @@ komeda_plane_atomic_check(struct drm_plane *plane,
> >  	if (!crtc_st->active)
> >  		return 0;
> >  
> > +	kcrtc = to_kcrtc(crtc_st->crtc);
> >  	kcrtc_st = to_kcrtc_st(crtc_st);
> >  
> >  	err = komeda_plane_init_data_flow(state, kcrtc_st, &dflow);
> >  	if (err)
> >  		return err;
> >  
> > -	if (dflow.en_split)
> > +	if (kcrtc->side_by_side)
> > +		err = komeda_build_layer_sbs_data_flow(layer,
> > +				kplane_st, kcrtc_st, &dflow);
> > +	else if (dflow.en_split)
> >  		err = komeda_build_layer_split_data_flow(layer,
> >  				kplane_st, kcrtc_st, &dflow);
> >  	else
> > 
> 
> 
> -- 
> Mihail
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/6] drm/komeda: Add side by side assembling
  2019-11-15  0:02   ` Mihail Atanassov
@ 2019-11-19  9:22     ` james qian wang (Arm Technology China)
  0 siblings, 0 replies; 11+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-11-19  9:22 UTC (permalink / raw)
  To: Mihail Atanassov
  Cc: nd, Liviu Dudau, airlied, Brian Starkey, maarten.lankhorst, sean,
	Jonathan Chai (Arm Technology China),
	Julien Yin (Arm Technology China),
	Thomas Sun (Arm Technology China),
	Lowry Li (Arm Technology China),
	Ayan Halder, Tiannan Zhu (Arm Technology China),
	Yiqi Kang (Arm Technology China),
	linux-kernel, dri-devel, Ben Davis,
	Oscar Zhang (Arm Technology China),
	Channing Chen (Arm Technology China)

On Fri, Nov 15, 2019 at 12:02:00AM +0000, Mihail Atanassov wrote:
> Hi James,
> 
> On Thursday, 14 November 2019 08:37:24 GMT james qian wang (Arm Technology China) wrote:
> > Komeda HW can support side by side, which splits the internal display
> > processing to two single halves (LEFT/RIGHT) and handle them by two
> > pipelines separately.
> > komeda "side by side" is enabled by DT property: "side_by_side_master",
> > once DT configured side by side, komeda need to verify it with HW's
> > configuration, and assemble it for the further usage.
> 
> A few problems I see with this approach:
>  - This property doesn't scale to >2 pipes;
>  - Our HW is capable of dynamically switching between SBS and non-SBS
> modes, with this DT property you're effectively denying the opportunity
> to use the second pipe when the first one can be satisfied with
> 4 planes and 1px/clk.
> 
> If we only want to fix the first problem, then at least we need this
> to be a property of the pipeline node with a phandle linking slave to
> master (or bidirectional).

I had consider this way before, but consider we have no product (now
and in next 2/3 years) can support >2 pipes. So for DT I decide to
focus on current, but you may see I add two side_by_side flags.

  - mdev->side_by_side.
  - crtc->side_by_side.

And beside the DT parse we use mdev->side_by_side, the real SBS
operation actually based on crtc->side_by_side, then once the HW
changed, we only need to update the SBS assemble/decision code, but no
need to update the real sbs logic.

thanks
James

> For the second, why not do the SBS decision at modeset time?
> If the first CRTC has dual-link output and the commit:
>  - only drives one CRTC
>  - uses up to 4 planes
>  - doesn't meet clk requirements without SBS but does with SBS
> then we can switch SBS on dynamically.
> And we can tweak that decision with power use in mind later on since
> there's no user-visible knob.

Yes, you're right, current implementation just use simplest way to
show the feature, and for dynamic enable/disable sbs will be added
when we have the real usage case.

> We can still keep a DT property if we have a use case for it (e.g.
> forcing SBS on for some reason), but we might want to name it slightly
> more conservatively then, so it doesn't imply that we never do SBS
> when it's not there.
> 
> Lastly, maintaining that property in combination with the dynamic
> modeset-time SBS decision tree means extra code for more or less the
> same functionality. <2c>I'm not 100% sure it's worth it.</2c>

I think we'd add such support when we have the real use case. :)

> > 
> > v3: Correct a typo.
> > 
> > Signed-off-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
> > ---
> >  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 13 ++++-
> >  .../gpu/drm/arm/display/komeda/komeda_dev.c   |  3 ++
> >  .../gpu/drm/arm/display/komeda/komeda_dev.h   |  9 ++++
> >  .../gpu/drm/arm/display/komeda/komeda_kms.h   |  3 ++
> >  .../drm/arm/display/komeda/komeda_pipeline.c  | 50 +++++++++++++++++--
> >  .../drm/arm/display/komeda/komeda_pipeline.h  |  1 +
> >  6 files changed, 73 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > index 1c452ea75999..cee9a1692e71 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > @@ -561,21 +561,30 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> >  	kms->n_crtcs = 0;
> >  
> >  	for (i = 0; i < mdev->n_pipelines; i++) {
> > +		/* if sbs, one komeda_dev only can represent one CRTC */
> > +		if (mdev->side_by_side && i != mdev->side_by_side_master)
> > +			continue;
> > +
> >  		crtc = &kms->crtcs[kms->n_crtcs];
> >  		master = mdev->pipelines[i];
> >  
> >  		crtc->master = master;
> >  		crtc->slave  = komeda_pipeline_get_slave(master);
> > +		crtc->side_by_side = mdev->side_by_side;
> >  
> >  		if (crtc->slave)
> >  			sprintf(str, "pipe-%d", crtc->slave->id);
> >  		else
> >  			sprintf(str, "None");
> >  
> > -		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
> > -			 kms->n_crtcs, master->id, str);
> > +		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s) sbs(%s).\n",
> > +			 kms->n_crtcs, master->id, str,
> > +			 crtc->side_by_side ? "On" : "Off");
> >  
> >  		kms->n_crtcs++;
> > +
> > +		if (mdev->side_by_side)
> > +			break;
> >  	}
> >  
> >  	return 0;
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > index 4e46f650fddf..c3fa4835cb8d 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > @@ -178,6 +178,9 @@ static int komeda_parse_dt(struct device *dev, struct komeda_dev *mdev)
> >  		}
> >  	}
> >  
> > +	mdev->side_by_side = !of_property_read_u32(np, "side_by_side_master",
> > +						   &mdev->side_by_side_master);
> > +
> >  	return ret;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > index d406a4d83352..471604b42431 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> > @@ -183,6 +183,15 @@ struct komeda_dev {
> >  
> >  	/** @irq: irq number */
> >  	int irq;
> > +	/**
> > +	 * @side_by_side:
> > +	 *
> > +	 * on sbs the whole display frame will be split to two halves (1:2),
> > +	 * master pipeline handles the left part, slave for the right part
> > +	 */
> > +	bool side_by_side;
> 
> That's a duplicate of the one in komeda_crtc. You don't need both.
> 
> > +	/** @side_by_side_master: master pipe id for side by side */
> > +	int side_by_side_master;
> 
> As I detailed above, this should be on the crtc, otherwise we can't
> scale to >2 pipes.
> 
> >  
> >  	/** @lock: used to protect dpmode */
> >  	struct mutex lock;
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > index 456f3c435719..ae6654fe95e2 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > @@ -76,6 +76,9 @@ struct komeda_crtc {
> >  	 */
> >  	struct komeda_pipeline *slave;
> >  
> > +	/** @side_by_side: if the master and slave works on side by side mode */
> > +	bool side_by_side;
> > +
> >  	/** @slave_planes: komeda slave planes mask */
> >  	u32 slave_planes;
> >  
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> > index 452e505a1fd3..104e27cc1dc3 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> > @@ -326,14 +326,56 @@ static void komeda_pipeline_assemble(struct komeda_pipeline *pipe)
> >  struct komeda_pipeline *
> >  komeda_pipeline_get_slave(struct komeda_pipeline *master)
> >  {
> > -	struct komeda_component *slave;
> > +	struct komeda_dev *mdev = master->mdev;
> > +	struct komeda_component *comp, *slave;
> > +	u32 avail_inputs;
> > +
> > +	/* on SBS, slave pipeline merge to master via image processor */
> > +	if (mdev->side_by_side) {
> > +		comp = &master->improc->base;
> > +		avail_inputs = KOMEDA_PIPELINE_IMPROCS;
> > +	} else {
> > +		comp = &master->compiz->base;
> > +		avail_inputs = KOMEDA_PIPELINE_COMPIZS;
> > +	}
> >  
> > -	slave = komeda_component_pickup_input(&master->compiz->base,
> > -					      KOMEDA_PIPELINE_COMPIZS);
> > +	slave = komeda_component_pickup_input(comp, avail_inputs);
> >  
> >  	return slave ? slave->pipeline : NULL;
> >  }
> >  
> > +static int komeda_assemble_side_by_side(struct komeda_dev *mdev)
> > +{
> > +	struct komeda_pipeline *master, *slave;
> > +	int i;
> > +
> > +	if (!mdev->side_by_side)
> > +		return 0;
> > +
> > +	if (mdev->side_by_side_master >= mdev->n_pipelines) {
> > +		DRM_ERROR("DT configured side by side master-%d is invalid.\n",
> > +			  mdev->side_by_side_master);
> > +		return -EINVAL;
> > +	}
> > +
> > +	master = mdev->pipelines[mdev->side_by_side_master];
> > +	slave = komeda_pipeline_get_slave(master);
> > +	if (!slave || slave->n_layers != master->n_layers) {
> > +		DRM_ERROR("Current HW doesn't support side by side.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!master->dual_link) {
> > +		DRM_DEBUG_ATOMIC("SBS can not work without dual link.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = 0; i < master->n_layers; i++)
> > +		master->layers[i]->sbs_slave = slave->layers[i];
> > +
> > +	return 0;
> > +}
> > +
> >  int komeda_assemble_pipelines(struct komeda_dev *mdev)
> >  {
> >  	struct komeda_pipeline *pipe;
> > @@ -346,7 +388,7 @@ int komeda_assemble_pipelines(struct komeda_dev *mdev)
> >  		komeda_pipeline_dump(pipe);
> >  	}
> >  
> > -	return 0;
> > +	return komeda_assemble_side_by_side(mdev);
> >  }
> >  
> >  void komeda_pipeline_dump_register(struct komeda_pipeline *pipe,
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > index ac8725e24853..20a076cce635 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > @@ -237,6 +237,7 @@ struct komeda_layer {
> >  	 * not the source buffer.
> >  	 */
> >  	struct komeda_layer *right;
> > +	struct komeda_layer *sbs_slave;
> >  };
> >  
> >  struct komeda_layer_state {
> > 
> 
> 
> -- 
> Mihail
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-11-19  9:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-14  8:37 [PATCH v3 0/6] arm/komeda: Add side_by_side support james qian wang (Arm Technology China)
2019-11-14  8:37 ` [PATCH v3 1/6] drm/komeda: Add side by side assembling james qian wang (Arm Technology China)
2019-11-15  0:02   ` Mihail Atanassov
2019-11-19  9:22     ` james qian wang (Arm Technology China)
2019-11-14  8:37 ` [PATCH v3 2/6] drm/komeda: Add side by side plane_state split james qian wang (Arm Technology China)
2019-11-15  0:00   ` Mihail Atanassov
2019-11-19  8:42     ` james qian wang (Arm Technology China)
2019-11-14  8:37 ` [PATCH v3 3/6] drm/komeda: Build side by side display output pipeline james qian wang (Arm Technology China)
2019-11-14  8:37 ` [PATCH v3 4/6] drm/komeda: Add side by side support for writeback james qian wang (Arm Technology China)
2019-11-14  8:37 ` [PATCH v3 5/6] drm/komeda: Update writeback signal for side_by_side james qian wang (Arm Technology China)
2019-11-14  8:37 ` [PATCH v3 6/6] drm/komeda: Expose side_by_side by sysfs/config_id james qian wang (Arm Technology China)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).