Re: [PATCH] drm/rockchip: Require the YTR modifier for AFBC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Daniel,

    RK3399 and px30 can support YTR afbc format[RGB only], there is an hidden control bit to control this.

Hi Alyssa,

    Can you add the following patch to test on your platform? thanks.

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 99bdb5a2a185..0780ad46230a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -105,7 +105,7 @@
 #define AFBC_FMT_U8U8U8U8      0x5
 #define AFBC_FMT_U8U8U8                0x4

-#define AFBC_TILE_16x16                BIT(4)
+#define AFBC_FMT_YTR           BIT(4)

 /*
  * The coefficients of the following matrix are all fixed points.
@@ -952,7 +952,9 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
        if (rockchip_afbc(fb->modifier)) {
                int afbc_format = vop_convert_afbc_format(fb->format->format);

-               VOP_AFBC_SET(vop, format, afbc_format | AFBC_TILE_16x16);
+               if (fb->modifier & AFBC_FORMAT_MOD_YTR)
+                       afbc_format |= AFBC_FMT_YTR;
+               VOP_AFBC_SET(vop, format, afbc_format);
                VOP_AFBC_SET(vop, hreg_block_split, 0);
                VOP_AFBC_SET(vop, win_sel, VOP_WIN_TO_INDEX(vop_win));
                VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 4a2099cb582e..48e131b88c23 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -20,6 +20,7 @@
 #define ROCKCHIP_AFBC_MOD \
        DRM_FORMAT_MOD_ARM_AFBC( \
                AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE \
+                       | AFBC_FORMAT_MOD_YTR \
        )

在 2021/2/23 22:27, Daniel Stone 写道:
Hi,

On Wed, 12 Aug 2020 at 08:05, Alyssa Rosenzweig
<alyssa.rosenzweig@xxxxxxxxxxxxx> wrote:
The AFBC decoder used in the Rockchip VOP assumes the use of the
YUV-like colourspace transform (YTR). YTR is lossless for RGB(A)
buffers, which covers the RGBA8 and RGB565 formats supported in
vop_convert_afbc_format. Use of YTR is signaled with the
AFBC_FORMAT_MOD_YTR modifier, which prior to this commit was missing. As
such, a producer would have to generate buffers that do not use YTR,
which the VOP would erroneously decode as YTR, leading to severe visual
corruption.

The upstream AFBC support was developed against a captured frame, which
failed to exercise modifier support. Prior to bring-up of AFBC in Mesa
(in the Panfrost driver), no open userspace respected modifier
reporting. As such, this change is not expected to affect broken
userspaces.

Tested on RK3399 with Panfrost and Weston.
Bumping this one: it seems like the Rockchip VOP either always applies
the YTR transform, or has a YTR control bit which is not documented in
the driver's register definitions. This means that it is incorrect to
advertise the currently-used modifier, which specifies that YTR is
_not_ used, and doing so breaks Panfrost which correctly uses the
modifier as documented. Based on our knowledge of Mali, we believe
that Panfrost is correct, and the error lies with Rockchip erroneously
using the YTR transform in the VOP's AFBC decoder despite declaring
through the modifier that YTR is not in use.

Looking at the downstream vendor tree, VOP2 as used in newer SoCs has
explicit control bits for YTR and other AFBC knobs, but this has been
substantially reworked from the original VOP and is not applicable to
this IP block.

Mark, or others from Rockchip, can you please:
- explain if there is a way to enable/disable the YTR transform in the
VOP's AFBC decoder, similar to the split-block control bit?
- ack this patch which correctly declares that the YTR transform is in
use in order to make Panfrost work, so it can be merged through
drm-misc, or provide another solution which fixes this API mistake?
- if VOP does have a hidden YTR-disable bit, add support to disable
YTR so rockchip-drm can continue advertising the non-YTR modifier, and
Cc this patch for backporting to every kernel tree which declares AFBC
modifier support?

Thanks in advance.

Cheers,
Daniel



--
Best Regard

黄家钗
Sandy Huang
Addr: 福州市鼓楼区铜盘路软件大道89号福州软件园A区21号楼(350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
Tel:+86 0591-87884919  8690
E-mail:hjc@xxxxxxxxxxxxxx



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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux