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

From: Daniel Vetter <daniel@ffwll.ch>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: heiko@sntech.de, airlied@linux.ie,
	dri-devel@lists.freedesktop.org, hjc@rock-chips.com,
	stable@vger.kernel.org, andrzej.p@collabora.com,
	linux-rockchip@lists.infradead.org,
	Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>,
	daniel@ffwll.ch, linux-arm-kernel@lists.infradead.org,
	daniels@collabora.com
Subject: Re: [PATCH] drm/rockchip: Require the YTR modifier for AFBC
Date: Wed, 12 Aug 2020 16:11:37 +0200	[thread overview]
Message-ID: <20200812141137.GH2352366@phenom.ffwll.local> (raw)
In-Reply-To: <20200812063154.GB1300894@kroah.com>

On Wed, Aug 12, 2020 at 08:31:54AM +0200, Greg KH wrote:
> On Tue, Aug 11, 2020 at 04:26:31PM -0400, Alyssa Rosenzweig 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.
> > 
> > Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> > ---
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > index 4a2099cb5..857d97cdc 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > @@ -17,9 +17,20 @@
> >  
> >  #define NUM_YUV2YUV_COEFFICIENTS 12
> >  
> > +/* AFBC supports a number of configurable modes. Relevant to us is block size
> > + * (16x16 or 32x8), storage modifiers (SPARSE, SPLIT), and the YUV-like
> > + * colourspace transform (YTR). 16x16 SPARSE mode is always used. SPLIT mode
> > + * could be enabled via the hreg_block_split register, but is not currently
> > + * handled. The colourspace transform is implicitly always assumed by the
> > + * decoder, so consumers must use this transform as well.
> > + *
> > + * Failure to match modifiers will cause errors displaying AFBC buffers
> > + * produced by conformant AFBC producers, including Mesa.
> > + */
> >  #define ROCKCHIP_AFBC_MOD \
> >  	DRM_FORMAT_MOD_ARM_AFBC( \
> >  		AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE \
> > +			| AFBC_FORMAT_MOD_YTR \
> >  	)
> >  
> >  enum vop_data_format {
> > -- 
> > 2.28.0
> > 
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.

Greg's bot wants a cc: stable on the commit (i.e. in the commit message),
otherwise it's lost since it doesn't track what's all submitted to it
before it's merged.
-Daniel

> 
> </formletter>

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel