diff mbox series

[1/4,v2] drm/mcde: Improve pixel fetcher FIFO depth setting

Message ID 20200808223122.1492124-1-linus.walleij@linaro.org
State Accepted
Commit 44c3867a56e3cdda4b7ea4e4983c4fdafd45b3c9
Headers show
Series [1/4,v2] drm/mcde: Improve pixel fetcher FIFO depth setting | expand

Commit Message

Linus Walleij Aug. 8, 2020, 10:31 p.m. UTC
The pixel fetcher FIFO depth was just hardcoded to 48
which works fine as long as the framebuffer is 32BPP
and the DSI output is RGB888.

We will need more elaborate handling for some buffer
formats and displays, so start to improve this function
by setting reasonable defaults for 32, 24 and 16 BPP
framebuffers.

Cc: newbytee@protonmail.com
Cc: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/gpu/drm/mcde/mcde_display.c | 34 +++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Daniel Vetter Aug. 10, 2020, 12:54 p.m. UTC | #1
On Sun, Aug 09, 2020 at 12:31:19AM +0200, Linus Walleij wrote:
> The pixel fetcher FIFO depth was just hardcoded to 48

> which works fine as long as the framebuffer is 32BPP

> and the DSI output is RGB888.

> 

> We will need more elaborate handling for some buffer

> formats and displays, so start to improve this function

> by setting reasonable defaults for 32, 24 and 16 BPP

> framebuffers.

> 

> Cc: newbytee@protonmail.com

> Cc: Stephan Gerhold <stephan@gerhold.net>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  drivers/gpu/drm/mcde/mcde_display.c | 34 +++++++++++++++++++++++++----

>  1 file changed, 30 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c

> index cac660ac8803..cbc7c0c4955a 100644

> --- a/drivers/gpu/drm/mcde/mcde_display.c

> +++ b/drivers/gpu/drm/mcde/mcde_display.c

> @@ -333,7 +333,7 @@ static void mcde_configure_overlay(struct mcde *mcde, enum mcde_overlay ovl,

>  				   enum mcde_extsrc src,

>  				   enum mcde_channel ch,

>  				   const struct drm_display_mode *mode,

> -				   u32 format)

> +				   u32 format, int cpp)


Note that format->cpp is a bit outdated, since it doesn't work for planar
stuff. But I guess not a problem for you (for now at least). Blocky
formats like yuv don't set format->cpp, or at least not in the way you'd
expect.

Anyway now idea on the hw, but patch looks reasonable.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>


>  {

>  	u32 val;

>  	u32 conf1;

> @@ -342,6 +342,7 @@ static void mcde_configure_overlay(struct mcde *mcde, enum mcde_overlay ovl,

>  	u32 ljinc;

>  	u32 cr;

>  	u32 comp;

> +	u32 pixel_fetcher_watermark;

>  

>  	switch (ovl) {

>  	case MCDE_OVERLAY_0:

> @@ -426,8 +427,33 @@ static void mcde_configure_overlay(struct mcde *mcde, enum mcde_overlay ovl,

>  			format);

>  		break;

>  	}

> -	/* The default watermark level for overlay 0 is 48 */

> -	val |= 48 << MCDE_OVLXCONF2_PIXELFETCHERWATERMARKLEVEL_SHIFT;

> +

> +	/*

> +	 * Pixel fetch watermark level is max 0x1FFF pixels.

> +	 * Two basic rules should be followed:

> +	 * 1. The value should be at least 256 bits.

> +	 * 2. The sum of all active overlays pixelfetch watermark level

> +	 *    multiplied with bits per pixel, should be lower than the

> +	 *    size of input_fifo_size in bits.

> +	 * 3. The value should be a multiple of a line (256 bits).

> +	 */

> +	switch (cpp) {

> +	case 2:

> +		pixel_fetcher_watermark = 128;

> +		break;

> +	case 3:

> +		pixel_fetcher_watermark = 96;

> +		break;

> +	case 4:

> +		pixel_fetcher_watermark = 48;

> +		break;

> +	default:

> +		pixel_fetcher_watermark = 48;

> +		break;

> +	}

> +	dev_dbg(mcde->dev, "pixel fetcher watermark level %d pixels\n",

> +		pixel_fetcher_watermark);

> +	val |= pixel_fetcher_watermark << MCDE_OVLXCONF2_PIXELFETCHERWATERMARKLEVEL_SHIFT;

>  	writel(val, mcde->regs + conf2);

>  

>  	/* Number of bytes to fetch per line */

> @@ -932,7 +958,7 @@ static void mcde_display_enable(struct drm_simple_display_pipe *pipe,

>  	 * channel 0

>  	 */

>  	mcde_configure_overlay(mcde, MCDE_OVERLAY_0, MCDE_EXTSRC_0,

> -			       MCDE_CHANNEL_0, mode, format);

> +			       MCDE_CHANNEL_0, mode, format, cpp);

>  

>  	/*

>  	 * Configure pixel-per-line and line-per-frame for channel 0 and then

> -- 

> 2.26.2

> 

> _______________________________________________

> dri-devel mailing list

> dri-devel@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/dri-devel


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c
index cac660ac8803..cbc7c0c4955a 100644
--- a/drivers/gpu/drm/mcde/mcde_display.c
+++ b/drivers/gpu/drm/mcde/mcde_display.c
@@ -333,7 +333,7 @@  static void mcde_configure_overlay(struct mcde *mcde, enum mcde_overlay ovl,
 				   enum mcde_extsrc src,
 				   enum mcde_channel ch,
 				   const struct drm_display_mode *mode,
-				   u32 format)
+				   u32 format, int cpp)
 {
 	u32 val;
 	u32 conf1;
@@ -342,6 +342,7 @@  static void mcde_configure_overlay(struct mcde *mcde, enum mcde_overlay ovl,
 	u32 ljinc;
 	u32 cr;
 	u32 comp;
+	u32 pixel_fetcher_watermark;
 
 	switch (ovl) {
 	case MCDE_OVERLAY_0:
@@ -426,8 +427,33 @@  static void mcde_configure_overlay(struct mcde *mcde, enum mcde_overlay ovl,
 			format);
 		break;
 	}
-	/* The default watermark level for overlay 0 is 48 */
-	val |= 48 << MCDE_OVLXCONF2_PIXELFETCHERWATERMARKLEVEL_SHIFT;
+
+	/*
+	 * Pixel fetch watermark level is max 0x1FFF pixels.
+	 * Two basic rules should be followed:
+	 * 1. The value should be at least 256 bits.
+	 * 2. The sum of all active overlays pixelfetch watermark level
+	 *    multiplied with bits per pixel, should be lower than the
+	 *    size of input_fifo_size in bits.
+	 * 3. The value should be a multiple of a line (256 bits).
+	 */
+	switch (cpp) {
+	case 2:
+		pixel_fetcher_watermark = 128;
+		break;
+	case 3:
+		pixel_fetcher_watermark = 96;
+		break;
+	case 4:
+		pixel_fetcher_watermark = 48;
+		break;
+	default:
+		pixel_fetcher_watermark = 48;
+		break;
+	}
+	dev_dbg(mcde->dev, "pixel fetcher watermark level %d pixels\n",
+		pixel_fetcher_watermark);
+	val |= pixel_fetcher_watermark << MCDE_OVLXCONF2_PIXELFETCHERWATERMARKLEVEL_SHIFT;
 	writel(val, mcde->regs + conf2);
 
 	/* Number of bytes to fetch per line */
@@ -932,7 +958,7 @@  static void mcde_display_enable(struct drm_simple_display_pipe *pipe,
 	 * channel 0
 	 */
 	mcde_configure_overlay(mcde, MCDE_OVERLAY_0, MCDE_EXTSRC_0,
-			       MCDE_CHANNEL_0, mode, format);
+			       MCDE_CHANNEL_0, mode, format, cpp);
 
 	/*
 	 * Configure pixel-per-line and line-per-frame for channel 0 and then