Re: [PATCH v4 2/7] media: vsp1: wpf: Fix partition configuration for display pipelines

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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@ideasonboard.com>
> 
> 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@ideasonboard.com>
> 
> > ---
> >  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