drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
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
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
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
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
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
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
© 2016 - 2025 Red Hat, Inc.