[PATCH v2] (drm/rockchip) Reject AFBC for resolutions >2560x1600

[PATCH v2] (drm/rockchip) Reject AFBC for resolutions >2560x1600

[Konstantin Shabanov]

As it isn't supported by hardware. At least, RK3399 doesn't support
it. From the datasheet[1]
("1.2.10 Video IN/OUT", "Display Interface", p. 17):

  Support AFBC function co-operation with GPU
    * support 2560x1600 UI

Manually tested on RockPro64 (rk3399):
- ARM_AFBC modifier is used for 1920x1080
- DRM_FORMAT_MOD_LINEAR modifier us used for 3840x2160
- No noise on the screen when sway is running in 4k
- Dynamic resolution switching works correctly in sway

Signed-off-by: Konstantin Shabanov <mail@etehtsea.me>
Cc: Daniel Stone <daniel@fooishbar.org>
Reported-by: Dan Callaghan <djc@djc.id.au>
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7968

[1]: https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index dcc1f07632c3..1379bc3cd937 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -18,6 +18,8 @@
 #include "rockchip_drm_fb.h"
 #include "rockchip_drm_gem.h"

+#define ROCKCHIP_AFBC_MAX_WIDTH		2560
+
 static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
 	.destroy       = drm_gem_fb_destroy,
 	.create_handle = drm_gem_fb_create_handle,
@@ -52,6 +54,13 @@ rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
 	}

 	if (drm_is_afbc(mode_cmd->modifier[0])) {
+		if (mode_cmd->width > ROCKCHIP_AFBC_MAX_WIDTH) {
+			DRM_DEBUG_KMS("AFBC is not supported for the width %d (max %d)\n",
+				      mode_cmd->width,
+				      ROCKCHIP_AFBC_MAX_WIDTH);
+			return ERR_PTR(-EINVAL);
+		};
+
 		int ret, i;

 		ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);

base-commit: e7bb7d44c3b97aea1f0e354c6499900154ac67f2
--
2.48.1

Re:[PATCH v2] (drm/rockchip) Reject AFBC for resolutions >2560x1600

[Andy Yan]

Hi Konstantin，

the Subject should be: drm/rockchip: xxxxxxxxxxxx

At 2025-04-14 17:53:31, "Konstantin Shabanov" <mail@etehtsea.me> wrote:
>As it isn't supported by hardware. At least, RK3399 doesn't support
>it. From the datasheet[1]
>("1.2.10 Video IN/OUT", "Display Interface", p. 17):
>
>  Support AFBC function co-operation with GPU
>    * support 2560x1600 UI
>
>Manually tested on RockPro64 (rk3399):
>- ARM_AFBC modifier is used for 1920x1080
>- DRM_FORMAT_MOD_LINEAR modifier us used for 3840x2160
>- No noise on the screen when sway is running in 4k
>- Dynamic resolution switching works correctly in sway
>
>Signed-off-by: Konstantin Shabanov <mail@etehtsea.me>
>Cc: Daniel Stone <daniel@fooishbar.org>
>Reported-by: Dan Callaghan <djc@djc.id.au>
>Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7968
>
>[1]: https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
>---
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>index dcc1f07632c3..1379bc3cd937 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>@@ -18,6 +18,8 @@
> #include "rockchip_drm_fb.h"
> #include "rockchip_drm_gem.h"
>
>+#define ROCKCHIP_AFBC_MAX_WIDTH		2560
>+
> static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
> 	.destroy       = drm_gem_fb_destroy,
> 	.create_handle = drm_gem_fb_create_handle,
>@@ -52,6 +54,13 @@ rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
> 	}
>
> 	if (drm_is_afbc(mode_cmd->modifier[0])) {
>+		if (mode_cmd->width > ROCKCHIP_AFBC_MAX_WIDTH) {
>+			DRM_DEBUG_KMS("AFBC is not supported for the width %d (max %d)\n",
>+				      mode_cmd->width,
>+				      ROCKCHIP_AFBC_MAX_WIDTH);
>+			return ERR_PTR(-EINVAL);
>+		};

As the commit message saying: This restriction only applies to rk3399.

There is no such limitation for rk356x/rk3588.
But this patch will Reject AFBC for resolutions >2560x1600 on all rockchip platforms,
so this is not right.


>+
> 		int ret, i;
>
> 		ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);
>
>base-commit: e7bb7d44c3b97aea1f0e354c6499900154ac67f2
>--
>2.48.1

