Hi Laurent,
Thank you for the patch,
On 25/11/2018 14:40, Laurent Pinchart wrote:
> DU channels are routed to DPAD outputs in an SoC-dependent way. The
> routing can be fixed (e.g. DU3 to DPAD0 on H3) or configurable (e.g. DU0
> or DU1 to DPAD0 on D3/E3). The hardware offers no option to disconnect
> DPAD outputs, which are thus always driven by a DU channel.
>
> On SoCs that have less DU channels than DU outputs, such as D3 and E3,
> the DPAD output is always driven when all channels are in used by other
s/used/use/
> outputs (such as the internal LVDS and HDMI encoders). This creates an
> unwanted clone on the DPAD output.
>
> However, the parallel output of the DU channels routed to DPAD can be
> set to fixed levels in the DU channels themselves through the DOFLR
> group register. Use this to turn the DPAD on or off by driving fixed
> signals at the output of any DU channel not routed to a DPAD output.
> This doesn't affect the DU output signals going to other outputs.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
Only spelling and bikeshedding here - so:
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/rcar-du/rcar_du_group.c | 42 +++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 7e440f61977f..5aaf41b7a2ca 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -287,6 +287,46 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
> return 0;
> }
>
> +static void rcar_du_group_set_output_levels(struct rcar_du_group *rgrp)
> +{
> + static const u32 doflr_values[2] = {
> + DOFLR_HSYCFL0 | DOFLR_VSYCFL0 | DOFLR_ODDFL0 |
> + DOFLR_DISPFL0 | DOFLR_CDEFL0 | DOFLR_RGBFL0,
> + DOFLR_HSYCFL1 | DOFLR_VSYCFL1 | DOFLR_ODDFL1 |
> + DOFLR_DISPFL1 | DOFLR_CDEFL1 | DOFLR_RGBFL1,
> + };
> + static const u32 dpad_mask = BIT(RCAR_DU_OUTPUT_DPAD1)
> + | BIT(RCAR_DU_OUTPUT_DPAD0);
> + struct rcar_du_device *rcdu = rgrp->dev;
> + u32 doflr = DOFLR_CODE;
> + unsigned int i;
> +
> + if (rcdu->info->gen < 2)
> + return;
> +
> + /*
> + * The DPAD outputs can't be controlled directly. However, the parallel
> + * output of the DU channels routed to DPAD can be set to fixed levels
> + * through the DOFLR group register. Use this to turn the DPAD on or off
> + * by driving fixed signals at the output of any DU channel not routed
> + * to a DPAD output. This doesn't affect the DU output signals going to
> + * other outputs, such as the internal LVDS and HDMI encoders.
Perhaps more out of interest - what /fixed/ levels do we output.
High/Low/Hi-Z ?
> + */
> +
> + for (i = 0; i < rgrp->num_crtcs; ++i) {
> + struct rcar_du_crtc_state *rstate;
> + struct rcar_du_crtc *rcrtc;
> +
> + rcrtc = &rcdu->crtcs[rgrp->index * 2 + i];> + rstate = to_rcar_crtc_state(rcrtc->crtc.state);
> +
> + if (!(rstate->outputs & dpad_mask))
> + doflr |= doflr_values[i];> + }
> +
> + rcar_du_group_write(rgrp, DOFLR, doflr);
> +}
> +
> int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
> {
> struct rcar_du_device *rcdu = rgrp->dev;
> @@ -306,5 +346,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
>
> rcar_du_group_write(rgrp, DORCR, dorcr);
>
> + rcar_du_group_set_output_levels(rgrp);
Shouldn't this be:
rcar_du_group_set_dpad_levels()
Anyway - that's just bikeshedding - I'll leave the decision (even if
that's keeping this as is) to you.
> +
> return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
> }
>
--
Regards
--
Kieran
