[PATCH 2/2] gpu: ipu-v3: pre: add dynamic buffer layout reconfiguration

[Philipp Zabel]

On Thu, Jan 26, 2023 at 07:47:44PM +0100, Lucas Stach wrote:
> imx-drm doesn't mandate a modeset when the framebuffer modifier changes,
> but currently the tile prefetch and resolve (TPR) configuration of the
> PRE is only set up on the initial modeset.
> 
> As the TPR configuration is double buffered, same as all the other PRE
> states, we can support dynamic reconfiguration of the buffer layout from
> one frame to another. As switching between (super-)tiled and linear
> prefetch needs to touch the CTRL register make sure to do the
> reconfiguration inside the safe window.
> 
> Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
> ---
>  drivers/gpu/ipu-v3/ipu-pre.c | 59 +++++++++++++++++++++++++++++-------
>  drivers/gpu/ipu-v3/ipu-prg.c |  2 +-
>  drivers/gpu/ipu-v3/ipu-prv.h |  2 +-
>  3 files changed, 50 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
> index befffc85a146..e8d9792827dd 100644
> --- a/drivers/gpu/ipu-v3/ipu-pre.c
> +++ b/drivers/gpu/ipu-v3/ipu-pre.c
> @@ -99,8 +99,12 @@ struct ipu_pre {
>  
>  	struct {
>  		bool		in_use;
> +		uint64_t	modifier;
> +		unsigned int	height;
>  		unsigned int	safe_window_end;
>  		unsigned int	bufaddr;
> +		u32		ctrl;
> +		u8		cpp;
>  	} cur;
>  };
>  
> @@ -173,6 +177,11 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int width,
>  	u32 active_bpp = info->cpp[0] >> 1;
>  	u32 val;
>  
> +	pre->cur.bufaddr = bufaddr;
> +	pre->cur.height = height;
> +	pre->cur.modifier = modifier;
> +	pre->cur.cpp = info->cpp[0];
> +
>  	/* calculate safe window for ctrl register updates */
>  	if (modifier == DRM_FORMAT_MOD_LINEAR)
>  		pre->cur.safe_window_end = height - 2;
> @@ -181,7 +190,6 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int width,
>  
>  	writel(bufaddr, pre->regs + IPU_PRE_CUR_BUF);
>  	writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
> -	pre->cur.bufaddr = bufaddr;
>  
>  	val = IPU_PRE_PREF_ENG_CTRL_INPUT_PIXEL_FORMAT(0) |
>  	      IPU_PRE_PREF_ENG_CTRL_INPUT_ACTIVE_BPP(active_bpp) |
> @@ -223,28 +231,56 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int width,
>  	}
>  	writel(val, pre->regs + IPU_PRE_TPR_CTRL);
>  
> -	val = readl(pre->regs + IPU_PRE_CTRL);
> -	val |= IPU_PRE_CTRL_EN_REPEAT | IPU_PRE_CTRL_ENABLE |
> -	       IPU_PRE_CTRL_SDW_UPDATE;
> +	pre->cur.ctrl = readl(pre->regs + IPU_PRE_CTRL);
> +	pre->cur.ctrl |= IPU_PRE_CTRL_EN_REPEAT | IPU_PRE_CTRL_ENABLE;
>  	if (modifier == DRM_FORMAT_MOD_LINEAR)
> -		val &= ~IPU_PRE_CTRL_BLOCK_EN;
> +		pre->cur.ctrl &= ~IPU_PRE_CTRL_BLOCK_EN;
>  	else
> -		val |= IPU_PRE_CTRL_BLOCK_EN;
> -	writel(val, pre->regs + IPU_PRE_CTRL);
> +		pre->cur.ctrl |= IPU_PRE_CTRL_BLOCK_EN;
> +	writel(pre->cur.ctrl | IPU_PRE_CTRL_SDW_UPDATE,
> +	       pre->regs + IPU_PRE_CTRL);
>  }
>  
> -void ipu_pre_update(struct ipu_pre *pre, unsigned int bufaddr)
> +void ipu_pre_update(struct ipu_pre *pre, uint64_t modifier, unsigned int bufaddr)
>  {
>  	unsigned long timeout = jiffies + msecs_to_jiffies(5);
>  	unsigned short current_yblock;
> +	unsigned int safe_window_end = pre->cur.safe_window_end;
>  	u32 val;
>  
> -	if (bufaddr == pre->cur.bufaddr)
> +	if (bufaddr == pre->cur.bufaddr &&
> +	    modifier == pre->cur.modifier)
>  		return;
>  
>  	writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
>  	pre->cur.bufaddr = bufaddr;
>  
> +	if (modifier != pre->cur.modifier) {
> +		val = readl(pre->regs + IPU_PRE_TPR_CTRL);
> +		val &= ~IPU_PRE_TPR_CTRL_TILE_FORMAT_MASK;
> +		if (modifier != DRM_FORMAT_MOD_LINEAR) {
> +			/* only support single buffer formats for now */
> +			val |= IPU_PRE_TPR_CTRL_TILE_FORMAT_SINGLE_BUF;
> +			if (modifier == DRM_FORMAT_MOD_VIVANTE_SUPER_TILED)
> +				val |= IPU_PRE_TPR_CTRL_TILE_FORMAT_SUPER_TILED;
> +			if (pre->cur.cpp == 2)
> +				val |= IPU_PRE_TPR_CTRL_TILE_FORMAT_16_BIT;
> +		}
> +		writel(val, pre->regs + IPU_PRE_TPR_CTRL);
> +
> +		if (modifier == DRM_FORMAT_MOD_LINEAR)
> +			pre->cur.ctrl &= ~IPU_PRE_CTRL_BLOCK_EN;
> +		else
> +			pre->cur.ctrl |= IPU_PRE_CTRL_BLOCK_EN;
> +
> +		if (modifier == DRM_FORMAT_MOD_LINEAR)
> +			pre->cur.safe_window_end = pre->cur.height - 2;
> +		else
> +			pre->cur.safe_window_end = DIV_ROUND_UP(pre->cur.height, 4) - 1;

Could you extract the same code from ipu_pre_configure() into a separate
function, say ipu_pre_configure_modifier(), instead, and call that from
both places?

regards
Philipp