[PATCH v3] drm/rockchip: Disable AFBC for res >2560 on rk3399

[Konstantin Shabanov]

As it isn't supported by hardware. At least, RK3399 doesn't support
it. From the datasheet[1]
("1.2.10 Video IN/OUT", "Display Interface", p. 17):

  Support AFBC function co-operation with GPU
    * support 2560x1600 UI

Manually tested on RockPro64 (rk3399):
- ARM_AFBC modifier is used for 1920x1080
- DRM_FORMAT_MOD_LINEAR modifier us used for 3840x2160
- No noise on the screen when sway is running in 4k
- Dynamic resolution switching works correctly in sway

Signed-off-by: Konstantin Shabanov <mail@etehtsea.me>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Andy Yan <andyshrk@163.com>
Reported-by: Dan Callaghan <djc@djc.id.au>
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7968

[1]: https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
---
V2 -> V3: Run check only on rk3399
V1 -> V2: Move the check to the fb_create callback

Hi Andy and Daniel!
Thank you for the review!

Best wishes,
Konstantin

 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index dcc1f07632c3..6ca757f80a30 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -5,6 +5,7 @@
  */

 #include <linux/kernel.h>
+#include <linux/of.h>

 #include <drm/drm.h>
 #include <drm/drm_atomic.h>
@@ -17,6 +18,9 @@
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
 #include "rockchip_drm_gem.h"
+#include "rockchip_drm_vop.h"
+
+#define RK3399_AFBC_MAX_WIDTH		2560

 static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
 	.destroy       = drm_gem_fb_destroy,
@@ -52,6 +56,15 @@ rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
 	}

 	if (drm_is_afbc(mode_cmd->modifier[0])) {
+		if (of_machine_is_compatible("rockchip,rk3399")) {
+			if (mode_cmd->width > RK3399_AFBC_MAX_WIDTH) {
+				DRM_DEBUG_KMS("AFBC is not supported for the width %d (max %d)\n",
+					      mode_cmd->width,
+					      RK3399_AFBC_MAX_WIDTH);
+				return ERR_PTR(-EINVAL);
+			};
+		}
+
 		int ret, i;

 		ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);

base-commit: 4890d68db651562ea80250f2c93205a5c0327a6a
--
2.48.1

[PATCH v4] drm/rockchip: Disable AFBC for res >2560 on rk3399

[Konstantin Shabanov]

As it isn't supported by hardware. At least, RK3399 doesn't support
it. From the datasheet[1]
("1.2.10 Video IN/OUT", "Display Interface", p. 17):

  Support AFBC function co-operation with GPU
    * support 2560x1600 UI

Manually tested on RockPro64 (rk3399):
- ARM_AFBC modifier is used for 1920x1080
- DRM_FORMAT_MOD_LINEAR modifier us used for 3840x2160
- No noise on the screen when sway is running in 4k
- Dynamic resolution switching works correctly in sway

Signed-off-by: Konstantin Shabanov <mail@etehtsea.me>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Andy Yan <andyshrk@163.com>
Reported-by: Dan Callaghan <djc@djc.id.au>
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7968

[1]: https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
---
V3 -> V4: Correct redundant header inclusion
V2 -> V3: Run check only on rk3399
V1 -> V2: Move the check to the fb_create callback

 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index dcc1f07632c3..45e1619b5c97 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -5,6 +5,7 @@
  */

 #include <linux/kernel.h>
+#include <linux/of.h>

 #include <drm/drm.h>
 #include <drm/drm_atomic.h>
@@ -18,6 +19,8 @@
 #include "rockchip_drm_fb.h"
 #include "rockchip_drm_gem.h"

+#define RK3399_AFBC_MAX_WIDTH		2560
+
 static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
 	.destroy       = drm_gem_fb_destroy,
 	.create_handle = drm_gem_fb_create_handle,
@@ -52,6 +55,15 @@ rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
 	}

 	if (drm_is_afbc(mode_cmd->modifier[0])) {
+		if (of_machine_is_compatible("rockchip,rk3399")) {
+			if (mode_cmd->width > RK3399_AFBC_MAX_WIDTH) {
+				DRM_DEBUG_KMS("AFBC is not supported for the width %d (max %d)\n",
+					      mode_cmd->width,
+					      RK3399_AFBC_MAX_WIDTH);
+				return ERR_PTR(-EINVAL);
+			};
+		}
+
 		int ret, i;

 		ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);

