Hi Kieran,
On Wednesday 12 Jul 2017 11:30:19 Kieran Bingham wrote:
> On 11/07/17 23:29, Laurent Pinchart wrote:
> > The DU can compose the output of a VSP with other planes on Gen2
> > hardware, and of two VSPs on Gen3 hardware. Neither of these features
> > are supported by the driver, and the current implementation always
> > assigns planes to CRTCs the same way.
> >
> > Simplify the implementation by configuring plane assignment when setting
> > up DU groups, instead of recomputing it for every atomic plane update.
> > This allows skipping the wait for vertical blanking when stopping a
> > CRTC, as there's no need to reconfigure plane assignment at that point.
> >
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>
> > ---
> >
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 31 ++++++++++++++-------------
> > drivers/gpu/drm/rcar-du/rcar_du_group.c | 12 ++++++++++++
> > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 28 +++++++++++++++++-----------
> > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 10 +---------
> > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 9 ---------
> > 5 files changed, 46 insertions(+), 44 deletions(-)
[snip]
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_group.c index
> > 00d5f470d377..d26b647207b8 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > @@ -126,6 +126,18 @@ static void rcar_du_group_setup(struct rcar_du_group
> > *rgrp)>
> > if (rcdu->info->gen >= 3)
> > rcar_du_group_write(rgrp, DEFR10, DEFR10_CODE |
DEFR10_DEFE10);
> >
> > + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) {
> > + /*
> > + * The CRTCs can compose the output of a VSP with other planes
> > + * on Gen2 hardware, and of two VSPs on Gen3 hardware. Neither
> > + * of these features are supported by the driver, so we
hardcode
> > + * plane assignment to CRTCs when setting the group up to
avoid
> > + * the need to restart then group when setting planes up.
>
> Minor nits in comment:
>
> /restart then group/restart the group/
>
> I would also possibly swap the final 'planes up' as 'up planes' if you
> update here anyway:
>
> * so we hardcode plane assignment to CRTCs when setting the group up to
> avoid
> * the need to restart the group when setting up planes.
>
> Up to you of course :)
Thanks, I've fixed both, and also replaced "setting the group up" with
"setting up the group".
> > + */
> > + rcar_du_group_write(rgrp, DS1PR, 1);
> > + rcar_du_group_write(rgrp, DS2PR, rcdu->info->gen >= 3 ? 3 :
2);
>
> whew ... that DS2PR indexing change from g2 to g3 looks annoying ... I had
> to write out the logic tables on paper to verify the change here from the
> previous code.
That's also how I wrote the code :-)
> > + }
> > +
> >
> > /*
> >
> > * Use DS1PR and DS2PR to configure planes priorities and connects the
> > * superposition 0 to DU0 pins. DU1 pins will be configured
dynamically.
--
Regards,
Laurent Pinchart
