All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gpu: ipu-v3: pre: move state into struct
@ 2023-01-26 18:47 Lucas Stach
  2023-01-26 18:47 ` [PATCH 2/2] gpu: ipu-v3: pre: add dynamic buffer layout reconfiguration Lucas Stach
  2023-01-27 10:20 ` [PATCH 1/2] gpu: ipu-v3: pre: move state into struct Philipp Zabel
  0 siblings, 2 replies; 4+ messages in thread
From: Lucas Stach @ 2023-01-26 18:47 UTC (permalink / raw)
  To: Philipp Zabel; +Cc: kernel, dri-devel, patchwork-lst

Move the variables tracking the current dynamic state into a struct
to separate it a bit better from the static device properties.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/ipu-v3/ipu-pre.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
index ad82c9e0252f..befffc85a146 100644
--- a/drivers/gpu/ipu-v3/ipu-pre.c
+++ b/drivers/gpu/ipu-v3/ipu-pre.c
@@ -96,9 +96,12 @@ struct ipu_pre {
 
 	dma_addr_t		buffer_paddr;
 	void			*buffer_virt;
-	bool			in_use;
-	unsigned int		safe_window_end;
-	unsigned int		last_bufaddr;
+
+	struct {
+		bool		in_use;
+		unsigned int	safe_window_end;
+		unsigned int	bufaddr;
+	} cur;
 };
 
 static DEFINE_MUTEX(ipu_pre_list_mutex);
@@ -138,7 +141,7 @@ int ipu_pre_get(struct ipu_pre *pre)
 {
 	u32 val;
 
-	if (pre->in_use)
+	if (pre->cur.in_use)
 		return -EBUSY;
 
 	/* first get the engine out of reset and remove clock gating */
@@ -151,7 +154,7 @@ int ipu_pre_get(struct ipu_pre *pre)
 	      IPU_PRE_CTRL_SDW_UPDATE;
 	writel(val, pre->regs + IPU_PRE_CTRL);
 
-	pre->in_use = true;
+	pre->cur.in_use = true;
 	return 0;
 }
 
@@ -159,7 +162,7 @@ void ipu_pre_put(struct ipu_pre *pre)
 {
 	writel(IPU_PRE_CTRL_SFTRST, pre->regs + IPU_PRE_CTRL);
 
-	pre->in_use = false;
+	pre->cur.in_use = false;
 }
 
 void ipu_pre_configure(struct ipu_pre *pre, unsigned int width,
@@ -172,13 +175,13 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int width,
 
 	/* calculate safe window for ctrl register updates */
 	if (modifier == DRM_FORMAT_MOD_LINEAR)
-		pre->safe_window_end = height - 2;
+		pre->cur.safe_window_end = height - 2;
 	else
-		pre->safe_window_end = DIV_ROUND_UP(height, 4) - 1;
+		pre->cur.safe_window_end = DIV_ROUND_UP(height, 4) - 1;
 
 	writel(bufaddr, pre->regs + IPU_PRE_CUR_BUF);
 	writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
-	pre->last_bufaddr = bufaddr;
+	pre->cur.bufaddr = bufaddr;
 
 	val = IPU_PRE_PREF_ENG_CTRL_INPUT_PIXEL_FORMAT(0) |
 	      IPU_PRE_PREF_ENG_CTRL_INPUT_ACTIVE_BPP(active_bpp) |
@@ -236,11 +239,11 @@ void ipu_pre_update(struct ipu_pre *pre, unsigned int bufaddr)
 	unsigned short current_yblock;
 	u32 val;
 
-	if (bufaddr == pre->last_bufaddr)
+	if (bufaddr == pre->cur.bufaddr)
 		return;
 
 	writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
-	pre->last_bufaddr = bufaddr;
+	pre->cur.bufaddr = bufaddr;
 
 	do {
 		if (time_after(jiffies, timeout)) {
@@ -252,7 +255,7 @@ void ipu_pre_update(struct ipu_pre *pre, unsigned int bufaddr)
 		current_yblock =
 			(val >> IPU_PRE_STORE_ENG_STATUS_STORE_BLOCK_Y_SHIFT) &
 			IPU_PRE_STORE_ENG_STATUS_STORE_BLOCK_Y_MASK;
-	} while (current_yblock == 0 || current_yblock >= pre->safe_window_end);
+	} while (current_yblock == 0 || current_yblock >= pre->cur.safe_window_end);
 
 	writel(IPU_PRE_CTRL_SDW_UPDATE, pre->regs + IPU_PRE_CTRL_SET);
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] gpu: ipu-v3: pre: add dynamic buffer layout reconfiguration
  2023-01-26 18:47 [PATCH 1/2] gpu: ipu-v3: pre: move state into struct Lucas Stach
