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;
MIME-Version: 1.0
References: <1561112433-5308-1-git-send-email-raymond.smith@arm.com>
In-Reply-To: <1561112433-5308-1-git-send-email-raymond.smith@arm.com>
From:   Daniel Vetter <daniel@ffwll.ch>
Date:   Fri, 21 Jun 2019 17:27:00 +0200
Message-ID: <CAKMK7uEjh+GrSy5AgbVLVQd1S5oJ8KFiWEUmxtMMVdcMSBtdCQ@mail.gmail.com>
Subject: Re: [PATCH] drm/fourcc: Add Arm 16x16 block modifier
To:     Raymond Smith <Raymond.Smith@arm.com>
Cc:     "maarten.lankhorst@linux.intel.com" 
        <maarten.lankhorst@linux.intel.com>,
        "maxime.ripard@bootlin.com" <maxime.ripard@bootlin.com>,
        "sean@poorly.run" <sean@poorly.run>,
        "airlied@linux.ie" <airlied@linux.ie>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "yuq825@gmail.com" <yuq825@gmail.com>,
        Ayan Halder <Ayan.Halder@arm.com>,
        Brian Starkey <Brian.Starkey@arm.com>,
        "malidp@foss.arm.com" <malidp@foss.arm.com>, nd <nd@arm.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 21, 2019 at 12:21 PM Raymond Smith <Raymond.Smith@arm.com> wrote:
>
> Add the DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED modifier to
> denote the 16x16 block u-interleaved format used in Arm Utgard and
> Midgard GPUs.
>
> Signed-off-by: Raymond Smith <raymond.smith@arm.com>
> ---
>  include/uapi/drm/drm_fourcc.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3..8ed7ecf 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -743,6 +743,16 @@ extern "C" {
>  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
>
>  /*
> + * Arm 16x16 Block U-Interleaved modifier
> + *
> + * This is used by Arm Mali Utgard and Midgard GPUs. It divides the image
> + * into 16x16 pixel blocks. Blocks are stored linearly in order, but pixels
> + * in the block are reordered.
> + */
> +#define DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED \
> +       fourcc_mod_code(ARM, ((1ULL << 55) | 1))

This seems to be an extremely random pick for a new number. What's the
thinking here? Aside from "doesnt match any of the afbc combos" ofc.
If you're already up to having thrown away 55bits, then it's not going
to last long really :-)

I think a good idea would be to reserve a bunch of the high bits as
some form of index (afbc would get index 0 for backwards compat). And
then the lower bits would be for free use for a given index/mode. And
the first mode is probably an enumeration, where possible modes simple
get enumerated without further flags or anything.

The other bit: Would be real good to define the format a bit more
precisely, including the layout within the tile.

Also ofc needs acks from lima/panfrost people since I assume they'll
be using this, too.

Thanks, Daniel

> +
> +/*
>   * Allwinner tiled modifier
>   *
>   * This tiling mode is implemented by the VPU found on all Allwinner platforms,
> --
> 2.7.4
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch