Hi Kieran,
On Sun, Feb 17, 2019 at 08:16:27PM +0000, Kieran Bingham wrote:
> On 17/02/2019 02:48, Laurent Pinchart wrote:
> > The WPF accesses partition configuration from pipe->partition in the
> > partition configuration that is not used for display pipelines.
>
> That sentence is hard to read...
Indeed. I'll rewrite it.
> > Writeback support will require full configuration of the WPF while not
> > providing a valid pipe->partition. Rework the configuration code to fall
> > back to the full image width in that case, as is already done for the
> > part of the configuration currently relevant for display pipelines.
> >
>
> Ah yes - this is probably a better route than allocating a table for a
> single partition (which is what I had locally).
>
>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
>
> I like that this change also simplifies the flip/rotate handling code to
> make that easier to read.
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>
> > ---
> > drivers/media/platform/vsp1/vsp1_wpf.c | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
> > index 32bb207b2007..a07c5944b598 100644
> > --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> > +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> > @@ -362,6 +362,7 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
> > const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
> > unsigned int width;
> > unsigned int height;
> > + unsigned int left;
> > unsigned int offset;
> > unsigned int flip;
> > unsigned int i;
> > @@ -371,13 +372,16 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
> > RWPF_PAD_SINK);
> > width = sink_format->width;
> > height = sink_format->height;
> > + left = 0;
> >
> > /*
> > * Cropping. The partition algorithm can split the image into
> > * multiple slices.
> > */
> > - if (pipe->partitions > 1)
> > + if (pipe->partitions > 1) {
> > width = pipe->partition->wpf.width;
> > + left = pipe->partition->wpf.left;
> > + }
> >
> > vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
> > (0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
> > @@ -408,13 +412,11 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
> > flip = wpf->flip.active;
> >
> > if (flip & BIT(WPF_CTRL_HFLIP) && !wpf->flip.rotate)
> > - offset = format->width - pipe->partition->wpf.left
> > - - pipe->partition->wpf.width;
> > + offset = format->width - left - width;
> > else if (flip & BIT(WPF_CTRL_VFLIP) && wpf->flip.rotate)
> > - offset = format->height - pipe->partition->wpf.left
> > - - pipe->partition->wpf.width;
> > + offset = format->height - left - width;
> > else
> > - offset = pipe->partition->wpf.left;
> > + offset = left;
>
> This hunk looks a lot simpler :)
>
>
> >
> > for (i = 0; i < format->num_planes; ++i) {
> > unsigned int hsub = i > 0 ? fmtinfo->hsub : 1;
> > @@ -436,7 +438,7 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
> > * image height.
> > */
> > if (wpf->flip.rotate)
> > - height = pipe->partition->wpf.width;
> > + height = width;
> > else
> > height = format->height;
> >
> >
>
> --
> Regards
> --
> Kieran
--
Regards,
Laurent Pinchart
