Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jani Nikula <jani.nikula@linux.intel.com>
To:     Liviu Dudau <Liviu.Dudau@arm.com>,
        Ezequiel Garcia <ezequiel@collabora.com>
Cc:     nd <nd@arm.com>,
        "linux-doc\@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "arnd\@arndb.de" <arnd@arndb.de>,
        "corbet\@lwn.net" <corbet@lwn.net>,
        "airlied\@linux.ie" <airlied@linux.ie>,
        "gregkh\@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "nicolas.ferre\@microchip.com" <nicolas.ferre@microchip.com>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "dri-devel\@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "davem\@davemloft.net" <davem@davemloft.net>,
        "maxime.ripard\@bootlin.com" <maxime.ripard@bootlin.com>,
        "malidp\@foss.arm.com" <malidp@foss.arm.com>,
        "mchehab+samsung\@kernel.org" <mchehab+samsung@kernel.org>,
        "akpm\@linux-foundation.org" <akpm@linux-foundation.org>,
        Ayan Halder <Ayan.Halder@arm.com>,
        "sean\@poorly.run" <sean@poorly.run>
Subject: Re: [RFC AFBC 03/12] drm/afbc: Add AFBC modifier usage documentation
In-Reply-To: <20190111180758.GK20661@e110455-lin.cambridge.arm.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
References: <1543836703-8491-1-git-send-email-ayan.halder@arm.com> <1543836703-8491-4-git-send-email-ayan.halder@arm.com> <2158b458c240874e5c974a2cae374c3fbf682e48.camel@collabora.com> <20190111180758.GK20661@e110455-lin.cambridge.arm.com>
Date:   Mon, 14 Jan 2019 14:23:46 +0200
Message-ID: <871s5f5sx9.fsf@intel.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 11 Jan 2019, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Thu, Jan 03, 2019 at 05:44:26PM -0300, Ezequiel Garcia wrote:
>> Hi Liviu,
>> 
>> On Mon, 2018-12-03 at 11:31 +0000, Ayan Halder wrote:
>> > From: Brian Starkey <brian.starkey@arm.com>
>> > 
>> > AFBC is a flexible, proprietary, lossless compression protocol and
>> > format, with a number of defined DRM format modifiers. To facilitate
>> > consistency and compatibility between different AFBC producers and
>> > consumers, document the expectations for usage of the AFBC DRM format
>> > modifiers in a new .rst chapter.
>> > 
>> > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
>> > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
>> > ---
>> 
>> I can't find this commit anywhere. Did you decide to reject
>> this or perhaps it just fell thru the cracks?
>
> Cracks have opened wide enough to let this through, sorry about that!
>
> I've now sent a pull request to get it merged.

Okay, so this is a very late comment, so feel free to ignore or,
perhaps, add a change on top.

Documentation/gpu mostly contains files that document high level stuff,
mostly one file per driver (with names matching the directories under
drivers/gpu/drm) or one file per drm core functional area.

Perhaps start an arm.rst, or at least name it more descriptively, say
arm-fbc.rst? Contrast msm-crash-dump.rst.

BR,
Jani.