base-commit: 4890d68db651562ea80250f2c93205a5c0327a6a
--
2.48.1

Re:[PATCH v4] drm/rockchip: Disable AFBC for res >2560 on rk3399

[Andy Yan]

Hi Konstantin,

At 2025-04-17 14:57:58, "Konstantin Shabanov" <mail@etehtsea.me> wrote:
>As it isn't supported by hardware. At least, RK3399 doesn't support
>it. From the datasheet[1]
>("1.2.10 Video IN/OUT", "Display Interface", p. 17):
>
>  Support AFBC function co-operation with GPU
>    * support 2560x1600 UI
>
>Manually tested on RockPro64 (rk3399):
>- ARM_AFBC modifier is used for 1920x1080
>- DRM_FORMAT_MOD_LINEAR modifier us used for 3840x2160
>- No noise on the screen when sway is running in 4k
>- Dynamic resolution switching works correctly in sway
>
>Signed-off-by: Konstantin Shabanov <mail@etehtsea.me>
>Cc: Daniel Stone <daniel@fooishbar.org>
>Cc: Andy Yan <andyshrk@163.com>
>Reported-by: Dan Callaghan <djc@djc.id.au>
>Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7968
>
>[1]: https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
>---
>V3 -> V4: Correct redundant header inclusion
>V2 -> V3: Run check only on rk3399
>V1 -> V2: Move the check to the fb_create callback
>
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>index dcc1f07632c3..45e1619b5c97 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>@@ -5,6 +5,7 @@
>  */
>
> #include <linux/kernel.h>
>+#include <linux/of.h>
>
> #include <drm/drm.h>
> #include <drm/drm_atomic.h>
>@@ -18,6 +19,8 @@
> #include "rockchip_drm_fb.h"
> #include "rockchip_drm_gem.h"
>
>+#define RK3399_AFBC_MAX_WIDTH		2560
>+
> static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
> 	.destroy       = drm_gem_fb_destroy,
> 	.create_handle = drm_gem_fb_create_handle,
>@@ -52,6 +55,15 @@ rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
> 	}
>
> 	if (drm_is_afbc(mode_cmd->modifier[0])) {
>+		if (of_machine_is_compatible("rockchip,rk3399")) {
>+			if (mode_cmd->width > RK3399_AFBC_MAX_WIDTH) {
>+				DRM_DEBUG_KMS("AFBC is not supported for the width %d (max %d)\n",
>+					      mode_cmd->width,
>+					      RK3399_AFBC_MAX_WIDTH);
>+				return ERR_PTR(-EINVAL);
>+			};
>+		}
>+


I prefer the V1 version PATCH[0]. This is because we do not deal with hardware-related
differences at this level.  It involves a VOP-related restriction and we always  handle
limitiation like this  within the VOP driver . 

[0] https://lore.kernel.org/dri-devel/20250402125320.21836-1-mail@etehtsea.me/


> 		int ret, i;
>
> 		ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);
>
>base-commit: 4890d68db651562ea80250f2c93205a5c0327a6a
>--
>2.48.1

Re: [PATCH v4] drm/rockchip: Disable AFBC for res >2560 on rk3399

[Daniel Stone]

Hi Andy,

On Fri, 18 Apr 2025 at 01:16, Andy Yan <andyshrk@163.com> wrote:
> I prefer the V1 version PATCH[0]. This is because we do not deal with hardware-related
> differences at this level.  It involves a VOP-related restriction and we always  handle
> limitiation like this  within the VOP driver .

As said in the review for v1, this is not enough for generic userspace.

If drmModeAddFB2WithModifiers() fails, userspace knows that the buffer
can never work in that configuration, and it should fall back. If
drmModeAtomicCommit(DRM_MODE_ATOMIC_TEST_ONLY) fails, userspace does
not know that the buffer can _never_ work, because the failure may be
transient or due to a combination of things.

Returning the more localised error saves userspace a lot of work and
enables a more optimal system.

Cheers,
Daniel

Re:Re: [PATCH v4] drm/rockchip: Disable AFBC for res >2560 on rk3399

[Andy Yan]

Hi 
At 2025-04-18 17:43:19, "Daniel Stone" <daniel@fooishbar.org> wrote:
>Hi Andy,
>
>On Fri, 18 Apr 2025 at 01:16, Andy Yan <andyshrk@163.com> wrote:
>> I prefer the V1 version PATCH[0]. This is because we do not deal with hardware-related
>> differences at this level.  It involves a VOP-related restriction and we always  handle
>> limitiation like this  within the VOP driver .
>
>As said in the review for v1, this is not enough for generic userspace.
>
>If drmModeAddFB2WithModifiers() fails, userspace knows that the buffer
>can never work in that configuration, and it should fall back. If
>drmModeAtomicCommit(DRM_MODE_ATOMIC_TEST_ONLY) fails, userspace does
>not know that the buffer can _never_ work, because the failure may be
>transient or due to a combination of things.
>
>Returning the more localised error saves userspace a lot of work and
>enables a more optimal system.

Okay,  fine, ACK.

>
>Cheers,
>Daniel

Re:Re:Re: [PATCH v4] drm/rockchip: Disable AFBC for res >2560 on rk3399

[Konstantin Shabanov]

Hi, Andy and Daniel!

What is the status of this one? I've noticed it went to archived.
Is it good to go or not? Any further recommendations?

Thank you,
Konstantin

[PATCH v5] drm/rockchip: Reject AFBC for res >2560 on rk3399

[Konstantin Shabanov]

As it isn't supported by rk3399. From the datasheet[1]
("1.2.10 Video IN/OUT", "Display Interface", p. 17):

  Support AFBC function co-operation with GPU
    * support 2560x1600 UI

Signed-off-by: Konstantin Shabanov <mail@etehtsea.me>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Andy Yan <andyshrk@163.com>
Reported-by: Dan Callaghan <djc@djc.id.au>
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7968

[1]: https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
---
 V4 -> V5: Extract AFBC support check into drv
 V3 -> V4: Correct redundant header inclusion
 V2 -> V3: Run check only on rk3399
 V1 -> V2: Move the check to the fb_create callback

 AFBC check is implemented in a similar manner as in the malidp driver.
 
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 16 ++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  2 ++
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  3 +++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 180fad5d49ad..9fb04022b2f3 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -42,6 +42,8 @@
 #define DRIVER_MAJOR	1
 #define DRIVER_MINOR	0
 
+#define RK3399_AFBC_MAX_WIDTH		2560
+
 static const struct drm_driver rockchip_drm_driver;
 
 /*
@@ -350,6 +352,20 @@ int rockchip_drm_endpoint_is_subdriver(struct device_node *ep)
 	return false;
 }
 
+bool rockchip_verify_afbc_framebuffer_size(struct drm_device *dev,
+					   const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	if (of_machine_is_compatible("rockchip,rk3399") &&
+	    mode_cmd->width > RK3399_AFBC_MAX_WIDTH) {
+		DRM_DEBUG_KMS("AFBC is not supported for the width %d (max %d)\n",
+			      mode_cmd->width,
+			      RK3399_AFBC_MAX_WIDTH);
+		return false;
+	}
+
+	return true;
+}
+
 static void rockchip_drm_match_remove(struct device *dev)
 {
 	struct device_link *link;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index c183e82a42a5..5dabceaa4fd6 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -86,6 +86,8 @@ int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
 int rockchip_drm_encoder_set_crtc_endpoint_id(struct rockchip_encoder *rencoder,
 					      struct device_node *np, int port, int reg);
 int rockchip_drm_endpoint_is_subdriver(struct device_node *ep);
+bool rockchip_verify_afbc_framebuffer_size(struct drm_device *dev,
+					   const struct drm_mode_fb_cmd2 *mode_cmd);
 extern struct platform_driver cdn_dp_driver;
 extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
 extern struct platform_driver dw_hdmi_qp_rockchip_pltfm_driver;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 5829ee061c61..f0527f12f568 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -52,6 +52,9 @@ rockchip_fb_create(struct drm_device *dev, struct drm_file *file,
 	}
 
 	if (drm_is_afbc(mode_cmd->modifier[0])) {
+		if (!rockchip_verify_afbc_framebuffer_size(dev, mode_cmd))
+			return ERR_PTR(-EINVAL);
+
 		ret = drm_gem_fb_afbc_init(dev, mode_cmd, afbc_fb);
 		if (ret) {
 			drm_framebuffer_put(&afbc_fb->base);

base-commit: 6ad88bf9e74dae83c992d8a16683360117b7e2d8
-- 
2.49.0