Hi Kieran,
On Friday, 7 December 2018 14:50:47 EET Kieran Bingham wrote:
> 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 ?
It's low-level, I'll update the comment.
> > + */
> > +
> > + 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.
Good idea, I'll rename the function.
> > +
> >
> > return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
> >
> > }
--
Regards,
Laurent Pinchart