>
> Best regards,
> Liviu
>
>> 
>> Thanks!
>> Ezequiel
>> 
>> 
>> >  Documentation/gpu/afbc.rst    | 233 ++++++++++++++++++++++++++++++++++++++++++
>> >  Documentation/gpu/drivers.rst |   1 +
>> >  MAINTAINERS                   |   1 +
>> >  include/uapi/drm/drm_fourcc.h |   3 +
>> >  4 files changed, 238 insertions(+)
>> >  create mode 100644 Documentation/gpu/afbc.rst
>> > 
>> > diff --git a/Documentation/gpu/afbc.rst b/Documentation/gpu/afbc.rst
>> > new file mode 100644
>> > index 0000000..922d955
>> > --- /dev/null
>> > +++ b/Documentation/gpu/afbc.rst
>> > @@ -0,0 +1,233 @@
>> > +===================================
>> > + Arm Framebuffer Compression (AFBC)
>> > +===================================
>> > +
>> > +AFBC is a proprietary lossless image compression protocol and format.
>> > +It provides fine-grained random access and minimizes the amount of
>> > +data transferred between IP blocks.
>> > +
>> > +AFBC can be enabled on drivers which support it via use of the AFBC
>> > +format modifiers defined in drm_fourcc.h. See DRM_FORMAT_MOD_ARM_AFBC(*).
>> > +
>> > +All users of the AFBC modifiers must follow the usage guidelines laid
>> > +out in this document, to ensure compatibility across different AFBC
>> > +producers and consumers.
>> > +
>> > +Components and Ordering
>> > +=======================
>> > +
>> > +AFBC streams can contain several components - where a component
>> > +corresponds to a color channel (i.e. R, G, B, X, A, Y, Cb, Cr).
>> > +The assignment of input/output color channels must be consistent
>> > +between the encoder and the decoder for correct operation, otherwise
>> > +the consumer will interpret the decoded data incorrectly.
>> > +
>> > +Furthermore, when the lossless colorspace transform is used
>> > +(AFBC_FORMAT_MOD_YTR, which should be enabled for RGB buffers for
>> > +maximum compression efficiency), the component order must be:
>> > +
>> > + * Component 0: R
>> > + * Component 1: G
>> > + * Component 2: B
>> > +
>> > +The component ordering is communicated via the fourcc code in the
>> > +fourcc:modifier pair. In general, component '0' is considered to
>> > +reside in the least-significant bits of the corresponding linear
>> > +format. For example, COMP(bits):
>> > +
>> > + * DRM_FORMAT_ABGR8888
>> > +
>> > +   * Component 0: R(8)
>> > +   * Component 1: G(8)
>> > +   * Component 2: B(8)
>> > +   * Component 3: A(8)
>> > +
>> > + * DRM_FORMAT_BGR888
>> > +
>> > +   * Component 0: R(8)
>> > +   * Component 1: G(8)
>> > +   * Component 2: B(8)
>> > +
>> > + * DRM_FORMAT_YUYV
>> > +
>> > +   * Component 0: Y(8)
>> > +   * Component 1: Cb(8, 2x1 subsampled)
>> > +   * Component 2: Cr(8, 2x1 subsampled)
>> > +
>> > +In AFBC, 'X' components are not treated any differently from any other
>> > +component. Therefore, an AFBC buffer with fourcc DRM_FORMAT_XBGR8888
>> > +encodes with 4 components, like so:
>> > +
>> > + * DRM_FORMAT_XBGR8888
>> > +
>> > +   * Component 0: R(8)
>> > +   * Component 1: G(8)
>> > +   * Component 2: B(8)
>> > +   * Component 3: X(8)
>> > +
>> > +Please note, however, that the inclusion of a "wasted" 'X' channel is
>> > +bad for compression efficiency, and so it's recommended to avoid
>> > +formats containing 'X' bits. If a fourth component is
>> > +required/expected by the encoder/decoder, then it is recommended to
>> > +instead use an equivalent format with alpha, setting all alpha bits to
>> > +'1'. If there is no requirement for a fourth component, then a format
>> > +which doesn't include alpha can be used, e.g. DRM_FORMAT_BGR888.
>> > +
>> > +Number of Planes
>> > +================
>> > +
>> > +Formats which are typically multi-planar in linear layouts (e.g. YUV
>> > +420), can be encoded into one, or multiple, AFBC planes. As with
>> > +component order, the encoder and decoder must agree about the number
>> > +of planes in order to correctly decode the buffer. The fourcc code is
>> > +used to determine the number of encoded planes in an AFBC buffer,
>> > +matching the number of planes for the linear (unmodified) format.
>> > +Within each plane, the component ordering also follows the fourcc
>> > +code:
>> > +
>> > +For example:
>> > +
>> > + * DRM_FORMAT_YUYV: nplanes = 1
>> > +
>> > +   * Plane 0:
>> > +
>> > +     * Component 0: Y(8)
>> > +     * Component 1: Cb(8, 2x1 subsampled)
>> > +     * Component 2: Cr(8, 2x1 subsampled)
>> > +
>> > + * DRM_FORMAT_NV12: nplanes = 2
>> > +
>> > +   * Plane 0:
>> > +
>> > +     * Component 0: Y(8)
>> > +
>> > +   * Plane 1:
>> > +
>> > +     * Component 0: Cb(8, 2x1 subsampled)
>> > +     * Component 1: Cr(8, 2x1 subsampled)
>> > +
>> > +Cross-device interoperability
>> > +=============================
>> > +
>> > +For maximum compatibility across devices, the table below defines
>> > +canonical formats for use between AFBC-enabled devices. Formats which
>> > +are listed here must be used exactly as specified when using the AFBC
>> > +modifiers. Formats which are not listed should be avoided.
>> > +
>> > +.. flat-table:: AFBC formats
>> > +
>> > +   * - Fourcc code
>> > +     - Description
>> > +     - Planes/Components
>> > +
>> > +   * - DRM_FORMAT_ABGR2101010
>> > +     - 10-bit per component RGB, with 2-bit alpha
>> > +     - Plane 0: 4 components
>> > +              * Component 0: R(10)
>> > +              * Component 1: G(10)
>> > +              * Component 2: B(10)
>> > +              * Component 3: A(2)
>> > +
>> > +   * - DRM_FORMAT_ABGR8888
>> > +     - 8-bit per component RGB, with 8-bit alpha
>> > +     - Plane 0: 4 components
>> > +              * Component 0: R(8)
>> > +              * Component 1: G(8)
>> > +              * Component 2: B(8)
>> > +              * Component 3: A(8)
>> > +
>> > +   * - DRM_FORMAT_BGR888
>> > +     - 8-bit per component RGB
>> > +     - Plane 0: 3 components
>> > +              * Component 0: R(8)
>> > +              * Component 1: G(8)
>> > +              * Component 2: B(8)
>> > +
>> > +   * - DRM_FORMAT_BGR565
>> > +     - 5/6-bit per component RGB
>> > +     - Plane 0: 3 components
>> > +              * Component 0: R(5)
>> > +              * Component 1: G(6)
>> > +              * Component 2: B(5)
>> > +
>> > +   * - DRM_FORMAT_ABGR1555
>> > +     - 5-bit per component RGB, with 1-bit alpha
>> > +     - Plane 0: 4 components
>> > +              * Component 0: R(5)
>> > +              * Component 1: G(5)
>> > +              * Component 2: B(5)
>> > +              * Component 3: A(1)
>> > +
>> > +   * - DRM_FORMAT_VUY888
>> > +     - 8-bit per component YCbCr 444, single plane
>> > +     - Plane 0: 3 components
>> > +              * Component 0: Y(8)
>> > +              * Component 1: Cb(8)
>> > +              * Component 2: Cr(8)
>> > +
>> > +   * - DRM_FORMAT_VUY101010
>> > +     - 10-bit per component YCbCr 444, single plane
>> > +     - Plane 0: 3 components
>> > +              * Component 0: Y(10)
>> > +              * Component 1: Cb(10)
>> > +              * Component 2: Cr(10)
>> > +
>> > +   * - DRM_FORMAT_YUYV
>> > +     - 8-bit per component YCbCr 422, single plane
>> > +     - Plane 0: 3 components
>> > +              * Component 0: Y(8)
>> > +              * Component 1: Cb(8, 2x1 subsampled)
>> > +              * Component 2: Cr(8, 2x1 subsampled)
>> > +
>> > +   * - DRM_FORMAT_NV16
>> > +     - 8-bit per component YCbCr 422, two plane
>> > +     - Plane 0: 1 component
>> > +              * Component 0: Y(8)
>> > +       Plane 1: 2 components
>> > +              * Component 0: Cb(8, 2x1 subsampled)
>> > +              * Component 1: Cr(8, 2x1 subsampled)
>> > +
>> > +   * - DRM_FORMAT_Y210
>> > +     - 10-bit per component YCbCr 422, single plane
>> > +     - Plane 0: 3 components
>> > +              * Component 0: Y(10)
>> > +              * Component 1: Cb(10, 2x1 subsampled)
>> > +              * Component 2: Cr(10, 2x1 subsampled)
>> > +
>> > +   * - DRM_FORMAT_P210
>> > +     - 10-bit per component YCbCr 422, two plane
>> > +     - Plane 0: 1 component
>> > +              * Component 0: Y(10)
>> > +       Plane 1: 2 components
>> > +              * Component 0: Cb(10, 2x1 subsampled)
>> > +              * Component 1: Cr(10, 2x1 subsampled)
>> > +
>> > +   * - DRM_FORMAT_YUV420_8BIT
>> > +     - 8-bit per component YCbCr 420, single plane
>> > +     - Plane 0: 3 components
>> > +              * Component 0: Y(8)
>> > +              * Component 1: Cb(8, 2x2 subsampled)
>> > +              * Component 2: Cr(8, 2x2 subsampled)
>> > +
>> > +   * - DRM_FORMAT_YUV420_10BIT
>> > +     - 10-bit per component YCbCr 420, single plane
>> > +     - Plane 0: 3 components
>> > +              * Component 0: Y(10)
>> > +              * Component 1: Cb(10, 2x2 subsampled)
>> > +              * Component 2: Cr(10, 2x2 subsampled)
>> > +
>> > +   * - DRM_FORMAT_NV12
>> > +     - 8-bit per component YCbCr 420, two plane
>> > +     - Plane 0: 1 component
>> > +              * Component 0: Y(8)
>> > +       Plane 1: 2 components
>> > +              * Component 0: Cb(8, 2x2 subsampled)
>> > +              * Component 1: Cr(8, 2x2 subsampled)
>> > +
>> > +   * - DRM_FORMAT_P010
>> > +     - 10-bit per component YCbCr 420, two plane
>> > +     - Plane 0: 1 component
>> > +              * Component 0: Y(10)
>> > +       Plane 1: 2 components
>> > +              * Component 0: Cb(10, 2x2 subsampled)
>> > +              * Component 1: Cr(10, 2x2 subsampled)
>> > diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
>> > index 7c16721..c176b34 100644
>> > --- a/Documentation/gpu/drivers.rst
>> > +++ b/Documentation/gpu/drivers.rst
>> > @@ -17,6 +17,7 @@ GPU Driver Documentation
>> >     vkms
>> >     bridge/dw-hdmi
>> >     xen-front
>> > +   afbc
>> >  
>> >  .. only::  subproject and html
>> >  
>> > diff --git a/MAINTAINERS b/MAINTAINERS
>> > index 254b7b2..aef18e3 100644
>> > --- a/MAINTAINERS
>> > +++ b/MAINTAINERS
>> > @@ -1131,6 +1131,7 @@ M:	Mali DP Maintainers <malidp@foss.arm.com>
>> >  S:	Supported
>> >  F:	drivers/gpu/drm/arm/
>> >  F:	Documentation/devicetree/bindings/display/arm,malidp.txt
>> > +F:	Documentation/gpu/afbc.rst
>> >  
>> >  ARM MFM AND FLOPPY DRIVERS
>> >  M:	Ian Molton <spyro@f2s.com>
>> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> > index 75c4b5a..0adde4d 100644
>> > --- a/include/uapi/drm/drm_fourcc.h
>> > +++ b/include/uapi/drm/drm_fourcc.h
>> > @@ -597,6 +597,9 @@ extern "C" {
>> >   * AFBC has several features which may be supported and/or used, which are
>> >   * represented using bits in the modifier. Not all combinations are valid,
>> >   * and different devices or use-cases may support different combinations.
>> > + *
>> > + * Further information on the use of AFBC modifiers can be found in
>> > + * Documentation/gpu/afbc.rst
>> >   */
>> >  #define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode)	fourcc_mod_code(ARM, __afbc_mode)
>> >  
>> 
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center