@ 2023-01-26 18:47 ` Lucas Stach
  2023-01-27 10:20   ` Philipp Zabel
  2023-01-27 10:20 ` [PATCH 1/2] gpu: ipu-v3: pre: move state into struct Philipp Zabel
  1 sibling, 1 reply; 4+ messages in thread
From: Lucas Stach @ 2023-01-26 18:47 UTC (permalink / raw)
  To: Philipp Zabel; +Cc: kernel, dri-devel, patchwork-lst

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@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;
+
+		pre->cur.modifier = modifier;
+	}
+
 	do {
 		if (time_after(jiffies, timeout)) {
 			dev_warn(pre->dev, "timeout waiting for PRE safe window\n");
@@ -255,9 +291,10 @@ void ipu_pre_update(struct ipu_pre *pre, unsigned int bufaddr)
 		current_yblock =
 			(val >> IPU_PRE_STORE_ENG_STATUS_STORE_BLOCK_Y_SHIFT) &
 			IPU_PRE_STORE_ENG_STATUS_STORE_BLOCK_Y_MASK;
-	} while (current_yblock == 0 || current_yblock >= pre->cur.safe_window_end);
+	} while (current_yblock == 0 || current_yblock >= safe_window_end);
 
-	writel(IPU_PRE_CTRL_SDW_UPDATE, pre->regs + IPU_PRE_CTRL_SET);
+	writel(pre->cur.ctrl | IPU_PRE_CTRL_SDW_UPDATE,
+	       pre->regs + IPU_PRE_CTRL);
 }
 
 bool ipu_pre_update_pending(struct ipu_pre *pre)
diff --git a/drivers/gpu/ipu-v3/ipu-prg.c b/drivers/gpu/ipu-v3/ipu-prg.c
index 196797c1b4b3..eb8cfcd9f5b4 100644
--- a/drivers/gpu/ipu-v3/ipu-prg.c
+++ b/drivers/gpu/ipu-v3/ipu-prg.c
@@ -287,7 +287,7 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
 	chan = &prg->chan[prg_chan];
 
 	if (chan->enabled) {
-		ipu_pre_update(prg->pres[chan->used_pre], *eba);
+		ipu_pre_update(prg->pres[chan->used_pre], modifier, *eba);
 		return 0;
 	}
 
diff --git a/drivers/gpu/ipu-v3/ipu-prv.h b/drivers/gpu/ipu-v3/ipu-prv.h
index 291ac1bab66d..24f05ff3c047 100644
--- a/drivers/gpu/ipu-v3/ipu-prv.h
+++ b/drivers/gpu/ipu-v3/ipu-prv.h
@@ -262,7 +262,7 @@ u32 ipu_pre_get_baddr(struct ipu_pre *pre);
 void ipu_pre_configure(struct ipu_pre *pre, unsigned int width,
 		       unsigned int height, unsigned int stride, u32 format,
 		       uint64_t modifier, unsigned int bufaddr);
-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);
 bool ipu_pre_update_pending(struct ipu_pre *pre);
 
 struct ipu_prg *ipu_prg_lookup_by_phandle(struct device *dev, const char *name,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] gpu: ipu-v3: pre: add dynamic buffer layout reconfiguration
  2023-01-26 18:47 ` [PATCH 2/2] gpu: ipu-v3: pre: add dynamic buffer layout reconfiguration Lucas Stach
@ 2023-01-27 10:20   ` Philipp Zabel
  0 siblings, 0 replies; 4+ messages in thread
From: Philipp Zabel @ 2023-01-27 10:20 UTC (permalink / raw)
  To: Lucas Stach; +Cc: kernel, dri-devel, patchwork-lst

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@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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] gpu: ipu-v3: pre: move state into struct
  2023-01-26 18:47 [PATCH 1/2] gpu: ipu-v3: pre: move state into struct Lucas Stach
  2023-01-26 18:47 ` [PATCH 2/2] gpu: ipu-v3: pre: add dynamic buffer layout reconfiguration Lucas Stach
@ 2023-01-27 10:20 ` Philipp Zabel
  1 sibling, 0 replies; 4+ messages in thread
From: Philipp Zabel @ 2023-01-27 10:20 UTC (permalink / raw)
  To: Lucas Stach; +Cc: kernel, dri-devel, patchwork-lst

On Thu, Jan 26, 2023 at 07:47:43PM +0100, Lucas Stach wrote:
> Move the variables tracking the current dynamic state into a struct
> to separate it a bit better from the static device properties.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-01-27 10:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-26 18:47 [PATCH 1/2] gpu: ipu-v3: pre: move state into struct Lucas Stach
2023-01-26 18:47 ` [PATCH 2/2] gpu: ipu-v3: pre: add dynamic buffer layout reconfiguration Lucas Stach
2023-01-27 10:20   ` Philipp Zabel
2023-01-27 10:20 ` [PATCH 1/2] gpu: ipu-v3: pre: move state into struct Philipp Zabel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.