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

[Daniel Stone]

Hi,

On Wed, 12 Aug 2020 at 08:05, Alyssa Rosenzweig
<alyssa.rosenzweig at collabora.com> 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