Hi: All
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
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) <[email protected]>
---
.../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 10a0dc9291b8..b1e90feb5c55 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
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) <[email protected]>
---
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
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) <[email protected]>
---
.../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 b1e90feb5c55..79f7e7b6526f 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
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) <[email protected]>
---
.../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
On Thu, Nov 21, 2019 at 07:12:55AM +0000, james qian wang (Arm Technology China) wrote:
> 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) <[email protected]>
> ---
> 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)
Uh, this sysfs file here looks a lot like uapi for some compositor to
decide what to do. Do you have the userspace for this?
Also a few more thoughts on this:
- You can't just add more fields to uapi structs.
- This doesn't really feel like it was ever reviewed to fit into atomic.
- sysfs should be one value per file, not a smorgasbrod of things stuffed
into a binary structure.
-Daniel
> 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
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Thu, Nov 21, 2019 at 10:49:26AM +0100, Daniel Vetter wrote:
> On Thu, Nov 21, 2019 at 07:12:55AM +0000, james qian wang (Arm Technology China) wrote:
> > 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) <[email protected]>
> > ---
> > 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)
>
> Uh, this sysfs file here looks a lot like uapi for some compositor to
> decide what to do. Do you have the userspace for this?
Yes, our HWC driver uses this config_id and product_id for identifying the
HW caps.
> Also a few more thoughts on this:
> - You can't just add more fields to uapi structs.
> - This doesn't really feel like it was ever reviewed to fit into atomic.
> - sysfs should be one value per file, not a smorgasbrod of things stuffed
> into a binary structure.
I will sent a series and split this struct to multiple files.
| This doesn't really feel like it was ever reviewed to fit into atomic
These values don't have atomic problem, since config_id is for
representing the HW caps info, which are not configurable.
Thanks
James
> -Daniel
>
> > 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
> >
> > _______________________________________________
> > dri-devel mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Thu, 21 Nov 2019 at 20:21, james qian wang (Arm Technology China)
<[email protected]> wrote:
>
> On Thu, Nov 21, 2019 at 10:49:26AM +0100, Daniel Vetter wrote:
> > On Thu, Nov 21, 2019 at 07:12:55AM +0000, james qian wang (Arm Technology China) wrote:
> > > 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) <[email protected]>
> > > ---
> > > 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)
> >
> > Uh, this sysfs file here looks a lot like uapi for some compositor to
> > decide what to do. Do you have the userspace for this?
>
> Yes, our HWC driver uses this config_id and product_id for identifying the
> HW caps.
>
This seems like it should be done more in the kernel, why does
userspace needs all that info, to make more informed decisions?
How would drm_hwcomposer get the same result?
I'd prefer we just remove the sysfs nodes from upstream unless we have
an upstream user for them.
Dave.