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

From: Brian Starkey <brian.starkey@arm.com>
To: Daniel Stone <daniel@fooishbar.org>
Cc: Sandy Huang <hjc@rock-chips.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	linux-rockchip <linux-rockchip@lists.infradead.org>,
	Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>,
	nd <nd@arm.com>
Subject: Re: [PATCH] drm/rockchip: Require the YTR modifier for AFBC
Date: Tue, 23 Feb 2021 16:53:48 +0000	[thread overview]
Message-ID: <20210223165348.edghgglgx4g2lvfw@DESKTOP-E1NTVVP.localdomain> (raw)
In-Reply-To: <CAPj87rMkSdtrHnoLzK6zAVvST2KH8SprNnp5bS92qpr84g=fPg@mail.gmail.com>

On Tue, Feb 23, 2021 at 03:29:13PM +0000, Daniel Stone wrote:
> On Tue, 23 Feb 2021 at 14:58, Brian Starkey <brian.starkey@arm.com> wrote:
> > On Tue, Feb 23, 2021 at 02:27:11PM +0000, Daniel Stone wrote:
> > > 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?
> >
> > Drive-by $0.02:
> >
> > As described in https://www.kernel.org/doc/Documentation/gpu/afbc.rst,
> > YTR is only valid for "BGR" component order, so this shouldn't be set
> > or used for "RGB" order (or YUV) formats. For BGR-order formats, it's
> > best to always enable YTR to get the best compression ratio.
> >
> > In an ideal world, there wouldn't be hardware/drivers which
> > support/allow non-standard encodings, but we don't live in that world
> > :-(
> 
> This implies that RGB component ordering cannot be used together with
> AFBC on RK3399, no?

If YTR can't be turned off, then according to the AFBC spec - correct.

If the hardware allows it to be configured to use YTR with other
component orders, I don't know exactly what the impact would be -
maybe nothing.

I raised some of these concerns when the patches were first sent:
https://lore.kernel.org/dri-devel/20190925093913.z4vduybwcokn3awi@DESKTOP-E1NTVVP.localdomain/

We wrote the .rst doc to try and avoid incompatibilities between
different drivers and parts of the stack. BGR is Arm's preferred order
for AFBC. Unfortunately amongst shifting organisations and priorities,
AFBC in upstream hasn't got much attention.

Cheers,
-Brian

> 
> Cheers,
> Daniel

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip