Re: [PATCH v2 12/14] drm: rcar-du: Support multiple sources from the same VSP
Hi Kieran, On Tuesday 01 Aug 2017 19:10:20 Kieran Bingham wrote: > On 26/06/17 19:12, Laurent Pinchart wrote: > > On R-Car H3 ES2.0, DU channels 0 and 3 are served by two separate > > pipelines from the same VSP. Support this in the DU driver. > > > > Signed-off-by: Laurent Pinchart > >> > This looks good to me. > > Minor nit / comment can be safely ignored. Mostly just me thinking outload. > > Reviewed-by: Kieran Bingham > > > --- > > > > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +- > > drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 3 ++ > > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 91 + > > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 37 +++--- > > drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 10 +++- > > 5 files changed, 110 insertions(+), 33 deletions(-) [snip] > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index f4125c8ca902..82b978a5dae6 > > 100644 > > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > > @@ -432,6 +432,83 @@ static int rcar_du_properties_init(struct > > rcar_du_device *rcdu) > > return 0; > > } > > > > +static int rcar_du_vsps_init(struct rcar_du_device *rcdu) > > +{ > > + const struct device_node *np = rcdu->dev->of_node; > > + struct of_phandle_args args; > > + struct { > > + struct device_node *np; > > + unsigned int crtcs_mask; > > + } vsps[RCAR_DU_MAX_VSPS] = { { 0, }, }; > > + unsigned int vsps_count = 0; > > + unsigned int cells; > > + unsigned int i; > > + int ret; > > + > > + /* > > +* First parse the DT vsps property to populate the list of VSPs. Each > > +* entry contains a pointer to the VSP DT node and a bitmask of the > > +* connected DU CRTCs. > > +*/ > > + cells = of_property_count_u32_elems(np, "vsps") / rcdu->num_crtcs - 1; > > + if (cells > 1) > > + return -EINVAL; > > + > > + for (i = 0; i < rcdu->num_crtcs; ++i) { > > + unsigned int j; > > + > > + ret = of_parse_phandle_with_fixed_args(np, "vsps", cells, i, > > + ); > > + if (ret < 0) > > + goto error; > > + > > + /* > > +* Add the VSP to the list or update the corresponding > > existing > > +* entry if the VSP has already been added. > > +*/ > > + for (j = 0; j < vsps_count; ++j) { > > + if (vsps[j].np == args.np) > > + break; > > + } > > + > > + if (j < vsps_count) > > + of_node_put(args.np); > > + else > > + vsps[vsps_count++].np = args.np; > > + > > + vsps[j].crtcs_mask |= 1 << i; > > I do love the BIT(x) macro personally - but it's not important :) I wonder why I don't like the BIT macro. There's probably no good reason. > > + > > + /* Store the VSP pointer and pipe index in the CRTC. */ > > + rcdu->crtcs[i].vsp = >vsps[j]; > > + rcdu->crtcs[i].vsp_pipe = cells >= 1 ? args.args[0] : 0; > > + } > > + > > + /* > > +* Then initialize all the VSPs from the node pointers and CRTCs > > bitmask > > +* computed previously. > > +*/ > > + for (i = 0; i < vsps_count; ++i) { > > + struct rcar_du_vsp *vsp = >vsps[i]; > > + > > + vsp->index = i; > > + vsp->dev = rcdu; > > + > > + ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask); > > + if (ret < 0) > > + goto error; > > + } > > + > > + return 0; > > + > > +error: > > + for (i = 0; i < ARRAY_SIZE(vsps); ++i) { > > + if (vsps[i].np) > > Minor nit: of_node_put already has NULL protection so we don't need this > 'if' but it probably does make it clearer that we are only putting back > nodes that we collected. It's a good point, I'll remove the check. > > + of_node_put(vsps[i].np); > > + } > > + > > + return ret; > > +} [snip] -- Regards, Laurent Pinchart
Re: [PATCH v2 12/14] drm: rcar-du: Support multiple sources from the same VSP
Hi Laurent, On 26/06/17 19:12, Laurent Pinchart wrote: > On R-Car H3 ES2.0, DU channels 0 and 3 are served by two separate > pipelines from the same VSP. Support this in the DU driver. > > Signed-off-by: Laurent PinchartThis looks good to me. Minor nit / comment can be safely ignored. Mostly just me thinking outload. Reviewed-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +- > drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 3 ++ > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 91 > ++ > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 37 +++--- > drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 10 +++- > 5 files changed, 110 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > index 345eff72f581..8f942ebdd0c6 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -721,7 +721,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, > unsigned int index) > rcrtc->index = index; > > if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) > - primary = >vsp->planes[0].plane; > + primary = >vsp->planes[rcrtc->vsp_pipe].plane; > else > primary = >planes[index % 2].plane; > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > index b199ed5adf36..0b6d26ecfc38 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > @@ -35,6 +35,8 @@ struct rcar_du_vsp; > * @flip_wait: wait queue used to signal page flip completion > * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC > * @group: CRTC group this CRTC belongs to > + * @vsp: VSP feeding video to this CRTC > + * @vsp_pipe: index of the VSP pipeline feeding video to this CRTC > */ > struct rcar_du_crtc { > struct drm_crtc crtc; > @@ -52,6 +54,7 @@ struct rcar_du_crtc { > > struct rcar_du_group *group; > struct rcar_du_vsp *vsp; > + unsigned int vsp_pipe; > }; > > #define to_rcar_crtc(c) container_of(c, struct rcar_du_crtc, crtc) > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > index f4125c8ca902..82b978a5dae6 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > @@ -432,6 +432,83 @@ static int rcar_du_properties_init(struct rcar_du_device > *rcdu) > return 0; > } > > +static int rcar_du_vsps_init(struct rcar_du_device *rcdu) > +{ > + const struct device_node *np = rcdu->dev->of_node; > + struct of_phandle_args args; > + struct { > + struct device_node *np; > + unsigned int crtcs_mask; > + } vsps[RCAR_DU_MAX_VSPS] = { { 0, }, }; > + unsigned int vsps_count = 0; > + unsigned int cells; > + unsigned int i; > + int ret; > + > + /* > + * First parse the DT vsps property to populate the list of VSPs. Each > + * entry contains a pointer to the VSP DT node and a bitmask of the > + * connected DU CRTCs. > + */ > + cells = of_property_count_u32_elems(np, "vsps") / rcdu->num_crtcs - 1; > + if (cells > 1) > + return -EINVAL; > + > + for (i = 0; i < rcdu->num_crtcs; ++i) { > + unsigned int j; > + > + ret = of_parse_phandle_with_fixed_args(np, "vsps", cells, i, > +); > + if (ret < 0) > + goto error; > + > + /* > + * Add the VSP to the list or update the corresponding existing > + * entry if the VSP has already been added. > + */ > + for (j = 0; j < vsps_count; ++j) { > + if (vsps[j].np == args.np) > + break; > + } > + > + if (j < vsps_count) > + of_node_put(args.np); > + else > + vsps[vsps_count++].np = args.np; > + > + vsps[j].crtcs_mask |= 1 << i; I do love the BIT(x) macro personally - but it's not important :) > + > + /* Store the VSP pointer and pipe index in the CRTC. */ > + rcdu->crtcs[i].vsp = >vsps[j]; > + rcdu->crtcs[i].vsp_pipe = cells >= 1 ? args.args[0] : 0; > + } > + > + /* > + * Then initialize all the VSPs from the node pointers and CRTCs bitmask > + * computed previously. > + */ > + for (i = 0; i < vsps_count; ++i) { > + struct rcar_du_vsp *vsp = >vsps[i]; > + > + vsp->index = i; > + vsp->dev = rcdu; > + > + ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask); > + if (ret < 0) > + goto error; > + } > + > + return 0; > + >
[PATCH v2 12/14] drm: rcar-du: Support multiple sources from the same VSP
On R-Car H3 ES2.0, DU channels 0 and 3 are served by two separate pipelines from the same VSP. Support this in the DU driver. Signed-off-by: Laurent Pinchart--- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 3 ++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 91 ++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 37 +++--- drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 10 +++- 5 files changed, 110 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 345eff72f581..8f942ebdd0c6 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -721,7 +721,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index) rcrtc->index = index; if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) - primary = >vsp->planes[0].plane; + primary = >vsp->planes[rcrtc->vsp_pipe].plane; else primary = >planes[index % 2].plane; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index b199ed5adf36..0b6d26ecfc38 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h @@ -35,6 +35,8 @@ struct rcar_du_vsp; * @flip_wait: wait queue used to signal page flip completion * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC * @group: CRTC group this CRTC belongs to + * @vsp: VSP feeding video to this CRTC + * @vsp_pipe: index of the VSP pipeline feeding video to this CRTC */ struct rcar_du_crtc { struct drm_crtc crtc; @@ -52,6 +54,7 @@ struct rcar_du_crtc { struct rcar_du_group *group; struct rcar_du_vsp *vsp; + unsigned int vsp_pipe; }; #define to_rcar_crtc(c)container_of(c, struct rcar_du_crtc, crtc) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index f4125c8ca902..82b978a5dae6 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -432,6 +432,83 @@ static int rcar_du_properties_init(struct rcar_du_device *rcdu) return 0; } +static int rcar_du_vsps_init(struct rcar_du_device *rcdu) +{ + const struct device_node *np = rcdu->dev->of_node; + struct of_phandle_args args; + struct { + struct device_node *np; + unsigned int crtcs_mask; + } vsps[RCAR_DU_MAX_VSPS] = { { 0, }, }; + unsigned int vsps_count = 0; + unsigned int cells; + unsigned int i; + int ret; + + /* +* First parse the DT vsps property to populate the list of VSPs. Each +* entry contains a pointer to the VSP DT node and a bitmask of the +* connected DU CRTCs. +*/ + cells = of_property_count_u32_elems(np, "vsps") / rcdu->num_crtcs - 1; + if (cells > 1) + return -EINVAL; + + for (i = 0; i < rcdu->num_crtcs; ++i) { + unsigned int j; + + ret = of_parse_phandle_with_fixed_args(np, "vsps", cells, i, + ); + if (ret < 0) + goto error; + + /* +* Add the VSP to the list or update the corresponding existing +* entry if the VSP has already been added. +*/ + for (j = 0; j < vsps_count; ++j) { + if (vsps[j].np == args.np) + break; + } + + if (j < vsps_count) + of_node_put(args.np); + else + vsps[vsps_count++].np = args.np; + + vsps[j].crtcs_mask |= 1 << i; + + /* Store the VSP pointer and pipe index in the CRTC. */ + rcdu->crtcs[i].vsp = >vsps[j]; + rcdu->crtcs[i].vsp_pipe = cells >= 1 ? args.args[0] : 0; + } + + /* +* Then initialize all the VSPs from the node pointers and CRTCs bitmask +* computed previously. +*/ + for (i = 0; i < vsps_count; ++i) { + struct rcar_du_vsp *vsp = >vsps[i]; + + vsp->index = i; + vsp->dev = rcdu; + + ret = rcar_du_vsp_init(vsp, vsps[i].np, vsps[i].crtcs_mask); + if (ret < 0) + goto error; + } + + return 0; + +error: + for (i = 0; i < ARRAY_SIZE(vsps); ++i) { + if (vsps[i].np) + of_node_put(vsps[i].np); + } + + return ret; +} + int rcar_du_modeset_init(struct rcar_du_device *rcdu) { static const unsigned int mmio_offsets[] = { @@ -499,17 +576,9 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) /* Initialize the compositors. */ if