Re: [v2,1/2] drm/komeda: Add slave pipeline support

From: james qian wang (Arm Technology China)
Date: Wed Jun 12 2019 - 07:02:10 EST


On Tue, Jun 11, 2019 at 11:13:39AM +0000, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@xxxxxxx>
>
> One crtc can use two komeda_pipeline, and one works as master and as
> slave. the slave pipeline doesn't have its own output and timing
> ctrlr, but pre-composite the input layer data flow and then feed the
> result to master. the pipeline configuration like:
>
> slave-layer-0 \
> ... slave->CU
> slave-layer-4 / \
> \
> master-layer-0 --------> master->CU -> ...
> ... /
> master-layer-4 ------>
>
> Since komeda Compiz doesn't output alpha, so the slave->CU result
> only can be used as bottom input when blend it with master input data
> flows.
>
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@xxxxxxx>
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 41 ++++++++++++++++++++--
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 10 ++++++
> drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 9 +++++
> .../gpu/drm/arm/display/komeda/komeda_pipeline.c | 22 ++++++++++++
> .../gpu/drm/arm/display/komeda/komeda_pipeline.h | 2 ++
> .../drm/arm/display/komeda/komeda_pipeline_state.c | 15 ++++++++
> drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 32 ++++++++++++++++-
> 7 files changed, 128 insertions(+), 3 deletions(-)
>
> --
> 1.9.1
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index a2d656f..cafb445 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -64,6 +64,10 @@ static void komeda_crtc_update_clock_ratio(struct komeda_crtc_state *kcrtc_st)
> }
>
> /* release unclaimed pipeline resources */
> + err = komeda_release_unclaimed_resources(kcrtc->slave, kcrtc_st);
> + if (err)
> + return err;
> +
> err = komeda_release_unclaimed_resources(kcrtc->master, kcrtc_st);
> if (err)
> return err;
> @@ -226,6 +230,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(crtc->state);
> struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
> struct komeda_pipeline *master = kcrtc->master;
> + struct komeda_pipeline *slave = kcrtc->slave;
> struct komeda_wb_connector *wb_conn = kcrtc->wb_conn;
> struct drm_connector_state *conn_st;
>
> @@ -237,6 +242,9 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> if (has_bit(master->id, kcrtc_st->affected_pipes))
> komeda_pipeline_update(master, old->state);
>
> + if (slave && has_bit(slave->id, kcrtc_st->affected_pipes))
> + komeda_pipeline_update(slave, old->state);
> +
> conn_st = wb_conn ? wb_conn->base.base.state : NULL;
> if (conn_st && conn_st->writeback_job)
> drm_writeback_queue_job(&wb_conn->base, conn_st);
> @@ -262,6 +270,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> struct komeda_crtc_state *old_st = to_kcrtc_st(old);
> struct komeda_dev *mdev = crtc->dev->dev_private;
> struct komeda_pipeline *master = kcrtc->master;
> + struct komeda_pipeline *slave = kcrtc->slave;
> struct completion *disable_done = &crtc->state->commit->flip_done;
> struct completion temp;
> int timeout;
> @@ -270,6 +279,9 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> drm_crtc_index(crtc),
> old_st->active_pipes, old_st->affected_pipes);
>
> + if (slave && has_bit(slave->id, old_st->active_pipes))
> + komeda_pipeline_disable(slave, old->state);
> +
> if (has_bit(master->id, old_st->active_pipes))
> komeda_pipeline_disable(master, old->state);
>
> @@ -414,6 +426,7 @@ static void komeda_crtc_reset(struct drm_crtc *crtc)
>
> new->affected_pipes = old->active_pipes;
> new->clock_ratio = old->clock_ratio;
> + new->max_slave_zorder = old->max_slave_zorder;
>
> return &new->base;
> }
> @@ -488,7 +501,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> master = mdev->pipelines[i];
>
> crtc->master = master;
> - crtc->slave = NULL;
> + crtc->slave = komeda_pipeline_get_slave(master);
>
> if (crtc->slave)
> sprintf(str, "pipe-%d", crtc->slave->id);
> @@ -522,6 +535,26 @@ static int komeda_crtc_create_clock_ratio_property(struct komeda_crtc *kcrtc)
> return 0;
> }
>
> +static int komeda_crtc_create_slave_planes_property(struct komeda_crtc *kcrtc)
> +{
> + struct drm_crtc *crtc = &kcrtc->base;
> + struct drm_property *prop;
> +
> + if (kcrtc->slave_planes == 0)
> + return 0;
> +
> + prop = drm_property_create_range(crtc->dev, DRM_MODE_PROP_IMMUTABLE,
> + "slave_planes", 0, U32_MAX);
> + if (!prop)
> + return -ENOMEM;
> +
> + drm_object_attach_property(&crtc->base, prop, kcrtc->slave_planes);
> +
> + kcrtc->slave_planes_property = prop;
> +
> + return 0;
> +}
> +
> static struct drm_plane *
> get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
> {
> @@ -562,7 +595,11 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
> if (err)
> return err;
>
> - return 0;
> + err = komeda_crtc_create_slave_planes_property(kcrtc);
> + if (err)
> + return err;
> +
> + return err;
> }
>
> int komeda_kms_add_crtcs(struct komeda_kms_dev *kms, struct komeda_dev *mdev)
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index 5d10c55..8543860 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -143,6 +143,8 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> struct drm_crtc_state *crtc_st)
> {
> struct drm_atomic_state *state = crtc_st->state;
> + struct komeda_crtc *kcrtc = to_kcrtc(crtc);
> + struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(crtc_st);
> struct komeda_plane_state *kplane_st;
> struct drm_plane_state *plane_st;
> struct drm_framebuffer *fb;
> @@ -167,6 +169,8 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> return err;
> }
>
> + kcrtc_st->max_slave_zorder = 0;
> +
> list_for_each_entry(kplane_st, &zorder_list, zlist_node) {
> plane_st = &kplane_st->base;
> fb = plane_st->fb;
> @@ -185,6 +189,12 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] zpos:%d, normalized zpos: %d\n",
> plane->base.id, plane->name,
> plane_st->zpos, plane_st->normalized_zpos);
> +
> + /* calculate max slave zorder */
> + if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
> + kcrtc_st->max_slave_zorder =
> + max(plane_st->normalized_zpos,
> + kcrtc_st->max_slave_zorder);
> }
>
> crtc_st->zpos_changed = true;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> index 9dcfe5a..219fa3f 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> @@ -86,6 +86,9 @@ struct komeda_crtc {
> */
> struct komeda_pipeline *slave;
>
> + /** @slave_planes: komeda slave planes mask */
> + u32 slave_planes;
> +
> /** @wb_conn: komeda write back connector */
> struct komeda_wb_connector *wb_conn;
>
> @@ -94,6 +97,9 @@ struct komeda_crtc {
>
> /** @clock_ratio_property: property for ratio of (aclk << 32)/pxlclk */
> struct drm_property *clock_ratio_property;
> +
> + /** @slave_planes_property: property for slaves of the planes */
> + struct drm_property *slave_planes_property;
> };
>
> /**
> @@ -119,6 +125,9 @@ struct komeda_crtc_state {
>
> /** @clock_ratio: ratio of (aclk << 32)/pxlclk */
> u64 clock_ratio;
> +
> + /** @max_slave_zorder: the maximum of slave zorder */
> + u32 max_slave_zorder;
> };
>
> /** struct komeda_kms_dev - for gather KMS related things */
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> index eb9e0c0..a3d90d8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
> @@ -142,6 +142,14 @@ struct komeda_component *
> return c;
> }
>
> +static struct komeda_component *
> +komeda_component_pickup_input(struct komeda_component *c, u32 avail_comps)
> +{
> + u32 avail_inputs = c->supported_inputs & (avail_comps);
> +
> + return komeda_pipeline_get_first_component(c->pipeline, avail_inputs);
> +}
> +
> /** komeda_component_add - Add a component to &komeda_pipeline */
> struct komeda_component *
> komeda_component_add(struct komeda_pipeline *pipe,
> @@ -296,6 +304,20 @@ static void komeda_pipeline_assemble(struct komeda_pipeline *pipe)
> }
> }
>
> +/* if pipeline_A accept another pipeline_B's component as input, treat
> + * pipeline_B as slave of pipeline_A.
> + */
> +struct komeda_pipeline *
> +komeda_pipeline_get_slave(struct komeda_pipeline *master)
> +{
> + struct komeda_component *slave;
> +
> + slave = komeda_component_pickup_input(&master->compiz->base,
> + KOMEDA_PIPELINE_COMPIZS);
> +
> + return slave ? slave->pipeline : NULL;
> +}
> +
> int komeda_assemble_pipelines(struct komeda_dev *mdev)
> {
> 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 f6a4a51..2a67c8a 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -452,6 +452,8 @@ struct komeda_pipeline *
> const struct komeda_pipeline_funcs *funcs);
> void komeda_pipeline_destroy(struct komeda_dev *mdev,
> struct komeda_pipeline *pipe);
> +struct komeda_pipeline *
> +komeda_pipeline_get_slave(struct komeda_pipeline *master);
> int komeda_assemble_pipelines(struct komeda_dev *mdev);
> struct komeda_component *
> komeda_pipeline_get_component(struct komeda_pipeline *pipe, int id);
> 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 23182ed..6c35afd 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -1032,10 +1032,25 @@ int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
> struct komeda_crtc_state *kcrtc_st)
> {
> struct komeda_pipeline *master = kcrtc->master;
> + struct komeda_pipeline *slave = kcrtc->slave;
> struct komeda_data_flow_cfg m_dflow; /* master data flow */
> + struct komeda_data_flow_cfg s_dflow; /* slave data flow */
> int err;
>
> memset(&m_dflow, 0, sizeof(m_dflow));
> + memset(&s_dflow, 0, sizeof(s_dflow));
> +
> + if (slave && has_bit(slave->id, kcrtc_st->active_pipes)) {
> + err = komeda_compiz_validate(slave->compiz, kcrtc_st, &s_dflow);
> + if (err)
> + return err;
> +
> + /* merge the slave dflow into master pipeline */
> + err = komeda_compiz_set_input(master->compiz, kcrtc_st,
> + &s_dflow);
> + if (err)
> + return err;
> + }
>
> err = komeda_compiz_validate(master->compiz, kcrtc_st, &m_dflow);
> if (err)
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> index d1c58a8..04b122f 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> @@ -14,15 +14,27 @@
>
> static int
> komeda_plane_init_data_flow(struct drm_plane_state *st,
> + struct komeda_crtc_state *kcrtc_st,
> struct komeda_data_flow_cfg *dflow)
> {
> + struct komeda_plane *kplane = to_kplane(st->plane);
> struct komeda_plane_state *kplane_st = to_kplane_st(st);
> struct drm_framebuffer *fb = st->fb;
> const struct komeda_format_caps *caps = to_kfb(fb)->format_caps;
> + struct komeda_pipeline *pipe = kplane->layer->base.pipeline;
>
> memset(dflow, 0, sizeof(*dflow));
>
> dflow->blending_zorder = st->normalized_zpos;
> + if (pipe == to_kcrtc(st->crtc)->master)
> + dflow->blending_zorder -= kcrtc_st->max_slave_zorder;
> + if (dflow->blending_zorder < 0) {
> + DRM_DEBUG_ATOMIC("%s zorder:%d < max_slave_zorder: %d.\n",
> + st->plane->name, st->normalized_zpos,
> + kcrtc_st->max_slave_zorder);
> + return -EINVAL;
> + }
> +
> dflow->pixel_blend_mode = st->pixel_blend_mode;
> dflow->layer_alpha = st->alpha >> 8;
>
> @@ -88,7 +100,7 @@
>
> kcrtc_st = to_kcrtc_st(crtc_st);
>
> - err = komeda_plane_init_data_flow(state, &dflow);
> + err = komeda_plane_init_data_flow(state, kcrtc_st, &dflow);
> if (err)
> return err;
>
> @@ -288,6 +300,22 @@ static u32 get_possible_crtcs(struct komeda_kms_dev *kms,
> return possible_crtcs;
> }
>
> +static void
> +komeda_set_crtc_plane_mask(struct komeda_kms_dev *kms,
> + struct komeda_pipeline *pipe,
> + struct drm_plane *plane)
> +{
> + struct komeda_crtc *kcrtc;
> + int i;
> +
> + for (i = 0; i < kms->n_crtcs; i++) {
> + kcrtc = &kms->crtcs[i];
> +
> + if (pipe == kcrtc->slave)
> + kcrtc->slave_planes |= BIT(drm_plane_index(plane));
> + }
> +}
> +
> /* use Layer0 as primary */
> static u32 get_plane_type(struct komeda_kms_dev *kms,
> struct komeda_component *c)
> @@ -366,6 +394,8 @@ static int komeda_plane_add(struct komeda_kms_dev *kms,
> if (err)
> goto cleanup;
>
> + komeda_set_crtc_plane_mask(kms, c->pipeline, plane);
> +
> return 0;
> cleanup:
> komeda_plane_destroy(plane);


Looks good to me.

Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@xxxxxxx>